[Qemu-devel] [PATCH 0/9] spapr: Enable ibm, client-architecture-support

[Qemu-devel] [PATCH 0/9] spapr: Enable ibm, client-architecture-support

[Alexey Kardashevskiy]

This enables a ibm,client-architecture-support RTAS call.

This allows older distros (such as SLES11 or RHEL6) to work on modern
POWERPC hardware (such as POWER8) in "architected" mode.

The previous try was "RFC", so this is "v1".

The very first patch here is for the reference, it is already  on its way to
upstream.

Please comment. Thanks!


Alexey Kardashevskiy (9):
  kvm: add set_one_reg/get_one_reg helpers
  target-ppc: Add "compat" CPU option
  spapr: Move server# property out of skeleton fdt
  target-ppc: Implement "compat" CPU option
  target-ppc: Define Processor Compatibility Masks
  spapr: Add ibm,client-architecture-support call
  spapr: Limit threads per core according to current compatibility mode
  spapr: Implement processor compatibility in
    ibm,client-architecture-support
  KVM: PPC: Enable compatibility mode

 hw/ppc/spapr.c              | 149 +++++++++++++++++++++++++++++++++++++++-----
 hw/ppc/spapr_hcall.c        | 108 ++++++++++++++++++++++++++++++++
 include/hw/ppc/spapr.h      |   9 ++-
 include/sysemu/kvm.h        |  21 +++++++
 kvm-all.c                   |  18 ++++++
 target-ppc/cpu-models.h     |  17 +++++
 target-ppc/cpu-qom.h        |   5 ++
 target-ppc/cpu.h            |   3 +
 target-ppc/kvm.c            |   5 ++
 target-ppc/kvm_ppc.h        |   6 ++
 target-ppc/translate_init.c | 113 +++++++++++++++++++++++++++++++++
 trace-events                |   9 +++
 12 files changed, 446 insertions(+), 17 deletions(-)

-- 
1.9.rc0

[Qemu-devel] [PATCH 1/9] kvm: add set_one_reg/get_one_reg helpers

[Alexey Kardashevskiy]

This adds QEMU wrappers for KVM_SET_ONE_REG/KVM_GET_ONE_REG ioctls.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* changed comments

v2:
* added Doc Comments
* removed error_print
---
 include/sysemu/kvm.h | 21 +++++++++++++++++++++
 kvm-all.c            | 18 ++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 63e241a..b1566a4 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -383,4 +383,25 @@ void kvm_init_irq_routing(KVMState *s);
  *          > 0: irq chip was created
  */
 int kvm_arch_irqchip_create(KVMState *s);
+
+/**
+ * kvm_set_one_reg - set a register value in KVM via KVM_SET_ONE_REG ioctl
+ * @id: The register ID
+ * @addr: The pointer to a value must point to a variable of the correct
+ * type/size for the register being accessed.
+ *
+ * Returns: 0 on success, or a negative errno on failure.
+ */
+int kvm_set_one_reg(CPUState *cs, uint64_t id, void *addr);
+
+/**
+ * kvm_get_one_reg - get a register value from KVM via KVM_GET_ONE_REG ioctl
+ * @id: The register ID
+ * @addr: The pointer to a value must point to a variable of the correct
+ * type/size for the register being accessed.
+ *
+ * Returns: 0 on success, or a negative errno on failure.
+ */
+int kvm_get_one_reg(CPUState *cs, uint64_t id, void *addr);
+
 #endif
diff --git a/kvm-all.c b/kvm-all.c
index 5cb7f26..ebb1afa 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -2114,3 +2114,21 @@ int kvm_create_device(KVMState *s, uint64_t type, bool test)
 
     return test ? 0 : create_dev.fd;
 }
+
+int kvm_set_one_reg(CPUState *cs, uint64_t id, void *addr)
+{
+    struct kvm_one_reg reg = {
+        .id = id,
+        .addr = (uintptr_t)addr,
+    };
+    return kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, &reg);
+}
+
+int kvm_get_one_reg(CPUState *cs, uint64_t id, void *addr)
+{
+    struct kvm_one_reg reg = {
+        .id = id,
+        .addr = (uintptr_t)addr,
+    };
+    return kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, &reg);
+}
-- 
1.9.rc0

[Qemu-devel] [PATCH 2/9] target-ppc: Add "compat" CPU option

[Alexey Kardashevskiy]

PowerISA defines a compatibility mode for server POWERPC CPUs which
is supported by the PCR special register. To support this feature,
SPAPR defines a set of virtual PVRs, once per PowerISA spec version.

This introduces a "compat" CPU option which defines maximal compatibility
mode enabled. The supported modes are power6/power7/power8.

This does not change the existing behaviour, new property will be used
by next patches.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 target-ppc/cpu-models.h     | 11 +++++++
 target-ppc/cpu-qom.h        |  2 ++
 target-ppc/translate_init.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 88 insertions(+)

diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
index 9a003b4..33d2c51 100644
--- a/target-ppc/cpu-models.h
+++ b/target-ppc/cpu-models.h
@@ -595,6 +595,17 @@ enum {
     CPU_POWERPC_PA6T               = 0x00900000,
 };
 
+/* Logical PVR definitions for sPAPR */
+enum {
+    /* Logical CPUs */
+    CPU_POWERPC_LOGICAL_2_04       = 0x0F000001,
+    CPU_POWERPC_LOGICAL_2_05       = 0x0F000002,
+    CPU_POWERPC_LOGICAL_2_06       = 0x0F000003,
+    CPU_POWERPC_LOGICAL_2_06_PLUS  = 0x0F100003,
+    CPU_POWERPC_LOGICAL_2_07       = 0x0F000004,
+    CPU_POWERPC_LOGICAL_2_08       = 0x0F000005,
+};
+
 /* System version register (used on MPC 8xxx)                                */
 enum {
     POWERPC_SVR_NONE               = 0x00000000,
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index 47dc8e6..2a8515b 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -82,6 +82,7 @@ typedef struct PowerPCCPUClass {
  * 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
  *
  * A PowerPC CPU.
  */
@@ -92,6 +93,7 @@ struct PowerPCCPU {
 
     CPUPPCState env;
     int cpu_dt_id;
+    uint32_t max_compat;
 };
 
 static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 1d64ec9..6f61b34 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -28,6 +28,8 @@
 #include "mmu-hash32.h"
 #include "mmu-hash64.h"
 #include "qemu/error-report.h"
+#include "qapi/visitor.h"
+#include "hw/qdev-properties.h"
 
 //#define PPC_DUMP_CPU
 //#define PPC_DEBUG_SPR
@@ -7654,6 +7656,76 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
     pcc->l1_icache_size = 0x10000;
 }
 
+static void powerpc_get_compat(Object *obj, Visitor *v,
+                               void *opaque, const char *name, 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_setg(errp, "Internal error: compat is set to %x",
+                   max_compat ? *max_compat : -1);
+        break;
+    }
+
+    visit_type_str(v, &value, name, errp);
+}
+
+static void powerpc_set_compat(Object *obj, Visitor *v,
+                               void *opaque, const char *name, 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, &value, name, &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);
+    }
+
+    g_free(value);
+}
+
+static PropertyInfo powerpc_compat_propinfo = {
+    .name = "str",
+    .legacy_name = "powerpc-server-compat",
+    .get = powerpc_get_compat,
+    .set = powerpc_set_compat,
+};
+
+#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),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void init_proc_POWER7 (CPUPPCState *env)
 {
     gen_spr_ne_601(env);
@@ -7740,6 +7812,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
 
     dc->fw_name = "PowerPC,POWER7";
     dc->desc = "POWER7";
+    dc->props = powerpc_servercpu_properties;
     pcc->pvr = CPU_POWERPC_POWER7_BASE;
     pcc->pvr_mask = CPU_POWERPC_POWER7_MASK;
     pcc->init_proc = init_proc_POWER7;
@@ -7798,6 +7871,7 @@ POWERPC_FAMILY(POWER7P)(ObjectClass *oc, void *data)
 
     dc->fw_name = "PowerPC,POWER7+";
     dc->desc = "POWER7+";
+    dc->props = powerpc_servercpu_properties;
     pcc->pvr = CPU_POWERPC_POWER7P_BASE;
     pcc->pvr_mask = CPU_POWERPC_POWER7P_MASK;
     pcc->init_proc = init_proc_POWER7;
@@ -7868,6 +7942,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
 
     dc->fw_name = "PowerPC,POWER8";
     dc->desc = "POWER8";
+    dc->props = powerpc_servercpu_properties;
     pcc->pvr = CPU_POWERPC_POWER8_BASE;
     pcc->pvr_mask = CPU_POWERPC_POWER8_MASK;
     pcc->init_proc = init_proc_POWER8;
-- 
1.9.rc0

[Qemu-devel] [PATCH 3/9] spapr: Move server# property out of skeleton fdt

[Alexey Kardashevskiy]

The upcoming support of the "ibm,client-architecture-support"
reconfiguration method will be able to reduce the number of threads per
core so the server# and gserver# device tree properties are not parts
of the FDT skeleton anymore.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 166c1c6..0f8bd95 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -219,6 +219,9 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
                                     cpu_to_be32(0x0),
                                     cpu_to_be32(cpu->numa_node),
                                     cpu_to_be32(index)};
+        uint32_t servers_prop[smp_threads];
+        uint32_t gservers_prop[smp_threads * 2];
+        int i;
 
         if ((index % smt) != 0) {
             continue;
@@ -245,6 +248,24 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
         if (ret < 0) {
             return ret;
         }
+
+        /* Build interrupt servers and gservers properties */
+        for (i = 0; i < smp_threads; i++) {
+            servers_prop[i] = cpu_to_be32(index + i);
+            /* Hack, direct the group queues back to cpu 0 */
+            gservers_prop[i*2] = cpu_to_be32(index + i);
+            gservers_prop[i*2 + 1] = 0;
+        }
+        ret = fdt_setprop(fdt, offset, "ibm,ppc-interrupt-server#s",
+                           servers_prop, sizeof(servers_prop));
+        if (ret < 0) {
+            return ret;
+        }
+        ret = fdt_setprop(fdt, offset, "ibm,ppc-interrupt-gserver#s",
+                          gservers_prop, sizeof(gservers_prop));
+        if (ret < 0) {
+            return ret;
+        }
     }
     return ret;
 }
@@ -311,7 +332,7 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
     char qemu_hypertas_prop[] = "hcall-memop1";
     uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)};
     uint32_t interrupt_server_ranges_prop[] = {0, cpu_to_be32(smp_cpus)};
-    int i, smt = kvmppc_smt_threads();
+    int smt = kvmppc_smt_threads();
     unsigned char vec5[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x80};
     QemuOpts *opts = qemu_opts_find(qemu_find_opts("smp-opts"), NULL);
     unsigned sockets = opts ? qemu_opt_get_number(opts, "sockets", 0) : 0;
@@ -378,8 +399,6 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
         DeviceClass *dc = DEVICE_GET_CLASS(cs);
         PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
         int index = ppc_get_vcpu_dt_id(cpu);
-        uint32_t servers_prop[smp_threads];
-        uint32_t gservers_prop[smp_threads * 2];
         char *nodename;
         uint32_t segs[] = {cpu_to_be32(28), cpu_to_be32(40),
                            0xffffffff, 0xffffffff};
@@ -428,18 +447,6 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
         _FDT((fdt_property_string(fdt, "status", "okay")));
         _FDT((fdt_property(fdt, "64-bit", NULL, 0)));
 
-        /* Build interrupt servers and gservers properties */
-        for (i = 0; i < smp_threads; i++) {
-            servers_prop[i] = cpu_to_be32(index + i);
-            /* Hack, direct the group queues back to cpu 0 */
-            gservers_prop[i*2] = cpu_to_be32(index + i);
-            gservers_prop[i*2 + 1] = 0;
-        }
-        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
-                           servers_prop, sizeof(servers_prop))));
-        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
-                           gservers_prop, sizeof(gservers_prop))));
-
         if (env->spr_cb[SPR_PURR].oea_read) {
             _FDT((fdt_property(fdt, "ibm,purr", NULL, 0)));
         }
-- 
1.9.rc0

[Qemu-devel] [PATCH 4/9] target-ppc: Implement "compat" CPU option

[Alexey Kardashevskiy]

This adds basic support for the "compat" CPU option. By specifying
the compat property, the user can manually switch guest CPU mode from
"raw" to "architected".

Since the actual compatibility mode is not implemented yet, this does
not change the existing behavior.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c              | 15 ++++++++++++++-
 target-ppc/cpu-models.h     |  6 ++++++
 target-ppc/cpu-qom.h        |  2 ++
 target-ppc/cpu.h            |  3 +++
 target-ppc/translate_init.c | 35 +++++++++++++++++++++++++++++++++++
 5 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0f8bd95..61d0675 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -212,7 +212,8 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
 
     CPU_FOREACH(cpu) {
         DeviceClass *dc = DEVICE_GET_CLASS(cpu);
-        int index = ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
+        PowerPCCPU *pcpu = POWERPC_CPU(cpu);
+        int index = ppc_get_vcpu_dt_id(pcpu);
         uint32_t associativity[] = {cpu_to_be32(0x5),
                                     cpu_to_be32(0x0),
                                     cpu_to_be32(0x0),
@@ -249,6 +250,14 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
             return ret;
         }
 
+        if (pcpu->cpu_version) {
+            ret = fdt_setprop(fdt, offset, "cpu-version",
+                              &pcpu->cpu_version, sizeof(pcpu->cpu_version));
+            if (ret < 0) {
+                return ret;
+            }
+        }
+
         /* Build interrupt servers and gservers properties */
         for (i = 0; i < smp_threads; i++) {
             servers_prop[i] = cpu_to_be32(index + i);
@@ -1278,6 +1287,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
             kvmppc_set_papr(cpu);
         }
 
+        if (cpu->max_compat) {
+            ppc_set_compat(cpu, cpu->max_compat);
+        }
+
         xics_cpu_setup(spapr->icp, cpu);
 
         qemu_register_reset(spapr_cpu_reset, cpu);
diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
index 33d2c51..45c0028 100644
--- a/target-ppc/cpu-models.h
+++ b/target-ppc/cpu-models.h
@@ -753,4 +753,10 @@ enum {
     POWERPC_SVR_8641D              = 0x80900121,
 };
 
+/* Processor Compatibility mask (PCR) */
+enum {
+    POWERPC_ISA_COMPAT_2_05 = 0x02,
+    POWERPC_ISA_COMPAT_2_06 = 0x04,
+};
+
 #endif
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index 2a8515b..dfd1419 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -83,6 +83,7 @@ typedef struct PowerPCCPUClass {
  * @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
+ * @cpu_version: Current logical PVR, zero if in "raw" mode
  *
  * A PowerPC CPU.
  */
@@ -94,6 +95,7 @@ struct PowerPCCPU {
     CPUPPCState env;
     int cpu_dt_id;
     uint32_t max_compat;
+    uint32_t cpu_version;
 };
 
 static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index d498340..f61675a 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1123,6 +1123,8 @@ void ppc_store_msr (CPUPPCState *env, target_ulong value);
 
 void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
 
+void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
+
 /* Time-base and decrementer management */
 #ifndef NO_CPU_IO_DEFS
 uint64_t cpu_ppc_load_tbl (CPUPPCState *env);
@@ -1338,6 +1340,7 @@ static inline int cpu_mmu_index (CPUPPCState *env)
 #define SPR_LPCR              (0x13E)
 #define SPR_BOOKE_DVC2        (0x13F)
 #define SPR_BOOKE_TSR         (0x150)
+#define SPR_PCR               (0x152)
 #define SPR_BOOKE_TCR         (0x154)
 #define SPR_BOOKE_TLB0PS      (0x158)
 #define SPR_BOOKE_TLB1PS      (0x159)
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 6f61b34..c4bd5de 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7803,6 +7803,15 @@ static void init_proc_POWER7 (CPUPPCState *env)
     /* Can't find information on what this should be on reset.  This
      * value is the one used by 74xx processors. */
     vscr_init(env, 0x00010000);
+
+    /*
+     * Register PCR to report POWERPC_EXCP_PRIV_REG instead of
+     * POWERPC_EXCP_INVAL_SPR.
+     */
+    spr_register(env, SPR_PCR, "PCR",
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 SPR_NOACCESS, SPR_NOACCESS,
+                 0x00000000);
 }
 
 POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
@@ -8880,6 +8889,32 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
     }
 }
 
