* [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o
@ 2023-08-02 9:11 Nikunj A Dadhania
2023-08-02 14:02 ` Sean Christopherson
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Nikunj A Dadhania @ 2023-08-02 9:11 UTC (permalink / raw)
To: kvm, Sean Christopherson
Cc: Nikunj A Dadhania, Josh Poimboeuf, Peter Zijlstra, Paolo Bonzini,
Randy Dunlap, Tom Lendacky, Ravi Bangoria
commit 7f4b5cde2409 ("kvm: Disable objtool frame pointer checking for
vmenter.S") had added the vmenter.o file to the exception list.
objtool gives the following warnings in the newer kernel builds:
arch/x86/kvm/kvm-amd.o: warning: objtool: __svm_vcpu_run+0x17d: BP used as a scratch register
arch/x86/kvm/kvm-amd.o: warning: objtool: __svm_sev_es_vcpu_run+0x72: BP used as a scratch register
As kvm-amd.o is a link time object, skipping the kvm-amd.o is not possible
as per the objtool documentation, better to skip the offending functions.
Functions __svm_vcpu_run() and __svm_sev_es_vcpu_run() saves and restores
RBP. Below is the snippet:
SYM_FUNC_START(__svm_vcpu_run)
push %_ASM_BP
<…>
pop %_ASM_BP
RET
Add exceptions to skip both these functions. Remove the
OBJECT_FILES_NON_STANDARD for vmenter.o
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Sean Christopherson <seanjc@google.com>
Reported-by: Ravi Bangoria <ravi.bangoria@amd.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
---
arch/x86/kvm/Makefile | 4 ----
arch/x86/kvm/svm/vmenter.S | 2 ++
2 files changed, 2 insertions(+), 4 deletions(-)
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..8fd37d661c33 100644
--- a/arch/x86/kvm/svm/vmenter.S
+++ b/arch/x86/kvm/svm/vmenter.S
@@ -289,6 +289,7 @@ SYM_FUNC_START(__svm_vcpu_run)
_ASM_EXTABLE(7b, 70b)
SYM_FUNC_END(__svm_vcpu_run)
+STACK_FRAME_NON_STANDARD(__svm_vcpu_run)
/**
* __svm_sev_es_vcpu_run - Run a SEV-ES vCPU via a transition to SVM guest mode
@@ -388,3 +389,4 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run)
_ASM_EXTABLE(1b, 3b)
SYM_FUNC_END(__svm_sev_es_vcpu_run)
+STACK_FRAME_NON_STANDARD_FP(__svm_sev_es_vcpu_run)
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-02 9:11 [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o Nikunj A Dadhania @ 2023-08-02 14:02 ` Sean Christopherson 2023-08-03 6:25 ` Nikunj A. Dadhania 2023-08-03 12:06 ` Peter Zijlstra ` (2 subsequent siblings) 3 siblings, 1 reply; 17+ messages in thread From: Sean Christopherson @ 2023-08-02 14:02 UTC (permalink / raw) To: Nikunj A Dadhania Cc: kvm, Josh Poimboeuf, Peter Zijlstra, Paolo Bonzini, Randy Dunlap, Tom Lendacky, Ravi Bangoria On Wed, Aug 02, 2023, Nikunj A Dadhania wrote: > objtool gives the following warnings in the newer kernel builds: Define "newer". As in, exactly what commit is causing problems? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-02 14:02 ` Sean Christopherson @ 2023-08-03 6:25 ` Nikunj A. Dadhania 0 siblings, 0 replies; 17+ messages in thread From: Nikunj A. Dadhania @ 2023-08-03 6:25 UTC (permalink / raw) To: Sean Christopherson Cc: kvm, Josh Poimboeuf, Peter Zijlstra, Paolo Bonzini, Randy Dunlap, Tom Lendacky, Ravi Bangoria, Arnd Bergmann On 8/2/2023 7:32 PM, Sean Christopherson wrote: > On Wed, Aug 02, 2023, Nikunj A Dadhania wrote: >> objtool gives the following warnings in the newer kernel builds: > > Define "newer". As in, exactly what commit is causing problems? Not that new it seems, I have tried till v5.19 and the warning is there but with a different signature. Do you want me to further bisect it ? arch/x86/kvm/kvm-amd.o: warning: objtool: .altinstr_replacement+0x4d: call without frame pointer save/setup arch/x86/kvm/kvm-amd.o: warning: objtool: .altinstr_replacement+0x57: call without frame pointer save/setup kvm-amd.o warnings were also reported in the below thread: https://lore.kernel.org/lkml/9698eff1-9680-4f0a-94de-590eaa923e94@app.fastmail.com/ Regards, Nikunj ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-02 9:11 [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o Nikunj A Dadhania 2023-08-02 14:02 ` Sean Christopherson @ 2023-08-03 12:06 ` Peter Zijlstra 2023-08-03 18:06 ` Paolo Bonzini 2023-08-04 11:14 ` Peter Zijlstra 2023-08-12 0:51 ` kernel test robot 3 siblings, 1 reply; 17+ messages in thread From: Peter Zijlstra @ 2023-08-03 12:06 UTC (permalink / raw) To: Nikunj A Dadhania Cc: kvm, Sean Christopherson, Josh Poimboeuf, Paolo Bonzini, Randy Dunlap, Tom Lendacky, Ravi Bangoria On Wed, Aug 02, 2023 at 02:41:07PM +0530, Nikunj A Dadhania wrote: > commit 7f4b5cde2409 ("kvm: Disable objtool frame pointer checking for > vmenter.S") had added the vmenter.o file to the exception list. > > objtool gives the following warnings in the newer kernel builds: > > arch/x86/kvm/kvm-amd.o: warning: objtool: __svm_vcpu_run+0x17d: BP used as a scratch register > arch/x86/kvm/kvm-amd.o: warning: objtool: __svm_sev_es_vcpu_run+0x72: BP used as a scratch register > > As kvm-amd.o is a link time object, skipping the kvm-amd.o is not possible > as per the objtool documentation, better to skip the offending functions. > > Functions __svm_vcpu_run() and __svm_sev_es_vcpu_run() saves and restores > RBP. Below is the snippet: > > SYM_FUNC_START(__svm_vcpu_run) > push %_ASM_BP > <…> > pop %_ASM_BP > RET > > Add exceptions to skip both these functions. Remove the > OBJECT_FILES_NON_STANDARD for vmenter.o > > Cc: Josh Poimboeuf <jpoimboe@redhat.com> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Cc: Sean Christopherson <seanjc@google.com> > Reported-by: Ravi Bangoria <ravi.bangoria@amd.com> > Signed-off-by: Nikunj A Dadhania <nikunj@amd.com> > --- > arch/x86/kvm/Makefile | 4 ---- > arch/x86/kvm/svm/vmenter.S | 2 ++ > 2 files changed, 2 insertions(+), 4 deletions(-) > > 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..8fd37d661c33 100644 > --- a/arch/x86/kvm/svm/vmenter.S > +++ b/arch/x86/kvm/svm/vmenter.S > @@ -289,6 +289,7 @@ SYM_FUNC_START(__svm_vcpu_run) > _ASM_EXTABLE(7b, 70b) > > SYM_FUNC_END(__svm_vcpu_run) > +STACK_FRAME_NON_STANDARD(__svm_vcpu_run) > > /** > * __svm_sev_es_vcpu_run - Run a SEV-ES vCPU via a transition to SVM guest mode > @@ -388,3 +389,4 @@ SYM_FUNC_START(__svm_sev_es_vcpu_run) > _ASM_EXTABLE(1b, 3b) > > SYM_FUNC_END(__svm_sev_es_vcpu_run) > +STACK_FRAME_NON_STANDARD_FP(__svm_sev_es_vcpu_run) Urgh... no, no, this is all broken. By marking them with STACK_FRAME_NON_STANDARD you will get no ORC data at all, and then you also violate the normal framepointer calling convention. This means that if you need to unwind here you're up a creek without no paddles on. Objtool complains for a reason, your changelog does not provide a counter argument for that reason. Hardware/firmware interfaces that require one to violate basic calling conventions are horrible crap. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-03 12:06 ` Peter Zijlstra @ 2023-08-03 18:06 ` Paolo Bonzini 2023-08-03 19:07 ` Peter Zijlstra 0 siblings, 1 reply; 17+ messages in thread From: Paolo Bonzini @ 2023-08-03 18:06 UTC (permalink / raw) To: Peter Zijlstra, Nikunj A Dadhania Cc: kvm, Sean Christopherson, Josh Poimboeuf, Randy Dunlap, Tom Lendacky, Ravi Bangoria On 8/3/23 14:06, Peter Zijlstra wrote: > > By marking them with STACK_FRAME_NON_STANDARD you will get no ORC data > at all, and then you also violate the normal framepointer calling > convention. > > This means that if you need to unwind here you're up a creek without no > paddles on. The only weird thing that can happen is ud2 instructions that are executed in case the vmload/vmrun/vmsave instructions causes a #GP, from the exception handler. If I understand correctly those ud2 would use ORC information to show the backtrace, but even then the frame pointer should be correct. Of these instructions, vmrun is the only one that runs with wrong %rbp; and it is unlikely or even impossible that a #GP happens at vmrun, because the same operand has been used for a vmload ten instructions before. The only time I saw that #GP it was due to a processor errata, but it happened consistently on the vmload. So if frame pointer unwinding can be used in the absence of ORC, Nikunj patch should not break anything. Paolo ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-03 18:06 ` Paolo Bonzini @ 2023-08-03 19:07 ` Peter Zijlstra 2023-08-04 3:25 ` Nikunj A. Dadhania 2023-08-04 10:20 ` Paolo Bonzini 0 siblings, 2 replies; 17+ messages in thread From: Peter Zijlstra @ 2023-08-03 19:07 UTC (permalink / raw) To: Paolo Bonzini Cc: Nikunj A Dadhania, kvm, Sean Christopherson, Josh Poimboeuf, Randy Dunlap, Tom Lendacky, Ravi Bangoria On Thu, Aug 03, 2023 at 08:06:20PM +0200, Paolo Bonzini wrote: > On 8/3/23 14:06, Peter Zijlstra wrote: > > > > By marking them with STACK_FRAME_NON_STANDARD you will get no ORC data > > at all, and then you also violate the normal framepointer calling > > convention. > > > > This means that if you need to unwind here you're up a creek without no > > paddles on. > > The only weird thing that can happen is ud2 instructions that are executed > in case the vmload/vmrun/vmsave instructions causes a #GP, from the > exception handler. This code is ran with GIF disabled, so NMIs are not in the books, right? Does GIF block #MC ? > If I understand correctly those ud2 would use ORC information to show the > backtrace, but even then the frame pointer should be correct. Of these > instructions, vmrun is the only one that runs with wrong %rbp; and it is > unlikely or even impossible that a #GP happens at vmrun, because the same > operand has been used for a vmload ten instructions before. The only time I > saw that #GP it was due to a processor errata, but it happened consistently > on the vmload. > > So if frame pointer unwinding can be used in the absence of ORC, Nikunj > patch should not break anything. But framepointer unwinds rely on BP, and that is clobbered per the objtool complaint. Also, if you look at the makefile hunk that's being replaced, that was conditional on CONFIG_FRAMEPOINTS, while the annotation that's being added is not. I think objtool won't complain for ORC builds, only for framepoints builds. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-03 19:07 ` Peter Zijlstra @ 2023-08-04 3:25 ` Nikunj A. Dadhania 2023-08-04 10:20 ` Paolo Bonzini 1 sibling, 0 replies; 17+ messages in thread From: Nikunj A. Dadhania @ 2023-08-04 3:25 UTC (permalink / raw) To: Peter Zijlstra, Paolo Bonzini Cc: kvm, Sean Christopherson, Josh Poimboeuf, Randy Dunlap, Tom Lendacky, Ravi Bangoria On 8/4/2023 12:37 AM, Peter Zijlstra wrote: > On Thu, Aug 03, 2023 at 08:06:20PM +0200, Paolo Bonzini wrote: >> On 8/3/23 14:06, Peter Zijlstra wrote: >>> >>> By marking them with STACK_FRAME_NON_STANDARD you will get no ORC data >>> at all, and then you also violate the normal framepointer calling >>> convention. >>> >>> This means that if you need to unwind here you're up a creek without no >>> paddles on. >> >> The only weird thing that can happen is ud2 instructions that are executed >> in case the vmload/vmrun/vmsave instructions causes a #GP, from the >> exception handler. > > This code is ran with GIF disabled, so NMIs are not in the books, right? > Does GIF block #MC ? > >> If I understand correctly those ud2 would use ORC information to show the >> backtrace, but even then the frame pointer should be correct. Of these >> instructions, vmrun is the only one that runs with wrong %rbp; and it is >> unlikely or even impossible that a #GP happens at vmrun, because the same >> operand has been used for a vmload ten instructions before. The only time I >> saw that #GP it was due to a processor errata, but it happened consistently >> on the vmload. >> >> So if frame pointer unwinding can be used in the absence of ORC, Nikunj >> patch should not break anything. > > But framepointer unwinds rely on BP, and that is clobbered per the > objtool complaint. > > Also, if you look at the makefile hunk that's being replaced, that was > conditional on CONFIG_FRAMEPOINTS, while the annotation that's being > added is not. Even with the hunk present with CONFIG_FRAME_POINTER=y, I was getting the warning. That is why I had added the STACK_FRAME_NON_STANDARD and removed the hunk in makefile. Do you recommend using the below instead ? #ifdef CONFIG_FRAME_POINTER STACK_FRAME_NON_STANDARD() #endif Regards Nikunj ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-03 19:07 ` Peter Zijlstra 2023-08-04 3:25 ` Nikunj A. Dadhania @ 2023-08-04 10:20 ` Paolo Bonzini 2023-08-04 20:48 ` Peter Zijlstra 1 sibling, 1 reply; 17+ messages in thread From: Paolo Bonzini @ 2023-08-04 10:20 UTC (permalink / raw) To: Peter Zijlstra Cc: Nikunj A Dadhania, kvm, Sean Christopherson, Josh Poimboeuf, Randy Dunlap, Tom Lendacky, Ravi Bangoria On 8/3/23 21:07, Peter Zijlstra wrote: >> The only weird thing that can happen is ud2 instructions that are executed >> in case the vmload/vmrun/vmsave instructions causes a #GP, from the >> exception handler. > > This code is ran with GIF disabled, so NMIs are not in the books, right? Yep. > Does GIF block #MC ? No, #MC is an exception not an interrupt. >> So if frame pointer unwinding can be used in the absence of ORC, Nikunj >> patch should not break anything. > > But framepointer unwinds rely on BP, and that is clobbered per the > objtool complaint. 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. Paolo diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S index 8e8295e774f0..58fab5e0f7ae 100644 --- a/arch/x86/kvm/svm/vmenter.S +++ b/arch/x86/kvm/svm/vmenter.S @@ -99,6 +99,8 @@ */ SYM_FUNC_START(__svm_vcpu_run) push %_ASM_BP + mov %_ASM_SP, %_ASM_BP + #ifdef CONFIG_X86_64 push %r15 push %r14 @@ -121,7 +123,8 @@ 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. */ + push %_ASM_BP push %_ASM_ARG1 .ifnc _ASM_ARG1, _ASM_DI @@ -153,7 +156,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 +167,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 +180,15 @@ 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) + pop %_ASM_BP 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 +304,7 @@ SYM_FUNC_END(__svm_vcpu_run) */ SYM_FUNC_START(__svm_sev_es_vcpu_run) push %_ASM_BP + mov %_ASM_SP, %_ASM_BP #ifdef CONFIG_X86_64 push %r15 push %r14 @@ -316,7 +324,8 @@ 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. */ + push %_ASM_BP push %_ASM_ARG1 .ifnc _ASM_ARG1, _ASM_DI @@ -341,8 +350,12 @@ 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 + pop %_ASM_BP #ifdef CONFIG_RETPOLINE /* IMPORTANT: Stuff the RSB immediately after VM-Exit, before RET! */ ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-04 10:20 ` Paolo Bonzini @ 2023-08-04 20:48 ` Peter Zijlstra 2023-08-04 23:19 ` Josh Poimboeuf 0 siblings, 1 reply; 17+ messages in thread From: Peter Zijlstra @ 2023-08-04 20:48 UTC (permalink / raw) To: Paolo Bonzini Cc: Nikunj A Dadhania, kvm, Sean Christopherson, Josh Poimboeuf, Randy Dunlap, Tom Lendacky, Ravi Bangoria 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? --- arch/x86/kvm/Makefile | 4 ---- arch/x86/kvm/svm/vmenter.S | 38 ++++++++++++++++++++++++++++++++------ 2 files changed, 32 insertions(+), 10 deletions(-) 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..99b9be9a56c3 100644 --- a/arch/x86/kvm/svm/vmenter.S +++ b/arch/x86/kvm/svm/vmenter.S @@ -99,6 +99,8 @@ */ SYM_FUNC_START(__svm_vcpu_run) push %_ASM_BP + mov %_ASM_SP, %_ASM_BP + #ifdef CONFIG_X86_64 push %r15 push %r14 @@ -121,7 +123,18 @@ 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. + */ + UNWIND_HINT_SAVE + push %_ASM_BP push %_ASM_ARG1 .ifnc _ASM_ARG1, _ASM_DI @@ -153,7 +166,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 +177,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 +190,16 @@ 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) + pop %_ASM_BP + UNWIND_HINT_RESTORE 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 +315,7 @@ SYM_FUNC_END(__svm_vcpu_run) */ SYM_FUNC_START(__svm_sev_es_vcpu_run) push %_ASM_BP + mov %_ASM_SP, %_ASM_BP #ifdef CONFIG_X86_64 push %r15 push %r14 @@ -316,7 +335,9 @@ 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. */ + UNWIND_HINT_SAVE + push %_ASM_BP push %_ASM_ARG1 .ifnc _ASM_ARG1, _ASM_DI @@ -341,8 +362,13 @@ 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 + pop %_ASM_BP + UNWIND_HINT_RESTORE #ifdef CONFIG_RETPOLINE /* IMPORTANT: Stuff the RSB immediately after VM-Exit, before RET! */ ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-04 20:48 ` Peter Zijlstra @ 2023-08-04 23:19 ` Josh Poimboeuf 2023-08-05 0:55 ` Peter Zijlstra 0 siblings, 1 reply; 17+ messages in thread From: Josh Poimboeuf @ 2023-08-04 23:19 UTC (permalink / raw) To: Peter Zijlstra Cc: Paolo Bonzini, Nikunj A Dadhania, kvm, Sean Christopherson, Josh Poimboeuf, Randy Dunlap, Tom Lendacky, Ravi Bangoria 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! */ ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-04 23:19 ` Josh Poimboeuf @ 2023-08-05 0:55 ` Peter Zijlstra 2023-08-10 14:17 ` Paolo Bonzini 0 siblings, 1 reply; 17+ messages in thread From: Peter Zijlstra @ 2023-08-05 0:55 UTC (permalink / raw) To: Josh Poimboeuf Cc: Paolo Bonzini, Nikunj A Dadhania, kvm, Sean Christopherson, Josh Poimboeuf, Randy Dunlap, Tom Lendacky, Ravi Bangoria On Fri, Aug 04, 2023 at 06:19:54PM -0500, Josh Poimboeuf wrote: > 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: Fair enough. > 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. > + */ Aside from me not being able to type #MC, I did realize that the kvm_reboot exception will go outside noinstr code and can hit tracing/instrumentation and do unwinds from there. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-05 0:55 ` Peter Zijlstra @ 2023-08-10 14:17 ` Paolo Bonzini 2023-08-10 21:14 ` Peter Zijlstra 0 siblings, 1 reply; 17+ messages in thread From: Paolo Bonzini @ 2023-08-10 14:17 UTC (permalink / raw) To: Peter Zijlstra, Josh Poimboeuf Cc: Nikunj A Dadhania, kvm, Sean Christopherson, Josh Poimboeuf, Randy Dunlap, Tom Lendacky, Ravi Bangoria On 8/5/23 02:55, Peter Zijlstra wrote: >> + * 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. >> + */ > Aside from me not being able to type #MC, I did realize that the > kvm_reboot exception will go outside noinstr code and can hit > tracing/instrumentation and do unwinds from there. Asynchronously disabling SVM requires an IPI, so kvm_rebooting cannot change within CLGI/STGI. We can check it after CLGI instead of waiting for a #GP: diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 956726d867aa..e3755f5eaf81 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -4074,7 +4074,10 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu) if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL)) x86_spec_ctrl_set_guest(svm->virt_spec_ctrl); - svm_vcpu_enter_exit(vcpu, spec_ctrl_intercepted); + if (unlikely(kvm_rebooting)) + svm->vmcb->control.exit_code = SVM_EXIT_PAUSE; + else + svm_vcpu_enter_exit(vcpu, spec_ctrl_intercepted); if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL)) x86_spec_ctrl_restore_host(svm->virt_spec_ctrl); diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S index 8e8295e774f0..34641b3a6823 100644 --- a/arch/x86/kvm/svm/vmenter.S +++ b/arch/x86/kvm/svm/vmenter.S @@ -270,23 +270,12 @@ SYM_FUNC_START(__svm_vcpu_run) RESTORE_GUEST_SPEC_CTRL_BODY RESTORE_HOST_SPEC_CTRL_BODY -10: cmpb $0, kvm_rebooting - jne 2b - ud2 -30: cmpb $0, kvm_rebooting - jne 4b - ud2 -50: cmpb $0, kvm_rebooting - jne 6b - ud2 -70: cmpb $0, kvm_rebooting - jne 8b - ud2 +10: ud2 _ASM_EXTABLE(1b, 10b) - _ASM_EXTABLE(3b, 30b) - _ASM_EXTABLE(5b, 50b) - _ASM_EXTABLE(7b, 70b) + _ASM_EXTABLE(3b, 10b) + _ASM_EXTABLE(5b, 10b) + _ASM_EXTABLE(7b, 10b) SYM_FUNC_END(__svm_vcpu_run) Paolo ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-10 14:17 ` Paolo Bonzini @ 2023-08-10 21:14 ` Peter Zijlstra 0 siblings, 0 replies; 17+ messages in thread From: Peter Zijlstra @ 2023-08-10 21:14 UTC (permalink / raw) To: Paolo Bonzini Cc: Josh Poimboeuf, Nikunj A Dadhania, kvm, Sean Christopherson, Josh Poimboeuf, Randy Dunlap, Tom Lendacky, Ravi Bangoria On Thu, Aug 10, 2023 at 04:17:41PM +0200, Paolo Bonzini wrote: > On 8/5/23 02:55, Peter Zijlstra wrote: > > > + * 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. > > > + */ > > Aside from me not being able to type #MC, I did realize that the > > kvm_reboot exception will go outside noinstr code and can hit > > tracing/instrumentation and do unwinds from there. > > Asynchronously disabling SVM requires an IPI, so kvm_rebooting cannot change > within CLGI/STGI. We can check it after CLGI instead of waiting for a #GP: Seems fair; thanks! > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index 956726d867aa..e3755f5eaf81 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -4074,7 +4074,10 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct > kvm_vcpu *vcpu) > if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL)) > x86_spec_ctrl_set_guest(svm->virt_spec_ctrl); > > - svm_vcpu_enter_exit(vcpu, spec_ctrl_intercepted); > + if (unlikely(kvm_rebooting)) > + svm->vmcb->control.exit_code = SVM_EXIT_PAUSE; > + else > + svm_vcpu_enter_exit(vcpu, spec_ctrl_intercepted); > > if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL)) > x86_spec_ctrl_restore_host(svm->virt_spec_ctrl); > diff --git a/arch/x86/kvm/svm/vmenter.S b/arch/x86/kvm/svm/vmenter.S > index 8e8295e774f0..34641b3a6823 100644 > --- a/arch/x86/kvm/svm/vmenter.S > +++ b/arch/x86/kvm/svm/vmenter.S > @@ -270,23 +270,12 @@ SYM_FUNC_START(__svm_vcpu_run) > RESTORE_GUEST_SPEC_CTRL_BODY > RESTORE_HOST_SPEC_CTRL_BODY > > -10: cmpb $0, kvm_rebooting > - jne 2b > - ud2 > -30: cmpb $0, kvm_rebooting > - jne 4b > - ud2 > -50: cmpb $0, kvm_rebooting > - jne 6b > - ud2 > -70: cmpb $0, kvm_rebooting > - jne 8b > - ud2 > +10: ud2 > > _ASM_EXTABLE(1b, 10b) > - _ASM_EXTABLE(3b, 30b) > - _ASM_EXTABLE(5b, 50b) > - _ASM_EXTABLE(7b, 70b) > + _ASM_EXTABLE(3b, 10b) > + _ASM_EXTABLE(5b, 10b) > + _ASM_EXTABLE(7b, 10b) > > SYM_FUNC_END(__svm_vcpu_run) > > > Paolo > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-02 9:11 [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o Nikunj A Dadhania 2023-08-02 14:02 ` Sean Christopherson 2023-08-03 12:06 ` Peter Zijlstra @ 2023-08-04 11:14 ` Peter Zijlstra 2023-08-04 12:40 ` Nikunj A. Dadhania 2023-08-12 0:51 ` kernel test robot 3 siblings, 1 reply; 17+ messages in thread From: Peter Zijlstra @ 2023-08-04 11:14 UTC (permalink / raw) To: Nikunj A Dadhania Cc: kvm, Sean Christopherson, Josh Poimboeuf, Paolo Bonzini, Randy Dunlap, Tom Lendacky, Ravi Bangoria On Wed, Aug 02, 2023 at 02:41:07PM +0530, Nikunj A Dadhania wrote: > commit 7f4b5cde2409 ("kvm: Disable objtool frame pointer checking for > vmenter.S") had added the vmenter.o file to the exception list. > > objtool gives the following warnings in the newer kernel builds: > > arch/x86/kvm/kvm-amd.o: warning: objtool: __svm_vcpu_run+0x17d: BP used as a scratch register > arch/x86/kvm/kvm-amd.o: warning: objtool: __svm_sev_es_vcpu_run+0x72: BP used as a scratch register > I wanted to poke around a little, but can't reproduce this. I took x86_64-defconfig + KVM=m + KVM_AMD=m + UNWIND_FRAME_POINTER=y but objtool won't complain :/ What actual .config trips this? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-04 11:14 ` Peter Zijlstra @ 2023-08-04 12:40 ` Nikunj A. Dadhania 2023-08-04 20:42 ` Peter Zijlstra 0 siblings, 1 reply; 17+ messages in thread From: Nikunj A. Dadhania @ 2023-08-04 12:40 UTC (permalink / raw) To: Peter Zijlstra Cc: kvm, Sean Christopherson, Josh Poimboeuf, Paolo Bonzini, Randy Dunlap, Tom Lendacky, Ravi Bangoria [-- Attachment #1: Type: text/plain, Size: 781 bytes --] On 8/4/2023 4:44 PM, Peter Zijlstra wrote: > On Wed, Aug 02, 2023 at 02:41:07PM +0530, Nikunj A Dadhania wrote: >> commit 7f4b5cde2409 ("kvm: Disable objtool frame pointer checking for >> vmenter.S") had added the vmenter.o file to the exception list. >> >> objtool gives the following warnings in the newer kernel builds: >> >> arch/x86/kvm/kvm-amd.o: warning: objtool: __svm_vcpu_run+0x17d: BP used as a scratch register >> arch/x86/kvm/kvm-amd.o: warning: objtool: __svm_sev_es_vcpu_run+0x72: BP used as a scratch register >> > > I wanted to poke around a little, but can't reproduce this. > > I took x86_64-defconfig + KVM=m + KVM_AMD=m + UNWIND_FRAME_POINTER=y but > objtool won't complain :/ What actual .config trips this? I have attached the config. Regards, Nikunj [-- Attachment #2: config.gz --] [-- Type: application/x-gzip, Size: 38483 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-04 12:40 ` Nikunj A. Dadhania @ 2023-08-04 20:42 ` Peter Zijlstra 0 siblings, 0 replies; 17+ messages in thread From: Peter Zijlstra @ 2023-08-04 20:42 UTC (permalink / raw) To: Nikunj A. Dadhania Cc: kvm, Sean Christopherson, Josh Poimboeuf, Paolo Bonzini, Randy Dunlap, Tom Lendacky, Ravi Bangoria On Fri, Aug 04, 2023 at 06:10:19PM +0530, Nikunj A. Dadhania wrote: > On 8/4/2023 4:44 PM, Peter Zijlstra wrote: > > On Wed, Aug 02, 2023 at 02:41:07PM +0530, Nikunj A Dadhania wrote: > >> commit 7f4b5cde2409 ("kvm: Disable objtool frame pointer checking for > >> vmenter.S") had added the vmenter.o file to the exception list. > >> > >> objtool gives the following warnings in the newer kernel builds: > >> > >> arch/x86/kvm/kvm-amd.o: warning: objtool: __svm_vcpu_run+0x17d: BP used as a scratch register > >> arch/x86/kvm/kvm-amd.o: warning: objtool: __svm_sev_es_vcpu_run+0x72: BP used as a scratch register > >> > > > > I wanted to poke around a little, but can't reproduce this. > > > > I took x86_64-defconfig + KVM=m + KVM_AMD=m + UNWIND_FRAME_POINTER=y but > > objtool won't complain :/ What actual .config trips this? > > I have attached the config. Thanks, that does reproduce, although now I'm left wondering what exactly makes it go beep. Also, URGH, I thought the build was broken until I noticed it had BTF on :/ ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o 2023-08-02 9:11 [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o Nikunj A Dadhania ` (2 preceding siblings ...) 2023-08-04 11:14 ` Peter Zijlstra @ 2023-08-12 0:51 ` kernel test robot 3 siblings, 0 replies; 17+ messages in thread From: kernel test robot @ 2023-08-12 0:51 UTC (permalink / raw) To: Nikunj A Dadhania, kvm, Sean Christopherson Cc: llvm, oe-kbuild-all, Nikunj A Dadhania, Josh Poimboeuf, Peter Zijlstra, Paolo Bonzini, Randy Dunlap, Tom Lendacky, Ravi Bangoria Hi Nikunj, kernel test robot noticed the following build errors: [auto build test ERROR on kvm/queue] [also build test ERROR on mst-vhost/linux-next linus/master v6.5-rc5 next-20230809] [cannot apply to kvm/linux-next] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Nikunj-A-Dadhania/KVM-SVM-Add-exception-to-disable-objtool-warning-for-kvm-amd-o/20230802-171219 base: https://git.kernel.org/pub/scm/virt/kvm/kvm.git queue patch link: https://lore.kernel.org/r/20230802091107.1160320-1-nikunj%40amd.com patch subject: [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o config: i386-randconfig-i013-20230812 (https://download.01.org/0day-ci/archive/20230812/202308120845.gkEMVH5a-lkp@intel.com/config) compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07) reproduce: (https://download.01.org/0day-ci/archive/20230812/202308120845.gkEMVH5a-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202308120845.gkEMVH5a-lkp@intel.com/ All errors (new ones prefixed by >>): >> arch/x86/kvm/svm/vmenter.S:392:1: error: invalid instruction mnemonic 'stack_frame_non_standard_fp' STACK_FRAME_NON_STANDARD_FP(__svm_sev_es_vcpu_run) ^~~~~~~~~~~~~~~~~~~~~~~~~~~ vim +/stack_frame_non_standard_fp +392 arch/x86/kvm/svm/vmenter.S 352 353 /* Clobbers RAX, RCX, RDX. */ 354 RESTORE_HOST_SPEC_CTRL 355 356 /* 357 * Mitigate RETBleed for AMD/Hygon Zen uarch. RET should be 358 * untrained as soon as we exit the VM and are back to the 359 * kernel. This should be done before re-enabling interrupts 360 * because interrupt handlers won't sanitize RET if the return is 361 * from the kernel. 362 */ 363 UNTRAIN_RET 364 365 /* "Pop" @spec_ctrl_intercepted. */ 366 pop %_ASM_BX 367 368 pop %_ASM_BX 369 370 #ifdef CONFIG_X86_64 371 pop %r12 372 pop %r13 373 pop %r14 374 pop %r15 375 #else 376 pop %esi 377 pop %edi 378 #endif 379 pop %_ASM_BP 380 RET 381 382 RESTORE_GUEST_SPEC_CTRL_BODY 383 RESTORE_HOST_SPEC_CTRL_BODY 384 385 3: cmpb $0, kvm_rebooting 386 jne 2b 387 ud2 388 389 _ASM_EXTABLE(1b, 3b) 390 391 SYM_FUNC_END(__svm_sev_es_vcpu_run) > 392 STACK_FRAME_NON_STANDARD_FP(__svm_sev_es_vcpu_run) -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-08-12 0:52 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-02 9:11 [PATCH] KVM: SVM: Add exception to disable objtool warning for kvm-amd.o Nikunj A Dadhania 2023-08-02 14:02 ` Sean Christopherson 2023-08-03 6:25 ` Nikunj A. Dadhania 2023-08-03 12:06 ` Peter Zijlstra 2023-08-03 18:06 ` Paolo Bonzini 2023-08-03 19:07 ` Peter Zijlstra 2023-08-04 3:25 ` Nikunj A. Dadhania 2023-08-04 10:20 ` Paolo Bonzini 2023-08-04 20:48 ` Peter Zijlstra 2023-08-04 23:19 ` Josh Poimboeuf 2023-08-05 0:55 ` Peter Zijlstra 2023-08-10 14:17 ` Paolo Bonzini 2023-08-10 21:14 ` Peter Zijlstra 2023-08-04 11:14 ` Peter Zijlstra 2023-08-04 12:40 ` Nikunj A. Dadhania 2023-08-04 20:42 ` Peter Zijlstra 2023-08-12 0:51 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox