Re: [PATCH v2 1/3] PCI: Preserve the initial bus number in pci_dev

[Bjorn Helgaas <helgaas@kernel.org>]

On Tue, Oct 20, 2026 at 09:04:40AM -0500, Mario Limonciello wrote:
> Bridges that need bus reassignment are scanned in two passes. Save
> the firmware secondary bus number during the first pass, before the
> bridge is disabled, and carry it to the bus created in the second
> pass. Initialize pci_dev from this preserved bus number so drivers
> can match firmware records after PCI bus renumbering.
> 
> Handle pci_alloc_dev(NULL) as well; some in-tree callers use it to
> allocate a standalone pci_dev.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/pci/probe.c | 9 +++++++++
>  include/linux/pci.h | 3 +++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 27008e2ea5afc..4ece4b70cdc15 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1012,6 +1012,7 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
>  	bus->sysdata = bridge->sysdata;
>  	bus->ops = bridge->ops;
>  	bus->number = bus->busn_res.start = bridge->busnr;
> +	bus->initial_bus_number = bus->number;
>  #ifdef CONFIG_PCI_DOMAINS_GENERIC
>  	if (bridge->domain_nr == PCI_DOMAIN_NR_NOT_SET)
>  		bus->domain_nr = pci_bus_find_domain_nr(bus, parent);
> @@ -1232,6 +1233,7 @@ static struct pci_bus *pci_alloc_child_bus(struct pci_bus *parent,
>  
>  	/* Set up the primary, secondary and subordinate bus numbers */
>  	child->number = child->busn_res.start = busnr;
> +	child->initial_bus_number = busnr;
>  	child->primary = parent->busn_res.start;
>  	child->busn_res.end = 0xff;
>  
> @@ -1470,6 +1472,7 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
>  			child = pci_add_new_bus(bus, dev, secondary);
>  			if (!child)
>  				goto out;
> +			child->initial_bus_number = secondary;
>  			child->primary = primary;
>  			pci_bus_insert_busn_res(child, secondary, subordinate);
>  			child->bridge_ctl = bctl;
> @@ -1491,6 +1494,8 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
>  		 * do in the second pass.
>  		 */
>  		if (!pass) {
> +			if (!dev->initial_secondary_bus_number)
> +				dev->initial_secondary_bus_number = secondary;
>  			if (pcibios_assign_all_busses() || broken)
>  
>  				/*
> @@ -1526,6 +1531,9 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
>  			child = pci_add_new_bus(bus, dev, next_busnr);
>  			if (!child)
>  				goto out;
> +			if (dev->initial_secondary_bus_number)
> +				child->initial_bus_number =
> +					dev->initial_secondary_bus_number;
>  			pci_bus_insert_busn_res(child, next_busnr,
>  						bus->busn_res.end);
>  		}
> @@ -2508,6 +2516,7 @@ struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
>  	INIT_LIST_HEAD(&dev->bus_list);
>  	dev->dev.type = &pci_dev_type;
>  	dev->bus = pci_bus_get(bus);
> +	dev->initial_bus_number = bus ? bus->initial_bus_number : 0;
>  	dev->driver_exclusive_resource = (struct resource) {
>  		.name = "PCI Exclusive",
>  		.start = 0,
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 5ffcf5ce8c966..f6b377dcf7a14 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 */
> +	u8		initial_bus_number;	/* Bus number when device was discovered */
> +	u8		initial_secondary_bus_number;	/* Bus number behind this bridge */

Why do we need this in the pci_dev?  Is there something about
pci_scan_bridge_extend() that means it has to be in pci_dev as well as
pci_bus?  Multiple copies means possible inconsistencies.

>  	unsigned short	vendor;
>  	unsigned short	device;
>  	unsigned short	subsystem_vendor;
> @@ -724,6 +726,7 @@ struct pci_bus {
>  	struct proc_dir_entry *procdir;	/* Directory entry in /proc/bus/pci */
>  
>  	unsigned char	number;		/* Bus number */
> +	unsigned char	initial_bus_number;	/* Bus number before reassignment */

I think it should be stylistically similar to the existing
"pci_bus.number"; adding "initial" before or after is ok, but adding
both "initial" and "bus" makes them less connected.

Users would check "vhdr->PCIBus == rdev->pdev->bus->number_initial" or
similar.

>  	unsigned char	primary;	/* Number of primary bridge */
>  	unsigned char	max_bus_speed;	/* enum pci_bus_speed */
>  	unsigned char	cur_bus_speed;	/* enum pci_bus_speed */
> 
> -- 
> 2.43.0
>