+void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
+{
+    CPUPPCState *env = &cpu->env;
+
+    cpu->cpu_version = cpu_version;
+
+    /*
+     * Calculate PCR value from virtual PVR.
+     * TODO: support actual compatibility in TCG.
+     */
+    switch (cpu_version) {
+    case CPU_POWERPC_LOGICAL_2_05:
+        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_05;
+        break;
+    case CPU_POWERPC_LOGICAL_2_06:
+        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
+        break;
+    case CPU_POWERPC_LOGICAL_2_06_PLUS:
+        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
+        break;
+    default:
+        env->spr[SPR_PCR] = 0;
+        break;
+    }
+}
+
 static gint ppc_cpu_compare_class_pvr(gconstpointer a, gconstpointer b)
 {
     ObjectClass *oc = (ObjectClass *)a;
-- 
1.9.rc0

[Qemu-devel] [PATCH 5/9] target-ppc: Define Processor Compatibility Masks

[Alexey Kardashevskiy]

This introduces PCR mask for supported compatibility modes.
This will be used later by the ibm,client-architecture-support call.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 target-ppc/cpu-qom.h        | 1 +
 target-ppc/translate_init.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index dfd1419..093f09a 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -57,6 +57,7 @@ typedef struct PowerPCCPUClass {
 
     uint32_t pvr;
     uint32_t pvr_mask;
+    uint64_t pcr_mask;
     uint32_t svr;
     uint64_t insns_flags;
     uint64_t insns_flags2;
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index c4bd5de..1b72485 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7824,6 +7824,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
     dc->props = powerpc_servercpu_properties;
     pcc->pvr = CPU_POWERPC_POWER7_BASE;
     pcc->pvr_mask = CPU_POWERPC_POWER7_MASK;
+    pcc->pcr_mask = POWERPC_ISA_COMPAT_2_05 | POWERPC_ISA_COMPAT_2_06;
     pcc->init_proc = init_proc_POWER7;
     pcc->check_pow = check_pow_nocheck;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
@@ -7883,6 +7884,7 @@ POWERPC_FAMILY(POWER7P)(ObjectClass *oc, void *data)
     dc->props = powerpc_servercpu_properties;
     pcc->pvr = CPU_POWERPC_POWER7P_BASE;
     pcc->pvr_mask = CPU_POWERPC_POWER7P_MASK;
+    pcc->pcr_mask = POWERPC_ISA_COMPAT_2_05 | POWERPC_ISA_COMPAT_2_06;
     pcc->init_proc = init_proc_POWER7;
     pcc->check_pow = check_pow_nocheck;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
@@ -7954,6 +7956,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
     dc->props = powerpc_servercpu_properties;
     pcc->pvr = CPU_POWERPC_POWER8_BASE;
     pcc->pvr_mask = CPU_POWERPC_POWER8_MASK;
+    pcc->pcr_mask = POWERPC_ISA_COMPAT_2_05 | POWERPC_ISA_COMPAT_2_06;
     pcc->init_proc = init_proc_POWER8;
     pcc->check_pow = check_pow_nocheck;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
-- 
1.9.rc0

[Qemu-devel] [PATCH 6/9] spapr: Add ibm, client-architecture-support call

[Alexey Kardashevskiy]

The PAPR+ specification defines a ibm,client-architecture-support (CAS)
RTAS call which purpose is to provide a negotiation mechanism for
the guest and the hypervisor to work out the best compatibility parameters.
During the negotiation process, the guest provides an array of various
options and capabilities which it supports, the hypervisor adjusts
the device tree and (optionally) reboots the guest.

At the moment the Linux guest calls CAS method at early boot so SLOF
gets called. SLOF allocates a memory buffer for the device tree changes
and calls a custom KVMPPC_H_CAS hypercall. QEMU parses the options,
composes a diff for the device tree, copies it to the buffer provided
by SLOF and returns to SLOF. SLOF updates the device tree and returns
control to the guest kernel. Only then the Linux guest parses the device
tree so it is possible to avoid unnecessary reboot in most cases.

The device tree diff is a header with the update format version
(defined as 0 in this patch) followed by a device tree with the properties
which require update.

If QEMU detects during the option list parsing that it has to reboot
the guest, it silently does so as the guest expects reboot to happen as
pHyp firmware does it almost all the time.

This defines custom KVMPPC_H_CAS hypercall. The current SLOF already
has support for it.

This implements stub which returns empty tree to the guest.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c         | 26 ++++++++++++++++++++++++++
 hw/ppc/spapr_hcall.c   | 21 +++++++++++++++++++++
 include/hw/ppc/spapr.h |  9 ++++++++-
 trace-events           |  4 ++++
 4 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 61d0675..cf53a7a 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -53,6 +53,7 @@
 #include "hw/usb.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
+#include "trace.h"
 
 #include <libfdt.h>
 
@@ -562,6 +563,31 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
     return fdt;
 }
 
+int spapr_h_cas_compose_response(target_ulong addr, target_ulong size)
+{
+    void *fdt;
+    sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
+
+    size -= sizeof(hdr);
+
+    fdt = g_malloc0(size);
+    _FDT((fdt_create(fdt, size)));
+
+    _FDT((fdt_finish(fdt)));
+
+    if (fdt_totalsize(fdt) + sizeof(hdr) > size) {
+        trace_spapr_cas_failed(size);
+        return -1;
+    }
+
+    cpu_physical_memory_write(addr, &hdr, sizeof(hdr));
+    cpu_physical_memory_write(addr + sizeof(hdr), fdt, fdt_totalsize(fdt));
+    trace_spapr_cas_continue(fdt_totalsize(fdt) + sizeof(hdr));
+    g_free(fdt);
+
+    return 0;
+}
+
 static int spapr_populate_memory(sPAPREnvironment *spapr, void *fdt)
 {
     uint32_t associativity[] = {cpu_to_be32(0x4), cpu_to_be32(0x0),
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 0bae053..2f6aa5c 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -752,6 +752,24 @@ out:
     return ret;
 }
 
+static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
+                                                  sPAPREnvironment *spapr,
+                                                  target_ulong opcode,
+                                                  target_ulong *args)
+{
+    target_ulong list = args[0];
+
+    if (!list) {
+        return H_SUCCESS;
+    }
+
+    if (spapr_h_cas_compose_response(args[1], args[2])) {
+        qemu_system_reset_request();
+    }
+
+    return H_SUCCESS;
+}
+
 static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4) + 1];
 static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX - KVMPPC_HCALL_BASE + 1];
 
@@ -831,6 +849,9 @@ static void hypercall_register_types(void)
     spapr_register_hypercall(KVMPPC_H_RTAS, h_rtas);
 
     spapr_register_hypercall(H_SET_MODE, h_set_mode);
+
+    /* ibm,client-architecture-support support */
+    spapr_register_hypercall(KVMPPC_H_CAS, h_client_architecture_support);
 }
 
 type_init(hypercall_register_types)
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 5fdac1e..dd6d97a 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -302,10 +302,16 @@ typedef struct sPAPREnvironment {
 #define KVMPPC_HCALL_BASE       0xf000
 #define KVMPPC_H_RTAS           (KVMPPC_HCALL_BASE + 0x0)
 #define KVMPPC_H_LOGICAL_MEMOP  (KVMPPC_HCALL_BASE + 0x1)
-#define KVMPPC_HCALL_MAX        KVMPPC_H_LOGICAL_MEMOP
+/* Client Architecture support */
+#define KVMPPC_H_CAS            (KVMPPC_HCALL_BASE + 0x2)
+#define KVMPPC_HCALL_MAX        KVMPPC_H_CAS
 
 extern sPAPREnvironment *spapr;
 
+typedef struct sPAPRDeviceTreeUpdateHeader {
+    uint32_t version_id;
+} sPAPRDeviceTreeUpdateHeader;
+
 /*#define DEBUG_SPAPR_HCALLS*/
 
 #ifdef DEBUG_SPAPR_HCALLS
@@ -401,6 +407,7 @@ struct sPAPRTCETable {
 
 void spapr_events_init(sPAPREnvironment *spapr);
 void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq);
+int spapr_h_cas_compose_response(target_ulong addr, target_ulong size);
 sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn,
                                    size_t window_size);
 MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet);
diff --git a/trace-events b/trace-events
index af4449d..03565dc 100644
--- a/trace-events
+++ b/trace-events
@@ -1179,6 +1179,10 @@ xics_ics_write_xive(int nr, int srcno, int server, uint8_t priority) "ics_write_
 xics_ics_reject(int nr, int srcno) "reject irq %#x [src %d]"
 xics_ics_eoi(int nr) "ics_eoi: irq %#x"
 
+# hw/ppc/spapr.c
+spapr_cas_failed(unsigned long n) "DT diff buffer is too small: %ld bytes"
+spapr_cas_continue(unsigned long n) "Copy changes to the guest: %ld bytes"
+
 # hw/ppc/spapr_iommu.c
 spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64
 spapr_iommu_get(uint64_t liobn, uint64_t ioba, uint64_t ret, uint64_t tce) "liobn=%"PRIx64" ioba=0x%"PRIx64" ret=%"PRId64" tce=0x%"PRIx64
-- 
1.9.rc0

[Qemu-devel] [PATCH 7/9] spapr: Limit threads per core according to current compatibility mode

[Alexey Kardashevskiy]

This puts a limit to the number of threads per core based on the current
compatibility mode. Although PowerISA specs do not specify the maximum
threads per core number, the linux guest still expects that
PowerISA2.05-compatible CPU supports only 2 threads per core as this
is what POWER6 (2.05 compliant CPU) implements, same is true for
POWER7 (2.06, 4 threads) and POWER8 (2.07, 8 threads).

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index cf53a7a..a2c9106 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -34,6 +34,7 @@
 #include "sysemu/kvm.h"
 #include "kvm_ppc.h"
 #include "mmu-hash64.h"
+#include "cpu-models.h"
 
 #include "hw/boards.h"
 #include "hw/ppc/ppc.h"
@@ -203,6 +204,29 @@ static XICSState *xics_system_init(int nr_servers, int nr_irqs)
     return icp;
 }
 
+static int spapr_get_compat_smp_threads(PowerPCCPU *cpu)
+{
+    int ret = -1;
+
+    switch (cpu->cpu_version) {
+    case CPU_POWERPC_LOGICAL_2_05:
+        ret = 2;
+        break;
+    case CPU_POWERPC_LOGICAL_2_06:
+        ret = 4;
+        break;
+    case CPU_POWERPC_LOGICAL_2_07:
+        ret = 8;
+        break;
+    default:
+        ret = smp_threads;
+        break;
+    }
+    ret = MIN(ret, smp_threads);
+
+    return ret;
+}
+
 static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
 {
     int ret = 0, offset;
@@ -221,8 +245,9 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
                                     cpu_to_be32(0x0),
                                     cpu_to_be32(cpu->numa_node),
                                     cpu_to_be32(index)};
-        uint32_t servers_prop[smp_threads];
-        uint32_t gservers_prop[smp_threads * 2];
+        int smpt = spapr_get_compat_smp_threads(pcpu);
+        uint32_t servers_prop[smpt];
+        uint32_t gservers_prop[smpt * 2];
         int i;
 
         if ((index % smt) != 0) {
@@ -260,7 +285,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
         }
 
         /* Build interrupt servers and gservers properties */
-        for (i = 0; i < smp_threads; i++) {
+        for (i = 0; i < smpt; i++) {
             servers_prop[i] = cpu_to_be32(index + i);
             /* Hack, direct the group queues back to cpu 0 */
             gservers_prop[i*2] = cpu_to_be32(index + i);
-- 
1.9.rc0

[Qemu-devel] [PATCH 8/9] spapr: Implement processor compatibility in ibm, client-architecture-support

[Alexey Kardashevskiy]

Modern Linux kernels support last POWERPC CPUs so when a kernel boots,
in most cases it can find a matching cpu_spec in the kernel's cpu_specs
list. However if the kernel is quite old, it may be missing a definition
of the actual CPU. To provide an ability for old kernels to work on modern
hardware, a Processor Compatibility Mode has been introduced
by the PowerISA specification.

>From the hardware prospective, it is supported by the Processor
Compatibility Register (PCR) which is defined in PowerISA. The register
enables one of the compatibility modes (2.05/2.06/2.07).
Since PCR is a hypervisor privileged register and cannot be
accessed from the guest, the mode selection is done via
ibm,client-architecture-support (CAS) RTAS call using which the guest
specifies what "raw" and "architected" CPU versions it supports.
QEMU works out the best match, changes a "cpu-version" property of
every CPU and notifies the guest about the change by setting these
properties in the buffer passed as a response on a custom H_CAS hypercall.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c       | 40 +++++++++++++++++++++++++
 hw/ppc/spapr_hcall.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 trace-events         |  5 ++++
 3 files changed, 128 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a2c9106..a0882a1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -35,6 +35,7 @@
 #include "kvm_ppc.h"
 #include "mmu-hash64.h"
 #include "cpu-models.h"
+#include "qom/cpu.h"
 
 #include "hw/boards.h"
 #include "hw/ppc/ppc.h"
@@ -592,11 +593,50 @@ int spapr_h_cas_compose_response(target_ulong addr, target_ulong size)
 {
     void *fdt;
     sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
+    CPUState *cs;
+    int smt = kvmppc_smt_threads();
 
     size -= sizeof(hdr);
 
     fdt = g_malloc0(size);
     _FDT((fdt_create(fdt, size)));
+    _FDT((fdt_begin_node(fdt, "cpus")));
+
+    CPU_FOREACH(cs) {
+        PowerPCCPU *cpu = POWERPC_CPU(cs);
+        DeviceClass *dc = DEVICE_GET_CLASS(cpu);
+        int smpt = spapr_get_compat_smp_threads(cpu);
+        int i, index = ppc_get_vcpu_dt_id(cpu);
+        uint32_t servers_prop[smpt];
+        uint32_t gservers_prop[smpt * 2];
+        char tmp[32];
+
+        if ((index % smt) != 0) {
+            continue;
+        }
+
+        snprintf(tmp, 32, "%s@%x", dc->fw_name, index);
+        trace_spapr_cas_add_subnode(tmp);
+
+        _FDT((fdt_begin_node(fdt, tmp)));
+        _FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
+
+        /* Build interrupt servers and gservers properties */
+        for (i = 0; i < smpt; i++) {
+            servers_prop[i] = cpu_to_be32(index + i);
+            /* Hack, direct the group queues back to cpu 0 */
+            gservers_prop[i*2] = cpu_to_be32(index + i);
+            gservers_prop[i*2 + 1] = 0;
+        }
+        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
+                           servers_prop, sizeof(servers_prop))));
+        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
+                           gservers_prop, sizeof(gservers_prop))));
+
+        _FDT((fdt_end_node(fdt)));
+    }
+
+    _FDT((fdt_end_node(fdt)));
 
     _FDT((fdt_finish(fdt)));
 
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 2f6aa5c..cb815c3 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -3,6 +3,9 @@
 #include "helper_regs.h"
 #include "hw/ppc/spapr.h"
 #include "mmu-hash64.h"
+#include "cpu-models.h"
+#include "trace.h"
+#include "kvm_ppc.h"
 
 struct SPRSyncState {
     CPUState *cs;
@@ -752,12 +755,92 @@ out:
     return ret;
 }
 
+#define get_compat_level(cpuver) ( \
+    ((cpuver) == CPU_POWERPC_LOGICAL_2_05) ? 2050 : \
+    ((cpuver) == CPU_POWERPC_LOGICAL_2_06) ? 2060 : \
+    ((cpuver) == CPU_POWERPC_LOGICAL_2_06_PLUS) ? 2061 : \
+    ((cpuver) == CPU_POWERPC_LOGICAL_2_07) ? 2070 : 0)
+
 static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
                                                   sPAPREnvironment *spapr,
                                                   target_ulong opcode,
                                                   target_ulong *args)
 {
     target_ulong list = args[0];
+    PowerPCCPUClass *pcc_ = POWERPC_CPU_GET_CLASS(cpu_);
+    CPUState *cs;
+    CPUState *cs_ = CPU(cpu_);
+    bool cpu_match = false;
+    unsigned old_cpu_version = cpu_->cpu_version;
+    unsigned compat_lvl = 0, cpu_version = 0;
+    unsigned max_lvl = get_compat_level(cpu_->max_compat);
+
+    /* Parse PVR list */
+    for ( ; ; ) {
+        uint32_t pvr, pvr_mask;
+
+        pvr_mask = ldl_phys(cs_->as, list);
+        list += 4;
+        pvr = ldl_phys(cs_->as, list);
+        list += 4;
+
+        trace_spapr_cas_pvr_try(pvr);
+        if (!max_lvl &&
+            ((cpu_->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask))) {
+            cpu_match = true;
+            cpu_version = 0;
+        } else if (pvr == cpu_->cpu_version) {
+            cpu_match = true;
+            cpu_version = cpu_->cpu_version;
+        } else if (!cpu_match) {
+            /* If it is a logical PVR, try to determine the highest level */
+            unsigned lvl = get_compat_level(pvr);
+            if (lvl) {
+                bool is205 = (pcc_->pcr_mask & POWERPC_ISA_COMPAT_2_05) &&
+                     (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05));
+                bool is206 = (pcc_->pcr_mask & POWERPC_ISA_COMPAT_2_06) &&
+                    ((lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06)) ||
+                    (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06_PLUS)));
+
+                if (is205 || is206) {
+                    if (!max_lvl) {
+                        /* User did not set the level, choose the highest */
+                        if (compat_lvl <= lvl) {
+                            compat_lvl = lvl;
+                            cpu_version = pvr;
+                        }
+                    } else if (max_lvl >= lvl) {
+                        /* User chose the level, don't set higher than this */
+                        compat_lvl = lvl;
+                        cpu_version = pvr;
+                    }
+                }
+            }
+        }
+        /* Terminator record */
+        if (~pvr_mask & pvr) {
+            break;
+        }
+    }
+
+    /* For the future use: here @list points to the first capability */
+
+    /* Parsing finished */
+    trace_spapr_cas_pvr(cpu_->cpu_version, cpu_match,
+                        cpu_version, pcc_->pcr_mask);
+
+    /* Update CPUs */
+    if (old_cpu_version != cpu_version) {
+        CPU_FOREACH(cs) {
+            PowerPCCPU *cpu = POWERPC_CPU(cs);
+
+            ppc_set_compat(cpu, cpu_version);
+        }
+    }
+
+    if (!cpu_version) {
+        return H_SUCCESS;
+    }
 
     if (!list) {
         return H_SUCCESS;
diff --git a/trace-events b/trace-events
index 03565dc..c2db909 100644
--- a/trace-events
+++ b/trace-events
@@ -1182,6 +1182,11 @@ xics_ics_eoi(int nr) "ics_eoi: irq %#x"
 # hw/ppc/spapr.c
 spapr_cas_failed(unsigned long n) "DT diff buffer is too small: %ld bytes"
 spapr_cas_continue(unsigned long n) "Copy changes to the guest: %ld bytes"
+spapr_cas_add_subnode(const char *s) "%s"
+
+# hw/ppc/spapr_hcall.c
+spapr_cas_pvr_try(uint32_t pvr) "%x"
+spapr_cas_pvr(uint32_t cur_pvr, bool cpu_match, uint32_t new_pvr, uint64_t pcr) "current=%x, cpu_match=%u, new=%x, compat flags=%"PRIx64
 
 # hw/ppc/spapr_iommu.c
 spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64
-- 
1.9.rc0

[Qemu-devel] [PATCH 9/9] KVM: PPC: Enable compatibility mode

[Alexey Kardashevskiy]

The host kernel implements a KVM_REG_PPC_ARCH_COMPAT register which
this uses to enable a compatibility mode if any chosen.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr.c       | 6 ++++++
 hw/ppc/spapr_hcall.c | 4 ++++
 target-ppc/kvm.c     | 5 +++++
 target-ppc/kvm_ppc.h | 6 ++++++
 4 files changed, 21 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index a0882a1..f89be10 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1376,6 +1376,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
         /* Tell KVM that we're in PAPR mode */
         if (kvm_enabled()) {
             kvmppc_set_papr(cpu);
+
+            if (cpu->max_compat &&
+                kvmppc_set_compat(cpu, cpu->max_compat) < 0) {
+                fprintf(stderr, "Unable to set compatibility mode\n");
+                exit(1);
+            }
         }
 
         if (cpu->max_compat) {
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index cb815c3..2ab21d3 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -834,6 +834,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
         CPU_FOREACH(cs) {
             PowerPCCPU *cpu = POWERPC_CPU(cs);
 
+            if (kvmppc_set_compat(cpu, cpu_version) < 0) {
+                fprintf(stderr, "Unable to set compatibility mode\n");
+                return H_HARDWARE;
+            }
             ppc_set_compat(cpu, cpu_version);
         }
     }
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index ff319fc..f8e8453 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1513,6 +1513,11 @@ void kvmppc_set_papr(PowerPCCPU *cpu)
     cap_papr = 1;
 }
 
+int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
+{
+    return kvm_set_one_reg(CPU(cpu), KVM_REG_PPC_ARCH_COMPAT, &cpu_version);
+}
+
 void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
 {
     CPUState *cs = CPU(cpu);
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index ff077ec..716c33d 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -23,6 +23,7 @@ int kvmppc_get_hasidle(CPUPPCState *env);
 int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
 int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
 void kvmppc_set_papr(PowerPCCPU *cpu);
+int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
 void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
 int kvmppc_smt_threads(void);
 int kvmppc_clear_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits);
@@ -95,6 +96,11 @@ static inline void kvmppc_set_papr(PowerPCCPU *cpu)
 {
 }
 
+static inline int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
+{
+    return 0;
+}
+
 static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
 {
 }
-- 
1.9.rc0

Re: [Qemu-devel] [PATCH 4/9] target-ppc: Implement "compat" CPU option

[Alexander Graf]

