[+cc Christoph]
On Thu, Jul 25, 2024 at 02:07:30PM +0200, Philipp Stanner wrote:
> pci_intx() is a function that becomes managed if pcim_enable_device()
> has been called in advance. Commit 25216afc9db5 ("PCI: Add managed
> pcim_intx()") changed this behavior so that pci_intx() always leads to
> creation of a separate device resource for itself, whereas earlier, a
> shared resource was used for all PCI devres operations.
>
> Unfortunately, pci_intx() seems to be used in some drivers' remove()
> paths; in the managed case this causes a device resource to be created
> on driver detach.
>
> Fix the regression by only redirecting pci_intx() to its managed twin
> pcim_intx() if the pci_command changes.
>
> Fixes: 25216afc9db5 ("PCI: Add managed pcim_intx()")
> Reported-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
> Closes: https://lore.kernel.org/all/b8f4ba97-84fc-4b7e-ba1a-99de2d9f0118@xxxxxxxxxx/
> Signed-off-by: Philipp Stanner <pstanner@xxxxxxxxxx>
Applied to for-linus for v6.11, thanks!
> ---
> Alright, I reproduced this with QEMU as Damien described and this here
> fixes the issue on my side. Feedback welcome. Thank you very much,
> Damien.
>
> It seems that this might yet again be the issue of drivers not being
> aware that pci_intx() might become managed, so they use it in their
> unwind path (rightfully so; there probably was no alternative back
> then).
>
> That will make the long term cleanup difficult. But I think this for now
> is the most elegant possible workaround.
> ---
> drivers/pci/pci.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e3a49f66982d..ffaaca0978cb 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4477,12 +4477,6 @@ void pci_intx(struct pci_dev *pdev, int enable)
> {
> u16 pci_command, new;
>
> - /* Preserve the "hybrid" behavior for backwards compatibility */
> - if (pci_is_managed(pdev)) {
> - WARN_ON_ONCE(pcim_intx(pdev, enable) != 0);
> - return;
> - }
> -
> pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
>
> if (enable)
> @@ -4490,8 +4484,15 @@ void pci_intx(struct pci_dev *pdev, int enable)
> else
> new = pci_command | PCI_COMMAND_INTX_DISABLE;
>
> - if (new != pci_command)
> + if (new != pci_command) {
> + /* Preserve the "hybrid" behavior for backwards compatibility */
> + if (pci_is_managed(pdev)) {
> + WARN_ON_ONCE(pcim_intx(pdev, enable) != 0);
> + return;
> + }
> +
> pci_write_config_word(pdev, PCI_COMMAND, new);
> + }
> }
> EXPORT_SYMBOL_GPL(pci_intx);
>
> --
> 2.45.2
>
