[Qemu-devel] [PATCH 0/5] ppc: Improve sPAPR CPU compatibility mode settings

[Qemu-devel] [PATCH 0/5] ppc: Improve sPAPR CPU compatibility mode settings

[Thomas Huth]

If a guest currently only requests PowerISA 2.07 compatiblity mode with
the "ibm,client-architecture-support" firmware call, but does not
specify a matching real PVR for the host CPU on a POWER8 host,
it ends up in POWER7 / PowerISA 2.06 compatibility mode since QEMU
does not support 2.07 compatibility mode yet. This currently happens
when running a Linux guest on a POWER8NVL host, since Linux guests
do not use the PVR for these CPUs for the "ibm,client-architecture-
support" call yet (but I submitted a patch for the kernel to fix this
issue there last week, so the support should soon be there, too).

Anyway, QEMU should also support a proper 2.07 compatibility mode
if the host CPU can do it. So this patch series introduces such a
mode and does some clean-ups and other fixes along the way (e.g.
it splits the ambiguous pcr_mask setting into two variables, one
for defining the valid bits in the PCR register, and one for
storing the valid ISA levels).

Thomas Huth (5):
  ppc/spapr: Refactor h_client_architecture_support() CPU parsing code
  ppc: Split pcr_mask settings into supported bits and the register mask
  ppc: Provide function to get CPU class of the host CPU
  ppc: Improve PCR bit selection in ppc_set_compat()
  ppc: Add PowerISA 2.07 compatibility mode

 hw/ppc/spapr_hcall.c        | 63 +++++++++++++++++++++++++++------------------
 target-ppc/cpu-qom.h        |  3 ++-
 target-ppc/cpu.h            |  1 +
 target-ppc/kvm.c            | 19 ++++++++++----
 target-ppc/kvm_ppc.h        |  7 +++++
 target-ppc/translate_init.c | 22 +++++++++++-----
 6 files changed, 78 insertions(+), 37 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/5] ppc/spapr: Refactor h_client_architecture_support() CPU parsing code

[Thomas Huth]

The h_client_architecture_support() function has become quite big
and nested already. So factor out the code that takes care of the
sPAPR compatibility PVRs (which will be modified by the following
patches).

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/spapr_hcall.c | 61 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 36 insertions(+), 25 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 9a3f4ec..bb8f4de 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -922,6 +922,39 @@ static void do_set_compat(void *arg)
     ((cpuver) == CPU_POWERPC_LOGICAL_2_06_PLUS) ? 2061 : \
     ((cpuver) == CPU_POWERPC_LOGICAL_2_07) ? 2070 : 0)
 
+static void cas_handle_compat_cpu(PowerPCCPUClass *pcc, uint32_t pvr,
+                                  unsigned max_lvl, unsigned *compat_lvl,
+                                  unsigned *cpu_version)
+{
+    unsigned lvl = get_compat_level(pvr);
+    bool is205, is206;
+
+    if (!lvl) {
+        return;
+    }
+
+    /* If it is a logical PVR, try to determine the highest level */
+    is205 = (pcc->pcr_mask & PCR_COMPAT_2_05) &&
+            (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05));
+    is206 = (pcc->pcr_mask & PCR_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;
+        }
+    }
+}
+
 #define OV5_DRCONF_MEMORY 0x20
 
 static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
@@ -931,7 +964,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
 {
     target_ulong list = ppc64_phys_to_real(args[0]);
     target_ulong ov_table, ov5;
-    PowerPCCPUClass *pcc_ = POWERPC_CPU_GET_CLASS(cpu_);
+    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu_);
     CPUState *cs;
     bool cpu_match = false, cpu_update = true, memory_update = false;
     unsigned old_cpu_version = cpu_->cpu_version;
@@ -958,29 +991,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
             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 & PCR_COMPAT_2_05) &&
-                     (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05));
-                bool is206 = (pcc_->pcr_mask & PCR_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;
-                    }
-                }
-            }
+            cas_handle_compat_cpu(pcc, pvr, max_lvl, &compat_lvl, &cpu_version);
         }
         /* Terminator record */
         if (~pvr_mask & pvr) {
@@ -990,7 +1001,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
 
     /* Parsing finished */
     trace_spapr_cas_pvr(cpu_->cpu_version, cpu_match,
-                        cpu_version, pcc_->pcr_mask);
+                        cpu_version, pcc->pcr_mask);
 
     /* Update CPUs */
     if (old_cpu_version != cpu_version) {
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/5] ppc: Split pcr_mask settings into supported bits and the register mask

[Thomas Huth]