On 15.05.14 13:28, Alexey Kardashevskiy wrote:
> This adds basic support for the "compat" CPU option. By specifying
> the compat property, the user can manually switch guest CPU mode from
> "raw" to "architected".
>
> Since the actual compatibility mode is not implemented yet, this does
> not change the existing behavior.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>   hw/ppc/spapr.c              | 15 ++++++++++++++-
>   target-ppc/cpu-models.h     |  6 ++++++
>   target-ppc/cpu-qom.h        |  2 ++
>   target-ppc/cpu.h            |  3 +++
>   target-ppc/translate_init.c | 35 +++++++++++++++++++++++++++++++++++
>   5 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 0f8bd95..61d0675 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -212,7 +212,8 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
>   
>       CPU_FOREACH(cpu) {
>           DeviceClass *dc = DEVICE_GET_CLASS(cpu);
> -        int index = ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
> +        PowerPCCPU *pcpu = POWERPC_CPU(cpu);
> +        int index = ppc_get_vcpu_dt_id(pcpu);
>           uint32_t associativity[] = {cpu_to_be32(0x5),
>                                       cpu_to_be32(0x0),
>                                       cpu_to_be32(0x0),
> @@ -249,6 +250,14 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
>               return ret;
>           }
>   
> +        if (pcpu->cpu_version) {
> +            ret = fdt_setprop(fdt, offset, "cpu-version",
> +                              &pcpu->cpu_version, sizeof(pcpu->cpu_version));
> +            if (ret < 0) {
> +                return ret;
> +            }
> +        }
> +
>           /* Build interrupt servers and gservers properties */
>           for (i = 0; i < smp_threads; i++) {
>               servers_prop[i] = cpu_to_be32(index + i);
> @@ -1278,6 +1287,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>               kvmppc_set_papr(cpu);
>           }
>   
> +        if (cpu->max_compat) {
> +            ppc_set_compat(cpu, cpu->max_compat);
> +        }
> +
>           xics_cpu_setup(spapr->icp, cpu);
>   
>           qemu_register_reset(spapr_cpu_reset, cpu);
> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
> index 33d2c51..45c0028 100644
> --- a/target-ppc/cpu-models.h
> +++ b/target-ppc/cpu-models.h
> @@ -753,4 +753,10 @@ enum {
>       POWERPC_SVR_8641D              = 0x80900121,
>   };
>   
> +/* Processor Compatibility mask (PCR) */
> +enum {
> +    POWERPC_ISA_COMPAT_2_05 = 0x02,
> +    POWERPC_ISA_COMPAT_2_06 = 0x04,
> +};
> +
>   #endif
> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> index 2a8515b..dfd1419 100644
> --- a/target-ppc/cpu-qom.h
> +++ b/target-ppc/cpu-qom.h
> @@ -83,6 +83,7 @@ typedef struct PowerPCCPUClass {
>    * @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
> + * @cpu_version: Current logical PVR, zero if in "raw" mode
>    *
>    * A PowerPC CPU.
>    */
> @@ -94,6 +95,7 @@ struct PowerPCCPU {
>       CPUPPCState env;
>       int cpu_dt_id;
>       uint32_t max_compat;
> +    uint32_t cpu_version;
>   };
>   
>   static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index d498340..f61675a 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -1123,6 +1123,8 @@ void ppc_store_msr (CPUPPCState *env, target_ulong value);
>   
>   void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
>   
> +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
> +
>   /* Time-base and decrementer management */
>   #ifndef NO_CPU_IO_DEFS
>   uint64_t cpu_ppc_load_tbl (CPUPPCState *env);
> @@ -1338,6 +1340,7 @@ static inline int cpu_mmu_index (CPUPPCState *env)
>   #define SPR_LPCR              (0x13E)
>   #define SPR_BOOKE_DVC2        (0x13F)
>   #define SPR_BOOKE_TSR         (0x150)
> +#define SPR_PCR               (0x152)
>   #define SPR_BOOKE_TCR         (0x154)
>   #define SPR_BOOKE_TLB0PS      (0x158)
>   #define SPR_BOOKE_TLB1PS      (0x159)
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 6f61b34..c4bd5de 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -7803,6 +7803,15 @@ static void init_proc_POWER7 (CPUPPCState *env)
>       /* Can't find information on what this should be on reset.  This
>        * value is the one used by 74xx processors. */
>       vscr_init(env, 0x00010000);
> +
> +    /*
> +     * Register PCR to report POWERPC_EXCP_PRIV_REG instead of
> +     * POWERPC_EXCP_INVAL_SPR.
> +     */
> +    spr_register(env, SPR_PCR, "PCR",
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 SPR_NOACCESS, SPR_NOACCESS,
> +                 0x00000000);
>   }
>   
>   POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
> @@ -8880,6 +8889,32 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp)
>       }
>   }
>   
> +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
> +{
> +    CPUPPCState *env = &cpu->env;
> +
> +    cpu->cpu_version = cpu_version;
> +
> +    /*
> +     * Calculate PCR value from virtual PVR.
> +     * TODO: support actual compatibility in TCG.
> +     */
> +    switch (cpu_version) {
> +    case CPU_POWERPC_LOGICAL_2_05:
> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_05;
> +        break;
> +    case CPU_POWERPC_LOGICAL_2_06:
> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
> +        break;
> +    case CPU_POWERPC_LOGICAL_2_06_PLUS:
> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
> +        break;

Don't we have to disable all those fancy p8 features too?

#define   PCR_VEC_DIS   (1ul << (63-0)) /* Vec. disable (bit NA since 
POWER8) */
#define   PCR_VSX_DIS   (1ul << (63-1)) /* VSX disable (bit NA since 
POWER8) */
#define   PCR_TM_DIS    (1ul << (63-2)) /* Trans. memory disable (POWER8) */

In fact, we probably want those bits always set when compat < power8.


Alex

Re: [Qemu-devel] [PATCH 7/9] spapr: Limit threads per core according to current compatibility mode

[Alexander Graf]

On 15.05.14 13:28, Alexey Kardashevskiy wrote:
> This puts a limit to the number of threads per core based on the current
> compatibility mode. Although PowerISA specs do not specify the maximum
> threads per core number, the linux guest still expects that
> PowerISA2.05-compatible CPU supports only 2 threads per core as this
> is what POWER6 (2.05 compliant CPU) implements, same is true for
> POWER7 (2.06, 4 threads) and POWER8 (2.07, 8 threads).
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>   hw/ppc/spapr.c | 31 ++++++++++++++++++++++++++++---
>   1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index cf53a7a..a2c9106 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -34,6 +34,7 @@
>   #include "sysemu/kvm.h"
>   #include "kvm_ppc.h"
>   #include "mmu-hash64.h"
> +#include "cpu-models.h"
>   
>   #include "hw/boards.h"
>   #include "hw/ppc/ppc.h"
> @@ -203,6 +204,29 @@ static XICSState *xics_system_init(int nr_servers, int nr_irqs)
>       return icp;
>   }
>   
> +static int spapr_get_compat_smp_threads(PowerPCCPU *cpu)
> +{
> +    int ret = -1;
> +
> +    switch (cpu->cpu_version) {
> +    case CPU_POWERPC_LOGICAL_2_05:
> +        ret = 2;
> +        break;
> +    case CPU_POWERPC_LOGICAL_2_06:
> +        ret = 4;
> +        break;
> +    case CPU_POWERPC_LOGICAL_2_07:
> +        ret = 8;
> +        break;
> +    default:
> +        ret = smp_threads;
> +        break;
> +    }
> +    ret = MIN(ret, smp_threads);

Just call this in the function where you set the compat property and 
error out if threads > supported_threads. We always default to 1 thread 
anyway, so the only thing that could get us a broken configuration is 
the user.


Alex

Re: [Qemu-devel] [PATCH 8/9] spapr: Implement processor compatibility in ibm, client-architecture-support

[Alexander Graf]

On 15.05.14 13:28, Alexey Kardashevskiy wrote:
> Modern Linux kernels support last POWERPC CPUs so when a kernel boots,
> in most cases it can find a matching cpu_spec in the kernel's cpu_specs
> list. However if the kernel is quite old, it may be missing a definition
> of the actual CPU. To provide an ability for old kernels to work on modern
> hardware, a Processor Compatibility Mode has been introduced
> by the PowerISA specification.
>
>  From the hardware prospective, it is supported by the Processor
> Compatibility Register (PCR) which is defined in PowerISA. The register
> enables one of the compatibility modes (2.05/2.06/2.07).
> Since PCR is a hypervisor privileged register and cannot be
> accessed from the guest, the mode selection is done via
> ibm,client-architecture-support (CAS) RTAS call using which the guest
> specifies what "raw" and "architected" CPU versions it supports.
> QEMU works out the best match, changes a "cpu-version" property of
> every CPU and notifies the guest about the change by setting these
> properties in the buffer passed as a response on a custom H_CAS hypercall.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>   hw/ppc/spapr.c       | 40 +++++++++++++++++++++++++
>   hw/ppc/spapr_hcall.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   trace-events         |  5 ++++
>   3 files changed, 128 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a2c9106..a0882a1 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -35,6 +35,7 @@
>   #include "kvm_ppc.h"
>   #include "mmu-hash64.h"
>   #include "cpu-models.h"
> +#include "qom/cpu.h"
>   
>   #include "hw/boards.h"
>   #include "hw/ppc/ppc.h"
> @@ -592,11 +593,50 @@ int spapr_h_cas_compose_response(target_ulong addr, target_ulong size)
>   {
>       void *fdt;
>       sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
> +    CPUState *cs;
> +    int smt = kvmppc_smt_threads();
>   
>       size -= sizeof(hdr);
>   
>       fdt = g_malloc0(size);
>       _FDT((fdt_create(fdt, size)));
> +    _FDT((fdt_begin_node(fdt, "cpus")));
> +
> +    CPU_FOREACH(cs) {
> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
> +        DeviceClass *dc = DEVICE_GET_CLASS(cpu);
> +        int smpt = spapr_get_compat_smp_threads(cpu);
> +        int i, index = ppc_get_vcpu_dt_id(cpu);
> +        uint32_t servers_prop[smpt];
> +        uint32_t gservers_prop[smpt * 2];
> +        char tmp[32];
> +
> +        if ((index % smt) != 0) {
> +            continue;
> +        }
> +
> +        snprintf(tmp, 32, "%s@%x", dc->fw_name, index);
> +        trace_spapr_cas_add_subnode(tmp);
> +
> +        _FDT((fdt_begin_node(fdt, tmp)));
> +        _FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
> +
> +        /* Build interrupt servers and gservers properties */
> +        for (i = 0; i < smpt; i++) {
> +            servers_prop[i] = cpu_to_be32(index + i);
> +            /* Hack, direct the group queues back to cpu 0 */
> +            gservers_prop[i*2] = cpu_to_be32(index + i);
> +            gservers_prop[i*2 + 1] = 0;
> +        }
> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
> +                           servers_prop, sizeof(servers_prop))));
> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
> +                           gservers_prop, sizeof(gservers_prop))));
> +
> +        _FDT((fdt_end_node(fdt)));
> +    }
> +
> +    _FDT((fdt_end_node(fdt)));

Why is this so much code? Can we only replace full nodes? Then please 
extract the original CPU node creation into a function and just call it 
from the original generation and from here.

If we can also replace single properties I'd say we only need to 
override cpu-version.

>   
>       _FDT((fdt_finish(fdt)));
>   
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 2f6aa5c..cb815c3 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -3,6 +3,9 @@
>   #include "helper_regs.h"
>   #include "hw/ppc/spapr.h"
>   #include "mmu-hash64.h"
> +#include "cpu-models.h"
> +#include "trace.h"
> +#include "kvm_ppc.h"
>   
>   struct SPRSyncState {
>       CPUState *cs;
> @@ -752,12 +755,92 @@ out:
>       return ret;
>   }
>   
> +#define get_compat_level(cpuver) ( \
> +    ((cpuver) == CPU_POWERPC_LOGICAL_2_05) ? 2050 : \
> +    ((cpuver) == CPU_POWERPC_LOGICAL_2_06) ? 2060 : \
> +    ((cpuver) == CPU_POWERPC_LOGICAL_2_06_PLUS) ? 2061 : \
> +    ((cpuver) == CPU_POWERPC_LOGICAL_2_07) ? 2070 : 0)
> +
>   static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
>                                                     sPAPREnvironment *spapr,
>                                                     target_ulong opcode,
>                                                     target_ulong *args)
>   {
>       target_ulong list = args[0];
> +    PowerPCCPUClass *pcc_ = POWERPC_CPU_GET_CLASS(cpu_);
> +    CPUState *cs;
> +    CPUState *cs_ = CPU(cpu_);
> +    bool cpu_match = false;
> +    unsigned old_cpu_version = cpu_->cpu_version;
> +    unsigned compat_lvl = 0, cpu_version = 0;
> +    unsigned max_lvl = get_compat_level(cpu_->max_compat);
> +
> +    /* Parse PVR list */
> +    for ( ; ; ) {
> +        uint32_t pvr, pvr_mask;
> +
> +        pvr_mask = ldl_phys(cs_->as, list);

We access memory that the guest thinks gets accessed by real mode, so we 
need to mask it again. Just use the rtas helpers.

> +        list += 4;
> +        pvr = ldl_phys(cs_->as, list);
> +        list += 4;
> +
> +        trace_spapr_cas_pvr_try(pvr);
> +        if (!max_lvl &&
> +            ((cpu_->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask))) {
> +            cpu_match = true;
> +            cpu_version = 0;
> +        } else if (pvr == cpu_->cpu_version) {
> +            cpu_match = true;
> +            cpu_version = cpu_->cpu_version;
> +        } else if (!cpu_match) {
> +            /* If it is a logical PVR, try to determine the highest level */
> +            unsigned lvl = get_compat_level(pvr);
> +            if (lvl) {
> +                bool is205 = (pcc_->pcr_mask & POWERPC_ISA_COMPAT_2_05) &&
> +                     (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05));
> +                bool is206 = (pcc_->pcr_mask & POWERPC_ISA_COMPAT_2_06) &&
> +                    ((lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06)) ||
> +                    (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06_PLUS)));
> +
> +                if (is205 || is206) {
> +                    if (!max_lvl) {
> +                        /* User did not set the level, choose the highest */
> +                        if (compat_lvl <= lvl) {
> +                            compat_lvl = lvl;
> +                            cpu_version = pvr;
> +                        }
> +                    } else if (max_lvl >= lvl) {
> +                        /* User chose the level, don't set higher than this */
> +                        compat_lvl = lvl;
> +                        cpu_version = pvr;
> +                    }
> +                }
> +            }
> +        }
> +        /* Terminator record */
> +        if (~pvr_mask & pvr) {
> +            break;
> +        }
> +    }
> +
> +    /* For the future use: here @list points to the first capability */
> +
> +    /* Parsing finished */
> +    trace_spapr_cas_pvr(cpu_->cpu_version, cpu_match,
> +                        cpu_version, pcc_->pcr_mask);
> +
> +    /* Update CPUs */
> +    if (old_cpu_version != cpu_version) {
> +        CPU_FOREACH(cs) {
> +            PowerPCCPU *cpu = POWERPC_CPU(cs);
> +
> +            ppc_set_compat(cpu, cpu_version);

Better make this a run_on call to ensure the vcpus get their new compat 
properties set.


Alex

> +        }
> +    }
> +
> +    if (!cpu_version) {
> +        return H_SUCCESS;
> +    }
>   
>       if (!list) {
>           return H_SUCCESS;
> diff --git a/trace-events b/trace-events
> index 03565dc..c2db909 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1182,6 +1182,11 @@ xics_ics_eoi(int nr) "ics_eoi: irq %#x"
>   # hw/ppc/spapr.c
>   spapr_cas_failed(unsigned long n) "DT diff buffer is too small: %ld bytes"
>   spapr_cas_continue(unsigned long n) "Copy changes to the guest: %ld bytes"
> +spapr_cas_add_subnode(const char *s) "%s"
> +
> +# hw/ppc/spapr_hcall.c
> +spapr_cas_pvr_try(uint32_t pvr) "%x"
> +spapr_cas_pvr(uint32_t cur_pvr, bool cpu_match, uint32_t new_pvr, uint64_t pcr) "current=%x, cpu_match=%u, new=%x, compat flags=%"PRIx64
>   
>   # hw/ppc/spapr_iommu.c
>   spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64

Re: [Qemu-devel] [PATCH 9/9] KVM: PPC: Enable compatibility mode

[Alexander Graf]

On 15.05.14 13:28, Alexey Kardashevskiy wrote:
> The host kernel implements a KVM_REG_PPC_ARCH_COMPAT register which
> this uses to enable a compatibility mode if any chosen.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>   hw/ppc/spapr.c       | 6 ++++++
>   hw/ppc/spapr_hcall.c | 4 ++++
>   target-ppc/kvm.c     | 5 +++++
>   target-ppc/kvm_ppc.h | 6 ++++++
>   4 files changed, 21 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a0882a1..f89be10 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1376,6 +1376,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>           /* Tell KVM that we're in PAPR mode */
>           if (kvm_enabled()) {
>               kvmppc_set_papr(cpu);
> +
> +            if (cpu->max_compat &&
> +                kvmppc_set_compat(cpu, cpu->max_compat) < 0) {
> +                fprintf(stderr, "Unable to set compatibility mode\n");
> +                exit(1);

Why is KVM special here?

> +            }
>           }
>   
>           if (cpu->max_compat) {
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index cb815c3..2ab21d3 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -834,6 +834,10 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
>           CPU_FOREACH(cs) {
>               PowerPCCPU *cpu = POWERPC_CPU(cs);
>   
> +            if (kvmppc_set_compat(cpu, cpu_version) < 0) {
> +                fprintf(stderr, "Unable to set compatibility mode\n");
> +                return H_HARDWARE;
> +            }

Just fold this into ppc_set_compat which will run from vcpu context.


Alex

>               ppc_set_compat(cpu, cpu_version);
>           }
>       }
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index ff319fc..f8e8453 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1513,6 +1513,11 @@ void kvmppc_set_papr(PowerPCCPU *cpu)
>       cap_papr = 1;
>   }
>   
> +int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
> +{
> +    return kvm_set_one_reg(CPU(cpu), KVM_REG_PPC_ARCH_COMPAT, &cpu_version);
> +}
> +
>   void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>   {
>       CPUState *cs = CPU(cpu);
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index ff077ec..716c33d 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -23,6 +23,7 @@ int kvmppc_get_hasidle(CPUPPCState *env);
>   int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
>   int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
>   void kvmppc_set_papr(PowerPCCPU *cpu);
> +int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>   void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
>   int kvmppc_smt_threads(void);
>   int kvmppc_clear_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits);
> @@ -95,6 +96,11 @@ static inline void kvmppc_set_papr(PowerPCCPU *cpu)
>   {
>   }
>   
> +static inline int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
> +{
> +    return 0;
> +}
> +
>   static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>   {
>   }

Re: [Qemu-devel] [PATCH 9/9] KVM: PPC: Enable compatibility mode

[Alexey Kardashevskiy]

