From: Shawn Lin <shawn.lin@rock-chips.com>
To: Bjorn Helgaas <bhelgaas@google.com>
Cc: Nirmal Patel <nirmal.patel@linux.intel.com>,
Jonathan Derrick <jonathan.derrick@linux.dev>,
Kurt Schwemmer <kurt.schwemmer@microsemi.com>,
Logan Gunthorpe <logang@deltatee.com>,
Philipp Stanner <phasta@kernel.org>,
linux-pci@vger.kernel.org, Shawn Lin <shawn.lin@rock-chips.com>
Subject: [PATCH v4 0/7] Add Devres managed IRQ vectors allocation
Date: Fri, 24 Apr 2026 15:57:33 +0800 [thread overview]
Message-ID: <1777017460-243543-1-git-send-email-shawn.lin@rock-chips.com> (raw)
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
next reply other threads:[~2026-04-24 7:58 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-24 7:57 Shawn Lin [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1777017460-243543-1-git-send-email-shawn.lin@rock-chips.com \
--to=shawn.lin@rock-chips.com \
--cc=bhelgaas@google.com \
--cc=jonathan.derrick@linux.dev \
--cc=kurt.schwemmer@microsemi.com \
--cc=linux-pci@vger.kernel.org \
--cc=logang@deltatee.com \
--cc=nirmal.patel@linux.intel.com \
--cc=phasta@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).