Re: [PULL 01/13] vfio/igd: Clear saved BDSM in legacy VBIOS ROM at load time

On 7/24/26 10:33, Michael Tokarev wrote:
> On 7/21/26 20:05, Cédric Le Goater wrote:
>> From: Tomita Moeko <[email protected]>
>>
>> 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.
>>
>> Since it was a value latched into the VBIOS that breaks virtualization
>> (QEMU does not map the GTT at the same address in the VM), 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 locates 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.
>>
>> [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
>>
>> Fixes: 5aed8b0f0be2 ("vfio/igd: Remove GTT write quirk in IO BAR 4")
>> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3093
>> Reported-by: K S Maan <[email protected]>
>> Cc: [email protected]
>> Signed-off-by: Tomita Moeko <[email protected]>
>> Reviewed-by: Alex Williamson <[email protected]>
>> Link: https://lore.kernel.org/qemu-devel/[email protected]
>> Signed-off-by: Cédric Le Goater <[email protected]>
> 
> 
>> ---
>>   hw/vfio/pci.h        |   3 ++
>>   hw/vfio/igd-stubs.c  |   5 ++
>>   hw/vfio/igd.c        | 115 +++++++++++++++++++++++++++++++++++++++++++
>>   hw/vfio/pci-quirks.c |   5 ++
>>   hw/vfio/pci.c        |   2 +
>>   hw/vfio/trace-events |   1 +
>>   6 files changed, 131 insertions(+)
> 
> I'm picking this up for currently active stable qemu series.  Since this
> change touches several files, including new file since last release
> (hw/vfio/igd-stubs.c), I'm also picking a few other changes to the
> stable series, namely, for 11.0.x:
> 
> v11.0.0-2776-gc822ecd8066 vfio/pci: Initialize rom_read_failed in vfio_pci_load_rom()
>    this is a little bugfix by its own, so it's okay to pick up
> 
> v11.0.0-3-g55e25958aa1 hw/vfio/pci-quirks.c: remove CONFIG_VFIO_IGD,
>    Add stubs for needed functions.  This changes quite a few things,
>    but not much.  I can adjust this change (Clear saved BDSM) for the
>    context before CONFIG_VFIO_IGD removal, but it feels like it would
>    be better to pick up CONFIG_VFIO_IGD removal too, to make the branches
>    more similar.
> 
> For 10.0.x, things are a bit more complex here.  In particular, 10.0.x
> lacks
> 
>   v10.0.0-290-ge218ccf0c94 vfio: Rename VFIODevice related services
>   v10.0.0-756-g2e27becf17b vfio: consistently handle return value for helpers
>   v10.2.0-2365-g988499515da "hw/display: Build stubs once"
> 
> and these are more troublesome to pick up, including "CONFIG_VFIO_IGD
> removal", so I'm picking up "Initialize rom_read_failed", and adjusting
> "Clear saved BDSM" for the lack of "CONFIG_VFIO_IGD removal" (it's
> stright-forward, with #ifdef CONFIG_VFIO_IGD instead of empty stub).
> 
> I'm not very comfortable with the different approach to 10.0.x and 11.0.x,
> maybe I should omit "CONFIG_VFIO_IGD removal" from 11.0.x too.
> 
> What do you think?

For the stable branches, I would take the fixes only :

  c822ecd80661 ("vfio/pci: Initialize rom_read_failed in vfio_pci_load_rom()")
  256229e8f9b9 ("vfio/igd: Clear saved BDSM in legacy VBIOS ROM at load time")

and commit 55e25958aa1a ("hw/vfio/pci-quirks.c: remove CONFIG_VFIO_IGD") is
not one.

I would skip 55e25958aa1a and adapt the BDSM patch with #ifdef CONFIG_VFIO_IGD
in 10.0.x and 11.0.x.

How's that ?

C.