The current pcr_mask values are ambiguous: Should these be the mask
that defines valid bits in the PCR register? Or should these rather
indicate which compatibility levels are possible? Anyway, POWER6 and
POWER7 should certainly not use the same values here. So let's
introduce an additional variable "pcr_supported" here which is
used to indicate the valid compatibility levels, and use pcr_mask
to signal the valid bits in the PCR register.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/spapr_hcall.c        | 4 ++--
 target-ppc/cpu-qom.h        | 3 ++-
 target-ppc/cpu.h            | 1 +
 target-ppc/translate_init.c | 6 ++++--
 4 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index bb8f4de..cc16249 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -934,9 +934,9 @@ static void cas_handle_compat_cpu(PowerPCCPUClass *pcc, uint32_t pvr,
     }
 
     /* If it is a logical PVR, try to determine the highest level */
-    is205 = (pcc->pcr_mask & PCR_COMPAT_2_05) &&
+    is205 = (pcc->pcr_supported & PCR_COMPAT_2_05) &&
             (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05));
-    is206 = (pcc->pcr_mask & PCR_COMPAT_2_06) &&
+    is206 = (pcc->pcr_supported & PCR_COMPAT_2_06) &&
             ((lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06)) ||
              (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06_PLUS)));
 
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index 07358aa..969ecdf 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -165,7 +165,8 @@ typedef struct PowerPCCPUClass {
 
     uint32_t pvr;
     bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr);
