From 6cff20ce3b92ffbf2fc5eb9e5a030b3672aa414a Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Sun, 13 Jul 2025 16:31:01 +0200 Subject: PCI/ACPI: Fix runtime PM ref imbalance on Hot-Plug Capable ports pci_bridge_d3_possible() is called from both pcie_portdrv_probe() and pcie_portdrv_remove() to determine whether runtime power management shall be enabled (on probe) or disabled (on remove) on a PCIe port. The underlying assumption is that pci_bridge_d3_possible() always returns the same value, else a runtime PM reference imbalance would occur. That assumption is not given if the PCIe port is inaccessible on remove due to hot-unplug: pci_bridge_d3_possible() calls pciehp_is_native(), which accesses Config Space to determine whether the port is Hot-Plug Capable. An inaccessible port returns "all ones", which is converted to "all zeroes" by pcie_capability_read_dword(). Hence the port no longer seems Hot-Plug Capable on remove even though it was on probe. The resulting runtime PM ref imbalance causes warning messages such as: pcieport 0000:02:04.0: Runtime PM usage count underflow! Avoid the Config Space access (and thus the runtime PM ref imbalance) by caching the Hot-Plug Capable bit in struct pci_dev. The struct already contains an "is_hotplug_bridge" flag, which however is not only set on Hot-Plug Capable PCIe ports, but also Conventional PCI Hot-Plug bridges and ACPI slots. The flag identifies bridges which are allocated additional MMIO and bus number resources to allow for hierarchy expansion. The kernel is somewhat sloppily using "is_hotplug_bridge" in a number of places to identify Hot-Plug Capable PCIe ports, even though the flag encompasses other devices. Subsequent commits replace these occurrences with the new flag to clearly delineate Hot-Plug Capable PCIe ports from other kinds of hotplug bridges. Document the existing "is_hotplug_bridge" and the new "is_pciehp" flag and document the (non-obvious) requirement that pci_bridge_d3_possible() always returns the same value across the entire lifetime of a bridge, including its hot-removal. Fixes: 5352a44a561d ("PCI: pciehp: Make pciehp_is_native() stricter") Reported-by: Laurent Bigonville Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220216 Reported-by: Mario Limonciello Closes: https://lore.kernel.org/r/20250609020223.269407-3-superm1@kernel.org/ Link: https://lore.kernel.org/all/20250620025535.3425049-3-superm1@kernel.org/T/#u Signed-off-by: Lukas Wunner Signed-off-by: Bjorn Helgaas Acked-by: Rafael J. Wysocki Cc: stable@vger.kernel.org # v4.18+ Link: https://patch.msgid.link/fe5dcc3b2e62ee1df7905d746bde161eb1b3291c.1752390101.git.lukas@wunner.de --- include/linux/pci.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'include/linux') diff --git a/include/linux/pci.h b/include/linux/pci.h index 05e68f35f392..d56d0dd80afb 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -328,6 +328,11 @@ struct rcec_ea; * determined (e.g., for Root Complex Integrated * Endpoints without the relevant Capability * Registers). + * @is_hotplug_bridge: Hotplug bridge of any kind (e.g. PCIe Hot-Plug Capable, + * Conventional PCI Hot-Plug, ACPI slot). + * Such bridges are allocated additional MMIO and bus + * number resources to allow for hierarchy expansion. + * @is_pciehp: PCIe Hot-Plug Capable bridge. */ struct pci_dev { struct list_head bus_list; /* Node in per-bus list */ @@ -451,6 +456,7 @@ struct pci_dev { unsigned int is_physfn:1; unsigned int is_virtfn:1; unsigned int is_hotplug_bridge:1; + unsigned int is_pciehp:1; unsigned int shpc_managed:1; /* SHPC owned by shpchp */ unsigned int is_thunderbolt:1; /* Thunderbolt controller */ /* -- cgit v1.2.3 From c2f9de5e2db29158a8caa86a37aa479488e4ba43 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Sun, 13 Jul 2025 16:31:04 +0200 Subject: PCI: Move is_pciehp check out of pciehp_is_native() pci_bridge_d3_possible() seeks to forbid runtime power management on: * Non Hot-Plug Capable PCIe ports which are nevertheless ACPI slots (recognizable as: bridge->is_hotplug_bridge && !bridge->is_pciehp) * Hot-Plug Capable PCIe ports for which platform firmware has not granted PCIe Native Hot-Plug control to the operating system (recognizable as: bridge->is_pciehp && !pciehp_is_native(bridge)) Somewhat confusingly, the check for is_hotplug_bridge is in pci_bridge_d3_possible(), whereas the one for is_pciehp is in pciehp_is_native(). For clarity, check is_pciehp directly in pci_bridge_d3_possible() (and in the other caller of pciehp_is_native(), hotplug_is_native()). Rephrase the code comment preceding these checks to no longer mention "System Management Mode", which is an x86 term inappropriate in generic PCI code. Likewise no longer mention "Thunderbolt on non-Macs", because there is nothing Thunderbolt-specific about these checks. It used to be the case that non-Macs relied on the platform for Thunderbolt tunnel management and hotplug, but they've since moved to OS-native tunnel management (as Macs always have), hence the code comment is no longer accurate. There is a subsequent check for is_hotplug_bridge further down in pci_bridge_d3_possible(). Change the check to is_pciehp because any ports matching "bridge->is_hotplug_bridge && !bridge->is_pciehp" are already filtered out at the top of the function. Do the same for another check in acpi_pci_bridge_d3(), which is called from pci_bridge_d3_possible() via platform_pci_bridge_d3(). No functional change intended. Signed-off-by: Lukas Wunner Signed-off-by: Bjorn Helgaas Link: https://patch.msgid.link/18b2c2110ad0f27a34b189d793310b9c4f2f24a0.1752390102.git.lukas@wunner.de --- include/linux/pci_hotplug.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'include/linux') diff --git a/include/linux/pci_hotplug.h b/include/linux/pci_hotplug.h index ec77ccf1fc4d..ddf79641917f 100644 --- a/include/linux/pci_hotplug.h +++ b/include/linux/pci_hotplug.h @@ -104,6 +104,7 @@ static inline bool shpchp_is_native(struct pci_dev *bridge) { return true; } static inline bool hotplug_is_native(struct pci_dev *bridge) { - return pciehp_is_native(bridge) || shpchp_is_native(bridge); + return (bridge->is_pciehp && pciehp_is_native(bridge)) || + shpchp_is_native(bridge); } #endif -- cgit v1.2.3