Re: [PATCH v5 1/3] PCI: Cache bridge firmware secondary bus number

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] PCI: unconfigured secondary bus 0 cached as valid in
  pci_scan_bridge_extend()
--

commit fd23152db632066be6cd9884f50d5722e11a3151
Author: Mario Limonciello <mario.limonciello@amd.com>

PCI: Cache bridge firmware secondary bus number

This commit caches a bridge's firmware-assigned secondary bus number on
struct pci_dev during pass 0 of bridge scanning so devices enumerated in
pass 1 can still access the original bus number after renumbering.

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 27008e2ea5afc..14b3dd7dac271 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1418,6 +1418,10 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
>  
>  	pci_dbg(dev, "scanning [bus %02x-%02x] behind bridge, pass %d\n",
>  		secondary, subordinate, pass);
> +	if (!pass && !dev->fw_sec_busnr_valid) {
> +		dev->fw_sec_busnr = secondary;
> +		dev->fw_sec_busnr_valid = true;
> +	}

[Severity: Medium]
Does this mark dev->fw_sec_busnr_valid as true even when secondary is 0
(unconfigured by firmware or hot-plugged) or 0xff (failed config read)?

Bus 0 is the root bus and is never a valid secondary bus behind a bridge.
When firmware leaves a bridge unconfigured for pass 1 to assign, this caches
dev->fw_sec_busnr = 0 with dev->fw_sec_busnr_valid = true, so consumers
checking bridge->fw_sec_busnr_valid will use bus 0 instead of falling back
to pdev->bus->number.

Note that simply gating this assignment on secondary > 0 && secondary < 0xff
would leave dev->fw_sec_busnr_valid false on an initially unconfigured
bridge, allowing a later rescan to cache the runtime-assigned bus number, so
tracking whether pass 0 has already run may need to be separate from whether
fw_sec_busnr is valid.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-vfct-regression-v5-0-f49fae981bf3@amd.com?part=1