[PATCH v3] drm/amdgpu: prefer exact bus match in VFCT VBIOS lookup

[Matthias Kesenheimer]

When multiple cards share the same PCI device ID (e.g. RX 6600 and
RX 6600 XT both use 0x73ff), the VFCT table contains separate VBIOS
images for each card distinguished by their bus number.

The current implementation iterates VFCT entries and returns on the
first device-identity match, which may not have an exact bus match.
This causes cards to load the wrong VBIOS image (e.g. RX 6600 loading
RX 6600 XT's VBIOS), resulting in incorrect clock speeds and
potential system hangs.

Walk the VFCT table in a single pass, preferring an exact bus match.
amdgpu_acpi_vfct_match() returns 0 on an exact bus match, 1 on a
device-identity-only match, or -ENODEV on no match. The walk records
the matched entry - an exact bus match ends the walk immediately,
otherwise the first device-identity match is kept as a fallback. The
VBIOS is then loaded and validated once, after the walk, and the
bus-mismatch notice is emitted only when that fallback is actually
used.

Changes since v2:
- Load and validate the VBIOS once, after the walk, instead of
  duplicating the kmemdup()/check_atom_bios()/amdgpu_bios_release()
  sequence between the exact-match and fallback branches.
- Only emit the bus-mismatch notice when the device-identity fallback
  is actually used, avoiding a false notice on multi-GPU systems where
  an exact bus match exists.

Signed-off-by: Matthias Kesenheimer <m.kesenheimer@xxxxxxx>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 66 ++++++++++++++++--------
 1 file changed, 44 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
index f9315e0472fe..d69f8df87249 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
@@ -441,7 +441,8 @@ static bool amdgpu_read_disabled_bios(struct amdgpu_device *adev)
  * the bus numbers disagree we accept the entry if the device identity
  * matches.
  *
- * Returns: 0 on match, -ENODEV on no match
+ * Returns: 0 on exact bus match, 1 on device-identity match (bus
+ * mismatch), -ENODEV on no match
  */
 static int amdgpu_acpi_vfct_match(struct amdgpu_device *adev,
 				  VFCT_IMAGE_HEADER *vhdr)
@@ -460,11 +461,7 @@ static int amdgpu_acpi_vfct_match(struct amdgpu_device *adev,
 		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 1;
 }

 static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
@@ -474,6 +471,7 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
 	UEFI_ACPI_VFCT *vfct;
 	unsigned int offset;
 	bool r = false;
+	GOP_VBIOS_CONTENT *vbios = NULL;

 	if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
 		return false;
@@ -487,8 +485,9 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
 	offset = vfct->VBIOSImageOffset;

 	while (offset < tbl_size) {
-		GOP_VBIOS_CONTENT *vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + offset);
-		VFCT_IMAGE_HEADER *vhdr = &vbios->VbiosHeader;
+		GOP_VBIOS_CONTENT *entry = (GOP_VBIOS_CONTENT *)((char *)hdr + offset);
+		VFCT_IMAGE_HEADER *vhdr = &entry->VbiosHeader;
+		int match;

 		offset += sizeof(VFCT_IMAGE_HEADER);
 		if (offset > tbl_size) {
@@ -502,23 +501,46 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
 			goto out;
 		}

-		if (vhdr->ImageLength &&
-		    !amdgpu_acpi_vfct_match(adev, vhdr)) {
-			adev->bios = kmemdup(&vbios->VbiosContent,
-					     vhdr->ImageLength,
-					     GFP_KERNEL);
-
-			if (!check_atom_bios(adev, vhdr->ImageLength)) {
-				amdgpu_bios_release(adev);
-				goto out;
-			}
-			adev->bios_size = vhdr->ImageLength;
-			r = true;
-			goto out;
+		if (!vhdr->ImageLength)
+			continue;
+
+		match = amdgpu_acpi_vfct_match(adev, vhdr);
+		if (match < 0)
+			continue;
+
+		if (match == 0) {
+			/* Exact bus match wins - use it and stop scanning */
+			vbios = entry;
+			break;
 		}
+
+		/* match == 1: record the first device-identity match as a fallback */
+		if (!vbios)
+			vbios = entry;
 	}

-	dev_info(adev->dev, "ACPI VFCT table present but broken (too short #2),skipping\n");
+	/* Load and validate the matched VBIOS once */
+	if (vbios) {
+		VFCT_IMAGE_HEADER *vhdr = &vbios->VbiosHeader;
+
+		if (vhdr->PCIBus != adev->pdev->bus->number)
+			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);
+		adev->bios = kmemdup(&vbios->VbiosContent,
+				     vhdr->ImageLength,
+				     GFP_KERNEL);
+
+		if (!check_atom_bios(adev, vhdr->ImageLength)) {
+			amdgpu_bios_release(adev);
+			goto out;
+		}
+		adev->bios_size = vhdr->ImageLength;
+		r = true;
+	} else {
+		dev_info(adev->dev, "ACPI VFCT table present but broken (too short #2),skipping\n");
+	}

 out:
 	acpi_put_table(hdr);
