[PATCH v3 0/3] Fix amdgpu detection of pci=realloc

[PATCH v3 0/3] Fix amdgpu detection of pci=realloc

[Mario Limonciello]

amdgpu needs to know the bus number the platform assigned a device to
be able to match firmware images against the right device.

This was previously adjusted to be a soft filter due to
commit 11c141672045f ("drm/amdgpu: Fix VFCT bus number matching with
soft filter") where if everything but the number matched to take a device.

This however fails when there are two identical devices.  Instead save the
initial bus number to the PCI device structure and then match that.

Link: https://lore.kernel.org/linux-pci/20261015124843.203905-1-mario.limonciello@amd.com/
Link: https://lore.kernel.org/amd-gfx/20261019194106.2796150-1-alexander.deucher@amd.com/
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
Changes in v3:
- Preserve the firmware bus number across bridge renumbering, capturing the
  secondary bus number before pci_scan_bridge_extend() clears it.
- Adjust member name in all patches
- Add tags
- Link to v2: https://patch.msgid.link/20261020-vfct-regression-v2-0-d2394d11450a@amd.com

Changes in v2:
- pci_alloc_dev(NULL) remains safe.
- For buses behind a reassigned bridge, the PCI core now saves the firmware secondary bus
  number before clearing the bridge configuration and carries it into the newly allocated bus.
  Devices discovered there inherit that original number.
- Add changes for radeon too.
- Link to v1: https://patch.msgid.link/20261019-vfct-regression-v1-0-5a63ddd378dd@amd.com

---
Mario Limonciello (3):
      PCI: Preserve the initial bus number in pci_bus
      drm/amdgpu: Match VFCT using initial PCI bus number
      drm/radeon: Match VFCT using initial PCI bus number

 drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 17 +++--------------
 drivers/gpu/drm/radeon/radeon_bios.c     | 17 +++--------------
 drivers/pci/probe.c                      |  9 +++++++++
 include/linux/pci.h                      |  2 ++
 4 files changed, 17 insertions(+), 28 deletions(-)
---
base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
change-id: 20261019-vfct-regression-2142fc52852a

Best regards,
--  
Mario Limonciello <mario.limonciello@amd.com>

[PATCH v3 1/3] PCI: Preserve the initial bus number in pci_bus

[Mario Limonciello]

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. Drivers can then match firmware records against the bus number
from before PCI bus renumbering.

The initial number belongs to the bus, so consumers can use the
pci_bus value directly without copying it into every pci_dev.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/pci/probe.c | 9 +++++++++
 include/linux/pci.h | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 27008e2ea5afc..9950a9dd1db1f 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->number_initial = 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->number_initial = 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->number_initial = 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->secondary_busnr)
+				dev->secondary_busnr = secondary;
 			if (pcibios_assign_all_busses() || broken)
 
 				/*
@@ -1529,6 +1534,10 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
 			pci_bus_insert_busn_res(child, next_busnr,
 						bus->busn_res.end);
 		}
+		if (dev->secondary_busnr) {
+			child->number_initial = dev->secondary_busnr;
+			dev->secondary_busnr = 0;
+		}
 		max++;
 		if (available_buses)
 			available_buses--;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 5ffcf5ce8c966..75a5fd2374acb 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -363,6 +363,7 @@ struct pci_dev {
 	struct pci_slot	*slot;		/* Physical slot this device is in */
 
 	unsigned int	devfn;		/* Encoded device & function index */
+	u8		secondary_busnr;	/* Original secondary bus number */
 	unsigned short	vendor;
 	unsigned short	device;
 	unsigned short	subsystem_vendor;
