Re: [PATCH] KVM: x86: Add kvm_x86_ops hook to short circuit emulation

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Wed, Sep 16, 2020 at 09:31:22AM +0800, Wanpeng Li wrote:
> On Wed, 16 Sep 2020 at 07:29, Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> >
> > Replace the existing kvm_x86_ops.need_emulation_on_page_fault() with a
> > more generic is_emulatable(), and unconditionally call the new function
> > in x86_emulate_instruction().
> >
> > KVM will use the generic hook to support multiple security related
> > technologies that prevent emulation in one way or another.  Similar to
> > the existing AMD #NPF case where emulation of the current instruction is
> > not possible due to lack of information, AMD's SEV-ES and Intel's SGX
> > and TDX will introduce scenarios where emulation is impossible due to
> > the guest's register state being inaccessible.  And again similar to the
> > existing #NPF case, emulation can be initiated by kvm_mmu_page_fault(),
> > i.e. outside of the control of vendor-specific code.
> >
> > While the cause and architecturally visible behavior of the various
> > cases are different, e.g. SGX will inject a #UD, AMD #NPF is a clean
> > resume or complete shutdown, and SEV-ES and TDX "return" an error, the
> > impact on the common emulation code is identical: KVM must stop
> > emulation immediately and resume the guest.
> >
> > Query is_emulatable() in handle_ud() as well so that the
> > force_emulation_prefix code doesn't incorrectly modify RIP before
> > calling emulate_instruction() in the absurdly unlikely scenario that
> > KVM encounters forced emulation in conjunction with "do not emulate".

...

> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 539ea1cd6020..5208217049d9 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -5707,6 +5707,9 @@ int handle_ud(struct kvm_vcpu *vcpu)
> >         char sig[5]; /* ud2; .ascii "kvm" */
> >         struct x86_exception e;
> >
> > +       if (unlikely(!kvm_x86_ops.is_emulatable(vcpu, NULL, 0)))
> > +               return 1;
> > +
> 
> Both VMX and SVM scenarios always fail this check.

Ah, right.  This patch was extracted from my SGX series, in which case there
would be a follow-up patch to add a VMX scenario where is_emulated() could
return false.

The intent of posting the patch standalone is so that SGX, SEV-ES, and/or TDX
have "ready to go" support in upstream, i.e. can change only the VMX/SVM
implementation of is_emulated().  I'm a-ok dropping the handle_ud() change,
or even the whole patch, until one of the above three is actually ready for
inclusion.