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