[PATCH 0/8] KVM: SVM: Clean up VMRUN=>#VMEXIT assembly

[PATCH 0/8] KVM: SVM: Clean up VMRUN=>#VMEXIT assembly

[Sean Christopherson]

Clean up SVM's enter/exit assembly code so that it can be compiled
without OBJECT_FILES_NON_STANDARD.  The "standard" __svm_vcpu_run() can't
be made 100% bulletproof, as RBP isn't restored on #VMEXIT, but that's
also the case for __vmx_vcpu_run(), and getting "close enough" is better
than not even trying.

As for SEV-ES, after yet another refresher on swap types, I realized KVM
can simply let the hardware restore registers after #VMEXIT, all that's
missing is storing the current values to the host save area (I learned the
hard way that they are swap Type B, *sigh*).  Unless I'm missing something,
this provides 100% accuracy when using stack frames for unwinding, and
requires less assembly (though probably not fewer code bytes; I didn't check).

In between, build the SEV-ES code iff CONFIG_KVM_AMD_SEV=y, and yank out
"support" for 32-bit kernels, which was unncessarily polluting the code.

I'm pretty sure I actually managed to test all of this, thanks to the SEV-ES
smoke selftests, and a bit of hacking to disable V_SPEC_CTRL, passthrough
SPEC_CTRL unconditionally, and have the selftests W/R SPEC_CTRL from its
guest.

Sean Christopherson (8):
  KVM: SVM: Create a stack frame in __svm_vcpu_run() for unwinding
  KVM: SVM: Wrap __svm_sev_es_vcpu_run() with #ifdef CONFIG_KVM_AMD_SEV
  KVM: SVM: Drop 32-bit "support" from __svm_sev_es_vcpu_run()
  KVM: SVM: Clobber RAX instead of RBX when discarding
    spec_ctrl_intercepted
  KVM: SVM: Save/restore non-volatile GPRs in SEV-ES VMRUN via host save
    area
  KVM: SVM: Save/restore args across SEV-ES VMRUN via host save area
  KVM: SVM: Create a stack frame in __svm_sev_es_vcpu_run()
  KVM: x86: Stop compiling vmenter.S with OBJECT_FILES_NON_STANDARD

 arch/x86/kvm/Makefile      |  4 --
 arch/x86/kvm/svm/svm.c     | 17 ++++---
 arch/x86/kvm/svm/svm.h     |  3 +-
 arch/x86/kvm/svm/vmenter.S | 97 +++++++++++++++++---------------------
 4 files changed, 56 insertions(+), 65 deletions(-)


base-commit: ec1e3d33557babed2c2c2c7da6e84293c2f56f58
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 1/8] KVM: SVM: Create a stack frame in __svm_vcpu_run() for unwinding

[Sean Christopherson]

Unconditionally create a stack frame in __svm_vcpu_run() to play nice with
unwinding via frame pointers, at least until the point where RBP is loaded
with the guest's value.  Don't bother conditioning the code on
CONFIG_FRAME_POINTER=y, as RBP needs to be saved and restored anyways (due
to it being clobbered with the guest's value); omitting the "MOV RSP, RBP"
is not worth the extra #ifdef.

Creating a stack frame will allow removing the OBJECT_FILES_NON_STANDARD
tag from vmenter.S once __svm_sev_es_vcpu_run() is fixed to not stomp all
over RBP for no reason.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/vmenter.S | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S
index 9499f9c6b077..ee5d5a30da88 100644
--- a/arch/x86/kvm/svm/vmenter.S
+++ b/arch/x86/kvm/svm/vmenter.S
@@ -99,6 +99,7 @@
  */
 SYM_FUNC_START(__svm_vcpu_run)
 	push %_ASM_BP
+	mov  %_ASM_SP, %_ASM_BP
 #ifdef CONFIG_X86_64
 	push %r15
 	push %r14
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 2/8] KVM: SVM: Wrap __svm_sev_es_vcpu_run() with #ifdef CONFIG_KVM_AMD_SEV

[Sean Christopherson]

Compile (and link) __svm_sev_es_vcpu_run() if and only if SEV support is
actually enabled.  This will allow dropping non-existent 32-bit "support"
from __svm_sev_es_vcpu_run() without causing undue confusion.

