[PATCH] KVM: x86: Fix lost EOI when unmasking a pending level IOAPIC pin

[PATCH] KVM: x86: Fix lost EOI when unmasking a pending level IOAPIC pin

[Denis V. Lunev]

When the guest unmasks a level-triggered redirection entry whose line is
already asserted, ioapic_write_indirect() injects the pending interrupt
first and only then asks the destination vCPUs to rescan their
ioapic_handled_vectors and EOI exit bitmaps. If the same write also set
the vector or the trigger mode, a destination vCPU can take the interrupt
and EOI it before the rescan. A posted interrupt reaches a running vCPU
without a VM exit, and the EOI is then either virtualized without an exit
or dropped by kvm_ioapic_send_eoi() because the vector is not in
ioapic_handled_vectors yet. Remote IRR stays set and the pin delivers no
more interrupts until the guest reprograms it.

Linux programs the entry masked and unmasks it with a separate write, so
the vector is scanned before anything can be injected. Windows writes
the vector, the trigger mode and the mask bit in one go. A virtio balloon
on INTx whose line is already up when the driver connects its interrupt
then stops receiving interrupts for the rest of the boot, and the guest
ignores every later balloon target.

Make the scan request before servicing the pending IRR. The request
waits for the destination vCPUs to leave guest mode, and each of them
rescans the routes and reloads its EOI exit bitmap before it can enter
the guest and take the interrupt.

Fixes: c7c9c56ca26f ("x86, apicv: add virtual interrupt delivery support")
Fixes: 3bb345f387dd ("KVM: x86: store IOAPIC-handled vectors in each VCPU")
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
Reproducer: "[kvm-unit-tests PATCH] x86: add ioapic_eoi_race test for
unmasking a pending level pin", posted separately. On a nested Intel
host with APICv and a 7.2.13 kernel it loses about 95% of the EOIs in
the single-write case. With this patch it loses none, the masked-write
case never loses any, kernel_irqchip=split is unaffected, and
x86/ioapic.flat passes.

The symptom in the field is a Windows guest whose virtio balloon is on
INTx: the balloon raises its line before the driver connects the
interrupt, Windows programs the pin and unmasks it in one write, and the
balloon gets no further interrupt until the next boot.

 arch/x86/kvm/ioapic.c | 66 +++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 0d59b9c758c2..8cca333cdc9d 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -384,39 +384,6 @@ static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val)
 		mask_after = e->fields.mask;
 		if (mask_before != mask_after)
 			kvm_fire_mask_notifiers(ioapic->kvm, KVM_IRQCHIP_IOAPIC, index, mask_after);
-		if (e->fields.trig_mode == IOAPIC_LEVEL_TRIG &&
-		    ioapic->irr & (1 << index) && !e->fields.mask && !e->fields.remote_irr) {
-			/*
-			 * Pending status in irr may be outdated: the IRQ line may have
-			 * already been deasserted by a device while the IRQ was masked.
-			 * This occurs, for instance, if the interrupt is handled in a
-			 * Linux guest as a oneshot interrupt (IRQF_ONESHOT). In this
-			 * case the guest acknowledges the interrupt to the device in
-			 * its threaded irq handler, i.e. after the EOI but before
-			 * unmasking, so at the time of unmasking the IRQ line is
-			 * already down but our pending irr bit is still set. In such
-			 * cases, injecting this pending interrupt to the guest is
-			 * buggy: the guest will receive an extra unwanted interrupt.
-			 *
-			 * So we need to check here if the IRQ is actually still pending.
-			 * As we are generally not able to probe the IRQ line status
-			 * directly, we do it through irqfd resampler. Namely, we clear
-			 * the pending status and notify the resampler that this interrupt
-			 * is done, without actually injecting it into the guest. If the
-			 * IRQ line is actually already deasserted, we are done. If it is
-			 * still asserted, a new interrupt will be shortly triggered
-			 * through irqfd and injected into the guest.
-			 *
-			 * If, however, it's not possible to resample (no irqfd resampler
-			 * registered for this irq), then unconditionally inject this
-			 * pending interrupt into the guest, so the guest will not miss
-			 * an interrupt, although may get an extra unwanted interrupt.
-			 */
-			if (kvm_notify_irqfd_resampler(ioapic->kvm, KVM_IRQCHIP_IOAPIC, index))
-				ioapic->irr &= ~(1 << index);
-			else
-				ioapic_service(ioapic, index, false);
-		}
 		if (e->fields.delivery_mode == APIC_DM_FIXED) {
 			struct kvm_lapic_irq irq;
 
@@ -451,6 +418,39 @@ static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val)
 		} else {
 			kvm_make_scan_ioapic_request(ioapic->kvm);
 		}
+		if (e->fields.trig_mode == IOAPIC_LEVEL_TRIG &&
+		    ioapic->irr & (1 << index) && !e->fields.mask && !e->fields.remote_irr) {
+			/*
+			 * Pending status in irr may be outdated: the IRQ line may have
+			 * already been deasserted by a device while the IRQ was masked.
+			 * This occurs, for instance, if the interrupt is handled in a
+			 * Linux guest as a oneshot interrupt (IRQF_ONESHOT). In this
+			 * case the guest acknowledges the interrupt to the device in
+			 * its threaded irq handler, i.e. after the EOI but before
+			 * unmasking, so at the time of unmasking the IRQ line is
+			 * already down but our pending irr bit is still set. In such
+			 * cases, injecting this pending interrupt to the guest is
+			 * buggy: the guest will receive an extra unwanted interrupt.
+			 *
+			 * So we need to check here if the IRQ is actually still pending.
+			 * As we are generally not able to probe the IRQ line status
+			 * directly, we do it through irqfd resampler. Namely, we clear
+			 * the pending status and notify the resampler that this interrupt
+			 * is done, without actually injecting it into the guest. If the
+			 * IRQ line is actually already deasserted, we are done. If it is
+			 * still asserted, a new interrupt will be shortly triggered
+			 * through irqfd and injected into the guest.
+			 *
+			 * If, however, it's not possible to resample (no irqfd resampler
+			 * registered for this irq), then unconditionally inject this
+			 * pending interrupt into the guest, so the guest will not miss
+			 * an interrupt, although may get an extra unwanted interrupt.
+			 */
+			if (kvm_notify_irqfd_resampler(ioapic->kvm, KVM_IRQCHIP_IOAPIC, index))
+				ioapic->irr &= ~(1 << index);
+			else
+				ioapic_service(ioapic, index, false);
+		}
 		break;
 	}
 }

base-commit: 5b0764a53235d3c77e1ed674e836e1b3fe1a4b1f
-- 
2.53.0