On 6/23/25 5:11 AM, Rafael J. Wysocki wrote:
On Mon, Jun 23, 2025 at 12:05 PM Lukas Wunner <lukas@xxxxxxxxx> wrote:
On Mon, Jun 23, 2025 at 09:37:07AM +0200, Lukas Wunner wrote:
On Mon, Jun 23, 2025 at 08:43:25AM +0200, Lukas Wunner wrote:
On Sun, Jun 22, 2025 at 01:39:26PM -0500, Mario Limonciello wrote:
On 6/21/25 2:05 PM, Lukas Wunner wrote:
So the refcount decrement happens in pcie_portdrv_probe() and
the refcount increment happens in pcie_portdrv_remove().
Both times it's conditional on pci_bridge_d3_possible().
Does that return a different value on probe versus remove?
I did this check and yes specifically on this PCIe port with the underflow
the d3 possible lookup returns false during pcie_portdrv_remove(). It
returns true during pcie_portdrv_probe().
That's not supposed to happen. The expectation is that
pci_bridge_d3_possible() always returns the same value.
I'm wondering if the patch below fixes the issue?
Refined patch below with proper commit message,
also avoids a compiler warning caused by an unused variable.
Yes this works, thanks!
-- >8 --
From: Lukas Wunner <lukas@xxxxxxxxx>
Subject: [PATCH] PCI/ACPI: Fix runtime PM ref imbalance on hot-plug capable
ports
pcie_portdrv_probe() and pcie_portdrv_remove() both call
pci_bridge_d3_possible() to determine whether to use runtime power
management. The underlying assumption is that pci_bridge_d3_possible()
always returns the same value because otherwise a runtime PM reference
imbalance occurs.
That assumption falls apart if the device is inaccessible on ->remove()
due to hot-unplug: pci_bridge_d3_possible() calls pciehp_is_native(),
which accesses Config Space to determine whether the device is Hot-Plug
Capable. An inaccessible device generally returns "all ones" for such
Config Read Requests. Hence the device may seem Hot-Plug Capable on
->remove() even though it wasn't on ->probe().
Use the cached copy of the Hot-Plug Capable bit to avoid the Config Space
access and the resulting runtime PM ref imbalance.
Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
LGTM
Reviewed-by: Rafael J. Wysocki <rafael@xxxxxxxxxx>
Tested-by: Mario Limonciello <mario.limonciello@xxxxxxx>>
---
drivers/pci/pci-acpi.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index b78e0e4..8859cce 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -816,13 +816,11 @@ int pci_acpi_program_hp_params(struct pci_dev *dev)
bool pciehp_is_native(struct pci_dev *bridge)
{
const struct pci_host_bridge *host;
- u32 slot_cap;
if (!IS_ENABLED(CONFIG_HOTPLUG_PCI_PCIE))
return false;
- pcie_capability_read_dword(bridge, PCI_EXP_SLTCAP, &slot_cap);
- if (!(slot_cap & PCI_EXP_SLTCAP_HPC))
+ if (!bridge->is_hotplug_bridge)
return false;
if (pcie_ports_native)
--
2.47.2