Intentionally don't provide a stub (but keep the declaration), as any sane
compiler, even with things like KASAN enabled, should eliminate the call
to __svm_sev_es_vcpu_run() since sev_es_guest() unconditionally returns
"false" if CONFIG_KVM_AMD_SEV=n.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/vmenter.S | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S
index ee5d5a30da88..7ee363d7517c 100644
--- a/arch/x86/kvm/svm/vmenter.S
+++ b/arch/x86/kvm/svm/vmenter.S
@@ -291,6 +291,7 @@ SYM_FUNC_START(__svm_vcpu_run)
 
 SYM_FUNC_END(__svm_vcpu_run)
 
+#ifdef CONFIG_KVM_AMD_SEV
 /**
  * __svm_sev_es_vcpu_run - Run a SEV-ES vCPU via a transition to SVM guest mode
  * @svm:	struct vcpu_svm *
@@ -389,3 +390,4 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
 	_ASM_EXTABLE(1b, 3b)
 
 SYM_FUNC_END(__svm_sev_es_vcpu_run)
+#endif /* CONFIG_KVM_AMD_SEV */
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 3/8] KVM: SVM: Drop 32-bit "support" from __svm_sev_es_vcpu_run()

[Sean Christopherson]

Drop 32-bit "support" from __svm_sev_es_vcpu_run(), as SEV/SEV-ES firmly
64-bit only.  The "support" was purely the result of bad copy+paste from
__svm_vcpu_run(), which in turn was slightly less bad copy+paste from
__vmx_vcpu_run().

Opportunistically convert to unadulterated register accesses so that it's
easier (but still not easy) to follow which registers hold what arguments,
and when.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/vmenter.S | 44 +++++++++++---------------------------
 1 file changed, 13 insertions(+), 31 deletions(-)

diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S
index 7ee363d7517c..0026b4a56d25 100644
--- a/arch/x86/kvm/svm/vmenter.S
+++ b/arch/x86/kvm/svm/vmenter.S
@@ -298,17 +298,12 @@ SYM_FUNC_END(__svm_vcpu_run)
  * @spec_ctrl_intercepted: bool
  */
 SYM_FUNC_START(__svm_sev_es_vcpu_run)
-	push %_ASM_BP
-#ifdef CONFIG_X86_64
+	push %rbp
 	push %r15
 	push %r14
 	push %r13
 	push %r12
-#else
-	push %edi
-	push %esi
-#endif
-	push %_ASM_BX
+	push %rbx
 
 	/*
 	 * Save variables needed after vmexit on the stack, in inverse
@@ -316,39 +311,31 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
 	 */
 
 	/* Accessed directly from the stack in RESTORE_HOST_SPEC_CTRL.  */
-	push %_ASM_ARG2
+	push %rsi
 
 	/* Save @svm. */
-	push %_ASM_ARG1
-
-.ifnc _ASM_ARG1, _ASM_DI
-	/*
-	 * Stash @svm in RDI early. On 32-bit, arguments are in RAX, RCX
-	 * and RDX which are clobbered by RESTORE_GUEST_SPEC_CTRL.
-	 */
-	mov %_ASM_ARG1, %_ASM_DI
-.endif
+	push %rdi
 
 	/* Clobbers RAX, RCX, RDX.  */
 	RESTORE_GUEST_SPEC_CTRL
 
 	/* Get svm->current_vmcb->pa into RAX. */
-	mov SVM_current_vmcb(%_ASM_DI), %_ASM_AX
-	mov KVM_VMCB_pa(%_ASM_AX), %_ASM_AX
+	mov SVM_current_vmcb(%rdi), %rax
+	mov KVM_VMCB_pa(%rax), %rax
 
 	/* Enter guest mode */
 	sti
 
-1:	vmrun %_ASM_AX
+1:	vmrun %rax
 
 2:	cli
 
 	/* Pop @svm to RDI, guest registers have been saved already. */
-	pop %_ASM_DI
+	pop %rdi
 
 #ifdef CONFIG_RETPOLINE
 	/* IMPORTANT: Stuff the RSB immediately after VM-Exit, before RET! */
-	FILL_RETURN_BUFFER %_ASM_AX, RSB_CLEAR_LOOPS, X86_FEATURE_RETPOLINE
+	FILL_RETURN_BUFFER %rax, RSB_CLEAR_LOOPS, X86_FEATURE_RETPOLINE
 #endif
 
 	/* Clobbers RAX, RCX, RDX.  */