-    uint64_t pcr_mask;
+    uint64_t pcr_mask;          /* Available bits in PCR register */
+    uint64_t pcr_supported;     /* Bits for supported PowerISA versions */
     uint32_t svr;
     uint64_t insns_flags;
     uint64_t insns_flags2;
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index c2962d7..c00a3b5 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -2202,6 +2202,7 @@ enum {
 enum {
     PCR_COMPAT_2_05     = 1ull << (63-62),
     PCR_COMPAT_2_06     = 1ull << (63-61),
+    PCR_COMPAT_2_07     = 1ull << (63-60),
     PCR_VEC_DIS         = 1ull << (63-0), /* Vec. disable (bit NA since POWER8) */
     PCR_VSX_DIS         = 1ull << (63-1), /* VSX disable (bit NA since POWER8) */
     PCR_TM_DIS          = 1ull << (63-2), /* Trans. memory disable (POWER8) */
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index a1db500..fa09183 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8365,7 +8365,8 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
     dc->desc = "POWER7";
     dc->props = powerpc_servercpu_properties;
     pcc->pvr_match = ppc_pvr_match_power7;
-    pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
+    pcc->pcr_mask = PCR_VEC_DIS | PCR_VSX_DIS | PCR_COMPAT_2_05;
+    pcc->pcr_supported = PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
     pcc->init_proc = init_proc_POWER7;
     pcc->check_pow = check_pow_nocheck;
     pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
@@ -8445,7 +8446,8 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
     dc->desc = "POWER8";
     dc->props = powerpc_servercpu_properties;
     pcc->pvr_match = ppc_pvr_match_power8;
-    pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
+    pcc->pcr_mask = PCR_TM_DIS | PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
+    pcc->pcr_supported = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
     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.8.3.1

[Qemu-devel] [PATCH 3/5] ppc: Provide function to get CPU class of the host CPU

[Thomas Huth]

When running with KVM, we might be interested in some details
of the host CPU class, too, so provide a function to get the
corresponding CPU class.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target-ppc/kvm.c     | 19 ++++++++++++++-----
 target-ppc/kvm_ppc.h |  7 +++++++
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 24d6032..6c15361 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -2329,6 +2329,19 @@ static PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
     return POWERPC_CPU_CLASS(oc);
 }
 
+PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
+{
+    uint32_t host_pvr = mfpvr();
+    PowerPCCPUClass *pvr_pcc;
+
+    pvr_pcc = ppc_cpu_class_by_pvr(host_pvr);
+    if (pvr_pcc == NULL) {
+        pvr_pcc = ppc_cpu_class_by_pvr_mask(host_pvr);
+    }
+
+    return pvr_pcc;
+}
+
 static int kvm_ppc_register_host_cpu_type(void)
 {
     TypeInfo type_info = {
@@ -2336,14 +2349,10 @@ static int kvm_ppc_register_host_cpu_type(void)
         .instance_init = kvmppc_host_cpu_initfn,
         .class_init = kvmppc_host_cpu_class_init,
     };
-    uint32_t host_pvr = mfpvr();
     PowerPCCPUClass *pvr_pcc;
     DeviceClass *dc;
 
-    pvr_pcc = ppc_cpu_class_by_pvr(host_pvr);
-    if (pvr_pcc == NULL) {
-        pvr_pcc = ppc_cpu_class_by_pvr_mask(host_pvr);
-    }
+    pvr_pcc = kvm_ppc_get_host_cpu_class();
     if (pvr_pcc == NULL) {
         return -1;
     }
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 3b2090e..20bfb59 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -56,6 +56,7 @@ void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
 bool kvmppc_has_cap_fixup_hcalls(void);
 int kvmppc_enable_hwrng(void);
 int kvmppc_put_books_sregs(PowerPCCPU *cpu);
+PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
 
 #else
 
@@ -252,6 +253,12 @@ static inline int kvmppc_put_books_sregs(PowerPCCPU *cpu)
 {
     abort();
 }
+
+static inline PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
+{
+    return NULL;
+}
+
 #endif
 
 #ifndef CONFIG_KVM
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/5] ppc: Improve PCR bit selection in ppc_set_compat()

[Thomas Huth]

When using an olderr PowerISA level, all the upper compatibility
bits have to be enabled, too. For example when we want to run
something in PowerISA 2.05 compatibility mode on POWER8, the bit
for 2.06 has to be set beside the bit for 2.05.
Additionally, to make sure that we do not set bits that are not
supported by the host, we apply a mask with the known-to-be-good
bits here, too.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target-ppc/translate_init.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index fa09183..ee2bc14 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9519,24 +9519,29 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **errp)
 {
     int ret = 0;
     CPUPPCState *env = &cpu->env;
+    PowerPCCPUClass *host_pcc;
 
     cpu->cpu_version = cpu_version;
 
     switch (cpu_version) {
     case CPU_POWERPC_LOGICAL_2_05:
-        env->spr[SPR_PCR] = PCR_COMPAT_2_05;
+        env->spr[SPR_PCR] = PCR_TM_DIS | PCR_VSX_DIS | PCR_COMPAT_2_07 |
+                            PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
         break;
     case CPU_POWERPC_LOGICAL_2_06:
-        env->spr[SPR_PCR] = PCR_COMPAT_2_06;
-        break;
     case CPU_POWERPC_LOGICAL_2_06_PLUS:
-        env->spr[SPR_PCR] = PCR_COMPAT_2_06;
+        env->spr[SPR_PCR] = PCR_TM_DIS | PCR_COMPAT_2_07 | PCR_COMPAT_2_06;
         break;
     default:
         env->spr[SPR_PCR] = 0;
         break;
     }
 
+    host_pcc = kvm_ppc_get_host_cpu_class();
+    if (host_pcc) {
+        env->spr[SPR_PCR] &= host_pcc->pcr_mask;
+    }
+
     if (kvm_enabled()) {
         ret = kvmppc_set_compat(cpu, cpu->cpu_version);
         if (ret < 0) {
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/5] ppc: Add PowerISA 2.07 compatibility mode

[Thomas Huth]

Make sure that guests can use the PowerISA 2.07 CPU sPAPR
compatibility mode when they request it and the target CPU
supports it.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/ppc/spapr_hcall.c        | 6 ++++--
 target-ppc/translate_init.c | 3 +++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index cc16249..2ba5cbd 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -927,7 +927,7 @@ static void cas_handle_compat_cpu(PowerPCCPUClass *pcc, uint32_t pvr,
                                   unsigned *cpu_version)
 {
     unsigned lvl = get_compat_level(pvr);
-    bool is205, is206;
+    bool is205, is206, is207;
 
     if (!lvl) {
         return;
@@ -939,8 +939,10 @@ static void cas_handle_compat_cpu(PowerPCCPUClass *pcc, uint32_t pvr,
     is206 = (pcc->pcr_supported & PCR_COMPAT_2_06) &&
             ((lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06)) ||
              (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06_PLUS)));
+    is207 = (pcc->pcr_supported & PCR_COMPAT_2_07) &&
+            (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_07));
 
-    if (is205 || is206) {
+    if (is205 || is206 || is207) {
         if (!max_lvl) {
             /* User did not set the level, choose the highest */
             if (*compat_lvl <= lvl) {
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index ee2bc14..46fa375 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -9532,6 +9532,9 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **errp)
     case CPU_POWERPC_LOGICAL_2_06_PLUS:
         env->spr[SPR_PCR] = PCR_TM_DIS | PCR_COMPAT_2_07 | PCR_COMPAT_2_06;
         break;
+    case CPU_POWERPC_LOGICAL_2_07:
+        env->spr[SPR_PCR] = PCR_COMPAT_2_07;
+        break;
     default:
         env->spr[SPR_PCR] = 0;
         break;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 1/5] ppc/spapr: Refactor h_client_architecture_support() CPU parsing code

[Michael Roth]

Quoting Thomas Huth (2016-06-07 10:39:36)
> The h_client_architecture_support() function has become quite big
> and nested already. So factor out the code that takes care of the
> sPAPR compatibility PVRs (which will be modified by the following
> patches).
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Restructuring looks sane.

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  hw/ppc/spapr_hcall.c | 61 +++++++++++++++++++++++++++++++---------------------
>  1 file changed, 36 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 9a3f4ec..bb8f4de 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -922,6 +922,39 @@ static void do_set_compat(void *arg)
>      ((cpuver) == CPU_POWERPC_LOGICAL_2_06_PLUS) ? 2061 : \
>      ((cpuver) == CPU_POWERPC_LOGICAL_2_07) ? 2070 : 0)
> 
> +static void cas_handle_compat_cpu(PowerPCCPUClass *pcc, uint32_t pvr,
> +                                  unsigned max_lvl, unsigned *compat_lvl,
> +                                  unsigned *cpu_version)
> +{
> +    unsigned lvl = get_compat_level(pvr);
> +    bool is205, is206;
> +
> +    if (!lvl) {
> +        return;
> +    }
> +
> +    /* If it is a logical PVR, try to determine the highest level */
> +    is205 = (pcc->pcr_mask & PCR_COMPAT_2_05) &&
> +            (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05));
> +    is206 = (pcc->pcr_mask & PCR_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;
> +        }
> +    }
> +}
> +
>  #define OV5_DRCONF_MEMORY 0x20
> 
>  static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
> @@ -931,7 +964,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
>  {
>      target_ulong list = ppc64_phys_to_real(args[0]);
>      target_ulong ov_table, ov5;
> -    PowerPCCPUClass *pcc_ = POWERPC_CPU_GET_CLASS(cpu_);
> +    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu_);
>      CPUState *cs;
>      bool cpu_match = false, cpu_update = true, memory_update = false;
>      unsigned old_cpu_version = cpu_->cpu_version;
> @@ -958,29 +991,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
>              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 & PCR_COMPAT_2_05) &&
> -                     (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05));
> -                bool is206 = (pcc_->pcr_mask & PCR_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;
> -                    }
> -                }
> -            }
> +            cas_handle_compat_cpu(pcc, pvr, max_lvl, &compat_lvl, &cpu_version);
>          }
>          /* Terminator record */
>          if (~pvr_mask & pvr) {
> @@ -990,7 +1001,7 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
> 
>      /* Parsing finished */
>      trace_spapr_cas_pvr(cpu_->cpu_version, cpu_match,
> -                        cpu_version, pcc_->pcr_mask);
> +                        cpu_version, pcc->pcr_mask);
> 
>      /* Update CPUs */
>      if (old_cpu_version != cpu_version) {
> -- 
> 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH 2/5] ppc: Split pcr_mask settings into supported bits and the register mask