@@ -724,6 +725,7 @@ struct pci_bus {
 	struct proc_dir_entry *procdir;	/* Directory entry in /proc/bus/pci */
 
 	unsigned char	number;		/* Bus number */
+	unsigned char	number_initial;	/* Bus number before reassignment */
 	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

[PATCH v3 2/3] drm/amdgpu: Match VFCT using initial PCI bus number

[Mario Limonciello]

VFCT records the PCI bus number from BIOS POST. The current bus number
can change after PCI core renumbers the bus, while accepting a mismatch
based only on device identity can select the wrong VFCT image when
matching devices are present. Compare against the bus number captured
from the firmware bus assignment before bridge renumbering.

Reported-by: Reza Nikzad Chaleshori <rezanikzadch@gmail.com>
Closes: https://lore.kernel.org/amd-gfx/20261014080523.186533-1-rezanikzadch@gmail.com/
Fixes: 11c141672045f ("drm/amdgpu: Fix VFCT bus number matching with soft filter")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
index 08ed1eb695586..051be1905e296 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
@@ -386,10 +386,8 @@ static bool amdgpu_read_disabled_bios(struct amdgpu_device *adev)
  * VFCT entries contain the PCI bus number as recorded during BIOS POST.
  * On systems where the kernel renumbers PCI buses (e.g. pci=realloc or
  * resource conflicts), the runtime bus number may differ from the POST
- * value.  Match by device identity (vendor + device + function) and use
- * the bus number as a preference: exact bus match is preferred, but when
- * the bus numbers disagree we accept the entry if the device identity
- * matches.
+ * value.  Match using the bus number captured when the device was
+ * discovered, before any bus renumbering.
  *
  * Returns: 0 on match, -ENODEV on no match
  */
@@ -405,16 +403,7 @@ static int amdgpu_acpi_vfct_match(struct amdgpu_device *adev,
 	    vhdr->PCIFunction != PCI_FUNC(adev->pdev->devfn))
 		return -ENODEV;
 
-	/* Exact bus number match - preferred */
-	if (vhdr->PCIBus == adev->pdev->bus->number)
-		return 0;
-
-	/* Bus mismatch but device identity matches (PCI renumbering case) */
-	dev_notice(adev->dev,
-		   "VFCT bus number mismatch: table %u != runtime %u, matching by device identity (vendor 0x%04x device 0x%04x)\n",
-		   vhdr->PCIBus, adev->pdev->bus->number,
-		   adev->pdev->vendor, adev->pdev->device);
-	return 0;
+	return vhdr->PCIBus == adev->pdev->bus->number_initial ? 0 : -ENODEV;
 }
 
 static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)

-- 
2.43.0

[PATCH v3 3/3] drm/radeon: Match VFCT using initial PCI bus number

[Mario Limonciello]

VFCT records the PCI bus number from BIOS POST. The current bus number
can change after PCI core renumbers the bus, while accepting a mismatch
based only on device identity can select the wrong VFCT image when
matching devices are present. Compare against the bus number captured
from the firmware bus assignment before bridge renumbering.

Fixes: 4059e2f02c8a ("drm/radeon: Fix VFCT bus number matching with soft filter")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/gpu/drm/radeon/radeon_bios.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
index 1da6bf1c17ff3..bdbca4fad6b8c 100644
--- a/drivers/gpu/drm/radeon/radeon_bios.c
+++ b/drivers/gpu/drm/radeon/radeon_bios.c
@@ -609,10 +609,8 @@ static bool radeon_read_disabled_bios(struct radeon_device *rdev)
  * VFCT entries contain the PCI bus number as recorded during BIOS POST.
  * On systems where the kernel renumbers PCI buses (e.g. pci=realloc or
  * resource conflicts), the runtime bus number may differ from the POST
- * value.  Match by device identity (vendor + device + function) and use
- * the bus number as a preference: exact bus match is preferred, but when
- * the bus numbers disagree we accept the entry if the device identity
- * matches.
+ * value.  Match using the bus number captured when the device was
+ * discovered, before any bus renumbering.
  *
  * Returns: 0 on match, -ENODEV on no match
  */
@@ -628,16 +626,7 @@ static int radeon_acpi_vfct_match(struct radeon_device *rdev,
 	    vhdr->PCIFunction != PCI_FUNC(rdev->pdev->devfn))
 		return -ENODEV;
 
