From mboxrd@z Thu Jan  1 00:00:00 1970
From: Radim =?utf-8?B?S3LEjW3DocWZ?= <rkrcmar@redhat.com>
Subject: Re: [PATCH] KVM: nVMX: fix instruction skipping during emulated
 vm-entry
Date: Tue, 20 Dec 2016 18:47:36 +0100
Message-ID: <20161220174735.GA13251@potion>
References: <1482180521-71290-1-git-send-email-dmatlack@google.com>
 <CAP045AqQF4Hs8hP2dC2TbFXCMikGqhg2WeVr0SoYZ-=mnaGbXQ@mail.gmail.com>
 <20161220152430.GB21302@potion>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Cc: David Matlack <dmatlack@google.com>,
        kvm list <kvm@vger.kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>
To: Kyle Huey <me@kylehuey.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:36774 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1750797AbcLTRrk (ORCPT <rfc822;kvm@vger.kernel.org>);
        Tue, 20 Dec 2016 12:47:40 -0500
Content-Disposition: inline
In-Reply-To: <20161220152430.GB21302@potion>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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.