[Michael Roth]

Quoting Thomas Huth (2016-06-07 10:39:37)
> The current pcr_mask values are ambiguous: Should these be the mask
> that defines valid bits in the PCR register? Or should these rather
> indicate which compatibility levels are possible? Anyway, POWER6 and
> POWER7 should certainly not use the same values here. So let's
> introduce an additional variable "pcr_supported" here which is
> used to indicate the valid compatibility levels, and use pcr_mask
> to signal the valid bits in the PCR register.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  hw/ppc/spapr_hcall.c        | 4 ++--
>  target-ppc/cpu-qom.h        | 3 ++-
>  target-ppc/cpu.h            | 1 +
>  target-ppc/translate_init.c | 6 ++++--
>  4 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index bb8f4de..cc16249 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -934,9 +934,9 @@ static void cas_handle_compat_cpu(PowerPCCPUClass *pcc, uint32_t pvr,
>      }
> 
>      /* If it is a logical PVR, try to determine the highest level */
> -    is205 = (pcc->pcr_mask & PCR_COMPAT_2_05) &&
> +    is205 = (pcc->pcr_supported & PCR_COMPAT_2_05) &&
>              (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05));
> -    is206 = (pcc->pcr_mask & PCR_COMPAT_2_06) &&
> +    is206 = (pcc->pcr_supported & PCR_COMPAT_2_06) &&
>              ((lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06)) ||
>               (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06_PLUS)));
> 
> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> index 07358aa..969ecdf 100644
> --- a/target-ppc/cpu-qom.h
> +++ b/target-ppc/cpu-qom.h
> @@ -165,7 +165,8 @@ typedef struct PowerPCCPUClass {
> 
>      uint32_t pvr;
>      bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr);
> -    uint64_t pcr_mask;
> +    uint64_t pcr_mask;          /* Available bits in PCR register */
> +    uint64_t pcr_supported;     /* Bits for supported PowerISA versions */
>      uint32_t svr;
>      uint64_t insns_flags;
>      uint64_t insns_flags2;
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index c2962d7..c00a3b5 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -2202,6 +2202,7 @@ enum {
>  enum {
>      PCR_COMPAT_2_05     = 1ull << (63-62),
>      PCR_COMPAT_2_06     = 1ull << (63-61),
> +    PCR_COMPAT_2_07     = 1ull << (63-60),

This gets introduced somewhat subtly here, maybe move it to patch 5?

>      PCR_VEC_DIS         = 1ull << (63-0), /* Vec. disable (bit NA since POWER8) */
>      PCR_VSX_DIS         = 1ull << (63-1), /* VSX disable (bit NA since POWER8) */
>      PCR_TM_DIS          = 1ull << (63-2), /* Trans. memory disable (POWER8) */
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index a1db500..fa09183 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -8365,7 +8365,8 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>      dc->desc = "POWER7";
>      dc->props = powerpc_servercpu_properties;
>      pcc->pvr_match = ppc_pvr_match_power7;
> -    pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
> +    pcc->pcr_mask = PCR_VEC_DIS | PCR_VSX_DIS | PCR_COMPAT_2_05;
> +    pcc->pcr_supported = PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
>      pcc->init_proc = init_proc_POWER7;
>      pcc->check_pow = check_pow_nocheck;
>      pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
> @@ -8445,7 +8446,8 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>      dc->desc = "POWER8";
>      dc->props = powerpc_servercpu_properties;
>      pcc->pvr_match = ppc_pvr_match_power8;
> -    pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
> +    pcc->pcr_mask = PCR_TM_DIS | PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
> +    pcc->pcr_supported = PCR_COMPAT_2_07 | PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
>      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.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH 3/5] ppc: Provide function to get CPU class of the host CPU

[Michael Roth]

Quoting Thomas Huth (2016-06-07 10:39:38)
> When running with KVM, we might be interested in some details
> of the host CPU class, too, so provide a function to get the
> corresponding CPU class.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  target-ppc/kvm.c     | 19 ++++++++++++++-----
>  target-ppc/kvm_ppc.h |  7 +++++++
>  2 files changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 24d6032..6c15361 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -2329,6 +2329,19 @@ static PowerPCCPUClass *ppc_cpu_get_family_class(PowerPCCPUClass *pcc)
>      return POWERPC_CPU_CLASS(oc);
>  }
> 
> +PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
> +{
> +    uint32_t host_pvr = mfpvr();
> +    PowerPCCPUClass *pvr_pcc;
> +
> +    pvr_pcc = ppc_cpu_class_by_pvr(host_pvr);
> +    if (pvr_pcc == NULL) {
> +        pvr_pcc = ppc_cpu_class_by_pvr_mask(host_pvr);
> +    }
> +
> +    return pvr_pcc;
> +}
> +
>  static int kvm_ppc_register_host_cpu_type(void)
>  {
>      TypeInfo type_info = {
> @@ -2336,14 +2349,10 @@ static int kvm_ppc_register_host_cpu_type(void)
>          .instance_init = kvmppc_host_cpu_initfn,
>          .class_init = kvmppc_host_cpu_class_init,
>      };
> -    uint32_t host_pvr = mfpvr();
>      PowerPCCPUClass *pvr_pcc;
>      DeviceClass *dc;
> 
> -    pvr_pcc = ppc_cpu_class_by_pvr(host_pvr);
> -    if (pvr_pcc == NULL) {
> -        pvr_pcc = ppc_cpu_class_by_pvr_mask(host_pvr);
> -    }
> +    pvr_pcc = kvm_ppc_get_host_cpu_class();
>      if (pvr_pcc == NULL) {
>          return -1;
>      }
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 3b2090e..20bfb59 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -56,6 +56,7 @@ void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
>  bool kvmppc_has_cap_fixup_hcalls(void);
>  int kvmppc_enable_hwrng(void);
>  int kvmppc_put_books_sregs(PowerPCCPU *cpu);
> +PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
> 
>  #else
> 
> @@ -252,6 +253,12 @@ static inline int kvmppc_put_books_sregs(PowerPCCPU *cpu)
>  {
>      abort();
>  }
> +
> +static inline PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
> +{
> +    return NULL;
> +}
> +
>  #endif
> 
>  #ifndef CONFIG_KVM
> -- 
> 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH 0/5] ppc: Improve sPAPR CPU compatibility mode settings

