Re: [RFC PATCH v5 083/104] KVM: x86: Split core of hypercall emulation to helper function

[Paolo Bonzini <pbonzini@xxxxxxxxxx>]

On 3/21/22 19:32, Sagi Shahar wrote:
> On Fri, Mar 4, 2022 at 12:00 PM <isaku.yamahata@xxxxxxxxx> wrote:
> > From: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> >
> > By necessity, TDX will use a different register ABI for hypercalls.
> > Break out the core functionality so that it may be reused for TDX.
> >
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> > Signed-off-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> > ---
> >   arch/x86/include/asm/kvm_host.h |  4 +++
> >   arch/x86/kvm/x86.c              | 54 ++++++++++++++++++++-------------
> >   2 files changed, 37 insertions(+), 21 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> > index 8dab9f16f559..33b75b0e3de1 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -1818,6 +1818,10 @@ void kvm_request_apicv_update(struct kvm *kvm, bool activate,
> >   void __kvm_request_apicv_update(struct kvm *kvm, bool activate,
> >                                  unsigned long bit);
> >
> > +unsigned long __kvm_emulate_hypercall(struct kvm_vcpu *vcpu, unsigned long nr,
> > +                                     unsigned long a0, unsigned long a1,
> > +                                     unsigned long a2, unsigned long a3,
> > +                                     int op_64_bit);
> >   int kvm_emulate_hypercall(struct kvm_vcpu *vcpu);
> >
> >   int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa, u64 error_code,
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 314ae43e07bf..9acb33a17445 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -9090,26 +9090,15 @@ static int complete_hypercall_exit(struct kvm_vcpu *vcpu)
> >          return kvm_skip_emulated_instruction(vcpu);
> >   }
> >
> > -int kvm_emulate_hypercall(struct kvm_vcpu *vcpu)
> > +unsigned long __kvm_emulate_hypercall(struct kvm_vcpu *vcpu, unsigned long nr,
> > +                                     unsigned long a0, unsigned long a1,
> > +                                     unsigned long a2, unsigned long a3,
> > +                                     int op_64_bit)
> >   {
> > -       unsigned long nr, a0, a1, a2, a3, ret;
> > -       int op_64_bit;
> > -
> > -       if (kvm_xen_hypercall_enabled(vcpu->kvm))
> > -               return kvm_xen_hypercall(vcpu);
> > -
> > -       if (kvm_hv_hypercall_enabled(vcpu))
> > -               return kvm_hv_hypercall(vcpu);

Please keep Xen and Hyper-V hypercalls to kvm_emulate_hypercall (more on 
this in the reply to patch 89).  __kvm_emulate_hypercall should only 
handle KVM hypercalls.

> > +       if (static_call(kvm_x86_get_cpl)(vcpu) != 0) {
> > +               ret = -KVM_EPERM;
> > +               goto out;
> > +       }

Is this guaranteed by TDG.VP.VMCALL?

Paolo

> > +       ret = __kvm_emulate_hypercall(vcpu, nr, a0, a1, a2, a3, op_64_bit);
> >   out:
> >          if (!op_64_bit)
> >                  ret = (u32)ret;
> > --
> > 2.25.1
>
> Sagi