Re: [Qemu-devel] [RFCv2 00/12] Clean up compatibility mode handling

[David Gibson]

On Sat, Nov 26, 2016 at 01:33:16AM +0100, Greg Kurz wrote:
> On Wed, 16 Nov 2016 09:17:43 +1100
> David Gibson <address@hidden> wrote:
> 
> > This series is a significant rework to how we handle CPU compatibility
> > modes on ppc.
> > 
> >  * Information about compatibility modes was previously open coded and
> >    scattered across a number of functions in both target-ppc and spapr
> >    code.  It's now brought together into a common table of
> >    compatibility modes.
> > 
> >  * There was significant conceptual confusion about what a
> >    compatibility mode means, and how it interacts with the machine
> >    type.  This cleans that up, clarifying that a compatibility mode
> >    (as an externally set option) only makes sense on machine types
> >    that don't permit the guest hypervisor privilege (i.e. 'pseries')
> > 
> >  * It was previously the user's (or management layer's) responsibility
> >    to determine compatibility of CPUs on either end for migration.
> >    This uses the compatibility modes to check that properly during an
> >    incoming migration.
> > 
> >  * Some ill-considered sanity checks broke migration from 2.6 to 2.7,
> >    due to some new instruction classes being added.  This should avoid
> >    a repeat of that problem for 2.8 (we may be able to backport a
> >    minimal subset to 2.7-stable to fix the existing problem).
> > 
> > Patches 1-3 are preliminary cleanups which could stand on their own.
> > Patches 4-12 are the compatibility mode cleanup proper.
> > 
> > So far, this has been mimimally tested.  There are quite a few
> > migration cases to check.  For example:
> > 
> > Basic:
> > 
> > 1) Boot guest with -cpu host
> >     Should go into POWER8 compat mode after CAS
> >     Previously would have been raw mode
> > 


Thanks for doing these detailed tests, Greg.

> 
> == QEMU ==
> 
> spapr_cas_pvr current=0, explicit_match=1, new=f000004
> 
> == guest ==
> 
> cpu             : POWER8 (architected), altivec supported
> 
> > 2) Boot guest with -machine pseries,max-cpu-compat=power7 -cpu host
> >     Should go into POWER7 compat mode
> > 
> 
> == QEMU ==
> 
> spapr_cas_pvr current=f000003, explicit_match=1, new=f000003
> 
> == guest ==
> 
> cpu             : POWER7 (architected), altivec supported
> 
> > 3) Boot guest with -cpu host,compat=power7
> >     Should act as (2), but print a warning
> > 
> 
> With extra patch to add explicit null to string visitors:
> 
> qapi: add explicit null to string input and  output visitors
> Message-Id: <address@hidden>
> 
> == QEMU ==
> 
> CPU 'compat' property is deprecated and has no effect; use max-cpu-compat 
> machine
> property instead
> 
> spapr_cas_pvr current=f000003, explicit_match=1, new=f000003
> 
> == guest ==
> 
> cpu             : POWER7 (architected), altivec supported
> 
> > 4) Boot guest via libvirt with power7 compat mode specified in XML
> >     Should act as (3), (2) once we fix libvirt
> > 
> 
> Not tested yet.
> 
> > 5) Hack guest to only advertise power7 compatibility, boot with -cpu host
> >     Should go into POWER7 compat mode after CAS
> > 
> 
> == QEMU ==
> 
> spapr_cas_pvr current=0, explicit_match=1, new=f000003
> 
> == guest ==
> 
> cpu             : POWER7 (architected), altivec supported
> 
> > 6) Hack guest to only advertise real PVRs
> >     Should remain in POWER8 raw mode after CAS
> > 
> 
> == QEMU ==
> 
> spapr_cas_pvr current=0, explicit_match=1, new=0
> 
> == guest ==
> 
> cpu             : POWER8 (raw), altivec supported
> 
> > 7) Hack guest to only advertise real PVRs
> >    Boot with -machine pseries,max-cpu-compat=power8
> >     Should fail at CAS time
> > 
> 
> == QEMU ==
> 
> h_client_architecture_support() returns H_HARDWARE as
> expected because max-cpu-compat is set and no compat
> PVR was found (even though the real PVR was found).
> 
> == guest ==
> 
> WARNING: ibm,client-architecture-support call FAILED!
> 
> but the guest boots anyway and we end up with:
> 
> cpu             : POWER8 (architected), altivec supported
> 
> This looks weird since the guest explicitly said it only
> supports real PVRs... raw mode like case 6) would make
> more sense IMHO but patch 11/12 sets the default to max-cpu-compat
> at machine reset time:
> 
> +        ppc_set_compat_all(spapr->max_compat_pvr, &error_abort);
> 
> Maybe we should at least switch to raw mode, return an error
> and let the guest decide ? 
> 
> Another option would be to do as specified in LoPAPR section B.6.2.3
> when no acceptable PVR was found and to simply terminate the guest.

So.. I suspect this is probably good enough in practice, given that
all known guests will actually advertise compat modes.  The FAILED
gives at least a hint as to what's going on.

To get it strictly correct, then yes, I think terminating the guest is
probably the PAPRishly correct thing to do, although it's not clear
quite what we do then.  We don't really have a mechanism for shutting
the VM down entirely (which might surprise management), and if we
reboot we're likely to just hit the same error again.

Hence my inclination to only worry about those details if someone
starts hitting them for real.

> > 8) Hack guest to only advertise power7 compatibility, boot with -cpu host
> >    Reboot to normal guest
> >     Should go to power7 compat mode after CAS of boot 1
> >     Should revert to raw mode on reboot
> >     SHould go to power8 compat mode after CAS of boot 2
> > 
> 
> == QEMU ==
> 
> boot 1: spapr_cas_pvr current=0, explicit_match=1, new=f000003
> boot 2: spapr_cas_pvr current=0, explicit_match=1, new=f000004
> 
> == guest ==
> 
> boot 1: cpu             : POWER7 (architected), altivec supported
> boot 2: cpu             : POWER8 (architected), altivec supported
> 
> > Migration:
> > 
> 
> I'll give a try to migration next week.
> 
> Cheers.
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature