Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 04, 2023 at 10:48:40PM +0200, Peter Zijlstra wrote:
> On Fri, Aug 04, 2023 at 12:20:05PM +0200, Paolo Bonzini wrote:
> > It's not clobbered in a part that will cause unwinding; we can further
> > restrict the part to a handful of instructions (and add a mov %rsp, %rbp
> > at the top, see untested patch after signature).
> > 
> > I think the chance of this failure is similar or lower to the chance of
> > a memory failure that hits the exception handler code itself.
> 
> Yes, that's very helpful, the below is your patch with a few extra
> hints and a comment. This seems to cure things.
> 
> Specifically, your change is needed to put UNWIND_HINT_RESTORE before we
> go CALL things (like entry_ibpb), otherwise objtool gets upset we CALL
> without having a valid framepointer.
> 
> Josh, this look ok to you?

Looks mostly right, except this now creates an unnecessary gap in
unwinding coverage for the ORC unwinder.  So it's better to put the
FP-specific changes behind CONFIG_FRAME_POINTER:

diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
index 80e3fe184d17..0c5c2f090e93 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 \
diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S
index 8e8295e774f0..51f6851b1ae5 100644
--- a/arch/x86/kvm/svm/vmenter.S
+++ b/arch/x86/kvm/svm/vmenter.S
@@ -99,6 +99,9 @@
  */
 SYM_FUNC_START(__svm_vcpu_run)
 	push %_ASM_BP
+#ifdef CONFIG_FRAME_POINTER
+	mov %_ASM_SP, %_ASM_BP
+#endif
 #ifdef CONFIG_X86_64
 	push %r15
 	push %r14
@@ -121,7 +124,20 @@ SYM_FUNC_START(__svm_vcpu_run)
 	/* Needed to restore access to percpu variables.  */
 	__ASM_SIZE(push) PER_CPU_VAR(svm_data + SD_save_area_pa)
 
-	/* Finally save @svm. */
+	/*
+	 * Finally save frame pointer and @svm.
+	 *
+	 * Clobbering BP here is mostly ok since GIF will block NMIs and with
+	 * the exception of #MC and the kvm_rebooting _ASM_EXTABLE()s below
+	 * nothing untoward will happen until BP is restored.
+	 *
+	 * The kvm_rebooting exceptions should not want to unwind stack, and
+	 * while #MV might want to unwind stack, it is ultimately fatal.
+	 */
+#ifdef CONFIG_FRAME_POINTER
+	UNWIND_HINT_SAVE
+	push %_ASM_BP
+#endif
 	push %_ASM_ARG1
 
 .ifnc _ASM_ARG1, _ASM_DI
@@ -153,7 +169,6 @@ SYM_FUNC_START(__svm_vcpu_run)
 	mov VCPU_RCX(%_ASM_DI), %_ASM_CX
 	mov VCPU_RDX(%_ASM_DI), %_ASM_DX
 	mov VCPU_RBX(%_ASM_DI), %_ASM_BX
-	mov VCPU_RBP(%_ASM_DI), %_ASM_BP
 	mov VCPU_RSI(%_ASM_DI), %_ASM_SI
 #ifdef CONFIG_X86_64
 	mov VCPU_R8 (%_ASM_DI),  %r8
@@ -165,6 +180,7 @@ SYM_FUNC_START(__svm_vcpu_run)
 	mov VCPU_R14(%_ASM_DI), %r14
 	mov VCPU_R15(%_ASM_DI), %r15
 #endif
+	mov VCPU_RBP(%_ASM_DI), %_ASM_BP
 	mov VCPU_RDI(%_ASM_DI), %_ASM_DI
 
 	/* Enter guest mode */
@@ -177,11 +193,18 @@ SYM_FUNC_START(__svm_vcpu_run)
 	/* Pop @svm to RAX while it's the only available register. */
 	pop %_ASM_AX
 
-	/* Save all guest registers.  */
+	/*
+	 * Save all guest registers. Pop the frame pointer as soon as possible
+	 * to enable unwinding.
+	 */
+	mov %_ASM_BP,   VCPU_RBP(%_ASM_AX)
+#ifdef CONFIG_FRAME_POINTER
+	pop %_ASM_BP
+	UNWIND_HINT_RESTORE
+#endif
 	mov %_ASM_CX,   VCPU_RCX(%_ASM_AX)
 	mov %_ASM_DX,   VCPU_RDX(%_ASM_AX)
 	mov %_ASM_BX,   VCPU_RBX(%_ASM_AX)
-	mov %_ASM_BP,   VCPU_RBP(%_ASM_AX)
 	mov %_ASM_SI,   VCPU_RSI(%_ASM_AX)
 	mov %_ASM_DI,   VCPU_RDI(%_ASM_AX)
 #ifdef CONFIG_X86_64
@@ -297,6 +320,9 @@ SYM_FUNC_END(__svm_vcpu_run)
  */
 SYM_FUNC_START(__svm_sev_es_vcpu_run)
 	push %_ASM_BP
+#ifdef CONFIG_FRAME_POINTER
+	mov %_ASM_SP, %_ASM_BP
+#endif
 #ifdef CONFIG_X86_64
 	push %r15
 	push %r14
@@ -316,7 +342,11 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
 	/* Accessed directly from the stack in RESTORE_HOST_SPEC_CTRL.  */
 	push %_ASM_ARG2
 
-	/* Save @svm. */
+	/* Save frame pointer and @svm. */
+#ifdef CONFIG_FRAME_POINTER
+	UNWIND_HINT_SAVE
+	push %_ASM_BP
+#endif
 	push %_ASM_ARG1
 
 .ifnc _ASM_ARG1, _ASM_DI
@@ -341,8 +371,15 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
 
 2:	cli
 
-	/* Pop @svm to RDI, guest registers have been saved already. */
+	/*
+	 * Guest registers have been saved already.
+	 * Pop @svm to RDI and restore the frame pointer to allow unwinding.
+	 */
 	pop %_ASM_DI
+#ifdef CONFIG_FRAME_POINTER
+	pop %_ASM_BP
+	UNWIND_HINT_RESTORE
+#endif
 
 #ifdef CONFIG_RETPOLINE
 	/* IMPORTANT: Stuff the RSB immediately after VM-Exit, before RET! */



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux