Re: [PATCH] PCI: dwc: Separate MSI out to different controller

Manivannan Sadhasivam
2025-02-19T18:09:39Z

On Wed, Feb 19, 2025 at 06:02:24PM +0000, Marc Zyngier wrote:
> On Wed, 19 Feb 2025 17:51:19 +0000,
> Manivannan Sadhasivam <mani@kernel.org> wrote:
> >
> > On Fri, Feb 14, 2025 at 11:54:52AM -0800, Brian Norris wrote:
> > > On Fri, Feb 14, 2025 at 12:45:52PM +0530, Manivannan Sadhasivam wrote:
> > > > On Tue, Feb 11, 2025 at 04:23:53PM +0800, Tsai Sung-Fu wrote:
> > > > > >Because you cannot set affinity for chained MSIs as these MSIs are muxed to
> > > > > >another parent interrupt. Since the IRQ affinity is all about changing which CPU
> > > > > >gets the IRQ, affinity setting is only possible for the MSI parent.
> > > > >
> > > > > So if we can find the MSI parent by making use of chained
> > > > > relationships (32 MSI vectors muxed to 1 parent),
> > > > > is it possible that we can add that implementation back ?
> > > > > We have another patch that would like to add the
> > > > > dw_pci_msi_set_affinity feature.
> > > > > Would it be a possible try from your perspective ?
> > > > >
> > > >
> > > > This question was brought up plenty of times and the concern from the irqchip
> > > > maintainer Marc was that if you change the affinity of the parent when the child
> > > > MSI affinity changes, it tends to break the userspace ABI of the parent.
> > > >
> > > > See below links:
> > > >
> > > > https://lore.kernel.org/all/87mtg0i8m8.wl-maz@kernel.org/
> > > > https://lore.kernel.org/all/874k0bf7f7.wl-maz@kernel.org/
> > >
> > > It's hard to meaningfully talk about a patch that hasn't been posted
> > > yet, but the implementation we have at least attempts to make *some*
> > > kind of resolution to those ABI questions. For one, it rejects affinity
> > > changes that are incompatible (by some definition) with affinities
> > > requested by other virqs shared on the same parent line. It also updates
> > > their effective affinities upon changes.
> > >
> > > Those replies seem to over-focus on dynamic, user-space initiated
> > > changes too. But how about for "managed-affinity" interrupts? Those are
> > > requested by drivers internally to the kernel (a la
> > > pci_alloc_irq_vectors_affinity()), and can't be changed by user space
> > > afterward. It seems like there'd be room for supporting that, provided
> > > we don't allow conflicting/non-overlapping configurations.
> > >
> > > I do see that Marc sketched out a complex sysfs/hierarchy API in some of
> > > his replies. I'm not sure that would provide too much value to the
> > > managed-affinity cases we're interested in, but I get the appeal for
> > > user-managed affinity.
> > >
> >
> > Whatever your proposal is, please get it reviewed by Marc.
>
> Please see b673fe1a6229a and avoid dragging me into discussion I have
> purposely removed myself from. I'd also appreciate if you didn't
> volunteer me for review tasks I have no intention to perform (this is
> the second time you are doing it, and that's not on).
>

Apologies for not catching up with the MAINTAINERS update. Since you were pretty
much involved in the affinity discussion, I thought about asking your help.

Regarding looping you in, I thought you only wanted to avoid reviewing the new
driver changes that were deviating from the spec too much.

But anyway, now it is clear to me that you are not maintaining the irqchip
drivers, so I will not bother you anymore. Sorry about that.

- Mani

--
மணிவண்ணன் சதாசிவம்