[PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free

[PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free

[Shawn Lin]

This patch series addresses a long-standing design issue in the PCI/MSI
subsystem where the implicit, automatic management of IRQ vectors by
the devres framework conflicts with explicit driver cleanup, creating
ambiguity and potential resource management bugs.

==== The Problem: Implicit vs. Explicit Management ====
Historically, `pcim_enable_device()` not only manages standard PCI resources
(BARs) via devres but also implicitly triggers automatic IRQ vector management
by setting a flag that registers `pcim_msi_release()` as 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 35 other drivers correctly rely solely on the implicit cleanup.

==== The Solution: Making Management Explicit ====
This series enforces a clear, predictable model:
1.  New Managed API (Patch 1/37): Introduces pcim_alloc_irq_vectors() and
    pcim_alloc_irq_vectors_affinity(). Drivers that desire devres-managed IRQ
    vectors should use these functions, which set the is_msi_managed flag and
    ensure automatic cleanup.
2.  Patches 2 through 36 convert each driver that uses pcim_enable_device() alongside
    pci_alloc_irq_vectors() and relies on devres for IRQ vector cleanup to instead
    make an explicit call to pcim_alloc_irq_vectors().
3.  Core Change (Patch 37/37): With the former cleanup, now modifies pcim_setup_msi_release()
    to check only the is_msi_managed flag. This decouples automatic IRQ cleanup from
    pcim_enable_device(). IRQ vectors allocated via pci_alloc_irq_vectors*()
    are now solely the driver's responsibility to free with pci_free_irq_vectors().

With these changes, we clear ownership model: Explicit resource management eliminates
ambiguity and follows the "principle of least surprise." New drivers choose one model and
be consistent.
- Use `pci_alloc_irq_vectors()` + `pci_free_irq_vectors()` for explicit control.
- Use `pcim_alloc_irq_vectors()` for devres-managed, automatic cleanup.

==== Testing And Review ====
1. This series is only compiled test with allmodconfig.
2. Given the substantial size of this patch series, I have structured the mailing
   to facilitate efficient review. The cover letter, the first patch and the last one will be sent
   to all relevant mailing lists and key maintainers to ensure broad visibility and
   initial feedback on the overall approach. The remaining subsystem-specific patches
   will be sent only to the respective subsystem maintainers and their associated
   mailing lists, reducing noise.

Please help review it, much thanks!



Shawn Lin (37):
  PCI/MSI: Add Devres managed IRQ vectors allocation
  mmc: cavium: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  media: ipu6: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  gpio: merrifield: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  PCI: switchtec: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  PCI: vmd: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  spi: spi-pci1xxxx: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  spi: pxa2xx: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  i2c: amd-mp2: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  i2c: mchp-pci1xxxx: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  i2c: thunderx: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  i2c: designware: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  bus: mhi: host: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  serial: 8250_mid: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  serial: 8250_exar: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  platform/x86/intel: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  crypto: safexcel: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  crypto: octeontx2: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  cxl/pci: Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors()
  drm/hisilicon/hibmc: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  iommu/riscv: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  thunderbolt: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  accel/amdxdna: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  accel/ivpu: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  accel/qaic: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  net: stmmac: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  r8169: Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors()
  net: thunder_bgx: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  net: hibmcge: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  mfd: intel-lpss: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  dmaengine: hsu: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  dmaengine: hisilicon: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  i3c: mipi-i3c-hci-pci: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  HID: intel-ish-ipc: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  HID: Intel-thc-hid: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  misc: microchip: pci1xxxx: Replace pci_alloc_irq_vectors() with
    pcim_alloc_irq_vectors()
  PCI/MSI: Only check is_msi_managed in pcim_setup_msi_release()

 drivers/accel/amdxdna/aie2_pci.c                   |  2 +-
 drivers/accel/ivpu/ivpu_drv.c                      |  2 +-
 drivers/accel/qaic/qaic_drv.c                      |  4 ++--
 drivers/bus/mhi/host/pci_generic.c                 |  3 ++-
 drivers/crypto/inside-secure/safexcel.c            |  8 +++----
 drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c |  2 +-
 drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c |  4 ++--
 drivers/cxl/pci.c                                  |  8 ++-----
 drivers/dma/hisi_dma.c                             |  3 +--
 drivers/dma/hsu/pci.c                              |  2 +-
 drivers/gpio/gpio-merrifield.c                     |  2 +-
 drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c    |  4 ++--
 drivers/hid/intel-ish-hid/ipc/pci-ish.c            |  2 +-
 .../intel-thc-hid/intel-quicki2c/pci-quicki2c.c    |  2 +-
 drivers/i2c/busses/i2c-amd-mp2-pci.c               |  2 +-
 drivers/i2c/busses/i2c-designware-pcidrv.c         |  2 +-
 drivers/i2c/busses/i2c-mchp-pci1xxxx.c             |  2 +-
 drivers/i2c/busses/i2c-thunderx-pcidrv.c           |  2 +-
 drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c |  2 +-
 drivers/iommu/riscv/iommu-pci.c                    |  4 ++--
 drivers/media/pci/intel/ipu6/ipu6.c                |  2 +-
 drivers/mfd/intel-lpss-pci.c                       |  2 +-
 drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c      |  2 +-
 drivers/mmc/host/cavium-thunderx.c                 |  2 +-
 drivers/net/ethernet/cavium/thunder/thunder_bgx.c  |  4 ++--
 drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c   |  4 ++--
 drivers/net/ethernet/realtek/r8169_main.c          |  2 +-
 drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c  |  6 ++---
 drivers/pci/controller/vmd.c                       |  4 ++--
 drivers/pci/msi/api.c                              | 26 ++++++++++++++++++++++
 drivers/pci/msi/msi.c                              |  4 +---
 drivers/pci/switch/switchtec.c                     |  6 ++---
 drivers/platform/x86/intel/ehl_pse_io.c            |  2 +-
 drivers/spi/spi-pci1xxxx.c                         |  4 ++--
 drivers/spi/spi-pxa2xx-pci.c                       |  2 +-
 drivers/thunderbolt/nhi.c                          |  6 ++---
 drivers/tty/serial/8250/8250_exar.c                |  2 +-
 drivers/tty/serial/8250/8250_mid.c                 |  2 +-
 include/linux/pci.h                                | 22 ++++++++++++++++++
 39 files changed, 104 insertions(+), 62 deletions(-)

-- 
2.7.4

[PATCH 01/37] PCI/MSI: Add Devres managed IRQ vectors allocation

[Shawn Lin]

pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
pci device drivers which rely on the devres machinery to help cleanup the IRQ
vectors.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/pci/msi/api.c | 26 ++++++++++++++++++++++++++
 include/linux/pci.h   | 22 ++++++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/drivers/pci/msi/api.c b/drivers/pci/msi/api.c
index c18559b..2362fca 100644
--- a/drivers/pci/msi/api.c
+++ b/drivers/pci/msi/api.c
@@ -297,6 +297,32 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 EXPORT_SYMBOL(pci_alloc_irq_vectors_affinity);
 
 /**
+ * pcim_alloc_irq_vectors() - devres managed pci_alloc_irq_vectors()
+ * Interrupt vectors are automatically freed by the devres machinery
+ */
+int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+			   unsigned int max_vecs, unsigned int flags)
+{
+	return pcim_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
+					       flags, NULL);
+}
+EXPORT_SYMBOL(pcim_alloc_irq_vectors);
+
+/**
+ * pcim_alloc_irq_vectors_affinity() - devres managed pci_alloc_irq_vectors_affinity()
+ * Interrupt vectors are automatically freed by the devres machinery
+ */
+int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
+				   unsigned int max_vecs, unsigned int flags,
+				   struct irq_affinity *affd)
+{
+	dev->is_msi_managed = true;
+	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
+					      flags, affd);
+}
+EXPORT_SYMBOL(pcim_alloc_irq_vectors_affinity);
+
+/**
  * pci_irq_vector() - Get Linux IRQ number of a device interrupt vector
  * @dev: the PCI device to operate on
  * @nr:  device-relative interrupt vector index (0-based); has different
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d5ec0f8..ae58f70 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1764,6 +1764,12 @@ int pci_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
 				   unsigned int max_vecs, unsigned int flags,
 				   struct irq_affinity *affd);
 
+int pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+			  unsigned int max_vecs, unsigned int flags);
+int pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
+				   unsigned int max_vecs, unsigned int flags,
+				   struct irq_affinity *affd);
+
 bool pci_msix_can_alloc_dyn(struct pci_dev *dev);
 struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,
 				     const struct irq_affinity_desc *affdesc);
@@ -1806,6 +1812,22 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
 					      flags, NULL);
 }
 
+static inline int
+pcim_alloc_irq_vectors_affinity(struct pci_dev *dev, unsigned int min_vecs,
+			       unsigned int max_vecs, unsigned int flags,
+			       struct irq_affinity *aff_desc)
+{
+	return pci_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
+					      flags, aff_desc);
+}
+static inline int
+pcim_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
+		      unsigned int max_vecs, unsigned int flags)
+{
+	return pcim_alloc_irq_vectors_affinity(dev, min_vecs, max_vecs,
+					      flags, NULL);
+}
+
 static inline bool pci_msix_can_alloc_dyn(struct pci_dev *dev)
 { return false; }
 static inline struct msi_map pci_msix_alloc_irq_at(struct pci_dev *dev, unsigned int index,
-- 
2.7.4

[PATCH 37/37] PCI/MSI: Only check is_msi_managed in pcim_setup_msi_release()

[Shawn Lin]

The function pcim_enable_device() sets the is_managed flag, which
causes the device's IRQ vectors to be automatically managed and released
by the devres framework. If a driver subsequently calls
pci_free_irq_vectors() manually, it can lead to a double-free of the
interrupt resources.

Analysis reveals most PCI drivers call pci_free_irq_vectors()
while also using pcim_enable_device(), making them susceptible to this
double-free issue. In contrast, 35 drivers follow the pattern of
relying on devres to handle the cleanup.

To address this inconsistency and enforce explicit, driver-managed
control of IRQ vectors, this patch removes the pci_is_managed() check
from pcim_setup_msi_release() and let devres help cleanup if is_msi_managed
is true. This change ensures that interrupt vectors are not automatically
freed by the devres machinery when pcim_enable_device() is used, placing
the responsibility for their release squarely on the driver logic via
pci_free_irq_vectors(). If the driver need devres to help cleanup, newly added
pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() helpers could be used.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

 drivers/pci/msi/msi.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
index 81d24a2..0727a0a 100644
--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -70,7 +70,6 @@ static void pcim_msi_release(void *pcidev)
 {
 	struct pci_dev *dev = pcidev;
 
-	dev->is_msi_managed = false;
 	pci_free_irq_vectors(dev);
 }
 
@@ -92,14 +91,13 @@ static int pcim_setup_msi_release(struct pci_dev *dev)
 {
 	int ret;
 
-	if (!pci_is_managed(dev) || dev->is_msi_managed)
+	if (!dev->is_msi_managed)
 		return 0;
 
 	ret = devm_add_action(&dev->dev, pcim_msi_release, dev);
 	if (ret)
 		return ret;
 
-	dev->is_msi_managed = true;
 	return 0;
 }
 
-- 
2.7.4

Re: [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free

[Andy Shevchenko]

On Mon, Feb 23, 2026 at 5:32 PM Shawn Lin <shawn.lin@rock-chips.com> wrote:
>
> This patch series addresses a long-standing design issue in the PCI/MSI
> subsystem where the implicit, automatic management of IRQ vectors by
> the devres framework conflicts with explicit driver cleanup, creating
> ambiguity and potential resource management bugs.
>
> ==== The Problem: Implicit vs. Explicit Management ====
> Historically, `pcim_enable_device()` not only manages standard PCI resources
> (BARs) via devres but also implicitly triggers automatic IRQ vector management
> by setting a flag that registers `pcim_msi_release()` as 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 35 other drivers correctly rely solely on the implicit cleanup.

Is this confirmed? What I read from the cover letter, this series was
only compile-tested, so how can you prove the problem exists in the
first place?

> ==== The Solution: Making Management Explicit ====
> This series enforces a clear, predictable model:
> 1.  New Managed API (Patch 1/37): Introduces pcim_alloc_irq_vectors() and
>     pcim_alloc_irq_vectors_affinity(). Drivers that desire devres-managed IRQ
>     vectors should use these functions, which set the is_msi_managed flag and
>     ensure automatic cleanup.
> 2.  Patches 2 through 36 convert each driver that uses pcim_enable_device() alongside
>     pci_alloc_irq_vectors() and relies on devres for IRQ vector cleanup to instead
>     make an explicit call to pcim_alloc_irq_vectors().
> 3.  Core Change (Patch 37/37): With the former cleanup, now modifies pcim_setup_msi_release()
>     to check only the is_msi_managed flag. This decouples automatic IRQ cleanup from
>     pcim_enable_device(). IRQ vectors allocated via pci_alloc_irq_vectors*()
>     are now solely the driver's responsibility to free with pci_free_irq_vectors().
>
> With these changes, we clear ownership model: Explicit resource management eliminates
> ambiguity and follows the "principle of least surprise." New drivers choose one model and
> be consistent.
> - Use `pci_alloc_irq_vectors()` + `pci_free_irq_vectors()` for explicit control.
> - Use `pcim_alloc_irq_vectors()` for devres-managed, automatic cleanup.

Have you checked previous attempts? Why is your series better than those?

> ==== Testing And Review ====
> 1. This series is only compiled test with allmodconfig.
> 2. Given the substantial size of this patch series, I have structured the mailing
>    to facilitate efficient review. The cover letter, the first patch and the last one will be sent
>    to all relevant mailing lists and key maintainers to ensure broad visibility and
>    initial feedback on the overall approach. The remaining subsystem-specific patches
>    will be sent only to the respective subsystem maintainers and their associated
>    mailing lists, reducing noise.

-- 
With Best Regards,
Andy Shevchenko

[PATCH 17/37] crypto: safexcel: Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors()

[Shawn Lin]

pcim_enable_device() no longer automatically manages IRQ vectors via devres.
Drivers must now manually call pci_free_irq_vectors() for cleanup. Alternatively,
pcim_alloc_irq_vectors() should be used.

To: Antoine Tenart <atenart@kernel.org>
To: Herbert Xu <herbert@gondor.apana.org.au>
To: davem@davemloft.net
Cc: linux-crypto@vger.kernel.org
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Philipp Stanner <phasta@kernel.org>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/crypto/inside-secure/safexcel.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index c3b2b22..4e4991e 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -1596,10 +1596,10 @@ static int safexcel_probe_generic(void *pdev,
 		 */
 		struct pci_dev *pci_pdev = pdev;
 
