KVM: X86: Fix loss of exception which has not yet been injected

vmx_complete_interrupts() assumes that the exception is always injected,
so it can be dropped by kvm_clear_exception_queue().  However,
an exception cannot be injected immediately if it is: 1) originally
destined to a nested guest; 2) trapped to cause a vmexit; 3) happening
right after VMLAUNCH/VMRESUME, i.e. when nested_run_pending is true.

This patch applies to exceptions the same algorithm that is used for
NMIs, replacing exception.reinject with "exception.injected" (equivalent
to nmi_injected).

exception.pending now represents an exception that is queued and whose
side effects (e.g., update RFLAGS.RF or DR7) have not been applied yet.
If exception.pending is true, the exception might result in a nested
vmexit instead, too (in which case the side effects must not be applied).

exception.injected instead represents an exception that is going to be
injected into the guest at the next vmentry.

Reported-by: Radim Krčmář <rkrcmar@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index b4d4f5151489074a1aecb52458d6161f44bb5e62..afa70749903e57351fe22f4b1480860952ecd9e2 100644 (file)
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -547,8 +547,8 @@ struct kvm_vcpu_arch {
 
        struct kvm_queued_exception {
                bool pending;
+               bool injected;
                bool has_error_code;
-               bool reinject;
                u8 nr;
                u32 error_code;
                u8 nested_apf;

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 32d5e2f00f3aaf3fcd1dd509490a208dff467eb9..9f6fb0ef206cd6cbb82250c54df84e9e090b9b3d 100644 (file)
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -655,7 +655,7 @@ static void svm_queue_exception(struct kvm_vcpu *vcpu)
        struct vcpu_svm *svm = to_svm(vcpu);
        unsigned nr = vcpu->arch.exception.nr;
        bool has_error_code = vcpu->arch.exception.has_error_code;
-       bool reinject = vcpu->arch.exception.reinject;
+       bool reinject = vcpu->arch.exception.injected;
        u32 error_code = vcpu->arch.exception.error_code;
 
        /*

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 1ee5fe7ea5ffc425dfa38e5e9b3b41b888216c31..e9e5f4fd834fee78245cedf1fea2681eb1a9241b 100644 (file)
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2516,7 +2516,7 @@ static void vmx_queue_exception(struct kvm_vcpu *vcpu)
        struct vcpu_vmx *vmx = to_vmx(vcpu);
        unsigned nr = vcpu->arch.exception.nr;
        bool has_error_code = vcpu->arch.exception.has_error_code;
-       bool reinject = vcpu->arch.exception.reinject;
+       bool reinject = vcpu->arch.exception.injected;
        u32 error_code = vcpu->arch.exception.error_code;
        u32 intr_info = nr | INTR_INFO_VALID_MASK;
 
@@ -10972,7 +10972,7 @@ static void vmcs12_save_pending_event(struct kvm_vcpu *vcpu,
        u32 idt_vectoring;
        unsigned int nr;
 
-       if (vcpu->arch.exception.pending && vcpu->arch.exception.reinject) {
+       if (vcpu->arch.exception.injected) {
                nr = vcpu->arch.exception.nr;
                idt_vectoring = nr | VECTORING_INFO_VALID_MASK;
 

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 80eceb7f80c5e2455484f0ce3ca151a35b712dfc..3abaff61829dcdd4dad7e0642be6da8b55d3ea25 100644 (file)
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -389,15 +389,28 @@ static void kvm_multiple_exception(struct kvm_vcpu *vcpu,
 
        kvm_make_request(KVM_REQ_EVENT, vcpu);
 
-       if (!vcpu->arch.exception.pending) {
+       if (!vcpu->arch.exception.pending && !vcpu->arch.exception.injected) {
        queue:
                if (has_error && !is_protmode(vcpu))
                        has_error = false;
-               vcpu->arch.exception.pending = true;
+               if (reinject) {
+                       /*
+                        * On vmentry, vcpu->arch.exception.pending is only
+                        * true if an event injection was blocked by
+                        * nested_run_pending.  In that case, however,
+                        * vcpu_enter_guest requests an immediate exit,
+                        * and the guest shouldn't proceed far enough to
+                        * need reinjection.
+                        */
+                       WARN_ON_ONCE(vcpu->arch.exception.pending);
+                       vcpu->arch.exception.injected = true;
+               } else {
+                       vcpu->arch.exception.pending = true;
+                       vcpu->arch.exception.injected = false;
+               }
                vcpu->arch.exception.has_error_code = has_error;
                vcpu->arch.exception.nr = nr;
                vcpu->arch.exception.error_code = error_code;
-               vcpu->arch.exception.reinject = reinject;
                return;
        }
 
