On 7/24/24 1:56 PM, Damien Le Moal wrote:
>
> Commit 25216afc9db5 ("PCI: Add managed pcim_intx()") is causing a regression,
> which is easy to see using qemu with an AHCI device and the ahci driver
> compiled as a module.
>
> 1) Boot qemu: the AHCI controller is initialized and the drive(s) attached to
> it visible.
> 2) Run "rmmod ahci": the drives go away, all normal
> 3) Re-initialize the AHCI adapter and rescan the drives by running "modprobe
> ahci". That fails with the message "pci 0000:00:1f.2: Resources present before
> probing"
>
> The reason is that before commit 25216afc9db5, pci_intx(dev, 0) was called to
> disable INTX as MSI are used for the adapter, and for that case, pci_intx()
> would NOT allocate a device resource if the INTX enable/disable state was not
> being changed:
>
> if (new != pci_command) {
> struct pci_devres *dr;
>
> pci_write_config_word(pdev, PCI_COMMAND, new);
>
> dr = find_pci_dr(pdev);
> if (dr && !dr->restore_intx) {
> dr->restore_intx = 1;
> dr->orig_intx = !enable;
> }
> }
>
> The former code was only looking for the resource and not allocating it.
>
> Now, with pcim_intx() being used, the intx resource is *always* allocated,
> including when INTX is disabled when the device is being disabled on rmmod.
> This leads to the device resource list to always have the intx resource
> remaining and thus causes the modprobe error.
>
> Reverting Commit 25216afc9db5 is one solution to fix this, and I can send a
> patch for that, unless someone has an idea how to fix this ? I tried but I do
> not see a clean way of fixing this...
> Thoughts ?
This change works as a fix, but it is not pretty...
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 3780a9f9ec00..4e14f87e3d22 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -466,13 +466,22 @@ static struct pcim_intx_devres *get_or_create_intx_devres(struct device *dev)
int pcim_intx(struct pci_dev *pdev, int enable)
{
struct pcim_intx_devres *res;
+ u16 pci_command, new;
- res = get_or_create_intx_devres(&pdev->dev);
- if (!res)
- return -ENOMEM;
+ 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) {
+ res = get_or_create_intx_devres(&pdev->dev);
+ if (!res)
+ return -ENOMEM;
- res->orig_intx = !enable;
- __pcim_intx(pdev, enable);
+ res->orig_intx = !enable;
+ pci_write_config_word(pdev, PCI_COMMAND, new);
+ }
return 0;
}
--
Damien Le Moal
Western Digital Research
