[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

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

[Long Li]

> 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>

I think this patch is correct in that it retriggers interrupts unconditional on Hyper-V. This is needed because Hyper-V does MSI retarget asynchronously, unlike all other platforms.

As an alternative, this can also be fixed in fixup_irqs(). IRQ layer already has this logic in fixup_irqs() for dealing with a lost interrupt when programming the chip for retargeting, calling chip->irq_retrigger() unconditionally for Hyper-V in fixup_irqs() is another option.

Thanks,

Long


> ---
>  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
> 

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

[Thomas Gleixner]

On Fri, Jul 10 2026 at 05:40, Naman Jain wrote:
>
> Fixes: e84cf6aa501c5 ("x86/apic/vector: Handle vector release on CPU unplug correctly")

Errm. This does not fix anything in the common code. It fixes a hyperv
PCI/MSI problem.

>  	} 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.

What has msi_set_affinity() and IRQCHIP_MOVE_DEFERRED to do with this?

  - msi_set_affinity() solves a completely different problem. It handles
    the case when the PCI/MSI device does not provide masking of the MSI
    interrupt across the message change.

    Q: What exactly is mirrored here?
    A: Nothing

  - In the context of CPU hot-unplug IRQCHIP_MOVE_DEFERRED is not
    deferring anything simply because there is no way to do the deferred
    (in interrupt context) move on the original target CPU.

    fixup_irqs()
       irq_migrate_all_off_this_cpu()
          migrate_one_irq()
            irq_do_set_affinity()

    The only check there which is related to IRQCHIP_MOVE_DEFERRED is
    the masking decision in migrate_one_irq(), but that has nothing to
    do with the semantics of IRQCHIP_MOVE_DEFERRED.

    That flag tells the interrupt core that regular affinity changes
    have to be delayed into the context of the next device interrupt,
    which is obviously not possible for interrupt startup and forced
    migration on CPU hot-unplug, which both ignore that restriction. The
    only effect in CPU hot-unplug migrations is that the interrupt is
    masked at the outermost domain level across the affinity change
    operation because that's an requirement for msi_set_affinity() and
    similar workarounds (loongarch) to avoid the multi vector dance.

    The fact that hyper-V invokes the retarget hypercall in context of
    the unmask() callback has nothing to with that flag and due to the
    lack of explanation why the flag is needed in the first place.

I'm all for commenting complex issues, but comments have to be
technically correct, which excludes both LLM and human hallucinations.
The same applies to the change log which carries the same fairy tale.

Aside of that, doing the re-targeting in the unmask() callback
unconditionally is pointless overhead. Why?

    disable_irq($IRQ);             // Sets the disabled flag, but does not mask

    --> $IRQ is raised
    handle_edge_irq($IRQ)
    if (disabled($IRQ))
      mask_irq($IRQ);              // Masks at the PCI/MSI level

    enable_irq($IRQ)
    if (masked($IRQ))
      unmask_irq($IRQ)
        issue_hypercall(RETARGET)  // Pointless exercise when affinity
                                   // has not changed
        unmask_pci_msi();          // Unmasks at the PCI/MSI level

Not that I care at all, but I have no idea why pci-hyperv has to do the
affinity change in the unmask() callback and why it needs to take the
overhead which comes with the deferred affinity changes in general.

> +		 * 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))

TBH. This is a tasteless layering violation. We are not cluttering
common code with hypervisor specific MSI workarounds. The non-remapped
MSI workaround is not implemented here either.

> +			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.

What's exactly unconditional about

       if (cond)
          write()

?

> +              						(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.)

What has msi_set_affinity() to do with this? It sets RETRIGGER for a
very different reason. These references are confusing at best. Simply
explain WHY you need to do this and explain it correctly.

> +		 */
> +		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

What's a freed vIRR and which completion? This code deals with vector
management and not with hyperV MSI implementation specific completions
and it does not care whether it operates on a physical or a virtual
APIC.

> +	 * on the new target (see the else-branch above). Issued after the new

'See the else-branch above' is as useful as 'See some random explanation
elsewhere'. It's obvious where to look for that information as the
hv_retrigger condition must be set by something to make the condition true.