@@ -364,26 +351,21 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
 	UNTRAIN_RET_VM
 
 	/* "Pop" @spec_ctrl_intercepted.  */
-	pop %_ASM_BX
+	pop %rbx
 
-	pop %_ASM_BX
+	pop %rbx
 
-#ifdef CONFIG_X86_64
 	pop %r12
 	pop %r13
 	pop %r14
 	pop %r15
-#else
-	pop %esi
-	pop %edi
-#endif
-	pop %_ASM_BP
+	pop %rbp
 	RET
 
 	RESTORE_GUEST_SPEC_CTRL_BODY
 	RESTORE_HOST_SPEC_CTRL_BODY
 
-3:	cmpb $0, _ASM_RIP(kvm_rebooting)
+3:	cmpb $0, kvm_rebooting(%rip)
 	jne 2b
 	ud2
 
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 4/8] KVM: SVM: Clobber RAX instead of RBX when discarding spec_ctrl_intercepted

[Sean Christopherson]

POP @spec_ctrl_intercepted into RAX instead of RBX when discarding it from
the stack so that __svm_sev_es_vcpu_run() doesn't modify any non-volatile
registers.  __svm_sev_es_vcpu_run() doesn't return a value, and RAX is
already are clobbered multiple times in the #VMEXIT path.

This will allowing using the host save area to save/restore non-volatile
registers in __svm_sev_es_vcpu_run().

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/vmenter.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S
index 0026b4a56d25..edbaadaacba7 100644
--- a/arch/x86/kvm/svm/vmenter.S
+++ b/arch/x86/kvm/svm/vmenter.S
@@ -350,8 +350,8 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
 	 */
 	UNTRAIN_RET_VM
 
-	/* "Pop" @spec_ctrl_intercepted.  */
-	pop %rbx
+	/* "Pop" and discard @spec_ctrl_intercepted. */
+	pop %rax
 
 	pop %rbx
 
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 5/8] KVM: SVM: Save/restore non-volatile GPRs in SEV-ES VMRUN via host save area

[Sean Christopherson]

Use the host save area to save/restore non-volatile (callee-saved)
registers in __svm_sev_es_vcpu_run() to take advantage of hardware loading
all registers from the save area on #VMEXIT.  KVM still needs to save the
registers it wants restored, but the loads are handled automatically by
hardware.

Aside from less assembly code, letting hardware do the restoration means
stack frames are preserved for the entirety of __svm_sev_es_vcpu_run().

Opportunistically add a comment to call out why @svm needs to be saved
across VMRUN->#VMEXIT, as it's not easy to decipher that from the macro
hell.

Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Michael Roth <michael.roth@amd.com>
Cc: Alexey Kardashevskiy <aik@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/svm.c     | 17 +++++++++-------
 arch/x86/kvm/svm/svm.h     |  3 ++-
 arch/x86/kvm/svm/vmenter.S | 41 +++++++++++++++++++++-----------------
 3 files changed, 35 insertions(+), 26 deletions(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e90b429c84f1..e7c8a48e36eb 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1503,6 +1503,11 @@ static void svm_vcpu_free(struct kvm_vcpu *vcpu)
 	__free_pages(virt_to_page(svm->msrpm), get_order(MSRPM_SIZE));
 }
 