On 05/17/2014 12:18 AM, Alexander Graf wrote:
> 
> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>> The host kernel implements a KVM_REG_PPC_ARCH_COMPAT register which
>> this uses to enable a compatibility mode if any chosen.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>   hw/ppc/spapr.c       | 6 ++++++
>>   hw/ppc/spapr_hcall.c | 4 ++++
>>   target-ppc/kvm.c     | 5 +++++
>>   target-ppc/kvm_ppc.h | 6 ++++++
>>   4 files changed, 21 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index a0882a1..f89be10 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1376,6 +1376,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>           /* Tell KVM that we're in PAPR mode */
>>           if (kvm_enabled()) {
>>               kvmppc_set_papr(cpu);
>> +
>> +            if (cpu->max_compat &&
>> +                kvmppc_set_compat(cpu, cpu->max_compat) < 0) {
>> +                fprintf(stderr, "Unable to set compatibility mode\n");
>> +                exit(1);
> 
> Why is KVM special here?

Sorry, I am not following you here. There is no SPR for that which guest
kernel could change, this is a register from "kvm set/get one reg" interface.

> 
>> +            }
>>           }
>>             if (cpu->max_compat) {
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index cb815c3..2ab21d3 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -834,6 +834,10 @@ static target_ulong
>> h_client_architecture_support(PowerPCCPU *cpu_,
>>           CPU_FOREACH(cs) {
>>               PowerPCCPU *cpu = POWERPC_CPU(cs);
>>   +            if (kvmppc_set_compat(cpu, cpu_version) < 0) {
>> +                fprintf(stderr, "Unable to set compatibility mode\n");
>> +                return H_HARDWARE;
>> +            }
> 
> Just fold this into ppc_set_compat which will run from vcpu context.


TCG version cannot fail and KVM's one can. Adding non-void return value
would give impression that ppc_set_compat may fail while it cannot. Still fold?



> 
> 
> Alex
> 
>>               ppc_set_compat(cpu, cpu_version);
>>           }
>>       }
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index ff319fc..f8e8453 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -1513,6 +1513,11 @@ void kvmppc_set_papr(PowerPCCPU *cpu)
>>       cap_papr = 1;
>>   }
>>   +int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
>> +{
>> +    return kvm_set_one_reg(CPU(cpu), KVM_REG_PPC_ARCH_COMPAT,
>> &cpu_version);
>> +}
>> +
>>   void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>>   {
>>       CPUState *cs = CPU(cpu);
>> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
>> index ff077ec..716c33d 100644
>> --- a/target-ppc/kvm_ppc.h
>> +++ b/target-ppc/kvm_ppc.h
>> @@ -23,6 +23,7 @@ int kvmppc_get_hasidle(CPUPPCState *env);
>>   int kvmppc_get_hypercall(CPUPPCState *env, uint8_t *buf, int buf_len);
>>   int kvmppc_set_interrupt(PowerPCCPU *cpu, int irq, int level);
>>   void kvmppc_set_papr(PowerPCCPU *cpu);
>> +int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>>   void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
>>   int kvmppc_smt_threads(void);
>>   int kvmppc_clear_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits);
>> @@ -95,6 +96,11 @@ static inline void kvmppc_set_papr(PowerPCCPU *cpu)
>>   {
>>   }
>>   +static inline int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t
>> cpu_version)
>> +{
>> +    return 0;
>> +}
>> +
>>   static inline void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy)
>>   {
>>   }
> 


-- 
Alexey

Re: [Qemu-devel] [PATCH 9/9] KVM: PPC: Enable compatibility mode

[Alexander Graf]

On 16.05.14 16:27, Alexey Kardashevskiy wrote:
> On 05/17/2014 12:18 AM, Alexander Graf wrote:
>> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>>> The host kernel implements a KVM_REG_PPC_ARCH_COMPAT register which
>>> this uses to enable a compatibility mode if any chosen.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>    hw/ppc/spapr.c       | 6 ++++++
>>>    hw/ppc/spapr_hcall.c | 4 ++++
>>>    target-ppc/kvm.c     | 5 +++++
>>>    target-ppc/kvm_ppc.h | 6 ++++++
>>>    4 files changed, 21 insertions(+)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index a0882a1..f89be10 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -1376,6 +1376,12 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>>            /* Tell KVM that we're in PAPR mode */
>>>            if (kvm_enabled()) {
>>>                kvmppc_set_papr(cpu);
>>> +
>>> +            if (cpu->max_compat &&
>>> +                kvmppc_set_compat(cpu, cpu->max_compat) < 0) {
>>> +                fprintf(stderr, "Unable to set compatibility mode\n");
>>> +                exit(1);
>> Why is KVM special here?
> Sorry, I am not following you here. There is no SPR for that which guest
> kernel could change, this is a register from "kvm set/get one reg" interface.

So semantically, what is ppc_set_compat() then? Why would we have to set 
the KVM SPR that represents which compat mode we're in here, but not the 
TCG version of it?

>
>>> +            }
>>>            }
>>>              if (cpu->max_compat) {
>>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>>> index cb815c3..2ab21d3 100644
>>> --- a/hw/ppc/spapr_hcall.c
>>> +++ b/hw/ppc/spapr_hcall.c
>>> @@ -834,6 +834,10 @@ static target_ulong
>>> h_client_architecture_support(PowerPCCPU *cpu_,
>>>            CPU_FOREACH(cs) {
>>>                PowerPCCPU *cpu = POWERPC_CPU(cs);
>>>    +            if (kvmppc_set_compat(cpu, cpu_version) < 0) {
>>> +                fprintf(stderr, "Unable to set compatibility mode\n");
>>> +                return H_HARDWARE;
>>> +            }
>> Just fold this into ppc_set_compat which will run from vcpu context.
>
> TCG version cannot fail and KVM's one can. Adding non-void return value
> would give impression that ppc_set_compat may fail while it cannot. Still fold?

Yes, please.


Alex

Re: [Qemu-devel] [PATCH 4/9] target-ppc: Implement "compat" CPU option

[Alexey Kardashevskiy]

On 05/17/2014 12:05 AM, Alexander Graf wrote:
> 
> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>> This adds basic support for the "compat" CPU option. By specifying
>> the compat property, the user can manually switch guest CPU mode from
>> "raw" to "architected".
>>
>> Since the actual compatibility mode is not implemented yet, this does
>> not change the existing behavior.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>   hw/ppc/spapr.c              | 15 ++++++++++++++-
>>   target-ppc/cpu-models.h     |  6 ++++++
>>   target-ppc/cpu-qom.h        |  2 ++
>>   target-ppc/cpu.h            |  3 +++
>>   target-ppc/translate_init.c | 35 +++++++++++++++++++++++++++++++++++
>>   5 files changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 0f8bd95..61d0675 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -212,7 +212,8 @@ static int spapr_fixup_cpu_dt(void *fdt,
>> sPAPREnvironment *spapr)
>>         CPU_FOREACH(cpu) {
>>           DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>> -        int index = ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>> +        PowerPCCPU *pcpu = POWERPC_CPU(cpu);
>> +        int index = ppc_get_vcpu_dt_id(pcpu);
>>           uint32_t associativity[] = {cpu_to_be32(0x5),
>>                                       cpu_to_be32(0x0),
>>                                       cpu_to_be32(0x0),
>> @@ -249,6 +250,14 @@ static int spapr_fixup_cpu_dt(void *fdt,
>> sPAPREnvironment *spapr)
>>               return ret;
>>           }
>>   +        if (pcpu->cpu_version) {
>> +            ret = fdt_setprop(fdt, offset, "cpu-version",
>> +                              &pcpu->cpu_version,
>> sizeof(pcpu->cpu_version));
>> +            if (ret < 0) {
>> +                return ret;
>> +            }
>> +        }
>> +
>>           /* Build interrupt servers and gservers properties */
>>           for (i = 0; i < smp_threads; i++) {
>>               servers_prop[i] = cpu_to_be32(index + i);
>> @@ -1278,6 +1287,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>               kvmppc_set_papr(cpu);
>>           }
>>   +        if (cpu->max_compat) {
>> +            ppc_set_compat(cpu, cpu->max_compat);
>> +        }
>> +
>>           xics_cpu_setup(spapr->icp, cpu);
>>             qemu_register_reset(spapr_cpu_reset, cpu);
>> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
>> index 33d2c51..45c0028 100644
>> --- a/target-ppc/cpu-models.h
>> +++ b/target-ppc/cpu-models.h
>> @@ -753,4 +753,10 @@ enum {
>>       POWERPC_SVR_8641D              = 0x80900121,
>>   };
>>   +/* Processor Compatibility mask (PCR) */
>> +enum {
>> +    POWERPC_ISA_COMPAT_2_05 = 0x02,
>> +    POWERPC_ISA_COMPAT_2_06 = 0x04,
>> +};
>> +
>>   #endif
>> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
>> index 2a8515b..dfd1419 100644
>> --- a/target-ppc/cpu-qom.h
>> +++ b/target-ppc/cpu-qom.h
>> @@ -83,6 +83,7 @@ typedef struct PowerPCCPUClass {
>>    * @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
>> + * @cpu_version: Current logical PVR, zero if in "raw" mode
>>    *
>>    * A PowerPC CPU.
>>    */
>> @@ -94,6 +95,7 @@ struct PowerPCCPU {
>>       CPUPPCState env;
>>       int cpu_dt_id;
>>       uint32_t max_compat;
>> +    uint32_t cpu_version;
>>   };
>>     static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>> index d498340..f61675a 100644
>> --- a/target-ppc/cpu.h
>> +++ b/target-ppc/cpu.h
>> @@ -1123,6 +1123,8 @@ void ppc_store_msr (CPUPPCState *env, target_ulong
>> value);
>>     void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
>>   +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>> +
>>   /* Time-base and decrementer management */
>>   #ifndef NO_CPU_IO_DEFS
>>   uint64_t cpu_ppc_load_tbl (CPUPPCState *env);
>> @@ -1338,6 +1340,7 @@ static inline int cpu_mmu_index (CPUPPCState *env)
>>   #define SPR_LPCR              (0x13E)
>>   #define SPR_BOOKE_DVC2        (0x13F)
>>   #define SPR_BOOKE_TSR         (0x150)
>> +#define SPR_PCR               (0x152)
>>   #define SPR_BOOKE_TCR         (0x154)
>>   #define SPR_BOOKE_TLB0PS      (0x158)
>>   #define SPR_BOOKE_TLB1PS      (0x159)
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 6f61b34..c4bd5de 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -7803,6 +7803,15 @@ static void init_proc_POWER7 (CPUPPCState *env)
>>       /* Can't find information on what this should be on reset.  This
>>        * value is the one used by 74xx processors. */
>>       vscr_init(env, 0x00010000);
>> +
>> +    /*
>> +     * Register PCR to report POWERPC_EXCP_PRIV_REG instead of
>> +     * POWERPC_EXCP_INVAL_SPR.
>> +     */
>> +    spr_register(env, SPR_PCR, "PCR",
>> +                 SPR_NOACCESS, SPR_NOACCESS,
>> +                 SPR_NOACCESS, SPR_NOACCESS,
>> +                 0x00000000);
>>   }
>>     POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>> @@ -8880,6 +8889,32 @@ static void ppc_cpu_unrealizefn(DeviceState *dev,
>> Error **errp)
>>       }
>>   }
>>   +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
>> +{
>> +    CPUPPCState *env = &cpu->env;
>> +
>> +    cpu->cpu_version = cpu_version;
>> +
>> +    /*
>> +     * Calculate PCR value from virtual PVR.
>> +     * TODO: support actual compatibility in TCG.
>> +     */
>> +    switch (cpu_version) {
>> +    case CPU_POWERPC_LOGICAL_2_05:
>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_05;
>> +        break;
>> +    case CPU_POWERPC_LOGICAL_2_06:
>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>> +        break;
>> +    case CPU_POWERPC_LOGICAL_2_06_PLUS:
>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>> +        break;
> 
> Don't we have to disable all those fancy p8 features too?
> 
> #define   PCR_VEC_DIS   (1ul << (63-0)) /* Vec. disable (bit NA since
> POWER8) */
> #define   PCR_VSX_DIS   (1ul << (63-1)) /* VSX disable (bit NA since
> POWER8) */
> #define   PCR_TM_DIS    (1ul << (63-2)) /* Trans. memory disable (POWER8) */

Yep, makes sense to set "TM" for POWER < 8 and VSX+VEC for POWER < 7.

> In fact, we probably want those bits always set when compat < power8.

Why? VSX is present on power7. VEC is defined in 2.06 too.



-- 
Alexey

Re: [Qemu-devel] [PATCH 8/9] spapr: Implement processor compatibility in ibm, client-architecture-support

[Alexey Kardashevskiy]

On 05/17/2014 12:16 AM, Alexander Graf wrote:
> 
> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>> Modern Linux kernels support last POWERPC CPUs so when a kernel boots,
>> in most cases it can find a matching cpu_spec in the kernel's cpu_specs
>> list. However if the kernel is quite old, it may be missing a definition
>> of the actual CPU. To provide an ability for old kernels to work on modern
>> hardware, a Processor Compatibility Mode has been introduced
>> by the PowerISA specification.
>>
>>  From the hardware prospective, it is supported by the Processor
>> Compatibility Register (PCR) which is defined in PowerISA. The register
>> enables one of the compatibility modes (2.05/2.06/2.07).
>> Since PCR is a hypervisor privileged register and cannot be
>> accessed from the guest, the mode selection is done via
>> ibm,client-architecture-support (CAS) RTAS call using which the guest
>> specifies what "raw" and "architected" CPU versions it supports.
>> QEMU works out the best match, changes a "cpu-version" property of
>> every CPU and notifies the guest about the change by setting these
>> properties in the buffer passed as a response on a custom H_CAS hypercall.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>   hw/ppc/spapr.c       | 40 +++++++++++++++++++++++++
>>   hw/ppc/spapr_hcall.c | 83
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   trace-events         |  5 ++++
>>   3 files changed, 128 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index a2c9106..a0882a1 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -35,6 +35,7 @@
>>   #include "kvm_ppc.h"
>>   #include "mmu-hash64.h"
>>   #include "cpu-models.h"
>> +#include "qom/cpu.h"
>>     #include "hw/boards.h"
>>   #include "hw/ppc/ppc.h"
>> @@ -592,11 +593,50 @@ int spapr_h_cas_compose_response(target_ulong addr,
>> target_ulong size)
>>   {
>>       void *fdt;
>>       sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
>> +    CPUState *cs;
>> +    int smt = kvmppc_smt_threads();
>>         size -= sizeof(hdr);
>>         fdt = g_malloc0(size);
>>       _FDT((fdt_create(fdt, size)));
>> +    _FDT((fdt_begin_node(fdt, "cpus")));
>> +
>> +    CPU_FOREACH(cs) {
>> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
>> +        DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>> +        int smpt = spapr_get_compat_smp_threads(cpu);
>> +        int i, index = ppc_get_vcpu_dt_id(cpu);
>> +        uint32_t servers_prop[smpt];
>> +        uint32_t gservers_prop[smpt * 2];
>> +        char tmp[32];
>> +
>> +        if ((index % smt) != 0) {
>> +            continue;
>> +        }
>> +
>> +        snprintf(tmp, 32, "%s@%x", dc->fw_name, index);
>> +        trace_spapr_cas_add_subnode(tmp);
>> +
>> +        _FDT((fdt_begin_node(fdt, tmp)));
>> +        _FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
>> +
>> +        /* Build interrupt servers and gservers properties */
>> +        for (i = 0; i < smpt; i++) {
>> +            servers_prop[i] = cpu_to_be32(index + i);
>> +            /* Hack, direct the group queues back to cpu 0 */
>> +            gservers_prop[i*2] = cpu_to_be32(index + i);
>> +            gservers_prop[i*2 + 1] = 0;
>> +        }
>> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
>> +                           servers_prop, sizeof(servers_prop))));
>> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
>> +                           gservers_prop, sizeof(gservers_prop))));
>> +
>> +        _FDT((fdt_end_node(fdt)));
>> +    }
>> +
>> +    _FDT((fdt_end_node(fdt)));
> 
> Why is this so much code? Can we only replace full nodes?

It is a diff tree, in only has properties to change.

> Then please
> extract the original CPU node creation into a function and just call it
> from the original generation and from here.
> 
> If we can also replace single properties I'd say we only need to override
> cpu-version.


Mmm. The user could run qemu with threads=8 on POWER8 with SLES11 which
must not see 8 threads but it can update itself and reboot with the kernel
which is aware of POWER8. You are saying we do not want to support that?




-- 
Alexey

Re: [Qemu-devel] [PATCH 8/9] spapr: Implement processor compatibility in ibm, client-architecture-support

[Alexander Graf]

On 16.05.14 17:57, Alexey Kardashevskiy wrote:
> On 05/17/2014 12:16 AM, Alexander Graf wrote:
>> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>>> Modern Linux kernels support last POWERPC CPUs so when a kernel boots,
>>> in most cases it can find a matching cpu_spec in the kernel's cpu_specs
>>> list. However if the kernel is quite old, it may be missing a definition
>>> of the actual CPU. To provide an ability for old kernels to work on modern
>>> hardware, a Processor Compatibility Mode has been introduced
>>> by the PowerISA specification.
>>>
>>>   From the hardware prospective, it is supported by the Processor
>>> Compatibility Register (PCR) which is defined in PowerISA. The register
>>> enables one of the compatibility modes (2.05/2.06/2.07).
>>> Since PCR is a hypervisor privileged register and cannot be
>>> accessed from the guest, the mode selection is done via
>>> ibm,client-architecture-support (CAS) RTAS call using which the guest
>>> specifies what "raw" and "architected" CPU versions it supports.
>>> QEMU works out the best match, changes a "cpu-version" property of
>>> every CPU and notifies the guest about the change by setting these
>>> properties in the buffer passed as a response on a custom H_CAS hypercall.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>    hw/ppc/spapr.c       | 40 +++++++++++++++++++++++++
>>>    hw/ppc/spapr_hcall.c | 83
>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>    trace-events         |  5 ++++
>>>    3 files changed, 128 insertions(+)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index a2c9106..a0882a1 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -35,6 +35,7 @@
>>>    #include "kvm_ppc.h"
>>>    #include "mmu-hash64.h"
>>>    #include "cpu-models.h"
>>> +#include "qom/cpu.h"
>>>      #include "hw/boards.h"
>>>    #include "hw/ppc/ppc.h"
>>> @@ -592,11 +593,50 @@ int spapr_h_cas_compose_response(target_ulong addr,
>>> target_ulong size)
>>>    {
>>>        void *fdt;
>>>        sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
>>> +    CPUState *cs;
>>> +    int smt = kvmppc_smt_threads();
>>>          size -= sizeof(hdr);
>>>          fdt = g_malloc0(size);
>>>        _FDT((fdt_create(fdt, size)));
>>> +    _FDT((fdt_begin_node(fdt, "cpus")));
>>> +
>>> +    CPU_FOREACH(cs) {
>>> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
>>> +        DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>>> +        int smpt = spapr_get_compat_smp_threads(cpu);
>>> +        int i, index = ppc_get_vcpu_dt_id(cpu);
>>> +        uint32_t servers_prop[smpt];
>>> +        uint32_t gservers_prop[smpt * 2];
>>> +        char tmp[32];
>>> +
>>> +        if ((index % smt) != 0) {
>>> +            continue;
>>> +        }
>>> +
>>> +        snprintf(tmp, 32, "%s@%x", dc->fw_name, index);
>>> +        trace_spapr_cas_add_subnode(tmp);
>>> +
>>> +        _FDT((fdt_begin_node(fdt, tmp)));
>>> +        _FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
>>> +
>>> +        /* Build interrupt servers and gservers properties */
>>> +        for (i = 0; i < smpt; i++) {
>>> +            servers_prop[i] = cpu_to_be32(index + i);
>>> +            /* Hack, direct the group queues back to cpu 0 */
>>> +            gservers_prop[i*2] = cpu_to_be32(index + i);
>>> +            gservers_prop[i*2 + 1] = 0;
>>> +        }
>>> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
>>> +                           servers_prop, sizeof(servers_prop))));
>>> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
>>> +                           gservers_prop, sizeof(gservers_prop))));
>>> +
>>> +        _FDT((fdt_end_node(fdt)));
>>> +    }
>>> +
>>> +    _FDT((fdt_end_node(fdt)));
>> Why is this so much code? Can we only replace full nodes?
> It is a diff tree, in only has properties to change.

