On Tue, Jan 15, 2019 at 08:13:22AM +0100, Paolo Bonzini wrote:
> On 15/01/19 08:04, Qian Cai wrote:
> >
> >
> > On 1/15/19 1:44 AM, Qian Cai wrote:
> >> compilation warning since v5.0-rc1,
> >>
> >> arch/x86/kvm/vmx/vmx.o: warning: objtool: vmx_vcpu_run.part.17()+0x3171:
> >> call without frame pointer save/setup
The warning is complaining about vmx_vcpu_run() in vmx.c, not vmenter.S.
The rule being "broken" is that a call is made without creating a stack
frame, and vmx_vmenter() obviously makes no calls.
E.g., manually running objtool check:
$ tools/objtool/objtool check arch/x86/kvm/vmx/vmenter.o
$ tools/objtool/objtool check arch/x86/kvm/vmx/vmx.o
arch/x86/kvm/vmx/vmx.o: warning: objtool: vmx_vcpu_run.part.19()+0x83e: call without frame pointer save/setup
I put "broken" in quotes because AFAICT we're not actually violating the
rule. From tools/objtool/Documentation/stack-validation.txt:
If it's a GCC-compiled .c file, the error may be because the function
uses an inline asm() statement which has a "call" instruction. An
asm() statement with a call instruction must declare the use of the
stack pointer in its output operand. On x86_64, this means adding
the ASM_CALL_CONSTRAINT as an output constraint:
asm volatile("call func" : ASM_CALL_CONSTRAINT);
Otherwise the stack frame may not get created before the call.
The asm() blob that calls vmx_vmenter() uses ASM_CALL_CONSTRAINT, and
the resulting asm output generates a frame pointer, e.g. this is from
the vmx.o that objtool warns on:
Dump of assembler code for function vmx_vcpu_run:
0x0000000000007440 <+0>: e8 00 00 00 00 callq 0x7445 <vmx_vcpu_run+5>
0x0000000000007445 <+5>: 55 push %rbp
0x0000000000007446 <+6>: 48 89 e5 mov %rsp,%rbp
The warning only shows up in certain configs, e.g. I was only able to
reproduce this using the .config provided by lkp. Even explicitly
enabling CONFIG_FRAME_POINTERS and CONFIG_STACK_VALIDATION didn't
trigger the warning using my usual config.
And all that being said, I'm pretty sure this isn't related to the call
to vmx_vmenter() at all, but rather is something that was exposed by
removing __noclone from vmx_vcpu_run().
E.g. I still get the warning if I comment out the call to vmx_vmenter,
it just shifts to something else (and continues to shift I comment out
more calls). The warning goes away if I re-add __noclone, regardless
of whether or not commit 2bcbd406715d ("Revert "compiler-gcc: disable
-ftracer for __noclone functions"") is applied.
0x6659 is in vmx_vcpu_run (./arch/x86/include/asm/spec-ctrl.h:43).
38 * Avoids writing to the MSR if the content/bits are the same
39 */
40 static inline
41 void x86_spec_ctrl_restore_host(u64 guest_spec_ctrl, u64 guest_virt_spec_ctrl)
42 {
43 x86_virt_spec_ctrl(guest_spec_ctrl, guest_virt_spec_ctrl, false);
44 }
45
46 /* AMD specific Speculative Store Bypass MSR data */
47 extern u64 x86_amd_ls_cfg_base;
What's really baffling is that we explicitly tell objtool to not do stack
metadata validation on vmx_vcpu_run():
STACK_FRAME_NON_STANDARD(vmx_vcpu_run);
As an aside, we might be able to remove STACK_FRAME_NON_STANDARD, assuming
we get this warning sorted out.
> >> Fixes: 453eafbe65f (KVM: VMX: Move VM-Enter + VM-Exit handling to
> >> non-inline sub-routines)
> >
> > Oops, wrong fix. Back to square one.
> >
>
> Hmm, maybe like this:
>
> diff --git a/arch/x86/kvm/vmx/vmenter.S b/arch/x86/kvm/vmx/vmenter.S
> index bcef2c7e9bc4..33122fa9d4bd 100644
> --- a/arch/x86/kvm/vmx/vmenter.S
> +++ b/arch/x86/kvm/vmx/vmenter.S
> @@ -26,19 +26,17 @@ ENTRY(vmx_vmenter)
> ret
>
> 2: vmlaunch
> +3:
> ret
>
> -3: cmpb $0, kvm_rebooting
> - jne 4f
> - call kvm_spurious_fault
> -4: ret
> -
> .pushsection .fixup, "ax"
> -5: jmp 3b
> +4: cmpb $0, kvm_rebooting
> + jne 3b
> + jmp kvm_spurious_fault
> .popsection
>
> - _ASM_EXTABLE(1b, 5b)
> - _ASM_EXTABLE(2b, 5b)
> + _ASM_EXTABLE(1b, 4b)
> + _ASM_EXTABLE(2b, 4b)
>
> ENDPROC(vmx_vmenter)
>
>
> Paolo