[David Gibson]

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

On Tue, Jun 07, 2016 at 05:39:35PM +0200, Thomas Huth wrote:
> If a guest currently only requests PowerISA 2.07 compatiblity mode with
> the "ibm,client-architecture-support" firmware call, but does not
> specify a matching real PVR for the host CPU on a POWER8 host,
> it ends up in POWER7 / PowerISA 2.06 compatibility mode since QEMU
> does not support 2.07 compatibility mode yet. This currently happens
> when running a Linux guest on a POWER8NVL host, since Linux guests
> do not use the PVR for these CPUs for the "ibm,client-architecture-
> support" call yet (but I submitted a patch for the kernel to fix this
> issue there last week, so the support should soon be there, too).
> 
> Anyway, QEMU should also support a proper 2.07 compatibility mode
> if the host CPU can do it. So this patch series introduces such a
> mode and does some clean-ups and other fixes along the way (e.g.
> it splits the ambiguous pcr_mask setting into two variables, one
> for defining the valid bits in the PCR register, and one for
> storing the valid ISA levels).
> 
> Thomas Huth (5):
>   ppc/spapr: Refactor h_client_architecture_support() CPU parsing code
>   ppc: Split pcr_mask settings into supported bits and the register mask
>   ppc: Provide function to get CPU class of the host CPU
>   ppc: Improve PCR bit selection in ppc_set_compat()
>   ppc: Add PowerISA 2.07 compatibility mode
> 
>  hw/ppc/spapr_hcall.c        | 63 +++++++++++++++++++++++++++------------------
>  target-ppc/cpu-qom.h        |  3 ++-
>  target-ppc/cpu.h            |  1 +
>  target-ppc/kvm.c            | 19 ++++++++++----
>  target-ppc/kvm_ppc.h        |  7 +++++
>  target-ppc/translate_init.c | 22 +++++++++++-----
>  6 files changed, 78 insertions(+), 37 deletions(-)

