[PATCH v2 0/7] vfio/igd: Fix garbled screen on IGD passthrough with legacy VBIOS

[PATCH v2 0/7] vfio/igd: Fix garbled screen on IGD passthrough with legacy VBIOS

[Tomita Moeko]

This series fixes the regression that on IGD passthrough with legacy
BIOS boot and VBIOS, the screen is garbled during BIOS POST and GRUB
(which uses standard VGA output routines), starting from QEMU 10.0.
Though the kernel i915 driver still works, it reports an error about
the initial GTT programmed by VBIOS is using invalid address.

i915 0000:00:02.0: [drm] *ERROR* Initial plane programming using invalid range, dma_addr=0x00000000db200000 ((null) [0x00000000baf00000-0x00000000beefffff])

With the help of AI disassembling the VBIOS image dumped from host, it
is found that the VBIOS itself implements a routine like:

    uint32_t get_BDSM() {
        static uint32_t saved = 0;
        if (saved != 0) {
            return saved;
        }
        return read_pci_config(BDSM_REG);
    }

And the saved value is not cleared after initialization. Given that IGD
devices don't have a real ROM BAR, the VBIOS image read by default from
host is actually the VBIOS shadow RAM region, containing host-side
modifications like the saved BDSM value above during POST. When the
image is executed in guest, it still uses the saved host BDSM (HPA)
instead of the value programmed by SeaBIOS in config space (GPA). This
address mismatch leads to the garbled screen and i915 error.

The previous solution, c4c45e943e51 ("vfio/pci: Intel graphics legacy
mode assignment"), adjusts GTT entry addresses to (addr - host BDSM +
guest BDSM) to workaround that. But it is removed in 5aed8b0f0be2
("vfio/igd: Remove GTT write quirk in IO BAR 4") due to inconsistent
values in MMIO BAR0 and IO BAR4. Considering it's unsafe to expose HPA
to guest, a ROM quirk clearing the saved value in VBIOS image is
introduced to fix the issue.

During debugging, it is also found that IGD VBIOS ROM doesn't always
match the actual IGD device ID, due to the fact that IGD of the same
CPU family has multiple device IDs but shares the same ROM image.
However, SeaBIOS checks the device ID strictly and refuses to run if
IDs does not match. Currently only the default path, reading ROM from
kernel patches the device ID, but the romfile path doesn't. So the ROM
ID patching logic is also refactored in this patch series to also handle
the romfile path.

These changes are tested on Haswell platform with legacy BIOS boot, by
K S Maan. Thanks to K S Maan for continuous help on locating and testing
the issue!

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3093
Reported-by: K S Maan <kirandeepmaan45@gmail.com>

Changelog:
v2:
* New patch 2/7 to fix regression with EFI option ROMs
* Refine logic in ROM ID and checksum patching
* Reorder patch 4 and 5 for cleaner bisection
* Address comments from v1
Link: https://lore.kernel.org/all/20260603173355.36121-1-tomitamoeko@gmail.com/t

Tomita Moeko (7):
  hw/pci: Recalculate option ROM checksum before patching ID
  hw/pci: Skip EFI option ROM in pci_patch_ids()
  hw/pci: Introduce rom_need_patch_id flag in PCIDevice
  hw/pci: Promote pci_patch_ids() to public pci_rom_patch_ids()
  vfio/igd: Toggle rom_need_patch_id flag on IGD devices
  vfio/pci: Use pci_rom_patch_ids() for IGD ROM ID patching
  vfio/igd: Clear saved BDSM in legacy VBIOS ROM at load time

 hw/pci/pci.c                |  58 ++++++++++------
 hw/vfio/igd-stubs.c         |   5 ++
 hw/vfio/igd.c               | 132 ++++++++++++++++++++++++++++++++++--
 hw/vfio/pci-quirks.c        |   5 ++
 hw/vfio/pci.c               |  33 ++-------
 hw/vfio/pci.h               |   3 +
 hw/vfio/trace-events        |   1 +
 include/hw/pci/pci.h        |   3 +
 include/hw/pci/pci_device.h |   1 +
 9 files changed, 188 insertions(+), 53 deletions(-)

-- 
2.53.0

[PATCH v2 1/7] hw/pci: Recalculate option ROM checksum before patching ID

[Tomita Moeko]

pci_patch_ids() only adjusts checksum based on the new IDs. For an
option ROM with invalid checksum, the patched one will still have
an invalid checksum. Always calculate the checksum and patch it if
necessary to ensure the option ROM is valid.

This is intended for fixing the romfile used in IGD passthrough as
multiple IGD devices share the same rom with possible non-matching
device ID, and its checksum is known to be bogus [1].

A helper function pci_rom_calculate_checksum() is added and exported
for reusing in IGD-specific quirk later.

[1] hw/vfio/pci.c:1090

Reported-by: K S Maan <kirandeepmaan45@gmail.com>
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 hw/pci/pci.c         | 35 ++++++++++++++++++++++++++---------
 include/hw/pci/pci.h |  2 ++
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index cec065d108..742917f79d 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2479,6 +2479,21 @@ static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset)
     return found;
 }
 
+uint8_t pci_rom_calculate_checksum(const uint8_t *ptr, uint32_t size)
+{
+    uint8_t checksum = 0;
+    uint32_t i;
+
+    for (i = 0; i < size; i++) {
+        if (i == 6) {
+            continue;
+        }
+        checksum += ptr[i];
+    }
+
+    return checksum;
+}
+
 /* Patch the PCI vendor and device ids in a PCI rom image if necessary.
    This is needed for an option rom which is used for more than one device. */
 static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size)
@@ -2514,25 +2529,27 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size)
     trace_pci_rom_and_pci_ids(pdev->romfile, vendor_id, device_id,
                               rom_vendor_id, rom_device_id);
 
-    checksum = ptr[6];
+    /* In case the checksum is bogus */
+    checksum = pci_rom_calculate_checksum(ptr, size);
 
     if (vendor_id != rom_vendor_id) {
         /* Patch vendor id and checksum (at offset 6 for etherboot roms). */
-        checksum += (uint8_t)rom_vendor_id + (uint8_t)(rom_vendor_id >> 8);
-        checksum -= (uint8_t)vendor_id + (uint8_t)(vendor_id >> 8);
-        trace_pci_rom_checksum_change(ptr[6], checksum);
-        ptr[6] = checksum;
+        checksum += (uint8_t)vendor_id + (uint8_t)(vendor_id >> 8);
+        checksum -= (uint8_t)rom_vendor_id + (uint8_t)(rom_vendor_id >> 8);
         pci_set_word(ptr + pcir_offset + 4, vendor_id);
     }
 
     if (device_id != rom_device_id) {
         /* Patch device id and checksum (at offset 6 for etherboot roms). */
-        checksum += (uint8_t)rom_device_id + (uint8_t)(rom_device_id >> 8);
-        checksum -= (uint8_t)device_id + (uint8_t)(device_id >> 8);
-        trace_pci_rom_checksum_change(ptr[6], checksum);
-        ptr[6] = checksum;
+        checksum += (uint8_t)device_id + (uint8_t)(device_id >> 8);
+        checksum -= (uint8_t)rom_device_id + (uint8_t)(rom_device_id >> 8);
         pci_set_word(ptr + pcir_offset + 6, device_id);
     }
+
+    if (ptr[6] != (uint8_t)-checksum) {
+        trace_pci_rom_checksum_change(ptr[6], (uint8_t)-checksum);
+        ptr[6] = (uint8_t)-checksum;
+    }
 }
 
 /* Add an option rom for the device */
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 5b179091de..2d8a4ad0eb 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -1103,4 +1103,6 @@ void pci_set_enabled(PCIDevice *pci_dev, bool state);
 void pci_set_power(PCIDevice *pci_dev, bool state);
 int pci_pm_init(PCIDevice *pci_dev, uint8_t offset, Error **errp);
 
+uint8_t pci_rom_calculate_checksum(const uint8_t *ptr, uint32_t size);
+
 #endif
-- 
2.53.0

[PATCH v2 2/7] hw/pci: Skip EFI option ROM in pci_patch_ids()

[Tomita Moeko]

pci_patch_ids() patches the checksum at the reserved 0x06 byte, but
for EFI option ROMs the 32 bits at 0x04 are the EFI signature and
must be 0x00000EF1. Since OVMF does not check vendor/device IDs in
the PCIR header or the checksum, skip patching for EFI ROMs.

Reported-by: K S Maan <kirandeepmaan45@gmail.com>
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 hw/pci/pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 742917f79d..eb10e586d5 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2521,6 +2521,11 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size)
         return;
     }
 
