Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH v4 0/7] Add Devres managed IRQ vectors allocation
@ 2026-04-24  7:57 Shawn Lin
  2026-04-24  7:57 ` [PATCH v4 1/7] PCI/MSI: Split __pci_enable_msi_range() for reuse Shawn Lin
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Shawn Lin @ 2026-04-24  7:57 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Nirmal Patel, Jonathan Derrick, Kurt Schwemmer, Logan Gunthorpe,
	Philipp Stanner, linux-pci, Shawn Lin


There is a long-standing design ambiguity in the PCI/MSI subsystem regarding
the management of IRQ vectors. Currently, the management operates in a "hybrid"
mode. Sometimes it's handled by devres but sometimes not, creating potential
for resource management bugs.

Historically, pcim_enable_device() implicitly triggers automatic IRQ vector
management when calling pci_alloc_irq_vectors[_affinity], as pcim_enable_device()
sets the is_managed flag, thus pcim_msi_release() will register a cleanup action.
However, using pci_enable_device() will not make pci_alloc_irq_vectors[_affinity]
register a cleanup action.

This creates an ambiguous ownership model. Many drivers follow a pattern of:
1. Calling pci_alloc_irq_vectors() to allocate interrupts.
2. Also calling pci_free_irq_vectors() in their error paths or remove routines.

When such a driver also uses pcim_enable_device(), the devres framework may
attempt to free the IRQ vectors a second time upon device release, leading to
a double-free. Analysis of the tree shows this hazardous pattern exists widely,
while 37 other drivers correctly rely solely on the implicit cleanup.

To identify the scope of this issue, I used the following shell commands to
search for drivers using pcim_enable_device and allocation functions but missing
the explicit free call:

grep -rl "pcim_enable_device" drivers/ \
  | xargs grep -l -E "pci_alloc_irq_vectors|pci_alloc_irq_vectors_affinity" 2>/dev/null \
  | xargs grep -L "pci_free_irq_vectors" 2>/dev/null

drivers/dma/hsu/pci.c
drivers/dma/hisi_dma.c
drivers/hid/intel-thc-hidintel-quicki2c/pci-quicki2c.
drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
drivers/hid/intel-ish-hid/ipc/pci-ish.c
drivers/accel/ivpu/ivpu_drv.c
drivers/accel/qaic/qaic_drv.c
drivers/accel/amdxdna/aie2_pci.c
drivers/platform/x86/intel/ehl_pse_io.c
drivers/media/pci/intel/ipu6/ipu6.c
drivers/mfd/intel-lpss-pci.c
drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c
drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c
drivers/crypto/inside-secure/safexcel.c
drivers/thunderbolt/nhi.c
drivers/i2c/busses/i2c-mchp-pci1xxxx.c
drivers/i2c/busses/i2c-amd-mp2-pci.c
drivers/i2c/busses/i2c-thunderx-pcidrv.c
drivers/i2c/busses/i2c-designware-pcidrv.c
drivers/tty/serial/8250/8250_exar.c
drivers/tty/serial/8250/8250_pci.c
drivers/tty/serial/8250/8250_mid.c
drivers/gpio/gpio-merrifield.c
drivers/iommu/riscv/iommu-pci.c
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
drivers/pci/switch/switchtec.c
drivers/pci/controller/vmd.c
drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c
drivers/net/ethernet/cavium/thunder/thunder_bgx.c
drivers/net/ethernet/realtek/r8169_main.c
drivers/mmc/host/cavium-thunderx.c
drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c
drivers/cxl/pci.c
drivers/spi/spi-pxa2xx-pci.c
drivers/spi/spi-pci1xxxx.c

To count how many drivers, including using lagacy APIs, have the double-free bugs, the
following shell commands find out 60 drivers.
grep -rl "pcim_enable_device" drivers/ --include="*.c" \
  | xargs grep -l -E "pci_alloc_irq_vectors|pci_alloc_irq_vectors_affinity" 2>/dev/null \
  | xargs grep -l "pci_free_irq_vectors" 2>/dev/null \
  | tee /dev/tty \
  | wc -l

drivers/dma/dw-edma/dw-edma-pcie.c
drivers/dma/plx_dma.c
drivers/dma/amd/ae4dma/ae4dma-pci.c
drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
drivers/perf/hisilicon/hisi_pcie_pmu.c
drivers/perf/hisilicon/hns3_pmu.c
drivers/platform/x86/intel_ips.c
....
drivers/hwtracing/intel_th/pci.c
drivers/bus/mhi/host/pci_generic.c
drivers/fpga/dfl-pci.c
51

grep -rl "pcim_enable_device" drivers/ --include="*.c" \
  | xargs grep -l -E "pci_enable_msi|pci_enable_msix" 2>/dev/null \
  | xargs grep -l "pci_disable_msi" 2>/dev/null \
  | tee /dev/tty \
  | wc -l

drivers/dma/ioat/init.c
drivers/dma/amd/ptdma/ptdma-pci.c
drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci_legacy.c
drivers/crypto/ccp/sp-pci.c
drivers/i2c/busses/i2c-ismt.c
drivers/pci/msi/api.c
drivers/misc/mei/pci-me.c
drivers/misc/mei/pci-txe.c
drivers/block/mtip32xx/mtip32xx.c
9

This series introduces new managed APIs: pcim_alloc_irq_vectors() and
pcim_alloc_irq_vectors_affinity(). Drivers that wish to have devres-managed IRQ
vectors should use these functions to ensure proper automatic cleanup without
ambiguity.

Regarding the removal strategy discussed in v3 [1], two approaches were considered:
a) Introducing another hybrid function temporarily.
b) Duplicating code temporarily.

Following the discussion with Philipp, I agree that adding another hybrid function
does not guarantee the completion of the final cleanup work. Therefore, starting
from v4, I have chosen to duplicate the necessary code temporarily. Helper functions
have been factored out to minimize code duplication.

In the short term, the series converts two drivers within the PCI subsystem to
use the new APIs. The long-term goal is to convert all other drivers which wish to
use these managed functions. For the legacy APIs, like pci_enable_msix_range_*() and
pci_enable_msi(), we won't consider to provide devres-managed version for them but to
fix the only 9 double-free drivers by using the new APIs provided by this series. And
once ready, we could remove the problematic hybrid management pattern from
pcim_setup_msi_release() entirely.

[1]: https://lore.kernel.org/linux-pci/9dc66010d61670dc5182062ef5f1a932f7374323.camel@mailbox.org/T/#t


Changes in v4:
- Rework to create dedicated devres-managed function (Philipp)

Changes in v3:
- Rework the commit message and function doc (Philipp)
- Remove setting is_msi_managed flag from new APIs (Philipp)

Changes in v2:
- Rebase
- Introduce patches only for PCI subsystem to convert the API

Shawn Lin (7):
  PCI/MSI: Split __pci_enable_msi_range() for reuse
  PCI/MSI: Split __pci_enable_msix_range() for reuse
  PCI/MSI: Introduce __pcim_enable_msi_range()
  PCI/MSI: Introduce __pcim_enable_msix_range()
  PCI/MSI: Add Devres managed IRQ vectors allocation
  PCI: switchtec: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  PCI: vmd: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()

 drivers/pci/controller/vmd.c   |   4 +-
 drivers/pci/msi/api.c          |  84 +++++++++++++++++++++++++++++++
 drivers/pci/msi/msi.c          | 110 +++++++++++++++++++++++++++++++++++------
 drivers/pci/msi/msi.h          |   3 ++
 drivers/pci/switch/switchtec.c |   6 +--
 include/linux/pci.h            |  22 +++++++++
 6 files changed, 210 insertions(+), 19 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2026-05-12 13:28 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-24  7:57 [PATCH v4 0/7] Add Devres managed IRQ vectors allocation Shawn Lin
2026-04-24  7:57 ` [PATCH v4 1/7] PCI/MSI: Split __pci_enable_msi_range() for reuse Shawn Lin
2026-05-12 13:08   ` Philipp Stanner
2026-05-12 13:09     ` Philipp Stanner
2026-04-24  7:57 ` [PATCH v4 2/7] PCI/MSI: Split __pci_enable_msix_range() " Shawn Lin
2026-05-12 13:20   ` Philipp Stanner
2026-04-24  7:57 ` [PATCH v4 3/7] PCI/MSI: Introduce __pcim_enable_msi_range() Shawn Lin
2026-05-12 13:22   ` Philipp Stanner
2026-04-24  7:57 ` [PATCH v4 4/7] PCI/MSI: Introduce __pcim_enable_msix_range() Shawn Lin
2026-05-12 13:26   ` Philipp Stanner
2026-04-24  7:57 ` [PATCH v4 5/7] PCI/MSI: Add Devres managed IRQ vectors allocation Shawn Lin
2026-05-12 13:28   ` Philipp Stanner
2026-04-24  7:57 ` [PATCH v4 6/7] PCI: switchtec: Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors() Shawn Lin
2026-04-24  7:57 ` [PATCH v4 7/7] PCI: vmd: " Shawn Lin
2026-05-12 12:57   ` Manivannan Sadhasivam
2026-04-24 11:48 ` [PATCH v4 0/7] Add Devres managed IRQ vectors allocation Philipp Stanner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox