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

On 8/19/2026 9:29 PM, Michael Kelley wrote:
> 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

Hi Michael,
Config dependencies make sure that CONFIG_IRQ_DOMAIN_HIERARCHY is 
defined wherever irq_chip_retrigger_hierarchy() is getting called.

x86 SMP or x86-64 guarantees X86_LOCAL_APIC → IRQ_DOMAIN_HIERARCHY, 
PCI_MSI guarantees it via GENERIC_MSI_IRQ. So the #ifdef may not be 
required.

Hi Thomas,
Can you please comment if you are OK with me adding this fallback 
mechanism in next version.

Regards,
Naman