+    /* OVMF won't check IDs in PCIR header, skip EFI roms */
+    if (pci_get_byte(ptr + pcir_offset + 0x14) == 0x03) {
+        return;
+    }
+
     vendor_id = pci_get_word(pdev->config + PCI_VENDOR_ID);
     device_id = pci_get_word(pdev->config + PCI_DEVICE_ID);
     rom_vendor_id = pci_get_word(ptr + pcir_offset + 4);
-- 
2.53.0

[PATCH v2 3/7] hw/pci: Introduce rom_need_patch_id flag in PCIDevice

[Tomita Moeko]

Allow external code (e.g., VFIO IGD passthrough) to request PCI ID
patching for a custom romfile, replacing the existing is_default_rom
flag for this purpose.

Reported-by: K S Maan <kirandeepmaan45@gmail.com>
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 hw/pci/pci.c                | 14 ++++++--------
 include/hw/pci/pci_device.h |  1 +
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index eb10e586d5..0facc05aed 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -352,7 +352,7 @@ static const TypeInfo cxl_bus_info = {
 
 static void pci_update_mappings(PCIDevice *d);
 static void pci_irq_handler(void *opaque, int irq_num, int level);
-static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom, Error **);
+static void pci_add_option_rom(PCIDevice *pdev, Error **);
 static void pci_del_option_rom(PCIDevice *pdev);
 
 static uint16_t pci_default_sub_vendor_id = PCI_SUBVENDOR_ID_REDHAT_QUMRANET;
@@ -2257,7 +2257,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
     PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pci_dev);
     ObjectClass *klass = OBJECT_CLASS(pc);
     Error *local_err = NULL;
-    bool is_default_rom;
     uint16_t class_id;
 
     /*
@@ -2370,13 +2369,13 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
     }
 
     /* rom loading */
-    is_default_rom = false;
     if (pci_dev->romfile == NULL && pc->romfile != NULL) {
+        /* using a built-in default rom */
         pci_dev->romfile = g_strdup(pc->romfile);
-        is_default_rom = true;
+        pci_dev->rom_need_patch_id = true;
     }
 
-    pci_add_option_rom(pci_dev, is_default_rom, &local_err);
+    pci_add_option_rom(pci_dev, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
         pci_qdev_unrealize(DEVICE(pci_dev));
@@ -2558,8 +2557,7 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size)
 }
 
 /* Add an option rom for the device */
-static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom,
-                               Error **errp)
+static void pci_add_option_rom(PCIDevice *pdev, Error **errp)
 {
     int64_t size = 0;
     g_autofree char *path = NULL;
@@ -2657,7 +2655,7 @@ static void pci_add_option_rom(PCIDevice *pdev, bool is_default_rom,
             return;
         }
 
-        if (is_default_rom) {
+        if (pdev->rom_need_patch_id) {
             /* Only the default rom images will be patched (if needed). */
             pci_patch_ids(pdev, ptr, size);
         }
diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index 5cac6e1688..8b9b6470e9 100644
--- a/include/hw/pci/pci_device.h
+++ b/include/hw/pci/pci_device.h
@@ -157,6 +157,7 @@ struct PCIDevice {
     char *romfile;
     uint32_t romsize;
     bool has_rom;
+    bool rom_need_patch_id;
     MemoryRegion rom;
     int32_t rom_bar;
 
-- 
2.53.0

[PATCH v2 4/7] hw/pci: Promote pci_patch_ids() to public pci_rom_patch_ids()

[Tomita Moeko]

Remove the static qualifier from pci_patch_ids() and rename it to
pci_rom_patch_ids(), adding a declaration in include/hw/pci/pci.h so
external callers can reuse it.

Reported-by: K S Maan <kirandeepmaan45@gmail.com>
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 hw/pci/pci.c         | 4 ++--
 include/hw/pci/pci.h | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 0facc05aed..752cea0e77 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2495,7 +2495,7 @@ uint8_t pci_rom_calculate_checksum(const uint8_t *ptr, uint32_t size)
 
 /* Patch the PCI vendor and device ids in a PCI rom image if necessary.
    This is needed for an option rom which is used for more than one device. */
-static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size)
+void pci_rom_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size)
 {
     uint16_t vendor_id;
     uint16_t device_id;
@@ -2657,7 +2657,7 @@ static void pci_add_option_rom(PCIDevice *pdev, Error **errp)
 
         if (pdev->rom_need_patch_id) {
             /* Only the default rom images will be patched (if needed). */
-            pci_patch_ids(pdev, ptr, size);
+            pci_rom_patch_ids(pdev, ptr, size);
         }
     }
 
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 2d8a4ad0eb..298e0e6c31 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -1104,5 +1104,6 @@ void pci_set_power(PCIDevice *pci_dev, bool state);
 int pci_pm_init(PCIDevice *pci_dev, uint8_t offset, Error **errp);
 
 uint8_t pci_rom_calculate_checksum(const uint8_t *ptr, uint32_t size);
+void pci_rom_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size);
 
 #endif
-- 
2.53.0

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

[Tomita Moeko]

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.

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

diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index e091f21b6a..834539affb 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -512,12 +512,14 @@ void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr)
 static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
 {
     struct vfio_region_info *opregion = NULL;
+    struct vfio_region_info *rom = NULL;
     PCIDevice *pdev = PCI_DEVICE(vdev);
-    int ret, gen;
+    int gen;
     uint64_t gms_size = 0;
     uint64_t *bdsm_size;
     uint32_t gmch;
     bool legacy_mode_enabled = false;
+    bool has_rombar = false;
     Error *err = NULL;
 
     if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
@@ -534,6 +536,10 @@ static bool vfio_pci_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
     gen = igd_gen(vdev);
     gmch = vfio_pci_read_config(pdev, IGD_GMCH, 4);
 
+    has_rombar = !vfio_device_get_region_info(&vdev->vbasedev,
+                                              VFIO_PCI_ROM_REGION_INDEX,
+                                              &rom) && rom->size;
+
     /*
      * For backward compatibility, enable legacy mode when
      * - Device geneation is 6 to 9 (including both)
@@ -556,8 +562,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 +571,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 (!has_rombar && !pdev->romfile) {
             error_setg(&err, "Device has no ROM");
             goto error;
         }
@@ -610,6 +612,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 (has_rombar || pdev->romfile) {
+        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

[PATCH v2 6/7] vfio/pci: Use pci_rom_patch_ids() for IGD ROM ID patching

[Tomita Moeko]

Remove the duplicate inline logic in vfio_pci_load_rom() that patched
the device ID in an IGD option ROM and replace it with a call to
pci_rom_patch_ids(), conditioned on the rom_need_patch_id flag.

Reported-by: K S Maan <kirandeepmaan45@gmail.com>
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 hw/vfio/pci.c | 31 +++----------------------------
 1 file changed, 3 insertions(+), 28 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 9c06b25e63..6cbd65126e 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1029,6 +1029,7 @@ static void vfio_update_msi(VFIOPCIDevice *vdev)
 
 static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
 {
+    PCIDevice *pdev = PCI_DEVICE(vdev);
     VFIODevice *vbasedev = &vdev->vbasedev;
     struct vfio_region_info *reg_info = NULL;
     uint64_t size;
@@ -1084,34 +1085,8 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
         }
     }
 
-    /*
-     * Test the ROM signature against our device, if the vendor is correct
-     * but the device ID doesn't match, store the correct device ID and
-     * recompute the checksum.  Intel IGD devices need this and are known
-     * to have bogus checksums so we can't simply adjust the checksum.
-     */
-    if (pci_get_word(vdev->rom) == 0xaa55 &&
-        pci_get_word(vdev->rom + 0x18) + 8 < vdev->rom_size &&
-        !memcmp(vdev->rom + pci_get_word(vdev->rom + 0x18), "PCIR", 4)) {
-        uint16_t vid, did;
-
-        vid = pci_get_word(vdev->rom + pci_get_word(vdev->rom + 0x18) + 4);
-        did = pci_get_word(vdev->rom + pci_get_word(vdev->rom + 0x18) + 6);
-
-        if (vid == vdev->vendor_id && did != vdev->device_id) {
-            int i;
-            uint8_t csum, *data = vdev->rom;
-
-            pci_set_word(vdev->rom + pci_get_word(vdev->rom + 0x18) + 6,
-                         vdev->device_id);
-            data[6] = 0;
-
-            for (csum = 0, i = 0; i < vdev->rom_size; i++) {
-                csum += data[i];
-            }
-
-            data[6] = -csum;
-        }
+    if (pdev->rom_need_patch_id) {
+        pci_rom_patch_ids(pdev, vdev->rom, vdev->rom_size);
     }
 }
 
-- 
2.53.0

[PATCH v2 7/7] vfio/igd: Clear saved BDSM in legacy VBIOS ROM at load time

[Tomita Moeko]

IGD does not come with a ROM BAR [1], the ROM BAR read by default from
kernel is actually the host VBIOS shadow RAM region that contains host
modifications on boot. With AI-assisted reverse engineering on VBIOS
binaries, it is observed that VBIOS saves BDSM register value on first
access and uses saved value if present.

When the image is executed in guest, since there is already a saved HPA
in VBIOS, it keeps using that value instead of the GPA programmed by
SeaBIOS in BDSM register in PCI config space, causing VBIOS to program
GTT entries with wrong address, resulting in garbled output in BIOS
POST and the error below detected by i915 driver.

i915 0000:00:02.0: [drm] *ERROR* Initial plane programming using invalid range, dma_addr=0x00000000db200000 ((null) [0x00000000baf00000-0x00000000beefffff])

The previous solution, c4c45e943e51 ("vfio/pci: Intel graphics legacy
mode assignment"), adjusts GTT entry addresses to (addr - host BDSM +
guest BDSM) to workaround that. But it is removed in 5aed8b0f0be2
("vfio/igd: Remove GTT write quirk in IO BAR 4") due to inconsistent
values in MMIO BAR0 and IO BAR4.

Considering it's unsafe to expose HPA to guest, a ROM quirk clearing
the saved value in VBIOS image is introduced. It searches the BDSM
accessor routine by matching a 19-byte signature anchored on the unique
`mov $0x105e,%ax` instruction, then locate the offset of saved BDSM and
clears it. This makes the routine fall through to the PCI config read
on the first call inside the guest.

The quirk is invoked in vfio_pci_load_rom(), and is gated on Gen 6-9
IGD devices with VGA access enabled and legacy (non-UEFI) PCIR code
type in the ROM header. A new trace event vfio_pci_igd_vbios_patched
is also introduced.

[1] 3.5.15, 4th Generation Intel Core Processor Family Datasheet Vol. 2
    https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/4th-gen-core-family-desktop-vol-2-datasheet.pdf

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3093
Reported-by: K S Maan <kirandeepmaan45@gmail.com>
Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
---
 hw/vfio/igd-stubs.c  |   5 ++
 hw/vfio/igd.c        | 110 +++++++++++++++++++++++++++++++++++++++++++
 hw/vfio/pci-quirks.c |   5 ++
 hw/vfio/pci.c        |   2 +
 hw/vfio/pci.h        |   3 ++
 hw/vfio/trace-events |   1 +
 6 files changed, 126 insertions(+)

diff --git a/hw/vfio/igd-stubs.c b/hw/vfio/igd-stubs.c
index f7687d9091..879a8aff56 100644
--- a/hw/vfio/igd-stubs.c
+++ b/hw/vfio/igd-stubs.c
@@ -18,3 +18,8 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
 {
     return true;
 }
+
+void vfio_probe_igd_legacy_rom_quirk(VFIOPCIDevice *vdev)
+{
+    return;
+}
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 834539affb..4b49038753 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -734,3 +734,113 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
 
     return vfio_pci_igd_config_quirk(vdev, errp);
 }
+
+/*
+ * IGD ROM BAR read from kernel is actually the host VBIOS shadow RAM region,
+ * which contains host modifications. In Gen 6-9 VBIOS, the routine below is
+ * used to get BDSM value when programming the initial GTT.
+ *   xx xx xx xx           v: .long ?                  # saved value
+ *   66 53                    push  %ebx
+ *   66 2e 83 3e xx xx 00     cmpl  $0x0,%cs:v         # is saved value empty?
+ *   74 07                    je    1f                 # if zero, go compute
+ *   66 2e a1 xx xx           mov   %cs:v,%eax         # else return saved value
+ *   eb 0f                    jmp   2f
+ *   b8 5e 10              1: mov   $0x105e,%ax        # dev 00:02.0, offset 5E
+ *   e8 xx xx                 call  pci_read_cfg_word
+ *   66 c1 e0 10              shl   $0x10,%eax         # left shift 16 bits
+ *   66 2e a3 xx xx           mov   %eax,%cs:v         # save the result
+ *   66 5b                 2：pop   %ebx
+ *   c3                       ret
+ * When running the VBIOS in guest, saved value still reflects the host stolen
+ * memory base address, which is not correct in guest. So we need to patch the
+ * VBIOS to clear the saved value.
+ *
+ * The unique 19-byte starts at `cmpl $0,%cs:v` and ends at `mov $0x105e,%ax`
+ * anchors the match to the routine. Both `cs:` displacements must reference
+ * the same offset.
+ */
+static int igd_vbios_find_saved_bdsm(const uint8_t *rom, size_t rom_size,
+                                     uint16_t *bdsm_offset)
+{
+    static const uint8_t start[] = { 0x66, 0x2e, 0x83, 0x3e };
+    static const uint8_t middle[] = { 0x00, 0x74, 0x07, 0x66, 0x2e, 0xa1 };
+    static const uint8_t end[] = { 0xeb, 0x0f, 0xb8, 0x5e, 0x10 };
+    uint16_t val;
+    size_t i;
+    bool found = false;
+
+    if (rom_size < 19) {
+        return -ENOENT;
+    }
+
+    for (i = 0; i + 19 <= rom_size; i++) {
+        if (memcmp(rom + i, start, sizeof(start)) != 0 ||
+            memcmp(rom + i + 6, middle, sizeof(middle)) != 0 ||
+            memcmp(rom + i + 14, end, sizeof(end)) != 0) {
+            continue;
+        }
+
+        /* same saved value address? */
+        if (rom[i + 4] != rom[i + 12] || rom[i + 5] != rom[i + 13]) {
+            continue;
+        }
+
+        if (found) {
+            return -EEXIST;
+        }
+
+        val = rom[i + 4] | ((uint16_t)rom[i + 5] << 8);
+        if (val < rom_size) {
+            *bdsm_offset = val;
+            found = true;
+        }
+    }
+
+    if (!found) {
+        return -ENOENT;
+    }
+
+    return 0;
+}
+
+void vfio_probe_igd_legacy_rom_quirk(VFIOPCIDevice *vdev)
+{
+    int ret, gen;
+    uint16_t pcir_offset, bdsm_offset = 0;
+    uint8_t checksum;
+
+    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
+        !vfio_is_vga(vdev) || !vdev->vga) {
+        return;
+    }
+
+    /* Only Gen 6~9 devices have legacy VBIOS as Option ROM */
+    gen = igd_gen(vdev);
+    if (gen < 6 || gen > 9) {
+        return;
+    }
+
+    if (pci_get_word(vdev->rom) != 0xaa55) {
+        return;
+    }
+
+    /* Must be a legacy ROM */
+    pcir_offset = pci_get_word(vdev->rom + 0x18);
+    if (pcir_offset + 0x14 >= vdev->rom_size ||
+        memcmp(vdev->rom + pcir_offset, "PCIR", 4) ||
+        pci_get_byte(vdev->rom + pcir_offset + 0x14) != 0x00) {
+        return;
+    }
+
+    ret = igd_vbios_find_saved_bdsm(vdev->rom, vdev->rom_size, &bdsm_offset);
+    if (ret < 0) {
+        return;
+    }
+
+    memset(vdev->rom + bdsm_offset, 0, sizeof(uint32_t));
+
+    checksum = pci_rom_calculate_checksum(vdev->rom, vdev->rom_size);
+    ((uint8_t *)vdev->rom)[6] = (uint8_t)-checksum;
+
+    trace_vfio_pci_igd_vbios_patched(vdev->vbasedev.name);
+}
diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
index bccf31751f..45db968681 100644
--- a/hw/vfio/pci-quirks.c
+++ b/hw/vfio/pci-quirks.c
@@ -1592,3 +1592,8 @@ bool vfio_add_virt_caps(VFIOPCIDevice *vdev, Error **errp)
 
     return true;
 }
+
+void vfio_rom_quirk_setup(VFIOPCIDevice *vdev)
+{
+    vfio_probe_igd_legacy_rom_quirk(vdev);
+}
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 6cbd65126e..66d6315e6f 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1088,6 +1088,8 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
     if (pdev->rom_need_patch_id) {
         pci_rom_patch_ids(pdev, vdev->rom, vdev->rom_size);
     }
