Re: [PATCH v6] PCI: Make sure the bus bridge powered on when scanning bus

From: Yicong Yang
Date: Mon Jul 11 2022 - 09:05:10 EST


Hi,

a gentle ping...will it be considered for this window?

Thanks.

On 2022/6/6 21:00, Yicong Yang wrote:
> A bridge in a non-D0 power state does not forward config accesses to
> its secondary side (PCIe r6.0, sec 5.3.1). Make sure the bridge is
> in D0 while we enumerate devices below it.
>
> The case can be produced when the bridge is runtime-suspended (either
> in D3hot or D3cold) like below:
>
> $ echo 1 > /sys/bus/pci/devices/0000:80:00.0/0000:81:00.1/remove
> $ echo 1 > /sys/bus/pci/devices/0000:80:00.0/pci_bus/0000:81/rescan
>
> 0000:80:00.0 is a Root Port and it is runtime-suspended, so the
> configuration space of 0000:81:00.1 is unreachable in rescan
> and the device cannot be enumerated.
>
> Power up the bridge when scanning the child bus and allow it to
> suspend again by adding pm_runtime_get_sync()/pm_runtime_put()
> in pci_scan_child_bus_extend().
>
> Cc: Rafael J. Wysocki <rafael@xxxxxxxxxx>
> Cc: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Signed-off-by: Yicong Yang <yangyicong@xxxxxxxxxxxxx>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> ---
> Change since v5:
> - Tweak the commit message suggested by Bjorn
> Link: https://lore.kernel.org/linux-pci/20220525141930.GA290827@bhelgaas/
>
> Change since v4:
> - rephrase the commit suggested by Rafael
> Link: https://lore.kernel.org/lkml/20220422080404.27724-1-yangyicong@xxxxxxxxxxxxx/
>
> Change since v3:
> - retain the pm_runtime_*() calls in pci_scan_bridge_extend() as Rafael points
> out that it's necessary when the brigde is in D3cold
> Link: https://lore.kernel.org/linux-pci/20220414123736.34150-1-yangyicong@xxxxxxxxxxxxx/
>
> Change since v2:
> - just rebase it on v5.18-rc2
> Link: https://lore.kernel.org/linux-pci/1601029386-4928-1-git-send-email-yangyicong@xxxxxxxxxxxxx/
>
> Change since v1:
> - use an intermediate variable *bridge as suggested
> - remove the pm_runtime_*() calls in pci_scan_bridge_extend()
> Link: https://lore.kernel.org/linux-pci/1596022223-4765-1-git-send-email-yangyicong@xxxxxxxxxxxxx/
>
> drivers/pci/probe.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 17a969942d37..b108e72b6586 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2859,11 +2859,20 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
> unsigned int used_buses, normal_bridges = 0, hotplug_bridges = 0;
> unsigned int start = bus->busn_res.start;
> unsigned int devfn, fn, cmax, max = start;
> + struct pci_dev *bridge = bus->self;
> struct pci_dev *dev;
> int nr_devs;
>
> dev_dbg(&bus->dev, "scanning bus\n");
>
> + /*
> + * Make sure the bus bridge is powered on, otherwise we may not be
> + * able to scan the devices as we may fail to access the configuration
> + * space of subordinates.
> + */
> + if (bridge)
> + pm_runtime_get_sync(&bridge->dev);
> +
> /* Go find them, Rover! */
> for (devfn = 0; devfn < 256; devfn += 8) {
> nr_devs = pci_scan_slot(bus, devfn);
> @@ -2976,6 +2985,9 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
> }
> }
>
> + if (bridge)
> + pm_runtime_put(&bridge->dev);
> +
> /*
> * We've scanned the bus and so we know all about what's on
> * the other side of any bridges that may be on this bus plus
>