Re: [PATCH for-7.1] hw/mips/malta: turn off x86 specific features of PIIX4_PM

[Peter Maydell <peter.maydell@linaro.org>]

On Thu, 28 Jul 2022 at 16:09, Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> * Igor Mammedov (imammedo@redhat.com) wrote:
> > On Thu, 28 Jul 2022 15:44:20 +0100
> > "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> >
> > > * Igor Mammedov (imammedo@redhat.com) wrote:
> > > > QEMU crashes trying to save VMSTATE when only MIPS target are compiled in
> > > >   $ qemu-system-mips -monitor stdio
> > > >   (qemu) migrate "exec:gzip -c > STATEFILE.gz"
> > > >   Segmentation fault (core dumped)
> > > >
> > > > It happens due to PIIX4_PM trying to parse hotplug vmstate structures
> > > > which are valid only for x86 and not for MIPS (as it requires ACPI
> > > > tables support which is not existent for ithe later)
> > > >
> > > > Issue was probably exposed by trying to cleanup/compile out unused
> > > > ACPI bits from MIPS target (but forgetting about migration bits).
> > > >
> > > > Disable compiled out features using compat properties as the least
> > > > risky way to deal with issue.
> > >
> > > Isn't the problem partially due to a 'stub' vmsd which isn't terminated?
> >
> > Not sure what "'stub' vmsd" is, can you explain?
>
> In hw/acpi/acpi-pci-hotplug-stub.c there is :
> const VMStateDescription vmstate_acpi_pcihp_pci_status;
>
> this seg happens when the migration code walks into that - this should
> always get populated with some of the minimal fields, in particular the
> .name and .fields array terminated with VMSTATE_END_OF_LIST().

Either:
 (1) we should be sure the vmstate struct does not get used if the
     compile-time config has ended up with the stub
or
 (2) it needs to actually match the real vmstate struct, otherwise
     migration between a QEMU built with a config that just got the
     stub version and a QEMU built with a config that got the full
     version will break

This patch does the former. Segfaulting if we got something wrong
and tried to use the vmstate when we weren't expecting to is
arguably better than producing an incompatible migration stream.
(Better still would be if we caught this on machine startup rather
than only when savevm was invoked.)

thanks
-- PMM