[v1] vfio/igd: Fix garbled screen on IGD passthrough with legacy VBIOS

[PATCH 5/6] vfio/igd: Toggle rom_need_patch_id flag on IGD devices

Posted by Tomita Moeko 2 months, 1 week ago

IGD ROMs are known to have wrong device IDs and bogus checksums.
Toggle the rom_need_patch_id flag when the IGD has ROM BAR or custom
romfile so that pci_rom_patch_ids() will correct the vendor/device IDs
and checksum.

The ROM BAR availability check is extracted into
vfio_pci_igd_has_rom_bar() to avoid duplicating the region info lookup.

Reported-by: K S Maan <kirandeepmaan45@gmail.com>
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
Tested-by: K S Maan <kirandeepmaan45@gmail.com>
---
 hw/vfio/igd.c | 27 +++++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index e091f21b6a..17437ae18d 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -509,11 +509,22 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
     QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, bdsm_quirk, next);
 }
 
+static bool vfio_pci_igd_has_rom_bar(VFIOPCIDevice *vdev)
+{
+    struct vfio_region_info *rom = NULL;
+    int ret;
+
+    ret = vfio_device_get_region_info(&vdev->vbasedev,
+                                      VFIO_PCI_ROM_REGION_INDEX, &rom);
+
+    return !ret && rom->size;
+}
+
 static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
 {
     struct vfio_region_info *opregion = NULL;
     PCIDevice *pdev = PCI_DEVICE(vdev);
-    int ret, gen;
+    int gen;
     uint64_t gms_size = 0;
     uint64_t *bdsm_size;
     uint32_t gmch;
@@ -556,8 +567,6 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
          * - OpRegion
          * - Same LPC bridge and Host bridge VID/DID/SVID/SSID as host
          */
-        struct vfio_region_info *rom = NULL;
-
         legacy_mode_enabled = true;
         info_report("IGD legacy mode enabled, "
                     "use x-igd-legacy-mode=off to disable it if unwanted.");
@@ -567,9 +576,7 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
          * there's no ROM, there's no point in setting up this quirk.
          * NB. We only seem to get BIOS ROMs, so UEFI VM would need CSM support.
          */
-        ret = vfio_device_get_region_info(&vdev->vbasedev,
-                                          VFIO_PCI_ROM_REGION_INDEX, &rom);
-        if ((ret || !rom->size) && !pdev->romfile) {
+        if (!vfio_pci_igd_has_rom_bar(vdev) && !pdev->romfile) {
             error_setg(&err, "Device has no ROM");
             goto error;
         }
@@ -610,6 +617,14 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
         goto error;
     }
 
+    /*
+     * IGD are known to have bad checksums and wrong device ID in its rom,
+     * request to patch it.
+     */
+    if (pdev->romfile || vfio_pci_igd_has_rom_bar(vdev)) {
+        pdev->rom_need_patch_id = true;
+    }
+
     /*
      * ASLS (OpRegion address) is read-only, emulated
      * It contains HPA, guest firmware need to reprogram it with GPA.
-- 
2.53.0

Re: [PATCH 5/6] vfio/igd: Toggle rom_need_patch_id flag on IGD devices

Posted by Alex Williamson 2 months, 1 week ago

On Thu,  4 Jun 2026 01:33:53 +0800
Tomita Moeko <tomitamoeko@gmail.com> wrote:

> IGD ROMs are known to have wrong device IDs and bogus checksums.
> Toggle the rom_need_patch_id flag when the IGD has ROM BAR or custom
> romfile so that pci_rom_patch_ids() will correct the vendor/device IDs
> and checksum.
> 
> The ROM BAR availability check is extracted into
> vfio_pci_igd_has_rom_bar() to avoid duplicating the region info lookup.
> 
> Reported-by: K S Maan <kirandeepmaan45@gmail.com>
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> Tested-by: K S Maan <kirandeepmaan45@gmail.com>
> ---
>  hw/vfio/igd.c | 27 +++++++++++++++++++++------
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index e091f21b6a..17437ae18d 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -509,11 +509,22 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
>      QLIST_INSERT_HEAD(&vdev->bars[nr].quirks, bdsm_quirk, next);
>  }
>  
> +static bool vfio_pci_igd_has_rom_bar(VFIOPCIDevice *vdev)
> +{
> +    struct vfio_region_info *rom = NULL;

Nit, copied from existing code, but the initialization here is
unnecessary.

> +    int ret;
> +
> +    ret = vfio_device_get_region_info(&vdev->vbasedev,
> +                                      VFIO_PCI_ROM_REGION_INDEX, &rom);
> +
> +    return !ret && rom->size;
> +}
> +
>  static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>  {
>      struct vfio_region_info *opregion = NULL;
>      PCIDevice *pdev = PCI_DEVICE(vdev);
> -    int ret, gen;
> +    int gen;
>      uint64_t gms_size = 0;
>      uint64_t *bdsm_size;
>      uint32_t gmch;
> @@ -556,8 +567,6 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>           * - OpRegion
>           * - Same LPC bridge and Host bridge VID/DID/SVID/SSID as host
>           */
> -        struct vfio_region_info *rom = NULL;
> -
>          legacy_mode_enabled = true;
>          info_report("IGD legacy mode enabled, "
>                      "use x-igd-legacy-mode=off to disable it if unwanted.");
> @@ -567,9 +576,7 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>           * there's no ROM, there's no point in setting up this quirk.
>           * NB. We only seem to get BIOS ROMs, so UEFI VM would need CSM support.
>           */
> -        ret = vfio_device_get_region_info(&vdev->vbasedev,
> -                                          VFIO_PCI_ROM_REGION_INDEX, &rom);
> -        if ((ret || !rom->size) && !pdev->romfile) {
> +        if (!vfio_pci_igd_has_rom_bar(vdev) && !pdev->romfile) {
>              error_setg(&err, "Device has no ROM");
>              goto error;
>          }
> @@ -610,6 +617,14 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>          goto error;
>      }
>  
> +    /*
> +     * IGD are known to have bad checksums and wrong device ID in its rom,
> +     * request to patch it.
> +     */
> +    if (pdev->romfile || vfio_pci_igd_has_rom_bar(vdev)) {
> +        pdev->rom_need_patch_id = true;
> +    }
> +
>      /*
>       * ASLS (OpRegion address) is read-only, emulated
>       * It contains HPA, guest firmware need to reprogram it with GPA.

Nit, the commit log rationalizes extracting vfio_pci_igd_has_rom_bar()
to avoid duplication, but both call sites are within the same function
and the status of whether the device has a ROM BAR is invariant.  We
could test once before we branch into legacy mode test and avoid the
helper and duplicate test.  Thanks,

Alex

[PATCH 1/6] hw/pci: Recalculate option ROM checksum before patching ID
[PATCH 2/6] hw/pci: Introduce rom_need_patch_id flag in PCIDevice
[PATCH 3/6] hw/pci: Promote pci_patch_ids() to public pci_rom_patch_ids()
[PATCH 4/6] vfio/pci: Use pci_rom_patch_ids() for IGD ROM ID patching
[PATCH 5/6] vfio/igd: Toggle rom_need_patch_id flag on IGD devices
[PATCH 6/6] vfio/igd: Clear saved BDSM in legacy VBIOS ROM at load time