KVM: SVM: fix race between interrupt delivery and AVIC inhibition

If svm_deliver_avic_intr is called just after the target vcpu's AVIC got
inhibited, it might read a stale value of vcpu->arch.apicv_active
which can lead to the target vCPU not noticing the interrupt.

To fix this use load-acquire/store-release so that, if the target vCPU
is IN_GUEST_MODE, we're guaranteed to see a previous disabling of the
AVIC.  If AVIC has been disabled in the meanwhile, proceed with the
KVM_REQ_EVENT-based delivery.

Incomplete IPI vmexit has the same races as svm_deliver_avic_intr, and
in fact it can be handled in exactly the same way; the only difference
lies in who has set IRR, whether svm_deliver_interrupt or the processor.
Therefore, svm_complete_interrupt_delivery can be used to fix incomplete
IPI vmexits as well.

Co-developed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index 2b2932f..fb3e207 100644 (file)
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -269,7 +269,7 @@ static int avic_init_backing_page(struct kvm_vcpu *vcpu)
        return 0;
 }
 
-static void avic_ring_doorbell(struct kvm_vcpu *vcpu)
+void avic_ring_doorbell(struct kvm_vcpu *vcpu)
 {
        /*
         * Note, the vCPU could get migrated to a different pCPU at any point,
@@ -300,8 +300,13 @@ static void avic_kick_target_vcpus(struct kvm *kvm, struct kvm_lapic *source,
        kvm_for_each_vcpu(i, vcpu, kvm) {
                if (kvm_apic_match_dest(vcpu, source, icrl & APIC_SHORT_MASK,
                                        GET_APIC_DEST_FIELD(icrh),
-                                       icrl & APIC_DEST_MASK))
-                       kvm_vcpu_wake_up(vcpu);
+                                       icrl & APIC_DEST_MASK)) {
+                       vcpu->arch.apic->irr_pending = true;
+                       svm_complete_interrupt_delivery(vcpu,
+                                                       icrl & APIC_MODE_MASK,
+                                                       icrl & APIC_INT_LEVELTRIG,
+                                                       icrl & APIC_VECTOR_MASK);
+               }
        }
 }
 
@@ -663,43 +668,6 @@ void svm_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap)
        return;
 }
 
-int svm_deliver_avic_intr(struct kvm_vcpu *vcpu, int vec)
-{
-       if (!vcpu->arch.apicv_active)
-               return -1;
-
-       /*
-        * Pairs with the smp_mb_*() after setting vcpu->guest_mode in
-        * vcpu_enter_guest() to ensure the write to the vIRR is ordered before
-        * the read of guest_mode, which guarantees that either VMRUN will see
-        * and process the new vIRR entry, or that the below code will signal
-        * the doorbell if the vCPU is already running in the guest.
-        */
-       smp_mb__after_atomic();
-
-       /*
-        * Signal the doorbell to tell hardware to inject the IRQ if the vCPU
-        * is in the guest.  If the vCPU is not in the guest, hardware will
-        * automatically process AVIC interrupts at VMRUN.
-        */
-       if (vcpu->mode == IN_GUEST_MODE) {
-               /*
-                * Signal the doorbell to tell hardware to inject the IRQ.  If
-                * the vCPU exits the guest before the doorbell chimes, hardware
-                * will automatically process AVIC interrupts at the next VMRUN.
-                */
-               avic_ring_doorbell(vcpu);
-       } else {
-               /*
-                * Wake the vCPU if it was blocking.  KVM will then detect the
-                * pending IRQ when checking if the vCPU has a wake event.
-                */
-               kvm_vcpu_wake_up(vcpu);
-       }
-
-       return 0;
-}
-
 bool svm_dy_apicv_has_pending_interrupt(struct kvm_vcpu *vcpu)
 {
        return false;

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index cd769ff..821edf6 100644 (file)
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3299,21 +3299,55 @@ static void svm_set_irq(struct kvm_vcpu *vcpu)
                SVM_EVTINJ_VALID | SVM_EVTINJ_TYPE_INTR;
 }
 