+static struct sev_es_save_area *sev_es_host_save_area(struct svm_cpu_data *sd)
+{
+	return page_address(sd->save_area) + 0x400;
+}
+
 static void svm_prepare_switch_to_guest(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
@@ -1519,12 +1524,8 @@ static void svm_prepare_switch_to_guest(struct kvm_vcpu *vcpu)
 	 * or subsequent vmload of host save area.
 	 */
 	vmsave(sd->save_area_pa);
-	if (sev_es_guest(vcpu->kvm)) {
-		struct sev_es_save_area *hostsa;
-		hostsa = (struct sev_es_save_area *)(page_address(sd->save_area) + 0x400);
-
-		sev_es_prepare_switch_to_guest(hostsa);
-	}
+	if (sev_es_guest(vcpu->kvm))
+		sev_es_prepare_switch_to_guest(sev_es_host_save_area(sd));
 
 	if (tsc_scaling)
 		__svm_write_tsc_multiplier(vcpu->arch.tsc_scaling_ratio);
@@ -4101,6 +4102,7 @@ static fastpath_t svm_exit_handlers_fastpath(struct kvm_vcpu *vcpu)
 
 static noinstr void svm_vcpu_enter_exit(struct kvm_vcpu *vcpu, bool spec_ctrl_intercepted)
 {
+	struct svm_cpu_data *sd = per_cpu_ptr(&svm_data, vcpu->cpu);
 	struct vcpu_svm *svm = to_svm(vcpu);
 
 	guest_state_enter_irqoff();
@@ -4108,7 +4110,8 @@ static noinstr void svm_vcpu_enter_exit(struct kvm_vcpu *vcpu, bool spec_ctrl_in
 	amd_clear_divider();
 
 	if (sev_es_guest(vcpu->kvm))
-		__svm_sev_es_vcpu_run(svm, spec_ctrl_intercepted);
+		__svm_sev_es_vcpu_run(svm, spec_ctrl_intercepted,
+				      sev_es_host_save_area(sd));
 	else
 		__svm_vcpu_run(svm, spec_ctrl_intercepted);
 
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 8ef95139cd24..b98cced44e48 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -697,7 +697,8 @@ void sev_es_unmap_ghcb(struct vcpu_svm *svm);
 
 /* vmenter.S */
 
-void __svm_sev_es_vcpu_run(struct vcpu_svm *svm, bool spec_ctrl_intercepted);
+void __svm_sev_es_vcpu_run(struct vcpu_svm *svm, bool spec_ctrl_intercepted,
+			   struct sev_es_save_area *hostsa);
 void __svm_vcpu_run(struct vcpu_svm *svm, bool spec_ctrl_intercepted);
 
 #define DEFINE_KVM_GHCB_ACCESSORS(field)						\
diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S
index edbaadaacba7..e92953427100 100644
--- a/arch/x86/kvm/svm/vmenter.S
+++ b/arch/x86/kvm/svm/vmenter.S
@@ -292,23 +292,35 @@ SYM_FUNC_START(__svm_vcpu_run)
 SYM_FUNC_END(__svm_vcpu_run)
 
 #ifdef CONFIG_KVM_AMD_SEV
+
+
+#ifdef CONFIG_X86_64
+#define SEV_ES_GPRS_BASE 0x300
+#define SEV_ES_RBX	(SEV_ES_GPRS_BASE + __VCPU_REGS_RBX * WORD_SIZE)
+#define SEV_ES_RBP	(SEV_ES_GPRS_BASE + __VCPU_REGS_RBP * WORD_SIZE)
+#define SEV_ES_R12	(SEV_ES_GPRS_BASE + __VCPU_REGS_R12 * WORD_SIZE)
+#define SEV_ES_R13	(SEV_ES_GPRS_BASE + __VCPU_REGS_R13 * WORD_SIZE)
+#define SEV_ES_R14	(SEV_ES_GPRS_BASE + __VCPU_REGS_R14 * WORD_SIZE)
+#define SEV_ES_R15	(SEV_ES_GPRS_BASE + __VCPU_REGS_R15 * WORD_SIZE)
+#endif
+
 /**
  * __svm_sev_es_vcpu_run - Run a SEV-ES vCPU via a transition to SVM guest mode
  * @svm:	struct vcpu_svm *
  * @spec_ctrl_intercepted: bool
  */
 SYM_FUNC_START(__svm_sev_es_vcpu_run)
-	push %rbp
-	push %r15
-	push %r14
-	push %r13
-	push %r12
-	push %rbx
-
 	/*
-	 * Save variables needed after vmexit on the stack, in inverse
-	 * order compared to when they are needed.
+	 * Save non-volatile (callee-saved) registers to the host save area.
+	 * Except for RAX and RSP, all GPRs are restored on #VMEXIT, but not
+	 * saved on VMRUN.
 	 */
+	mov %rbp, SEV_ES_RBP (%rdx)
+	mov %r15, SEV_ES_R15 (%rdx)
+	mov %r14, SEV_ES_R14 (%rdx)
+	mov %r13, SEV_ES_R13 (%rdx)
+	mov %r12, SEV_ES_R12 (%rdx)
+	mov %rbx, SEV_ES_RBX (%rdx)
 
 	/* Accessed directly from the stack in RESTORE_HOST_SPEC_CTRL.  */
 	push %rsi
@@ -316,7 +328,7 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
 	/* Save @svm. */
 	push %rdi
 
-	/* Clobbers RAX, RCX, RDX.  */
+	/* Clobbers RAX, RCX, RDX (@hostsa). */
 	RESTORE_GUEST_SPEC_CTRL
 
 	/* Get svm->current_vmcb->pa into RAX. */
@@ -338,7 +350,7 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
 	FILL_RETURN_BUFFER %rax, RSB_CLEAR_LOOPS, X86_FEATURE_RETPOLINE
 #endif
 
-	/* Clobbers RAX, RCX, RDX.  */
+	/* Clobbers RAX, RCX, RDX, consumes RDI (@svm). */
 	RESTORE_HOST_SPEC_CTRL
 
 	/*
@@ -353,13 +365,6 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
 	/* "Pop" and discard @spec_ctrl_intercepted. */
 	pop %rax
 
-	pop %rbx
-
-	pop %r12
-	pop %r13
-	pop %r14
-	pop %r15
-	pop %rbp
 	RET
 
 	RESTORE_GUEST_SPEC_CTRL_BODY
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 6/8] KVM: SVM: Save/restore args across SEV-ES VMRUN via host save area

[Sean Christopherson]

Use the host save area to preserve volatile registers that are used in
__svm_sev_es_vcpu_run() to access function parameters after #VMEXIT.
Like saving/restoring non-volatile registers, there's no reason not to
take advantage of hardware restoring registers on #VMEXIT, as doing so
shaves a few instructions and the save area is going to be accessed no
matter what.

Converting all register save/restore code to use the host save area also
make it easier to follow the SEV-ES VMRUN flow in its entirety, as
opposed to having a mix of stack-based versus host save area save/restore.

Add a parameter to RESTORE_HOST_SPEC_CTRL_BODY so that the SEV-ES path
doesn't need to write @spec_ctrl_intercepted to memory just to play nice
with the common macro.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/vmenter.S | 29 +++++++++++++----------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S
index e92953427100..48cdba47622c 100644
--- a/arch/x86/kvm/svm/vmenter.S
+++ b/arch/x86/kvm/svm/vmenter.S
@@ -67,7 +67,7 @@
 		"", X86_FEATURE_V_SPEC_CTRL
 901:
 .endm
-.macro RESTORE_HOST_SPEC_CTRL_BODY
+.macro RESTORE_HOST_SPEC_CTRL_BODY spec_ctrl_intercepted:req
 900:
 	/* Same for after vmexit.  */
 	mov $MSR_IA32_SPEC_CTRL, %ecx
@@ -76,7 +76,7 @@
 	 * Load the value that the guest had written into MSR_IA32_SPEC_CTRL,
 	 * if it was not intercepted during guest execution.
 	 */
-	cmpb $0, (%_ASM_SP)
+	cmpb $0, \spec_ctrl_intercepted
 	jnz 998f
 	rdmsr
 	movl %eax, SVM_spec_ctrl(%_ASM_DI)
@@ -269,7 +269,7 @@ SYM_FUNC_START(__svm_vcpu_run)
 	RET
 
 	RESTORE_GUEST_SPEC_CTRL_BODY
-	RESTORE_HOST_SPEC_CTRL_BODY
+	RESTORE_HOST_SPEC_CTRL_BODY (%_ASM_SP)
 
 10:	cmpb $0, _ASM_RIP(kvm_rebooting)
 	jne 2b
@@ -298,6 +298,8 @@ SYM_FUNC_END(__svm_vcpu_run)
 #define SEV_ES_GPRS_BASE 0x300
 #define SEV_ES_RBX	(SEV_ES_GPRS_BASE + __VCPU_REGS_RBX * WORD_SIZE)
 #define SEV_ES_RBP	(SEV_ES_GPRS_BASE + __VCPU_REGS_RBP * WORD_SIZE)
+#define SEV_ES_RSI	(SEV_ES_GPRS_BASE + __VCPU_REGS_RSI * WORD_SIZE)
+#define SEV_ES_RDI	(SEV_ES_GPRS_BASE + __VCPU_REGS_RDI * WORD_SIZE)
 #define SEV_ES_R12	(SEV_ES_GPRS_BASE + __VCPU_REGS_R12 * WORD_SIZE)
 #define SEV_ES_R13	(SEV_ES_GPRS_BASE + __VCPU_REGS_R13 * WORD_SIZE)
 #define SEV_ES_R14	(SEV_ES_GPRS_BASE + __VCPU_REGS_R14 * WORD_SIZE)
@@ -322,11 +324,12 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
 	mov %r12, SEV_ES_R12 (%rdx)
 	mov %rbx, SEV_ES_RBX (%rdx)
 
-	/* Accessed directly from the stack in RESTORE_HOST_SPEC_CTRL.  */
-	push %rsi
-
-	/* Save @svm. */
-	push %rdi
+	/*
+	 * Save volatile registers that hold arguments that are needed after
+	 * #VMEXIT (RDI=@svm and RSI=@spec_ctrl_intercepted).
+	 */
+	mov %rdi, SEV_ES_RDI (%rdx)
+	mov %rsi, SEV_ES_RSI (%rdx)
 
 	/* Clobbers RAX, RCX, RDX (@hostsa). */
 	RESTORE_GUEST_SPEC_CTRL
@@ -342,15 +345,12 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
 
 2:	cli
 
-	/* Pop @svm to RDI, guest registers have been saved already. */
-	pop %rdi
-
 #ifdef CONFIG_RETPOLINE
 	/* IMPORTANT: Stuff the RSB immediately after VM-Exit, before RET! */
 	FILL_RETURN_BUFFER %rax, RSB_CLEAR_LOOPS, X86_FEATURE_RETPOLINE
 #endif
 
-	/* Clobbers RAX, RCX, RDX, consumes RDI (@svm). */
+	/* Clobbers RAX, RCX, RDX, consumes RDI (@svm) and RSI (@spec_ctrl_intercepted). */
 	RESTORE_HOST_SPEC_CTRL
 
 	/*
@@ -362,13 +362,10 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
 	 */
 	UNTRAIN_RET_VM
 
-	/* "Pop" and discard @spec_ctrl_intercepted. */
-	pop %rax
-
 	RET
 
 	RESTORE_GUEST_SPEC_CTRL_BODY
-	RESTORE_HOST_SPEC_CTRL_BODY
+	RESTORE_HOST_SPEC_CTRL_BODY %sil
 
 3:	cmpb $0, kvm_rebooting(%rip)
 	jne 2b
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 7/8] KVM: SVM: Create a stack frame in __svm_sev_es_vcpu_run()

[Sean Christopherson]

Now that KVM uses the host save area to context switch RBP, i.e.
preserves RBP for the entirety of __svm_sev_es_vcpu_run(), create a stack
frame using the standared FRAME_{BEGIN,END} macros.

Note, __svm_sev_es_vcpu_run() is subtly not a leaf function as it can call
into ibpb_feature() via UNTRAIN_RET_VM.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/svm/vmenter.S | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S
index 48cdba47622c..5461c23ee762 100644
--- a/arch/x86/kvm/svm/vmenter.S
+++ b/arch/x86/kvm/svm/vmenter.S
@@ -3,6 +3,7 @@
 #include <asm/asm.h>
 #include <asm/asm-offsets.h>
 #include <asm/bitsperlong.h>
+#include <asm/frame.h>
 #include <asm/kvm_vcpu_regs.h>
 #include <asm/nospec-branch.h>
 #include "kvm-asm-offsets.h"
@@ -312,6 +313,8 @@ SYM_FUNC_END(__svm_vcpu_run)
  * @spec_ctrl_intercepted: bool
  */
 SYM_FUNC_START(__svm_sev_es_vcpu_run)
+	FRAME_BEGIN
+
 	/*
 	 * Save non-volatile (callee-saved) registers to the host save area.
 	 * Except for RAX and RSP, all GPRs are restored on #VMEXIT, but not
@@ -362,6 +365,7 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
 	 */
 	UNTRAIN_RET_VM
 
+	FRAME_END
 	RET
 
 	RESTORE_GUEST_SPEC_CTRL_BODY
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH 8/8] KVM: x86: Stop compiling vmenter.S with OBJECT_FILES_NON_STANDARD

