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

[Alex Williamson <alex@shazbot.org>]

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