> +	 * mapping is installed so the handler is present when it is serviced.
> +	 */
> +	if (hv_retrigger)
> +		__apic_send_IPI(newcpu, newvec);

You are claiming above that the MSI retarget hypercall is asynchronous,
but you fail to specify in which way.

  - If you refer to the retarget invocation in the unmask() callback,
    then that happens still before the new target can handle it.

  - If it is truly asynchronous, i.e. the hypervisor handles it at some
    arbitrary point in time, then the retriggered vector on the new
    target CPU can be handled _before_ the retarget is in effect. In
    that case a subsequent interrupt might still end up on the dead CPU,
    no?

IOW, your or your LLM's elaborate comment is useless word salad as it
does not explain the problem in a coherent and comprehensible way. Quite
to the contrary it is more confusing than helpful.

But as I told you already, this is not a problem of incoherent comments,
this is simply the wrong place to address that.

Looking at the pci-hyperv implementation made me immediately notice a
bug, which exists since the driver was merged:

  The driver does not set the irq_retrigger() callback for the
  interrupt chip.

That means that fixup_irqs() can't do anything even when it observes the
IRR bit set. And that matches the incoherent problem description as far
as I can tell from trying to oracle something out of the word salad.

Great that your AI ass-istant did not figure that out and you blindly
trusted the nonsense it hallucinated.

The uncompiled and therefore untested below should exactly do what you
want to achieve without having hyperV specific hackery in the common
code.

If it does not, then you need to provide a coherent technical
explanation why it is not sufficient.

Thanks,

        tglx

---
Subject: TBD

  Instead of adding the missing callback to the hyperv driver, which
  would be the trivial "fix", it changes the x86 fixup_irqs()
  implementation. Why?

  That makes the code more resilient and allows to remove quite a bit of
  initializations of interrupt chips all over the place.

