On Thu, 4 Jun 2026 01:33:49 +0800 Tomita Moeko <[email protected]> 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 <[email protected]> > Signed-off-by: Tomita Moeko <[email protected]> > Tested-by: K S Maan <[email protected]> > --- > hw/pci/pci.c | 22 +++++++++++++++++++++- > include/hw/pci/pci.h | 2 ++ > 2 files changed, 23 insertions(+), 1 deletion(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 4298adf5a0..043fef1954 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -2472,6 +2472,21 @@ static uint8_t pci_find_capability_at_offset(PCIDevice > *pdev, uint8_t offset) > return found; > } > > +uint8_t pci_rom_calculate_checksum(uint8_t *ptr, uint32_t size) > +{ > + uint8_t checksum = 0; > + uint8_t orig_checksum = ptr[6]; > + uint32_t i; > + > + ptr[6] = 0; > + for (i = 0; i < size; i++) { > + checksum += ptr[i]; > + } > + ptr[6] = orig_checksum; > + > + return -checksum; > +} This seems to compose better below if we just have the function return the actual checksum without excluding the current fixup and move the fixup to the end, ex: checksum = pci_rom_calculate_csum(ptr, size); if (vendor_id != rom_vendor_id) { 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) { 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], (uint8_t)(ptr[6] - checksum)); ptr[6] -= checksum; } Just a style/consistency suggestion, not a functional difference. Thanks, Alex