So why do we change so much then? :)

>
>> Then please
>> extract the original CPU node creation into a function and just call it
>> from the original generation and from here.
>>
>> If we can also replace single properties I'd say we only need to override
>> cpu-version.
>
> Mmm. The user could run qemu with threads=8 on POWER8 with SLES11 which
> must not see 8 threads but it can update itself and reboot with the kernel
> which is aware of POWER8. You are saying we do not want to support that?

First off, I think that practically speaking people will want to use 
2-thread granularity on POWER8 because that gives them the best load 
exploitation on the system.

So if we just disable CPU nodes that would not be visible usually (there 
was a disabled flag in cpu nodes, right?) for threads that are 
incompatible with a new CAS mode, we can then remove the disabled flag 
when the guest accepts more threads again.

That means we could expose 8 threads (POWER8), guest only supports 2 
threads (POWER6), so we waste 6 threads. But the remaining 2 will be 
fast ;). We basically degrade the system to SMT2 mode.

What does pHyp do here?


Alex

Re: [Qemu-devel] [PATCH 4/9] target-ppc: Implement "compat" CPU option

[Alexander Graf]

On 16.05.14 17:17, Alexey Kardashevskiy wrote:
> On 05/17/2014 12:05 AM, Alexander Graf wrote:
>> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>>> This adds basic support for the "compat" CPU option. By specifying
>>> the compat property, the user can manually switch guest CPU mode from
>>> "raw" to "architected".
>>>
>>> Since the actual compatibility mode is not implemented yet, this does
>>> not change the existing behavior.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>    hw/ppc/spapr.c              | 15 ++++++++++++++-
>>>    target-ppc/cpu-models.h     |  6 ++++++
>>>    target-ppc/cpu-qom.h        |  2 ++
>>>    target-ppc/cpu.h            |  3 +++
>>>    target-ppc/translate_init.c | 35 +++++++++++++++++++++++++++++++++++
>>>    5 files changed, 60 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>> index 0f8bd95..61d0675 100644
>>> --- a/hw/ppc/spapr.c
>>> +++ b/hw/ppc/spapr.c
>>> @@ -212,7 +212,8 @@ static int spapr_fixup_cpu_dt(void *fdt,
>>> sPAPREnvironment *spapr)
>>>          CPU_FOREACH(cpu) {
>>>            DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>>> -        int index = ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>>> +        PowerPCCPU *pcpu = POWERPC_CPU(cpu);
>>> +        int index = ppc_get_vcpu_dt_id(pcpu);
>>>            uint32_t associativity[] = {cpu_to_be32(0x5),
>>>                                        cpu_to_be32(0x0),
>>>                                        cpu_to_be32(0x0),
>>> @@ -249,6 +250,14 @@ static int spapr_fixup_cpu_dt(void *fdt,
>>> sPAPREnvironment *spapr)
>>>                return ret;
>>>            }
>>>    +        if (pcpu->cpu_version) {
>>> +            ret = fdt_setprop(fdt, offset, "cpu-version",
>>> +                              &pcpu->cpu_version,
>>> sizeof(pcpu->cpu_version));
>>> +            if (ret < 0) {
>>> +                return ret;
>>> +            }
>>> +        }
>>> +
>>>            /* Build interrupt servers and gservers properties */
>>>            for (i = 0; i < smp_threads; i++) {
>>>                servers_prop[i] = cpu_to_be32(index + i);
>>> @@ -1278,6 +1287,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>>>                kvmppc_set_papr(cpu);
>>>            }
>>>    +        if (cpu->max_compat) {
>>> +            ppc_set_compat(cpu, cpu->max_compat);
>>> +        }
>>> +
>>>            xics_cpu_setup(spapr->icp, cpu);
>>>              qemu_register_reset(spapr_cpu_reset, cpu);
>>> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
>>> index 33d2c51..45c0028 100644
>>> --- a/target-ppc/cpu-models.h
>>> +++ b/target-ppc/cpu-models.h
>>> @@ -753,4 +753,10 @@ enum {
>>>        POWERPC_SVR_8641D              = 0x80900121,
>>>    };
>>>    +/* Processor Compatibility mask (PCR) */
>>> +enum {
>>> +    POWERPC_ISA_COMPAT_2_05 = 0x02,
>>> +    POWERPC_ISA_COMPAT_2_06 = 0x04,
>>> +};
>>> +
>>>    #endif
>>> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
>>> index 2a8515b..dfd1419 100644
>>> --- a/target-ppc/cpu-qom.h
>>> +++ b/target-ppc/cpu-qom.h
>>> @@ -83,6 +83,7 @@ typedef struct PowerPCCPUClass {
>>>     * @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
>>> + * @cpu_version: Current logical PVR, zero if in "raw" mode
>>>     *
>>>     * A PowerPC CPU.
>>>     */
>>> @@ -94,6 +95,7 @@ struct PowerPCCPU {
>>>        CPUPPCState env;
>>>        int cpu_dt_id;
>>>        uint32_t max_compat;
>>> +    uint32_t cpu_version;
>>>    };
>>>      static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>> index d498340..f61675a 100644
>>> --- a/target-ppc/cpu.h
>>> +++ b/target-ppc/cpu.h
>>> @@ -1123,6 +1123,8 @@ void ppc_store_msr (CPUPPCState *env, target_ulong
>>> value);
>>>      void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
>>>    +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>>> +
>>>    /* Time-base and decrementer management */
>>>    #ifndef NO_CPU_IO_DEFS
>>>    uint64_t cpu_ppc_load_tbl (CPUPPCState *env);
>>> @@ -1338,6 +1340,7 @@ static inline int cpu_mmu_index (CPUPPCState *env)
>>>    #define SPR_LPCR              (0x13E)
>>>    #define SPR_BOOKE_DVC2        (0x13F)
>>>    #define SPR_BOOKE_TSR         (0x150)
>>> +#define SPR_PCR               (0x152)
>>>    #define SPR_BOOKE_TCR         (0x154)
>>>    #define SPR_BOOKE_TLB0PS      (0x158)
>>>    #define SPR_BOOKE_TLB1PS      (0x159)
>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>> index 6f61b34..c4bd5de 100644
>>> --- a/target-ppc/translate_init.c
>>> +++ b/target-ppc/translate_init.c
>>> @@ -7803,6 +7803,15 @@ static void init_proc_POWER7 (CPUPPCState *env)
>>>        /* Can't find information on what this should be on reset.  This
>>>         * value is the one used by 74xx processors. */
>>>        vscr_init(env, 0x00010000);
>>> +
>>> +    /*
>>> +     * Register PCR to report POWERPC_EXCP_PRIV_REG instead of
>>> +     * POWERPC_EXCP_INVAL_SPR.
>>> +     */
>>> +    spr_register(env, SPR_PCR, "PCR",
>>> +                 SPR_NOACCESS, SPR_NOACCESS,
>>> +                 SPR_NOACCESS, SPR_NOACCESS,
>>> +                 0x00000000);
>>>    }
>>>      POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>>> @@ -8880,6 +8889,32 @@ static void ppc_cpu_unrealizefn(DeviceState *dev,
>>> Error **errp)
>>>        }
>>>    }
>>>    +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
>>> +{
>>> +    CPUPPCState *env = &cpu->env;
>>> +
>>> +    cpu->cpu_version = cpu_version;
>>> +
>>> +    /*
>>> +     * Calculate PCR value from virtual PVR.
>>> +     * TODO: support actual compatibility in TCG.
>>> +     */
>>> +    switch (cpu_version) {
>>> +    case CPU_POWERPC_LOGICAL_2_05:
>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_05;
>>> +        break;
>>> +    case CPU_POWERPC_LOGICAL_2_06:
>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>>> +        break;
>>> +    case CPU_POWERPC_LOGICAL_2_06_PLUS:
>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>>> +        break;
>> Don't we have to disable all those fancy p8 features too?
>>
>> #define   PCR_VEC_DIS   (1ul << (63-0)) /* Vec. disable (bit NA since
>> POWER8) */
>> #define   PCR_VSX_DIS   (1ul << (63-1)) /* VSX disable (bit NA since
>> POWER8) */
>> #define   PCR_TM_DIS    (1ul << (63-2)) /* Trans. memory disable (POWER8) */
> Yep, makes sense to set "TM" for POWER < 8 and VSX+VEC for POWER < 7.
>
>> In fact, we probably want those bits always set when compat < power8.
> Why? VSX is present on power7. VEC is defined in 2.06 too.

True. So how does pre-p8 treat those p8 and above bits?


Alex

Re: [Qemu-devel] [PATCH 8/9] spapr: Implement processor compatibility in ibm, client-architecture-support

[Alexey Kardashevskiy]

On 05/17/2014 06:46 AM, Alexander Graf wrote:
> 
> On 16.05.14 17:57, Alexey Kardashevskiy wrote:
>> On 05/17/2014 12:16 AM, Alexander Graf wrote:
>>> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>>>> Modern Linux kernels support last POWERPC CPUs so when a kernel boots,
>>>> in most cases it can find a matching cpu_spec in the kernel's cpu_specs
>>>> list. However if the kernel is quite old, it may be missing a definition
>>>> of the actual CPU. To provide an ability for old kernels to work on modern
>>>> hardware, a Processor Compatibility Mode has been introduced
>>>> by the PowerISA specification.
>>>>
>>>>   From the hardware prospective, it is supported by the Processor
>>>> Compatibility Register (PCR) which is defined in PowerISA. The register
>>>> enables one of the compatibility modes (2.05/2.06/2.07).
>>>> Since PCR is a hypervisor privileged register and cannot be
>>>> accessed from the guest, the mode selection is done via
>>>> ibm,client-architecture-support (CAS) RTAS call using which the guest
>>>> specifies what "raw" and "architected" CPU versions it supports.
>>>> QEMU works out the best match, changes a "cpu-version" property of
>>>> every CPU and notifies the guest about the change by setting these
>>>> properties in the buffer passed as a response on a custom H_CAS hypercall.
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>> ---
>>>>    hw/ppc/spapr.c       | 40 +++++++++++++++++++++++++
>>>>    hw/ppc/spapr_hcall.c | 83
>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>    trace-events         |  5 ++++
>>>>    3 files changed, 128 insertions(+)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index a2c9106..a0882a1 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -35,6 +35,7 @@
>>>>    #include "kvm_ppc.h"
>>>>    #include "mmu-hash64.h"
>>>>    #include "cpu-models.h"
>>>> +#include "qom/cpu.h"
>>>>      #include "hw/boards.h"
>>>>    #include "hw/ppc/ppc.h"
>>>> @@ -592,11 +593,50 @@ int spapr_h_cas_compose_response(target_ulong addr,
>>>> target_ulong size)
>>>>    {
>>>>        void *fdt;
>>>>        sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
>>>> +    CPUState *cs;
>>>> +    int smt = kvmppc_smt_threads();
>>>>          size -= sizeof(hdr);
>>>>          fdt = g_malloc0(size);
>>>>        _FDT((fdt_create(fdt, size)));
>>>> +    _FDT((fdt_begin_node(fdt, "cpus")));
>>>> +
>>>> +    CPU_FOREACH(cs) {
>>>> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
>>>> +        DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>>>> +        int smpt = spapr_get_compat_smp_threads(cpu);
>>>> +        int i, index = ppc_get_vcpu_dt_id(cpu);
>>>> +        uint32_t servers_prop[smpt];
>>>> +        uint32_t gservers_prop[smpt * 2];
>>>> +        char tmp[32];
>>>> +
>>>> +        if ((index % smt) != 0) {
>>>> +            continue;
>>>> +        }
>>>> +
>>>> +        snprintf(tmp, 32, "%s@%x", dc->fw_name, index);
>>>> +        trace_spapr_cas_add_subnode(tmp);
>>>> +
>>>> +        _FDT((fdt_begin_node(fdt, tmp)));
>>>> +        _FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
>>>> +
>>>> +        /* Build interrupt servers and gservers properties */
>>>> +        for (i = 0; i < smpt; i++) {
>>>> +            servers_prop[i] = cpu_to_be32(index + i);
>>>> +            /* Hack, direct the group queues back to cpu 0 */
>>>> +            gservers_prop[i*2] = cpu_to_be32(index + i);
>>>> +            gservers_prop[i*2 + 1] = 0;
>>>> +        }
>>>> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
>>>> +                           servers_prop, sizeof(servers_prop))));
>>>> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
>>>> +                           gservers_prop, sizeof(gservers_prop))));
>>>> +
>>>> +        _FDT((fdt_end_node(fdt)));
>>>> +    }
>>>> +
>>>> +    _FDT((fdt_end_node(fdt)));
>>> Why is this so much code? Can we only replace full nodes?
>> It is a diff tree, in only has properties to change.
> 
> So why do we change so much then? :)


3 (three) properties are "much" now? :)

>>> Then please
>>> extract the original CPU node creation into a function and just call it
>>> from the original generation and from here.
>>>
>>> If we can also replace single properties I'd say we only need to override
>>> cpu-version.
>>
>> Mmm. The user could run qemu with threads=8 on POWER8 with SLES11 which
>> must not see 8 threads but it can update itself and reboot with the kernel
>> which is aware of POWER8. You are saying we do not want to support that?
> 
> First off, I think that practically speaking people will want to use
> 2-thread granularity on POWER8 because that gives them the best load
> exploitation on the system.

Practically speaking, people will want to use kernels which can work on
POWER8 in raw mode, no? Here we are trying to fix older guests.

> So if we just disable CPU nodes that would not be visible usually (there
> was a disabled flag in cpu nodes, right?) for threads that are incompatible
> with a new CAS mode, we can then remove the disabled flag when the guest
> accepts more threads again.
> 
> That means we could expose 8 threads (POWER8), guest only supports 2
> threads (POWER6), so we waste 6 threads. But the remaining 2 will be fast
> ;). We basically degrade the system to SMT2 mode.

We do not want to show more threads (i.e. bigger ibm,ppc-interrupt-server#s
properties) if we do not support more threads as we do not really know what
guest kernel (including AIX) or some ppc64_cpu/drmgr/whatever tool will do
if they find more.

> What does pHyp do here?

I wish it was easy to verify :)

I fail to see what exactly your objections are all about, I definitely miss
something big here. Is it a minor code duplication or some dangerous bug?
If it is just a code duplication, I could do something about it.


-- 
Alexey

Re: [Qemu-devel] [PATCH 8/9] spapr: Implement processor compatibility in ibm, client-architecture-support

[Alexey Kardashevskiy]

On 05/17/2014 11:45 AM, Alexey Kardashevskiy wrote:
> On 05/17/2014 06:46 AM, Alexander Graf wrote:
>>
>> On 16.05.14 17:57, Alexey Kardashevskiy wrote:
>>> On 05/17/2014 12:16 AM, Alexander Graf wrote:
>>>> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>>>>> Modern Linux kernels support last POWERPC CPUs so when a kernel boots,
>>>>> in most cases it can find a matching cpu_spec in the kernel's cpu_specs
>>>>> list. However if the kernel is quite old, it may be missing a definition
>>>>> of the actual CPU. To provide an ability for old kernels to work on modern
>>>>> hardware, a Processor Compatibility Mode has been introduced
>>>>> by the PowerISA specification.
>>>>>
>>>>>   From the hardware prospective, it is supported by the Processor
>>>>> Compatibility Register (PCR) which is defined in PowerISA. The register
>>>>> enables one of the compatibility modes (2.05/2.06/2.07).
>>>>> Since PCR is a hypervisor privileged register and cannot be
>>>>> accessed from the guest, the mode selection is done via
>>>>> ibm,client-architecture-support (CAS) RTAS call using which the guest
>>>>> specifies what "raw" and "architected" CPU versions it supports.
>>>>> QEMU works out the best match, changes a "cpu-version" property of
>>>>> every CPU and notifies the guest about the change by setting these
>>>>> properties in the buffer passed as a response on a custom H_CAS hypercall.
>>>>>
>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>> ---
>>>>>    hw/ppc/spapr.c       | 40 +++++++++++++++++++++++++
>>>>>    hw/ppc/spapr_hcall.c | 83
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>    trace-events         |  5 ++++
>>>>>    3 files changed, 128 insertions(+)
>>>>>
>>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>>> index a2c9106..a0882a1 100644
>>>>> --- a/hw/ppc/spapr.c
>>>>> +++ b/hw/ppc/spapr.c
>>>>> @@ -35,6 +35,7 @@
>>>>>    #include "kvm_ppc.h"
>>>>>    #include "mmu-hash64.h"
>>>>>    #include "cpu-models.h"
>>>>> +#include "qom/cpu.h"
>>>>>      #include "hw/boards.h"
>>>>>    #include "hw/ppc/ppc.h"
>>>>> @@ -592,11 +593,50 @@ int spapr_h_cas_compose_response(target_ulong addr,
>>>>> target_ulong size)
>>>>>    {
>>>>>        void *fdt;
>>>>>        sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
>>>>> +    CPUState *cs;
>>>>> +    int smt = kvmppc_smt_threads();
>>>>>          size -= sizeof(hdr);
>>>>>          fdt = g_malloc0(size);
>>>>>        _FDT((fdt_create(fdt, size)));
>>>>> +    _FDT((fdt_begin_node(fdt, "cpus")));
>>>>> +
>>>>> +    CPU_FOREACH(cs) {
>>>>> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
>>>>> +        DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>>>>> +        int smpt = spapr_get_compat_smp_threads(cpu);
>>>>> +        int i, index = ppc_get_vcpu_dt_id(cpu);
>>>>> +        uint32_t servers_prop[smpt];
>>>>> +        uint32_t gservers_prop[smpt * 2];
>>>>> +        char tmp[32];
>>>>> +
>>>>> +        if ((index % smt) != 0) {
>>>>> +            continue;
>>>>> +        }
>>>>> +
>>>>> +        snprintf(tmp, 32, "%s@%x", dc->fw_name, index);
>>>>> +        trace_spapr_cas_add_subnode(tmp);
>>>>> +
>>>>> +        _FDT((fdt_begin_node(fdt, tmp)));
>>>>> +        _FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
>>>>> +
>>>>> +        /* Build interrupt servers and gservers properties */
>>>>> +        for (i = 0; i < smpt; i++) {
>>>>> +            servers_prop[i] = cpu_to_be32(index + i);
>>>>> +            /* Hack, direct the group queues back to cpu 0 */
>>>>> +            gservers_prop[i*2] = cpu_to_be32(index + i);
>>>>> +            gservers_prop[i*2 + 1] = 0;
>>>>> +        }
>>>>> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
>>>>> +                           servers_prop, sizeof(servers_prop))));
>>>>> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
>>>>> +                           gservers_prop, sizeof(gservers_prop))));
>>>>> +
>>>>> +        _FDT((fdt_end_node(fdt)));
>>>>> +    }
>>>>> +
>>>>> +    _FDT((fdt_end_node(fdt)));
>>>> Why is this so much code? Can we only replace full nodes?
>>> It is a diff tree, in only has properties to change.
>>
>> So why do we change so much then? :)
> 
> 
> 3 (three) properties are "much" now? :)
> 
>>>> Then please
>>>> extract the original CPU node creation into a function and just call it
>>>> from the original generation and from here.
>>>>
>>>> If we can also replace single properties I'd say we only need to override
>>>> cpu-version.
>>>
>>> Mmm. The user could run qemu with threads=8 on POWER8 with SLES11 which
>>> must not see 8 threads but it can update itself and reboot with the kernel
>>> which is aware of POWER8. You are saying we do not want to support that?
>>
>> First off, I think that practically speaking people will want to use
>> 2-thread granularity on POWER8 because that gives them the best load
>> exploitation on the system.
> 
> Practically speaking, people will want to use kernels which can work on
> POWER8 in raw mode, no? Here we are trying to fix older guests.
> 
>> So if we just disable CPU nodes that would not be visible usually (there
>> was a disabled flag in cpu nodes, right?) for threads that are incompatible
>> with a new CAS mode, we can then remove the disabled flag when the guest
>> accepts more threads again.

