[{"id":3681995,"web_url":"http://patchwork.ozlabs.org/comment/3681995/","msgid":"","list_archive_url":null,"date":"2026-04-24T11:48:20","subject":"Re: [PATCH v4 0/7] Add Devres managed IRQ vectors allocation","submitter":{"id":90407,"url":"http://patchwork.ozlabs.org/api/people/90407/","name":"Philipp Stanner","email":"phasta@mailbox.org"},"content":"Hi Shawn,\n\nOn Fri, 2026-04-24 at 15:57 +0800, Shawn Lin wrote:\n> \n> There is a long-standing design ambiguity in the PCI/MSI subsystem regarding\n> the management of IRQ vectors. Currently, the management operates in a \"hybrid\"\n> mode. Sometimes it's handled by devres but sometimes not, creating potential\n> for resource management bugs.\n> \n> Historically, pcim_enable_device() implicitly triggers automatic IRQ vector\n> management when calling pci_alloc_irq_vectors[_affinity], as pcim_enable_device()\n> sets the is_managed flag, thus pcim_msi_release() will register a cleanup action.\n> However, using pci_enable_device() will not make pci_alloc_irq_vectors[_affinity]\n> register a cleanup action.\n> \n> This creates an ambiguous ownership model. Many drivers follow a pattern of:\n> 1. Calling pci_alloc_irq_vectors() to allocate interrupts.\n> 2. Also calling pci_free_irq_vectors() in their error paths or remove routines.\n> \n> When such a driver also uses pcim_enable_device(), the devres framework may\n> attempt to free the IRQ vectors a second time upon device release, leading to\n> a double-free. Analysis of the tree shows this hazardous pattern exists widely,\n> while 37 other drivers correctly rely solely on the implicit cleanup.\n> \n> To identify the scope of this issue, I used the following shell commands to\n> search for drivers using pcim_enable_device and allocation functions but missing\n> the explicit free call:\n> \n> grep -rl \"pcim_enable_device\" drivers/ \\\n>   | xargs grep -l -E \"pci_alloc_irq_vectors|pci_alloc_irq_vectors_affinity\" 2>/dev/null \\\n>   | xargs grep -L \"pci_free_irq_vectors\" 2>/dev/null\n> \n> drivers/dma/hsu/pci.c\n> drivers/dma/hisi_dma.c\n> drivers/hid/intel-thc-hidintel-quicki2c/pci-quicki2c.\n> drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c\n> drivers/hid/intel-ish-hid/ipc/pci-ish.c\n> drivers/accel/ivpu/ivpu_drv.c\n> drivers/accel/qaic/qaic_drv.c\n> drivers/accel/amdxdna/aie2_pci.c\n> drivers/platform/x86/intel/ehl_pse_io.c\n> drivers/media/pci/intel/ipu6/ipu6.c\n> drivers/mfd/intel-lpss-pci.c\n> drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c\n> drivers/crypto/marvell/octeontx2/otx2_cptpf_main.c\n> drivers/crypto/marvell/octeontx2/otx2_cptvf_main.c\n> drivers/crypto/inside-secure/safexcel.c\n> drivers/thunderbolt/nhi.c\n> drivers/i2c/busses/i2c-mchp-pci1xxxx.c\n> drivers/i2c/busses/i2c-amd-mp2-pci.c\n> drivers/i2c/busses/i2c-thunderx-pcidrv.c\n> drivers/i2c/busses/i2c-designware-pcidrv.c\n> drivers/tty/serial/8250/8250_exar.c\n> drivers/tty/serial/8250/8250_pci.c\n> drivers/tty/serial/8250/8250_mid.c\n> drivers/gpio/gpio-merrifield.c\n> drivers/iommu/riscv/iommu-pci.c\n> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c\n> drivers/pci/switch/switchtec.c\n> drivers/pci/controller/vmd.c\n> drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c\n> drivers/net/ethernet/hisilicon/hibmcge/hbg_irq.c\n> drivers/net/ethernet/cavium/thunder/thunder_bgx.c\n> drivers/net/ethernet/realtek/r8169_main.c\n> drivers/mmc/host/cavium-thunderx.c\n> drivers/misc/mchp_pci1xxxx/mchp_pci1xxxx_gp.c\n> drivers/cxl/pci.c\n> drivers/spi/spi-pxa2xx-pci.c\n> drivers/spi/spi-pci1xxxx.c\n> \n> To count how many drivers, including using lagacy APIs, have the double-free bugs, the\n> following shell commands find out 60 drivers.\n> grep -rl \"pcim_enable_device\" drivers/ --include=\"*.c\" \\\n>   | xargs grep -l -E \"pci_alloc_irq_vectors|pci_alloc_irq_vectors_affinity\" 2>/dev/null \\\n>   | xargs grep -l \"pci_free_irq_vectors\" 2>/dev/null \\\n>   | tee /dev/tty \\\n>   | wc -l\n> \n> drivers/dma/dw-edma/dw-edma-pcie.c\n> drivers/dma/plx_dma.c\n> drivers/dma/amd/ae4dma/ae4dma-pci.c\n> drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c\n> drivers/perf/hisilicon/hisi_pcie_pmu.c\n> drivers/perf/hisilicon/hns3_pmu.c\n> drivers/platform/x86/intel_ips.c\n> ....\n> drivers/hwtracing/intel_th/pci.c\n> drivers/bus/mhi/host/pci_generic.c\n> drivers/fpga/dfl-pci.c\n> 51\n> \n> grep -rl \"pcim_enable_device\" drivers/ --include=\"*.c\" \\\n>   | xargs grep -l -E \"pci_enable_msi|pci_enable_msix\" 2>/dev/null \\\n>   | xargs grep -l \"pci_disable_msi\" 2>/dev/null \\\n>   | tee /dev/tty \\\n>   | wc -l\n> \n> drivers/dma/ioat/init.c\n> drivers/dma/amd/ptdma/ptdma-pci.c\n> drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci_legacy.c\n> drivers/crypto/ccp/sp-pci.c\n> drivers/i2c/busses/i2c-ismt.c\n> drivers/pci/msi/api.c\n> drivers/misc/mei/pci-me.c\n> drivers/misc/mei/pci-txe.c\n> drivers/block/mtip32xx/mtip32xx.c\n> 9\n> \n> This series introduces new managed APIs: pcim_alloc_irq_vectors() and\n> pcim_alloc_irq_vectors_affinity(). Drivers that wish to have devres-managed IRQ\n> vectors should use these functions to ensure proper automatic cleanup without\n> ambiguity.\n> \n> Regarding the removal strategy discussed in v3 [1], two approaches were considered:\n> a) Introducing another hybrid function temporarily.\n> b) Duplicating code temporarily.\n> \n> Following the discussion with Philipp, I agree that adding another hybrid function\n> does not guarantee the completion of the final cleanup work. Therefore, starting\n> from v4, I have chosen to duplicate the necessary code temporarily. Helper functions\n> have been factored out to minimize code duplication.\n> \n> In the short term, the series converts two drivers within the PCI subsystem to\n> use the new APIs. The long-term goal is to convert all other drivers which wish to\n> use these managed functions. For the legacy APIs, like pci_enable_msix_range_*() and\n> pci_enable_msi(), we won't consider to provide devres-managed version for them but to\n> fix the only 9 double-free drivers by using the new APIs provided by this series. And\n> once ready, we could remove the problematic hybrid management pattern from\n> pcim_setup_msi_release() entirely.\n> \n> [1]: https://lore.kernel.org/linux-pci/9dc66010d61670dc5182062ef5f1a932f7374323.camel@mailbox.org/T/#t\n> \n> \n> Changes in v4:\n> - Rework to create dedicated devres-managed function (Philipp)\n\nThanks for the rework.\n\nI skimmed through it and it very much looks spot-on, I don't see major\nconcerns.\n\nI'm a bit loaded right now. Can do a more detailed review earliest next\nweek, probably later.\n\n\nRegards\nP.\n\n> \n> Changes in v3:\n> - Rework the commit message and function doc (Philipp)\n> - Remove setting is_msi_managed flag from new APIs (Philipp)\n> \n> Changes in v2:\n> - Rebase\n> - Introduce patches only for PCI subsystem to convert the API\n> \n> Shawn Lin (7):\n>   PCI/MSI: Split __pci_enable_msi_range() for reuse\n>   PCI/MSI: Split __pci_enable_msix_range() for reuse\n>   PCI/MSI: Introduce __pcim_enable_msi_range()\n>   PCI/MSI: Introduce __pcim_enable_msix_range()\n>   PCI/MSI: Add Devres managed IRQ vectors allocation\n>   PCI: switchtec: Replace pci_alloc_irq_vectors() with\n>     pcim_alloc_irq_vectors()\n>   PCI: vmd: Replace pci_alloc_irq_vectors() with\n>     pcim_alloc_irq_vectors()\n> \n>  drivers/pci/controller/vmd.c   |   4 +-\n>  drivers/pci/msi/api.c          |  84 +++++++++++++++++++++++++++++++\n>  drivers/pci/msi/msi.c          | 110 +++++++++++++++++++++++++++++++++++------\n>  drivers/pci/msi/msi.h          |   3 ++\n>  drivers/pci/switch/switchtec.c |   6 +--\n>  include/linux/pci.h            |  22 +++++++++\n>  6 files changed, 210 insertions(+), 19 deletions(-)\n>","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-pci@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n secure) header.d=mailbox.org header.i=@mailbox.org header.a=rsa-sha256\n header.s=mail20150812 header.b=TPmoOkHu;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=linux-pci+bounces-53135-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=mailbox.org header.i=@mailbox.org\n header.b=\"TPmoOkHu\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=80.241.56.152","smtp.subspace.kernel.org;\n dmarc=pass (p=reject dis=none) header.from=mailbox.org","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=mailbox.org"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4g2B9N23cqz1yD5\n\tfor ; Fri, 24 Apr 2026 21:50:32 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id D6365300C93D\n\tfor ; Fri, 24 Apr 2026 11:48:36 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 1FCFF3BED2B;\n\tFri, 24 Apr 2026 11:48:36 +0000 (UTC)","from mout-p-102.mailbox.org (mout-p-102.mailbox.org [80.241.56.152])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 9DB861862\n\tfor ; Fri, 24 Apr 2026 11:48:33 +0000 (UTC)","from smtp202.mailbox.org (smtp202.mailbox.org\n [IPv6:2001:67c:2050:b231:465::202])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest\n SHA256)\n\t(No client certificate requested)\n\tby mout-p-102.mailbox.org (Postfix) with ESMTPS id 4g2B6v5Cdnz9vLN;\n\tFri, 24 Apr 2026 13:48:23 +0200 (CEST)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1777031316; cv=none;\n b=GEzX2oUPlDbM0gz6m2JwWogB1SdLV2SyJMmUtn4+ijz32fjgGYjOC6SEogQsJk/v0LKWqa/0sAZmymQ/hSnZc5OUjmJMKxOC3kdQBvUHZ6imcn/c5AIsqbr8YpDXmTwzl67rU8yx3kFaNpmi3IoKpGnVRHXkHzweqzblYGRZTEc=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1777031316; c=relaxed/simple;\n\tbh=G0OTqPwFHZrR2eXz3Qd/K/P3zXcGupz+/PbGhfnUtOw=;\n\th=Message-ID:Subject:From:To:Cc:Date:In-Reply-To:References:\n\t Content-Type:MIME-Version;\n b=IupsIW+0VvnzD2THjQiEmQJAWdF2abdSsj5ta/5aiupS6HcZlxDXHg00MYeBN4IsT+4TQU8rXKNiVRNZZwqGEKGsp5WdmZ59LsCHUJ1PiIzvxnzjBnVB/DqhPN4AyyuqYxIlnHYMKqIg5IYJ4ZlhY+SKsw9efDiGP7YUP5fRHQA=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=reject dis=none) header.from=mailbox.org;\n spf=pass smtp.mailfrom=mailbox.org;\n dkim=pass (2048-bit key) header.d=mailbox.org header.i=@mailbox.org\n header.b=TPmoOkHu; arc=none smtp.client-ip=80.241.56.152","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=mailbox.org;\n s=mail20150812;\n\tt=1777031303; h=from:from:reply-to:reply-to:subject:subject:date:date:\n\t message-id:message-id:to:to:cc:cc:mime-version:mime-version:\n\t content-type:content-type:\n\t content-transfer-encoding:content-transfer-encoding:\n\t in-reply-to:in-reply-to:references:references;\n\tbh=G0OTqPwFHZrR2eXz3Qd/K/P3zXcGupz+/PbGhfnUtOw=;\n\tb=TPmoOkHuLo8BjS4tXnspkwlN6s2QMoiznhg0yYFxPXOq81Xou5l605iDB1s/rHbAt3D5kM\n\tEfKsgEAm8Zq/MZROYb/QLrYBPw1vm2+SmxGtWiaiI4fH2mUXYLWIh2T/1ce4Bh9ZbE0wdN\n\tfBAm9ahqba1zupF5AjE5InfeMLByZcG7b8UT8xOHypOYqOpPBefDK1Df+ysEA2JhPIdvIc\n\tueWyEAhe+RyIyqiodyuBAVbdQ0aWGo2D6i2hd1H5iQq3GKFnqab2qKX1AlkFkchMwMEBlI\n\tIQVHyfSAwQ2O4vYaeGG7mLTr4d1MIhF9FFKXUaaJw+Vf7hnLYWaQsWgx/YPqWQ==","Message-ID":"","Subject":"Re: [PATCH v4 0/7] Add Devres managed IRQ vectors allocation","From":"Philipp Stanner ","Reply-To":"phasta@kernel.org","To":"Shawn Lin , Bjorn Helgaas ","Cc":"Nirmal Patel , Jonathan Derrick\n , Kurt Schwemmer\n , Logan Gunthorpe ,\n Philipp Stanner , linux-pci@vger.kernel.org","Date":"Fri, 24 Apr 2026 13:48:20 +0200","In-Reply-To":"<1777017460-243543-1-git-send-email-shawn.lin@rock-chips.com>","References":"<1777017460-243543-1-git-send-email-shawn.lin@rock-chips.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Precedence":"bulk","X-Mailing-List":"linux-pci@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","X-MBO-RS-ID":"ef197ca0ef727f5c6bd","X-MBO-RS-META":"we5axgudwz584nokq79qj7oqm7ttg9ow"}}]