* [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-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-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
` (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