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:

Basic:

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

2) Boot guest with -machine pseries,max-cpu-compat=power7 -cpu host
        Should go into POWER7 compat mode

3) Boot guest with -cpu host,compat=power7
        Should act as (2), but print a warning

4) Boot guest via libvirt with power7 compat mode specified in XML
        Should act as (3), (2) once we fix libvirt

5) Hack guest to only advertise power7 compatibility, boot with -cpu host
        Should go into POWER7 compat mode after CAS

6) Hack guest to only advertise real PVRs
        Should remain in POWER8 raw mode after CAS

7) Hack guest to only advertise real PVRs
   Boot with -machine pseries,max-cpu-compat=power8
        Should fail at CAS time

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

Migration:

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

-- 
2.9.3

Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20170427072843.8089-1-david@gibson.dropbear.id.au
Subject: [Qemu-devel] [PATCHv3 0/4] Clean up compatibility mode handling
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]      patchew/20170426221046.4596-1-eblake@redhat.com -> patchew/20170426221046.4596-1-eblake@redhat.com
Switched to a new branch 'test'
28f8147 ppc: Rework CPU compatibility testing across migration
07b83df pseries: Reset CPU compatibility mode
f9b3835 pseries: Move CPU compatibility property to machine
504c501 qapi: add explicit null to string input and output visitors

=== OUTPUT BEGIN ===
Checking PATCH 1/4: qapi: add explicit null to string input and output visitors...
Checking PATCH 2/4: pseries: Move CPU compatibility property to machine...
ERROR: line over 90 characters
#372: FILE: target/ppc/translate_init.c:8430:
+    error_report("CPU 'compat' property is deprecated and has no effect; use max-cpu-compat machine property instead");

total: 1 errors, 0 warnings, 332 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 3/4: pseries: Reset CPU compatibility mode...
Checking PATCH 4/4: ppc: Rework CPU compatibility testing across migration...
ERROR: braces {} are necessary for all arms of this statement
#96: FILE: target/ppc/machine.c:239:
+    if (cpu->compat_pvr) {
[...]
+    } else
[...]

total: 1 errors, 0 warnings, 102 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

On Thu, 27 Apr 2017 17:28:39 +1000
David Gibson <david@gibson.dropbear.id.au> 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:
> 

Hi David,

I'll rerun the tests shortly.

Cheers,

--
Greg

> Basic:
> 
> 1) Boot guest with -cpu host
>         Should go into POWER8 compat mode after CAS
>         Previously would have been raw mode
> 
> 2) Boot guest with -machine pseries,max-cpu-compat=power7 -cpu host
>         Should go into POWER7 compat mode
> 
> 3) Boot guest with -cpu host,compat=power7
>         Should act as (2), but print a warning
> 
> 4) Boot guest via libvirt with power7 compat mode specified in XML
>         Should act as (3), (2) once we fix libvirt
> 
> 5) Hack guest to only advertise power7 compatibility, boot with -cpu host
>         Should go into POWER7 compat mode after CAS
> 
> 6) Hack guest to only advertise real PVRs
>         Should remain in POWER8 raw mode after CAS
> 
> 7) Hack guest to only advertise real PVRs
>    Boot with -machine pseries,max-cpu-compat=power8
>         Should fail at CAS time
> 
> 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
> 
> Migration:
> 
> 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(-)
> 

On Thu, 27 Apr 2017 17:28:39 +1000
David Gibson <david@gibson.dropbear.id.au> 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(-)
> 

On Thu, 2017-04-27 at 17:28 +1000, David Gibson 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.

I started playing with this, mostly with libvirt integration
in mind of course, and quickly ran into a behavior that I
believe was not intentional and unfortunately managed to slip
into 2.9, I assume through the patches which were initially
part of this series (mentioned above).

More specifically, when I run a guest with

  -M pseries-2.8 -cpu host

using QEMU 2.8, the CPU in the guest is reported as

  POWER8E (raw), altivec supported

However, when using QEMU 2.9 with the very same command line,
including explicitly using the pseries-2.8 machine type, I
get

  POWER8 (architected), altivec supported

instead. The same happens with current master + your patches
applied on top.

