Re: [PATCH 1/3] PCI/PM: Honor PCI_DEV_FLAGS_NO_D3 in pci_bridge_d3_possible()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] PCI/PM: pci_bridge_d3_possible() checks NO_D3 before final
  quirks
--

commit 5fefa5fbcbd02246d38adb690d51a8862ddc1cd4
Author: Aviv Bakal <avivb@amazon.com>

PCI/PM: Honor PCI_DEV_FLAGS_NO_D3 in pci_bridge_d3_possible()

This commit returns false from pci_bridge_d3_possible() when
PCI_DEV_FLAGS_NO_D3 is set on the bridge.

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 7c6f2aba81443..1a23be4b6a073 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3055,6 +3055,10 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
>  		if (pci_bridge_d3_disable)
>  			return false;
>  
> +		/* The port is known not to work correctly in D3 */
> +		if (bridge->dev_flags & PCI_DEV_FLAGS_NO_D3)
> +			return false;

[Severity: Medium]
Can this leave bridge->bridge_d3 stuck at true when PCI_DEV_FLAGS_NO_D3 is
set by a DECLARE_PCI_FIXUP_FINAL quirk?

During enumeration, pci_device_add() -> pci_init_capabilities() ->
pci_pm_init() initializes dev->bridge_d3 = pci_bridge_d3_possible(dev)
before final quirks run in pci_bus_add_device() or pci_apply_final_quirks().
When a final quirk such as quirk_plx_pex_no_d3() or quirk_tuxeo_rp_d3()
later sets PCI_DEV_FLAGS_NO_D3 on a port, pci_bridge_d3_possible() flips
from true to false, causing pci_bridge_d3_update() on child devices to return
early without ever clearing bridge->bridge_d3:

drivers/pci/pci.c:pci_bridge_d3_update() {
	bridge = pci_upstream_bridge(dev);
	if (!bridge || !pci_bridge_d3_possible(bridge))
		return;
    ...
}

In addition, when a child device's final quirk (like quirk_tuxeo_rp_d3())
sets PCI_DEV_FLAGS_NO_D3 on its parent Root Port after pcie_portdrv_probe()
has already bound to the port, pci_bridge_d3_possible() returns true during
probe and false during pcie_portdrv_remove() and pcie_portdrv_shutdown(),
leaving runtime PM enabled and skipping the matching pm_runtime_forbid() and
pm_runtime_get_noresume() calls on removal.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261115171831.1498006-1-avivb@amazon.com?part=1