[Qemu-devel] [PATCHv5 0/4] Clean up compatibility mode handling

[Qemu-devel] [PATCHv5 0/4] Clean up compatibility mode handling

[David Gibson]

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 v4:
  * Fixed a crash bug in the smp option compatiblity mangling
  * Removed an unnecessary fallback for missing pvr_match
  * Some spelling corrections
  * Migration core patch removed (alread merged to ppc-for-2.10)

Changes since v3:
  * Backwards compatible -cpu handling now removes compat= option from
    options passed on to the cpu, so it doesn't trigger further warnings
  * Add a migration fix make cpu_synchronize_state() safe in post_load
    handlers, which in turn fixes a bug in 5/5.
  * A number of bugfixes and other tweaks suggested by feedback on v2.

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      |  61 +++++++++++++++++++++-----
 hw/ppc/spapr_hcall.c         |   8 ++--
 include/hw/ppc/spapr.h       |  12 +++--
 qapi/string-input-visitor.c  |  11 +++++
 qapi/string-output-visitor.c |  14 ++++++
 target/ppc/compat.c          | 102 +++++++++++++++++++++++++++++++++++++++++++
 target/ppc/cpu.h             |   5 ++-
 target/ppc/machine.c         |  69 +++++++++++++++++++++++++++--
 target/ppc/translate_init.c  |  86 +++++++++++-------------------------
 10 files changed, 292 insertions(+), 84 deletions(-)

-- 
2.9.4

[Qemu-devel] [PATCHv5 1/4] qapi: add explicit null to string input and output visitors

[David Gibson]

From: Greg Kurz <groug@kaod.org>

This may be used for deprecated object properties that are kept for
backwards compatibility.

Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 qapi/string-input-visitor.c  | 11 +++++++++++
 qapi/string-output-visitor.c | 14 ++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index c089491..63ae115 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -326,6 +326,16 @@ static void parse_type_number(Visitor *v, const char *name, double *obj,
     *obj = val;
 }
 
+static void parse_type_null(Visitor *v, const char *name, Error **errp)
+{
+    StringInputVisitor *siv = to_siv(v);
+
+    if (!siv->string || siv->string[0]) {
+        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null",
+                   "null");
+    }
+}
+
 static void string_input_free(Visitor *v)
 {
     StringInputVisitor *siv = to_siv(v);
@@ -349,6 +359,7 @@ Visitor *string_input_visitor_new(const char *str)
     v->visitor.type_bool = parse_type_bool;
     v->visitor.type_str = parse_type_str;
     v->visitor.type_number = parse_type_number;
+    v->visitor.type_null = parse_type_null;
     v->visitor.start_list = start_list;
     v->visitor.next_list = next_list;
     v->visitor.check_list = check_list;
diff --git a/qapi/string-output-visitor.c b/qapi/string-output-visitor.c
index 53c2175..af649e1 100644
--- a/qapi/string-output-visitor.c
+++ b/qapi/string-output-visitor.c
@@ -256,6 +256,19 @@ static void print_type_number(Visitor *v, const char *name, double *obj,
     string_output_set(sov, g_strdup_printf("%f", *obj));
 }
 
+static void print_type_null(Visitor *v, const char *name, Error **errp)
+{
+    StringOutputVisitor *sov = to_sov(v);
+    char *out;
+
+    if (sov->human) {
+        out = g_strdup("<null>");
+    } else {
+        out = g_strdup("");
+    }
+    string_output_set(sov, out);
+}
+
 static void
 start_list(Visitor *v, const char *name, GenericList **list, size_t size,
            Error **errp)
@@ -341,6 +354,7 @@ Visitor *string_output_visitor_new(bool human, char **result)
     v->visitor.type_bool = print_type_bool;
     v->visitor.type_str = print_type_str;
     v->visitor.type_number = print_type_number;
+    v->visitor.type_null = print_type_null;
     v->visitor.start_list = start_list;
     v->visitor.next_list = next_list;
     v->visitor.end_list = end_list;
-- 
2.9.4

[Qemu-devel] [PATCHv5 2/4] pseries: Move CPU compatibility property to machine

[David Gibson]

Server class POWER CPUs have a "compat" property, which is used to set the
backwards compatibility mode for the processor.  However, this only makes
sense for machine types which don't give the guest access to hypervisor
privilege - otherwise the compatibility level is under the guest's control.

To reflect this, this removes the CPU 'compat' property and instead
creates a 'max-cpu-compat' property on the pseries machine.  Strictly
speaking this breaks compatibility, but AFAIK the 'compat' option was
never (directly) used with -device or device_add.

The option was used with -cpu.  So, to maintain compatibility, this
patch adds a hack to the cpu option parsing to strip out any compat
options supplied with -cpu and set them on the machine property
instead of the now deprecated cpu property.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c              |   6 ++-
 hw/ppc/spapr_cpu_core.c     |  55 +++++++++++++++++++++++-
 hw/ppc/spapr_hcall.c        |   8 ++--
 include/hw/ppc/spapr.h      |  12 ++++--
 target/ppc/compat.c         | 102 ++++++++++++++++++++++++++++++++++++++++++++
 target/ppc/cpu.h            |   5 ++-
 target/ppc/translate_init.c |  86 +++++++++++--------------------------
 7 files changed, 201 insertions(+), 73 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ab3aab1..3c4e88f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2134,7 +2134,7 @@ static void ppc_spapr_init(MachineState *machine)
         machine->cpu_model = kvm_enabled() ? "host" : smc->tcg_default_cpu;
     }
 
-    ppc_cpu_parse_features(machine->cpu_model);
+    spapr_cpu_parse_features(spapr);
 
     spapr_init_cpus(spapr);
 
@@ -2497,6 +2497,10 @@ static void spapr_machine_initfn(Object *obj)
                                     " place of standard EPOW events when possible"
                                     " (required for memory hot-unplug support)",
                                     NULL);
+
+    ppc_compat_add_property(obj, "max-cpu-compat", &spapr->max_compat_pvr,
+                            "Maximum permitted CPU compatibility mode",
+                            &error_fatal);
 }
 
 static void spapr_machine_finalizefn(Object *obj)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index ff7058e..846d9e7 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -20,6 +20,57 @@
 #include "sysemu/numa.h"
 #include "qemu/error-report.h"
 
