From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH] KVM: nVMX: Fix bug of injecting L2 exception into L1 Date: Tue, 9 Jan 2018 11:08:36 +0100 Message-ID: References: <1511108743-11752-1-git-send-email-liran.alon@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: jmattson@google.com, wanpeng.li@hotmail.com, idan.brown@oracle.com, Krish Sadhukhan To: Liran Alon , rkrcmar@redhat.com, kvm@vger.kernel.org Return-path: Received: from mail-lf0-f67.google.com ([209.85.215.67]:46306 "EHLO mail-lf0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751098AbeAIKIm (ORCPT ); Tue, 9 Jan 2018 05:08:42 -0500 Received: by mail-lf0-f67.google.com with SMTP id a12so15276582lfe.13 for ; Tue, 09 Jan 2018 02:08:41 -0800 (PST) In-Reply-To: <1511108743-11752-1-git-send-email-liran.alon@oracle.com> Content-Language: en-US Sender: kvm-owner@vger.kernel.org List-ID: On 19/11/2017 17:25, Liran Alon wrote: > kvm_clear_exception_queue() should clear pending exception. > This also includes exceptions which were only marked pending but not > yet injected. This is because exception.pending is used for both L1 > and L2 to determine if an exception should be raised to guest. > Note that an exception which is pending but not yet injected will > be raised again once the guest will be resumed. > > Consider the following scenario: > 1) L0 KVM with ignore_msrs=false. > 2) L1 prepare vmcs12 with the following: > a) No intercepts on MSR (MSR_BITMAP exist and is filled with 0). > b) No intercept for #GP. > c) vmx-preemption-timer is configured. > 3) L1 enters into L2. > 4) L2 reads an unhandled MSR that exists in MSR_BITMAP > (such as 0x1fff). > > L2 RDMSR could be handled as described below: > 1) L2 exits to L0 on RDMSR and calls handle_rdmsr(). > 2) handle_rdmsr() calls kvm_inject_gp() which sets > KVM_REQ_EVENT, exception.pending=true and exception.injected=false. > 3) vcpu_enter_guest() consumes KVM_REQ_EVENT and calls > inject_pending_event() which calls vmx_check_nested_events() > which sees that exception.pending=true but > nested_vmx_check_exception() returns 0 and therefore does nothing at > this point. However let's assume it later sees vmx-preemption-timer > expired and therefore exits from L2 to L1 by calling > nested_vmx_vmexit(). > 4) nested_vmx_vmexit() calls prepare_vmcs12() > which calls vmcs12_save_pending_event() but it does nothing as > exception.injected is false. Also prepare_vmcs12() calls > kvm_clear_exception_queue() which does nothing as > exception.injected is already false. > 5) We now return from vmx_check_nested_events() with 0 while still > having exception.pending=true! > 6) Therefore inject_pending_event() continues > and we inject L2 exception to L1!... > > This commit will fix above issue by changing step (4) to > clear exception.pending in kvm_clear_exception_queue(). > > Fixes: 664f8e26b00c ("KVM: X86: Fix loss of exception which has not > yet been injected") > > Signed-off-by: Liran Alon > Reviewed-by: Nikita Leshenko > Reviewed-by: Krish Sadhukhan > Signed-off-by: Krish Sadhukhan > --- > arch/x86/kvm/vmx.c | 1 - > arch/x86/kvm/x86.h | 1 + > 2 files changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 7c3522a989d0..bee08782c781 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -11035,7 +11035,6 @@ static int vmx_check_nested_events(struct kvm_vcpu *vcpu, bool external_intr) > if (vmx->nested.nested_run_pending) > return -EBUSY; > nested_vmx_inject_exception_vmexit(vcpu, exit_qual); > - vcpu->arch.exception.pending = false; > return 0; > } > > diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h > index d0b95b7a90b4..6d112d8f799c 100644 > --- a/arch/x86/kvm/x86.h > +++ b/arch/x86/kvm/x86.h > @@ -12,6 +12,7 @@ > > static inline void kvm_clear_exception_queue(struct kvm_vcpu *vcpu) > { > + vcpu->arch.exception.pending = false; > vcpu->arch.exception.injected = false; > } > > Finally queued, thanks. Paolo