Re: [PATCH] KVM: nVMX: fix instruction skipping during emulated vm-entry

["Radim Krčmář" <rkrcmar@redhat.com>]

2016-12-20 16:24+0100, Radim Krčmář:
> 2016-12-19 19:15-0800, Kyle Huey:
> > On Mon, Dec 19, 2016 at 12:48 PM, David Matlack <dmatlack@google.com> wrote:
> >> kvm_skip_emulated_instruction() should not be called after emulating
> >> a VM-entry failure during or after loading guest state
> >> (nested_vmx_entry_failure()). Otherwise the L1 hypervisor is resumed
> >> some number of bytes past vmcs->host_rip.
> > 
> > Ah, I see.  Sorry for that regression.
> > 
> > These paths are supposed to trigger TF-induced singlestep exceptions
> > though.  Quoting from the Intel SDM (Vol 3, Chapter 26)
> > 
> > "EFLAGS.TF = 1 causes a VM-entry instruction to generate a single-step
> > debug exception only if failure of one of the checks in Section 26.1
> > and Section 26.2 causes control to pass to the following instruction.
> > A VM-entry does not generate a single-step debug exception in any of
> > the following cases: (1) the instruction generates a fault; (2)
> > failure of one of the checks in Section 26.3 or in loading MSRs causes
> > processor state to be loaded from the hoststate area of the VMCS; or
> > (3) the instruction passes all checks in Section 26.1, Section 26.2,
> > and Section 26.3 and there is no failure in loading MSRs"
> 
> Changed cases are in section 26.3 => not generating #DB is correct,
> 
> Reviewed-by: Radim Krčmář <rkrcmar@redhat.com>
> 
> Sorry for missing that while applying, I wonder if there is a reason why
> we didn't check them after enter_guest_mode() ...
> 
> >> @@ -10517,7 +10517,7 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
> >>                     ia32e != !!(vmcs12->host_ia32_efer & EFER_LME)) {
> >>                         nested_vmx_entry_failure(vcpu, vmcs12,
> >>                                 EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT);
> >> -                       goto out;
> >> +                       return 1;

My attention is in shambles these days ... this one looks like it is in
section 26.2.2 and should therefore be

  nested_vmx_failValid(vcpu, VMXERR_ENTRY_INVALID_HOST_STATE_FIELD);
  goto out;

Not a problem of this patch though, I'll go for a minor refactoring
after a beer.