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

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

On Mon, 1 Aug 2022 at 10:17, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
>
> * Peter Maydell (peter.maydell@linaro.org) wrote:
> > On Fri, 29 Jul 2022 at 10:57, Igor Mammedov <imammedo@redhat.com> wrote:
> > >
> > > On Thu, 28 Jul 2022 16:12:34 +0100
> > > Peter Maydell <peter.maydell@linaro.org> wrote:
> > > > 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.)
> > > Theoretically possible with a bunch of mips and x86 stubs, but ...
> > > we typically don't do this kind of checks for migration sake
> > > as that complicates things a lot in general.
> > > i.e. it's common to let migration fail in case of incompatible
> > > migration stream. It's not exactly friendly to user but it's
> > > graceful failure (assuming code is correct and not crashes QEMU)
> >
> > The point here is that if we ever try to do a migrate with the
> > stub vmstate struct then that's a bug in QEMU. We should prefer
> > to catch those early and clearly.
>
> I'd rather have something that was explicitly poisoned rather than just
> walking off the end of an uninitialised array and having to break out
> gdb.

It doesn't walk off the end of the array -- it segfaults because
it wants to dereference vmsd->name, which is NULL.

If we want to have a more obvious and concrete way to mark "this
vmsd is bad and should never be actively used" that's fine, but it
seems like a separate patch from this one, which is just fixing
the problem that we use a vmsd that we should not be using.

-- PMM