linux-kernel - Re: [PATCH 4/8] KVM: SVM: move guest vmsave/vmload to assembly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y2ki4Iz8AZzTODKS@hirez.programming.kicks-ass.net>
Date:   Mon, 7 Nov 2022 16:23:12 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        nathan@...nel.org, thomas.lendacky@....com,
        andrew.cooper3@...rix.com, jmattson@...gle.com, seanjc@...gle.com,
        stable@...r.kernel.org
Subject: Re: [PATCH 4/8] KVM: SVM: move guest vmsave/vmload to assembly

On Mon, Nov 07, 2022 at 09:54:32AM -0500, Paolo Bonzini wrote:
> @@ -56,6 +59,16 @@ SYM_FUNC_START(__svm_vcpu_run)
>  	/* Move @svm to RDI. */
>  	mov %_ASM_ARG2, %_ASM_DI
>  
> +	/*
> +	 * Use a single vmcb (vmcb01 because it's always valid) for
> +	 * context switching guest state via VMLOAD/VMSAVE, that way
> +	 * the state doesn't need to be copied between vmcb01 and
> +	 * vmcb02 when switching vmcbs for nested virtualization.
> +	 */
> +	mov SVM_vmcb01_pa(%_ASM_DI), %_ASM_AX
> +1:	vmload %_ASM_AX
> +2:
> +
>  	/* "POP" @vmcb to RAX. */
>  	pop %_ASM_AX
>  
> @@ -80,16 +93,11 @@ SYM_FUNC_START(__svm_vcpu_run)
>  	/* Enter guest mode */
>  	sti
>  
> +3:	vmrun %_ASM_AX
> +4:
> +	cli
>  
> +	/* Pop @svm to RAX while it's the only available register. */
>  	pop %_ASM_AX
>  
>  	/* Save all guest registers.  */

So Andrew noted that once the vmload has executed any exception taken
(say at 3) will crash and burn because %gs is scribbled.

Might be good to make a record of this in the code so it can be cleaned
up some day.

> @@ -159,11 +179,19 @@ SYM_FUNC_START(__svm_vcpu_run)
>  	pop %_ASM_BP
>  	RET
>  
> +10:	cmpb $0, kvm_rebooting
>  	jne 2b
>  	ud2
> +30:	cmpb $0, kvm_rebooting
> +	jne 4b
> +	ud2
> +50:	cmpb $0, kvm_rebooting
> +	jne 6b
> +	ud2
>  
> +	_ASM_EXTABLE(1b, 10b)
> +	_ASM_EXTABLE(3b, 30b)
> +	_ASM_EXTABLE(5b, 50b)