On Thu, 4 Jun 2026 01:33:52 +0800
Tomita Moeko <tomitamoeko@gmail.com> wrote:
> 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>
> Tested-by: K S Maan <kirandeepmaan45@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);
> }
> }
>
Wouldn't this provide cleaner bisection if we call the helper here
unconditionally and make it conditional in the next patch? Or just
re-order patches 4 & 5 so we set the flag before we make this
conditional on the flag. Thanks,
Alex