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

[David Gibson]

On Mon, Nov 28, 2016 at 03:23:46PM +1100, David Gibson wrote:
> 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.

Oh.. that said, I think we should at least throw in a meaningful
error_report() since that should get propagated up to the error logs
that mangement layers are likely to keep.

> 
> > > 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