-	/* Exact bus number match - preferred */
-	if (vhdr->PCIBus == rdev->pdev->bus->number)
-		return 0;
-
-	/* Bus mismatch but device identity matches (PCI renumbering case) */
-	dev_notice(&rdev->pdev->dev,
-		   "VFCT bus number mismatch: table %u != runtime %u, matching by device identity (vendor 0x%04x device 0x%04x)\n",
-		   vhdr->PCIBus, rdev->pdev->bus->number,
-		   rdev->pdev->vendor, rdev->pdev->device);
-	return 0;
+	return vhdr->PCIBus == rdev->pdev->bus->number_initial ? 0 : -ENODEV;
 }
 
 static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)

-- 
2.43.0

Re: [PATCH v3 2/3] drm/amdgpu: Match VFCT using initial PCI bus number

[Mario Limonciello]

On 10/26/26 10:48, Mario Limonciello wrote:
> VFCT records the PCI bus number from BIOS POST. The current bus number
> can change after PCI core renumbers the bus, while accepting a mismatch
> based only on device identity can select the wrong VFCT image when
> matching devices are present. Compare against the bus number captured
> from the firmware bus assignment before bridge renumbering.
> 
> Reported-by: Reza Nikzad Chaleshori <rezanikzadch@gmail.com>
> Closes: https://lore.kernel.org/amd-gfx/20261014080523.186533-1-rezanikzadch@gmail.com/
> Fixes: 11c141672045f ("drm/amdgpu: Fix VFCT bus number matching with soft filter")
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Accidentally lost Alex's tag when I rebased.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 17 +++--------------
>   1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> index 08ed1eb695586..051be1905e296 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
> @@ -386,10 +386,8 @@ static bool amdgpu_read_disabled_bios(struct amdgpu_device *adev)
>    * VFCT entries contain the PCI bus number as recorded during BIOS POST.
>    * On systems where the kernel renumbers PCI buses (e.g. pci=realloc or
>    * resource conflicts), the runtime bus number may differ from the POST
> - * value.  Match by device identity (vendor + device + function) and use
> - * the bus number as a preference: exact bus match is preferred, but when
> - * the bus numbers disagree we accept the entry if the device identity
> - * matches.
> + * value.  Match using the bus number captured when the device was
> + * discovered, before any bus renumbering.
>    *
>    * Returns: 0 on match, -ENODEV on no match
>    */
> @@ -405,16 +403,7 @@ static int amdgpu_acpi_vfct_match(struct amdgpu_device *adev,
>   	    vhdr->PCIFunction != PCI_FUNC(adev->pdev->devfn))
>   		return -ENODEV;
>   
> -	/* Exact bus number match - preferred */
> -	if (vhdr->PCIBus == adev->pdev->bus->number)
> -		return 0;
> -
> -	/* Bus mismatch but device identity matches (PCI renumbering case) */
> -	dev_notice(adev->dev,
> -		   "VFCT bus number mismatch: table %u != runtime %u, matching by device identity (vendor 0x%04x device 0x%04x)\n",
> -		   vhdr->PCIBus, adev->pdev->bus->number,
> -		   adev->pdev->vendor, adev->pdev->device);
> -	return 0;
> +	return vhdr->PCIBus == adev->pdev->bus->number_initial ? 0 : -ENODEV;
>   }
>   
>   static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
> 

Re: [PATCH v3 3/3] drm/radeon: Match VFCT using initial PCI bus number

[Mario Limonciello]