+
+    vfio_rom_quirk_setup(vdev);
 }
 
 /* "Raw" read of underlying config space. */
diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
index c3a1f53d35..d8d6c09632 100644
--- a/hw/vfio/pci.h
+++ b/hw/vfio/pci.h
@@ -251,10 +251,13 @@ void vfio_bar_quirk_exit(VFIOPCIDevice *vdev, int nr);
 void vfio_bar_quirk_finalize(VFIOPCIDevice *vdev, int nr);
 void vfio_setup_resetfn_quirk(VFIOPCIDevice *vdev);
 bool vfio_add_virt_caps(VFIOPCIDevice *vdev, Error **errp);
+void vfio_rom_quirk_setup(VFIOPCIDevice *vdev);
 void vfio_quirk_reset(VFIOPCIDevice *vdev);
 VFIOQuirk *vfio_quirk_alloc(int nr_mem);
+
 void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr);
 bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp);
+void vfio_probe_igd_legacy_rom_quirk(VFIOPCIDevice *vdev);
 
 extern const PropertyInfo qdev_prop_nv_gpudirect_clique;
 
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 2049159015..7dc334ccb3 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -90,6 +90,7 @@ vfio_pci_igd_bar4_write(const char *name, uint32_t index, uint32_t data, uint32_
 vfio_pci_igd_bdsm_enabled(const char *name, int size) "%s %dMB"
 vfio_pci_igd_host_bridge_enabled(const char *name) "%s"
 vfio_pci_igd_lpc_bridge_enabled(const char *name) "%s"
+vfio_pci_igd_vbios_patched(const char *name) "%s"
 
 # listener.c
 vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t iova_end) "iommu %s @ 0x%"PRIx64" - 0x%"PRIx64
-- 
2.53.0

Re: [PATCH v2 7/7] vfio/igd: Clear saved BDSM in legacy VBIOS ROM at load time

[K S Maan]

Hi, thank you for the fix i just tested v2 patch on Haswell platform 
with legacy mode guest where grabled screen of gitlab issue #3093 is 
fixed and regression it introduced in [Patch v1] for UEFI guest about 
romfile checksum is now ignored, so UEFI guest grub screen is now 
visible and no VBT table found issue also resolved, both Legacy\EFI 
guest i915 dmesg logs are clean now.

Thanks,
K S Maan

Re: [PATCH v2 7/7] vfio/igd: Clear saved BDSM in legacy VBIOS ROM at load time

[Cédric Le Goater]

Hello,

On 6/9/26 05:05, K S Maan wrote:
> Hi, thank you for the fix i just tested v2 patch on Haswell platform with legacy mode guest where grabled screen of gitlab issue #3093 is fixed and regression it introduced in [Patch v1] for UEFI guest about romfile checksum is now ignored, so UEFI guest grub screen is now visible and no VBT table found issue also resolved, both Legacy\EFI guest i915 dmesg logs are clean now.
Could you please reply to the cover letter email with a :

   Tested-by: YOUR NAME <your@email.addr>

Thanks,

C.

Re: [PATCH v2 0/7] vfio/igd: Fix garbled screen on IGD passthrough with legacy VBIOS

[K S Maan]

  Tested-by: K S Maan <kirandeepmaan45@gmail.com>

On 6/8/26 7:15 PM, Tomita Moeko wrote:
> This series fixes the regression that on IGD passthrough with legacy
> BIOS boot and VBIOS, the screen is garbled during BIOS POST and GRUB
> (which uses standard VGA output routines), starting from QEMU 10.0.
> Though the kernel i915 driver still works, it reports an error about
> the initial GTT programmed by VBIOS is using invalid address.
>
> i915 0000:00:02.0: [drm] *ERROR* Initial plane programming using invalid range, dma_addr=0x00000000db200000 ((null) [0x00000000baf00000-0x00000000beefffff])
>
> With the help of AI disassembling the VBIOS image dumped from host, it
> is found that the VBIOS itself implements a routine like:
>
>      uint32_t get_BDSM() {
>          static uint32_t saved = 0;
>          if (saved != 0) {
>              return saved;
>          }
>          return read_pci_config(BDSM_REG);
>      }
>
> And the saved value is not cleared after initialization. Given that IGD
> devices don't have a real ROM BAR, the VBIOS image read by default from
> host is actually the VBIOS shadow RAM region, containing host-side
> modifications like the saved BDSM value above during POST. When the
> image is executed in guest, it still uses the saved host BDSM (HPA)
> instead of the value programmed by SeaBIOS in config space (GPA). This
> address mismatch leads to the garbled screen and i915 error.
>
> The previous solution, c4c45e943e51 ("vfio/pci: Intel graphics legacy
> mode assignment"), adjusts GTT entry addresses to (addr - host BDSM +
> guest BDSM) to workaround that. But it is removed in 5aed8b0f0be2
> ("vfio/igd: Remove GTT write quirk in IO BAR 4") due to inconsistent
> values in MMIO BAR0 and IO BAR4. Considering it's unsafe to expose HPA
> to guest, a ROM quirk clearing the saved value in VBIOS image is
> introduced to fix the issue.
>
> During debugging, it is also found that IGD VBIOS ROM doesn't always
> match the actual IGD device ID, due to the fact that IGD of the same
> CPU family has multiple device IDs but shares the same ROM image.
> However, SeaBIOS checks the device ID strictly and refuses to run if
> IDs does not match. Currently only the default path, reading ROM from
> kernel patches the device ID, but the romfile path doesn't. So the ROM
> ID patching logic is also refactored in this patch series to also handle
> the romfile path.
>
> These changes are tested on Haswell platform with legacy BIOS boot, by
> K S Maan. Thanks to K S Maan for continuous help on locating and testing
> the issue!
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3093
> Reported-by: K S Maan <kirandeepmaan45@gmail.com>
>
> Changelog:
> v2:
> * New patch 2/7 to fix regression with EFI option ROMs
> * Refine logic in ROM ID and checksum patching
> * Reorder patch 4 and 5 for cleaner bisection
> * Address comments from v1
> Link: https://lore.kernel.org/all/20260603173355.36121-1-tomitamoeko@gmail.com/t
>
> Tomita Moeko (7):
>    hw/pci: Recalculate option ROM checksum before patching ID
>    hw/pci: Skip EFI option ROM in pci_patch_ids()
>    hw/pci: Introduce rom_need_patch_id flag in PCIDevice
>    hw/pci: Promote pci_patch_ids() to public pci_rom_patch_ids()
>    vfio/igd: Toggle rom_need_patch_id flag on IGD devices
>    vfio/pci: Use pci_rom_patch_ids() for IGD ROM ID patching
>    vfio/igd: Clear saved BDSM in legacy VBIOS ROM at load time
>
>   hw/pci/pci.c                |  58 ++++++++++------
>   hw/vfio/igd-stubs.c         |   5 ++
>   hw/vfio/igd.c               | 132 ++++++++++++++++++++++++++++++++++--
>   hw/vfio/pci-quirks.c        |   5 ++
>   hw/vfio/pci.c               |  33 ++-------
>   hw/vfio/pci.h               |   3 +
>   hw/vfio/trace-events        |   1 +
>   include/hw/pci/pci.h        |   3 +
>   include/hw/pci/pci_device.h |   1 +
>   9 files changed, 188 insertions(+), 53 deletions(-)
>

Re: [PATCH v2 7/7] vfio/igd: Clear saved BDSM in legacy VBIOS ROM at load time

[Cédric Le Goater]

