On Mon, Feb 25, 2019 at 08:57:08AM +0100, Jiri Slaby wrote:
> Hi,
>
> On 25. 01. 19, 16:41, Sean Christopherson wrote:
> > As evidenced by the myriad patches leading up to this moment, using
> > an inline asm blob for vCPU-run is nothing short of horrific. It's also
> > been called "unholy", "an abomination" and likely a whole host of other
> > names that would violate the Code of Conduct if recorded here and now.
> >
> > The code is relocated nearly verbatim, e.g. quotes, newlines, tabs and
> > __stringify need to be dropped, but other than those cosmetic changes
> > the only functional changees are to add the "call" and replace the final
> > "jmp" with a "ret".
> >
> > Note that STACK_FRAME_NON_STANDARD is also dropped from __vmx_vcpu_run().
> >
> > Suggested-by: Andi Kleen <ak@xxxxxxxxxxxxxxx>
> > Suggested-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> > ---
> > arch/x86/kvm/vmx/vmenter.S | 147 +++++++++++++++++++++++++++++++++++++
> > arch/x86/kvm/vmx/vmx.c | 138 +---------------------------------
> > 2 files changed, 148 insertions(+), 137 deletions(-)
> >
> > diff --git a/arch/x86/kvm/vmx/vmenter.S b/arch/x86/kvm/vmx/vmenter.S
> > index bcef2c7e9bc4..db223cfe9812 100644
> > --- a/arch/x86/kvm/vmx/vmenter.S
> > +++ b/arch/x86/kvm/vmx/vmenter.S
> ...
> > @@ -55,3 +82,123 @@ ENDPROC(vmx_vmenter)
> > ENTRY(vmx_vmexit)
> > ret
> > ENDPROC(vmx_vmexit)
> > +
> > +/**
> > + * ____vmx_vcpu_run - Run a vCPU via a transition to VMX guest mode
> > + * @vmx: struct vcpu_vmx *
> > + * @regs: unsigned long * (to guest registers)
> > + * %RBX: VMCS launched status (non-zero indicates already launched)
> > + *
> > + * Returns:
> > + * %RBX is 0 on VM-Exit, 1 on VM-Fail
> > + */
> > +ENTRY(____vmx_vcpu_run)
> > + push %_ASM_BP
> > + mov %_ASM_SP, %_ASM_BP
>
> Was there any particular reason not to use FRAME_BEGIN (and FRAME_END
> below)? It would compile to a nop on !CONFIG_FRAME_POINTER configs.
>
> I understand this patch is only a move of the code from .c to .S. So I
> would send a cleanup patch, but I just wonder if there is anything
> blocking it?
RBP needs to be saved/restored unconditionally as it will be crushed
by VM-Enter.
commit 63c73aa07fcabc090661a586f7ae5200a0fc5cb4
Author: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
Date: Fri Jan 25 07:41:11 2019 -0800
KVM: VMX: Create a stack frame in vCPU-run
...in preparation for moving to a proper assembly sub-routnine.
vCPU-run isn't a leaf function since it calls vmx_update_host_rsp()
and vmx_vmenter(). And since we need to save/restore RBP anyways,
unconditionally creating the frame costs a single MOV, i.e. don't
bother keying off CONFIG_FRAME_POINTER or using FRAME_BEGIN, etc...
Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
