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

[Matthias Kesenheimer]

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.

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

Fix: walk the VFCT table in a single pass, preferring an exact bus
match. amdgpu_acpi_vfct_match() now returns 0 on an exact bus match,
1 on a device-identity-only match, or -ENODEV on no match. An exact
bus match is loaded immediately; the first device-identity match is
recorded as a fallback and loaded only if no exact bus match is found.
This keeps the matching logic in one place while still supporting the
device-identity fallback for systems where the kernel renumbers PCI
buses.

Signed-off-by: Matthias Kesenheimer <m.kesenheimer@xxxxxxx>

---
 drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 41 +++++++++++++++++++++---
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
index f9315e0472fe..ef9fd98eb9e1 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;
 }

 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");
+	}

 out:
 	acpi_put_table(hdr);
--
2.56.0

> On 31. Oct 2026, at 11:37, sashiko-bot@xxxxxxxxxx wrote:
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Low] drm/amdgpu: unused variables in amdgpu_acpi_vfct_bios() break -Werror builds
> - [Low] drm/amdgpu: duplicated VFCT loop in amdgpu_acpi_vfct_bios() and dead code
> --
> 
> commit 50a61733895d9903e897b70fe18231ffdb69f9ae
> Author: Matthias Kesenheimer <m.kesenheimer@xxxxxxx>
> 
> drm/amdgpu: prefer exact bus match in VFCT VBIOS lookup
> 
> This commit updates amdgpu_acpi_vfct_bios() to perform two passes over the
> ACPI VFCT table, preferring an exact PCI bus match in the first pass before
> falling back to device-identity matching in the second pass.
> 
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>> index f9315e0472fe9..0cee0872100a5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
>> @@ -474,6 +474,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;
> 
> [Severity: Low]
> Are fallback_vbios and fallback_vhdr left over from an earlier single-pass
> version of this patch? Neither pointer is referenced anywhere in
> amdgpu_acpi_vfct_bios(), which triggers -Wunused-variable warnings (and breaks
> builds with CONFIG_WERROR=y or CONFIG_DRM_AMDGPU_WERROR=y) when CONFIG_ACPI is
> enabled.
> 
>> 
>> if (!ACPI_SUCCESS(acpi_get_table("VFCT", 1, &hdr)))
>> return false;
>> @@ -486,6 +488,44 @@ static bool amdgpu_acpi_vfct_bios(struct amdgpu_device *adev)
>> vfct = (UEFI_ACPI_VFCT *)hdr;
>> offset = vfct->VBIOSImageOffset;
>> 
>> + /* First pass: look for exact bus match */
>> + while (offset < tbl_size) {
>> + GOP_VBIOS_CONTENT *vbios = (GOP_VBIOS_CONTENT *)((char *)hdr + offset);
>> + VFCT_IMAGE_HEADER *vhdr = &vbios->VbiosHeader;
>> +
>> + offset += sizeof(VFCT_IMAGE_HEADER);
>> + if (offset > tbl_size) {
>> + dev_info(adev->dev, "ACPI VFCT image header truncated,skipping\n");
>> + goto out;
>> + }
>> +
>> + offset += vhdr->ImageLength;
>> + if (offset > tbl_size) {
>> + dev_info(adev->dev, "ACPI VFCT image truncated,skipping\n");
>> + goto out;
>> + }
>> +
>> + if (vhdr->ImageLength && vhdr->PCIBus == adev->pdev->bus->number &&
>> +    vhdr->VendorID == adev->pdev->vendor &&
>> +    vhdr->DeviceID == adev->pdev->device &&
>> +    vhdr->PCIDevice == PCI_SLOT(adev->pdev->devfn) &&
>> +    vhdr->PCIFunction == PCI_FUNC(adev->pdev->devfn)) {
>> + 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;
>> + }
>> + }
> 
> [Severity: Low]
> This isn't a bug, but the first pass duplicates the entire VFCT table walk,
> truncation checks, and VBIOS loading logic of the second pass, while also
> open-coding the VendorID, DeviceID, PCIDevice, and PCIFunction checks from
> amdgpu_acpi_vfct_match().
> 
> Because any entry with a matching bus and device identity now jumps to out in
> the first pass, the exact bus check in amdgpu_acpi_vfct_match() also becomes
> unreachable dead code during the second pass:
> 
> drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c:amdgpu_acpi_vfct_match() {
> ...
> /* Exact bus number match - preferred */
> if (vhdr->PCIBus == adev->pdev->bus->number)
> return 0;
> ...
> }
> 
> Would it be simpler to record the first device-identity match in
> fallback_vbios and fallback_vhdr during a single pass over the table (and load
> it after the loop if no exact bus match was found), avoiding the duplicated
> table walk and keeping the matching logic in one place?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/6C5E74D6-8F98-48BC-9173-7E85D243B3D3@xxxxxxx?part=1