Re: [PATCH 3/3] isa/piix4: Resolve global variables

[Philippe Mathieu-Daudé]

On 14/1/22 14:36, Peter Maydell wrote:
> On Wed, 12 Jan 2022 at 22:02, Bernhard Beschow <shentey@gmail.com> wrote:
> > Now that piix4_set_irq's opaque parameter references own PIIX4State,
> > piix4_dev becomes redundant and pci_irq_levels can be moved into PIIX4State.
> >
> > Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> > ---
> >   hw/isa/piix4.c                | 22 +++++++++-------------
> >   include/hw/southbridge/piix.h |  2 --
> >   2 files changed, 9 insertions(+), 15 deletions(-)
> >
> > diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
> > index a31e9714cf..964e09cf7f 100644
> > --- a/hw/isa/piix4.c
> > +++ b/hw/isa/piix4.c
> > @@ -39,14 +39,14 @@
> >   #include "sysemu/runstate.h"
> >   #include "qom/object.h"
> >
> > -PCIDevice *piix4_dev;
> > -
> >   struct PIIX4State {
> >       PCIDevice dev;
> >       qemu_irq cpu_intr;
> >       qemu_irq *isa;
> >       qemu_irq i8259[ISA_NUM_IRQS];
> >
> > +    int pci_irq_levels[PIIX_NUM_PIRQS];
> > +
>
> I wondered how we were migrating this state, and the answer
> seems to be that we aren't (and weren't before, when it was
> a global variable, so this is a pre-existing bug).

Indeed the migrated VM starts with PCI IRQ levels zeroed.

> Does the malta platform support migration save/load?

Maybe a "best effort" support, but not versioned machines.

> We should probably add this field to the vmstate struct
> (which will be a migration compatibility break, which is OK
> as the malta board isn't versioned.)

Yeah good catch.

Bernhard, do you mind adding it?

Regards,

Phil.