Skip to content

Commit d555ed4

Browse files
tiwaibjorn-helgaas
authored andcommitted
PCI: Restore original INTX_DISABLE bit by pcim_intx()
pcim_intx() tries to restore the INTx bit at removal via devres, but there is a chance that it restores a wrong value. Because the value to be restored is blindly assumed to be the negative of the enable argument, when a driver calls pcim_intx() unnecessarily for the already enabled state, it'll restore to the disabled state in turn. That is, the function assumes the case like: // INTx == 1 pcim_intx(pdev, 0); // old INTx value assumed to be 1 -> correct but it might be like the following, too: // INTx == 0 pcim_intx(pdev, 0); // old INTx value assumed to be 1 -> wrong Also, when a driver calls pcim_intx() multiple times with different enable argument values, the last one will win no matter what value it is. This can lead to inconsistency, e.g. // INTx == 1 pcim_intx(pdev, 0); // OK ... pcim_intx(pdev, 1); // now old INTx wrongly assumed to be 0 This patch addresses those inconsistencies by saving the original INTx state at the first pcim_intx() call. For that, get_or_create_intx_devres() is folded into pcim_intx() caller side; it allows us to simply check the already allocated devres and record the original INTx along with the devres_alloc() call. Link: https://lore.kernel.org/r/20241031134300.10296-1-tiwai@suse.de Fixes: 25216af ("PCI: Add managed pcim_intx()") Link: https://lore.kernel.org/87v7xk2ps5.wl-tiwai@suse.de Signed-off-by: Takashi Iwai <tiwai@suse.de> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Reviewed-by: Philipp Stanner <pstanner@redhat.com> Cc: stable@vger.kernel.org # v6.11+
1 parent 647d696 commit d555ed4

File tree

1 file changed

+19
-15
lines changed

1 file changed

+19
-15
lines changed

drivers/pci/devres.c

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -419,19 +419,12 @@ static void pcim_intx_restore(struct device *dev, void *data)
419419
pci_intx(pdev, res->orig_intx);
420420
}
421421

422-
static struct pcim_intx_devres *get_or_create_intx_devres(struct device *dev)
422+
static void save_orig_intx(struct pci_dev *pdev, struct pcim_intx_devres *res)
423423
{
424-
struct pcim_intx_devres *res;
425-
426-
res = devres_find(dev, pcim_intx_restore, NULL, NULL);
427-
if (res)
428-
return res;
424+
u16 pci_command;
429425

430-
res = devres_alloc(pcim_intx_restore, sizeof(*res), GFP_KERNEL);
431-
if (res)
432-
devres_add(dev, res);
433-
434-
return res;
426+
pci_read_config_word(pdev, PCI_COMMAND, &pci_command);
427+
res->orig_intx = !(pci_command & PCI_COMMAND_INTX_DISABLE);
435428
}
436429

437430
/**
@@ -447,12 +440,23 @@ static struct pcim_intx_devres *get_or_create_intx_devres(struct device *dev)
447440
int pcim_intx(struct pci_dev *pdev, int enable)
448441
{
449442
struct pcim_intx_devres *res;
443+
struct device *dev = &pdev->dev;
450444

451-
res = get_or_create_intx_devres(&pdev->dev);
452-
if (!res)
453-
return -ENOMEM;
445+
/*
446+
* pcim_intx() must only restore the INTx value that existed before the
447+
* driver was loaded, i.e., before it called pcim_intx() for the
448+
* first time.
449+
*/
450+
res = devres_find(dev, pcim_intx_restore, NULL, NULL);
451+
if (!res) {
452+
res = devres_alloc(pcim_intx_restore, sizeof(*res), GFP_KERNEL);
453+
if (!res)
454+
return -ENOMEM;
455+
456+
save_orig_intx(pdev, res);
457+
devres_add(dev, res);
458+
}
454459

455-
res->orig_intx = !enable;
456460
pci_intx(pdev, enable);
457461

458462
return 0;

0 commit comments

Comments
 (0)