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

From: Naman Jain <[email protected]> Sent: Tuesday, August 18, 2026 12:31 AM
> 
> On 8/18/2026 10:40 AM, Michael Kelley wrote:
> > From: Naman Jain <[email protected]> Sent: Monday, August 10, 2026 2:07 AM

[snip]

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