Not-Signed-off-yet-by: Thomas Gleixner <tglx@kernel.org>
Assisted-by: Human Intelligence
---
 arch/x86/kernel/irq.c |   23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -466,11 +466,6 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_posted_msi
 /* A cpu has been removed from cpu_online_mask.  Reset irq affinities. */
 void fixup_irqs(void)
 {
-	unsigned int vector;
-	struct irq_desc *desc;
-	struct irq_data *data;
-	struct irq_chip *chip;
-
 	irq_migrate_all_off_this_cpu();
 
 	/*
@@ -489,22 +484,18 @@ void fixup_irqs(void)
 	 * vector_lock because the cpu is already marked !online, so
 	 * nothing else will touch it.
 	 */
-	for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
-		if (IS_ERR_OR_NULL(__this_cpu_read(vector_irq[vector])))
+	for (unsigned int vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
+		struct irq_desc *desc = __this_cpu_read(vector_irq[vector]);
+
+		if (IS_ERR_OR_NULL(desc))
 			continue;
 
 		if (is_vector_pending(vector)) {
-			desc = __this_cpu_read(vector_irq[vector]);
-
-			raw_spin_lock(&desc->lock);
-			data = irq_desc_get_irq_data(desc);
-			chip = irq_data_get_irq_chip(data);
-			if (chip->irq_retrigger) {
-				chip->irq_retrigger(data);
+			guard(raw_spinlock)(&desc->lock);
+			if (irq_chip_retrigger_hierarchy(&desc->irq_data))
 				__this_cpu_write(vector_irq[vector], VECTOR_RETRIGGERED);
-			}
-			raw_spin_unlock(&desc->lock);
 		}
+
 		if (__this_cpu_read(vector_irq[vector]) != VECTOR_RETRIGGERED)
 			__this_cpu_write(vector_irq[vector], VECTOR_UNUSED);
 	}

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

[Naman Jain]

On 7/22/2026 2:40 AM, Thomas Gleixner wrote:
> On Fri, Jul 10 2026 at 05:40, Naman Jain wrote:
>>
>> Fixes: e84cf6aa501c5 ("x86/apic/vector: Handle vector release on CPU unplug correctly")
> 
> Errm. This does not fix anything in the common code. It fixes a hyperv
> PCI/MSI problem.
> >>   	} 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.
> 
> What has msi_set_affinity() and IRQCHIP_MOVE_DEFERRED to do with this?
> 
>    - msi_set_affinity() solves a completely different problem. It handles
>      the case when the PCI/MSI device does not provide masking of the MSI
>      interrupt across the message change.
> 
>      Q: What exactly is mirrored here?
>      A: Nothing
> 
>    - In the context of CPU hot-unplug IRQCHIP_MOVE_DEFERRED is not
>      deferring anything simply because there is no way to do the deferred
>      (in interrupt context) move on the original target CPU.
> 
>      fixup_irqs()
>         irq_migrate_all_off_this_cpu()
>            migrate_one_irq()
>              irq_do_set_affinity()
> 
>      The only check there which is related to IRQCHIP_MOVE_DEFERRED is
>      the masking decision in migrate_one_irq(), but that has nothing to
>      do with the semantics of IRQCHIP_MOVE_DEFERRED.
> 
>      That flag tells the interrupt core that regular affinity changes
>      have to be delayed into the context of the next device interrupt,
>      which is obviously not possible for interrupt startup and forced
>      migration on CPU hot-unplug, which both ignore that restriction. The
>      only effect in CPU hot-unplug migrations is that the interrupt is
>      masked at the outermost domain level across the affinity change
>      operation because that's an requirement for msi_set_affinity() and
>      similar workarounds (loongarch) to avoid the multi vector dance.
> 
>      The fact that hyper-V invokes the retarget hypercall in context of
>      the unmask() callback has nothing to with that flag and due to the
>      lack of explanation why the flag is needed in the first place.
> 
> I'm all for commenting complex issues, but comments have to be
> technically correct, which excludes both LLM and human hallucinations.
> The same applies to the change log which carries the same fairy tale.
> 
> Aside of that, doing the re-targeting in the unmask() callback
> unconditionally is pointless overhead. Why?
> 
>      disable_irq($IRQ);             // Sets the disabled flag, but does not mask
> 
>      --> $IRQ is raised
>      handle_edge_irq($IRQ)
>      if (disabled($IRQ))
>        mask_irq($IRQ);              // Masks at the PCI/MSI level
> 
>      enable_irq($IRQ)
>      if (masked($IRQ))
>        unmask_irq($IRQ)
>          issue_hypercall(RETARGET)  // Pointless exercise when affinity
>                                     // has not changed
>          unmask_pci_msi();          // Unmasks at the PCI/MSI level
> 
> Not that I care at all, but I have no idea why pci-hyperv has to do the
> affinity change in the unmask() callback and why it needs to take the
> overhead which comes with the deferred affinity changes in general.
> 
>> +		 * 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))
> 
> TBH. This is a tasteless layering violation. We are not cluttering
> common code with hypervisor specific MSI workarounds. The non-remapped
> MSI workaround is not implemented here either.
> 
>> +			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.
> 
> What's exactly unconditional about
> 
>         if (cond)
>            write()
> 
> ?
> 
>> +              						(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.)
> 
> What has msi_set_affinity() to do with this? It sets RETRIGGER for a
> very different reason. These references are confusing at best. Simply
> explain WHY you need to do this and explain it correctly.
> 
>> +		 */
>> +		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
> 
> What's a freed vIRR and which completion? This code deals with vector
> management and not with hyperV MSI implementation specific completions
> and it does not care whether it operates on a physical or a virtual
> APIC.
> 
>> +	 * on the new target (see the else-branch above). Issued after the new
> 
> 'See the else-branch above' is as useful as 'See some random explanation
> elsewhere'. It's obvious where to look for that information as the
> hv_retrigger condition must be set by something to make the condition true.
> 
>> +	 * mapping is installed so the handler is present when it is serviced.
>> +	 */
>> +	if (hv_retrigger)
>> +		__apic_send_IPI(newcpu, newvec);
> 
> You are claiming above that the MSI retarget hypercall is asynchronous,
> but you fail to specify in which way.
> 
>    - If you refer to the retarget invocation in the unmask() callback,
>      then that happens still before the new target can handle it.
> 
>    - If it is truly asynchronous, i.e. the hypervisor handles it at some
>      arbitrary point in time, then the retriggered vector on the new
>      target CPU can be handled _before_ the retarget is in effect. In
>      that case a subsequent interrupt might still end up on the dead CPU,
>      no?
> 
> IOW, your or your LLM's elaborate comment is useless word salad as it
> does not explain the problem in a coherent and comprehensible way. Quite
> to the contrary it is more confusing than helpful.
> 
> But as I told you already, this is not a problem of incoherent comments,
> this is simply the wrong place to address that.
> 
> Looking at the pci-hyperv implementation made me immediately notice a
> bug, which exists since the driver was merged:
> 
>    The driver does not set the irq_retrigger() callback for the
>    interrupt chip.
> 
> That means that fixup_irqs() can't do anything even when it observes the
> IRR bit set. And that matches the incoherent problem description as far
> as I can tell from trying to oracle something out of the word salad.
> 
> Great that your AI ass-istant did not figure that out and you blindly
> trusted the nonsense it hallucinated.
> 
> The uncompiled and therefore untested below should exactly do what you
> want to achieve without having hyperV specific hackery in the common
> code.
> 
> If it does not, then you need to provide a coherent technical
> explanation why it is not sufficient.
> 
> Thanks,
> 
>          tglx
> 
> ---