@@ -412,8 +425,13 @@ static void kvm_multiple_exception(struct kvm_vcpu *vcpu,
        class2 = exception_class(nr);
        if ((class1 == EXCPT_CONTRIBUTORY && class2 == EXCPT_CONTRIBUTORY)
                || (class1 == EXCPT_PF && class2 != EXCPT_BENIGN)) {
-               /* generate double fault per SDM Table 5-5 */
+               /*
+                * Generate double fault per SDM Table 5-5.  Set
+                * exception.pending = true so that the double fault
+                * can trigger a nested vmexit.
+                */
                vcpu->arch.exception.pending = true;
+               vcpu->arch.exception.injected = false;
                vcpu->arch.exception.has_error_code = true;
                vcpu->arch.exception.nr = DF_VECTOR;
                vcpu->arch.exception.error_code = 0;
@@ -3072,8 +3090,14 @@ static void kvm_vcpu_ioctl_x86_get_vcpu_events(struct kvm_vcpu *vcpu,
                                               struct kvm_vcpu_events *events)
 {
        process_nmi(vcpu);
+       /*
+        * FIXME: pass injected and pending separately.  This is only
+        * needed for nested virtualization, whose state cannot be
+        * migrated yet.  For now we can combine them.
+        */
        events->exception.injected =
-               vcpu->arch.exception.pending &&
+               (vcpu->arch.exception.pending ||
+                vcpu->arch.exception.injected) &&
                !kvm_exception_is_soft(vcpu->arch.exception.nr);
        events->exception.nr = vcpu->arch.exception.nr;
        events->exception.has_error_code = vcpu->arch.exception.has_error_code;
@@ -3128,6 +3152,7 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
                return -EINVAL;
 
        process_nmi(vcpu);
+       vcpu->arch.exception.injected = false;
        vcpu->arch.exception.pending = events->exception.injected;
        vcpu->arch.exception.nr = events->exception.nr;
        vcpu->arch.exception.has_error_code = events->exception.has_error_code;
@@ -6344,11 +6369,42 @@ static int inject_pending_event(struct kvm_vcpu *vcpu, bool req_int_win)
        int r;
 
        /* try to reinject previous events if any */
+       if (vcpu->arch.exception.injected) {
+               kvm_x86_ops->queue_exception(vcpu);
+               return 0;
+       }
+
+       /*
+        * Exceptions must be injected immediately, or the exception
+        * frame will have the address of the NMI or interrupt handler.
+        */
+       if (!vcpu->arch.exception.pending) {
+               if (vcpu->arch.nmi_injected) {
+                       kvm_x86_ops->set_nmi(vcpu);
+                       return 0;
+               }
+
+               if (vcpu->arch.interrupt.pending) {
+                       kvm_x86_ops->set_irq(vcpu);
+                       return 0;
+               }
+       }
+
+       if (is_guest_mode(vcpu) && kvm_x86_ops->check_nested_events) {
+               r = kvm_x86_ops->check_nested_events(vcpu, req_int_win);
+               if (r != 0)
+                       return r;
+       }
+
+       /* try to inject new event if pending */
        if (vcpu->arch.exception.pending) {
                trace_kvm_inj_exception(vcpu->arch.exception.nr,
                                        vcpu->arch.exception.has_error_code,
                                        vcpu->arch.exception.error_code);
 
+               vcpu->arch.exception.pending = false;
+               vcpu->arch.exception.injected = true;
+
                if (exception_type(vcpu->arch.exception.nr) == EXCPT_FAULT)
                        __kvm_set_rflags(vcpu, kvm_get_rflags(vcpu) |
                                             X86_EFLAGS_RF);
@@ -6360,27 +6416,7 @@ static int inject_pending_event(struct kvm_vcpu *vcpu, bool req_int_win)
                }
 
                kvm_x86_ops->queue_exception(vcpu);
-               return 0;
-       }
-
-       if (vcpu->arch.nmi_injected) {
-               kvm_x86_ops->set_nmi(vcpu);
-               return 0;
-       }
-
-       if (vcpu->arch.interrupt.pending) {
-               kvm_x86_ops->set_irq(vcpu);
-               return 0;
-       }
-
-       if (is_guest_mode(vcpu) && kvm_x86_ops->check_nested_events) {
-               r = kvm_x86_ops->check_nested_events(vcpu, req_int_win);
-               if (r != 0)
-                       return r;
-       }
-
-       /* try to inject new event if pending */
-       if (vcpu->arch.smi_pending && !is_smm(vcpu)) {
+       } else if (vcpu->arch.smi_pending && !is_smm(vcpu)) {
                vcpu->arch.smi_pending = false;
                enter_smm(vcpu);
        } else if (vcpu->arch.nmi_pending && kvm_x86_ops->nmi_allowed(vcpu)) {
@@ -6856,6 +6892,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
                                kvm_x86_ops->enable_nmi_window(vcpu);
                        if (kvm_cpu_has_injectable_intr(vcpu) || req_int_win)
                                kvm_x86_ops->enable_irq_window(vcpu);
+                       WARN_ON(vcpu->arch.exception.pending);
                }
 
                if (kvm_lapic_enabled(vcpu)) {
@@ -7730,6 +7767,7 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
        vcpu->arch.nmi_injected = false;
        kvm_clear_interrupt_queue(vcpu);
        kvm_clear_exception_queue(vcpu);
+       vcpu->arch.exception.pending = false;
 
        memset(vcpu->arch.db, 0, sizeof(vcpu->arch.db));
        kvm_update_dr0123(vcpu);

diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index de1abf4d792558bf46553df3e39d70a297a09d8e..51e349cf5f45276891883502d91d6bf4856186fb 100644 (file)
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -11,7 +11,7 @@
 
 static inline void kvm_clear_exception_queue(struct kvm_vcpu *vcpu)
 {
-       vcpu->arch.exception.pending = false;
+       vcpu->arch.exception.injected = false;
 }
 
 static inline void kvm_queue_interrupt(struct kvm_vcpu *vcpu, u8 vector,
@@ -29,7 +29,7 @@ static inline void kvm_clear_interrupt_queue(struct kvm_vcpu *vcpu)
 
 static inline bool kvm_event_needs_reinjection(struct kvm_vcpu *vcpu)
 {
-       return vcpu->arch.exception.pending || vcpu->arch.interrupt.pending ||
+       return vcpu->arch.exception.injected || vcpu->arch.interrupt.pending ||
                vcpu->arch.nmi_injected;
 }