-		ret = pci_alloc_irq_vectors(pci_pdev,
-					    priv->config.rings + 1,
-					    priv->config.rings + 1,
-					    PCI_IRQ_MSI | PCI_IRQ_MSIX);
+		ret = pcim_alloc_irq_vectors(pci_pdev,
+					     priv->config.rings + 1,
+					     priv->config.rings + 1,
+					     PCI_IRQ_MSI | PCI_IRQ_MSIX);
 		if (ret < 0) {
 			dev_err(dev, "Failed to allocate PCI MSI interrupts\n");
 			return ret;
-- 
2.7.4

[PATCH 18/37] crypto: octeontx2: Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors()

[Shawn Lin]

pcim_enable_device() no longer automatically manages IRQ vectors via devres.
Drivers must now manually call pci_free_irq_vectors() for cleanup. Alternatively,
pcim_alloc_irq_vectors() should be used.

To: Srujana Challa <schalla@marvell.com>
To: Bharat Bhushan <bbhushan2@marvell.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
To: davem@davemloft.net
Cc: linux-crypto@vger.kernel.org
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Philipp Stanner <phasta@kernel.org>
Cc: linux-pci@vger.kernel.org
Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c | 2 +-
 drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c b/drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c
index f54f905..fbcc65c 100644
--- a/drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c
+++ b/drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c
@@ -774,7 +774,7 @@ static int otx2_cptpf_probe(struct pci_dev *pdev,
 		goto clear_drvdata;
 	}
 
-	err = pci_alloc_irq_vectors(pdev, num_vec, num_vec, PCI_IRQ_MSIX);
+	err = pcim_alloc_irq_vectors(pdev, num_vec, num_vec, PCI_IRQ_MSIX);
 	if (err < 0) {
 		dev_err(dev, "Request for %d msix vectors failed\n",
 			RVU_PF_INT_VEC_CNT);
diff --git a/drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c b/drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c
index c1c44a7b..3f4d791 100644
--- a/drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c
+++ b/drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c
@@ -42,8 +42,8 @@ static int cptvf_register_interrupts(struct otx2_cptvf_dev *cptvf)
 		return -EINVAL;
 
 	/* Enable MSI-X */
-	ret = pci_alloc_irq_vectors(cptvf->pdev, num_vec, num_vec,
-				    PCI_IRQ_MSIX);
+	ret = pcim_alloc_irq_vectors(cptvf->pdev, num_vec, num_vec,
+				     PCI_IRQ_MSIX);
 	if (ret < 0) {
 		dev_err(&cptvf->pdev->dev,
 			"Request for %d msix vectors failed\n", num_vec);
-- 
2.7.4

Re: [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free

[Shawn Lin]

Hi Andy

在 2026/02/23 星期一 23:50, Andy Shevchenko 写道:
> On Mon, Feb 23, 2026 at 5:32 PM Shawn Lin <shawn.lin@rock-chips.com> wrote:
>>
>> This patch series addresses a long-standing design issue in the PCI/MSI
>> subsystem where the implicit, automatic management of IRQ vectors by
>> the devres framework conflicts with explicit driver cleanup, creating
>> ambiguity and potential resource management bugs.
>>
>> ==== The Problem: Implicit vs. Explicit Management ====
>> Historically, `pcim_enable_device()` not only manages standard PCI resources
>> (BARs) via devres but also implicitly triggers automatic IRQ vector management
>> by setting a flag that registers `pcim_msi_release()` as 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 35 other drivers correctly rely solely on the implicit cleanup.
> 
> Is this confirmed? What I read from the cover letter, this series was
> only compile-tested, so how can you prove the problem exists in the
> first place?

Yes, it's confirmed. My debug of a double free issue of a out-of-tree
PCIe wifi driver which uses
pcim_enable_device + pci_alloc_irq_vectors + pci_free_irq_vectors expose
it. And we did have a TODO to cleanup this hybrid usage, targeted in
this cycle[1] suggested by Philipp:

[1] https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=msi

> 
>> ==== The Solution: Making Management Explicit ====
>> This series enforces a clear, predictable model:
>> 1.  New Managed API (Patch 1/37): Introduces pcim_alloc_irq_vectors() and
>>      pcim_alloc_irq_vectors_affinity(). Drivers that desire devres-managed IRQ
>>      vectors should use these functions, which set the is_msi_managed flag and
>>      ensure automatic cleanup.
>> 2.  Patches 2 through 36 convert each driver that uses pcim_enable_device() alongside
>>      pci_alloc_irq_vectors() and relies on devres for IRQ vector cleanup to instead
>>      make an explicit call to pcim_alloc_irq_vectors().
>> 3.  Core Change (Patch 37/37): With the former cleanup, now modifies pcim_setup_msi_release()
>>      to check only the is_msi_managed flag. This decouples automatic IRQ cleanup from
>>      pcim_enable_device(). IRQ vectors allocated via pci_alloc_irq_vectors*()
>>      are now solely the driver's responsibility to free with pci_free_irq_vectors().
>>
>> With these changes, we clear ownership model: Explicit resource management eliminates
>> ambiguity and follows the "principle of least surprise." New drivers choose one model and
>> be consistent.
>> - Use `pci_alloc_irq_vectors()` + `pci_free_irq_vectors()` for explicit control.
>> - Use `pcim_alloc_irq_vectors()` for devres-managed, automatic cleanup.
> 
> Have you checked previous attempts? Why is your series better than those?

There seems not previous attempts.

> 
>> ==== Testing And Review ====
>> 1. This series is only compiled test with allmodconfig.
>> 2. Given the substantial size of this patch series, I have structured the mailing
>>     to facilitate efficient review. The cover letter, the first patch and the last one will be sent
>>     to all relevant mailing lists and key maintainers to ensure broad visibility and
>>     initial feedback on the overall approach. The remaining subsystem-specific patches
>>     will be sent only to the respective subsystem maintainers and their associated
>>     mailing lists, reducing noise.
> 

Re: [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free

[Andy Shevchenko]

On Tue, Feb 24, 2026 at 12:09:37AM +0800, Shawn Lin wrote:
> 在 2026/02/23 星期一 23:50, Andy Shevchenko 写道:
> > On Mon, Feb 23, 2026 at 5:32 PM Shawn Lin <shawn.lin@rock-chips.com> wrote:
> > > 
> > > This patch series addresses a long-standing design issue in the PCI/MSI
> > > subsystem where the implicit, automatic management of IRQ vectors by
> > > the devres framework conflicts with explicit driver cleanup, creating
> > > ambiguity and potential resource management bugs.
> > > 
> > > ==== The Problem: Implicit vs. Explicit Management ====
> > > Historically, `pcim_enable_device()` not only manages standard PCI resources
> > > (BARs) via devres but also implicitly triggers automatic IRQ vector management
> > > by setting a flag that registers `pcim_msi_release()` as 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 35 other drivers correctly rely solely on the implicit cleanup.
> > 
> > Is this confirmed? What I read from the cover letter, this series was
> > only compile-tested, so how can you prove the problem exists in the
> > first place?
> 
> Yes, it's confirmed. My debug of a double free issue of a out-of-tree
> PCIe wifi driver which uses
> pcim_enable_device + pci_alloc_irq_vectors + pci_free_irq_vectors expose
> it. And we did have a TODO to cleanup this hybrid usage, targeted in
> this cycle[1] suggested by Philipp:

Okay, fair enough. I think this bit was missing in the cover letter.

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=msi

> > > ==== The Solution: Making Management Explicit ====
> > > This series enforces a clear, predictable model:
> > > 1.  New Managed API (Patch 1/37): Introduces pcim_alloc_irq_vectors() and
> > >      pcim_alloc_irq_vectors_affinity(). Drivers that desire devres-managed IRQ
> > >      vectors should use these functions, which set the is_msi_managed flag and
> > >      ensure automatic cleanup.
> > > 2.  Patches 2 through 36 convert each driver that uses pcim_enable_device() alongside
> > >      pci_alloc_irq_vectors() and relies on devres for IRQ vector cleanup to instead
> > >      make an explicit call to pcim_alloc_irq_vectors().
> > > 3.  Core Change (Patch 37/37): With the former cleanup, now modifies pcim_setup_msi_release()
> > >      to check only the is_msi_managed flag. This decouples automatic IRQ cleanup from
> > >      pcim_enable_device(). IRQ vectors allocated via pci_alloc_irq_vectors*()
> > >      are now solely the driver's responsibility to free with pci_free_irq_vectors().
> > > 
> > > With these changes, we clear ownership model: Explicit resource management eliminates
> > > ambiguity and follows the "principle of least surprise." New drivers choose one model and
> > > be consistent.
> > > - Use `pci_alloc_irq_vectors()` + `pci_free_irq_vectors()` for explicit control.
> > > - Use `pcim_alloc_irq_vectors()` for devres-managed, automatic cleanup.
> > 
> > Have you checked previous attempts? Why is your series better than those?
> 
> There seems not previous attempts.

Maybe we are looking to the different projects...

https://lore.kernel.org/all/?q=pcim_alloc_irq_vectors

> > > ==== Testing And Review ====
> > > 1. This series is only compiled test with allmodconfig.
> > > 2. Given the substantial size of this patch series, I have structured the mailing
> > >     to facilitate efficient review. The cover letter, the first patch and the last one will be sent
> > >     to all relevant mailing lists and key maintainers to ensure broad visibility and
> > >     initial feedback on the overall approach. The remaining subsystem-specific patches
> > >     will be sent only to the respective subsystem maintainers and their associated
> > >     mailing lists, reducing noise.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 01/37] PCI/MSI: Add Devres managed IRQ vectors allocation

[Jakub Kicinski]

On Mon, 23 Feb 2026 23:29:40 +0800 Shawn Lin wrote:
> pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
> pci device drivers which rely on the devres machinery to help cleanup the IRQ
> vectors.

If you can please add this API with just a few users, and then convert
remaining users via the subsystem trees in the next cycle.
There's no need to risk wasting maintainer time on conflicts with
conversions like this.

Re: [PATCH 01/37] PCI/MSI: Add Devres managed IRQ vectors allocation

[Shawn Lin]

在 2026/02/24 星期二 8:04, Jakub Kicinski 写道:
> On Mon, 23 Feb 2026 23:29:40 +0800 Shawn Lin wrote:
>> pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
>> pci device drivers which rely on the devres machinery to help cleanup the IRQ
>> vectors.
> 
> If you can please add this API with just a few users, and then convert
> remaining users via the subsystem trees in the next cycle.
> There's no need to risk wasting maintainer time on conflicts with
> conversions like this.

Thanks for the suggestion, Jakub. I have little experience with 
cross-subsystem cleanups like this, so your suggestion is very helpful.

> 

Re: [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free

[Shawn Lin]

在 2026/02/24 星期二 1:38, Andy Shevchenko 写道:
> On Tue, Feb 24, 2026 at 12:09:37AM +0800, Shawn Lin wrote:
>> 在 2026/02/23 星期一 23:50, Andy Shevchenko 写道:
>>> On Mon, Feb 23, 2026 at 5:32 PM Shawn Lin <shawn.lin@rock-chips.com> wrote:
>>>>
>>>> This patch series addresses a long-standing design issue in the PCI/MSI
>>>> subsystem where the implicit, automatic management of IRQ vectors by
>>>> the devres framework conflicts with explicit driver cleanup, creating
>>>> ambiguity and potential resource management bugs.
>>>>
>>>> ==== The Problem: Implicit vs. Explicit Management ====
>>>> Historically, `pcim_enable_device()` not only manages standard PCI resources
>>>> (BARs) via devres but also implicitly triggers automatic IRQ vector management
>>>> by setting a flag that registers `pcim_msi_release()` as 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 35 other drivers correctly rely solely on the implicit cleanup.
>>>
>>> Is this confirmed? What I read from the cover letter, this series was
>>> only compile-tested, so how can you prove the problem exists in the
>>> first place?
>>
>> Yes, it's confirmed. My debug of a double free issue of a out-of-tree
>> PCIe wifi driver which uses
>> pcim_enable_device + pci_alloc_irq_vectors + pci_free_irq_vectors expose
>> it. And we did have a TODO to cleanup this hybrid usage, targeted in
>> this cycle[1] suggested by Philipp:
> 
> Okay, fair enough. I think this bit was missing in the cover letter.
> 
>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=msi
> 
>>>> ==== The Solution: Making Management Explicit ====
>>>> This series enforces a clear, predictable model:
>>>> 1.  New Managed API (Patch 1/37): Introduces pcim_alloc_irq_vectors() and
>>>>       pcim_alloc_irq_vectors_affinity(). Drivers that desire devres-managed IRQ
>>>>       vectors should use these functions, which set the is_msi_managed flag and
>>>>       ensure automatic cleanup.
>>>> 2.  Patches 2 through 36 convert each driver that uses pcim_enable_device() alongside
>>>>       pci_alloc_irq_vectors() and relies on devres for IRQ vector cleanup to instead
>>>>       make an explicit call to pcim_alloc_irq_vectors().
>>>> 3.  Core Change (Patch 37/37): With the former cleanup, now modifies pcim_setup_msi_release()
>>>>       to check only the is_msi_managed flag. This decouples automatic IRQ cleanup from
>>>>       pcim_enable_device(). IRQ vectors allocated via pci_alloc_irq_vectors*()
>>>>       are now solely the driver's responsibility to free with pci_free_irq_vectors().
>>>>
>>>> With these changes, we clear ownership model: Explicit resource management eliminates
>>>> ambiguity and follows the "principle of least surprise." New drivers choose one model and
>>>> be consistent.
>>>> - Use `pci_alloc_irq_vectors()` + `pci_free_irq_vectors()` for explicit control.
>>>> - Use `pcim_alloc_irq_vectors()` for devres-managed, automatic cleanup.
>>>
>>> Have you checked previous attempts? Why is your series better than those?
>>

Thanks for sharing this 5-years-old discusstion, I totally missed it.

I read the V7 discussion, and it seems to have disappeared without much
follow-up, like a stone dropped into the ocean. For five years, newly
added drivers have continued to misuse these APIs incorrectly, and
we’ve been watching it happen. I can’t really claim this patch series
is inherently better than Dejin’s earlier work at its core, this is
just about fixing one entire category of misuse in a single pass.

According to Bjorn's final search and reply, if we include the removal
of deprecated APIs, it would require a massive amount of work and might
span many release cycles. Unfortunately, the work never began, and the
cleanup might never be completed. I’m not sure if folks have changed
their minds now. Can we at least start by completing the changes for the
pci_alloc_irq_vectors category?


>> There seems not previous attempts.
> 
> Maybe we are looking to the different projects...
> 
> https://lore.kernel.org/all/?q=pcim_alloc_irq_vectors
> 
>>>> ==== Testing And Review ====
>>>> 1. This series is only compiled test with allmodconfig.
>>>> 2. Given the substantial size of this patch series, I have structured the mailing
>>>>      to facilitate efficient review. The cover letter, the first patch and the last one will be sent
>>>>      to all relevant mailing lists and key maintainers to ensure broad visibility and
>>>>      initial feedback on the overall approach. The remaining subsystem-specific patches
>>>>      will be sent only to the respective subsystem maintainers and their associated
>>>>      mailing lists, reducing noise.
> 

Re: [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free

[Simon Richter]

Hi,

On 2/24/26 12:29 AM, Shawn Lin wrote:

> 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 35 other drivers correctly rely solely on the implicit cleanup.

Would it make sense to have a function pcim_free_irq_vectors(), to allow 
explicit freeing even if the device is otherwise managed, analogous to 
pcim_iounmap()?

    Simon

Re: [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free

[Philipp Stanner]

On Tue, 2026-02-24 at 13:14 +0900, Simon Richter wrote:
> Hi,
> 
> On 2/24/26 12:29 AM, Shawn Lin wrote:
> 
> > 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 35 other drivers correctly rely solely on the implicit cleanup.
> 
> Would it make sense to have a function pcim_free_irq_vectors(), to allow 
> explicit freeing even if the device is otherwise managed, analogous to 
> pcim_iounmap()?

We used to add those. In part because it is easier to port old users.

Nowadays I tend to think that those APIs were more on the too-complex
than too-simple side for a long time. As an expert or as the API
designer you wouldn't expect it, but there are actually far too many
users who came to believe they always have to use pcim_iounmap() and
counter parts.

If I could design it from scratch I would probably try to tell users to
use the unmanaged versions instead of revoking the devres consequence.

Devres is actually about your consequence always happening whenever the
driver unloads, for whatever reason.


P.

Re: [PATCH 01/37] PCI/MSI: Add Devres managed IRQ vectors allocation

[Philipp Stanner]

On Tue, 2026-02-24 at 10:08 +0800, Shawn Lin wrote:
> 在 2026/02/24 星期二 8:04, Jakub Kicinski 写道:
> > On Mon, 23 Feb 2026 23:29:40 +0800 Shawn Lin wrote:
> > > pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
> > > pci device drivers which rely on the devres machinery to help cleanup the IRQ
> > > vectors.
> > 
> > If you can please add this API with just a few users, and then convert
> > remaining users via the subsystem trees in the next cycle.
> > There's no need to risk wasting maintainer time on conflicts with
> > conversions like this.
> 
> Thanks for the suggestion, Jakub. I have little experience with 
> cross-subsystem cleanups like this, so your suggestion is very helpful.


When I removed the hybrid nature of pci_request_region() et al., I
concluded that there were so few users that doing them all in one run
was sufficient.

For larger reworks, like removing pcim_iomap_table(), a slower step-by-
step strategy is necessary for the reasons that Jakub details.

It is then smart to omit an easy to port subsystem / driver for the
ultimate patch series where one then removes the hybrid behavior from
PCI itself, after porting the last driver.

In general,  as Jakub details, those step-by-step cleanups are a bit
safer, since you can proof valid behavior early on and in case of an
explosion they are very easy to revert.


P.

Re: [PATCH 01/37] PCI/MSI: Add Devres managed IRQ vectors allocation

[Shawn Lin]

在 2026/02/24 星期二 15:47, Philipp Stanner 写道:
> On Tue, 2026-02-24 at 10:08 +0800, Shawn Lin wrote:
>> 在 2026/02/24 星期二 8:04, Jakub Kicinski 写道:
>>> On Mon, 23 Feb 2026 23:29:40 +0800 Shawn Lin wrote:
>>>> pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
>>>> pci device drivers which rely on the devres machinery to help cleanup the IRQ
>>>> vectors.
>>>
>>> If you can please add this API with just a few users, and then convert
>>> remaining users via the subsystem trees in the next cycle.
>>> There's no need to risk wasting maintainer time on conflicts with
>>> conversions like this.
>>
>> Thanks for the suggestion, Jakub. I have little experience with
>> cross-subsystem cleanups like this, so your suggestion is very helpful.
> 
> 
> When I removed the hybrid nature of pci_request_region() et al., I
> concluded that there were so few users that doing them all in one run
> was sufficient.
> 
> For larger reworks, like removing pcim_iomap_table(), a slower step-by-
> step strategy is necessary for the reasons that Jakub details.
> 
> It is then smart to omit an easy to port subsystem / driver for the
> ultimate patch series where one then removes the hybrid behavior from
> PCI itself, after porting the last driver.
> 
> In general,  as Jakub details, those step-by-step cleanups are a bit
> safer, since you can proof valid behavior early on and in case of an
> explosion they are very easy to revert.
> 

Thank you, Philipp. I wish I had attended your talk at FOSDEM 2025 on
removing pcim_iomap_table earlier. This first version was perhaps a bit 
too aggressive. For v2, I think the plan should start with addressing 
the switchtec and vmd drivers, since both of those, along with the new 
API additions, can be handled entirely within the PCI subsystem scope.

> 
> P.
> 

Re: [PATCH 01/37] PCI/MSI: Add Devres managed IRQ vectors allocation

[Philipp Stanner]

On Tue, 2026-02-24 at 16:21 +0800, Shawn Lin wrote:
> 在 2026/02/24 星期二 15:47, Philipp Stanner 写道:
> > On Tue, 2026-02-24 at 10:08 +0800, Shawn Lin wrote:
> > > 在 2026/02/24 星期二 8:04, Jakub Kicinski 写道:
> > > > On Mon, 23 Feb 2026 23:29:40 +0800 Shawn Lin wrote:
> > > > > pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
> > > > > pci device drivers which rely on the devres machinery to help cleanup the IRQ
> > > > > vectors.
> > > > 
> > > > If you can please add this API with just a few users, and then convert
> > > > remaining users via the subsystem trees in the next cycle.
> > > > There's no need to risk wasting maintainer time on conflicts with
> > > > conversions like this.
> > > 
> > > Thanks for the suggestion, Jakub. I have little experience with
> > > cross-subsystem cleanups like this, so your suggestion is very helpful.
> > 
> > 
> > When I removed the hybrid nature of pci_request_region() et al., I
> > concluded that there were so few users that doing them all in one run
> > was sufficient.
> > 
> > For larger reworks, like removing pcim_iomap_table(), a slower step-by-
> > step strategy is necessary for the reasons that Jakub details.
> > 
> > It is then smart to omit an easy to port subsystem / driver for the
> > ultimate patch series where one then removes the hybrid behavior from
> > PCI itself, after porting the last driver.
> > 
> > In general,  as Jakub details, those step-by-step cleanups are a bit
> > safer, since you can proof valid behavior early on and in case of an
> > explosion they are very easy to revert.
> > 
> 
> Thank you, Philipp. I wish I had attended your talk at FOSDEM 2025 on
> removing pcim_iomap_table earlier. This first version was perhaps a bit 
> too aggressive.

No worries at all, it's very cool that you pick this work up!

>  For v2, I think the plan should start with addressing
> the switchtec and vmd drivers, since both of those, along with the new 
> API additions, can be handled entirely within the PCI subsystem scope.

Sounds reasonable to me.

Regards
Philipp

Re: [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free

[Andy Shevchenko]

On Tue, Feb 24, 2026 at 08:39:43AM +0100, Philipp Stanner wrote:
> On Tue, 2026-02-24 at 13:14 +0900, Simon Richter wrote:
> > On 2/24/26 12:29 AM, Shawn Lin wrote:

> > > 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 35 other drivers correctly rely solely on the implicit cleanup.
> > 
> > Would it make sense to have a function pcim_free_irq_vectors(), to allow 
> > explicit freeing even if the device is otherwise managed, analogous to 
> > pcim_iounmap()?
> 
> We used to add those. In part because it is easier to port old users.
> 
> Nowadays I tend to think that those APIs were more on the too-complex
> than too-simple side for a long time. As an expert or as the API
> designer you wouldn't expect it, but there are actually far too many
> users who came to believe they always have to use pcim_iounmap() and
> counter parts.
> 
> If I could design it from scratch I would probably try to tell users to
> use the unmanaged versions instead of revoking the devres consequence.

+many.

> Devres is actually about your consequence always happening whenever the
> driver unloads, for whatever reason.

I believe you meant "unbinds". The device<-->driver link can be broken
without unloading the driver.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free

[Philipp Stanner]

On Tue, 2026-02-24 at 11:12 +0200, Andy Shevchenko wrote:
> On Tue, Feb 24, 2026 at 08:39:43AM +0100, Philipp Stanner wrote:
> > On Tue, 2026-02-24 at 13:14 +0900, Simon Richter wrote:
> > > On 2/24/26 12:29 AM, Shawn Lin wrote:
> 
> > > > 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 35 other drivers correctly rely solely on the implicit cleanup.
> > > 
> > > Would it make sense to have a function pcim_free_irq_vectors(), to allow 
> > > explicit freeing even if the device is otherwise managed, analogous to 
> > > pcim_iounmap()?
> > 
> > We used to add those. In part because it is easier to port old users.
> > 
> > Nowadays I tend to think that those APIs were more on the too-complex
> > than too-simple side for a long time. As an expert or as the API
> > designer you wouldn't expect it, but there are actually far too many
> > users who came to believe they always have to use pcim_iounmap() and
> > counter parts.
> > 
> > If I could design it from scratch I would probably try to tell users to
> > use the unmanaged versions instead of revoking the devres consequence.
> 
> +many.

hm?

> 
> > Devres is actually about your consequence always happening whenever the
> > driver unloads, for whatever reason.
> 
> I believe you meant "unbinds". The device<-->driver link can be broken
> without unloading the driver.

Yes, thx for pointing that out. Greg KH AFAIK always calls it "driver
detach".

P.

Re: [PATCH 0/37] PCI/MSI: Enforce explicit IRQ vector management by removing devres auto-free

[Andy Shevchenko]

On Tue, Feb 24, 2026 at 11:30:28AM +0100, Philipp Stanner wrote:
> On Tue, 2026-02-24 at 11:12 +0200, Andy Shevchenko wrote:
> > On Tue, Feb 24, 2026 at 08:39:43AM +0100, Philipp Stanner wrote:
> > > On Tue, 2026-02-24 at 13:14 +0900, Simon Richter wrote:

...

> > > If I could design it from scratch I would probably try to tell users to
> > > use the unmanaged versions instead of revoking the devres consequence.
> > 
> > +many.

> hm?

I'm supporting you with many hands up (more than I possess)!

> > > Devres is actually about your consequence always happening whenever the
> > > driver unloads, for whatever reason.
> > 
> > I believe you meant "unbinds". The device<-->driver link can be broken
> > without unloading the driver.
> 
> Yes, thx for pointing that out. Greg KH AFAIK always calls it "driver
> detach".

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 17/37] crypto: safexcel: Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors()

[kernel test robot]

Hi Shawn,

kernel test robot noticed the following build errors:

[auto build test ERROR on pci/next]
[also build test ERROR on pci/for-linus andi-shyti/i2c/i2c-host drm-misc/drm-misc-next linus/master v7.0-rc1 next-20260223]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Shawn-Lin/PCI-MSI-Add-Devres-managed-IRQ-vectors-allocation/20260224-161502
base:   https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link:    https://lore.kernel.org/r/1771861910-88163-1-git-send-email-shawn.lin%40rock-chips.com
patch subject: [PATCH 17/37] crypto: safexcel: Replace pci_alloc_irq_vectors() with pcim_alloc_irq_vectors()
config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20260224/202602242339.UGFAC9fD-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260224/202602242339.UGFAC9fD-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602242339.UGFAC9fD-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/crypto/inside-secure/safexcel.c: In function 'safexcel_probe_generic':
>> drivers/crypto/inside-secure/safexcel.c:1599:23: error: implicit declaration of function 'pcim_alloc_irq_vectors'; did you mean 'pci_alloc_irq_vectors'? [-Wimplicit-function-declaration]
    1599 |                 ret = pcim_alloc_irq_vectors(pci_pdev,
         |                       ^~~~~~~~~~~~~~~~~~~~~~
         |                       pci_alloc_irq_vectors


vim +1599 drivers/crypto/inside-secure/safexcel.c

  1381	
  1382	/*
  1383	 * Generic part of probe routine, shared by platform and PCI driver
  1384	 *
  1385	 * Assumes IO resources have been mapped, private data mem has been allocated,
  1386	 * clocks have been enabled, device pointer has been assigned etc.
  1387	 *
  1388	 */
  1389	static int safexcel_probe_generic(void *pdev,
  1390					  struct safexcel_crypto_priv *priv,
  1391					  int is_pci_dev)
  1392	{
  1393		struct device *dev = priv->dev;
  1394		u32 peid, version, mask, val, hiaopt, hwopt, peopt;
  1395		int i, ret, hwctg;
  1396	
  1397		priv->context_pool = dmam_pool_create("safexcel-context", dev,
  1398						      sizeof(struct safexcel_context_record),
  1399						      1, 0);
  1400		if (!priv->context_pool)
  1401			return -ENOMEM;
  1402	
  1403		/*
  1404		 * First try the EIP97 HIA version regs
  1405		 * For the EIP197, this is guaranteed to NOT return any of the test
  1406		 * values
  1407		 */
  1408		version = readl(priv->base + EIP97_HIA_AIC_BASE + EIP197_HIA_VERSION);
  1409	
  1410		mask = 0;  /* do not swap */
  1411		if (EIP197_REG_LO16(version) == EIP197_HIA_VERSION_LE) {
  1412			priv->hwconfig.hiaver = EIP197_VERSION_MASK(version);
  1413		} else if (EIP197_REG_HI16(version) == EIP197_HIA_VERSION_BE) {
  1414			/* read back byte-swapped, so complement byte swap bits */
  1415			mask = EIP197_MST_CTRL_BYTE_SWAP_BITS;
  1416			priv->hwconfig.hiaver = EIP197_VERSION_SWAP(version);
  1417		} else {
  1418			/* So it wasn't an EIP97 ... maybe it's an EIP197? */
  1419			version = readl(priv->base + EIP197_HIA_AIC_BASE +
  1420					EIP197_HIA_VERSION);
  1421			if (EIP197_REG_LO16(version) == EIP197_HIA_VERSION_LE) {
  1422				priv->hwconfig.hiaver = EIP197_VERSION_MASK(version);
  1423				priv->flags |= SAFEXCEL_HW_EIP197;
  1424			} else if (EIP197_REG_HI16(version) ==
  1425				   EIP197_HIA_VERSION_BE) {
  1426				/* read back byte-swapped, so complement swap bits */
  1427				mask = EIP197_MST_CTRL_BYTE_SWAP_BITS;
  1428				priv->hwconfig.hiaver = EIP197_VERSION_SWAP(version);
  1429				priv->flags |= SAFEXCEL_HW_EIP197;
  1430			} else {
  1431				return -ENODEV;
  1432			}
  1433		}
  1434	
  1435		/* Now initialize the reg offsets based on the probing info so far */
  1436		safexcel_init_register_offsets(priv);
  1437	
  1438		/*
  1439		 * If the version was read byte-swapped, we need to flip the device
  1440		 * swapping Keep in mind here, though, that what we write will also be
  1441		 * byte-swapped ...
  1442		 */
  1443		if (mask) {
  1444			val = readl(EIP197_HIA_AIC(priv) + EIP197_HIA_MST_CTRL);
  1445			val = val ^ (mask >> 24); /* toggle byte swap bits */
  1446			writel(val, EIP197_HIA_AIC(priv) + EIP197_HIA_MST_CTRL);
  1447		}
  1448	
  1449		/*
  1450		 * We're not done probing yet! We may fall through to here if no HIA
  1451		 * was found at all. So, with the endianness presumably correct now and
  1452		 * the offsets setup, *really* probe for the EIP97/EIP197.
  1453		 */
  1454		version = readl(EIP197_GLOBAL(priv) + EIP197_VERSION);
  1455		if (((priv->flags & SAFEXCEL_HW_EIP197) &&
  1456		     (EIP197_REG_LO16(version) != EIP197_VERSION_LE) &&
  1457		     (EIP197_REG_LO16(version) != EIP196_VERSION_LE)) ||
  1458		    ((!(priv->flags & SAFEXCEL_HW_EIP197) &&
  1459		     (EIP197_REG_LO16(version) != EIP97_VERSION_LE)))) {
  1460			/*
  1461			 * We did not find the device that matched our initial probing
  1462			 * (or our initial probing failed) Report appropriate error.
  1463			 */
  1464			dev_err(priv->dev, "Probing for EIP97/EIP19x failed - no such device (read %08x)\n",
  1465				version);
  1466			return -ENODEV;
  1467		}
  1468	
  1469		priv->hwconfig.hwver = EIP197_VERSION_MASK(version);
  1470		hwctg = version >> 28;
  1471		peid = version & 255;
  1472	
  1473		/* Detect EIP206 processing pipe */
  1474		version = readl(EIP197_PE(priv) + + EIP197_PE_VERSION(0));
  1475		if (EIP197_REG_LO16(version) != EIP206_VERSION_LE) {
  1476			dev_err(priv->dev, "EIP%d: EIP206 not detected\n", peid);
  1477			return -ENODEV;
  1478		}
  1479		priv->hwconfig.ppver = EIP197_VERSION_MASK(version);
  1480	
  1481		/* Detect EIP96 packet engine and version */
  1482		version = readl(EIP197_PE(priv) + EIP197_PE_EIP96_VERSION(0));
  1483		if (EIP197_REG_LO16(version) != EIP96_VERSION_LE) {
  1484			dev_err(dev, "EIP%d: EIP96 not detected.\n", peid);
  1485			return -ENODEV;
  1486		}
  1487		priv->hwconfig.pever = EIP197_VERSION_MASK(version);
  1488	
  1489		hwopt = readl(EIP197_GLOBAL(priv) + EIP197_OPTIONS);
  1490		hiaopt = readl(EIP197_HIA_AIC(priv) + EIP197_HIA_OPTIONS);
  1491	
  1492		priv->hwconfig.icever = 0;
  1493		priv->hwconfig.ocever = 0;
  1494		priv->hwconfig.psever = 0;
  1495		if (priv->flags & SAFEXCEL_HW_EIP197) {
  1496			/* EIP197 */
  1497			peopt = readl(EIP197_PE(priv) + EIP197_PE_OPTIONS(0));
  1498	
  1499			priv->hwconfig.hwdataw  = (hiaopt >> EIP197_HWDATAW_OFFSET) &
  1500						  EIP197_HWDATAW_MASK;
  1501			priv->hwconfig.hwcfsize = ((hiaopt >> EIP197_CFSIZE_OFFSET) &
  1502						   EIP197_CFSIZE_MASK) +
  1503						  EIP197_CFSIZE_ADJUST;
  1504			priv->hwconfig.hwrfsize = ((hiaopt >> EIP197_RFSIZE_OFFSET) &
  1505						   EIP197_RFSIZE_MASK) +
  1506						  EIP197_RFSIZE_ADJUST;
  1507			priv->hwconfig.hwnumpes	= (hiaopt >> EIP197_N_PES_OFFSET) &
  1508						  EIP197_N_PES_MASK;
  1509			priv->hwconfig.hwnumrings = (hiaopt >> EIP197_N_RINGS_OFFSET) &
  1510						    EIP197_N_RINGS_MASK;
  1511			if (hiaopt & EIP197_HIA_OPT_HAS_PE_ARB)
  1512				priv->flags |= EIP197_PE_ARB;
  1513			if (EIP206_OPT_ICE_TYPE(peopt) == 1) {
  1514				priv->flags |= EIP197_ICE;
  1515				/* Detect ICE EIP207 class. engine and version */
  1516				version = readl(EIP197_PE(priv) +
  1517					  EIP197_PE_ICE_VERSION(0));
  1518				if (EIP197_REG_LO16(version) != EIP207_VERSION_LE) {
  1519					dev_err(dev, "EIP%d: ICE EIP207 not detected.\n",
  1520						peid);
  1521					return -ENODEV;
  1522				}
  1523				priv->hwconfig.icever = EIP197_VERSION_MASK(version);
  1524			}
  1525			if (EIP206_OPT_OCE_TYPE(peopt) == 1) {
  1526				priv->flags |= EIP197_OCE;
  1527				/* Detect EIP96PP packet stream editor and version */
  1528				version = readl(EIP197_PE(priv) + EIP197_PE_PSE_VERSION(0));
  1529				if (EIP197_REG_LO16(version) != EIP96_VERSION_LE) {
  1530					dev_err(dev, "EIP%d: EIP96PP not detected.\n", peid);
  1531					return -ENODEV;
  1532				}
  1533				priv->hwconfig.psever = EIP197_VERSION_MASK(version);
  1534				/* Detect OCE EIP207 class. engine and version */
  1535				version = readl(EIP197_PE(priv) +
  1536					  EIP197_PE_ICE_VERSION(0));
  1537				if (EIP197_REG_LO16(version) != EIP207_VERSION_LE) {
  1538					dev_err(dev, "EIP%d: OCE EIP207 not detected.\n",
  1539						peid);
  1540					return -ENODEV;
  1541				}
  1542				priv->hwconfig.ocever = EIP197_VERSION_MASK(version);
  1543			}
  1544			/* If not a full TRC, then assume simple TRC */
  1545			if (!(hwopt & EIP197_OPT_HAS_TRC))
  1546				priv->flags |= EIP197_SIMPLE_TRC;
  1547			/* EIP197 always has SOME form of TRC */
  1548			priv->flags |= EIP197_TRC_CACHE;
  1549		} else {
  1550			/* EIP97 */
  1551			priv->hwconfig.hwdataw  = (hiaopt >> EIP197_HWDATAW_OFFSET) &
  1552						  EIP97_HWDATAW_MASK;
  1553			priv->hwconfig.hwcfsize = (hiaopt >> EIP97_CFSIZE_OFFSET) &
  1554						  EIP97_CFSIZE_MASK;
  1555			priv->hwconfig.hwrfsize = (hiaopt >> EIP97_RFSIZE_OFFSET) &
  1556						  EIP97_RFSIZE_MASK;
  1557			priv->hwconfig.hwnumpes	= 1; /* by definition */
  1558			priv->hwconfig.hwnumrings = (hiaopt >> EIP197_N_RINGS_OFFSET) &
  1559						    EIP197_N_RINGS_MASK;
  1560		}
  1561	
  1562		/* Scan for ring AIC's */
  1563		for (i = 0; i < EIP197_MAX_RING_AIC; i++) {
  1564			version = readl(EIP197_HIA_AIC_R(priv) +
  1565					EIP197_HIA_AIC_R_VERSION(i));
  1566			if (EIP197_REG_LO16(version) != EIP201_VERSION_LE)
  1567				break;
  1568		}
  1569		priv->hwconfig.hwnumraic = i;
  1570		/* Low-end EIP196 may not have any ring AIC's ... */
  1571		if (!priv->hwconfig.hwnumraic) {
  1572			dev_err(priv->dev, "No ring interrupt controller present!\n");
  1573			return -ENODEV;
  1574		}
  1575	
  1576		/* Get supported algorithms from EIP96 transform engine */
  1577		priv->hwconfig.algo_flags = readl(EIP197_PE(priv) +
  1578					    EIP197_PE_EIP96_OPTIONS(0));
  1579	
  1580		/* Print single info line describing what we just detected */
  1581		dev_info(priv->dev, "EIP%d:%x(%d,%d,%d,%d)-HIA:%x(%d,%d,%d),PE:%x/%x(alg:%08x)/%x/%x/%x\n",
  1582			 peid, priv->hwconfig.hwver, hwctg, priv->hwconfig.hwnumpes,
  1583			 priv->hwconfig.hwnumrings, priv->hwconfig.hwnumraic,
  1584			 priv->hwconfig.hiaver, priv->hwconfig.hwdataw,
  1585			 priv->hwconfig.hwcfsize, priv->hwconfig.hwrfsize,
  1586			 priv->hwconfig.ppver, priv->hwconfig.pever,
  1587			 priv->hwconfig.algo_flags, priv->hwconfig.icever,
  1588			 priv->hwconfig.ocever, priv->hwconfig.psever);
  1589	
  1590		safexcel_configure(priv);
  1591	
  1592		if (IS_ENABLED(CONFIG_PCI) && priv->data->version == EIP197_DEVBRD) {
  1593			/*
  1594			 * Request MSI vectors for global + 1 per ring -
  1595			 * or just 1 for older dev images
  1596			 */
  1597			struct pci_dev *pci_pdev = pdev;
  1598	
> 1599			ret = pcim_alloc_irq_vectors(pci_pdev,
  1600						     priv->config.rings + 1,
  1601						     priv->config.rings + 1,
  1602						     PCI_IRQ_MSI | PCI_IRQ_MSIX);
  1603			if (ret < 0) {
  1604				dev_err(dev, "Failed to allocate PCI MSI interrupts\n");
  1605				return ret;
  1606			}
  1607		}
  1608	
  1609		/* Register the ring IRQ handlers and configure the rings */
  1610		priv->ring = devm_kcalloc(dev, priv->config.rings,
  1611					  sizeof(*priv->ring),
  1612					  GFP_KERNEL);
  1613		if (!priv->ring)
  1614			return -ENOMEM;
  1615	
  1616		for (i = 0; i < priv->config.rings; i++) {
  1617			char wq_name[9] = {0};
  1618			int irq;
  1619			struct safexcel_ring_irq_data *ring_irq;
  1620	
  1621			ret = safexcel_init_ring_descriptors(priv,
  1622							     &priv->ring[i].cdr,
  1623							     &priv->ring[i].rdr);
  1624			if (ret) {
  1625				dev_err(dev, "Failed to initialize rings\n");
  1626				goto err_cleanup_rings;
  1627			}
  1628	
  1629			priv->ring[i].rdr_req = devm_kcalloc(dev,
  1630				EIP197_DEFAULT_RING_SIZE,
  1631				sizeof(*priv->ring[i].rdr_req),
  1632				GFP_KERNEL);
  1633			if (!priv->ring[i].rdr_req) {
  1634				ret = -ENOMEM;
  1635				goto err_cleanup_rings;
  1636			}
  1637	
  1638			ring_irq = devm_kzalloc(dev, sizeof(*ring_irq), GFP_KERNEL);
  1639			if (!ring_irq) {
  1640				ret = -ENOMEM;
  1641				goto err_cleanup_rings;
  1642			}
  1643	
  1644			ring_irq->priv = priv;
  1645			ring_irq->ring = i;
  1646	
  1647			irq = safexcel_request_ring_irq(pdev,
  1648							EIP197_IRQ_NUMBER(i, is_pci_dev),
  1649							is_pci_dev,
  1650							i,
  1651							safexcel_irq_ring,
  1652							safexcel_irq_ring_thread,
  1653							ring_irq);
  1654			if (irq < 0) {
  1655				dev_err(dev, "Failed to get IRQ ID for ring %d\n", i);
  1656				ret = irq;
  1657				goto err_cleanup_rings;
  1658			}
  1659	
  1660			priv->ring[i].irq = irq;
  1661			priv->ring[i].work_data.priv = priv;
  1662			priv->ring[i].work_data.ring = i;
  1663			INIT_WORK(&priv->ring[i].work_data.work,
  1664				  safexcel_dequeue_work);
  1665	
  1666			snprintf(wq_name, 9, "wq_ring%d", i);
  1667			priv->ring[i].workqueue =
  1668				create_singlethread_workqueue(wq_name);
  1669			if (!priv->ring[i].workqueue) {
  1670				ret = -ENOMEM;
  1671				goto err_cleanup_rings;
  1672			}
  1673	
  1674			priv->ring[i].requests = 0;
  1675			priv->ring[i].busy = false;
  1676	
  1677			crypto_init_queue(&priv->ring[i].queue,
  1678					  EIP197_DEFAULT_RING_SIZE);
  1679	
  1680			spin_lock_init(&priv->ring[i].lock);
  1681			spin_lock_init(&priv->ring[i].queue_lock);
  1682		}
  1683	
  1684		atomic_set(&priv->ring_used, 0);
  1685	
  1686		ret = safexcel_hw_init(priv);
  1687		if (ret) {
  1688			dev_err(dev, "HW init failed (%d)\n", ret);
  1689			goto err_cleanup_rings;
  1690		}
  1691	
  1692		ret = safexcel_register_algorithms(priv);
  1693		if (ret) {
  1694			dev_err(dev, "Failed to register algorithms (%d)\n", ret);
  1695			goto err_cleanup_rings;
  1696		}
  1697	
  1698		return 0;
  1699	
  1700	err_cleanup_rings:
  1701		for (i = 0; i < priv->config.rings; i++) {
  1702			if (priv->ring[i].irq)
  1703				irq_set_affinity_hint(priv->ring[i].irq, NULL);
  1704			if (priv->ring[i].workqueue)
  1705				destroy_workqueue(priv->ring[i].workqueue);
  1706		}
  1707	
  1708		return ret;
  1709	}
  1710	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 01/37] PCI/MSI: Add Devres managed IRQ vectors allocation

[Jonathan Cameron]

On Mon, 23 Feb 2026 23:29:40 +0800
Shawn Lin <shawn.lin@rock-chips.com> wrote:

> pcim_alloc_irq_vectors() and pcim_alloc_irq_vectors_affinity() are created for
> pci device drivers which rely on the devres machinery to help cleanup the IRQ
> vectors.

It might be worth adding some details on why we need the is_msi_managed
flag in the first place vs just doing conventional devm_add_action_or_reset()
with pci_free_irq_vectors().