I just realized where misunderstanding happened - when I switch from 4
threads per core to 2 threads per core, the number of CPU nodes does not
change, only server#s and gserver#s properties do and this is the only way
of telling the guest the new number of threads.

So if you still want to disable some nodes, please explain more. Thanks!


>> That means we could expose 8 threads (POWER8), guest only supports 2
>> threads (POWER6), so we waste 6 threads. But the remaining 2 will be fast
>> ;). We basically degrade the system to SMT2 mode.
> 
> We do not want to show more threads (i.e. bigger ibm,ppc-interrupt-server#s
> properties) if we do not support more threads as we do not really know what
> guest kernel (including AIX) or some ppc64_cpu/drmgr/whatever tool will do
> if they find more.
> 
>> What does pHyp do here?
> 
> I wish it was easy to verify :)
> 
> I fail to see what exactly your objections are all about, I definitely miss
> something big here. Is it a minor code duplication or some dangerous bug?
> If it is just a code duplication, I could do something about it.
> 
> 


-- 
Alexey

Re: [Qemu-devel] [Qemu-ppc] [PATCH 6/9] spapr: Add ibm, client-architecture-support call

[Nikunj A Dadhania]

Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> The PAPR+ specification defines a ibm,client-architecture-support (CAS)
> RTAS call which purpose is to provide a negotiation mechanism for
> the guest and the hypervisor to work out the best compatibility parameters.
> During the negotiation process, the guest provides an array of various
> options and capabilities which it supports, the hypervisor adjusts
> the device tree and (optionally) reboots the guest.
>
> At the moment the Linux guest calls CAS method at early boot so SLOF
> gets called. SLOF allocates a memory buffer for the device tree changes
> and calls a custom KVMPPC_H_CAS hypercall. QEMU parses the options,
> composes a diff for the device tree, copies it to the buffer provided
> by SLOF and returns to SLOF. SLOF updates the device tree and returns
> control to the guest kernel. Only then the Linux guest parses the device
> tree so it is possible to avoid unnecessary reboot in most cases.
>
> The device tree diff is a header with the update format version
> (defined as 0 in this patch) 

That is 1 in the code.

> @@ -562,6 +563,31 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>      return fdt;
>  }
>
> +int spapr_h_cas_compose_response(target_ulong addr, target_ulong size)
> +{
> +    void *fdt;
> +    sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
> +

Regards
Nikunj

Re: [Qemu-devel] [PATCH 8/9] spapr: Implement processor compatibility in ibm, client-architecture-support

[Alexander Graf]

On 17.05.14 03:45, Alexey Kardashevskiy wrote:
> On 05/17/2014 06:46 AM, Alexander Graf wrote:
>> On 16.05.14 17:57, Alexey Kardashevskiy wrote:
>>> On 05/17/2014 12:16 AM, Alexander Graf wrote:
>>>> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>>>>> Modern Linux kernels support last POWERPC CPUs so when a kernel boots,
>>>>> in most cases it can find a matching cpu_spec in the kernel's cpu_specs
>>>>> list. However if the kernel is quite old, it may be missing a definition
>>>>> of the actual CPU. To provide an ability for old kernels to work on modern
>>>>> hardware, a Processor Compatibility Mode has been introduced
>>>>> by the PowerISA specification.
>>>>>
>>>>>    From the hardware prospective, it is supported by the Processor
>>>>> Compatibility Register (PCR) which is defined in PowerISA. The register
>>>>> enables one of the compatibility modes (2.05/2.06/2.07).
>>>>> Since PCR is a hypervisor privileged register and cannot be
>>>>> accessed from the guest, the mode selection is done via
>>>>> ibm,client-architecture-support (CAS) RTAS call using which the guest
>>>>> specifies what "raw" and "architected" CPU versions it supports.
>>>>> QEMU works out the best match, changes a "cpu-version" property of
>>>>> every CPU and notifies the guest about the change by setting these
>>>>> properties in the buffer passed as a response on a custom H_CAS hypercall.
>>>>>
>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>> ---
>>>>>     hw/ppc/spapr.c       | 40 +++++++++++++++++++++++++
>>>>>     hw/ppc/spapr_hcall.c | 83
>>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>>     trace-events         |  5 ++++
>>>>>     3 files changed, 128 insertions(+)
>>>>>
>>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>>> index a2c9106..a0882a1 100644
>>>>> --- a/hw/ppc/spapr.c
>>>>> +++ b/hw/ppc/spapr.c
>>>>> @@ -35,6 +35,7 @@
>>>>>     #include "kvm_ppc.h"
>>>>>     #include "mmu-hash64.h"
>>>>>     #include "cpu-models.h"
>>>>> +#include "qom/cpu.h"
>>>>>       #include "hw/boards.h"
>>>>>     #include "hw/ppc/ppc.h"
>>>>> @@ -592,11 +593,50 @@ int spapr_h_cas_compose_response(target_ulong addr,
>>>>> target_ulong size)
>>>>>     {
>>>>>         void *fdt;
>>>>>         sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
>>>>> +    CPUState *cs;
>>>>> +    int smt = kvmppc_smt_threads();
>>>>>           size -= sizeof(hdr);
>>>>>           fdt = g_malloc0(size);
>>>>>         _FDT((fdt_create(fdt, size)));
>>>>> +    _FDT((fdt_begin_node(fdt, "cpus")));
>>>>> +
>>>>> +    CPU_FOREACH(cs) {
>>>>> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
>>>>> +        DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>>>>> +        int smpt = spapr_get_compat_smp_threads(cpu);
>>>>> +        int i, index = ppc_get_vcpu_dt_id(cpu);
>>>>> +        uint32_t servers_prop[smpt];
>>>>> +        uint32_t gservers_prop[smpt * 2];
>>>>> +        char tmp[32];
>>>>> +
>>>>> +        if ((index % smt) != 0) {
>>>>> +            continue;
>>>>> +        }
>>>>> +
>>>>> +        snprintf(tmp, 32, "%s@%x", dc->fw_name, index);
>>>>> +        trace_spapr_cas_add_subnode(tmp);
>>>>> +
>>>>> +        _FDT((fdt_begin_node(fdt, tmp)));
>>>>> +        _FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
>>>>> +
>>>>> +        /* Build interrupt servers and gservers properties */
>>>>> +        for (i = 0; i < smpt; i++) {
>>>>> +            servers_prop[i] = cpu_to_be32(index + i);
>>>>> +            /* Hack, direct the group queues back to cpu 0 */
>>>>> +            gservers_prop[i*2] = cpu_to_be32(index + i);
>>>>> +            gservers_prop[i*2 + 1] = 0;
>>>>> +        }
>>>>> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
>>>>> +                           servers_prop, sizeof(servers_prop))));
>>>>> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
>>>>> +                           gservers_prop, sizeof(gservers_prop))));
>>>>> +
>>>>> +        _FDT((fdt_end_node(fdt)));
>>>>> +    }
>>>>> +
>>>>> +    _FDT((fdt_end_node(fdt)));
>>>> Why is this so much code? Can we only replace full nodes?
>>> It is a diff tree, in only has properties to change.
>> So why do we change so much then? :)
>
> 3 (three) properties are "much" now? :)
>
>>>> Then please
>>>> extract the original CPU node creation into a function and just call it
>>>> from the original generation and from here.
>>>>
>>>> If we can also replace single properties I'd say we only need to override
>>>> cpu-version.
>>> Mmm. The user could run qemu with threads=8 on POWER8 with SLES11 which
>>> must not see 8 threads but it can update itself and reboot with the kernel
>>> which is aware of POWER8. You are saying we do not want to support that?
>> First off, I think that practically speaking people will want to use
>> 2-thread granularity on POWER8 because that gives them the best load
>> exploitation on the system.
> Practically speaking, people will want to use kernels which can work on
> POWER8 in raw mode, no? Here we are trying to fix older guests.
>
>> So if we just disable CPU nodes that would not be visible usually (there
>> was a disabled flag in cpu nodes, right?) for threads that are incompatible
>> with a new CAS mode, we can then remove the disabled flag when the guest
>> accepts more threads again.
>>
>> That means we could expose 8 threads (POWER8), guest only supports 2
>> threads (POWER6), so we waste 6 threads. But the remaining 2 will be fast
>> ;). We basically degrade the system to SMT2 mode.
> We do not want to show more threads (i.e. bigger ibm,ppc-interrupt-server#s
> properties) if we do not support more threads as we do not really know what
> guest kernel (including AIX) or some ppc64_cpu/drmgr/whatever tool will do
> if they find more.
>
>> What does pHyp do here?
> I wish it was easy to verify :)
>
> I fail to see what exactly your objections are all about, I definitely miss
> something big here. Is it a minor code duplication or some dangerous bug?
> If it is just a code duplication, I could do something about it.

My main concern is that this is a big chunk of magic code that swizzles 
around random device tree properties without an easy to follow 
explanation that would show people what exactly the code does.

I think you can overcome all of this by combining the function you 
introduce here with the one that we already have for cpu node 
generation. You would of course need to parameterize it.

While you're at the process of combining the functions, you will realize 
that some things are not exactly obvious to every reader. For example

+        if ((index % smt) != 0) {
+            continue;
+        }


is based on the fact that only cores are listed in /cpus. But the code 
doesn't tell you, so if anyone who is not Alexey wants to debug this 
code and find out what's going wrong, he has to sit down for a few 
minutes are read up on that fact. It'd be a lot nicer to hint him 
towards the correct direction. This could either be a comment or reader 
friendly code such as

   for_each_core(...) {
   }

at which point the reader would immediately know what we're trying to do.

The same goes for the interrupt servers and gservers properties. If we 
don't explain the format at least by variable / function names in the 
code, you either have to know the format off the top of your head or 
constantly have the sPAPR spec (which is not public) lying next to you. 
This increases the barrier to understanding the code.

For example, if you would just extract the interrupt server generation 
into a function and give it parameters such as "num_threads" and give 
every value a good name before you assign it to the property array, the 
code suddenly becomes self-documenting. If you then share that function 
with the normal cpu node generation, anyone who fixes a bug in it also 
only has to look at a single spot.


Alex

Re: [Qemu-devel] [PATCH 4/9] target-ppc: Implement "compat" CPU option

[Alexey Kardashevskiy]

On 05/17/2014 06:47 AM, Alexander Graf wrote:
> 
> On 16.05.14 17:17, Alexey Kardashevskiy wrote:
>> On 05/17/2014 12:05 AM, Alexander Graf wrote:
>>> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>>>> This adds basic support for the "compat" CPU option. By specifying
>>>> the compat property, the user can manually switch guest CPU mode from
>>>> "raw" to "architected".
>>>>
>>>> Since the actual compatibility mode is not implemented yet, this does
>>>> not change the existing behavior.
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>> ---
>>>>    hw/ppc/spapr.c              | 15 ++++++++++++++-
>>>>    target-ppc/cpu-models.h     |  6 ++++++
>>>>    target-ppc/cpu-qom.h        |  2 ++
>>>>    target-ppc/cpu.h            |  3 +++
>>>>    target-ppc/translate_init.c | 35 +++++++++++++++++++++++++++++++++++
>>>>    5 files changed, 60 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>> index 0f8bd95..61d0675 100644
>>>> --- a/hw/ppc/spapr.c
>>>> +++ b/hw/ppc/spapr.c
>>>> @@ -212,7 +212,8 @@ static int spapr_fixup_cpu_dt(void *fdt,
>>>> sPAPREnvironment *spapr)
>>>>          CPU_FOREACH(cpu) {
>>>>            DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>>>> -        int index = ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>>>> +        PowerPCCPU *pcpu = POWERPC_CPU(cpu);
>>>> +        int index = ppc_get_vcpu_dt_id(pcpu);
>>>>            uint32_t associativity[] = {cpu_to_be32(0x5),
>>>>                                        cpu_to_be32(0x0),
>>>>                                        cpu_to_be32(0x0),
>>>> @@ -249,6 +250,14 @@ static int spapr_fixup_cpu_dt(void *fdt,
>>>> sPAPREnvironment *spapr)
>>>>                return ret;
>>>>            }
>>>>    +        if (pcpu->cpu_version) {
>>>> +            ret = fdt_setprop(fdt, offset, "cpu-version",
>>>> +                              &pcpu->cpu_version,
>>>> sizeof(pcpu->cpu_version));
>>>> +            if (ret < 0) {
>>>> +                return ret;
>>>> +            }
>>>> +        }
>>>> +
>>>>            /* Build interrupt servers and gservers properties */
>>>>            for (i = 0; i < smp_threads; i++) {
>>>>                servers_prop[i] = cpu_to_be32(index + i);
>>>> @@ -1278,6 +1287,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs
>>>> *args)
>>>>                kvmppc_set_papr(cpu);
>>>>            }
>>>>    +        if (cpu->max_compat) {
>>>> +            ppc_set_compat(cpu, cpu->max_compat);
>>>> +        }
>>>> +
>>>>            xics_cpu_setup(spapr->icp, cpu);
>>>>              qemu_register_reset(spapr_cpu_reset, cpu);
>>>> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
>>>> index 33d2c51..45c0028 100644
>>>> --- a/target-ppc/cpu-models.h
>>>> +++ b/target-ppc/cpu-models.h
>>>> @@ -753,4 +753,10 @@ enum {
>>>>        POWERPC_SVR_8641D              = 0x80900121,
>>>>    };
>>>>    +/* Processor Compatibility mask (PCR) */
>>>> +enum {
>>>> +    POWERPC_ISA_COMPAT_2_05 = 0x02,
>>>> +    POWERPC_ISA_COMPAT_2_06 = 0x04,
>>>> +};
>>>> +
>>>>    #endif
>>>> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
>>>> index 2a8515b..dfd1419 100644
>>>> --- a/target-ppc/cpu-qom.h
>>>> +++ b/target-ppc/cpu-qom.h
>>>> @@ -83,6 +83,7 @@ typedef struct PowerPCCPUClass {
>>>>     * @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
>>>> + * @cpu_version: Current logical PVR, zero if in "raw" mode
>>>>     *
>>>>     * A PowerPC CPU.
>>>>     */
>>>> @@ -94,6 +95,7 @@ struct PowerPCCPU {
>>>>        CPUPPCState env;
>>>>        int cpu_dt_id;
>>>>        uint32_t max_compat;
>>>> +    uint32_t cpu_version;
>>>>    };
>>>>      static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
>>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>>> index d498340..f61675a 100644
>>>> --- a/target-ppc/cpu.h
>>>> +++ b/target-ppc/cpu.h
>>>> @@ -1123,6 +1123,8 @@ void ppc_store_msr (CPUPPCState *env, target_ulong
>>>> value);
>>>>      void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
>>>>    +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>>>> +
>>>>    /* Time-base and decrementer management */
>>>>    #ifndef NO_CPU_IO_DEFS
>>>>    uint64_t cpu_ppc_load_tbl (CPUPPCState *env);
>>>> @@ -1338,6 +1340,7 @@ static inline int cpu_mmu_index (CPUPPCState *env)
>>>>    #define SPR_LPCR              (0x13E)
>>>>    #define SPR_BOOKE_DVC2        (0x13F)
>>>>    #define SPR_BOOKE_TSR         (0x150)
>>>> +#define SPR_PCR               (0x152)
>>>>    #define SPR_BOOKE_TCR         (0x154)
>>>>    #define SPR_BOOKE_TLB0PS      (0x158)
>>>>    #define SPR_BOOKE_TLB1PS      (0x159)
>>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>>> index 6f61b34..c4bd5de 100644
>>>> --- a/target-ppc/translate_init.c
>>>> +++ b/target-ppc/translate_init.c
>>>> @@ -7803,6 +7803,15 @@ static void init_proc_POWER7 (CPUPPCState *env)
>>>>        /* Can't find information on what this should be on reset.  This
>>>>         * value is the one used by 74xx processors. */
>>>>        vscr_init(env, 0x00010000);
>>>> +
>>>> +    /*
>>>> +     * Register PCR to report POWERPC_EXCP_PRIV_REG instead of
>>>> +     * POWERPC_EXCP_INVAL_SPR.
>>>> +     */
>>>> +    spr_register(env, SPR_PCR, "PCR",
>>>> +                 SPR_NOACCESS, SPR_NOACCESS,
>>>> +                 SPR_NOACCESS, SPR_NOACCESS,
>>>> +                 0x00000000);
>>>>    }
>>>>      POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>>>> @@ -8880,6 +8889,32 @@ static void ppc_cpu_unrealizefn(DeviceState *dev,
>>>> Error **errp)
>>>>        }
>>>>    }
>>>>    +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
>>>> +{
>>>> +    CPUPPCState *env = &cpu->env;
>>>> +
>>>> +    cpu->cpu_version = cpu_version;
>>>> +
>>>> +    /*
>>>> +     * Calculate PCR value from virtual PVR.
>>>> +     * TODO: support actual compatibility in TCG.
>>>> +     */
>>>> +    switch (cpu_version) {
>>>> +    case CPU_POWERPC_LOGICAL_2_05:
>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_05;
>>>> +        break;
>>>> +    case CPU_POWERPC_LOGICAL_2_06:
>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>>>> +        break;
>>>> +    case CPU_POWERPC_LOGICAL_2_06_PLUS:
>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>>>> +        break;
>>> Don't we have to disable all those fancy p8 features too?
>>>
>>> #define   PCR_VEC_DIS   (1ul << (63-0)) /* Vec. disable (bit NA since
>>> POWER8) */
>>> #define   PCR_VSX_DIS   (1ul << (63-1)) /* VSX disable (bit NA since
>>> POWER8) */
>>> #define   PCR_TM_DIS    (1ul << (63-2)) /* Trans. memory disable
>>> (POWER8) */
>> Yep, makes sense to set "TM" for POWER < 8 and VSX+VEC for POWER < 7.
>>
>>> In fact, we probably want those bits always set when compat < power8.
>> Why? VSX is present on power7. VEC is defined in 2.06 too.
> 
> True. So how does pre-p8 treat those p8 and above bits?

PowerISA 2.07 says if 2.06 mode is selected, the TM bit does not matter
anymore - TM will be disabled because 2.06 does not define it at all. The
same is true for VSX and 2.05 mode. So just setting a mode must be ok.



-- 
Alexey

Re: [Qemu-devel] [PATCH 4/9] target-ppc: Implement "compat" CPU option

[Alexander Graf]

