Re: [Qemu-devel] [PATCHv3 0/4] Clean up compatibility mode handling

[Greg Kurz]

On Thu, 27 Apr 2017 17:28:39 +1000
David Gibson <address@hidden> wrote:

> This is a rebased and revised version of my patches revising CPU
> compatiblity mode handling on ppc, last posted in November.  Since
> then, many of the patches have already been merged (some for 2.9, some
> since).  This is what's left.
> 
>  * There was 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.
> 
> This hasn't been extensively tested yet.  There are quite a few
> migration cases to consider, for example:
> 

I tested with the following change squashed into patch 2:

--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -8428,7 +8428,7 @@ static void getset_compat_deprecated(Object *obj, Visitor 
                                      void *opaque, Error **errp)
 {
     error_report("CPU 'compat' property is deprecated and has no effect; use ma
-    visit_type_null(v, name, errp);
+    visit_type_null(v, name, NULL);
 }
 
 static PropertyInfo ppc_compat_deprecated_propinfo = {

> Basic:
> 
> 1) Boot guest with -cpu host
>         Should go into POWER8 compat mode after CAS
>         Previously would have been raw mode
> 

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

== QEMU ==

We get the following warning as expected:

CPU 'compat' property is deprecated and has no effect; use max-cpu-compat 
machine property instead

We actually get one per vcpu. This may means a lot of lines if there are
many of them. But I can't think of any simple way to avoid that... the
user will have a motivation to update its command line :)

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.

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

Calling ibm,client-architecture-support...
WARNING: ibm,client-architecture-support call FAILED!

But as with your RFCv2, the guest boots anyway in compat
mode, even if the guest didn't advertise it:

cpu             : POWER8 (architected), altivec supported

We had discussed about two possible options at the time:
1) terminate the guest (LoPAPR section B.6.2.3)
or
2) just add an error_report()

You seemed to favor 2) at the time, but Sam Bobroff's recent work to
allow MMU mode selection via CAS (commit 9fb4541f5803) explicitly
has QEMU to "terminate the guest if it requests an unavailable mode,
as required by the architecture." (ie, QEMU exits).

Shouldn't we do the same when we asked for compat mode but the client
doesn't advertise any compat PVR ?

> 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 work on testing migration ASAP.

Cheers,

--
Greg

> 9) Boot guest with qemu-2.6 -machine pseries-2.6 -cpu host
>    Migrate to qemu-2.8 -machine pseries-2.6 -cpu host
>         Should work, end up running in power8 raw mode
> 
> 10) Boot guest with qemu-2.7 -machine pseries-2.7 -cpu host
>     Migrate to qemu-2.8 -machine pseries-2.7 -cpu host
>         Should work, end up running in power8 raw mode
> 
> 11) Boot guest with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
>     Migrate to qemu-2.8 -machine pseries-2.7 -cpu host,compat=power7
>         Should work, be running in POWER7 compat after, but give warning like
>         (3)
> 
> 12) Boot guest with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
>     Migrate to qemu-2.8 -machine pseries-2.7,max-cpu-compat=power7 -cpu host
>         Should work, be running in POWER7 compat after, no warning
> 
> 13) Boot to SLOF with qemu-2.6 -machine pseries-2.6 -cpu host
>     Migrate to qemu-2.8 -machine pseries-2.6 -cpu host
> 
>         ?
> 
> 14) Boot to SLOF with qemu-2.7 -machine pseries-2.7 -cpu host
>     Migrate to qemu-2.8 -machine pseries-2.7 -cpu host
>         ?
> 
> 15) Boot to SLOF with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
>     Migrate to qemu-2.8 -machine pseries-2.7 -cpu host,compat=power7
>         ?
> 
> 16) Boot to SLOF with qemu-2.7 -machine pseries-2.7 -cpu host,compat=power7
>     Migrate to qemu-2.8 -machine pseries-2.7,max-cpu-compat=power7 -cpu host
>         ?
> 
> 17) Boot guest with qemu-2.6 -machine pseries-2.6 -cpu host
>     Migrate to qemu-2.7.z -machine pseries-2.6 -cpu host
>         Should work
> 
> 18) Hack guest to only advertise power7 compatibility, boot with -cpu host
>     Boot with qemu-2.8, migrate to qemu-2.8
>         Should be in power7 compat mode after CAS on source, and still
>         in power7 compat mode on destination
> 
> 
> Changes since RFCv2:
>   * Many patches dropped, since they're already merged
>   * Rebased, fixed conflicts
>   * Restored support for backwards migration (wasn't as complicated as
>     I thought)
>   * Updated final patch's description to more accurately reflect the
>     logic
> 
> Changes since RFCv1:
>   * Change CAS logic to prefer compatibility modes over raw mode
>   * Simplified by giving up on half-hearted attempts to maintain
>     backwards migration
>   * Folded migration stream changes into a single patch
>   * Removed some preliminary patches which are already merged
> 
> 
> 
> David Gibson (3):
>   pseries: Move CPU compatibility property to machine
>   pseries: Reset CPU compatibility mode
>   ppc: Rework CPU compatibility testing across migration
> 
> Greg Kurz (1):
>   qapi: add explicit null to string input and output visitors
> 
>  hw/ppc/spapr.c               |  8 ++++-
>  hw/ppc/spapr_cpu_core.c      | 47 +++++++++++++++++++------
>  hw/ppc/spapr_hcall.c         |  2 +-
>  include/hw/ppc/spapr.h       | 12 ++++---
>  qapi/string-input-visitor.c  | 11 ++++++
>  qapi/string-output-visitor.c | 14 ++++++++
>  target/ppc/compat.c          | 65 ++++++++++++++++++++++++++++++++++
>  target/ppc/cpu.h             |  6 ++--
>  target/ppc/machine.c         | 71 +++++++++++++++++++++++++++++++++++--
>  target/ppc/translate_init.c  | 84 
> ++++++++++++--------------------------------
>  10 files changed, 238 insertions(+), 82 deletions(-)
>

pgp7OyN728lvs.pgp
Description: OpenPGP digital signature