On 6/8/26 15:45, Tomita Moeko wrote:
> IGD does not come with a ROM BAR [1], the ROM BAR read by default from
> kernel is actually the host VBIOS shadow RAM region that contains host
> modifications on boot. With AI-assisted reverse engineering on VBIOS
> binaries, it is observed that VBIOS saves BDSM register value on first
> access and uses saved value if present.
> 
> When the image is executed in guest, since there is already a saved HPA
> in VBIOS, it keeps using that value instead of the GPA programmed by
> SeaBIOS in BDSM register in PCI config space, causing VBIOS to program
> GTT entries with wrong address, resulting in garbled output in BIOS
> POST and the error below detected by i915 driver.
> 
> i915 0000:00:02.0: [drm] *ERROR* Initial plane programming using invalid range, dma_addr=0x00000000db200000 ((null) [0x00000000baf00000-0x00000000beefffff])
> 
> The previous solution, c4c45e943e51 ("vfio/pci: Intel graphics legacy
> mode assignment"), adjusts GTT entry addresses to (addr - host BDSM +
> guest BDSM) to workaround that. But it is removed in 5aed8b0f0be2
> ("vfio/igd: Remove GTT write quirk in IO BAR 4") due to inconsistent
> values in MMIO BAR0 and IO BAR4.
> 
> Considering it's unsafe to expose HPA to guest, a ROM quirk clearing
> the saved value in VBIOS image is introduced. It searches the BDSM
> accessor routine by matching a 19-byte signature anchored on the unique
> `mov $0x105e,%ax` instruction, then locate the offset of saved BDSM and
> clears it. This makes the routine fall through to the PCI config read
> on the first call inside the guest.
> 
> The quirk is invoked in vfio_pci_load_rom(), and is gated on Gen 6-9
> IGD devices with VGA access enabled and legacy (non-UEFI) PCIR code
> type in the ROM header. A new trace event vfio_pci_igd_vbios_patched
> is also introduced.
> 
> [1] 3.5.15, 4th Generation Intel Core Processor Family Datasheet Vol. 2
>      https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/4th-gen-core-family-desktop-vol-2-datasheet.pdf
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3093
> Reported-by: K S Maan <kirandeepmaan45@gmail.com>
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>   hw/vfio/igd-stubs.c  |   5 ++
>   hw/vfio/igd.c        | 110 +++++++++++++++++++++++++++++++++++++++++++
>   hw/vfio/pci-quirks.c |   5 ++
>   hw/vfio/pci.c        |   2 +
>   hw/vfio/pci.h        |   3 ++
>   hw/vfio/trace-events |   1 +
>   6 files changed, 126 insertions(+)
> 
> diff --git a/hw/vfio/igd-stubs.c b/hw/vfio/igd-stubs.c
> index f7687d9091..879a8aff56 100644
> --- a/hw/vfio/igd-stubs.c
> +++ b/hw/vfio/igd-stubs.c
> @@ -18,3 +18,8 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>   {
>       return true;
>   }
> +
> +void vfio_probe_igd_legacy_rom_quirk(VFIOPCIDevice *vdev)
> +{
> +    return;
> +}
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index 834539affb..4b49038753 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -734,3 +734,113 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>   
>       return vfio_pci_igd_config_quirk(vdev, errp);
>   }
> +
> +/*
> + * IGD ROM BAR read from kernel is actually the host VBIOS shadow RAM region,
> + * which contains host modifications. In Gen 6-9 VBIOS, the routine below is
> + * used to get BDSM value when programming the initial GTT.
> + *   xx xx xx xx           v: .long ?                  # saved value
> + *   66 53                    push  %ebx
> + *   66 2e 83 3e xx xx 00     cmpl  $0x0,%cs:v         # is saved value empty?
> + *   74 07                    je    1f                 # if zero, go compute
> + *   66 2e a1 xx xx           mov   %cs:v,%eax         # else return saved value
> + *   eb 0f                    jmp   2f
> + *   b8 5e 10              1: mov   $0x105e,%ax        # dev 00:02.0, offset 5E
> + *   e8 xx xx                 call  pci_read_cfg_word
> + *   66 c1 e0 10              shl   $0x10,%eax         # left shift 16 bits
> + *   66 2e a3 xx xx           mov   %eax,%cs:v         # save the result
> + *   66 5b                 2：pop   %ebx
> + *   c3                       ret
> + * When running the VBIOS in guest, saved value still reflects the host stolen
> + * memory base address, which is not correct in guest. So we need to patch the
> + * VBIOS to clear the saved value.
> + *
> + * The unique 19-byte starts at `cmpl $0,%cs:v` and ends at `mov $0x105e,%ax`
> + * anchors the match to the routine. Both `cs:` displacements must reference
> + * the same offset.
> + */
> +static int igd_vbios_find_saved_bdsm(const uint8_t *rom, size_t rom_size,
> +                                     uint16_t *bdsm_offset)
> +{
> +    static const uint8_t start[] = { 0x66, 0x2e, 0x83, 0x3e };
> +    static const uint8_t middle[] = { 0x00, 0x74, 0x07, 0x66, 0x2e, 0xa1 };
> +    static const uint8_t end[] = { 0xeb, 0x0f, 0xb8, 0x5e, 0x10 };
> +    uint16_t val;
> +    size_t i;
> +    bool found = false;
> +
> +    if (rom_size < 19) {
> +        return -ENOENT;
> +    }
> +
> +    for (i = 0; i + 19 <= rom_size; i++) {
> +        if (memcmp(rom + i, start, sizeof(start)) != 0 ||
> +            memcmp(rom + i + 6, middle, sizeof(middle)) != 0 ||
> +            memcmp(rom + i + 14, end, sizeof(end)) != 0) {
> +            continue;
> +        }
> +
> +        /* same saved value address? */
> +        if (rom[i + 4] != rom[i + 12] || rom[i + 5] != rom[i + 13]) {
> +            continue;
> +        }
> +
> +        if (found) {
> +            return -EEXIST;
> +        }
> +
> +        val = rom[i + 4] | ((uint16_t)rom[i + 5] << 8);
> +        if (val < rom_size) {
> +            *bdsm_offset = val;
> +            found = true;
> +        }
> +    }
> +
> +    if (!found) {
> +        return -ENOENT;
> +    }
> +
> +    return 0;
> +}
> +
> +void vfio_probe_igd_legacy_rom_quirk(VFIOPCIDevice *vdev)
> +{
> +    int ret, gen;
> +    uint16_t pcir_offset, bdsm_offset = 0;
> +    uint8_t checksum;
> +
> +    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
> +        !vfio_is_vga(vdev) || !vdev->vga) {
> +        return;
> +    }
> +
> +    /* Only Gen 6~9 devices have legacy VBIOS as Option ROM */
> +    gen = igd_gen(vdev);
> +    if (gen < 6 || gen > 9) {
> +        return;
> +    }
> +
> +    if (pci_get_word(vdev->rom) != 0xaa55) {
> +        return;
> +    }
> +
> +    /* Must be a legacy ROM */
> +    pcir_offset = pci_get_word(vdev->rom + 0x18);
> +    if (pcir_offset + 0x14 >= vdev->rom_size ||
> +        memcmp(vdev->rom + pcir_offset, "PCIR", 4) ||
> +        pci_get_byte(vdev->rom + pcir_offset + 0x14) != 0x00) {
> +        return;
> +    }
> +
> +    ret = igd_vbios_find_saved_bdsm(vdev->rom, vdev->rom_size, &bdsm_offset);
> +    if (ret < 0) {
> +        return;
> +    }
> +
> +    memset(vdev->rom + bdsm_offset, 0, sizeof(uint32_t));

May be add :

   if (bdsm_offset + sizeof(uint32_t) > vdev->rom_size) {
       return;
   }

C.


> +
> +    checksum = pci_rom_calculate_checksum(vdev->rom, vdev->rom_size);
> +    ((uint8_t *)vdev->rom)[6] = (uint8_t)-checksum;
> +
> +    trace_vfio_pci_igd_vbios_patched(vdev->vbasedev.name);> +}
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index bccf31751f..45db968681 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -1592,3 +1592,8 @@ bool vfio_add_virt_caps(VFIOPCIDevice *vdev, Error **errp)
>   
>       return true;
>   }
> +
> +void vfio_rom_quirk_setup(VFIOPCIDevice *vdev)
> +{
> +    vfio_probe_igd_legacy_rom_quirk(vdev);
> +}
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 6cbd65126e..66d6315e6f 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1088,6 +1088,8 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
>       if (pdev->rom_need_patch_id) {
>           pci_rom_patch_ids(pdev, vdev->rom, vdev->rom_size);
>       }
> +
> +    vfio_rom_quirk_setup(vdev);
>   }
>   
>   /* "Raw" read of underlying config space. */
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index c3a1f53d35..d8d6c09632 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -251,10 +251,13 @@ void vfio_bar_quirk_exit(VFIOPCIDevice *vdev, int nr);
>   void vfio_bar_quirk_finalize(VFIOPCIDevice *vdev, int nr);
>   void vfio_setup_resetfn_quirk(VFIOPCIDevice *vdev);
>   bool vfio_add_virt_caps(VFIOPCIDevice *vdev, Error **errp);
> +void vfio_rom_quirk_setup(VFIOPCIDevice *vdev);
>   void vfio_quirk_reset(VFIOPCIDevice *vdev);
>   VFIOQuirk *vfio_quirk_alloc(int nr_mem);
> +
>   void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr);
>   bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp);
> +void vfio_probe_igd_legacy_rom_quirk(VFIOPCIDevice *vdev);
>   
>   extern const PropertyInfo qdev_prop_nv_gpudirect_clique;
>   
> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
> index 2049159015..7dc334ccb3 100644
> --- a/hw/vfio/trace-events
> +++ b/hw/vfio/trace-events
> @@ -90,6 +90,7 @@ vfio_pci_igd_bar4_write(const char *name, uint32_t index, uint32_t data, uint32_
>   vfio_pci_igd_bdsm_enabled(const char *name, int size) "%s %dMB"
>   vfio_pci_igd_host_bridge_enabled(const char *name) "%s"
>   vfio_pci_igd_lpc_bridge_enabled(const char *name) "%s"
> +vfio_pci_igd_vbios_patched(const char *name) "%s"
>   
>   # listener.c
>   vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t iova_end) "iommu %s @ 0x%"PRIx64" - 0x%"PRIx64

