Re: [PATCH v3 1/7] hw/pci: Recalculate option ROM checksum before patching ID

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Jun 17, 2026 at 06:06:38PM +0800, Tomita Moeko wrote:
> pci_patch_ids() only adjusts checksum based on the new IDs. For an
> option ROM with invalid checksum, the patched one will still have
> an invalid checksum. Always calculate the checksum and patch it if
> necessary to ensure the option ROM is valid.
> 
> This is intended for fixing the romfile used in IGD passthrough as
> multiple IGD devices share the same rom with possible non-matching
> device ID, and its checksum is known to be bogus [1].
> 
> A helper function pci_rom_calculate_checksum() is added and exported
> for reusing in IGD-specific quirk later.
> 
> [1] hw/vfio/pci.c:1090
> 
> Reported-by: K S Maan <kirandeepmaan45@gmail.com>
> Signed-off-by: Tomita Moeko <tomitamoeko@gmail.com>

This seems very aggressive.
Why not recalculate in vfio then?
It has a chance to do a targeted fix.

> ---
>  hw/pci/pci.c         | 32 +++++++++++++++++++++++---------
>  include/hw/pci/pci.h |  2 ++
>  2 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index cec065d108..601d65aef3 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2479,6 +2479,18 @@ static uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offset)
>      return found;
>  }
>  
> +uint8_t pci_rom_calculate_checksum(const uint8_t *ptr, uint32_t size)
> +{
> +    uint8_t checksum = 0;
> +    uint32_t i;
> +
> +    for (i = 0; i < size; i++) {
> +        checksum += ptr[i];
> +    }
> +
> +    return checksum;
> +}
> +
>  /* Patch the PCI vendor and device ids in a PCI rom image if necessary.
>     This is needed for an option rom which is used for more than one device. */
>  static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size)
> @@ -2514,25 +2526,27 @@ static void pci_patch_ids(PCIDevice *pdev, uint8_t *ptr, uint32_t size)
>      trace_pci_rom_and_pci_ids(pdev->romfile, vendor_id, device_id,
>                                rom_vendor_id, rom_device_id);
>  
> -    checksum = ptr[6];
> +    /* In case the checksum is bogus */
> +    checksum = pci_rom_calculate_checksum(ptr, size);
>  
>      if (vendor_id != rom_vendor_id) {
>          /* Patch vendor id and checksum (at offset 6 for etherboot roms). */
> -        checksum += (uint8_t)rom_vendor_id + (uint8_t)(rom_vendor_id >> 8);
> -        checksum -= (uint8_t)vendor_id + (uint8_t)(vendor_id >> 8);
> -        trace_pci_rom_checksum_change(ptr[6], checksum);
> -        ptr[6] = checksum;
> +        checksum += (uint8_t)vendor_id + (uint8_t)(vendor_id >> 8);
> +        checksum -= (uint8_t)rom_vendor_id + (uint8_t)(rom_vendor_id >> 8);
>          pci_set_word(ptr + pcir_offset + 4, vendor_id);
>      }
>  
>      if (device_id != rom_device_id) {
>          /* Patch device id and checksum (at offset 6 for etherboot roms). */
> -        checksum += (uint8_t)rom_device_id + (uint8_t)(rom_device_id >> 8);
> -        checksum -= (uint8_t)device_id + (uint8_t)(device_id >> 8);
> -        trace_pci_rom_checksum_change(ptr[6], checksum);
> -        ptr[6] = checksum;
> +        checksum += (uint8_t)device_id + (uint8_t)(device_id >> 8);
> +        checksum -= (uint8_t)rom_device_id + (uint8_t)(rom_device_id >> 8);
>          pci_set_word(ptr + pcir_offset + 6, device_id);
>      }
> +
> +    if (checksum) {
> +        trace_pci_rom_checksum_change(ptr[6], ptr[6] - checksum);
> +        ptr[6] -= checksum;
> +    }
>  }
>  
>  /* Add an option rom for the device */
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 5b179091de..2d8a4ad0eb 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -1103,4 +1103,6 @@ void pci_set_enabled(PCIDevice *pci_dev, bool state);
>  void pci_set_power(PCIDevice *pci_dev, bool state);
>  int pci_pm_init(PCIDevice *pci_dev, uint8_t offset, Error **errp);
>  
> +uint8_t pci_rom_calculate_checksum(const uint8_t *ptr, uint32_t size);
> +
>  #endif
> -- 
> 2.53.0