On 21.05.14 08:57, Alexey Kardashevskiy wrote:
> On 05/17/2014 06:47 AM, Alexander Graf wrote:
>> On 16.05.14 17:17, Alexey Kardashevskiy wrote:
>>> On 05/17/2014 12:05 AM, Alexander Graf wrote:
>>>> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>>>>> This adds basic support for the "compat" CPU option. By specifying
>>>>> the compat property, the user can manually switch guest CPU mode from
>>>>> "raw" to "architected".
>>>>>
>>>>> Since the actual compatibility mode is not implemented yet, this does
>>>>> not change the existing behavior.
>>>>>
>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>> ---
>>>>>     hw/ppc/spapr.c              | 15 ++++++++++++++-
>>>>>     target-ppc/cpu-models.h     |  6 ++++++
>>>>>     target-ppc/cpu-qom.h        |  2 ++
>>>>>     target-ppc/cpu.h            |  3 +++
>>>>>     target-ppc/translate_init.c | 35 +++++++++++++++++++++++++++++++++++
>>>>>     5 files changed, 60 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>>> index 0f8bd95..61d0675 100644
>>>>> --- a/hw/ppc/spapr.c
>>>>> +++ b/hw/ppc/spapr.c
>>>>> @@ -212,7 +212,8 @@ static int spapr_fixup_cpu_dt(void *fdt,
>>>>> sPAPREnvironment *spapr)
>>>>>           CPU_FOREACH(cpu) {
>>>>>             DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>>>>> -        int index = ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>>>>> +        PowerPCCPU *pcpu = POWERPC_CPU(cpu);
>>>>> +        int index = ppc_get_vcpu_dt_id(pcpu);
>>>>>             uint32_t associativity[] = {cpu_to_be32(0x5),
>>>>>                                         cpu_to_be32(0x0),
>>>>>                                         cpu_to_be32(0x0),
>>>>> @@ -249,6 +250,14 @@ static int spapr_fixup_cpu_dt(void *fdt,
>>>>> sPAPREnvironment *spapr)
>>>>>                 return ret;
>>>>>             }
>>>>>     +        if (pcpu->cpu_version) {
>>>>> +            ret = fdt_setprop(fdt, offset, "cpu-version",
>>>>> +                              &pcpu->cpu_version,
>>>>> sizeof(pcpu->cpu_version));
>>>>> +            if (ret < 0) {
>>>>> +                return ret;
>>>>> +            }
>>>>> +        }
>>>>> +
>>>>>             /* Build interrupt servers and gservers properties */
>>>>>             for (i = 0; i < smp_threads; i++) {
>>>>>                 servers_prop[i] = cpu_to_be32(index + i);
>>>>> @@ -1278,6 +1287,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs
>>>>> *args)
>>>>>                 kvmppc_set_papr(cpu);
>>>>>             }
>>>>>     +        if (cpu->max_compat) {
>>>>> +            ppc_set_compat(cpu, cpu->max_compat);
>>>>> +        }
>>>>> +
>>>>>             xics_cpu_setup(spapr->icp, cpu);
>>>>>               qemu_register_reset(spapr_cpu_reset, cpu);
>>>>> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
>>>>> index 33d2c51..45c0028 100644
>>>>> --- a/target-ppc/cpu-models.h
>>>>> +++ b/target-ppc/cpu-models.h
>>>>> @@ -753,4 +753,10 @@ enum {
>>>>>         POWERPC_SVR_8641D              = 0x80900121,
>>>>>     };
>>>>>     +/* Processor Compatibility mask (PCR) */
>>>>> +enum {
>>>>> +    POWERPC_ISA_COMPAT_2_05 = 0x02,
>>>>> +    POWERPC_ISA_COMPAT_2_06 = 0x04,
>>>>> +};
>>>>> +
>>>>>     #endif
>>>>> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
>>>>> index 2a8515b..dfd1419 100644
>>>>> --- a/target-ppc/cpu-qom.h
>>>>> +++ b/target-ppc/cpu-qom.h
>>>>> @@ -83,6 +83,7 @@ typedef struct PowerPCCPUClass {
>>>>>      * @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
>>>>> + * @cpu_version: Current logical PVR, zero if in "raw" mode
>>>>>      *
>>>>>      * A PowerPC CPU.
>>>>>      */
>>>>> @@ -94,6 +95,7 @@ struct PowerPCCPU {
>>>>>         CPUPPCState env;
>>>>>         int cpu_dt_id;
>>>>>         uint32_t max_compat;
>>>>> +    uint32_t cpu_version;
>>>>>     };
>>>>>       static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
>>>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>>>> index d498340..f61675a 100644
>>>>> --- a/target-ppc/cpu.h
>>>>> +++ b/target-ppc/cpu.h
>>>>> @@ -1123,6 +1123,8 @@ void ppc_store_msr (CPUPPCState *env, target_ulong
>>>>> value);
>>>>>       void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
>>>>>     +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>>>>> +
>>>>>     /* Time-base and decrementer management */
>>>>>     #ifndef NO_CPU_IO_DEFS
>>>>>     uint64_t cpu_ppc_load_tbl (CPUPPCState *env);
>>>>> @@ -1338,6 +1340,7 @@ static inline int cpu_mmu_index (CPUPPCState *env)
>>>>>     #define SPR_LPCR              (0x13E)
>>>>>     #define SPR_BOOKE_DVC2        (0x13F)
>>>>>     #define SPR_BOOKE_TSR         (0x150)
>>>>> +#define SPR_PCR               (0x152)
>>>>>     #define SPR_BOOKE_TCR         (0x154)
>>>>>     #define SPR_BOOKE_TLB0PS      (0x158)
>>>>>     #define SPR_BOOKE_TLB1PS      (0x159)
>>>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>>>> index 6f61b34..c4bd5de 100644
>>>>> --- a/target-ppc/translate_init.c
>>>>> +++ b/target-ppc/translate_init.c
>>>>> @@ -7803,6 +7803,15 @@ static void init_proc_POWER7 (CPUPPCState *env)
>>>>>         /* Can't find information on what this should be on reset.  This
>>>>>          * value is the one used by 74xx processors. */
>>>>>         vscr_init(env, 0x00010000);
>>>>> +
>>>>> +    /*
>>>>> +     * Register PCR to report POWERPC_EXCP_PRIV_REG instead of
>>>>> +     * POWERPC_EXCP_INVAL_SPR.
>>>>> +     */
>>>>> +    spr_register(env, SPR_PCR, "PCR",
>>>>> +                 SPR_NOACCESS, SPR_NOACCESS,
>>>>> +                 SPR_NOACCESS, SPR_NOACCESS,
>>>>> +                 0x00000000);
>>>>>     }
>>>>>       POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>>>>> @@ -8880,6 +8889,32 @@ static void ppc_cpu_unrealizefn(DeviceState *dev,
>>>>> Error **errp)
>>>>>         }
>>>>>     }
>>>>>     +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
>>>>> +{
>>>>> +    CPUPPCState *env = &cpu->env;
>>>>> +
>>>>> +    cpu->cpu_version = cpu_version;
>>>>> +
>>>>> +    /*
>>>>> +     * Calculate PCR value from virtual PVR.
>>>>> +     * TODO: support actual compatibility in TCG.
>>>>> +     */
>>>>> +    switch (cpu_version) {
>>>>> +    case CPU_POWERPC_LOGICAL_2_05:
>>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_05;
>>>>> +        break;
>>>>> +    case CPU_POWERPC_LOGICAL_2_06:
>>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>>>>> +        break;
>>>>> +    case CPU_POWERPC_LOGICAL_2_06_PLUS:
>>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>>>>> +        break;
>>>> Don't we have to disable all those fancy p8 features too?
>>>>
>>>> #define   PCR_VEC_DIS   (1ul << (63-0)) /* Vec. disable (bit NA since
>>>> POWER8) */
>>>> #define   PCR_VSX_DIS   (1ul << (63-1)) /* VSX disable (bit NA since
>>>> POWER8) */
>>>> #define   PCR_TM_DIS    (1ul << (63-2)) /* Trans. memory disable
>>>> (POWER8) */
>>> Yep, makes sense to set "TM" for POWER < 8 and VSX+VEC for POWER < 7.
>>>
>>>> In fact, we probably want those bits always set when compat < power8.
>>> Why? VSX is present on power7. VEC is defined in 2.06 too.
>> True. So how does pre-p8 treat those p8 and above bits?
> PowerISA 2.07 says if 2.06 mode is selected, the TM bit does not matter
> anymore - TM will be disabled because 2.06 does not define it at all. The
> same is true for VSX and 2.05 mode. So just setting a mode must be ok.

And for POWER8-on-POWER9 (or whatever it will be called) we can just set 
PCR to the respective feature disable bits. I think that should work.


Alex

Re: [Qemu-devel] [PATCH 4/9] target-ppc: Implement "compat" CPU option

[Alexey Kardashevskiy]

On 05/21/2014 05:29 PM, Alexander Graf wrote:
> 
> On 21.05.14 08:57, Alexey Kardashevskiy wrote:
>> On 05/17/2014 06:47 AM, Alexander Graf wrote:
>>> On 16.05.14 17:17, Alexey Kardashevskiy wrote:
>>>> On 05/17/2014 12:05 AM, Alexander Graf wrote:
>>>>> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>>>>>> This adds basic support for the "compat" CPU option. By specifying
>>>>>> the compat property, the user can manually switch guest CPU mode from
>>>>>> "raw" to "architected".
>>>>>>
>>>>>> Since the actual compatibility mode is not implemented yet, this does
>>>>>> not change the existing behavior.
>>>>>>
>>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>>> ---
>>>>>>     hw/ppc/spapr.c              | 15 ++++++++++++++-
>>>>>>     target-ppc/cpu-models.h     |  6 ++++++
>>>>>>     target-ppc/cpu-qom.h        |  2 ++
>>>>>>     target-ppc/cpu.h            |  3 +++
>>>>>>     target-ppc/translate_init.c | 35 +++++++++++++++++++++++++++++++++++
>>>>>>     5 files changed, 60 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>>>> index 0f8bd95..61d0675 100644
>>>>>> --- a/hw/ppc/spapr.c
>>>>>> +++ b/hw/ppc/spapr.c
>>>>>> @@ -212,7 +212,8 @@ static int spapr_fixup_cpu_dt(void *fdt,
>>>>>> sPAPREnvironment *spapr)
>>>>>>           CPU_FOREACH(cpu) {
>>>>>>             DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>>>>>> -        int index = ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>>>>>> +        PowerPCCPU *pcpu = POWERPC_CPU(cpu);
>>>>>> +        int index = ppc_get_vcpu_dt_id(pcpu);
>>>>>>             uint32_t associativity[] = {cpu_to_be32(0x5),
>>>>>>                                         cpu_to_be32(0x0),
>>>>>>                                         cpu_to_be32(0x0),
>>>>>> @@ -249,6 +250,14 @@ static int spapr_fixup_cpu_dt(void *fdt,
>>>>>> sPAPREnvironment *spapr)
>>>>>>                 return ret;
>>>>>>             }
>>>>>>     +        if (pcpu->cpu_version) {
>>>>>> +            ret = fdt_setprop(fdt, offset, "cpu-version",
>>>>>> +                              &pcpu->cpu_version,
>>>>>> sizeof(pcpu->cpu_version));
>>>>>> +            if (ret < 0) {
>>>>>> +                return ret;
>>>>>> +            }
>>>>>> +        }
>>>>>> +
>>>>>>             /* Build interrupt servers and gservers properties */
>>>>>>             for (i = 0; i < smp_threads; i++) {
>>>>>>                 servers_prop[i] = cpu_to_be32(index + i);
>>>>>> @@ -1278,6 +1287,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs
>>>>>> *args)
>>>>>>                 kvmppc_set_papr(cpu);
>>>>>>             }
>>>>>>     +        if (cpu->max_compat) {
>>>>>> +            ppc_set_compat(cpu, cpu->max_compat);
>>>>>> +        }
>>>>>> +
>>>>>>             xics_cpu_setup(spapr->icp, cpu);
>>>>>>               qemu_register_reset(spapr_cpu_reset, cpu);
>>>>>> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
>>>>>> index 33d2c51..45c0028 100644
>>>>>> --- a/target-ppc/cpu-models.h
>>>>>> +++ b/target-ppc/cpu-models.h
>>>>>> @@ -753,4 +753,10 @@ enum {
>>>>>>         POWERPC_SVR_8641D              = 0x80900121,
>>>>>>     };
>>>>>>     +/* Processor Compatibility mask (PCR) */
>>>>>> +enum {
>>>>>> +    POWERPC_ISA_COMPAT_2_05 = 0x02,
>>>>>> +    POWERPC_ISA_COMPAT_2_06 = 0x04,
>>>>>> +};
>>>>>> +
>>>>>>     #endif
>>>>>> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
>>>>>> index 2a8515b..dfd1419 100644
>>>>>> --- a/target-ppc/cpu-qom.h
>>>>>> +++ b/target-ppc/cpu-qom.h
>>>>>> @@ -83,6 +83,7 @@ typedef struct PowerPCCPUClass {
>>>>>>      * @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
>>>>>> + * @cpu_version: Current logical PVR, zero if in "raw" mode
>>>>>>      *
>>>>>>      * A PowerPC CPU.
>>>>>>      */
>>>>>> @@ -94,6 +95,7 @@ struct PowerPCCPU {
>>>>>>         CPUPPCState env;
>>>>>>         int cpu_dt_id;
>>>>>>         uint32_t max_compat;
>>>>>> +    uint32_t cpu_version;
>>>>>>     };
>>>>>>       static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
>>>>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>>>>> index d498340..f61675a 100644
>>>>>> --- a/target-ppc/cpu.h
>>>>>> +++ b/target-ppc/cpu.h
>>>>>> @@ -1123,6 +1123,8 @@ void ppc_store_msr (CPUPPCState *env, target_ulong
>>>>>> value);
>>>>>>       void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
>>>>>>     +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>>>>>> +
>>>>>>     /* Time-base and decrementer management */
>>>>>>     #ifndef NO_CPU_IO_DEFS
>>>>>>     uint64_t cpu_ppc_load_tbl (CPUPPCState *env);
>>>>>> @@ -1338,6 +1340,7 @@ static inline int cpu_mmu_index (CPUPPCState *env)
>>>>>>     #define SPR_LPCR              (0x13E)
>>>>>>     #define SPR_BOOKE_DVC2        (0x13F)
>>>>>>     #define SPR_BOOKE_TSR         (0x150)
>>>>>> +#define SPR_PCR               (0x152)
>>>>>>     #define SPR_BOOKE_TCR         (0x154)
>>>>>>     #define SPR_BOOKE_TLB0PS      (0x158)
>>>>>>     #define SPR_BOOKE_TLB1PS      (0x159)
>>>>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>>>>> index 6f61b34..c4bd5de 100644
>>>>>> --- a/target-ppc/translate_init.c
>>>>>> +++ b/target-ppc/translate_init.c
>>>>>> @@ -7803,6 +7803,15 @@ static void init_proc_POWER7 (CPUPPCState *env)
>>>>>>         /* Can't find information on what this should be on reset.  This
>>>>>>          * value is the one used by 74xx processors. */
>>>>>>         vscr_init(env, 0x00010000);
>>>>>> +
>>>>>> +    /*
>>>>>> +     * Register PCR to report POWERPC_EXCP_PRIV_REG instead of
>>>>>> +     * POWERPC_EXCP_INVAL_SPR.
>>>>>> +     */
>>>>>> +    spr_register(env, SPR_PCR, "PCR",
>>>>>> +                 SPR_NOACCESS, SPR_NOACCESS,
>>>>>> +                 SPR_NOACCESS, SPR_NOACCESS,
>>>>>> +                 0x00000000);
>>>>>>     }
>>>>>>       POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>>>>>> @@ -8880,6 +8889,32 @@ static void ppc_cpu_unrealizefn(DeviceState *dev,
>>>>>> Error **errp)
>>>>>>         }
>>>>>>     }
>>>>>>     +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
>>>>>> +{
>>>>>> +    CPUPPCState *env = &cpu->env;
>>>>>> +
>>>>>> +    cpu->cpu_version = cpu_version;
>>>>>> +
>>>>>> +    /*
>>>>>> +     * Calculate PCR value from virtual PVR.
>>>>>> +     * TODO: support actual compatibility in TCG.
>>>>>> +     */
>>>>>> +    switch (cpu_version) {
>>>>>> +    case CPU_POWERPC_LOGICAL_2_05:
>>>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_05;
>>>>>> +        break;
>>>>>> +    case CPU_POWERPC_LOGICAL_2_06:
>>>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>>>>>> +        break;
>>>>>> +    case CPU_POWERPC_LOGICAL_2_06_PLUS:
>>>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>>>>>> +        break;
>>>>> Don't we have to disable all those fancy p8 features too?
>>>>>
>>>>> #define   PCR_VEC_DIS   (1ul << (63-0)) /* Vec. disable (bit NA since
>>>>> POWER8) */
>>>>> #define   PCR_VSX_DIS   (1ul << (63-1)) /* VSX disable (bit NA since
>>>>> POWER8) */
>>>>> #define   PCR_TM_DIS    (1ul << (63-2)) /* Trans. memory disable
>>>>> (POWER8) */
>>>> Yep, makes sense to set "TM" for POWER < 8 and VSX+VEC for POWER < 7.
>>>>
>>>>> In fact, we probably want those bits always set when compat < power8.
>>>> Why? VSX is present on power7. VEC is defined in 2.06 too.
>>> True. So how does pre-p8 treat those p8 and above bits?
>> PowerISA 2.07 says if 2.06 mode is selected, the TM bit does not matter
>> anymore - TM will be disabled because 2.06 does not define it at all. The
>> same is true for VSX and 2.05 mode. So just setting a mode must be ok.
> 
> And for POWER8-on-POWER9 (or whatever it will be called) we can just set
> PCR to the respective feature disable bits. I think that should work.

I lost you here. I suspect it will also have "2.07" compatibility mode and
we will use that instead of disabling feature one by one.



-- 
Alexey

Re: [Qemu-devel] [PATCH 4/9] target-ppc: Implement "compat" CPU option

[Alexander Graf]

