[01/13] KVM: nVMX: Preserve exception priority irrespective of exiting behavior
diff mbox series

Message ID 20200423022550.15113-2-sean.j.christopherson@intel.com
State New
Headers show
Series
  • KVM: x86: Event fixes and cleanup
Related show

Commit Message

Sean Christopherson April 23, 2020, 2:25 a.m. UTC
Short circuit vmx_check_nested_events() if an exception is pending and
needs to be injected into L2, priority between coincident events is not
dependent on exiting behavior.  This fixes a bug where a single-step #DB
that is not intercepted by L1 is incorrectly dropped due to servicing a
VMX Preemption Timer VM-Exit.

Injected exceptions also need to be blocked if nested VM-Enter is
pending or an exception was already injected, otherwise injecting the
exception could overwrite an existing event injection from L1.
Technically, this scenario should be impossible, i.e. KVM shouldn't
inject its own exception during nested VM-Enter.  This will be addressed
in a future patch.

Note, event priority between SMI, NMI and INTR is incorrect for L2, e.g.
SMI should take priority over VM-Exit on NMI/INTR, and NMI that is
injected into L2 should take priority over VM-Exit INTR.  This will also
be addressed in a future patch.

Fixes: b6b8a1451fc4 ("KVM: nVMX: Rework interception of IRQs and NMIs")
Reported-by: Jim Mattson <jmattson@google.com>
Cc: Oliver Upton <oupton@google.com>
Cc: Peter Shier <pshier@google.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kvm/vmx/nested.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Jim Mattson April 28, 2020, 6:54 p.m. UTC | #1
On Wed, Apr 22, 2020 at 7:26 PM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> Short circuit vmx_check_nested_events() if an exception is pending and
> needs to be injected into L2, priority between coincident events is not
> dependent on exiting behavior.  This fixes a bug where a single-step #DB
> that is not intercepted by L1 is incorrectly dropped due to servicing a
> VMX Preemption Timer VM-Exit.
>
> Injected exceptions also need to be blocked if nested VM-Enter is
> pending or an exception was already injected, otherwise injecting the
> exception could overwrite an existing event injection from L1.
> Technically, this scenario should be impossible, i.e. KVM shouldn't
> inject its own exception during nested VM-Enter.  This will be addressed
> in a future patch.
>
> Note, event priority between SMI, NMI and INTR is incorrect for L2, e.g.
> SMI should take priority over VM-Exit on NMI/INTR, and NMI that is
> injected into L2 should take priority over VM-Exit INTR.  This will also
> be addressed in a future patch.
>
> Fixes: b6b8a1451fc4 ("KVM: nVMX: Rework interception of IRQs and NMIs")
> Reported-by: Jim Mattson <jmattson@google.com>
> Cc: Oliver Upton <oupton@google.com>
> Cc: Peter Shier <pshier@google.com>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Reviewed-by: Jim Mattson <jmattson@google.com>
Oliver Upton April 28, 2020, 8:07 p.m. UTC | #2
On Tue, Apr 28, 2020 at 11:54 AM Jim Mattson <jmattson@google.com> wrote:
>
> On Wed, Apr 22, 2020 at 7:26 PM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> >
> > Short circuit vmx_check_nested_events() if an exception is pending and
> > needs to be injected into L2, priority between coincident events is not
> > dependent on exiting behavior.  This fixes a bug where a single-step #DB
> > that is not intercepted by L1 is incorrectly dropped due to servicing a
> > VMX Preemption Timer VM-Exit.
> >
> > Injected exceptions also need to be blocked if nested VM-Enter is
> > pending or an exception was already injected, otherwise injecting the
> > exception could overwrite an existing event injection from L1.
> > Technically, this scenario should be impossible, i.e. KVM shouldn't
> > inject its own exception during nested VM-Enter.  This will be addressed
> > in a future patch.
> >
> > Note, event priority between SMI, NMI and INTR is incorrect for L2, e.g.
> > SMI should take priority over VM-Exit on NMI/INTR, and NMI that is
> > injected into L2 should take priority over VM-Exit INTR.  This will also
> > be addressed in a future patch.
> >
> > Fixes: b6b8a1451fc4 ("KVM: nVMX: Rework interception of IRQs and NMIs")
> > Reported-by: Jim Mattson <jmattson@google.com>
> > Cc: Oliver Upton <oupton@google.com>
> > Cc: Peter Shier <pshier@google.com>
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Reviewed-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Oliver Upton <oupton@google.com>

Patch
diff mbox series

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index f228339cd0a0..dc7315b31fee 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -3716,11 +3716,11 @@  static int vmx_check_nested_events(struct kvm_vcpu *vcpu)
 	/*
 	 * Process any exceptions that are not debug traps before MTF.
 	 */
-	if (vcpu->arch.exception.pending &&
-	    !vmx_pending_dbg_trap(vcpu) &&
-	    nested_vmx_check_exception(vcpu, &exit_qual)) {
+	if (vcpu->arch.exception.pending && !vmx_pending_dbg_trap(vcpu)) {
 		if (block_nested_events)
 			return -EBUSY;
+		if (!nested_vmx_check_exception(vcpu, &exit_qual))
+			goto no_vmexit;
 		nested_vmx_inject_exception_vmexit(vcpu, exit_qual);
 		return 0;
 	}
@@ -3733,10 +3733,11 @@  static int vmx_check_nested_events(struct kvm_vcpu *vcpu)
 		return 0;
 	}
 
-	if (vcpu->arch.exception.pending &&
-	    nested_vmx_check_exception(vcpu, &exit_qual)) {
+	if (vcpu->arch.exception.pending) {
 		if (block_nested_events)
 			return -EBUSY;
+		if (!nested_vmx_check_exception(vcpu, &exit_qual))
+			goto no_vmexit;
 		nested_vmx_inject_exception_vmexit(vcpu, exit_qual);
 		return 0;
 	}
@@ -3771,6 +3772,7 @@  static int vmx_check_nested_events(struct kvm_vcpu *vcpu)
 		return 0;
 	}
 
+no_vmexit:
 	vmx_complete_nested_posted_interrupt(vcpu);
 	return 0;
 }