I'm not sure how much real trouble that will actually cause
for guests, but it's a guest-visible change as a result of
upgrading QEMU, which should just not happen.


I'll keep testing your series and get back to you as soon as
I have more feedback or questions.

-- 
Andrea Bolognani / Red Hat / Virtualization

On Thu, 04 May 2017 16:32:59 +0200
Andrea Bolognani <abologna@redhat.com> wrote:

> On Thu, 2017-04-27 at 17:28 +1000, David Gibson 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.  
> 
> I started playing with this, mostly with libvirt integration
> in mind of course, and quickly ran into a behavior that I
> believe was not intentional and unfortunately managed to slip
> into 2.9, I assume through the patches which were initially
> part of this series (mentioned above).
> 
> More specifically, when I run a guest with
> 
>   -M pseries-2.8 -cpu host
> 
> using QEMU 2.8, the CPU in the guest is reported as
> 
>   POWER8E (raw), altivec supported
> 
> However, when using QEMU 2.9 with the very same command line,
> including explicitly using the pseries-2.8 machine type, I
> get
> 
>   POWER8 (architected), altivec supported
> 

The goal of this series is indeed to switch from raw to architected but I
agree that it shouldn't affect existing machine types. This probably calls
for some compat prop.

> instead. The same happens with current master + your patches
> applied on top.
> 
> I'm not sure how much real trouble that will actually cause
> for guests, but it's a guest-visible change as a result of
> upgrading QEMU, which should just not happen.
> 
> 
> I'll keep testing your series and get back to you as soon as
> I have more feedback or questions.
> 
> -- 
> Andrea Bolognani / Red Hat / Virtualization
> 

On Thu, May 04, 2017 at 09:22:37PM +0200, Greg Kurz wrote:
> On Thu, 04 May 2017 16:32:59 +0200
> Andrea Bolognani <abologna@redhat.com> wrote:
> 
> > On Thu, 2017-04-27 at 17:28 +1000, David Gibson 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.  
> > 
> > I started playing with this, mostly with libvirt integration
> > in mind of course, and quickly ran into a behavior that I
> > believe was not intentional and unfortunately managed to slip
> > into 2.9, I assume through the patches which were initially
> > part of this series (mentioned above).
> > 
> > More specifically, when I run a guest with
> > 
> >   -M pseries-2.8 -cpu host
> > 
> > using QEMU 2.8, the CPU in the guest is reported as
> > 
> >   POWER8E (raw), altivec supported
> > 
> > However, when using QEMU 2.9 with the very same command line,
> > including explicitly using the pseries-2.8 machine type, I
> > get
> > 
> >   POWER8 (architected), altivec supported
> > 
> 
> The goal of this series is indeed to switch from raw to architected but I
> agree that it shouldn't affect existing machine types. This probably calls
> for some compat prop.

So, I have mixed feelings about this.

1. Yes, the series was intended to switch from preferring raw to
preferring architected mode.

2. No, it shouldn't have affected older machine types - I didn't think
that through.

But, having made the mistake, I'm not sure it's worth correcting.  I
don't actually expect this to break guests, and fixing it now that
it's already there will be messy, and raises the possibility of new
behavioural change between older and newer subversions of 2.9.


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

On Fri, 2017-05-12 at 17:33 +1000, David Gibson wrote:
> > The goal of this series is indeed to switch from raw to architected but I
> > agree that it shouldn't affect existing machine types. This probably calls
> > for some compat prop.
> 
> So, I have mixed feelings about this.
> 
> 1. Yes, the series was intended to switch from preferring raw to
> preferring architected mode.
> 
> 2. No, it shouldn't have affected older machine types - I didn't think
> that through.
> 
> But, having made the mistake, I'm not sure it's worth correcting.  I
> don't actually expect this to break guests, and fixing it now that
> it's already there will be messy, and raises the possibility of new
> behavioural change between older and newer subversions of 2.9.

I'm not sure how QEMU handles maintenance branches, but it
seems to me that it would be better to confine this issue
to a single buggy release rather than carrying it forward
forever.

Downstream releases can just backport the relevant commits
and pretend nothing ever happened :)

-- 
Andrea Bolognani / Red Hat / Virtualization