-static void svm_deliver_interrupt(struct kvm_lapic *apic, int delivery_mode,
-                                 int trig_mode, int vector)
+void svm_complete_interrupt_delivery(struct kvm_vcpu *vcpu, int delivery_mode,
+                                    int trig_mode, int vector)
 {
-       struct kvm_vcpu *vcpu = apic->vcpu;
+       /*
+        * vcpu->arch.apicv_active must be read after vcpu->mode.
+        * Pairs with smp_store_release in vcpu_enter_guest.
+        */
+       bool in_guest_mode = (smp_load_acquire(&vcpu->mode) == IN_GUEST_MODE);
 
-       kvm_lapic_set_irr(vector, apic);
-       if (svm_deliver_avic_intr(vcpu, vector)) {
+       if (!READ_ONCE(vcpu->arch.apicv_active)) {
+               /* Process the interrupt via inject_pending_event */
                kvm_make_request(KVM_REQ_EVENT, vcpu);
                kvm_vcpu_kick(vcpu);
+               return;
+       }
+
+       trace_kvm_apicv_accept_irq(vcpu->vcpu_id, delivery_mode, trig_mode, vector);
+       if (in_guest_mode) {
+               /*
+                * Signal the doorbell to tell hardware to inject the IRQ.  If
+                * the vCPU exits the guest before the doorbell chimes, hardware
+                * will automatically process AVIC interrupts at the next VMRUN.
+                */
+               avic_ring_doorbell(vcpu);
        } else {
-               trace_kvm_apicv_accept_irq(vcpu->vcpu_id, delivery_mode,
-                                          trig_mode, vector);
+               /*
+                * Wake the vCPU if it was blocking.  KVM will then detect the
+                * pending IRQ when checking if the vCPU has a wake event.
+                */
+               kvm_vcpu_wake_up(vcpu);
        }
 }
 
+static void svm_deliver_interrupt(struct kvm_lapic *apic,  int delivery_mode,
+                                 int trig_mode, int vector)
+{
+       kvm_lapic_set_irr(vector, apic);
+
+       /*
+        * Pairs with the smp_mb_*() after setting vcpu->guest_mode in
+        * vcpu_enter_guest() to ensure the write to the vIRR is ordered before
+        * the read of guest_mode.  This guarantees that either VMRUN will see
+        * and process the new vIRR entry, or that svm_complete_interrupt_delivery
+        * will signal the doorbell if the CPU has already entered the guest.
+        */
+       smp_mb__after_atomic();
+       svm_complete_interrupt_delivery(apic->vcpu, delivery_mode, trig_mode, vector);
+}
+
 static void svm_update_cr8_intercept(struct kvm_vcpu *vcpu, int tpr, int irr)
 {
        struct vcpu_svm *svm = to_svm(vcpu);

diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 8cc45f2..fa98d68 100644 (file)
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -489,6 +489,8 @@ void svm_set_gif(struct vcpu_svm *svm, bool value);
 int svm_invoke_exit_handler(struct kvm_vcpu *vcpu, u64 exit_code);
 void set_msr_interception(struct kvm_vcpu *vcpu, u32 *msrpm, u32 msr,
                          int read, int write);
+void svm_complete_interrupt_delivery(struct kvm_vcpu *vcpu, int delivery_mode,
+                                    int trig_mode, int vec);
 
 /* nested.c */
 
@@ -572,12 +574,12 @@ bool svm_check_apicv_inhibit_reasons(ulong bit);
 void svm_load_eoi_exitmap(struct kvm_vcpu *vcpu, u64 *eoi_exit_bitmap);
 void svm_hwapic_irr_update(struct kvm_vcpu *vcpu, int max_irr);
 void svm_hwapic_isr_update(struct kvm_vcpu *vcpu, int max_isr);
-int svm_deliver_avic_intr(struct kvm_vcpu *vcpu, int vec);
 bool svm_dy_apicv_has_pending_interrupt(struct kvm_vcpu *vcpu);
 int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
                       uint32_t guest_irq, bool set);
 void avic_vcpu_blocking(struct kvm_vcpu *vcpu);
 void avic_vcpu_unblocking(struct kvm_vcpu *vcpu);
+void avic_ring_doorbell(struct kvm_vcpu *vcpu);
 
 /* sev.c */
 

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7131d73..641044d 100644 (file)
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9983,7 +9983,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
         * result in virtual interrupt delivery.
         */
        local_irq_disable();
-       vcpu->mode = IN_GUEST_MODE;
+
+       /* Store vcpu->apicv_active before vcpu->mode.  */
+       smp_store_release(&vcpu->mode, IN_GUEST_MODE);
 
        srcu_read_unlock(&vcpu->kvm->srcu, vcpu->srcu_idx);