Re: [PATCH v2 7/7] vfio/igd: Clear saved BDSM in legacy VBIOS ROM at load time

[Alex Williamson]

On Mon,  8 Jun 2026 21:45:58 +0800
Tomita Moeko <tomitamoeko@gmail.com> wrote:

> IGD does not come with a ROM BAR [1], the ROM BAR read by default from
> kernel is actually the host VBIOS shadow RAM region that contains host
> modifications on boot. With AI-assisted reverse engineering on VBIOS
> binaries, it is observed that VBIOS saves BDSM register value on first
> access and uses saved value if present.
> 
> When the image is executed in guest, since there is already a saved HPA
> in VBIOS, it keeps using that value instead of the GPA programmed by
> SeaBIOS in BDSM register in PCI config space, causing VBIOS to program
> GTT entries with wrong address, resulting in garbled output in BIOS
> POST and the error below detected by i915 driver.
> 
> i915 0000:00:02.0: [drm] *ERROR* Initial plane programming using invalid range, dma_addr=0x00000000db200000 ((null) [0x00000000baf00000-0x00000000beefffff])
> 
> The previous solution, c4c45e943e51 ("vfio/pci: Intel graphics legacy
> mode assignment"), adjusts GTT entry addresses to (addr - host BDSM +
> guest BDSM) to workaround that. But it is removed in 5aed8b0f0be2
> ("vfio/igd: Remove GTT write quirk in IO BAR 4") due to inconsistent
> values in MMIO BAR0 and IO BAR4.
> 
> Considering it's unsafe to expose HPA to guest, a ROM quirk clearing
> the saved value in VBIOS image is introduced. It searches the BDSM
> accessor routine by matching a 19-byte signature anchored on the unique
> `mov $0x105e,%ax` instruction, then locate the offset of saved BDSM and
> clears it. This makes the routine fall through to the PCI config read
> on the first call inside the guest.
> 
> The quirk is invoked in vfio_pci_load_rom(), and is gated on Gen 6-9
> IGD devices with VGA access enabled and legacy (non-UEFI) PCIR code
> type in the ROM header. A new trace event vfio_pci_igd_vbios_patched
> is also introduced.
> 
> [1] 3.5.15, 4th Generation Intel Core Processor Family Datasheet Vol. 2
>     https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/4th-gen-core-family-desktop-vol-2-datasheet.pdf
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3093
> Reported-by: K S Maan <kirandeepmaan45@gmail.com>
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/vfio/igd-stubs.c  |   5 ++
>  hw/vfio/igd.c        | 110 +++++++++++++++++++++++++++++++++++++++++++
>  hw/vfio/pci-quirks.c |   5 ++
>  hw/vfio/pci.c        |   2 +
>  hw/vfio/pci.h        |   3 ++
>  hw/vfio/trace-events |   1 +
>  6 files changed, 126 insertions(+)
> 
> diff --git a/hw/vfio/igd-stubs.c b/hw/vfio/igd-stubs.c
> index f7687d9091..879a8aff56 100644
> --- a/hw/vfio/igd-stubs.c
> +++ b/hw/vfio/igd-stubs.c
> @@ -18,3 +18,8 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>  {
>      return true;
>  }
> +
> +void vfio_probe_igd_legacy_rom_quirk(VFIOPCIDevice *vdev)
> +{
> +    return;
> +}
> diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> index 834539affb..4b49038753 100644
> --- a/hw/vfio/igd.c
> +++ b/hw/vfio/igd.c
> @@ -734,3 +734,113 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
>  
>      return vfio_pci_igd_config_quirk(vdev, errp);
>  }
> +
> +/*
> + * IGD ROM BAR read from kernel is actually the host VBIOS shadow RAM region,
> + * which contains host modifications. In Gen 6-9 VBIOS, the routine below is
> + * used to get BDSM value when programming the initial GTT.
> + *   xx xx xx xx           v: .long ?                  # saved value
> + *   66 53                    push  %ebx
> + *   66 2e 83 3e xx xx 00     cmpl  $0x0,%cs:v         # is saved value empty?
> + *   74 07                    je    1f                 # if zero, go compute
> + *   66 2e a1 xx xx           mov   %cs:v,%eax         # else return saved value
> + *   eb 0f                    jmp   2f
> + *   b8 5e 10              1: mov   $0x105e,%ax        # dev 00:02.0, offset 5E
> + *   e8 xx xx                 call  pci_read_cfg_word
> + *   66 c1 e0 10              shl   $0x10,%eax         # left shift 16 bits
> + *   66 2e a3 xx xx           mov   %eax,%cs:v         # save the result
> + *   66 5b                 2：pop   %ebx
> + *   c3                       ret
> + * When running the VBIOS in guest, saved value still reflects the host stolen
> + * memory base address, which is not correct in guest. So we need to patch the
> + * VBIOS to clear the saved value.
> + *
> + * The unique 19-byte starts at `cmpl $0,%cs:v` and ends at `mov $0x105e,%ax`
> + * anchors the match to the routine. Both `cs:` displacements must reference
> + * the same offset.
> + */
> +static int igd_vbios_find_saved_bdsm(const uint8_t *rom, size_t rom_size,
> +                                     uint16_t *bdsm_offset)
> +{
> +    static const uint8_t start[] = { 0x66, 0x2e, 0x83, 0x3e };
> +    static const uint8_t middle[] = { 0x00, 0x74, 0x07, 0x66, 0x2e, 0xa1 };
> +    static const uint8_t end[] = { 0xeb, 0x0f, 0xb8, 0x5e, 0x10 };
> +    uint16_t val;
> +    size_t i;
> +    bool found = false;
> +
> +    if (rom_size < 19) {
> +        return -ENOENT;
> +    }
> +
> +    for (i = 0; i + 19 <= rom_size; i++) {
> +        if (memcmp(rom + i, start, sizeof(start)) != 0 ||
> +            memcmp(rom + i + 6, middle, sizeof(middle)) != 0 ||
> +            memcmp(rom + i + 14, end, sizeof(end)) != 0) {
> +            continue;
> +        }
> +
> +        /* same saved value address? */
> +        if (rom[i + 4] != rom[i + 12] || rom[i + 5] != rom[i + 13]) {
> +            continue;
> +        }
> +
> +        if (found) {
> +            return -EEXIST;
> +        }
> +
> +        val = rom[i + 4] | ((uint16_t)rom[i + 5] << 8);
> +        if (val < rom_size) {

We write sizeof(uint32_t) to this offset, so this should be:

        if (val + sizeof(uint32_t) < rom_size) {

> +            *bdsm_offset = val;
> +            found = true;
> +        }
> +    }
> +
> +    if (!found) {
> +        return -ENOENT;
> +    }
> +
> +    return 0;
> +}
> +
> +void vfio_probe_igd_legacy_rom_quirk(VFIOPCIDevice *vdev)
> +{
> +    int ret, gen;
> +    uint16_t pcir_offset, bdsm_offset = 0;
> +    uint8_t checksum;
> +
> +    if (!vfio_pci_is(vdev, PCI_VENDOR_ID_INTEL, PCI_ANY_ID) ||
> +        !vfio_is_vga(vdev) || !vdev->vga) {
> +        return;
> +    }
> +
> +    /* Only Gen 6~9 devices have legacy VBIOS as Option ROM */
> +    gen = igd_gen(vdev);
> +    if (gen < 6 || gen > 9) {
> +        return;
> +    }
> +
> +    if (pci_get_word(vdev->rom) != 0xaa55) {
> +        return;
> +    }
> +
> +    /* Must be a legacy ROM */
> +    pcir_offset = pci_get_word(vdev->rom + 0x18);
> +    if (pcir_offset + 0x14 >= vdev->rom_size ||
> +        memcmp(vdev->rom + pcir_offset, "PCIR", 4) ||
> +        pci_get_byte(vdev->rom + pcir_offset + 0x14) != 0x00) {
> +        return;
> +    }
> +
> +    ret = igd_vbios_find_saved_bdsm(vdev->rom, vdev->rom_size, &bdsm_offset);
> +    if (ret < 0) {
> +        return;
> +    }
> +
> +    memset(vdev->rom + bdsm_offset, 0, sizeof(uint32_t));
> +
> +    checksum = pci_rom_calculate_checksum(vdev->rom, vdev->rom_size);
> +    ((uint8_t *)vdev->rom)[6] = (uint8_t)-checksum;

This gets updated to:

    ((uint8_t *)vdev->rom)[6] -= checksum;

if we take the earlier suggestion.  Thanks,

Alex

> +
> +    trace_vfio_pci_igd_vbios_patched(vdev->vbasedev.name);
> +}
> diff --git a/hw/vfio/pci-quirks.c b/hw/vfio/pci-quirks.c
> index bccf31751f..45db968681 100644
> --- a/hw/vfio/pci-quirks.c
> +++ b/hw/vfio/pci-quirks.c
> @@ -1592,3 +1592,8 @@ bool vfio_add_virt_caps(VFIOPCIDevice *vdev, Error **errp)
>  
>      return true;
>  }
> +
> +void vfio_rom_quirk_setup(VFIOPCIDevice *vdev)
> +{
> +    vfio_probe_igd_legacy_rom_quirk(vdev);
> +}
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 6cbd65126e..66d6315e6f 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1088,6 +1088,8 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
>      if (pdev->rom_need_patch_id) {
>          pci_rom_patch_ids(pdev, vdev->rom, vdev->rom_size);
>      }
> +
> +    vfio_rom_quirk_setup(vdev);
>  }
>  
>  /* "Raw" read of underlying config space. */
> diff --git a/hw/vfio/pci.h b/hw/vfio/pci.h
> index c3a1f53d35..d8d6c09632 100644
> --- a/hw/vfio/pci.h
> +++ b/hw/vfio/pci.h
> @@ -251,10 +251,13 @@ void vfio_bar_quirk_exit(VFIOPCIDevice *vdev, int nr);
>  void vfio_bar_quirk_finalize(VFIOPCIDevice *vdev, int nr);
>  void vfio_setup_resetfn_quirk(VFIOPCIDevice *vdev);
>  bool vfio_add_virt_caps(VFIOPCIDevice *vdev, Error **errp);
> +void vfio_rom_quirk_setup(VFIOPCIDevice *vdev);
>  void vfio_quirk_reset(VFIOPCIDevice *vdev);
>  VFIOQuirk *vfio_quirk_alloc(int nr_mem);
> +
>  void vfio_probe_igd_bar0_quirk(VFIOPCIDevice *vdev, int nr);
>  bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp);
> +void vfio_probe_igd_legacy_rom_quirk(VFIOPCIDevice *vdev);
>  
>  extern const PropertyInfo qdev_prop_nv_gpudirect_clique;
>  
> diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
> index 2049159015..7dc334ccb3 100644
> --- a/hw/vfio/trace-events
> +++ b/hw/vfio/trace-events
> @@ -90,6 +90,7 @@ vfio_pci_igd_bar4_write(const char *name, uint32_t index, uint32_t data, uint32_
>  vfio_pci_igd_bdsm_enabled(const char *name, int size) "%s %dMB"
>  vfio_pci_igd_host_bridge_enabled(const char *name) "%s"
>  vfio_pci_igd_lpc_bridge_enabled(const char *name) "%s"
> +vfio_pci_igd_vbios_patched(const char *name) "%s"
>  
>  # listener.c
>  vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t iova_end) "iommu %s @ 0x%"PRIx64" - 0x%"PRIx64