Thanks a lot for reviewing the code. I was actually debugging hangs and 
timeouts when we test stress-FIO with hotplug. Occasionally I saw "No 
irq handler" logs in those hangs, and started debugging in that 
direction. I had tried adding the hyper-V irq_retrigger() callback, but 
the hang still reproduced. This kept me under the impression that this 
change did not fix the issue and led me to over-engineer this fix in the 
wrong place. Recently, I was able to root cause the hang to some other 
reason (using different NVME scheduler).

I completely agree with your comments, and now that I know how to fix 
this, do you suggest me to -

1. add irq_retrigger callback in pci-hyperv.c
or
2. go with your fixup-irq() change to no longer need drivers to add the 
above? And then see what all stale irq_retrigger inits can be removed

Going with 1) would be easier for me to fix this bug, and back port it 
to older kernels, with proper Fixes tag (pointing to some pci-hyperv.c 
change).
Then later, we can go with 2) separately as a general enhancement (not a 
Fix).

Regards,
Naman

> Subject: TBD
> 
>    Instead of adding the missing callback to the hyperv driver, which
>    would be the trivial "fix", it changes the x86 fixup_irqs()
>    implementation. Why?
> 
>    That makes the code more resilient and allows to remove quite a bit of
>    initializations of interrupt chips all over the place.
> 
> Not-Signed-off-yet-by: Thomas Gleixner <tglx@kernel.org>
> Assisted-by: Human Intelligence
> ---
>   arch/x86/kernel/irq.c |   23 +++++++----------------
>   1 file changed, 7 insertions(+), 16 deletions(-)
> 
> --- a/arch/x86/kernel/irq.c
> +++ b/arch/x86/kernel/irq.c
> @@ -466,11 +466,6 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_posted_msi
>   /* A cpu has been removed from cpu_online_mask.  Reset irq affinities. */
>   void fixup_irqs(void)
>   {
> -	unsigned int vector;
> -	struct irq_desc *desc;
> -	struct irq_data *data;
> -	struct irq_chip *chip;
> -
>   	irq_migrate_all_off_this_cpu();
>   
>   	/*
> @@ -489,22 +484,18 @@ void fixup_irqs(void)
>   	 * vector_lock because the cpu is already marked !online, so
>   	 * nothing else will touch it.
>   	 */
> -	for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
> -		if (IS_ERR_OR_NULL(__this_cpu_read(vector_irq[vector])))
> +	for (unsigned int vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
> +		struct irq_desc *desc = __this_cpu_read(vector_irq[vector]);
> +
> +		if (IS_ERR_OR_NULL(desc))
>   			continue;
>   
>   		if (is_vector_pending(vector)) {
> -			desc = __this_cpu_read(vector_irq[vector]);
> -
> -			raw_spin_lock(&desc->lock);
> -			data = irq_desc_get_irq_data(desc);
> -			chip = irq_data_get_irq_chip(data);
> -			if (chip->irq_retrigger) {
> -				chip->irq_retrigger(data);
> +			guard(raw_spinlock)(&desc->lock);
> +			if (irq_chip_retrigger_hierarchy(&desc->irq_data))
>   				__this_cpu_write(vector_irq[vector], VECTOR_RETRIGGERED);
> -			}
> -			raw_spin_unlock(&desc->lock);
>   		}
> +
>   		if (__this_cpu_read(vector_irq[vector]) != VECTOR_RETRIGGERED)
>   			__this_cpu_write(vector_irq[vector], VECTOR_UNUSED);
>   	}

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

[Thomas Gleixner]

Naman!

On Wed, Jul 22 2026 at 14:59, Naman Jain wrote:
> I completely agree with your comments, and now that I know how to fix 
> this, do you suggest me to -
>
> 1. add irq_retrigger callback in pci-hyperv.c
> or
> 2. go with your fixup-irq() change to no longer need drivers to add the 
> above? And then see what all stale irq_retrigger inits can be removed
>
> Going with 1) would be easier for me to fix this bug, and back port it 
> to older kernels, with proper Fixes tag (pointing to some pci-hyperv.c 
> change).
> Then later, we can go with 2) separately as a general enhancement (not a 
> Fix).