Applied to ppc-for-2.7, thanks.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

Re: [Qemu-devel] [PATCH 4/5] ppc: Improve PCR bit selection in ppc_set_compat()

[David Gibson]

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

On Tue, Jun 07, 2016 at 05:39:39PM +0200, Thomas Huth wrote:
> When using an olderr PowerISA level, all the upper compatibility
> bits have to be enabled, too. For example when we want to run
> something in PowerISA 2.05 compatibility mode on POWER8, the bit
> for 2.06 has to be set beside the bit for 2.05.
> Additionally, to make sure that we do not set bits that are not
> supported by the host, we apply a mask with the known-to-be-good
> bits here, too.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

This one confused me a bit until I realised that, roughly speaking,
bits in the PCR turn features off, rather than turning features on.
Does that sound correct?

> ---
>  target-ppc/translate_init.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index fa09183..ee2bc14 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9519,24 +9519,29 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **errp)
>  {
>      int ret = 0;
>      CPUPPCState *env = &cpu->env;
> +    PowerPCCPUClass *host_pcc;
>  
>      cpu->cpu_version = cpu_version;
>  
>      switch (cpu_version) {
>      case CPU_POWERPC_LOGICAL_2_05:
> -        env->spr[SPR_PCR] = PCR_COMPAT_2_05;
> +        env->spr[SPR_PCR] = PCR_TM_DIS | PCR_VSX_DIS | PCR_COMPAT_2_07 |
> +                            PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
>          break;
>      case CPU_POWERPC_LOGICAL_2_06:
> -        env->spr[SPR_PCR] = PCR_COMPAT_2_06;
> -        break;
>      case CPU_POWERPC_LOGICAL_2_06_PLUS:
> -        env->spr[SPR_PCR] = PCR_COMPAT_2_06;
> +        env->spr[SPR_PCR] = PCR_TM_DIS | PCR_COMPAT_2_07 | PCR_COMPAT_2_06;
>          break;
>      default:
>          env->spr[SPR_PCR] = 0;
>          break;
>      }
>  
> +    host_pcc = kvm_ppc_get_host_cpu_class();
> +    if (host_pcc) {
> +        env->spr[SPR_PCR] &= host_pcc->pcr_mask;
> +    }
> +
>      if (kvm_enabled()) {
>          ret = kvmppc_set_compat(cpu, cpu->cpu_version);
>          if (ret < 0) {

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

Re: [Qemu-devel] [Qemu-ppc] [PATCH 4/5] ppc: Improve PCR bit selection in ppc_set_compat()

[David Gibson]

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

On Tue, Jun 07, 2016 at 05:39:39PM +0200, Thomas Huth wrote:
> When using an olderr PowerISA level, all the upper compatibility
> bits have to be enabled, too. For example when we want to run
> something in PowerISA 2.05 compatibility mode on POWER8, the bit
> for 2.06 has to be set beside the bit for 2.05.
> Additionally, to make sure that we do not set bits that are not
> supported by the host, we apply a mask with the known-to-be-good
> bits here, too.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>

So, this breaks compile on 32-bit targets, because the spr values are
only 32-bit there, and the PCR constants exceed that.  But
ppc_set_compat() is only actually used on 64-bit machines, so I've
added a change to #if it out for 64-bit targets.

> ---
>  target-ppc/translate_init.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index fa09183..ee2bc14 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -9519,24 +9519,29 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **errp)
>  {
>      int ret = 0;
>      CPUPPCState *env = &cpu->env;
> +    PowerPCCPUClass *host_pcc;
>  
>      cpu->cpu_version = cpu_version;
>  
>      switch (cpu_version) {
>      case CPU_POWERPC_LOGICAL_2_05:
> -        env->spr[SPR_PCR] = PCR_COMPAT_2_05;
> +        env->spr[SPR_PCR] = PCR_TM_DIS | PCR_VSX_DIS | PCR_COMPAT_2_07 |
> +                            PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
>          break;
>      case CPU_POWERPC_LOGICAL_2_06:
> -        env->spr[SPR_PCR] = PCR_COMPAT_2_06;
> -        break;
>      case CPU_POWERPC_LOGICAL_2_06_PLUS:
> -        env->spr[SPR_PCR] = PCR_COMPAT_2_06;
> +        env->spr[SPR_PCR] = PCR_TM_DIS | PCR_COMPAT_2_07 | PCR_COMPAT_2_06;
>          break;
>      default:
>          env->spr[SPR_PCR] = 0;
>          break;
>      }
>  
> +    host_pcc = kvm_ppc_get_host_cpu_class();
> +    if (host_pcc) {
> +        env->spr[SPR_PCR] &= host_pcc->pcr_mask;
> +    }
> +
>      if (kvm_enabled()) {
>          ret = kvmppc_set_compat(cpu, cpu->cpu_version);
>          if (ret < 0) {

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

Re: [Qemu-devel] [PATCH 4/5] ppc: Improve PCR bit selection in ppc_set_compat()

[Thomas Huth]

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

On 08.06.2016 03:12, David Gibson wrote:
> On Tue, Jun 07, 2016 at 05:39:39PM +0200, Thomas Huth wrote:
>> When using an olderr PowerISA level, all the upper compatibility

s/olderr/older/ :-/

>> bits have to be enabled, too. For example when we want to run
>> something in PowerISA 2.05 compatibility mode on POWER8, the bit
>> for 2.06 has to be set beside the bit for 2.05.
>> Additionally, to make sure that we do not set bits that are not
>> supported by the host, we apply a mask with the known-to-be-good
>> bits here, too.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> This one confused me a bit until I realised that, roughly speaking,
> bits in the PCR turn features off, rather than turning features on.
> Does that sound correct?

Right. Turning on the 2.06 bit means "disable all things that have been
added additionally in 2.07", for example. So if you turn on bit 2.05,
but not bit 2.06 (which the old code was doing), I guess you end up in a
strange state where new instructions from PowerISA 2.07 can be used, but
the instructions that have been added in ISA 2.06 are disabled.

>> ---
>>  target-ppc/translate_init.c | 13 +++++++++----
>>  1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index fa09183..ee2bc14 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -9519,24 +9519,29 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t cpu_version, Error **errp)
>>  {
>>      int ret = 0;
>>      CPUPPCState *env = &cpu->env;
>> +    PowerPCCPUClass *host_pcc;
>>  
>>      cpu->cpu_version = cpu_version;
>>  
>>      switch (cpu_version) {
>>      case CPU_POWERPC_LOGICAL_2_05:
>> -        env->spr[SPR_PCR] = PCR_COMPAT_2_05;
>> +        env->spr[SPR_PCR] = PCR_TM_DIS | PCR_VSX_DIS | PCR_COMPAT_2_07 |
>> +                            PCR_COMPAT_2_06 | PCR_COMPAT_2_05;
>>          break;
>>      case CPU_POWERPC_LOGICAL_2_06:
>> -        env->spr[SPR_PCR] = PCR_COMPAT_2_06;
>> -        break;
>>      case CPU_POWERPC_LOGICAL_2_06_PLUS:
>> -        env->spr[SPR_PCR] = PCR_COMPAT_2_06;
>> +        env->spr[SPR_PCR] = PCR_TM_DIS | PCR_COMPAT_2_07 | PCR_COMPAT_2_06;
>>          break;
>>      default:
>>          env->spr[SPR_PCR] = 0;
>>          break;
>>      }
>>  
>> +    host_pcc = kvm_ppc_get_host_cpu_class();
>> +    if (host_pcc) {
>> +        env->spr[SPR_PCR] &= host_pcc->pcr_mask;
>> +    }
>> +
>>      if (kvm_enabled()) {
>>          ret = kvmppc_set_compat(cpu, cpu->cpu_version);
>>          if (ret < 0) {
> 

 Thomas



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

Re: [Qemu-devel] [Qemu-ppc] [PATCH 4/5] ppc: Improve PCR bit selection in ppc_set_compat()

[Thomas Huth]

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

On 08.06.2016 07:44, David Gibson wrote:
> On Tue, Jun 07, 2016 at 05:39:39PM +0200, Thomas Huth wrote:
>> When using an olderr PowerISA level, all the upper compatibility
>> bits have to be enabled, too. For example when we want to run
>> something in PowerISA 2.05 compatibility mode on POWER8, the bit
>> for 2.06 has to be set beside the bit for 2.05.
>> Additionally, to make sure that we do not set bits that are not
>> supported by the host, we apply a mask with the known-to-be-good
>> bits here, too.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
> 
> So, this breaks compile on 32-bit targets, because the spr values are
> only 32-bit there, and the PCR constants exceed that.  But
> ppc_set_compat() is only actually used on 64-bit machines, so I've
> added a change to #if it out for 64-bit targets.

D'oh, I explicitly compiled everything with a mingw32 cross-compiler to
catch such issues ... but apparently it compiled without -Werror here,
so I did not notice the warning :-(
Thanks for the fixup!

 Thomas



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

Re: [Qemu-devel] [Qemu-ppc] [PATCH 4/5] ppc: Improve PCR bit selection in ppc_set_compat()

[David Gibson]

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

On Wed, Jun 08, 2016 at 08:59:30AM +0200, Thomas Huth wrote:
> On 08.06.2016 07:44, David Gibson wrote:
> > On Tue, Jun 07, 2016 at 05:39:39PM +0200, Thomas Huth wrote:
> >> When using an olderr PowerISA level, all the upper compatibility
> >> bits have to be enabled, too. For example when we want to run
> >> something in PowerISA 2.05 compatibility mode on POWER8, the bit
> >> for 2.06 has to be set beside the bit for 2.05.
> >> Additionally, to make sure that we do not set bits that are not
> >> supported by the host, we apply a mask with the known-to-be-good
> >> bits here, too.
> >>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> > 
> > So, this breaks compile on 32-bit targets, because the spr values are
> > only 32-bit there, and the PCR constants exceed that.  But
> > ppc_set_compat() is only actually used on 64-bit machines, so I've
> > added a change to #if it out for 64-bit targets.
> 
> D'oh, I explicitly compiled everything with a mingw32 cross-compiler to
> catch such issues ... but apparently it compiled without -Werror here,
> so I did not notice the warning :-(

No, not 32-bit *host*, 32-bit *target*, so mingw32 wouldn't help.
Configuring in ppc-softmmu and ppcemb-softmmu would.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

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

Re: [Qemu-devel] [Qemu-ppc] [PATCH 4/5] ppc: Improve PCR bit selection in ppc_set_compat()

[Thomas Huth]

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

On 08.06.2016 09:24, David Gibson wrote:
> On Wed, Jun 08, 2016 at 08:59:30AM +0200, Thomas Huth wrote:
>> On 08.06.2016 07:44, David Gibson wrote:
>>> On Tue, Jun 07, 2016 at 05:39:39PM +0200, Thomas Huth wrote:
>>>> When using an olderr PowerISA level, all the upper compatibility
>>>> bits have to be enabled, too. For example when we want to run
>>>> something in PowerISA 2.05 compatibility mode on POWER8, the bit
>>>> for 2.06 has to be set beside the bit for 2.05.
>>>> Additionally, to make sure that we do not set bits that are not
>>>> supported by the host, we apply a mask with the known-to-be-good
>>>> bits here, too.
>>>>
>>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>>
>>> So, this breaks compile on 32-bit targets, because the spr values are
>>> only 32-bit there, and the PCR constants exceed that.  But
>>> ppc_set_compat() is only actually used on 64-bit machines, so I've
>>> added a change to #if it out for 64-bit targets.
>>
>> D'oh, I explicitly compiled everything with a mingw32 cross-compiler to
>> catch such issues ... but apparently it compiled without -Werror here,
>> so I did not notice the warning :-(
> 
> No, not 32-bit *host*, 32-bit *target*, so mingw32 wouldn't help.
> Configuring in ppc-softmmu and ppcemb-softmmu would.

Ok, right, ... I've now enabled these in my compile-test folder, too.

Anyway, seems like mingw is compiled without -Werror by default, unlike
the Linux builds ... I guess we could enable -Werror by default on mingw
nowadays, too...

 Thomas



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