Re: [PATCH v3 2/2] PCI: Fix runtime PM usage count underflow on device unplug

[Lukas Wunner <lukas@wunner.de>]

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.

-- >8 --

From: Lukas Wunner <lukas@wunner.de>
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@wunner.de>
---
 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