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

[Bjorn Helgaas <helgaas@kernel.org>]

[+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.

> 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