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

["Lazar, Lijo" <lijo.lazar@amd.com>]

On 13-Nov-26 3:24 AM, Mario Limonciello wrote:
> Drivers such as amdgpu and radeon need the bus number firmware assigned
> to a device to match it against firmware tables like VFCT. The PCI core
> may renumber buses (e.g. with pci=realloc), after which the runtime
> number no longer reflects what firmware programmed.
> 
> Record the firmware secondary bus number on the bridge's struct pci_dev
> the first time pci_read_bridge_windows() reads the bridge's bus numbers,
> which happens before any renumbering. Devices behind the bridge can then
> retrieve the original bus number via their upstream bridge.
> 
> Only store the value if it is usable: ignore unconfigured (0) and 0xff
> secondaries. Use a separate "sampled" flag so that later rescans, which
> would read the renumbered runtime value, never overwrite the cached
> number, and a "valid" flag so users can tell whether anything was
> recorded.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/pci/probe.c | 16 ++++++++++++++--
>   include/linux/pci.h |  4 ++++
>   2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 27008e2ea5afc..f20a4b8f7a23b 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -505,14 +505,26 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res,
>   static void pci_read_bridge_windows(struct pci_dev *bridge)
>   {
>   	u32 buses;
> +	u8 secondary;
> +	u8 subordinate;
>   	u16 io;
>   	u32 pmem, tmp;
>   	struct resource res;
>   
>   	pci_read_config_dword(bridge, PCI_PRIMARY_BUS, &buses);
>   	res.flags = IORESOURCE_BUS;
> -	res.start = FIELD_GET(PCI_SECONDARY_BUS_MASK, buses);
> -	res.end = FIELD_GET(PCI_SUBORDINATE_BUS_MASK, buses);
> +	secondary = FIELD_GET(PCI_SECONDARY_BUS_MASK, buses);
> +	subordinate = FIELD_GET(PCI_SUBORDINATE_BUS_MASK, buses);
> +	res.start = secondary;
> +	res.end = subordinate;
> +
> +	if (!bridge->fw_sec_bus_sampled) {
> +		bridge->fw_sec_bus_sampled = true;
> +		if (secondary > 0 && secondary < 0xff) {
> +			bridge->fw_sec_busnr = secondary;
> +			bridge->fw_sec_bus_valid = true;
> +		}
> +	}

I believe the original suggestion from Bjorn [1] to do this in 
pci_read_bridge_windows is to avoid these extra checks as it's one-time, 
and also to make sure it is done only for bridge devices.

[1] https://patchwork.freedesktop.org/patch/763534/

Thanks,
Lijo

>   	pci_info(bridge, "PCI bridge to %pR%s\n", &res,
>   		 bridge->transparent ? " (subtractive decode)" : "");
>   
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 5ffcf5ce8c966..045157488b8e7 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -363,6 +363,8 @@ struct pci_dev {
>   	struct pci_slot	*slot;		/* Physical slot this device is in */
>   
>   	unsigned int	devfn;		/* Encoded device & function index */
> +	/* Firmware secondary bus number saved across bridge scan passes */
> +	unsigned char	fw_sec_busnr;
>   	unsigned short	vendor;
>   	unsigned short	device;
>   	unsigned short	subsystem_vendor;
> @@ -414,6 +416,8 @@ struct pci_dev {
>   	unsigned int	pme_poll:1;	/* Poll device's PME status bit */
>   	unsigned int	pinned:1;	/* Whether this dev is pinned */
>   	unsigned int	config_rrs_sv:1; /* Config RRS software visibility */
> +	unsigned int	fw_sec_bus_valid:1; /* Firmware secondary bus number usable */
> +	unsigned int	fw_sec_bus_sampled:1; /* Firmware secondary bus number captured */
>   	unsigned int	imm_ready:1;	/* Supports Immediate Readiness */
>   	unsigned int	d1_support:1;	/* Low power state D1 is supported */
>   	unsigned int	d2_support:1;	/* Low power state D2 is supported */
>