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@xxxxxxxxx> 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.