This should be one patch series:

  1) Add the callback to pci-hyperv, i.e. in hv_pcie_init_dev_msi_info()

     +    chip->irq_retrigger = irq_chip_retrigger_hierarchy;

     That sets it on the outermost domain interrupt chip (PCI/MSI
     device domain), which is what fixup_irqs() looks at.

     This one needs a Fixes: tag (The first commit which introduced the
     driver) and a Cc: stable@... so it will be backported.

  2) Change fixup_irqs() to use irq_chip_retrigger_hierarchy()

     Feel free to pick up the patch I gave you, add a proper change log
     and a Suggested-by: Thomas .... tag.

  3) Remove the initialization of the outermost chip from arch/x86/* and
     drivers/iommu/amd/* as that's not longer required then.

Thanks,

        tglx

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

[Naman Jain]

On 7/22/2026 6:45 PM, Thomas Gleixner wrote:
> Naman!
> 
> On Wed, Jul 22 2026 at 14:59, Naman Jain wrote:
>> I completely agree with your comments, and now that I know how to fix
>> this, do you suggest me to -
>>
>> 1. add irq_retrigger callback in pci-hyperv.c
>> or
>> 2. go with your fixup-irq() change to no longer need drivers to add the
>> above? And then see what all stale irq_retrigger inits can be removed
>>
>> Going with 1) would be easier for me to fix this bug, and back port it
>> to older kernels, with proper Fixes tag (pointing to some pci-hyperv.c
>> change).
>> Then later, we can go with 2) separately as a general enhancement (not a
>> Fix).
> 
> This should be one patch series:
> 
>    1) Add the callback to pci-hyperv, i.e. in hv_pcie_init_dev_msi_info()
> 
>       +    chip->irq_retrigger = irq_chip_retrigger_hierarchy;
> 
>       That sets it on the outermost domain interrupt chip (PCI/MSI
>       device domain), which is what fixup_irqs() looks at.
> 
>       This one needs a Fixes: tag (The first commit which introduced the
>       driver) and a Cc: stable@... so it will be backported.
> 
>    2) Change fixup_irqs() to use irq_chip_retrigger_hierarchy()
> 
>       Feel free to pick up the patch I gave you, add a proper change log
>       and a Suggested-by: Thomas .... tag.
> 
>    3) Remove the initialization of the outermost chip from arch/x86/* and
>       drivers/iommu/amd/* as that's not longer required then.
> 
> Thanks,
> 
>          tglx
> 

Understood. Thanks.

Regards,
Naman