On 10/26/26 10:48, Mario Limonciello wrote:
> VFCT records the PCI bus number from BIOS POST. The current bus number
> can change after PCI core renumbers the bus, while accepting a mismatch
> based only on device identity can select the wrong VFCT image when
> matching devices are present. Compare against the bus number captured
> from the firmware bus assignment before bridge renumbering.
> 
> Fixes: 4059e2f02c8a ("drm/radeon: Fix VFCT bus number matching with soft filter")
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Lost alex's tag on the rebase.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   drivers/gpu/drm/radeon/radeon_bios.c | 17 +++--------------
>   1 file changed, 3 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/radeon/radeon_bios.c b/drivers/gpu/drm/radeon/radeon_bios.c
> index 1da6bf1c17ff3..bdbca4fad6b8c 100644
> --- a/drivers/gpu/drm/radeon/radeon_bios.c
> +++ b/drivers/gpu/drm/radeon/radeon_bios.c
> @@ -609,10 +609,8 @@ static bool radeon_read_disabled_bios(struct radeon_device *rdev)
>    * VFCT entries contain the PCI bus number as recorded during BIOS POST.
>    * On systems where the kernel renumbers PCI buses (e.g. pci=realloc or
>    * resource conflicts), the runtime bus number may differ from the POST
> - * value.  Match by device identity (vendor + device + function) and use
> - * the bus number as a preference: exact bus match is preferred, but when
> - * the bus numbers disagree we accept the entry if the device identity
> - * matches.
> + * value.  Match using the bus number captured when the device was
> + * discovered, before any bus renumbering.
>    *
>    * Returns: 0 on match, -ENODEV on no match
>    */
> @@ -628,16 +626,7 @@ static int radeon_acpi_vfct_match(struct radeon_device *rdev,
>   	    vhdr->PCIFunction != PCI_FUNC(rdev->pdev->devfn))
>   		return -ENODEV;
>   
> -	/* Exact bus number match - preferred */
> -	if (vhdr->PCIBus == rdev->pdev->bus->number)
> -		return 0;
> -
> -	/* Bus mismatch but device identity matches (PCI renumbering case) */
> -	dev_notice(&rdev->pdev->dev,
> -		   "VFCT bus number mismatch: table %u != runtime %u, matching by device identity (vendor 0x%04x device 0x%04x)\n",
> -		   vhdr->PCIBus, rdev->pdev->bus->number,
> -		   rdev->pdev->vendor, rdev->pdev->device);
> -	return 0;
> +	return vhdr->PCIBus == rdev->pdev->bus->number_initial ? 0 : -ENODEV;
>   }
>   
>   static bool radeon_acpi_vfct_bios(struct radeon_device *rdev)
> 

Re: [PATCH v3 3/3] drm/radeon: Match VFCT using initial PCI bus number

[sashiko-bot]

> VFCT records the PCI bus number from BIOS POST. The current bus number
> can change after PCI core renumbers the bus, while accepting a mismatch
> based only on device identity can select the wrong VFCT image when
> matching devices are present. Compare against the bus number captured
> from the firmware bus assignment before bridge renumbering.
> 
> Fixes: 4059e2f02c8a ("drm/radeon: Fix VFCT bus number matching with soft filter")
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-vfct-regression-v3-0-fe9586ed6978@amd.com?part=3

Re: [PATCH v3 2/3] drm/amdgpu: Match VFCT using initial PCI bus number

[sashiko-bot]

> VFCT records the PCI bus number from BIOS POST. The current bus number
> can change after PCI core renumbers the bus, while accepting a mismatch
> based only on device identity can select the wrong VFCT image when
> matching devices are present. Compare against the bus number captured
> from the firmware bus assignment before bridge renumbering.
> 
> Reported-by: Reza Nikzad Chaleshori <rezanikzadch@gmail.com>
> Closes: https://lore.kernel.org/amd-gfx/20261014080523.186533-1-rezanikzadch@gmail.com/
> Fixes: 11c141672045f ("drm/amdgpu: Fix VFCT bus number matching with soft filter")
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-vfct-regression-v3-0-fe9586ed6978@amd.com?part=2

Re: [PATCH v3 1/3] PCI: Preserve the initial bus number in pci_bus

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] PCI: number_initial clobbered on bus rescan in pci_scan_bridge_extend()
--

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

PCI: Preserve the initial bus number in pci_bus

This commit saves the firmware-assigned secondary bus number before PCI bus
renumbering and stores it in pci_bus->number_initial so drivers can match
firmware records against the initial bus number.

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 27008e2ea5afc..9950a9dd1db1f 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c

[ ... ]

