Re: [RESEND PATCH 04/13] KVM: x86: Drop EMULTYPE_NO_UD_ON_FAIL as a standalone type

[Sean Christopherson]

On Fri, Aug 23, 2019 at 04:32:05PM +0300, Liran Alon wrote:
> 
> > On 23 Aug 2019, at 16:21, Liran Alon <liran.alon@xxxxxxxxxx> wrote:
> > 
> >> On 23 Aug 2019, at 4:07, Sean Christopherson <sean.j.christopherson@xxxxxxxxx> wrote:
> >> 
> >> The "no #UD on fail" is used only in the VMWare case, and for the VMWare
> >> scenario it really means "#GP instead of #UD on fail".  Remove the flag
> >> in preparation for moving all fault injection into the emulation flow
> >> itself, which in turn will allow eliminating EMULATE_DONE and company.
> >> 
> >> Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> > 
> > When I created the commit which introduced this e23661712005 ("KVM: x86:
> > Add emulation_type to not raise #UD on emulation failure") I intentionally
> > introduced a new flag to emulation_type instead of using EMULTYPE_VMWARE as
> > I thought itâs weird to couple this behaviour specifically with VMware
> > emulation.  As it made sense to me that there could be more scenarios in
> > which some VMExit handler would like to use the x86 emulator but in case of
> > failure want to decide what would be the failure handling from the outside.
> > I also didnât want the x86 emulator to be aware of VMware interception
> > internals.
> > 
> > Having said that, one could argue that the x86 emulator already knows about
> > the VMware interception internals because of how x86_emulate_instruction()
> > use is_vmware_backdoor_opcode() and from the mere existence of
> > EMULTYPE_VMWARE. So I think itâs legit to decide that we will just move all
> > the VMware interception logic into the x86 emulator. Including handling
> > emulation failures. But then, I would make this patch of yours to also
> > modify handle_emulation_failure() to queue #GP to guest directly instead of
> > #GP intercept in VMX/SVM to do so.  I see you do it in a later patch "KVM:
> > x86: Move #GP injection for VMware into x86_emulate_instruction()" but I
> > think this should just be squashed with this patch to make sense.
> > 
> > To sum-up, I agree with your approach but I recommend you squash this patch
> > and patch 6 of the series to one and change commit message to explain that
> > you just move entire handling of VMware interception into the x86 emulator.
> > Instead of providing explanations such as VMware emulation is the only one
> > that use âno #UD on failâ.
> 
> After reading patch 5 as-well, I would recommend to first apply patch 5
> (filter out #GP with error-code != 0) and only then apply 4+6.

Works for me.