+void spapr_cpu_parse_features(sPAPRMachineState *spapr)
+{
+    /*
+     * Backwards compatibility hack:
+     *
+     *   CPUs had a "compat=" property which didn't make sense for
+     *   anything except pseries.  It was replaced by "max-cpu-compat"
+     *   machine option.  This supports old command lines like
+     *       -cpu POWER8,compat=power7
+     *   By stripping the compat option and applying it to the machine
+     *   before passing it on to the cpu level parser.
+     */
+    gchar **inpieces;
+    int i, j;
+    gchar *compat_str = NULL;
+
+    inpieces = g_strsplit(MACHINE(spapr)->cpu_model, ",", 0);
+
+    /* inpieces[0] is the actual model string */
+    i = 1;
+    j = 1;
+    while (inpieces[i]) {
+        if (g_str_has_prefix(inpieces[i], "compat=")) {
+            /* in case of multiple compat= options */
+            g_free(compat_str);
+            compat_str = inpieces[i];
+        } else {
+            j++;
+        }
+
+        i++;
+        /* Excise compat options from list */
+        inpieces[j] = inpieces[i];
+    }
+
+    if (compat_str) {
+        char *val = compat_str + strlen("compat=");
+        gchar *newprops = g_strjoinv(",", inpieces);
+
+        object_property_set_str(OBJECT(spapr), val, "max-cpu-compat",
+                                &error_fatal);
+
+        ppc_cpu_parse_features(newprops);
+        g_free(newprops);
+    } else {
+        ppc_cpu_parse_features(MACHINE(spapr)->cpu_model);
+    }
+
+    g_strfreev(inpieces);
+}
+
 static void spapr_cpu_reset(void *opaque)
 {
     sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
@@ -70,10 +121,10 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
     /* Enable PAPR mode in TCG or KVM */
     cpu_ppc_set_papr(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
 
-    if (cpu->max_compat) {
+    if (spapr->max_compat_pvr) {
         Error *local_err = NULL;
 
-        ppc_set_compat(cpu, cpu->max_compat, &local_err);
+        ppc_set_compat(cpu, spapr->max_compat_pvr, &local_err);
         if (local_err) {
             error_propagate(errp, local_err);
             return;
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index aae5a62..a9bb3ed 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1044,11 +1044,11 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
     }
 }
 
-static uint32_t cas_check_pvr(PowerPCCPU *cpu, target_ulong *addr,
-                              Error **errp)
+static uint32_t cas_check_pvr(sPAPRMachineState *spapr, PowerPCCPU *cpu,
+                              target_ulong *addr, Error **errp)
 {
     bool explicit_match = false; /* Matched the CPU's real PVR */
-    uint32_t max_compat = cpu->max_compat;
+    uint32_t max_compat = spapr->max_compat_pvr;
     uint32_t best_compat = 0;
     int i;
 
@@ -1104,7 +1104,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
     bool guest_radix;
     Error *local_err = NULL;
 
-    cas_pvr = cas_check_pvr(cpu, &addr, &local_err);
+    cas_pvr = cas_check_pvr(spapr, cpu, &addr, &local_err);
     if (local_err) {
         error_report_err(local_err);
         return H_HARDWARE;
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 98fb78b..4da92e2 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -87,16 +87,19 @@ struct sPAPRMachineState {
     uint64_t rtc_offset; /* Now used only during incoming migration */
     struct PPCTimebase tb;
     bool has_graphics;
-    sPAPROptionVector *ov5;         /* QEMU-supported option vectors */
-    sPAPROptionVector *ov5_cas;     /* negotiated (via CAS) option vectors */
-    bool cas_reboot;
-    bool cas_legacy_guest_workaround;
 
     Notifier epow_notifier;
     QTAILQ_HEAD(, sPAPREventLogEntry) pending_events;
     bool use_hotplug_event_source;
     sPAPREventSource *event_sources;
 
+    /* ibm,client-architecture-support option negotiation */
+    bool cas_reboot;
+    bool cas_legacy_guest_workaround;
+    sPAPROptionVector *ov5;         /* QEMU-supported option vectors */
+    sPAPROptionVector *ov5_cas;     /* negotiated (via CAS) option vectors */
+    uint32_t max_compat_pvr;
+
     /* Migration state */
     int htab_save_index;
     bool htab_first_pass;
@@ -639,6 +642,7 @@ void spapr_hotplug_req_add_by_count_indexed(sPAPRDRConnectorType drc_type,
                                             uint32_t count, uint32_t index);
 void spapr_hotplug_req_remove_by_count_indexed(sPAPRDRConnectorType drc_type,
                                                uint32_t count, uint32_t index);
+void spapr_cpu_parse_features(sPAPRMachineState *spapr);
 void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
                                     sPAPRMachineState *spapr);
 
diff --git a/target/ppc/compat.c b/target/ppc/compat.c
index e8ec1e1..e72839f 100644
--- a/target/ppc/compat.c
+++ b/target/ppc/compat.c
@@ -24,9 +24,11 @@
 #include "sysemu/cpus.h"
 #include "qemu/error-report.h"
 #include "qapi/error.h"
+#include "qapi/visitor.h"
 #include "cpu-models.h"
 
 typedef struct {
+    const char *name;
     uint32_t pvr;
     uint64_t pcr;
     uint64_t pcr_level;
@@ -38,6 +40,7 @@ static const CompatInfo compat_table[] = {
      * Ordered from oldest to newest - the code relies on this
      */
     { /* POWER6, ISA2.05 */
+        .name = "power6",
         .pvr = CPU_POWERPC_LOGICAL_2_05,
         .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
                PCR_COMPAT_2_05 | PCR_TM_DIS | PCR_VSX_DIS,
@@ -45,24 +48,28 @@ static const CompatInfo compat_table[] = {
         .max_threads = 2,
     },
     { /* POWER7, ISA2.06 */
+        .name = "power7",
         .pvr = CPU_POWERPC_LOGICAL_2_06,
         .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
         .pcr_level = PCR_COMPAT_2_06,
         .max_threads = 4,
     },
     {
+        .name = "power7+",
         .pvr = CPU_POWERPC_LOGICAL_2_06_PLUS,
         .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
         .pcr_level = PCR_COMPAT_2_06,
         .max_threads = 4,
     },
     { /* POWER8, ISA2.07 */
+        .name = "power8",
         .pvr = CPU_POWERPC_LOGICAL_2_07,
         .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07,
         .pcr_level = PCR_COMPAT_2_07,
         .max_threads = 8,
     },
     { /* POWER9, ISA3.00 */
+        .name = "power9",
         .pvr = CPU_POWERPC_LOGICAL_3_00,
         .pcr = PCR_COMPAT_3_00,
         .pcr_level = PCR_COMPAT_3_00,
@@ -189,3 +196,98 @@ int ppc_compat_max_threads(PowerPCCPU *cpu)
 
     return n_threads;
 }
+
+static void ppc_compat_prop_get(Object *obj, Visitor *v, const char *name,
+                                void *opaque, Error **errp)
+{
+    uint32_t compat_pvr = *((uint32_t *)opaque);
+    const char *value;
+
+    if (!compat_pvr) {
+        value = "";
+    } else {
+        const CompatInfo *compat = compat_by_pvr(compat_pvr);
+
+        g_assert(compat);
+
+        value = compat->name;
+    }
+
+    visit_type_str(v, name, (char **)&value, errp);
+}
+
+static void ppc_compat_prop_set(Object *obj, Visitor *v, const char *name,
+                                void *opaque, Error **errp)
+{
+    Error *error = NULL;
+    char *value;
+    uint32_t compat_pvr;
+
+    visit_type_str(v, name, &value, &error);
+    if (error) {
+        error_propagate(errp, error);
+        return;
+    }
+
+    if (strcmp(value, "") == 0) {
+        compat_pvr = 0;
+    } else {
+        int i;
+        const CompatInfo *compat = NULL;
+
+        for (i = 0; i < ARRAY_SIZE(compat_table); i++) {
+            if (strcmp(value, compat_table[i].name) == 0) {
+                compat = &compat_table[i];
+                break;
+
+            }
+        }
+
+        if (!compat) {
+            error_setg(errp, "Invalid compatibility mode \"%s\"", value);
+            goto out;
+        }
+        compat_pvr = compat->pvr;
+    }
+
+    *((uint32_t *)opaque) = compat_pvr;
+
+out:
+    g_free(value);
+}
+
+void ppc_compat_add_property(Object *obj, const char *name,
+                             uint32_t *compat_pvr, const char *basedesc,
+                             Error **errp)
+{
+    Error *local_err = NULL;
+    gchar *namesv[ARRAY_SIZE(compat_table) + 1];
+    gchar *names, *desc;
+    int i;
+
+    object_property_add(obj, name, "string",
+                        ppc_compat_prop_get, ppc_compat_prop_set, NULL,
+                        compat_pvr, &local_err);
+    if (local_err) {
+        goto out;
+    }
+
+    for (i = 0; i < ARRAY_SIZE(compat_table); i++) {
+        /*
+         * Have to discard const here, because g_strjoinv() takes
+         * (gchar **), not (const gchar **) :(
+         */
+        namesv[i] = (gchar *)compat_table[i].name;
+    }
+    namesv[ARRAY_SIZE(compat_table)] = NULL;
+
+    names = g_strjoinv(", ", namesv);
+    desc = g_strdup_printf("%s. Valid values are %s.", basedesc, names);
+    object_property_set_description(obj, name, desc, &local_err);
+
+    g_free(names);
+    g_free(desc);
+
+out:
+    error_propagate(errp, local_err);
+}
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 401e10e..4517b4b 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1189,7 +1189,6 @@ typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
  * PowerPCCPU:
  * @env: #CPUPPCState
  * @cpu_dt_id: CPU index used in the device tree. KVM uses this index too
- * @max_compat: Maximal supported logical PVR from the command line
  * @compat_pvr: Current logical PVR, zero if in "raw" mode
  *
  * A PowerPC CPU.
@@ -1201,7 +1200,6 @@ struct PowerPCCPU {
 
     CPUPPCState env;
     int cpu_dt_id;
-    uint32_t max_compat;
     uint32_t compat_pvr;
     PPCVirtualHypervisor *vhyp;
     Object *intc;
@@ -1374,6 +1372,9 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp);
 void ppc_set_compat_all(uint32_t compat_pvr, Error **errp);
 #endif
 int ppc_compat_max_threads(PowerPCCPU *cpu);
+void ppc_compat_add_property(Object *obj, const char *name,
+                             uint32_t *compat_pvr, const char *basedesc,
+                             Error **errp);
 #endif /* defined(TARGET_PPC64) */
 
 #include "exec/cpu-all.h"
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 56a0ab2..e837cd2 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -33,6 +33,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/ppc/ppc.h"
 #include "mmu-book3s-v3.h"
+#include "sysemu/qtest.h"
 
 //#define PPC_DUMP_CPU
 //#define PPC_DEBUG_SPR
@@ -8413,73 +8414,38 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
     pcc->l1_icache_size = 0x10000;
 }
 
-static void powerpc_get_compat(Object *obj, Visitor *v, const char *name,
-                               void *opaque, Error **errp)
-{
-    char *value = (char *)"";
-    Property *prop = opaque;
-    uint32_t *max_compat = qdev_get_prop_ptr(DEVICE(obj), prop);
-
-    switch (*max_compat) {
-    case CPU_POWERPC_LOGICAL_2_05:
-        value = (char *)"power6";
-        break;
-    case CPU_POWERPC_LOGICAL_2_06:
-        value = (char *)"power7";
-        break;
-    case CPU_POWERPC_LOGICAL_2_07:
-        value = (char *)"power8";
-        break;
-    case 0:
-        break;
-    default:
-        error_report("Internal error: compat is set to %x", *max_compat);
-        abort();
-        break;
-    }
-
-    visit_type_str(v, name, &value, errp);
-}
-
-static void powerpc_set_compat(Object *obj, Visitor *v, const char *name,
-                               void *opaque, Error **errp)
+/*
+ * The CPU used to have a "compat" property which set the
+ * compatibility mode PVR.  However, this was conceptually broken - it
+ * only makes sense on the pseries machine type (otherwise the guest
+ * owns the PCR and can control the compatibility mode itself).  It's
+ * been replaced with the 'max-cpu-compat' property on the pseries
+ * machine type.  For backwards compatibility, pseries specially
+ * parses the -cpu parameter and converts old compat= parameters into
+ * the appropriate machine parameters.  This stub implementation of
+ * the parameter catches any uses on explicitly created CPUs.
+ */
+static void getset_compat_deprecated(Object *obj, Visitor *v, const char *name,
+                                     void *opaque, Error **errp)
 {
-    Error *error = NULL;
-    char *value = NULL;
-    Property *prop = opaque;
-    uint32_t *max_compat = qdev_get_prop_ptr(DEVICE(obj), prop);
-
-    visit_type_str(v, name, &value, &error);
-    if (error) {
-        error_propagate(errp, error);
-        return;
-    }
-
-    if (strcmp(value, "power6") == 0) {
-        *max_compat = CPU_POWERPC_LOGICAL_2_05;
-    } else if (strcmp(value, "power7") == 0) {
-        *max_compat = CPU_POWERPC_LOGICAL_2_06;
-    } else if (strcmp(value, "power8") == 0) {
-        *max_compat = CPU_POWERPC_LOGICAL_2_07;
-    } else {
-        error_setg(errp, "Invalid compatibility mode \"%s\"", value);
+    if (!qtest_enabled()) {
+        error_report("CPU 'compat' property is deprecated and has no effect; "
+                     "use max-cpu-compat machine property instead");
     }
-
-    g_free(value);
+    visit_type_null(v, name, NULL);
 }
 
-static PropertyInfo powerpc_compat_propinfo = {
+static PropertyInfo ppc_compat_deprecated_propinfo = {
     .name = "str",
-    .description = "compatibility mode, power6/power7/power8",
-    .get = powerpc_get_compat,
-    .set = powerpc_set_compat,
+    .description = "compatibility mode (deprecated)",
+    .get = getset_compat_deprecated,
+    .set = getset_compat_deprecated,
 };
-
-#define DEFINE_PROP_POWERPC_COMPAT(_n, _s, _f) \
-    DEFINE_PROP(_n, _s, _f, powerpc_compat_propinfo, uint32_t)
-
 static Property powerpc_servercpu_properties[] = {
-    DEFINE_PROP_POWERPC_COMPAT("compat", PowerPCCPU, max_compat),
+    {
+        .name = "compat",
+        .info = &ppc_compat_deprecated_propinfo,
+    },
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.9.4

[Qemu-devel] [PATCHv5 3/4] pseries: Reset CPU compatibility mode

[David Gibson]

Currently, the CPU compatibility mode is set when the cpu is initialized,
then again when the guest negotiates features.  This means if a guest
negotiates a compatibility mode, then reboots, that compatibility mode
will be retained across the reset.

Usually that will get overridden when features are negotiated on the next
boot, but it's still not really correct.  This patch moves the initial set
up of the compatibility mode from cpu init to reset time.  The mode *is*
retained if the reboot was caused by the feature negotiation (it might
be important in that case, though it's unlikely).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 hw/ppc/spapr.c          |  2 ++
 hw/ppc/spapr_cpu_core.c | 10 ----------
 2 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3c4e88f..2821b7e 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1348,6 +1348,8 @@ static void ppc_spapr_reset(void)
     if (!spapr->cas_reboot) {
         spapr_ovec_cleanup(spapr->ov5_cas);
         spapr->ov5_cas = spapr_ovec_new();
+
+        ppc_set_compat_all(spapr->max_compat_pvr, &error_abort);
     }
 
     fdt = spapr_build_fdt(spapr, rtas_addr, spapr->rtas_size);
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 846d9e7..7970871 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -121,16 +121,6 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
     /* Enable PAPR mode in TCG or KVM */
     cpu_ppc_set_papr(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
 
-    if (spapr->max_compat_pvr) {
-        Error *local_err = NULL;
-
-        ppc_set_compat(cpu, spapr->max_compat_pvr, &local_err);
-        if (local_err) {
-            error_propagate(errp, local_err);
-            return;
-        }
-    }
-
     qemu_register_reset(spapr_cpu_reset, cpu);
     spapr_cpu_reset(cpu);
 }
-- 
2.9.4

[Qemu-devel] [PATCHv5 4/4] ppc: Rework CPU compatibility testing across migration

[David Gibson]

Migrating between different CPU versions is a bit complicated for ppc.
A long time ago, we ensured identical CPU versions at either end by
checking the PVR had the same value.  However, this breaks under KVM
HV, because we always have to use the host's PVR - it's not
virtualized.  That would mean we couldn't migrate between hosts with
different PVRs, even if the CPUs are close enough to compatible in
practice (sometimes identical cores with different surrounding logic
have different PVRs, so this happens in practice quite often).

So, we removed the PVR check, but instead checked that several flags
indicating supported instructions matched.  This turns out to be a bad
idea, because those instruction masks are not architected information, but
essentially a TCG implementation detail.  So changes to qemu internal CPU
modelling can break migration - this happened between qemu-2.6 and
qemu-2.7.  That was addressed by 146c11f1 "target-ppc: Allow eventual
removal of old migration mistakes".

Now, verification of CPU compatibility across a migration basically doesn't
happen.  We simply ignore the PVR of the incoming migration, and hope the
cpu on the destination is close enough to work.

Now that we've cleaned up handling of processor compatibility modes for
pseries machine type, we can do better.  We allow migration if:

    * The source and destination PVRs are for the same type of CPU, as
      determined by CPU class's pvr_match function
OR  * When the source was in a compatibility mode, and the destination CPU
      supports the same compatibility mode

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 target/ppc/machine.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 66 insertions(+), 3 deletions(-)

diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 6cb3a48..a29aabe 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -8,6 +8,7 @@
 #include "helper_regs.h"
 #include "mmu-hash64.h"
 #include "migration/cpu.h"
+#include "qapi/error.h"
 
 static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
 {
@@ -195,6 +196,27 @@ static void cpu_pre_save(void *opaque)
     }
 }
 
+/*
+ * Determine if a given PVR is a "close enough" match to the CPU
+ * object.  For TCG and KVM PR it would probably be sufficient to
+ * require an exact PVR match.  However for KVM HV the user is
+ * restricted to a PVR exactly matching the host CPU.  The correct way
+ * to handle this is to put the guest into an architected
+ * compatibility mode.  However, to allow a more forgiving transition
+ * and migration from before this was widely done, we allow migration
+ * between sufficiently similar PVRs, as determined by the CPU class's
+ * pvr_match() hook.
+ */
+static bool pvr_match(PowerPCCPU *cpu, uint32_t pvr)
+{
+    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+
+    if (pvr == pcc->pvr) {
+        return true;
+    }
+    return pcc->pvr_match(pcc, pvr);
+}
+
 static int cpu_post_load(void *opaque, int version_id)
 {
     PowerPCCPU *cpu = opaque;
@@ -203,10 +225,31 @@ static int cpu_post_load(void *opaque, int version_id)
     target_ulong msr;
 
     /*
-     * We always ignore the source PVR. The user or management
-     * software has to take care of running QEMU in a compatible mode.
+     * If we're operating in compat mode, we should be ok as long as
+     * the destination supports the same compatiblity mode.
+     *
+     * Otherwise, however, we require that the destination has exactly
+     * the same CPU model as the source.
      */
-    env->spr[SPR_PVR] = env->spr_cb[SPR_PVR].default_value;
+
+#if defined(TARGET_PPC64)
+    if (cpu->compat_pvr) {
+        Error *local_err = NULL;
+
+        ppc_set_compat(cpu, cpu->compat_pvr, &local_err);
+        if (local_err) {
+            error_report_err(local_err);
+            error_free(local_err);
+            return -1;
+        }
+    } else
+#endif
+    {
+        if (!pvr_match(cpu, env->spr[SPR_PVR])) {
+            return -1;
+        }
+    }
+
     env->lr = env->spr[SPR_LR];
     env->ctr = env->spr[SPR_CTR];
     cpu_write_xer(env, env->spr[SPR_XER]);
@@ -560,6 +603,25 @@ static const VMStateDescription vmstate_tlbmas = {
     }
 };
 
+static bool compat_needed(void *opaque)
+{
+    PowerPCCPU *cpu = opaque;
+
+    assert(!(cpu->compat_pvr && !cpu->vhyp));
+    return (cpu->compat_pvr != 0);
+}
+
+static const VMStateDescription vmstate_compat = {
+    .name = "cpu/compat",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = compat_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(compat_pvr, PowerPCCPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_ppc_cpu = {
     .name = "cpu",
     .version_id = 5,
@@ -613,6 +675,7 @@ const VMStateDescription vmstate_ppc_cpu = {
         &vmstate_tlb6xx,
         &vmstate_tlbemb,
         &vmstate_tlbmas,
+        &vmstate_compat,
         NULL
     }
 };
-- 
2.9.4

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

[no-reply]

Hi,

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

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

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

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

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
Switched to a new branch 'test'
6ccf47f ppc: Rework CPU compatibility testing across migration
2b75df3 pseries: Reset CPU compatibility mode
282a89c pseries: Move CPU compatibility property to machine
8741a73 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...
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
#94: FILE: target/ppc/machine.c:236:
+    if (cpu->compat_pvr) {
[...]
+    } else
[...]

total: 1 errors, 0 warnings, 100 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

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2093 bytes --]

On Thu, Jun 01, 2017 at 08:55:19PM -0700, no-reply@patchew.org wrote:
> Hi,
> 
> This series seems to have some coding style problems. See output below for
> more information:
> 
> Type: series
> Message-id: 20170602031507.29881-1-david@gibson.dropbear.id.au
> Subject: [Qemu-devel] [PATCHv5 0/4] Clean up compatibility mode handling
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> 
> BASE=base
> n=1
> total=$(git log --oneline $BASE.. | wc -l)
> failed=0
> 
> 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
> Switched to a new branch 'test'
> 6ccf47f ppc: Rework CPU compatibility testing across migration
> 2b75df3 pseries: Reset CPU compatibility mode
> 282a89c pseries: Move CPU compatibility property to machine
> 8741a73 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...
> 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
> #94: FILE: target/ppc/machine.c:236:
> +    if (cpu->compat_pvr) {
> [...]
> +    } else
> [...]
> 
> total: 1 errors, 0 warnings, 100 lines checked

This is a false positive triggered by an #ifdef intersecting the
if/else.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1171 bytes --]

On Fri, Jun 02, 2017 at 01:15:03PM +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.

Anyone willing to give some review and/or testing, particularly for
patch 2/4.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [Qemu-ppc] [PATCHv5 2/4] pseries: Move CPU compatibility property to machine

[Suraj Jitindar Singh]

On Fri, 2017-06-02 at 13:15 +1000, David Gibson wrote:
> Server class POWER CPUs have a "compat" property, which is used to
> set the
> backwards compatibility mode for the processor.  However, this only
> makes
> sense for machine types which don't give the guest access to
> hypervisor
> privilege - otherwise the compatibility level is under the guest's
> control.
> 
> To reflect this, this removes the CPU 'compat' property and instead
> creates a 'max-cpu-compat' property on the pseries machine.  Strictly
> speaking this breaks compatibility, but AFAIK the 'compat' option was
> never (directly) used with -device or device_add.
> 
> The option was used with -cpu.  So, to maintain compatibility, this
> patch adds a hack to the cpu option parsing to strip out any compat
> options supplied with -cpu and set them on the machine property
> instead of the now deprecated cpu property.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Looks good to me and no longer segfaults :)

Tried a few configurations and behaves as expected:

Tested-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>

> ---
>  hw/ppc/spapr.c              |   6 ++-
>  hw/ppc/spapr_cpu_core.c     |  55 +++++++++++++++++++++++-
>  hw/ppc/spapr_hcall.c        |   8 ++--
>  include/hw/ppc/spapr.h      |  12 ++++--
>  target/ppc/compat.c         | 102
> ++++++++++++++++++++++++++++++++++++++++++++
>  target/ppc/cpu.h            |   5 ++-
>  target/ppc/translate_init.c |  86 +++++++++++-----------------------
> ---
>  7 files changed, 201 insertions(+), 73 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ab3aab1..3c4e88f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2134,7 +2134,7 @@ static void ppc_spapr_init(MachineState
> *machine)
>          machine->cpu_model = kvm_enabled() ? "host" : smc-
> >tcg_default_cpu;
>      }
>  
> -    ppc_cpu_parse_features(machine->cpu_model);
> +    spapr_cpu_parse_features(spapr);
>  
>      spapr_init_cpus(spapr);
>  
> @@ -2497,6 +2497,10 @@ static void spapr_machine_initfn(Object *obj)
>                                      " place of standard EPOW events
> when possible"
>                                      " (required for memory hot-
> unplug support)",
>                                      NULL);
> +
> +    ppc_compat_add_property(obj, "max-cpu-compat", &spapr-
> >max_compat_pvr,
> +                            "Maximum permitted CPU compatibility
> mode",
> +                            &error_fatal);
>  }
>  
>  static void spapr_machine_finalizefn(Object *obj)
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index ff7058e..846d9e7 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -20,6 +20,57 @@
>  #include "sysemu/numa.h"
>  #include "qemu/error-report.h"
>  
> +void spapr_cpu_parse_features(sPAPRMachineState *spapr)
> +{
> +    /*
> +     * Backwards compatibility hack:
> +     *
> +     *   CPUs had a "compat=" property which didn't make sense for
> +     *   anything except pseries.  It was replaced by "max-cpu-
> compat"
> +     *   machine option.  This supports old command lines like
> +     *       -cpu POWER8,compat=power7
> +     *   By stripping the compat option and applying it to the
> machine
> +     *   before passing it on to the cpu level parser.
> +     */
> +    gchar **inpieces;
> +    int i, j;
> +    gchar *compat_str = NULL;
> +
> +    inpieces = g_strsplit(MACHINE(spapr)->cpu_model, ",", 0);
> +
> +    /* inpieces[0] is the actual model string */
> +    i = 1;
> +    j = 1;
> +    while (inpieces[i]) {
> +        if (g_str_has_prefix(inpieces[i], "compat=")) {
> +            /* in case of multiple compat= options */
> +            g_free(compat_str);
> +            compat_str = inpieces[i];
> +        } else {
> +            j++;
> +        }
> +
> +        i++;
> +        /* Excise compat options from list */
> +        inpieces[j] = inpieces[i];
> +    }
> +
> +    if (compat_str) {
> +        char *val = compat_str + strlen("compat=");
> +        gchar *newprops = g_strjoinv(",", inpieces);
> +
> +        object_property_set_str(OBJECT(spapr), val, "max-cpu-
> compat",
> +                                &error_fatal);
> +
> +        ppc_cpu_parse_features(newprops);
> +        g_free(newprops);
> +    } else {
> +        ppc_cpu_parse_features(MACHINE(spapr)->cpu_model);
> +    }
> +
> +    g_strfreev(inpieces);
> +}
> +
>  static void spapr_cpu_reset(void *opaque)
>  {
>      sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> @@ -70,10 +121,10 @@ static void spapr_cpu_init(sPAPRMachineState
> *spapr, PowerPCCPU *cpu,
>      /* Enable PAPR mode in TCG or KVM */
>      cpu_ppc_set_papr(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
>  
> -    if (cpu->max_compat) {
> +    if (spapr->max_compat_pvr) {
>          Error *local_err = NULL;
>  
> -        ppc_set_compat(cpu, cpu->max_compat, &local_err);
> +        ppc_set_compat(cpu, spapr->max_compat_pvr, &local_err);
>          if (local_err) {
>              error_propagate(errp, local_err);
>              return;
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index aae5a62..a9bb3ed 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1044,11 +1044,11 @@ static target_ulong
> h_signal_sys_reset(PowerPCCPU *cpu,
>      }
>  }
>  
> -static uint32_t cas_check_pvr(PowerPCCPU *cpu, target_ulong *addr,
> -                              Error **errp)
> +static uint32_t cas_check_pvr(sPAPRMachineState *spapr, PowerPCCPU
> *cpu,
> +                              target_ulong *addr, Error **errp)
>  {
>      bool explicit_match = false; /* Matched the CPU's real PVR */
> -    uint32_t max_compat = cpu->max_compat;
> +    uint32_t max_compat = spapr->max_compat_pvr;
>      uint32_t best_compat = 0;
>      int i;
>  
> @@ -1104,7 +1104,7 @@ static target_ulong
> h_client_architecture_support(PowerPCCPU *cpu,
>      bool guest_radix;
>      Error *local_err = NULL;
>  
> -    cas_pvr = cas_check_pvr(cpu, &addr, &local_err);
> +    cas_pvr = cas_check_pvr(spapr, cpu, &addr, &local_err);
>      if (local_err) {
>          error_report_err(local_err);
>          return H_HARDWARE;
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 98fb78b..4da92e2 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -87,16 +87,19 @@ struct sPAPRMachineState {
>      uint64_t rtc_offset; /* Now used only during incoming migration
> */
>      struct PPCTimebase tb;
>      bool has_graphics;
> -    sPAPROptionVector *ov5;         /* QEMU-supported option vectors
> */
> -    sPAPROptionVector *ov5_cas;     /* negotiated (via CAS) option
> vectors */
> -    bool cas_reboot;
> -    bool cas_legacy_guest_workaround;
>  
>      Notifier epow_notifier;
>      QTAILQ_HEAD(, sPAPREventLogEntry) pending_events;
>      bool use_hotplug_event_source;
>      sPAPREventSource *event_sources;
>  
> +    /* ibm,client-architecture-support option negotiation */
> +    bool cas_reboot;
> +    bool cas_legacy_guest_workaround;
> +    sPAPROptionVector *ov5;         /* QEMU-supported option vectors
> */
> +    sPAPROptionVector *ov5_cas;     /* negotiated (via CAS) option
> vectors */
> +    uint32_t max_compat_pvr;
> +
>      /* Migration state */
>      int htab_save_index;
>      bool htab_first_pass;
> @@ -639,6 +642,7 @@ void
> spapr_hotplug_req_add_by_count_indexed(sPAPRDRConnectorType drc_type,
>                                              uint32_t count, uint32_t
> index);
>  void spapr_hotplug_req_remove_by_count_indexed(sPAPRDRConnectorType
> drc_type,
>                                                 uint32_t count,
> uint32_t index);
> +void spapr_cpu_parse_features(sPAPRMachineState *spapr);
>  void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
>                                      sPAPRMachineState *spapr);
>  
> diff --git a/target/ppc/compat.c b/target/ppc/compat.c
> index e8ec1e1..e72839f 100644
> --- a/target/ppc/compat.c
> +++ b/target/ppc/compat.c
> @@ -24,9 +24,11 @@
>  #include "sysemu/cpus.h"
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
> +#include "qapi/visitor.h"
>  #include "cpu-models.h"
>  
>  typedef struct {
> +    const char *name;
>      uint32_t pvr;
>      uint64_t pcr;
>      uint64_t pcr_level;
> @@ -38,6 +40,7 @@ static const CompatInfo compat_table[] = {
>       * Ordered from oldest to newest - the code relies on this
>       */
>      { /* POWER6, ISA2.05 */
> +        .name = "power6",
>          .pvr = CPU_POWERPC_LOGICAL_2_05,
>          .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
>                 PCR_COMPAT_2_05 | PCR_TM_DIS | PCR_VSX_DIS,
> @@ -45,24 +48,28 @@ static const CompatInfo compat_table[] = {
>          .max_threads = 2,
>      },
>      { /* POWER7, ISA2.06 */
> +        .name = "power7",
>          .pvr = CPU_POWERPC_LOGICAL_2_06,
>          .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
> PCR_TM_DIS,
>          .pcr_level = PCR_COMPAT_2_06,
>          .max_threads = 4,
>      },
>      {
> +        .name = "power7+",
>          .pvr = CPU_POWERPC_LOGICAL_2_06_PLUS,
>          .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
> PCR_TM_DIS,
>          .pcr_level = PCR_COMPAT_2_06,
>          .max_threads = 4,
>      },
>      { /* POWER8, ISA2.07 */
> +        .name = "power8",
>          .pvr = CPU_POWERPC_LOGICAL_2_07,
>          .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07,
>          .pcr_level = PCR_COMPAT_2_07,
>          .max_threads = 8,
>      },
>      { /* POWER9, ISA3.00 */
> +        .name = "power9",
>          .pvr = CPU_POWERPC_LOGICAL_3_00,
>          .pcr = PCR_COMPAT_3_00,
>          .pcr_level = PCR_COMPAT_3_00,
> @@ -189,3 +196,98 @@ int ppc_compat_max_threads(PowerPCCPU *cpu)
>  
>      return n_threads;
>  }
> +
> +static void ppc_compat_prop_get(Object *obj, Visitor *v, const char
> *name,
> +                                void *opaque, Error **errp)
> +{
> +    uint32_t compat_pvr = *((uint32_t *)opaque);
> +    const char *value;
> +
> +    if (!compat_pvr) {
> +        value = "";
> +    } else {
> +        const CompatInfo *compat = compat_by_pvr(compat_pvr);
> +
> +        g_assert(compat);
> +
> +        value = compat->name;
> +    }
> +
> +    visit_type_str(v, name, (char **)&value, errp);
> +}
> +
> +static void ppc_compat_prop_set(Object *obj, Visitor *v, const char
> *name,
> +                                void *opaque, Error **errp)
> +{
> +    Error *error = NULL;
> +    char *value;
> +    uint32_t compat_pvr;
> +
> +    visit_type_str(v, name, &value, &error);
> +    if (error) {
> +        error_propagate(errp, error);
> +        return;
> +    }
> +
> +    if (strcmp(value, "") == 0) {
> +        compat_pvr = 0;
> +    } else {
> +        int i;
> +        const CompatInfo *compat = NULL;
> +
> +        for (i = 0; i < ARRAY_SIZE(compat_table); i++) {
> +            if (strcmp(value, compat_table[i].name) == 0) {
> +                compat = &compat_table[i];
> +                break;
> +
> +            }
> +        }
> +
> +        if (!compat) {
> +            error_setg(errp, "Invalid compatibility mode \"%s\"",
> value);
> +            goto out;
> +        }
> +        compat_pvr = compat->pvr;
> +    }
> +
> +    *((uint32_t *)opaque) = compat_pvr;
> +
> +out:
> +    g_free(value);
> +}
> +
> +void ppc_compat_add_property(Object *obj, const char *name,
> +                             uint32_t *compat_pvr, const char
> *basedesc,
> +                             Error **errp)
> +{
> +    Error *local_err = NULL;
> +    gchar *namesv[ARRAY_SIZE(compat_table) + 1];
> +    gchar *names, *desc;
> +    int i;
> +
> +    object_property_add(obj, name, "string",
> +                        ppc_compat_prop_get, ppc_compat_prop_set,
> NULL,
> +                        compat_pvr, &local_err);
> +    if (local_err) {
> +        goto out;
> +    }
> +
> +    for (i = 0; i < ARRAY_SIZE(compat_table); i++) {
> +        /*
> +         * Have to discard const here, because g_strjoinv() takes
> +         * (gchar **), not (const gchar **) :(
> +         */
> +        namesv[i] = (gchar *)compat_table[i].name;
> +    }
> +    namesv[ARRAY_SIZE(compat_table)] = NULL;
> +
> +    names = g_strjoinv(", ", namesv);
> +    desc = g_strdup_printf("%s. Valid values are %s.", basedesc,
> names);
> +    object_property_set_description(obj, name, desc, &local_err);
> +
> +    g_free(names);
> +    g_free(desc);
> +
> +out:
> +    error_propagate(errp, local_err);
> +}
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 401e10e..4517b4b 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1189,7 +1189,6 @@ typedef struct PPCVirtualHypervisorClass
> PPCVirtualHypervisorClass;
>   * PowerPCCPU:
>   * @env: #CPUPPCState
>   * @cpu_dt_id: CPU index used in the device tree. KVM uses this
> index too
> - * @max_compat: Maximal supported logical PVR from the command line
>   * @compat_pvr: Current logical PVR, zero if in "raw" mode
>   *
>   * A PowerPC CPU.
> @@ -1201,7 +1200,6 @@ struct PowerPCCPU {
>  
>      CPUPPCState env;
>      int cpu_dt_id;
> -    uint32_t max_compat;
>      uint32_t compat_pvr;
>      PPCVirtualHypervisor *vhyp;
>      Object *intc;
> @@ -1374,6 +1372,9 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t
> compat_pvr, Error **errp);
>  void ppc_set_compat_all(uint32_t compat_pvr, Error **errp);
>  #endif
>  int ppc_compat_max_threads(PowerPCCPU *cpu);
> +void ppc_compat_add_property(Object *obj, const char *name,
> +                             uint32_t *compat_pvr, const char
> *basedesc,
> +                             Error **errp);
>  #endif /* defined(TARGET_PPC64) */
>  
>  #include "exec/cpu-all.h"
> diff --git a/target/ppc/translate_init.c
> b/target/ppc/translate_init.c
> index 56a0ab2..e837cd2 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -33,6 +33,7 @@
>  #include "hw/qdev-properties.h"
>  #include "hw/ppc/ppc.h"
>  #include "mmu-book3s-v3.h"
> +#include "sysemu/qtest.h"
>  
>  //#define PPC_DUMP_CPU
>  //#define PPC_DEBUG_SPR
> @@ -8413,73 +8414,38 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void
> *data)
>      pcc->l1_icache_size = 0x10000;
>  }
>  
> -static void powerpc_get_compat(Object *obj, Visitor *v, const char
> *name,
> -                               void *opaque, Error **errp)
> -{
> -    char *value = (char *)"";
> -    Property *prop = opaque;
> -    uint32_t *max_compat = qdev_get_prop_ptr(DEVICE(obj), prop);
> -
> -    switch (*max_compat) {
> -    case CPU_POWERPC_LOGICAL_2_05:
> -        value = (char *)"power6";
> -        break;
> -    case CPU_POWERPC_LOGICAL_2_06:
> -        value = (char *)"power7";
> -        break;
> -    case CPU_POWERPC_LOGICAL_2_07:
> -        value = (char *)"power8";
> -        break;
> -    case 0:
> -        break;
> -    default:
> -        error_report("Internal error: compat is set to %x",
> *max_compat);
> -        abort();
> -        break;
> -    }
> -
> -    visit_type_str(v, name, &value, errp);
> -}
> -
> -static void powerpc_set_compat(Object *obj, Visitor *v, const char
> *name,
> -                               void *opaque, Error **errp)
> +/*
> + * The CPU used to have a "compat" property which set the
> + * compatibility mode PVR.  However, this was conceptually broken -
> it
> + * only makes sense on the pseries machine type (otherwise the guest
> + * owns the PCR and can control the compatibility mode
> itself).  It's
> + * been replaced with the 'max-cpu-compat' property on the pseries
> + * machine type.  For backwards compatibility, pseries specially
> + * parses the -cpu parameter and converts old compat= parameters
> into
> + * the appropriate machine parameters.  This stub implementation of
> + * the parameter catches any uses on explicitly created CPUs.
> + */
> +static void getset_compat_deprecated(Object *obj, Visitor *v, const
> char *name,
> +                                     void *opaque, Error **errp)
>  {
> -    Error *error = NULL;
> -    char *value = NULL;
> -    Property *prop = opaque;
> -    uint32_t *max_compat = qdev_get_prop_ptr(DEVICE(obj), prop);
> -
> -    visit_type_str(v, name, &value, &error);
> -    if (error) {
> -        error_propagate(errp, error);
> -        return;
> -    }
> -
> -    if (strcmp(value, "power6") == 0) {
> -        *max_compat = CPU_POWERPC_LOGICAL_2_05;
> -    } else if (strcmp(value, "power7") == 0) {
> -        *max_compat = CPU_POWERPC_LOGICAL_2_06;
> -    } else if (strcmp(value, "power8") == 0) {
> -        *max_compat = CPU_POWERPC_LOGICAL_2_07;
> -    } else {
> -        error_setg(errp, "Invalid compatibility mode \"%s\"",
> value);
> +    if (!qtest_enabled()) {
> +        error_report("CPU 'compat' property is deprecated and has no
> effect; "
> +                     "use max-cpu-compat machine property instead");
>      }
> -
> -    g_free(value);
> +    visit_type_null(v, name, NULL);
>  }
>  
> -static PropertyInfo powerpc_compat_propinfo = {
> +static PropertyInfo ppc_compat_deprecated_propinfo = {
>      .name = "str",
> -    .description = "compatibility mode, power6/power7/power8",
> -    .get = powerpc_get_compat,
> -    .set = powerpc_set_compat,
> +    .description = "compatibility mode (deprecated)",
> +    .get = getset_compat_deprecated,
> +    .set = getset_compat_deprecated,
>  };
> -
> -#define DEFINE_PROP_POWERPC_COMPAT(_n, _s, _f) \
> -    DEFINE_PROP(_n, _s, _f, powerpc_compat_propinfo, uint32_t)
> -
>  static Property powerpc_servercpu_properties[] = {
> -    DEFINE_PROP_POWERPC_COMPAT("compat", PowerPCCPU, max_compat),
> +    {
> +        .name = "compat",
> +        .info = &ppc_compat_deprecated_propinfo,
> +    },
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  

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

[Suraj Jitindar Singh]

On Thu, 2017-06-08 at 14:17 +1000, David Gibson wrote:
> On Fri, Jun 02, 2017 at 01:15:03PM +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.
> 
> Anyone willing to give some review and/or testing, particularly for
> patch 2/4.
> 

Sorry I had been playing around with this and meant to give it a
Tested-by

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

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 1199 bytes --]

On Thu, 8 Jun 2017 14:17:39 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Jun 02, 2017 at 01:15:03PM +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.  
> 
> Anyone willing to give some review and/or testing, particularly for
> patch 2/4.
> 

I'm planning to re-run all the tests when migration of pre-2.10 machine
types is fixed.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PATCHv5 2/4] pseries: Move CPU compatibility property to machine

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 18257 bytes --]

On Fri,  2 Jun 2017 13:15:05 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> Server class POWER CPUs have a "compat" property, which is used to set the
> backwards compatibility mode for the processor.  However, this only makes
> sense for machine types which don't give the guest access to hypervisor
> privilege - otherwise the compatibility level is under the guest's control.
> 
> To reflect this, this removes the CPU 'compat' property and instead
> creates a 'max-cpu-compat' property on the pseries machine.  Strictly
> speaking this breaks compatibility, but AFAIK the 'compat' option was
> never (directly) used with -device or device_add.
> 
> The option was used with -cpu.  So, to maintain compatibility, this
> patch adds a hack to the cpu option parsing to strip out any compat
> options supplied with -cpu and set them on the machine property
> instead of the now deprecated cpu property.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Just one trivial style nit in ppc_compat_prop_set().

Reviewed-by: Greg Kurz <groug@kaod.org>

and

Tested-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/spapr.c              |   6 ++-
>  hw/ppc/spapr_cpu_core.c     |  55 +++++++++++++++++++++++-
>  hw/ppc/spapr_hcall.c        |   8 ++--
>  include/hw/ppc/spapr.h      |  12 ++++--
>  target/ppc/compat.c         | 102 ++++++++++++++++++++++++++++++++++++++++++++
>  target/ppc/cpu.h            |   5 ++-
>  target/ppc/translate_init.c |  86 +++++++++++--------------------------
>  7 files changed, 201 insertions(+), 73 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ab3aab1..3c4e88f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2134,7 +2134,7 @@ static void ppc_spapr_init(MachineState *machine)
>          machine->cpu_model = kvm_enabled() ? "host" : smc->tcg_default_cpu;
>      }
>  
> -    ppc_cpu_parse_features(machine->cpu_model);
> +    spapr_cpu_parse_features(spapr);
>  
>      spapr_init_cpus(spapr);
>  
> @@ -2497,6 +2497,10 @@ static void spapr_machine_initfn(Object *obj)
>                                      " place of standard EPOW events when possible"
>                                      " (required for memory hot-unplug support)",
>                                      NULL);
> +
> +    ppc_compat_add_property(obj, "max-cpu-compat", &spapr->max_compat_pvr,
> +                            "Maximum permitted CPU compatibility mode",
> +                            &error_fatal);
>  }
>  
>  static void spapr_machine_finalizefn(Object *obj)
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index ff7058e..846d9e7 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -20,6 +20,57 @@
>  #include "sysemu/numa.h"
>  #include "qemu/error-report.h"
>  
> +void spapr_cpu_parse_features(sPAPRMachineState *spapr)
> +{
> +    /*
> +     * Backwards compatibility hack:
> +     *
> +     *   CPUs had a "compat=" property which didn't make sense for
> +     *   anything except pseries.  It was replaced by "max-cpu-compat"
> +     *   machine option.  This supports old command lines like
> +     *       -cpu POWER8,compat=power7
> +     *   By stripping the compat option and applying it to the machine
> +     *   before passing it on to the cpu level parser.
> +     */
> +    gchar **inpieces;
> +    int i, j;
> +    gchar *compat_str = NULL;
> +
> +    inpieces = g_strsplit(MACHINE(spapr)->cpu_model, ",", 0);
> +
> +    /* inpieces[0] is the actual model string */
> +    i = 1;
> +    j = 1;
> +    while (inpieces[i]) {
> +        if (g_str_has_prefix(inpieces[i], "compat=")) {
> +            /* in case of multiple compat= options */
> +            g_free(compat_str);
> +            compat_str = inpieces[i];
> +        } else {
> +            j++;
> +        }
> +
> +        i++;
> +        /* Excise compat options from list */
> +        inpieces[j] = inpieces[i];
> +    }
> +
> +    if (compat_str) {
> +        char *val = compat_str + strlen("compat=");
> +        gchar *newprops = g_strjoinv(",", inpieces);
> +
> +        object_property_set_str(OBJECT(spapr), val, "max-cpu-compat",
> +                                &error_fatal);
> +
> +        ppc_cpu_parse_features(newprops);
> +        g_free(newprops);
> +    } else {
> +        ppc_cpu_parse_features(MACHINE(spapr)->cpu_model);
> +    }
> +
> +    g_strfreev(inpieces);
> +}
> +
>  static void spapr_cpu_reset(void *opaque)
>  {
>      sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> @@ -70,10 +121,10 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
>      /* Enable PAPR mode in TCG or KVM */
>      cpu_ppc_set_papr(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
>  
> -    if (cpu->max_compat) {
> +    if (spapr->max_compat_pvr) {
>          Error *local_err = NULL;
>  
> -        ppc_set_compat(cpu, cpu->max_compat, &local_err);
> +        ppc_set_compat(cpu, spapr->max_compat_pvr, &local_err);
>          if (local_err) {
>              error_propagate(errp, local_err);
>              return;
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index aae5a62..a9bb3ed 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1044,11 +1044,11 @@ static target_ulong h_signal_sys_reset(PowerPCCPU *cpu,
>      }
>  }
>  
> -static uint32_t cas_check_pvr(PowerPCCPU *cpu, target_ulong *addr,
> -                              Error **errp)
> +static uint32_t cas_check_pvr(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> +                              target_ulong *addr, Error **errp)
>  {
>      bool explicit_match = false; /* Matched the CPU's real PVR */
> -    uint32_t max_compat = cpu->max_compat;
> +    uint32_t max_compat = spapr->max_compat_pvr;
>      uint32_t best_compat = 0;
>      int i;
>  
> @@ -1104,7 +1104,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>      bool guest_radix;
>      Error *local_err = NULL;
>  
> -    cas_pvr = cas_check_pvr(cpu, &addr, &local_err);
> +    cas_pvr = cas_check_pvr(spapr, cpu, &addr, &local_err);
>      if (local_err) {
>          error_report_err(local_err);
>          return H_HARDWARE;
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 98fb78b..4da92e2 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -87,16 +87,19 @@ struct sPAPRMachineState {
>      uint64_t rtc_offset; /* Now used only during incoming migration */
>      struct PPCTimebase tb;
>      bool has_graphics;
> -    sPAPROptionVector *ov5;         /* QEMU-supported option vectors */
> -    sPAPROptionVector *ov5_cas;     /* negotiated (via CAS) option vectors */
> -    bool cas_reboot;
> -    bool cas_legacy_guest_workaround;
>  
>      Notifier epow_notifier;
>      QTAILQ_HEAD(, sPAPREventLogEntry) pending_events;
>      bool use_hotplug_event_source;
>      sPAPREventSource *event_sources;
>  
> +    /* ibm,client-architecture-support option negotiation */
> +    bool cas_reboot;
> +    bool cas_legacy_guest_workaround;
> +    sPAPROptionVector *ov5;         /* QEMU-supported option vectors */
> +    sPAPROptionVector *ov5_cas;     /* negotiated (via CAS) option vectors */
> +    uint32_t max_compat_pvr;
> +
>      /* Migration state */
>      int htab_save_index;
>      bool htab_first_pass;
> @@ -639,6 +642,7 @@ void spapr_hotplug_req_add_by_count_indexed(sPAPRDRConnectorType drc_type,
>                                              uint32_t count, uint32_t index);
>  void spapr_hotplug_req_remove_by_count_indexed(sPAPRDRConnectorType drc_type,
>                                                 uint32_t count, uint32_t index);
> +void spapr_cpu_parse_features(sPAPRMachineState *spapr);
>  void *spapr_populate_hotplug_cpu_dt(CPUState *cs, int *fdt_offset,
>                                      sPAPRMachineState *spapr);
>  
> diff --git a/target/ppc/compat.c b/target/ppc/compat.c
> index e8ec1e1..e72839f 100644
> --- a/target/ppc/compat.c
> +++ b/target/ppc/compat.c
> @@ -24,9 +24,11 @@
>  #include "sysemu/cpus.h"
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
> +#include "qapi/visitor.h"
>  #include "cpu-models.h"
>  
>  typedef struct {
> +    const char *name;
>      uint32_t pvr;
>      uint64_t pcr;
>      uint64_t pcr_level;
> @@ -38,6 +40,7 @@ static const CompatInfo compat_table[] = {
>       * Ordered from oldest to newest - the code relies on this
>       */
>      { /* POWER6, ISA2.05 */
> +        .name = "power6",
>          .pvr = CPU_POWERPC_LOGICAL_2_05,
>          .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 |
>                 PCR_COMPAT_2_05 | PCR_TM_DIS | PCR_VSX_DIS,
> @@ -45,24 +48,28 @@ static const CompatInfo compat_table[] = {
>          .max_threads = 2,
>      },
>      { /* POWER7, ISA2.06 */
> +        .name = "power7",
>          .pvr = CPU_POWERPC_LOGICAL_2_06,
>          .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
>          .pcr_level = PCR_COMPAT_2_06,
>          .max_threads = 4,
>      },
>      {
> +        .name = "power7+",
>          .pvr = CPU_POWERPC_LOGICAL_2_06_PLUS,
>          .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_TM_DIS,
>          .pcr_level = PCR_COMPAT_2_06,
>          .max_threads = 4,
>      },
>      { /* POWER8, ISA2.07 */
> +        .name = "power8",
>          .pvr = CPU_POWERPC_LOGICAL_2_07,
>          .pcr = PCR_COMPAT_3_00 | PCR_COMPAT_2_07,
>          .pcr_level = PCR_COMPAT_2_07,
>          .max_threads = 8,
>      },
>      { /* POWER9, ISA3.00 */
> +        .name = "power9",
>          .pvr = CPU_POWERPC_LOGICAL_3_00,
>          .pcr = PCR_COMPAT_3_00,
>          .pcr_level = PCR_COMPAT_3_00,
> @@ -189,3 +196,98 @@ int ppc_compat_max_threads(PowerPCCPU *cpu)
>  
>      return n_threads;
>  }
> +
> +static void ppc_compat_prop_get(Object *obj, Visitor *v, const char *name,
> +                                void *opaque, Error **errp)
> +{
> +    uint32_t compat_pvr = *((uint32_t *)opaque);
> +    const char *value;
> +
> +    if (!compat_pvr) {
> +        value = "";
> +    } else {
> +        const CompatInfo *compat = compat_by_pvr(compat_pvr);
> +
> +        g_assert(compat);
> +
> +        value = compat->name;
> +    }
> +
> +    visit_type_str(v, name, (char **)&value, errp);
> +}
> +
> +static void ppc_compat_prop_set(Object *obj, Visitor *v, const char *name,
> +                                void *opaque, Error **errp)
> +{
> +    Error *error = NULL;

s/error/local_error maybe ? For consistency with ppc_compat_add_property() below.

> +    char *value;
> +    uint32_t compat_pvr;
> +
> +    visit_type_str(v, name, &value, &error);
> +    if (error) {
> +        error_propagate(errp, error);
> +        return;
> +    }
> +
> +    if (strcmp(value, "") == 0) {
> +        compat_pvr = 0;
> +    } else {
> +        int i;
> +        const CompatInfo *compat = NULL;
> +
> +        for (i = 0; i < ARRAY_SIZE(compat_table); i++) {
> +            if (strcmp(value, compat_table[i].name) == 0) {
> +                compat = &compat_table[i];
> +                break;
> +
> +            }
> +        }
> +
> +        if (!compat) {
> +            error_setg(errp, "Invalid compatibility mode \"%s\"", value);
> +            goto out;
> +        }
> +        compat_pvr = compat->pvr;
> +    }
> +
> +    *((uint32_t *)opaque) = compat_pvr;
> +
> +out:
> +    g_free(value);
> +}
> +
> +void ppc_compat_add_property(Object *obj, const char *name,
> +                             uint32_t *compat_pvr, const char *basedesc,
> +                             Error **errp)
> +{
> +    Error *local_err = NULL;
> +    gchar *namesv[ARRAY_SIZE(compat_table) + 1];
> +    gchar *names, *desc;
> +    int i;
> +
> +    object_property_add(obj, name, "string",
> +                        ppc_compat_prop_get, ppc_compat_prop_set, NULL,
> +                        compat_pvr, &local_err);
> +    if (local_err) {
> +        goto out;
> +    }
> +
> +    for (i = 0; i < ARRAY_SIZE(compat_table); i++) {
> +        /*
> +         * Have to discard const here, because g_strjoinv() takes
> +         * (gchar **), not (const gchar **) :(
> +         */
> +        namesv[i] = (gchar *)compat_table[i].name;
> +    }
> +    namesv[ARRAY_SIZE(compat_table)] = NULL;
> +
> +    names = g_strjoinv(", ", namesv);
> +    desc = g_strdup_printf("%s. Valid values are %s.", basedesc, names);
> +    object_property_set_description(obj, name, desc, &local_err);
> +
> +    g_free(names);
> +    g_free(desc);
> +
> +out:
> +    error_propagate(errp, local_err);
> +}
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 401e10e..4517b4b 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1189,7 +1189,6 @@ typedef struct PPCVirtualHypervisorClass PPCVirtualHypervisorClass;
>   * PowerPCCPU:
>   * @env: #CPUPPCState
>   * @cpu_dt_id: CPU index used in the device tree. KVM uses this index too
> - * @max_compat: Maximal supported logical PVR from the command line
>   * @compat_pvr: Current logical PVR, zero if in "raw" mode
>   *
>   * A PowerPC CPU.
> @@ -1201,7 +1200,6 @@ struct PowerPCCPU {
>  
>      CPUPPCState env;
>      int cpu_dt_id;
> -    uint32_t max_compat;
>      uint32_t compat_pvr;
>      PPCVirtualHypervisor *vhyp;
>      Object *intc;
> @@ -1374,6 +1372,9 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr, Error **errp);
>  void ppc_set_compat_all(uint32_t compat_pvr, Error **errp);
>  #endif
>  int ppc_compat_max_threads(PowerPCCPU *cpu);
> +void ppc_compat_add_property(Object *obj, const char *name,
> +                             uint32_t *compat_pvr, const char *basedesc,
> +                             Error **errp);
>  #endif /* defined(TARGET_PPC64) */
>  
>  #include "exec/cpu-all.h"
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index 56a0ab2..e837cd2 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -33,6 +33,7 @@
>  #include "hw/qdev-properties.h"
>  #include "hw/ppc/ppc.h"
>  #include "mmu-book3s-v3.h"
> +#include "sysemu/qtest.h"
>  
>  //#define PPC_DUMP_CPU
>  //#define PPC_DEBUG_SPR
> @@ -8413,73 +8414,38 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
>      pcc->l1_icache_size = 0x10000;
>  }
>  
> -static void powerpc_get_compat(Object *obj, Visitor *v, const char *name,
> -                               void *opaque, Error **errp)
> -{
> -    char *value = (char *)"";
> -    Property *prop = opaque;
> -    uint32_t *max_compat = qdev_get_prop_ptr(DEVICE(obj), prop);
> -
> -    switch (*max_compat) {
> -    case CPU_POWERPC_LOGICAL_2_05:
> -        value = (char *)"power6";
> -        break;
> -    case CPU_POWERPC_LOGICAL_2_06:
> -        value = (char *)"power7";
> -        break;
> -    case CPU_POWERPC_LOGICAL_2_07:
> -        value = (char *)"power8";
> -        break;
> -    case 0:
> -        break;
> -    default:
> -        error_report("Internal error: compat is set to %x", *max_compat);
> -        abort();
> -        break;
> -    }
> -
> -    visit_type_str(v, name, &value, errp);
> -}
> -
> -static void powerpc_set_compat(Object *obj, Visitor *v, const char *name,
> -                               void *opaque, Error **errp)
> +/*
> + * The CPU used to have a "compat" property which set the
> + * compatibility mode PVR.  However, this was conceptually broken - it
> + * only makes sense on the pseries machine type (otherwise the guest
> + * owns the PCR and can control the compatibility mode itself).  It's
> + * been replaced with the 'max-cpu-compat' property on the pseries
> + * machine type.  For backwards compatibility, pseries specially
> + * parses the -cpu parameter and converts old compat= parameters into
> + * the appropriate machine parameters.  This stub implementation of
> + * the parameter catches any uses on explicitly created CPUs.
> + */
> +static void getset_compat_deprecated(Object *obj, Visitor *v, const char *name,
> +                                     void *opaque, Error **errp)
>  {
> -    Error *error = NULL;
> -    char *value = NULL;
> -    Property *prop = opaque;
> -    uint32_t *max_compat = qdev_get_prop_ptr(DEVICE(obj), prop);
> -
> -    visit_type_str(v, name, &value, &error);
> -    if (error) {
> -        error_propagate(errp, error);
> -        return;
> -    }
> -
> -    if (strcmp(value, "power6") == 0) {
> -        *max_compat = CPU_POWERPC_LOGICAL_2_05;
> -    } else if (strcmp(value, "power7") == 0) {
> -        *max_compat = CPU_POWERPC_LOGICAL_2_06;
> -    } else if (strcmp(value, "power8") == 0) {
> -        *max_compat = CPU_POWERPC_LOGICAL_2_07;
> -    } else {
> -        error_setg(errp, "Invalid compatibility mode \"%s\"", value);
> +    if (!qtest_enabled()) {
> +        error_report("CPU 'compat' property is deprecated and has no effect; "
> +                     "use max-cpu-compat machine property instead");
>      }
> -
> -    g_free(value);
> +    visit_type_null(v, name, NULL);
>  }
>  
> -static PropertyInfo powerpc_compat_propinfo = {
> +static PropertyInfo ppc_compat_deprecated_propinfo = {
>      .name = "str",
> -    .description = "compatibility mode, power6/power7/power8",
> -    .get = powerpc_get_compat,
> -    .set = powerpc_set_compat,
> +    .description = "compatibility mode (deprecated)",
> +    .get = getset_compat_deprecated,
> +    .set = getset_compat_deprecated,
>  };
> -
> -#define DEFINE_PROP_POWERPC_COMPAT(_n, _s, _f) \
> -    DEFINE_PROP(_n, _s, _f, powerpc_compat_propinfo, uint32_t)
> -
>  static Property powerpc_servercpu_properties[] = {
> -    DEFINE_PROP_POWERPC_COMPAT("compat", PowerPCCPU, max_compat),
> +    {
> +        .name = "compat",
> +        .info = &ppc_compat_deprecated_propinfo,
> +    },
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

[Andrea Bolognani]

On Fri, 2017-06-02 at 13:15 +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.

I've tested this the same way I had tested one of the
previous respins, eg. for basic usage and general sanity
of the interface. All the issues I pointed out last time
seems to have been addressed.


Tested-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [Qemu-devel] [PATCHv5 2/4] pseries: Move CPU compatibility property to machine

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1857 bytes --]

On Fri, Jun 09, 2017 at 03:55:56PM +0200, Greg Kurz wrote:
> On Fri,  2 Jun 2017 13:15:05 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > Server class POWER CPUs have a "compat" property, which is used to set the
> > backwards compatibility mode for the processor.  However, this only makes
> > sense for machine types which don't give the guest access to hypervisor
> > privilege - otherwise the compatibility level is under the guest's control.
> > 
> > To reflect this, this removes the CPU 'compat' property and instead
> > creates a 'max-cpu-compat' property on the pseries machine.  Strictly
> > speaking this breaks compatibility, but AFAIK the 'compat' option was
> > never (directly) used with -device or device_add.
> > 
> > The option was used with -cpu.  So, to maintain compatibility, this
> > patch adds a hack to the cpu option parsing to strip out any compat
> > options supplied with -cpu and set them on the machine property
> > instead of the now deprecated cpu property.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> 
> Just one trivial style nit in ppc_compat_prop_set().
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> and
> 
> Tested-by: Greg Kurz <groug@kaod.org>

Thanks.

[snip]

> > +static void ppc_compat_prop_set(Object *obj, Visitor *v, const char *name,
> > +                                void *opaque, Error **errp)
> > +{
> > +    Error *error = NULL;
> 
> s/error/local_error maybe ? For consistency with ppc_compat_add_property() below.

Yeah, makes sense.  local_err does seem to be by far the more common
name for this idiom.


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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 941 bytes --]

On Sat, Jun 10, 2017 at 05:42:45PM +0200, Andrea Bolognani wrote:
> On Fri, 2017-06-02 at 13:15 +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.
> 
> I've tested this the same way I had tested one of the
> previous respins, eg. for basic usage and general sanity
> of the interface. All the issues I pointed out last time
> seems to have been addressed.
> 
> 
> Tested-by: Andrea Bolognani <abologna@redhat.com>

Ok, I think have enough acks of various sorts now.  I've merged this
into ppc-for-2.10.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Andrea Bolognani]

On Fri, 2017-06-02 at 13:15 +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.

As discussed yesterday on libvir-list, the current
implementation will abort() when the user attempts to use
a compat mode that's not valid for the host CPU, eg. on
a POWER8 host:

  $ qemu-system-ppc64 \
    -nodefaults \
    -M pseries,accel=kvm \
    -cpu host,compat=power9
  Unexpected error in ppc_set_compat() at target/ppc/compat.c:135:
  qemu-system-ppc64: Compatibility PVR 0x0f000005 not valid for CPU
  Aborted

We should probably report the error to the user in a slightly
less destructive fashion :)


[1] https://www.redhat.com/archives/libvir-list/2017-June/msg00476.html
-- 
Andrea Bolognani / Red Hat / Virtualization

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

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1289 bytes --]

On Tue, Jun 13, 2017 at 03:01:40PM +0800, Andrea Bolognani wrote:
> On Fri, 2017-06-02 at 13:15 +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.
> 
> As discussed yesterday on libvir-list, the current
> implementation will abort() when the user attempts to use
> a compat mode that's not valid for the host CPU, eg. on
> a POWER8 host:
> 
>   $ qemu-system-ppc64 \
>     -nodefaults \
>     -M pseries,accel=kvm \
>     -cpu host,compat=power9
>   Unexpected error in ppc_set_compat() at target/ppc/compat.c:135:
>   qemu-system-ppc64: Compatibility PVR 0x0f000005 not valid for CPU
>   Aborted
> 
> We should probably report the error to the user in a slightly
> less destructive fashion :)

Good point.  As I suspected, this was just an &error_abort somewhere I
should have used an &error_fatal.  Fixed in the ppc-for-2.10 I just
pushed.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCHv5 4/4] ppc: Rework CPU compatibility testing across migration

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 5734 bytes --]

On Fri,  2 Jun 2017 13:15:07 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> Migrating between different CPU versions is a bit complicated for ppc.
> A long time ago, we ensured identical CPU versions at either end by
> checking the PVR had the same value.  However, this breaks under KVM
> HV, because we always have to use the host's PVR - it's not
> virtualized.  That would mean we couldn't migrate between hosts with
> different PVRs, even if the CPUs are close enough to compatible in
> practice (sometimes identical cores with different surrounding logic
> have different PVRs, so this happens in practice quite often).
> 
> So, we removed the PVR check, but instead checked that several flags
> indicating supported instructions matched.  This turns out to be a bad
> idea, because those instruction masks are not architected information, but
> essentially a TCG implementation detail.  So changes to qemu internal CPU
> modelling can break migration - this happened between qemu-2.6 and
> qemu-2.7.  That was addressed by 146c11f1 "target-ppc: Allow eventual
> removal of old migration mistakes".
> 
> Now, verification of CPU compatibility across a migration basically doesn't
> happen.  We simply ignore the PVR of the incoming migration, and hope the
> cpu on the destination is close enough to work.
> 
> Now that we've cleaned up handling of processor compatibility modes for
> pseries machine type, we can do better.  We allow migration if:
> 
>     * The source and destination PVRs are for the same type of CPU, as
>       determined by CPU class's pvr_match function
> OR  * When the source was in a compatibility mode, and the destination CPU
>       supports the same compatibility mode
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> ---
>  target/ppc/machine.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 66 insertions(+), 3 deletions(-)
> 
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 6cb3a48..a29aabe 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -8,6 +8,7 @@
>  #include "helper_regs.h"
>  #include "mmu-hash64.h"
>  #include "migration/cpu.h"
> +#include "qapi/error.h"
>  
>  static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
>  {
> @@ -195,6 +196,27 @@ static void cpu_pre_save(void *opaque)
>      }
>  }
>  
> +/*
> + * Determine if a given PVR is a "close enough" match to the CPU
> + * object.  For TCG and KVM PR it would probably be sufficient to
> + * require an exact PVR match.  However for KVM HV the user is
> + * restricted to a PVR exactly matching the host CPU.  The correct way
> + * to handle this is to put the guest into an architected
> + * compatibility mode.  However, to allow a more forgiving transition
> + * and migration from before this was widely done, we allow migration
> + * between sufficiently similar PVRs, as determined by the CPU class's
> + * pvr_match() hook.
> + */
> +static bool pvr_match(PowerPCCPU *cpu, uint32_t pvr)
> +{
> +    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> +
> +    if (pvr == pcc->pvr) {
> +        return true;
> +    }
> +    return pcc->pvr_match(pcc, pvr);
> +}
> +
>  static int cpu_post_load(void *opaque, int version_id)
>  {
>      PowerPCCPU *cpu = opaque;
> @@ -203,10 +225,31 @@ static int cpu_post_load(void *opaque, int version_id)
>      target_ulong msr;
>  
>      /*
> -     * We always ignore the source PVR. The user or management
> -     * software has to take care of running QEMU in a compatible mode.
> +     * If we're operating in compat mode, we should be ok as long as
> +     * the destination supports the same compatiblity mode.
> +     *
> +     * Otherwise, however, we require that the destination has exactly
> +     * the same CPU model as the source.
>       */
> -    env->spr[SPR_PVR] = env->spr_cb[SPR_PVR].default_value;
> +
> +#if defined(TARGET_PPC64)
> +    if (cpu->compat_pvr) {
> +        Error *local_err = NULL;
> +
> +        ppc_set_compat(cpu, cpu->compat_pvr, &local_err);
> +        if (local_err) {
> +            error_report_err(local_err);
> +            error_free(local_err);
> +            return -1;
> +        }
> +    } else
> +#endif
> +    {
> +        if (!pvr_match(cpu, env->spr[SPR_PVR])) {
> +            return -1;
> +        }
> +    }
> +
>      env->lr = env->spr[SPR_LR];
>      env->ctr = env->spr[SPR_CTR];
>      cpu_write_xer(env, env->spr[SPR_XER]);
> @@ -560,6 +603,25 @@ static const VMStateDescription vmstate_tlbmas = {
>      }
>  };
>  
> +static bool compat_needed(void *opaque)
> +{
> +    PowerPCCPU *cpu = opaque;
> +
> +    assert(!(cpu->compat_pvr && !cpu->vhyp));
> +    return (cpu->compat_pvr != 0);

Since the CAS logic rework in 2.9, compat_pvr is likely to be != 0 and
the subsection is sent even for older machine types. This breaks backward
migration of such machines...

> +}
> +
> +static const VMStateDescription vmstate_compat = {
> +    .name = "cpu/compat",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = compat_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(compat_pvr, PowerPCCPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  const VMStateDescription vmstate_ppc_cpu = {
>      .name = "cpu",
>      .version_id = 5,
> @@ -613,6 +675,7 @@ const VMStateDescription vmstate_ppc_cpu = {
>          &vmstate_tlb6xx,
>          &vmstate_tlbemb,
>          &vmstate_tlbmas,
> +        &vmstate_compat,
>          NULL
>      }
>  };


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]