On Thu, 4 Jun 2026 01:33:53 +0800 Tomita Moeko <[email protected]> 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 <[email protected]> > Signed-off-by: Tomita Moeko <[email protected]> > Tested-by: K S Maan <[email protected]> > --- > 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
