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

> 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,
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.

Thanks,
Sangwoo


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