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

[Mario Limonciello <mario.limonciello@amd.com>]

On 10/23/26 12:47, Bjorn Helgaas wrote:
> 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.

Agreed that the initial bus number should have one canonical home. I'll 
remove the copy from pci_dev; the drivers now read it from pdev->bus.

The separate initial_secondary_bus_number is temporary bridge state: 
pci_scan_bridge_extend() captures the firmware value in pass 0, before 
clearing the bridge, then transfers it to the child bus in pass 1 and 
clears it.

> 
>>   	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.

OK, will adjust.

> 
>>   	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
>>