Re: [PATCH v2 2/7] hw/pci: Skip EFI option ROM in pci_patch_ids()

[Alex Williamson]

On Mon,  8 Jun 2026 21:45:53 +0800
Tomita Moeko <tomitamoeko@gmail.com> wrote:

> pci_patch_ids() patches the checksum at the reserved 0x06 byte, but
> for EFI option ROMs the 32 bits at 0x04 are the EFI signature and
> must be 0x00000EF1. Since OVMF does not check vendor/device IDs in
> the PCIR header or the checksum, skip patching for EFI ROMs.
> 
> Reported-by: K S Maan <kirandeepmaan45@gmail.com>
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/pci/pci.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 742917f79d..eb10e586d5 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2521,6 +2521,11 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size)
>          return;
>      }
>  
> +    /* OVMF won't check IDs in PCIR header, skip EFI roms */
> +    if (pci_get_byte(ptr + pcir_offset + 0x14) == 0x03) {
> +        return;
> +    }
> +

Just above this we have the sanity testing:

    if (pcir_offset + 8 >= size || memcmp(ptr + pcir_offset, "PCIR", 4)) {
        trace_pci_bad_pcir_offset(pcir_offset);
        return;
    }

+8 covers the vendor and device IDs, but now we're reaching deeper and
should extend the validation to match.  s/8/0x14/  Thanks,

Alex

>      vendor_id = pci_get_word(pdev->config + PCI_VENDOR_ID);
>      device_id = pci_get_word(pdev->config + PCI_DEVICE_ID);
>      rom_vendor_id = pci_get_word(ptr + pcir_offset + 4);

Re: [PATCH v2 1/7] hw/pci: Recalculate option ROM checksum before patching ID

[Alex Williamson]

On Mon,  8 Jun 2026 21:45:52 +0800
Tomita Moeko <tomitamoeko@gmail.com> wrote:

> pci_patch_ids() only adjusts checksum based on the new IDs. For an
> option ROM with invalid checksum, the patched one will still have
> an invalid checksum. Always calculate the checksum and patch it if
> necessary to ensure the option ROM is valid.
> 
> This is intended for fixing the romfile used in IGD passthrough as
> multiple IGD devices share the same rom with possible non-matching
> device ID, and its checksum is known to be bogus [1].
> 
> A helper function pci_rom_calculate_checksum() is added and exported
> for reusing in IGD-specific quirk later.
> 
> [1] hw/vfio/pci.c:1090
> 
> Reported-by: K S Maan <kirandeepmaan45@gmail.com>
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> ---
>  hw/pci/pci.c         | 35 ++++++++++++++++++++++++++---------
>  include/hw/pci/pci.h |  2 ++
>  2 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index cec065d108..742917f79d 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2479,6 +2479,21 @@ static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset)
>      return found;
>  }
>  
> +uint8_t pci_rom_calculate_checksum(const uint8_t *ptr, uint32_t size)
> +{
> +    uint8_t checksum = 0;
> +    uint32_t i;
> +
> +    for (i = 0; i < size; i++) {
> +        if (i == 6) {
> +            continue;
> +        }

If we remove this continue branch...

> +        checksum += ptr[i];
> +    }
> +
> +    return checksum;
> +}
> +
>  /* Patch the PCI vendor and device ids in a PCI rom image if necessary.
>     This is needed for an option rom which is used for more than one device. */
>  static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size)
> @@ -2514,25 +2529,27 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size)
>      trace_pci_rom_and_pci_ids(pdev->romfile, vendor_id, device_id,
>                                rom_vendor_id, rom_device_id);
>  
> -    checksum = ptr[6];
> +    /* In case the checksum is bogus */
> +    checksum = pci_rom_calculate_checksum(ptr, size);
>  
>      if (vendor_id != rom_vendor_id) {
>          /* Patch vendor id and checksum (at offset 6 for etherboot roms). */
> -        checksum += (uint8_t)rom_vendor_id + (uint8_t)(rom_vendor_id >> 8);
> -        checksum -= (uint8_t)vendor_id + (uint8_t)(vendor_id >> 8);
> -        trace_pci_rom_checksum_change(ptr[6], checksum);
> -        ptr[6] = checksum;
> +        checksum += (uint8_t)vendor_id + (uint8_t)(vendor_id >> 8);
> +        checksum -= (uint8_t)rom_vendor_id + (uint8_t)(rom_vendor_id >> 8);
>          pci_set_word(ptr + pcir_offset + 4, vendor_id);
>      }
>  
>      if (device_id != rom_device_id) {
>          /* Patch device id and checksum (at offset 6 for etherboot roms). */
> -        checksum += (uint8_t)rom_device_id + (uint8_t)(rom_device_id >> 8);
> -        checksum -= (uint8_t)device_id + (uint8_t)(device_id >> 8);
> -        trace_pci_rom_checksum_change(ptr[6], checksum);
> -        ptr[6] = checksum;
> +        checksum += (uint8_t)device_id + (uint8_t)(device_id >> 8);
> +        checksum -= (uint8_t)rom_device_id + (uint8_t)(rom_device_id >> 8);
>          pci_set_word(ptr + pcir_offset + 6, device_id);
>      }
> +
> +    if (ptr[6] != (uint8_t)-checksum) {
> +        trace_pci_rom_checksum_change(ptr[6], (uint8_t)-checksum);
> +        ptr[6] = (uint8_t)-checksum;
> +    }

