Re: [PATCH] PCI: brcmstb: Reserve only the MSI vectors that are handed out

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Aug 04, 2026 at 07:50:57AM -0500, Bjorn Helgaas wrote:
> [+cc Marc, author of 4615fbc3788d ("genirq/irqdomain: Don't try to
> free an interrupt that has no mapping"), Thomas; beginning of thread
> about multiple MSI vector alloc/free:
> https://lore.kernel.org/all/20260730072215.2090974-1-sangwoo.han@nearthlab.com]
> 
> On Tue, Aug 04, 2026 at 08:31:23PM +0900, Han / 한상우Sangwoo wrote:
> > > Several other PCI controller drivers have similar code.
> > 
> > I agree - eleven of them look like they can have the same problem.
> > 
> > > I think we should fix them all at once (or explain why they don't need
> > > similar fixes).  Might be worth a little helper so they all work the
> > > same way (e.g., some use order_base_2(), others use get_count_order(),
> > > which seems like a pointless difference).
> > 
> > I am working on a related fix for drivers/irqchip/irq-bcm2712-mip.c,
> 
> Similar pattern there, but it uses ilog2(), which differs from
> order_base_2() and get_count_order() in more cases.
> 
> > which I can test here.  I would rather not send changes to drivers I
> > cannot test, though, so I am leaving the eleven to whoever has the boards.
> > What I looked at is in the appendix, in case it is useful to them.
> > 
> > Two things seem more useful from where I am sitting.
> > 
> >  - The failure is silent.  Appendix A describes the symptom, so that
> >    anyone who hits it later can find this thread.
> > 
> >  - The contract is not written down.  It changed in 4615fbc3788d, where
> >    irq_domain_free_irqs_hierarchy() started freeing one vector at a time.
> > 
> > order_base_2() and get_count_order() give the same answer for any
> > nr_irqs >= 1, so consolidating those two is just cleanup.  Where a shared
> > helper should live I am not sure either.
> 
> I don't know either, but I hate fixing an issue in one place and
> leaving the same issue unfixed nearby.

Just to be clear here, I'm going to defer this until we can fix
everything in drivers/pci at once.  If we can add a fix early in the
cycle, e.g., soon after -rc1, people with other controllers can help
test it.

If we only fix one driver, the pattern will continue as people copy
and paste the bug into new drivers.

So that's why I haven't pulled the pci/controller/brcmstb branch
(which contains this patch) into pci/next.

> > Appendix A - the symptom
> > ========================
> > 
> > A PCIe device whose driver is unbound and rebound a few times, or whose
> > module is reloaded, ends up with fewer MSI vectors than it asked for.  It
> > does not recover until reboot.
> > 
> > Two things have to be true for it to happen: the device uses multi-MSI,
> > and the vector count it asks for is not a power of two.  Devices that ask
> > for a power of two are unaffected, and so is anything on MSI-X.
> > 
> > I have only seen the reduced-vector case.  A driver that insists on the
> > full count would fail to probe instead, but I have not seen that happen.
> > 
> > 
> > Appendix B - how I sorted the drivers
> > =====================================
> > 
> > I went through drivers/pci/controller/ while working out what my own fix
> > had to do, so this is that rather than a full audit.  Twenty of the
> > drivers own an MSI hwirq pool.  I asked three things about each:
> > 
> >  - does .alloc reserve a rounded-up block, or a single slot
> >  - does the core hand .free one vector at a time
> >  - can nr_irqs > 1 reach the driver at all
> > 
> > Eleven answer yes to all three:
> > 
> >   dwc/pcie-designware-host.c   pci-aardvark.c        pcie-apple.c
> >   pcie-aspeed.c                pcie-iproc-msi.c      pcie-mediatek-gen3.c
> >   pcie-rcar-host.c             pcie-rzg3s-host.c     pcie-xilinx-dma-pl.c
> >   pcie-xilinx-nwl.c            pci-hyperv.c
> > 
> > The other nine:
> > 
> >  - five reserve a single slot, so nothing rounds up:
> >    mobiveil/pcie-mobiveil-host.c, pci-xgene-msi.c, pcie-altera-msi.c,
> >    pcie-mediatek.c, plda/pcie-plda-host.c
> > 
> >  - vmd.c keeps a per-vector refcount instead of a bitmap
> > 
> >  - pci-tegra.c and pcie-xilinx.c reserve a rounded-up block, but neither
> >    lists MSI_FLAG_MULTI_PCI_MSI in msi_parent_ops.supported_flags
> > 
> >  - pcie-brcmstb.c is the one you applied