> @@ -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->number_initial = 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->number_initial = 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->secondary_busnr)
> +				dev->secondary_busnr = secondary;
>  			if (pcibios_assign_all_busses() || broken)
>  
>  				/*
> @@ -1529,6 +1534,10 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
>  			pci_bus_insert_busn_res(child, next_busnr,
>  						bus->busn_res.end);
>  		}
> +		if (dev->secondary_busnr) {
> +			child->number_initial = dev->secondary_busnr;
> +			dev->secondary_busnr = 0;
> +		}

[Severity: Medium]
Can this overwrite child->number_initial with the reassigned runtime bus
number during a subsequent PCI bus rescan when pcibios_assign_all_busses()
is enabled?

During the initial scan of a bridge taking the bus reassignment branch in
pci_scan_bridge_extend(), pass 0 saves the firmware secondary bus number in
dev->secondary_busnr. Pass 1 then transfers it to child->number_initial,
resets dev->secondary_busnr to 0, and writes the newly assigned bus number
(next_busnr) into the bridge's PCI_PRIMARY_BUS register:

drivers/pci/probe.c:pci_scan_bridge_extend() {
    ...
		buses = (buses & PCI_SEC_LATENCY_TIMER_MASK) |
			FIELD_PREP(PCI_PRIMARY_BUS_MASK, child->primary) |
			FIELD_PREP(PCI_SECONDARY_BUS_MASK, child->busn_res.start) |
			FIELD_PREP(PCI_SUBORDINATE_BUS_MASK, child->busn_res.end);

		/* We need to blast all three values with a single write */
		pci_write_config_dword(dev, PCI_PRIMARY_BUS, buses);
    ...
}

When a later bus rescan occurs (for example via pci_rescan_bus(),
/sys/bus/pci/rescan, pci_rescan_bus_bridge_resize(), or hotplug), pass 0
reads PCI_PRIMARY_BUS again, where secondary is now the reassigned bus
number. Because dev->secondary_busnr was reset to 0 in the first scan,
!dev->secondary_busnr is true and dev->secondary_busnr is set to the
reassigned bus number.

In pass 1 of the rescan, pci_find_bus(pci_domain_nr(bus), next_busnr) finds
the existing child bus, and the block after if (!child) overwrites
child->number_initial with the reassigned bus number. A similar overwrite
can occur if pci_find_bus() finds an already-existing bus during hotplug.

Also, in the new-bus case, pci_add_new_bus() calls pci_alloc_child_bus(),
which sets child->number_initial = busnr (next_busnr) before calling
device_register(&child->dev), pcibios_add_bus(child), and
child->ops->add_bus(child), so those callbacks see the reassigned bus
number in child->number_initial before pci_scan_bridge_extend() updates it
from dev->secondary_busnr.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-vfct-regression-v3-0-fe9586ed6978@amd.com?part=1

Re: [PATCH v3 1/3] PCI: Preserve the initial bus number in pci_bus

[Mario Limonciello]

