From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [PATCH] KVM: nVMX: fix instruction skipping during emulated vm-entry Date: Tue, 20 Dec 2016 16:24:31 +0100 Message-ID: <20161220152430.GB21302@potion> References: <1482180521-71290-1-git-send-email-dmatlack@google.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Cc: David Matlack , kvm list , Paolo Bonzini To: Kyle Huey Return-path: Received: from mx1.redhat.com ([209.132.183.28]:54676 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757461AbcLTPYe (ORCPT ); Tue, 20 Dec 2016 10:24:34 -0500 Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: 2016-12-19 19:15-0800, Kyle Huey: > On Mon, Dec 19, 2016 at 12:48 PM, David Matlack 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ář Sorry for missing that while applying, I wonder if there is a reason why we didn't check them after enter_guest_mode() ... >> Fixes: eb2775621701e6ee3ea2a474437d04e93ccdcb2f >> Signed-off-by: David Matlack >> --- >> arch/x86/kvm/vmx.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index c41d7ff..0e7ad72 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -10474,12 +10474,12 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) >> !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4)) { >> nested_vmx_entry_failure(vcpu, vmcs12, >> EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT); >> - goto out; >> + return 1; >> } >> if (vmcs12->vmcs_link_pointer != -1ull) { >> nested_vmx_entry_failure(vcpu, vmcs12, >> EXIT_REASON_INVALID_STATE, ENTRY_FAIL_VMCS_LINK_PTR); >> - goto out; >> + return 1; >> } >> >> /* >> @@ -10499,7 +10499,7 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch) >> ia32e != !!(vmcs12->guest_ia32_efer & EFER_LME))) { >> nested_vmx_entry_failure(vcpu, vmcs12, >> EXIT_REASON_INVALID_STATE, ENTRY_FAIL_DEFAULT); >> - goto out; >> + return 1; >> } >> } >> >> @@ -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; >> } >> } >> >> -- >> 2.8.0.rc3.226.g39d4020 >>