On Sat, 12 Feb 2022 at 13:42, BALATON Zoltan <bala...@eik.bme.hu> wrote: > By the way the corresponding member in struct PIIXState in > include/hw/southbridge/piix.h has a comment saying: > > /* This member isn't used. Just for save/load compatibility */ > int32_t pci_irq_levels_vmstate[PIIX_NUM_PIRQS]; > > and only seems to be filled in piix3_pre_save() but never used. So what's > the point of this then? Maybe piix3 also uses a bitmap to store these > levels instead? There's a uint64_t pic_levels member above the unused > array but I haven't checked the implementation.
I think what has happened here is that originally piix3 used the same implementation piix4 currently does -- where it stores locally the value of the (incoming) PCI IRQ levels, and then when it wants to know the value of the (outgoing) PIC IRQ levels it loops round to effectively OR together all the PCI IRQ levels for those PCI IRQs which are configured to that particular PIC interrupt. Then in commit e735b55a8c11 (in 2011) piix3 was changed to call pci_bus_get_irq_level() to get the value of a PCI IRQ rather than looking at its local cache of that information in the pci_irq_levels[] array. This is the source of the "save/load compatibility" thing -- before doing a vmsave piix3_pre_save() fills in that field so that it appears in the outbound data stream and thus a migration from a new QEMU to an old pre-e735b55a8c11 QEMU will still work. (This was important at the time, but could probably be cleaned up now.) The next commit after that one is ab431c283e7055bcd, which is an optimization that fixes the equivalent of the "XXX: optimize" marker in the gt64120_pci_set_irq()/piix4 code -- this does something slightly more complicated involving the pic_levels field, in order to avoid having to do the "loop over all the PCI IRQ levels" whenever it needs to set/reset a PIC interrupt. We could pick up one or both (or none!) of these two changes for the piix4 code. (If we're breaking migration compat anyway we wouldn't need to include the save-load compat part of the first change.) thanks -- PMM
