RE: [PATCH v2 3/3] x86/irq, iommu/amd, PCI: Drop redundant irq_retrigger inits
From: Michael Kelley <hidden>
Date: 2026-09-11 16:32:19
Also in:
linux-iommu, linux-pci, lkml
From: Thomas Gleixner <tglx@kernel.org> Sent: Friday, September 11, 2026 1:12 AM
On Thu, Sep 10 2026 at 18:51, Michael Kelley wrote:quoted
From: Thomas Gleixner <tglx@kernel.org> Sent: Wednesday, September 9, 2026 3:24 AMquoted
In any case 'irqd' in msi_set_affinity() will always point to the vector domain and therefore the exiting code is correct and can't be changed to retrigger hierarchy as that would see irqd->parent == NULL and do nothing. Neither will adding a conditional there do anything useful because the vector domain always has the retrigger callback set.Hmmm. I've been staring at the code to try to understand it all, and I'm not seeing that 'irqd' in msi_set_affinity() always points to the VECTOR domain.You are right and I'm an idiot. My brain was already melted from debating memory ordering issues, then I ended up completely confusing the set affinity and the retrigger logic. Let me try again. What I completely messed up was that the MSI core sets by default msi_domain_set_affinity(), which uses the parent domain, but only if the template/chp does not provide an .irq_set_affinity() callback. Which is not true when remapping is off because x86_init_dev_msi_info() sets it to msi_set_affinity(). Duh! So the change is _not_ broken. Though my argument that retrigger has to happen at the vector level still stands because the outermost chip is in fire and forget mode and to the best of my knowledge none of them have a retrigger at the interrupt chip level. And from a design perspective there won't be any in the future (famous last words).
OK. Good to know I'm on the right track! And I agree that having fixup_irq() and msi_set_affinity() just do irq_chip_retrigger_hierarchy() works today. As for the future, you may remember my RFC patch set from a couple years ago for the Synthetic Interrupt Controller (SynIC) that Hyper-V provides to guests. The SynIC currently has special case nodes under /sys/bus/vmbus to manage CPU affinity. As a better approach, the patch set modelled the SynIC as an irqchip so standard Linux mechanisms could manage affinity. You and I corresponded about per-cpu flow handler tweaks to make it all work. The RFC patch set didn't get traction with the Microsoft folks and I didn't push it. But it had a stand-alone IRQ domain not parented to VECTOR because Hyper-V interrupts are hard coded to HYPERVISOR_CALLBACK_VECTOR. As a stand-alone domain, a .irq_retrigger function might have been useful in fixup_irqs(). That's all speculative, but it was in the back of my head when I suggested to Naman that the outermost chip .irq_retrigger function should be checked. I'm fine with not doing that check now, and if a future case like the Hyper-V SynIC arises, the check can be added.
Devices might have a way to retrigger at the device level, but that's not reachable from the interrupt chip. So both places fixup_irq() and msi_set_affinity() can use retrigger hierarchy safely without the conditional checking the outermost domain chip.
And setting .irq_retrigger in x86_init_dev_msi_info() becomes superfluous, right? Michael