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

Reply via email to