Thread (27 messages) flat view 27 messages, 5 authors, 13d ago

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 AM
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help