2024-02-23 20:45:25

by Sean Christopherson

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

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



2024-02-23 20:45:30

by Sean Christopherson

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

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 <[email protected]>
---
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


2024-02-23 20:45:56

by Sean Christopherson

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

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 <[email protected]>
---
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


2024-02-23 20:46:00

by Sean Christopherson

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

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 <[email protected]>
---
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


2024-02-23 20:46:22

by Sean Christopherson

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

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 <[email protected]>
Cc: Michael Roth <[email protected]>
Cc: Alexey Kardashevskiy <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
---
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


2024-02-23 20:46:47

by Sean Christopherson

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

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 <[email protected]>
---
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


2024-02-23 20:46:54

by Sean Christopherson

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

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 <[email protected]>
---
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


2024-02-23 20:49:54

by Sean Christopherson

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

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/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
---
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


2024-02-26 20:26:34

by Tom Lendacky

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

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 <[email protected]>

>
>
> base-commit: ec1e3d33557babed2c2c2c7da6e84293c2f56f58

2024-04-10 00:23:34

by Sean Christopherson

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

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

2024-04-17 12:58:47

by Paolo Bonzini

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

On Wed, Apr 10, 2024 at 2:23 AM Sean Christopherson <[email protected]> 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