RE: [PATCH v2 2/3] x86/irq: Use irq_chip_retrigger_hierarchy() in fixup_irqs()

From: Naman Jain <redacted> Sent: Tuesday, August 18, 2026 12:31 AM

On 8/18/2026 10:40 AM, Michael Kelley wrote:

quoted

From: Naman Jain <redacted> Sent: Monday, August 10, 2026 2:07 AM

[snip]

quoted

Sashiko pointed out that irq_chip_retrigger_hierarchy() doesn't check
the outermost domain -- it immediately goes the parent. In v1 of this patch
series, you categorized this as a false positive. But I don't understand
your explanation. Even if there aren't currently any outermost domains
with a custom retrigger function, there could be at some point in the
future. So it seems wrong to skip it. But maybe I'm missing something.
Could you elaborate on your reasoning?

Michael

irq_chip_retrigger_hierarchy() deliberately skips the chip passed to it
and begins with its parent, i.e. apic_retrigger_irq(). The new code was
doing the same. However, if someone adds a custom .irq_retrigger, hoping
that it would get executed, it would not.

Sashiko's example was valid in principle, but it is not relevant to
these device interrupts because desc->irq_data is the outer
MSI/IOAPIC/Hyper-V chip. The LAPIC data is its parent, not the starting
data. I found no device IRQ in this path whose descriptor starts
directly at lapic_controller.

I am all in for adding this as a fallback -
+    if (chip->irq_retrigger)
+        ret = chip->irq_retrigger(data);
+    else
+        ret = irq_chip_retrigger_hierarchy(data);

This should solve this problem while still covering the chips which can
forget to add a retrigger function.

Please let me know if this looks good to you.

In principle, I think what you have is correct.  But there's a
problem in that irq_chip_retrigger_hierarchy() isn't defined
unless CONFIG_IRQ_DOMAIN_HIERARCHY=y.  try_retrigger()
is what you want, but it's static. You could add the #ifdef's here
like in try_retrigger(), but the same problem will occur in Patch 3
with msi_set_affinity(). Really need a non-static version of
try_retrigger(). Or maybe just doing the #ifdef's here and in
msi_set_affinity() is the simplest approach. I don't have a
strong opinion either way.  Sorry this is getting so messy ....

Michael