[Sean Christopherson]

Stop compiling vmenter.S with OBJECT_FILES_NON_STANDARD to skip objtool's
stack validation now that __svm_vcpu_run() and __svm_sev_es_vcpu_run()
create stack frames (thoughthe former's effectiveness is dubious).

Note, due to a quirk in how OBJECT_FILES_NON_STANDARD is handled by the
build system, this also affects vmx/vmenter.S.  But __vmx_vcpu_run()
already plays nice with frame pointers, i.e. it was collateral damage when
commit 7f4b5cde2409 ("kvm: Disable objtool frame pointer checking for
vmenter.S") added the OBJECT_FILES_NON_STANDARD hack-a-fix.

Link: https://lore.kernel.org/all/20240217055504.2059803-1-masahiroy@kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/Makefile | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index 475b5fa917a6..addc44fc7187 100644
--- a/arch/x86/kvm/Makefile
+++ b/arch/x86/kvm/Makefile
@@ -3,10 +3,6 @@
 ccflags-y += -I $(srctree)/arch/x86/kvm
 ccflags-$(CONFIG_KVM_WERROR) += -Werror
 
-ifeq ($(CONFIG_FRAME_POINTER),y)
-OBJECT_FILES_NON_STANDARD_vmenter.o := y
-endif
-
 include $(srctree)/virt/kvm/Makefile.kvm
 
 kvm-y			+= x86.o emulate.o i8259.o irq.o lapic.o \
-- 
2.44.0.rc0.258.g7320e95886-goog

Re: [PATCH 0/8] KVM: SVM: Clean up VMRUN=>#VMEXIT assembly

[Tom Lendacky]

On 2/23/24 14:42, Sean Christopherson wrote:
> Clean up SVM's enter/exit assembly code so that it can be compiled
> without OBJECT_FILES_NON_STANDARD.  The "standard" __svm_vcpu_run() can't
> be made 100% bulletproof, as RBP isn't restored on #VMEXIT, but that's
> also the case for __vmx_vcpu_run(), and getting "close enough" is better
> than not even trying.
> 
> As for SEV-ES, after yet another refresher on swap types, I realized KVM
> can simply let the hardware restore registers after #VMEXIT, all that's
> missing is storing the current values to the host save area (I learned the
> hard way that they are swap Type B, *sigh*).  Unless I'm missing something,
> this provides 100% accuracy when using stack frames for unwinding, and
> requires less assembly (though probably not fewer code bytes; I didn't check).
> 
> In between, build the SEV-ES code iff CONFIG_KVM_AMD_SEV=y, and yank out
> "support" for 32-bit kernels, which was unncessarily polluting the code.
> 
> I'm pretty sure I actually managed to test all of this, thanks to the SEV-ES
> smoke selftests, and a bit of hacking to disable V_SPEC_CTRL, passthrough
> SPEC_CTRL unconditionally, and have the selftests W/R SPEC_CTRL from its
> guest.
> 
> Sean Christopherson (8):
>    KVM: SVM: Create a stack frame in __svm_vcpu_run() for unwinding
>    KVM: SVM: Wrap __svm_sev_es_vcpu_run() with #ifdef CONFIG_KVM_AMD_SEV
>    KVM: SVM: Drop 32-bit "support" from __svm_sev_es_vcpu_run()
>    KVM: SVM: Clobber RAX instead of RBX when discarding
>      spec_ctrl_intercepted
>    KVM: SVM: Save/restore non-volatile GPRs in SEV-ES VMRUN via host save
>      area
>    KVM: SVM: Save/restore args across SEV-ES VMRUN via host save area
>    KVM: SVM: Create a stack frame in __svm_sev_es_vcpu_run()
>    KVM: x86: Stop compiling vmenter.S with OBJECT_FILES_NON_STANDARD
> 
>   arch/x86/kvm/Makefile      |  4 --
>   arch/x86/kvm/svm/svm.c     | 17 ++++---
>   arch/x86/kvm/svm/svm.h     |  3 +-
>   arch/x86/kvm/svm/vmenter.S | 97 +++++++++++++++++---------------------
>   4 files changed, 56 insertions(+), 65 deletions(-)

Nice cleanup, thanks!

For the series:
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> 
> 
> base-commit: ec1e3d33557babed2c2c2c7da6e84293c2f56f58

Re: [PATCH 0/8] KVM: SVM: Clean up VMRUN=>#VMEXIT assembly

[Sean Christopherson]

On Fri, 23 Feb 2024 12:42:25 -0800, Sean Christopherson wrote:
> Clean up SVM's enter/exit assembly code so that it can be compiled
> without OBJECT_FILES_NON_STANDARD.  The "standard" __svm_vcpu_run() can't
> be made 100% bulletproof, as RBP isn't restored on #VMEXIT, but that's
> also the case for __vmx_vcpu_run(), and getting "close enough" is better
> than not even trying.
> 
> As for SEV-ES, after yet another refresher on swap types, I realized KVM
> can simply let the hardware restore registers after #VMEXIT, all that's
> missing is storing the current values to the host save area (I learned the
> hard way that they are swap Type B, *sigh*).  Unless I'm missing something,
> this provides 100% accuracy when using stack frames for unwinding, and
> requires less assembly (though probably not fewer code bytes; I didn't check).
> 
> [...]

Applied to kvm-x86 svm, thanks!

[1/8] KVM: SVM: Create a stack frame in __svm_vcpu_run() for unwinding
      https://github.com/kvm-x86/linux/commit/19597a71a0c8
[2/8] KVM: SVM: Wrap __svm_sev_es_vcpu_run() with #ifdef CONFIG_KVM_AMD_SEV
      https://github.com/kvm-x86/linux/commit/7774c8f32e99
[3/8] KVM: SVM: Drop 32-bit "support" from __svm_sev_es_vcpu_run()
      https://github.com/kvm-x86/linux/commit/331282fdb15e
[4/8] KVM: SVM: Clobber RAX instead of RBX when discarding spec_ctrl_intercepted
      https://github.com/kvm-x86/linux/commit/87e8e360a05f
[5/8] KVM: SVM: Save/restore non-volatile GPRs in SEV-ES VMRUN via host save area
      https://github.com/kvm-x86/linux/commit/c92be2fd8edf
[6/8] KVM: SVM: Save/restore args across SEV-ES VMRUN via host save area
      https://github.com/kvm-x86/linux/commit/adac42bf42c1
[7/8] KVM: SVM: Create a stack frame in __svm_sev_es_vcpu_run()
      https://github.com/kvm-x86/linux/commit/4367a75887ec
[8/8] KVM: x86: Stop compiling vmenter.S with OBJECT_FILES_NON_STANDARD
      https://github.com/kvm-x86/linux/commit/27ca867042af

--
https://github.com/kvm-x86/linux/tree/next

Re: [PATCH 0/8] KVM: SVM: Clean up VMRUN=>#VMEXIT assembly

[Paolo Bonzini]

On Wed, Apr 10, 2024 at 2:23 AM Sean Christopherson <seanjc@google.com> wrote:
> Applied to kvm-x86 svm, thanks!
>
> [1/8] KVM: SVM: Create a stack frame in __svm_vcpu_run() for unwinding
>       https://github.com/kvm-x86/linux/commit/19597a71a0c8
> [2/8] KVM: SVM: Wrap __svm_sev_es_vcpu_run() with #ifdef CONFIG_KVM_AMD_SEV
>       https://github.com/kvm-x86/linux/commit/7774c8f32e99
> [3/8] KVM: SVM: Drop 32-bit "support" from __svm_sev_es_vcpu_run()
>       https://github.com/kvm-x86/linux/commit/331282fdb15e
> [4/8] KVM: SVM: Clobber RAX instead of RBX when discarding spec_ctrl_intercepted
>       https://github.com/kvm-x86/linux/commit/87e8e360a05f
> [5/8] KVM: SVM: Save/restore non-volatile GPRs in SEV-ES VMRUN via host save area
>       https://github.com/kvm-x86/linux/commit/c92be2fd8edf
> [6/8] KVM: SVM: Save/restore args across SEV-ES VMRUN via host save area
>       https://github.com/kvm-x86/linux/commit/adac42bf42c1
> [7/8] KVM: SVM: Create a stack frame in __svm_sev_es_vcpu_run()
>       https://github.com/kvm-x86/linux/commit/4367a75887ec
> [8/8] KVM: x86: Stop compiling vmenter.S with OBJECT_FILES_NON_STANDARD
>       https://github.com/kvm-x86/linux/commit/27ca867042af

Do we perhaps want this in 6.9 because of the issues that was reported
with objtool?

Paolo