Then this just becomes:

    if (checksum) {
        trace_pci_rom_checksum_change(ptr[6], ptr[6] - checksum);
        ptr[6] -= checksum;
    }

The result is the same, but this avoids the uint8_t casts where
checksum is promoted to an int for comparison.

Patch 7 would require an equivalent change:

  -    ((uint8_t *)vdev->rom)[6] = (uint8_t)-checksum;
  +    ((uint8_t *)vdev->rom)[6] -= checksum;

Minor change, slightly better form.  Thanks,

Alex

Re: [PATCH v2 7/7] vfio/igd: Clear saved BDSM in legacy VBIOS ROM at load time

[Alex Williamson]

On Tue, 9 Jun 2026 09:35:54 -0600
Alex Williamson <alex@shazbot.org> wrote:

> On Mon,  8 Jun 2026 21:45:58 +0800
> Tomita Moeko <tomitamoeko@gmail.com> wrote:
> 
> > IGD does not come with a ROM BAR [1], the ROM BAR read by default from
> > kernel is actually the host VBIOS shadow RAM region that contains host
> > modifications on boot. With AI-assisted reverse engineering on VBIOS
> > binaries, it is observed that VBIOS saves BDSM register value on first
> > access and uses saved value if present.
> > 
> > When the image is executed in guest, since there is already a saved HPA
> > in VBIOS, it keeps using that value instead of the GPA programmed by
> > SeaBIOS in BDSM register in PCI config space, causing VBIOS to program
> > GTT entries with wrong address, resulting in garbled output in BIOS
> > POST and the error below detected by i915 driver.
> > 
> > i915 0000:00:02.0: [drm] *ERROR* Initial plane programming using invalid range, dma_addr=0x00000000db200000 ((null) [0x00000000baf00000-0x00000000beefffff])
> > 
> > The previous solution, c4c45e943e51 ("vfio/pci: Intel graphics legacy
> > mode assignment"), adjusts GTT entry addresses to (addr - host BDSM +
> > guest BDSM) to workaround that. But it is removed in 5aed8b0f0be2
> > ("vfio/igd: Remove GTT write quirk in IO BAR 4") due to inconsistent
> > values in MMIO BAR0 and IO BAR4.
> > 
> > Considering it's unsafe to expose HPA to guest, a ROM quirk clearing
> > the saved value in VBIOS image is introduced. It searches the BDSM
> > accessor routine by matching a 19-byte signature anchored on the unique
> > `mov $0x105e,%ax` instruction, then locate the offset of saved BDSM and
> > clears it. This makes the routine fall through to the PCI config read
> > on the first call inside the guest.
> > 
> > The quirk is invoked in vfio_pci_load_rom(), and is gated on Gen 6-9
> > IGD devices with VGA access enabled and legacy (non-UEFI) PCIR code
> > type in the ROM header. A new trace event vfio_pci_igd_vbios_patched
> > is also introduced.
> > 
> > [1] 3.5.15, 4th Generation Intel Core Processor Family Datasheet Vol. 2
> >     https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/4th-gen-core-family-desktop-vol-2-datasheet.pdf
> > 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3093
> > Reported-by: K S Maan <kirandeepmaan45@gmail.com>
> > Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>
> > ---
> >  hw/vfio/igd-stubs.c  |   5 ++
> >  hw/vfio/igd.c        | 110 +++++++++++++++++++++++++++++++++++++++++++
> >  hw/vfio/pci-quirks.c |   5 ++
> >  hw/vfio/pci.c        |   2 +
> >  hw/vfio/pci.h        |   3 ++
> >  hw/vfio/trace-events |   1 +
> >  6 files changed, 126 insertions(+)
> > 
> > diff --git a/hw/vfio/igd-stubs.c b/hw/vfio/igd-stubs.c
> > index f7687d9091..879a8aff56 100644
> > --- a/hw/vfio/igd-stubs.c
> > +++ b/hw/vfio/igd-stubs.c
> > @@ -18,3 +18,8 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
> >  {
> >      return true;
> >  }
> > +
> > +void vfio_probe_igd_legacy_rom_quirk(VFIOPCIDevice *vdev)
> > +{
> > +    return;
> > +}
> > diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
> > index 834539affb..4b49038753 100644
> > --- a/hw/vfio/igd.c
> > +++ b/hw/vfio/igd.c
> > @@ -734,3 +734,113 @@ bool vfio_probe_igd_config_quirk(VFIOPCIDevice *vdev, Error **errp)
> >  
> >      return vfio_pci_igd_config_quirk(vdev, errp);
> >  }
> > +
> > +/*
> > + * IGD ROM BAR read from kernel is actually the host VBIOS shadow RAM region,
> > + * which contains host modifications. In Gen 6-9 VBIOS, the routine below is
> > + * used to get BDSM value when programming the initial GTT.
> > + *   xx xx xx xx           v: .long ?                  # saved value
> > + *   66 53                    push  %ebx
> > + *   66 2e 83 3e xx xx 00     cmpl  $0x0,%cs:v         # is saved value empty?
> > + *   74 07                    je    1f                 # if zero, go compute
> > + *   66 2e a1 xx xx           mov   %cs:v,%eax         # else return saved value
> > + *   eb 0f                    jmp   2f
> > + *   b8 5e 10              1: mov   $0x105e,%ax        # dev 00:02.0, offset 5E
> > + *   e8 xx xx                 call  pci_read_cfg_word
> > + *   66 c1 e0 10              shl   $0x10,%eax         # left shift 16 bits
> > + *   66 2e a3 xx xx           mov   %eax,%cs:v         # save the result
> > + *   66 5b                 2：pop   %ebx
> > + *   c3                       ret
> > + * When running the VBIOS in guest, saved value still reflects the host stolen
> > + * memory base address, which is not correct in guest. So we need to patch the
> > + * VBIOS to clear the saved value.
> > + *
> > + * The unique 19-byte starts at `cmpl $0,%cs:v` and ends at `mov $0x105e,%ax`
> > + * anchors the match to the routine. Both `cs:` displacements must reference
> > + * the same offset.
> > + */
> > +static int igd_vbios_find_saved_bdsm(const uint8_t *rom, size_t rom_size,
> > +                                     uint16_t *bdsm_offset)
> > +{
> > +    static const uint8_t start[] = { 0x66, 0x2e, 0x83, 0x3e };
> > +    static const uint8_t middle[] = { 0x00, 0x74, 0x07, 0x66, 0x2e, 0xa1 };
> > +    static const uint8_t end[] = { 0xeb, 0x0f, 0xb8, 0x5e, 0x10 };
> > +    uint16_t val;
> > +    size_t i;
> > +    bool found = false;
> > +
> > +    if (rom_size < 19) {
> > +        return -ENOENT;
> > +    }
> > +
> > +    for (i = 0; i + 19 <= rom_size; i++) {
> > +        if (memcmp(rom + i, start, sizeof(start)) != 0 ||
> > +            memcmp(rom + i + 6, middle, sizeof(middle)) != 0 ||
> > +            memcmp(rom + i + 14, end, sizeof(end)) != 0) {
> > +            continue;
> > +        }
> > +
> > +        /* same saved value address? */
> > +        if (rom[i + 4] != rom[i + 12] || rom[i + 5] != rom[i + 13]) {
> > +            continue;
> > +        }
> > +
> > +        if (found) {
> > +            return -EEXIST;
> > +        }
> > +
> > +        val = rom[i + 4] | ((uint16_t)rom[i + 5] << 8);
> > +        if (val < rom_size) {  
> 
> We write sizeof(uint32_t) to this offset, so this should be:
> 
>         if (val + sizeof(uint32_t) < rom_size) {

Oops, the comparison should have become <=.  Thanks,

Alex