On 21.05.14 09:59, Alexey Kardashevskiy wrote:
> On 05/21/2014 05:29 PM, Alexander Graf wrote:
>> On 21.05.14 08:57, Alexey Kardashevskiy wrote:
>>> On 05/17/2014 06:47 AM, Alexander Graf wrote:
>>>> On 16.05.14 17:17, Alexey Kardashevskiy wrote:
>>>>> On 05/17/2014 12:05 AM, Alexander Graf wrote:
>>>>>> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>>>>>>> This adds basic support for the "compat" CPU option. By specifying
>>>>>>> the compat property, the user can manually switch guest CPU mode from
>>>>>>> "raw" to "architected".
>>>>>>>
>>>>>>> Since the actual compatibility mode is not implemented yet, this does
>>>>>>> not change the existing behavior.
>>>>>>>
>>>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>>>> ---
>>>>>>>      hw/ppc/spapr.c              | 15 ++++++++++++++-
>>>>>>>      target-ppc/cpu-models.h     |  6 ++++++
>>>>>>>      target-ppc/cpu-qom.h        |  2 ++
>>>>>>>      target-ppc/cpu.h            |  3 +++
>>>>>>>      target-ppc/translate_init.c | 35 +++++++++++++++++++++++++++++++++++
>>>>>>>      5 files changed, 60 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>>>>> index 0f8bd95..61d0675 100644
>>>>>>> --- a/hw/ppc/spapr.c
>>>>>>> +++ b/hw/ppc/spapr.c
>>>>>>> @@ -212,7 +212,8 @@ static int spapr_fixup_cpu_dt(void *fdt,
>>>>>>> sPAPREnvironment *spapr)
>>>>>>>            CPU_FOREACH(cpu) {
>>>>>>>              DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>>>>>>> -        int index = ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>>>>>>> +        PowerPCCPU *pcpu = POWERPC_CPU(cpu);
>>>>>>> +        int index = ppc_get_vcpu_dt_id(pcpu);
>>>>>>>              uint32_t associativity[] = {cpu_to_be32(0x5),
>>>>>>>                                          cpu_to_be32(0x0),
>>>>>>>                                          cpu_to_be32(0x0),
>>>>>>> @@ -249,6 +250,14 @@ static int spapr_fixup_cpu_dt(void *fdt,
>>>>>>> sPAPREnvironment *spapr)
>>>>>>>                  return ret;
>>>>>>>              }
>>>>>>>      +        if (pcpu->cpu_version) {
>>>>>>> +            ret = fdt_setprop(fdt, offset, "cpu-version",
>>>>>>> +                              &pcpu->cpu_version,
>>>>>>> sizeof(pcpu->cpu_version));
>>>>>>> +            if (ret < 0) {
>>>>>>> +                return ret;
>>>>>>> +            }
>>>>>>> +        }
>>>>>>> +
>>>>>>>              /* Build interrupt servers and gservers properties */
>>>>>>>              for (i = 0; i < smp_threads; i++) {
>>>>>>>                  servers_prop[i] = cpu_to_be32(index + i);
>>>>>>> @@ -1278,6 +1287,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs
>>>>>>> *args)
>>>>>>>                  kvmppc_set_papr(cpu);
>>>>>>>              }
>>>>>>>      +        if (cpu->max_compat) {
>>>>>>> +            ppc_set_compat(cpu, cpu->max_compat);
>>>>>>> +        }
>>>>>>> +
>>>>>>>              xics_cpu_setup(spapr->icp, cpu);
>>>>>>>                qemu_register_reset(spapr_cpu_reset, cpu);
>>>>>>> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
>>>>>>> index 33d2c51..45c0028 100644
>>>>>>> --- a/target-ppc/cpu-models.h
>>>>>>> +++ b/target-ppc/cpu-models.h
>>>>>>> @@ -753,4 +753,10 @@ enum {
>>>>>>>          POWERPC_SVR_8641D              = 0x80900121,
>>>>>>>      };
>>>>>>>      +/* Processor Compatibility mask (PCR) */
>>>>>>> +enum {
>>>>>>> +    POWERPC_ISA_COMPAT_2_05 = 0x02,
>>>>>>> +    POWERPC_ISA_COMPAT_2_06 = 0x04,
>>>>>>> +};
>>>>>>> +
>>>>>>>      #endif
>>>>>>> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
>>>>>>> index 2a8515b..dfd1419 100644
>>>>>>> --- a/target-ppc/cpu-qom.h
>>>>>>> +++ b/target-ppc/cpu-qom.h
>>>>>>> @@ -83,6 +83,7 @@ typedef struct PowerPCCPUClass {
>>>>>>>       * @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
>>>>>>> + * @cpu_version: Current logical PVR, zero if in "raw" mode
>>>>>>>       *
>>>>>>>       * A PowerPC CPU.
>>>>>>>       */
>>>>>>> @@ -94,6 +95,7 @@ struct PowerPCCPU {
>>>>>>>          CPUPPCState env;
>>>>>>>          int cpu_dt_id;
>>>>>>>          uint32_t max_compat;
>>>>>>> +    uint32_t cpu_version;
>>>>>>>      };
>>>>>>>        static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
>>>>>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>>>>>> index d498340..f61675a 100644
>>>>>>> --- a/target-ppc/cpu.h
>>>>>>> +++ b/target-ppc/cpu.h
>>>>>>> @@ -1123,6 +1123,8 @@ void ppc_store_msr (CPUPPCState *env, target_ulong
>>>>>>> value);
>>>>>>>        void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
>>>>>>>      +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>>>>>>> +
>>>>>>>      /* Time-base and decrementer management */
>>>>>>>      #ifndef NO_CPU_IO_DEFS
>>>>>>>      uint64_t cpu_ppc_load_tbl (CPUPPCState *env);
>>>>>>> @@ -1338,6 +1340,7 @@ static inline int cpu_mmu_index (CPUPPCState *env)
>>>>>>>      #define SPR_LPCR              (0x13E)
>>>>>>>      #define SPR_BOOKE_DVC2        (0x13F)
>>>>>>>      #define SPR_BOOKE_TSR         (0x150)
>>>>>>> +#define SPR_PCR               (0x152)
>>>>>>>      #define SPR_BOOKE_TCR         (0x154)
>>>>>>>      #define SPR_BOOKE_TLB0PS      (0x158)
>>>>>>>      #define SPR_BOOKE_TLB1PS      (0x159)
>>>>>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>>>>>> index 6f61b34..c4bd5de 100644
>>>>>>> --- a/target-ppc/translate_init.c
>>>>>>> +++ b/target-ppc/translate_init.c
>>>>>>> @@ -7803,6 +7803,15 @@ static void init_proc_POWER7 (CPUPPCState *env)
>>>>>>>          /* Can't find information on what this should be on reset.  This
>>>>>>>           * value is the one used by 74xx processors. */
>>>>>>>          vscr_init(env, 0x00010000);
>>>>>>> +
>>>>>>> +    /*
>>>>>>> +     * Register PCR to report POWERPC_EXCP_PRIV_REG instead of
>>>>>>> +     * POWERPC_EXCP_INVAL_SPR.
>>>>>>> +     */
>>>>>>> +    spr_register(env, SPR_PCR, "PCR",
>>>>>>> +                 SPR_NOACCESS, SPR_NOACCESS,
>>>>>>> +                 SPR_NOACCESS, SPR_NOACCESS,
>>>>>>> +                 0x00000000);
>>>>>>>      }
>>>>>>>        POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>>>>>>> @@ -8880,6 +8889,32 @@ static void ppc_cpu_unrealizefn(DeviceState *dev,
>>>>>>> Error **errp)
>>>>>>>          }
>>>>>>>      }
>>>>>>>      +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
>>>>>>> +{
>>>>>>> +    CPUPPCState *env = &cpu->env;
>>>>>>> +
>>>>>>> +    cpu->cpu_version = cpu_version;
>>>>>>> +
>>>>>>> +    /*
>>>>>>> +     * Calculate PCR value from virtual PVR.
>>>>>>> +     * TODO: support actual compatibility in TCG.
>>>>>>> +     */
>>>>>>> +    switch (cpu_version) {
>>>>>>> +    case CPU_POWERPC_LOGICAL_2_05:
>>>>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_05;
>>>>>>> +        break;
>>>>>>> +    case CPU_POWERPC_LOGICAL_2_06:
>>>>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>>>>>>> +        break;
>>>>>>> +    case CPU_POWERPC_LOGICAL_2_06_PLUS:
>>>>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>>>>>>> +        break;
>>>>>> Don't we have to disable all those fancy p8 features too?
>>>>>>
>>>>>> #define   PCR_VEC_DIS   (1ul << (63-0)) /* Vec. disable (bit NA since
>>>>>> POWER8) */
>>>>>> #define   PCR_VSX_DIS   (1ul << (63-1)) /* VSX disable (bit NA since
>>>>>> POWER8) */
>>>>>> #define   PCR_TM_DIS    (1ul << (63-2)) /* Trans. memory disable
>>>>>> (POWER8) */
>>>>> Yep, makes sense to set "TM" for POWER < 8 and VSX+VEC for POWER < 7.
>>>>>
>>>>>> In fact, we probably want those bits always set when compat < power8.
>>>>> Why? VSX is present on power7. VEC is defined in 2.06 too.
>>>> True. So how does pre-p8 treat those p8 and above bits?
>>> PowerISA 2.07 says if 2.06 mode is selected, the TM bit does not matter
>>> anymore - TM will be disabled because 2.06 does not define it at all. The
>>> same is true for VSX and 2.05 mode. So just setting a mode must be ok.
>> And for POWER8-on-POWER9 (or whatever it will be called) we can just set
>> PCR to the respective feature disable bits. I think that should work.
> I lost you here. I suspect it will also have "2.07" compatibility mode and
> we will use that instead of disabling feature one by one.

Considering that the architects added by-feature bits, I would assume 
they won't introduce a 2.07 bit but rather make every new feature 
disableable one by one. But maybe I'm wrong - the point is that either 
way will work and is future proof ;).


Alex

Re: [Qemu-devel] [PATCH 4/9] target-ppc: Implement "compat" CPU option

[Alexey Kardashevskiy]

On 05/21/2014 06:00 PM, Alexander Graf wrote:
> 
> On 21.05.14 09:59, Alexey Kardashevskiy wrote:
>> On 05/21/2014 05:29 PM, Alexander Graf wrote:
>>> On 21.05.14 08:57, Alexey Kardashevskiy wrote:
>>>> On 05/17/2014 06:47 AM, Alexander Graf wrote:
>>>>> On 16.05.14 17:17, Alexey Kardashevskiy wrote:
>>>>>> On 05/17/2014 12:05 AM, Alexander Graf wrote:
>>>>>>> On 15.05.14 13:28, Alexey Kardashevskiy wrote:
>>>>>>>> This adds basic support for the "compat" CPU option. By specifying
>>>>>>>> the compat property, the user can manually switch guest CPU mode from
>>>>>>>> "raw" to "architected".
>>>>>>>>
>>>>>>>> Since the actual compatibility mode is not implemented yet, this does
>>>>>>>> not change the existing behavior.
>>>>>>>>
>>>>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>>>>>> ---
>>>>>>>>      hw/ppc/spapr.c              | 15 ++++++++++++++-
>>>>>>>>      target-ppc/cpu-models.h     |  6 ++++++
>>>>>>>>      target-ppc/cpu-qom.h        |  2 ++
>>>>>>>>      target-ppc/cpu.h            |  3 +++
>>>>>>>>      target-ppc/translate_init.c | 35
>>>>>>>> +++++++++++++++++++++++++++++++++++
>>>>>>>>      5 files changed, 60 insertions(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>>>>>>>> index 0f8bd95..61d0675 100644
>>>>>>>> --- a/hw/ppc/spapr.c
>>>>>>>> +++ b/hw/ppc/spapr.c
>>>>>>>> @@ -212,7 +212,8 @@ static int spapr_fixup_cpu_dt(void *fdt,
>>>>>>>> sPAPREnvironment *spapr)
>>>>>>>>            CPU_FOREACH(cpu) {
>>>>>>>>              DeviceClass *dc = DEVICE_GET_CLASS(cpu);
>>>>>>>> -        int index = ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>>>>>>>> +        PowerPCCPU *pcpu = POWERPC_CPU(cpu);
>>>>>>>> +        int index = ppc_get_vcpu_dt_id(pcpu);
>>>>>>>>              uint32_t associativity[] = {cpu_to_be32(0x5),
>>>>>>>>                                          cpu_to_be32(0x0),
>>>>>>>>                                          cpu_to_be32(0x0),
>>>>>>>> @@ -249,6 +250,14 @@ static int spapr_fixup_cpu_dt(void *fdt,
>>>>>>>> sPAPREnvironment *spapr)
>>>>>>>>                  return ret;
>>>>>>>>              }
>>>>>>>>      +        if (pcpu->cpu_version) {
>>>>>>>> +            ret = fdt_setprop(fdt, offset, "cpu-version",
>>>>>>>> +                              &pcpu->cpu_version,
>>>>>>>> sizeof(pcpu->cpu_version));
>>>>>>>> +            if (ret < 0) {
>>>>>>>> +                return ret;
>>>>>>>> +            }
>>>>>>>> +        }
>>>>>>>> +
>>>>>>>>              /* Build interrupt servers and gservers properties */
>>>>>>>>              for (i = 0; i < smp_threads; i++) {
>>>>>>>>                  servers_prop[i] = cpu_to_be32(index + i);
>>>>>>>> @@ -1278,6 +1287,10 @@ static void ppc_spapr_init(QEMUMachineInitArgs
>>>>>>>> *args)
>>>>>>>>                  kvmppc_set_papr(cpu);
>>>>>>>>              }
>>>>>>>>      +        if (cpu->max_compat) {
>>>>>>>> +            ppc_set_compat(cpu, cpu->max_compat);
>>>>>>>> +        }
>>>>>>>> +
>>>>>>>>              xics_cpu_setup(spapr->icp, cpu);
>>>>>>>>                qemu_register_reset(spapr_cpu_reset, cpu);
>>>>>>>> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
>>>>>>>> index 33d2c51..45c0028 100644
>>>>>>>> --- a/target-ppc/cpu-models.h
>>>>>>>> +++ b/target-ppc/cpu-models.h
>>>>>>>> @@ -753,4 +753,10 @@ enum {
>>>>>>>>          POWERPC_SVR_8641D              = 0x80900121,
>>>>>>>>      };
>>>>>>>>      +/* Processor Compatibility mask (PCR) */
>>>>>>>> +enum {
>>>>>>>> +    POWERPC_ISA_COMPAT_2_05 = 0x02,
>>>>>>>> +    POWERPC_ISA_COMPAT_2_06 = 0x04,
>>>>>>>> +};
>>>>>>>> +
>>>>>>>>      #endif
>>>>>>>> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
>>>>>>>> index 2a8515b..dfd1419 100644
>>>>>>>> --- a/target-ppc/cpu-qom.h
>>>>>>>> +++ b/target-ppc/cpu-qom.h
>>>>>>>> @@ -83,6 +83,7 @@ typedef struct PowerPCCPUClass {
>>>>>>>>       * @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
>>>>>>>> + * @cpu_version: Current logical PVR, zero if in "raw" mode
>>>>>>>>       *
>>>>>>>>       * A PowerPC CPU.
>>>>>>>>       */
>>>>>>>> @@ -94,6 +95,7 @@ struct PowerPCCPU {
>>>>>>>>          CPUPPCState env;
>>>>>>>>          int cpu_dt_id;
>>>>>>>>          uint32_t max_compat;
>>>>>>>> +    uint32_t cpu_version;
>>>>>>>>      };
>>>>>>>>        static inline PowerPCCPU *ppc_env_get_cpu(CPUPPCState *env)
>>>>>>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>>>>>>> index d498340..f61675a 100644
>>>>>>>> --- a/target-ppc/cpu.h
>>>>>>>> +++ b/target-ppc/cpu.h
>>>>>>>> @@ -1123,6 +1123,8 @@ void ppc_store_msr (CPUPPCState *env,
>>>>>>>> target_ulong
>>>>>>>> value);
>>>>>>>>        void ppc_cpu_list (FILE *f, fprintf_function cpu_fprintf);
>>>>>>>>      +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version);
>>>>>>>> +
>>>>>>>>      /* Time-base and decrementer management */
>>>>>>>>      #ifndef NO_CPU_IO_DEFS
>>>>>>>>      uint64_t cpu_ppc_load_tbl (CPUPPCState *env);
>>>>>>>> @@ -1338,6 +1340,7 @@ static inline int cpu_mmu_index (CPUPPCState
>>>>>>>> *env)
>>>>>>>>      #define SPR_LPCR              (0x13E)
>>>>>>>>      #define SPR_BOOKE_DVC2        (0x13F)
>>>>>>>>      #define SPR_BOOKE_TSR         (0x150)
>>>>>>>> +#define SPR_PCR               (0x152)
>>>>>>>>      #define SPR_BOOKE_TCR         (0x154)
>>>>>>>>      #define SPR_BOOKE_TLB0PS      (0x158)
>>>>>>>>      #define SPR_BOOKE_TLB1PS      (0x159)
>>>>>>>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>>>>>>>> index 6f61b34..c4bd5de 100644
>>>>>>>> --- a/target-ppc/translate_init.c
>>>>>>>> +++ b/target-ppc/translate_init.c
>>>>>>>> @@ -7803,6 +7803,15 @@ static void init_proc_POWER7 (CPUPPCState *env)
>>>>>>>>          /* Can't find information on what this should be on
>>>>>>>> reset.  This
>>>>>>>>           * value is the one used by 74xx processors. */
>>>>>>>>          vscr_init(env, 0x00010000);
>>>>>>>> +
>>>>>>>> +    /*
>>>>>>>> +     * Register PCR to report POWERPC_EXCP_PRIV_REG instead of
>>>>>>>> +     * POWERPC_EXCP_INVAL_SPR.
>>>>>>>> +     */
>>>>>>>> +    spr_register(env, SPR_PCR, "PCR",
>>>>>>>> +                 SPR_NOACCESS, SPR_NOACCESS,
>>>>>>>> +                 SPR_NOACCESS, SPR_NOACCESS,
>>>>>>>> +                 0x00000000);
>>>>>>>>      }
>>>>>>>>        POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>>>>>>>> @@ -8880,6 +8889,32 @@ static void ppc_cpu_unrealizefn(DeviceState
>>>>>>>> *dev,
>>>>>>>> Error **errp)
>>>>>>>>          }
>>>>>>>>      }
>>>>>>>>      +void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version)
>>>>>>>> +{
>>>>>>>> +    CPUPPCState *env = &cpu->env;
>>>>>>>> +
>>>>>>>> +    cpu->cpu_version = cpu_version;
>>>>>>>> +
>>>>>>>> +    /*
>>>>>>>> +     * Calculate PCR value from virtual PVR.
>>>>>>>> +     * TODO: support actual compatibility in TCG.
>>>>>>>> +     */
>>>>>>>> +    switch (cpu_version) {
>>>>>>>> +    case CPU_POWERPC_LOGICAL_2_05:
>>>>>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_05;
>>>>>>>> +        break;
>>>>>>>> +    case CPU_POWERPC_LOGICAL_2_06:
>>>>>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>>>>>>>> +        break;
>>>>>>>> +    case CPU_POWERPC_LOGICAL_2_06_PLUS:
>>>>>>>> +        env->spr[SPR_PCR] = POWERPC_ISA_COMPAT_2_06;
>>>>>>>> +        break;
>>>>>>> Don't we have to disable all those fancy p8 features too?
>>>>>>>
>>>>>>> #define   PCR_VEC_DIS   (1ul << (63-0)) /* Vec. disable (bit NA since
>>>>>>> POWER8) */
>>>>>>> #define   PCR_VSX_DIS   (1ul << (63-1)) /* VSX disable (bit NA since
>>>>>>> POWER8) */
>>>>>>> #define   PCR_TM_DIS    (1ul << (63-2)) /* Trans. memory disable
>>>>>>> (POWER8) */
>>>>>> Yep, makes sense to set "TM" for POWER < 8 and VSX+VEC for POWER < 7.
>>>>>>
>>>>>>> In fact, we probably want those bits always set when compat < power8.
>>>>>> Why? VSX is present on power7. VEC is defined in 2.06 too.
>>>>> True. So how does pre-p8 treat those p8 and above bits?
>>>> PowerISA 2.07 says if 2.06 mode is selected, the TM bit does not matter
>>>> anymore - TM will be disabled because 2.06 does not define it at all. The
>>>> same is true for VSX and 2.05 mode. So just setting a mode must be ok.
>>> And for POWER8-on-POWER9 (or whatever it will be called) we can just set
>>> PCR to the respective feature disable bits. I think that should work.
>> I lost you here. I suspect it will also have "2.07" compatibility mode and
>> we will use that instead of disabling feature one by one.
> 
> Considering that the architects added by-feature bits, I would assume they
> won't introduce a 2.07 bit but rather make every new feature disableable
> one by one. 

2.05 defines PCR_VEC_DIS already and since then 2 specs were released so it
is unlikely to happen :)


> But maybe I'm wrong - the point is that either way will work
> and is future proof ;).



-- 
Alexey