[RFC PATCH] x86/apic: Fix lost IRQ during forced vector migration on Hyper-V

[RFC PATCH] x86/apic: Fix lost IRQ during forced vector migration on Hyper-V

[Naman Jain]

On Hyper-V the MSI retarget hypercall is asynchronous. When a CPU is
taken offline, chip_data_update() frees the outgoing CPU's vector inline
because the deferred cleanup-vector mechanism is unavailable for an
offline CPU. A device interrupt raised inside the stop_machine window can
be posted to the outgoing CPU's old vIRR after that inline free, causing
two problems:

1) The completion it carries is lost, as nothing drains the old vector
   once it is freed. If it was the last in-flight completion on an
   otherwise idle queue, the command times out and the controller is
   reset. This is the functional bug.

2) When the outgoing CPU re-enables interrupts during teardown, the late
   delivery finds VECTOR_UNUSED and reevaluate_vector() logs "No irq
   handler" - harmless but noisy.

The native MSI path (msi_set_affinity()) handles this race with
VECTOR_RETRIGGERED + irq_retrigger, but the Hyper-V MSI chip carries
IRQCHIP_MOVE_DEFERRED and reaches the forced-migration else-branch in
chip_data_update() instead, so it never gets that protection.

Mirror that protection in the forced-migration path:

- Issue __apic_send_IPI(newcpu, newvec) after installing the new mapping
  so a raced completion is drained on the new target. The retarget is
  asynchronous, so the outgoing IRR is not authoritative and the IPI is
  unconditional; a spurious or duplicate ISR is harmless to the MSI
  completion-draining handlers (NVMe, netdev), which find an empty queue.
- Mark the freed slot VECTOR_RETRIGGERED so the late stray is absorbed by
  reevaluate_vector() rather than logged. The write is unconditional
  because apic_free_vector() leaves the slot pointing at this irq's stale
  desc, not an unused entry (unlike msi_set_affinity()).

The migration runs under stop_machine with interrupts disabled on all
CPUs, so any raced CQE is visible before the new CPU handles the
retrigger.

The guard is restricted to edge MSI vectors on Hyper-V: it requires
X86_HYPER_MS_HYPERV, an external vector, a non-level trigger, and an
attached msi_desc. Only the MSI retarget hypercall is asynchronous, so
this is the sole path with the race; edge IOAPIC lines retarget
synchronously via the RTE and IR-remapped interrupts via the IRTE, and
neither is touched. Narrowing to MSI also keeps the unconditional
retrigger confined to completion-draining handlers, which tolerate a
phantom ISR, rather than arbitrary edge handlers. The slot marking and
completion drain must happen where the vector is freed and the new
mapping is installed, which is why the guard lives in chip_data_update()
rather than the Hyper-V MSI chip, which cannot reach vector_irq[].

Verified on a 64-CPU Azure VM: zero "No irq handler" events across 320
CPU hotplug passes with concurrent NVMe I/O, compared to ~460 per 5
minutes before the fix. No NVMe timeouts or controller resets observed.

Fixes: e84cf6aa501c5 ("x86/apic/vector: Handle vector release on CPU unplug correctly")
Cc: stable@vger.kernel.org
Assisted-by: GitHub-Copilot:claude-opus-4.8
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
---
 arch/x86/kernel/apic/vector.c | 48 +++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index bddc544653999..3577e9ca23ba6 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -20,6 +20,7 @@
 #include <asm/i8259.h>
 #include <asm/desc.h>
 #include <asm/irq_remapping.h>
+#include <asm/hypervisor.h>
 
 #include <asm/trace/irq_vectors.h>
 
@@ -152,6 +153,7 @@ static void chip_data_update(struct irq_data *irqd, unsigned int newvec, unsigne
 	struct apic_chip_data *apicd = apic_chip_data(irqd);
 	struct irq_desc *desc = irq_data_to_desc(irqd);
 	bool managed = irqd_affinity_is_managed(irqd);
+	bool hv_retrigger = false;
 
 	lockdep_assert_held(&vector_lock);
 
@@ -181,7 +183,46 @@ static void chip_data_update(struct irq_data *irqd, unsigned int newvec, unsigne
 		apicd->prev_cpu = apicd->cpu;
 		WARN_ON_ONCE(apicd->cpu == newcpu);
 	} else {
+		/*
+		 * The outgoing CPU cannot use the deferred cleanup-vector
+		 * mechanism, so its vector is freed inline below. On Hyper-V the
+		 * MSI retarget hypercall is asynchronous, so an interrupt raised
+		 * inside the stop_machine window can be posted to the outgoing
+		 * CPU's old vIRR after the free. Two complementary steps handle
+		 * that (see also the retrigger at the end of the function):
+		 *
+		 *  - Retrigger on the new target so a raced completion is drained
+		 *    there rather than lost. The retarget is asynchronous, so the
+		 *    outgoing IRR is not authoritative and the IPI is issued
+		 *    unconditionally; a spurious ISR is harmless to
+		 *    completion-draining handlers (they find an empty queue).
+		 *  - Mark the freed slot VECTOR_RETRIGGERED so a late stray is
+		 *    absorbed by reevaluate_vector() instead of logging "No irq
+		 *    handler" while the CPU still takes interrupts during
+		 *    teardown; __setup_vector_irq() resets it on re-online.
+		 *
+		 * This mirrors msi_set_affinity()'s protection, which the Hyper-V
+		 * MSI chip bypasses via IRQCHIP_MOVE_DEFERRED. The guard is
+		 * restricted to edge MSI vectors on Hyper-V (msi_desc present):
+		 * only the MSI retarget hypercall is asynchronous, so edge IOAPIC
+		 * lines (retargeted synchronously via the RTE) and level-triggered
+		 * lines never see this race and must not be force-injected.
+		 */
+		if (hypervisor_is_type(X86_HYPER_MS_HYPERV) &&
+		    apicd->vector >= FIRST_EXTERNAL_VECTOR &&
+		    !irqd_is_level_type(irqd) && irq_data_get_msi_desc(irqd))
+			hv_retrigger = true;
 		apic_free_vector(apicd->cpu, apicd->vector, managed);
+		/*
+		 * apic_free_vector() releases the matrix bit but leaves the
+		 * outgoing CPU's vector_irq[] slot pointing at the stale desc, so
+		 * the marker is written unconditionally here. (Unlike
+		 * msi_set_affinity(), which marks a genuinely unused slot and
+		 * therefore guards with IS_ERR_OR_NULL, the slot here still holds
+		 * this irq's old desc.)
+		 */
+		if (hv_retrigger)
+			per_cpu(vector_irq, apicd->cpu)[apicd->vector] = VECTOR_RETRIGGERED;
 	}
 
 setnew:
@@ -190,6 +231,13 @@ static void chip_data_update(struct irq_data *irqd, unsigned int newvec, unsigne
 	BUG_ON(!IS_ERR_OR_NULL(per_cpu(vector_irq, newcpu)[newvec]));
 	per_cpu(vector_irq, newcpu)[newvec] = desc;
 	apic_update_irq_cfg(irqd, newvec, newcpu);
+	/*
+	 * Drain any completion that raced onto the freed vIRR by retriggering
+	 * on the new target (see the else-branch above). Issued after the new
+	 * mapping is installed so the handler is present when it is serviced.
+	 */
+	if (hv_retrigger)
+		__apic_send_IPI(newcpu, newvec);
 }
 
 static void vector_assign_managed_shutdown(struct irq_data *irqd)
-- 
2.43.0