Hello Philipp, I have reviewed and tested this patch, looks to be working fine and fixes the issue. Thanks, Ashish On 7/9/2024 3:56 AM, Philipp Stanner wrote: > From c24bd5b66e798a341caf183fb7cdbdf235502d90 Mon Sep 17 00:00:00 2001 > From: Philipp Stanner <pstanner@xxxxxxxxxx> > Date: Tue, 9 Jul 2024 09:45:48 +0200 > Subject: [PATCH] PCI: Fix pcim_intx() recursive calls > > pci_intx() calls into pcim_intx() in managed mode, i.e., when > pcim_enable_device() had been called. This recursive call causes a bug > by re-registering the device resource in the release callback. > > This is the same phenomenon that made it necessary to implement some > functionality a second time, see __pcim_request_region(). > > Implement __pcim_intx() to bypass the hybrid nature of pci_intx() on > driver detach. > > Fixes: https://lore.kernel.org/all/20240708214656.4721-1-Ashish.Kalra@xxxxxxx/ > Reported-by: Ashish Kalra <Ashish.Kalra@xxxxxxx> > Signed-off-by: Philipp Stanner <pstanner@xxxxxxxxxx> > --- > Hi Ashish, > I hacked down this fix that should be applyable on top. > Could you maybe have a first quick look whether this fixes the issue? > --- > drivers/pci/devres.c | 33 +++++++++++++++++++++------------ > 1 file changed, 21 insertions(+), 12 deletions(-) > > diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c > index 2f0379a4e58f..dcef049b72fe 100644 > --- a/drivers/pci/devres.c > +++ b/drivers/pci/devres.c > @@ -408,12 +408,31 @@ static inline bool mask_contains_bar(int mask, int bar) > return mask & BIT(bar); > } > > +/* > + * This is a copy of pci_intx() used to bypass the problem of occuring > + * recursive function calls due to the hybrid nature of pci_intx(). > + */ > +static void __pcim_intx(struct pci_dev *pdev, int enable) > +{ > + u16 pci_command, new; > + > + pci_read_config_word(pdev, PCI_COMMAND, &pci_command); > + > + if (enable) > + new = pci_command & ~PCI_COMMAND_INTX_DISABLE; > + else > + new = pci_command | PCI_COMMAND_INTX_DISABLE; > + > + if (new != pci_command) > + pci_write_config_word(pdev, PCI_COMMAND, new); > +} > + > static void pcim_intx_restore(struct device *dev, void *data) > { > struct pci_dev *pdev = to_pci_dev(dev); > struct pcim_intx_devres *res = data; > > - pci_intx(pdev, res->orig_intx); > + __pcim_intx(pdev, res->orig_intx); > } > > static struct pcim_intx_devres *get_or_create_intx_devres(struct device *dev) > @@ -443,7 +462,6 @@ static struct pcim_intx_devres *get_or_create_intx_devres(struct device *dev) > */ > int pcim_intx(struct pci_dev *pdev, int enable) > { > - u16 pci_command, new; > struct pcim_intx_devres *res; > > res = get_or_create_intx_devres(&pdev->dev); > @@ -451,16 +469,7 @@ int pcim_intx(struct pci_dev *pdev, int enable) > return -ENOMEM; > > res->orig_intx = !enable; > - > - pci_read_config_word(pdev, PCI_COMMAND, &pci_command); > - > - if (enable) > - new = pci_command & ~PCI_COMMAND_INTX_DISABLE; > - else > - new = pci_command | PCI_COMMAND_INTX_DISABLE; > - > - if (new != pci_command) > - pci_write_config_word(pdev, PCI_COMMAND, new); > + __pcim_intx(pdev, enable); > > return 0; > } Tested-by: Ashish Kalra <Ashish.Kalra@xxxxxxx>
