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@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>
---
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