--
2.56.0

> On 31. Oct 2026, at 21:51, sashiko-bot@xxxxxxxxxx wrote:
> 
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] drm/amdgpu: changelog and Signed-off-by placed below --- separator
> - [Low] drm/amdgpu: false bus-mismatch notice in amdgpu_acpi_vfct_match()
> - [Low] drm/amdgpu: duplicated VBIOS load logic in amdgpu_acpi_vfct_bios()
> --
> 
> commit 02e5f189d060ccb33fa88a19690736b51eba300f
> Author: Matthias Kesenheimer <m.kesenheimer@xxxxxxx>
> 
> drm/amdgpu: prefer exact bus match in VFCT VBIOS lookup
> 
> This commit modifies amdgpu_acpi_vfct_bios() and amdgpu_acpi_vfct_match()
> to walk the ACPI VFCT table in a single pass, preferring an exact PCI bus
> number match and falling back to the first device-identity match only if no
> exact bus match is found.
> 
>>    drm/amdgpu: prefer exact bus match in VFCT VBIOS lookup
>> 
>>    Thanks for the review. Addressing both points:
>>     - The unused fallback_vbios/fallback_vhdr are now actually used: the first device-identity match is recorded during the pass and loaded only if no exact bus match is found.
>>    - Collapsed the two passes into a single table walk, keeping all matching logic in amdgpu_acpi_vfct_match() (returns 0 exact-bus / 1 identity-only / -ENODEV none). Matching behavior is unchanged from v1.
>> 
>>    The patch works on a live system.
> 
> [Severity: Medium]
> It looks like the v2 review reply was placed above the --- separator, while
> the actual commit description and Signed-off-by tag were placed below it.
> When applied via git am, everything below --- is stripped, leaving the
> commit without its changelog explanation or Signed-off-by tag. Could the
> commit description and Signed-off-by be moved above the --- line, with the
> v2 changelog notes below it?
> 
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>> index f9315e0472fe9..ef9fd98eb9e1a 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>> @@ -441,7 +441,8 @@ static bool amdgpu_read_disabled_bios(struct amdgpu_device *adev)
>>  * the bus numbers disagree we accept the entry if the device identity
>>  * matches.
>>  *
>> - * Returns: 0 on match, -ENODEV on no match
>> + * Returns: 0 on exact bus match, 1 on device-identity match (bus
>> + * mismatch), -ENODEV on no match
>>  */
>> static int amdgpu_acpi_vfct_match(struct amdgpu_device *adev,
>>  VFCT_IMAGE_HEADER *vhdr)
>> @@ -464,7 +465,7 @@ static int amdgpu_acpi_vfct_match(struct amdgpu_device *adev,
>>   "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 1;
>> }
> 
> [Severity: Low]
> Will amdgpu_acpi_vfct_match() log a false bus-mismatch notice when probing
> the second GPU on a system with multiple identical PCI device IDs?
> 
> When amdgpu_acpi_vfct_bios() walks the VFCT table for the second card,
> amdgpu_acpi_vfct_match() first encounters the entry for the first card on a
> different bus and immediately prints:
> 
>    dev_notice(adev->dev,
>               "VFCT bus number mismatch: table %u != runtime %u, matching by device identity (vendor 0x%04x device 0x%04x)\n",
>               ...);
> 
> before returning 1, even though a subsequent VFCT entry matches the second
> card's bus number exactly and is used instead.
> 
> Would it be better to move this dev_notice() out of amdgpu_acpi_vfct_match()
> and emit it in amdgpu_acpi_vfct_bios() only when the fallback entry is
> actually selected after the loop?
> 
>> 
>> static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
>> @@ -474,6 +475,8 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
>> UEFI_ACPI_VFCT *vfct;
>> unsigned int offset;
>> bool r = false;
>> + GOP_VBIOS_CONTENT *fallback_vbios = NULL;
>> + VFCT_IMAGE_HEADER *fallback_vhdr = NULL;
>> 
>> if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
>> return false;
>> @@ -489,6 +492,7 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
>> while (offset < tbl_size) {
>> GOP_VBIOS_CONTENT *vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + offset);
>> VFCT_IMAGE_HEADER *vhdr = &vbios->VbiosHeader;
>> + int match;
>> 
>> offset += sizeof(VFCT_IMAGE_HEADER);
>> if (offset > tbl_size) {
>> @@ -502,8 +506,15 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
>> goto out;
>> }
>> 
>> - if (vhdr->ImageLength &&
>> -    !amdgpu_acpi_vfct_match(adev, vhdr)) {
>> + if (!vhdr->ImageLength)
>> + continue;
>> +
>> + match = amdgpu_acpi_vfct_match(adev, vhdr);
>> + if (match < 0)
>> + continue;
>> +
>> + /* Exact bus match - use immediately */
>> + if (match == 0) {
>> adev->bios = kmemdup(&vbios->VbiosContent,
>>     vhdr->ImageLength,
>>     GFP_KERNEL);
>> @@ -516,9 +527,29 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
>> r = true;
>> goto out;
>> }
>> +
>> + /* Device-identity match (bus mismatch) - record as fallback */
>> + if (!fallback_vbios) {
>> + fallback_vbios = vbios;
>> + fallback_vhdr = vhdr;
>> + }
>> }
>> 
>> - dev_info(adev->dev, "ACPI VFCT table present but broken (too short #2),skipping\n");
>> + /* No exact bus match - use the first device-identity match */
>> + if (fallback_vbios) {
>> + adev->bios = kmemdup(&fallback_vbios->VbiosContent,
>> +     fallback_vhdr->ImageLength,
>> +     GFP_KERNEL);
>> +
>> + if (!check_atom_bios(adev, fallback_vhdr->ImageLength)) {
>> + amdgpu_bios_release(adev);
>> + goto out;
>> + }
>> + adev->bios_size = fallback_vhdr->ImageLength;
>> + r = true;
>> + } else {
>> + dev_info(adev->dev, "ACPI VFCT table present but broken (too short #2),skipping\n");
>> + }
> 
> [Severity: Low]
> As reported by Sashiko on v1 [1], this isn't a bug, but the kmemdup(),
> check_atom_bios(), and amdgpu_bios_release() sequence in
> amdgpu_acpi_vfct_bios() is still duplicated between the exact-match branch
> inside the loop and the fallback_vbios branch after the loop.
> 
> Since vhdr is &vbios->VbiosHeader, could the loop simply record the matched
> GOP_VBIOS_CONTENT pointer (breaking immediately when match == 0) and run the
> allocation and validation once after the loop?
> 
> [1] https://lore.kernel.org/r/6C5E74D6-8F98-48BC-9173-7E85D243B3D3@xxxxxxx
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/2630FE08-2F46-4D29-AF3C-DD9EF2DE581E@xxxxxxx?part=1