Skip to content

Commit 390ca88

Browse files
author
Alex Williamson
committed
PCI: Fix active state requirement in PME polling
JIRA: https://issues.redhat.com/browse/RHEL-25125 Upstream Status: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/log/?h=for-linus commit 41044d5 Author: Alex Williamson <alex.williamson@redhat.com> Date: Tue Jan 23 11:55:31 2024 -0700 PCI: Fix active state requirement in PME polling The commit noted in fixes added a bogus requirement that runtime PM managed devices need to be in the RPM_ACTIVE state for PME polling. In fact, only devices in low power states should be polled. However there's still a requirement that the device config space must be accessible, which has implications for both the current state of the polled device and the parent bridge, when present. It's not sufficient to assume the bridge remains in D0 and cases have been observed where the bridge passes the D0 test, but the PM state indicates RPM_SUSPENDING and config space of the polled device becomes inaccessible during pci_pme_wakeup(). Therefore, since the bridge is already effectively required to be in the RPM_ACTIVE state, formalize this in the code and elevate the PM usage count to maintain the state while polling the subordinate device. This resolves a regression reported in the bugzilla below where a Thunderbolt/USB4 hierarchy fails to scan for an attached NVMe endpoint downstream of a bridge in a D3hot power state. Link: https://lore.kernel.org/r/20240123185548.1040096-1-alex.williamson@redhat.com Fixes: d3fcd73 ("PCI: Fix runtime PM race with PME polling") Reported-by: Sanath S <sanath.s@amd.com> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218360 Signed-off-by: Alex Williamson <alex.williamson@redhat.com> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com> Tested-by: Sanath S <sanath.s@amd.com> Reviewed-by: Rafael J. Wysocki <rafael@kernel.org> Cc: Lukas Wunner <lukas@wunner.de> Cc: Mika Westerberg <mika.westerberg@linux.intel.com> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
1 parent 665af06 commit 390ca88

File tree

1 file changed

+22
-15
lines changed

1 file changed

+22
-15
lines changed

drivers/pci/pci.c

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2425,29 +2425,36 @@ static void pci_pme_list_scan(struct work_struct *work)
24252425
if (pdev->pme_poll) {
24262426
struct pci_dev *bridge = pdev->bus->self;
24272427
struct device *dev = &pdev->dev;
2428-
int pm_status;
2428+
struct device *bdev = bridge ? &bridge->dev : NULL;
2429+
int bref = 0;
24292430

24302431
/*
2431-
* If bridge is in low power state, the
2432-
* configuration space of subordinate devices
2433-
* may be not accessible
2432+
* If we have a bridge, it should be in an active/D0
2433+
* state or the configuration space of subordinate
2434+
* devices may not be accessible or stable over the
2435+
* course of the call.
24342436
*/
2435-
if (bridge && bridge->current_state != PCI_D0)
2436-
continue;
2437+
if (bdev) {
2438+
bref = pm_runtime_get_if_active(bdev, true);
2439+
if (!bref)
2440+
continue;
2441+
2442+
if (bridge->current_state != PCI_D0)
2443+
goto put_bridge;
2444+
}
24372445

24382446
/*
2439-
* If the device is in a low power state it
2440-
* should not be polled either.
2447+
* The device itself should be suspended but config
2448+
* space must be accessible, therefore it cannot be in
2449+
* D3cold.
24412450
*/
2442-
pm_status = pm_runtime_get_if_active(dev, true);
2443-
if (!pm_status)
2444-
continue;
2445-
2446-
if (pdev->current_state != PCI_D3cold)
2451+
if (pm_runtime_suspended(dev) &&
2452+
pdev->current_state != PCI_D3cold)
24472453
pci_pme_wakeup(pdev, NULL);
24482454

2449-
if (pm_status > 0)
2450-
pm_runtime_put(dev);
2455+
put_bridge:
2456+
if (bref > 0)
2457+
pm_runtime_put(bdev);
24512458
} else {
24522459
list_del(&pme_dev->list);
24532460
kfree(pme_dev);

0 commit comments

Comments
 (0)