On 10/26/26 11:01, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] PCI: number_initial clobbered on bus rescan in pci_scan_bridge_extend()
> --
> 
> commit 66abeabff58a043b28934ad23308d764456e9fa4
> Author: Mario Limonciello <mario.limonciello@amd.com>
> 
> PCI: Preserve the initial bus number in pci_bus
> 
> This commit saves the firmware-assigned secondary bus number before PCI bus
> renumbering and stores it in pci_bus->number_initial so drivers can match
> firmware records against the initial bus number.
> 
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 27008e2ea5afc..9950a9dd1db1f 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
> 
> [ ... ]
> 
>> @@ -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->number_initial = 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->number_initial = 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->secondary_busnr)
>> +				dev->secondary_busnr = secondary;
>>   			if (pcibios_assign_all_busses() || broken)
>>   
>>   				/*
>> @@ -1529,6 +1534,10 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
>>   			pci_bus_insert_busn_res(child, next_busnr,
>>   						bus->busn_res.end);
>>   		}
>> +		if (dev->secondary_busnr) {
>> +			child->number_initial = dev->secondary_busnr;
>> +			dev->secondary_busnr = 0;
>> +		}
> 
> [Severity: Medium]
> Can this overwrite child->number_initial with the reassigned runtime bus
> number during a subsequent PCI bus rescan when pcibios_assign_all_busses()
> is enabled?
> 
> During the initial scan of a bridge taking the bus reassignment branch in
> pci_scan_bridge_extend(), pass 0 saves the firmware secondary bus number in
> dev->secondary_busnr. Pass 1 then transfers it to child->number_initial,
> resets dev->secondary_busnr to 0, and writes the newly assigned bus number
> (next_busnr) into the bridge's PCI_PRIMARY_BUS register:
> 
> drivers/pci/probe.c:pci_scan_bridge_extend() {
>      ...
> 		buses = (buses & PCI_SEC_LATENCY_TIMER_MASK) |
> 			FIELD_PREP(PCI_PRIMARY_BUS_MASK, child->primary) |
> 			FIELD_PREP(PCI_SECONDARY_BUS_MASK, child->busn_res.start) |
> 			FIELD_PREP(PCI_SUBORDINATE_BUS_MASK, child->busn_res.end);
> 
> 		/* We need to blast all three values with a single write */
> 		pci_write_config_dword(dev, PCI_PRIMARY_BUS, buses);
>      ...
> }
> 
> When a later bus rescan occurs (for example via pci_rescan_bus(),
> /sys/bus/pci/rescan, pci_rescan_bus_bridge_resize(), or hotplug), pass 0
> reads PCI_PRIMARY_BUS again, where secondary is now the reassigned bus
> number. Because dev->secondary_busnr was reset to 0 in the first scan,
> !dev->secondary_busnr is true and dev->secondary_busnr is set to the
> reassigned bus number.
> 
> In pass 1 of the rescan, pci_find_bus(pci_domain_nr(bus), next_busnr) finds
> the existing child bus, and the block after if (!child) overwrites
> child->number_initial with the reassigned bus number. A similar overwrite
> can occur if pci_find_bus() finds an already-existing bus during hotplug.
> 
> Also, in the new-bus case, pci_add_new_bus() calls pci_alloc_child_bus(),
> which sets child->number_initial = busnr (next_busnr) before calling
> device_register(&child->dev), pcibios_add_bus(child), and
> child->ops->add_bus(child), so those callbacks see the reassigned bus
> number in child->number_initial before pci_scan_bridge_extend() updates it
> from dev->secondary_busnr.
> 

I'll dig in.  I initially think this one is true.

Re: [PATCH v3 1/3] PCI: Preserve the initial bus number in pci_bus

[Lazar, Lijo]

On 26-Oct-26 9:18 PM, 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. Drivers can then match firmware records against the bus number
> from before PCI bus renumbering.
> 
> The initial number belongs to the bus, so consumers can use the
> pci_bus value directly without copying it into every pci_dev.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/pci/probe.c | 9 +++++++++
>   include/linux/pci.h | 2 ++
>   2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 27008e2ea5afc..9950a9dd1db1f 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->number_initial = 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->number_initial = 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->number_initial = 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->secondary_busnr)
> +				dev->secondary_busnr = secondary;
>   			if (pcibios_assign_all_busses() || broken)
>   
>   				/*
> @@ -1529,6 +1534,10 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
>   			pci_bus_insert_busn_res(child, next_busnr,
>   						bus->busn_res.end);
>   		}
> +		if (dev->secondary_busnr) {
> +			child->number_initial = dev->secondary_busnr;
> +			dev->secondary_busnr = 0;
> +		}
>   		max++;
>   		if (available_buses)
>   			available_buses--;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 5ffcf5ce8c966..75a5fd2374acb 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -363,6 +363,7 @@ struct pci_dev {
>   	struct pci_slot	*slot;		/* Physical slot this device is in */
>   
>   	unsigned int	devfn;		/* Encoded device & function index */
> +	u8		secondary_busnr;	/* Original secondary bus number */

This doesn't make sense for end point devices. Is it possible to keep 
under bus itself?

Thanks,
Lijo

>   	unsigned short	vendor;
>   	unsigned short	device;
>   	unsigned short	subsystem_vendor;
> @@ -724,6 +725,7 @@ struct pci_bus {
>   	struct proc_dir_entry *procdir;	/* Directory entry in /proc/bus/pci */
>   
>   	unsigned char	number;		/* Bus number */
> +	unsigned char	number_initial;	/* Bus number before reassignment */
>   	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 */
>