All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] target-ppc: Merge POWER7+ to POWER7 and POWER8E to POWER8
@ 2014-06-30 14:30 Alexey Kardashevskiy
  2014-06-30 14:30 ` [Qemu-devel] [PATCH 1/2] target-ppc: Add pvr_match() callback Alexey Kardashevskiy
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Alexey Kardashevskiy @ 2014-06-30 14:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, qemu-ppc, Alexander Graf


Here are 2 patches to simplify handling of the same CPUs with slightly different PVRs.
Now with a callback instead of additional levels in PPC CPU QOM class tree.

Please comment. Thanks.



Alexey Kardashevskiy (2):
  target-ppc: Add pvr_match() callback
  target-ppc: Remove POWER7+ and POWER8E families

 target-ppc/cpu-models.c     |   5 +-
 target-ppc/cpu-models.h     |   5 +-
 target-ppc/cpu-qom.h        |   2 +-
 target-ppc/translate_init.c | 120 +++++++++++++-------------------------------
 4 files changed, 40 insertions(+), 92 deletions(-)

-- 
2.0.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH 1/2] target-ppc: Add pvr_match() callback
  2014-06-30 14:30 [Qemu-devel] [PATCH 0/2] target-ppc: Merge POWER7+ to POWER7 and POWER8E to POWER8 Alexey Kardashevskiy
@ 2014-06-30 14:30 ` Alexey Kardashevskiy
  2014-07-03 12:44   ` Alexander Graf
  2014-06-30 14:30 ` [Qemu-devel] [PATCH 2/2] target-ppc: Remove POWER7+ and POWER8E families Alexey Kardashevskiy
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Alexey Kardashevskiy @ 2014-06-30 14:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, qemu-ppc, Alexander Graf

So far it was enough to have a base PVR value and mask per CPU
family such as POWER7 or POWER8. However there CPUs which are
completely architecturally compatible but have different PVRs such
as POWER7/POWER7+ and POWER8/POWER8E. For these CPUs, top 16 bits
are CPU family and low 16 bits are the version. The families have
PVR base values different enough so defining a mask which
would cover both (or potentially more) CPUs within the family is
not possible.

This adds a pvr_match() callback to PowerPCCPUClass. The default
handler simply compares PVR defined in the class.

This implements ppc_pvr_match_power7/ppc_pvr_match_power8 callbacks
for POWER7/8 families. These check for POWER7/POWER7+ and POWER8/POWER8E.

This changes ppc_cpu_compare_class_pvr_mask() not to check masks but
use the pvr_match() callback.

Since all server CPUs use the same mask, this defines one mask
value - CPU_POWERPC_POWER_SERVER_MASK - which is used everywhere now.
This removes other mask definitions.

This removes pvr_mask from PowerPCCPUClass as it is not used anymore.
This removes pvr initialization for POWER7/8 families as it is not used
to find the class, the pvr_match() callback is used instead.

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

diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
index 9a91af9..c9112e9 100644
--- a/target-ppc/cpu-models.c
+++ b/target-ppc/cpu-models.c
@@ -44,7 +44,6 @@
         PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);                       \
                                                                             \
         pcc->pvr          = _pvr;                                           \
-        pcc->pvr_mask     = CPU_POWERPC_DEFAULT_MASK;                       \
         pcc->svr          = _svr;                                           \
         dc->desc          = _desc;                                          \
     }                                                                       \
diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
index c39d03a..9d2ee91 100644
--- a/target-ppc/cpu-models.h
+++ b/target-ppc/cpu-models.h
@@ -553,17 +553,14 @@ enum {
     CPU_POWERPC_POWER6             = 0x003E0000,
     CPU_POWERPC_POWER6_5           = 0x0F000001, /* POWER6 in POWER5 mode */
     CPU_POWERPC_POWER6A            = 0x0F000002,
+    CPU_POWERPC_POWER_SERVER_MASK  = 0xFFFF0000,
     CPU_POWERPC_POWER7_BASE        = 0x003F0000,
-    CPU_POWERPC_POWER7_MASK        = 0xFFFF0000,
     CPU_POWERPC_POWER7_v23         = 0x003F0203,
     CPU_POWERPC_POWER7P_BASE       = 0x004A0000,
-    CPU_POWERPC_POWER7P_MASK       = 0xFFFF0000,
     CPU_POWERPC_POWER7P_v21        = 0x004A0201,
     CPU_POWERPC_POWER8E_BASE       = 0x004B0000,
-    CPU_POWERPC_POWER8E_MASK       = 0xFFFF0000,
     CPU_POWERPC_POWER8E_v10        = 0x004B0100,
     CPU_POWERPC_POWER8_BASE        = 0x004D0000,
-    CPU_POWERPC_POWER8_MASK        = 0xFFFF0000,
     CPU_POWERPC_POWER8_v10         = 0x004D0100,
     CPU_POWERPC_970                = 0x00390202,
     CPU_POWERPC_970FX_v10          = 0x00391100,
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index f1f0a52..0fee36f 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -56,7 +56,7 @@ typedef struct PowerPCCPUClass {
     void (*parent_reset)(CPUState *cpu);
 
     uint32_t pvr;
-    uint32_t pvr_mask;
+    bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr);
     uint64_t pcr_mask;
     uint32_t svr;
     uint64_t insns_flags;
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 2ab2810..5d4fee7 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8062,6 +8062,17 @@ static void init_proc_POWER7 (CPUPPCState *env)
     init_proc_book3s_64(env, BOOK3S_CPU_POWER7);
 }
 
+static bool ppc_pvr_match_power7(PowerPCCPUClass *pcc, uint32_t pvr)
+{
+    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7P_BASE) {
+        return true;
+    }
+    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7_BASE) {
+        return true;
+    }
+    return false;
+}
+
 POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -8070,8 +8081,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->pvr_match = ppc_pvr_match_power7;
     pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
     pcc->init_proc = init_proc_POWER7;
     pcc->check_pow = check_pow_nocheck;
@@ -8131,8 +8141,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->pvr_match = ppc_pvr_match_power7;
     pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
     pcc->init_proc = init_proc_POWER7;
     pcc->check_pow = check_pow_nocheck;
@@ -8189,6 +8198,17 @@ static void init_proc_POWER8(CPUPPCState *env)
     init_proc_book3s_64(env, BOOK3S_CPU_POWER8);
 }
 
+static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr)
+{
+    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8E_BASE) {
+        return true;
+    }
+    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8_BASE) {
+        return true;
+    }
+    return false;
+}
+
 POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -8197,8 +8217,7 @@ POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
     dc->fw_name = "PowerPC,POWER8";
     dc->desc = "POWER8E";
     dc->props = powerpc_servercpu_properties;
-    pcc->pvr = CPU_POWERPC_POWER8E_BASE;
-    pcc->pvr_mask = CPU_POWERPC_POWER8E_MASK;
+    pcc->pvr_match = ppc_pvr_match_power8;
     pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
     pcc->init_proc = init_proc_POWER8;
     pcc->check_pow = check_pow_nocheck;
@@ -8256,13 +8275,10 @@ POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
 POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
-    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
 
     ppc_POWER8E_cpu_family_class_init(oc, data);
 
     dc->desc = "POWER8";
-    pcc->pvr = CPU_POWERPC_POWER8_BASE;
-    pcc->pvr_mask = CPU_POWERPC_POWER8_MASK;
 }
 #endif /* defined (TARGET_PPC64) */
 
@@ -9245,7 +9261,6 @@ static gint ppc_cpu_compare_class_pvr_mask(gconstpointer a, gconstpointer b)
     ObjectClass *oc = (ObjectClass *)a;
     uint32_t pvr = *(uint32_t *)b;
     PowerPCCPUClass *pcc = (PowerPCCPUClass *)a;
-    gint ret;
 
     /* -cpu host does a PVR lookup during construction */
     if (unlikely(strcmp(object_class_get_name(oc),
@@ -9257,9 +9272,11 @@ static gint ppc_cpu_compare_class_pvr_mask(gconstpointer a, gconstpointer b)
         return -1;
     }
 
-    ret = (((pcc->pvr & pcc->pvr_mask) == (pvr & pcc->pvr_mask)) ? 0 : -1);
+    if (pcc->pvr_match && pcc->pvr_match(pcc, pvr)) {
+        return 0;
+    }
 
-    return ret;
+    return -1;
 }
 
 PowerPCCPUClass *ppc_cpu_class_by_pvr_mask(uint32_t pvr)
@@ -9656,6 +9673,11 @@ static void ppc_cpu_initfn(Object *obj)
     }
 }
 
+static bool ppc_pvr_match_default(PowerPCCPUClass *pcc, uint32_t pvr)
+{
+    return pcc->pvr == pvr;
+}
+
 static void ppc_cpu_class_init(ObjectClass *oc, void *data)
 {
     PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
@@ -9663,8 +9685,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
 
     pcc->parent_realize = dc->realize;
-    pcc->pvr = CPU_POWERPC_DEFAULT_MASK;
-    pcc->pvr_mask = CPU_POWERPC_DEFAULT_MASK;
+    pcc->pvr_match = ppc_pvr_match_default;
     pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_always;
     dc->realize = ppc_cpu_realizefn;
     dc->unrealize = ppc_cpu_unrealizefn;
-- 
2.0.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH 2/2] target-ppc: Remove POWER7+ and POWER8E families
  2014-06-30 14:30 [Qemu-devel] [PATCH 0/2] target-ppc: Merge POWER7+ to POWER7 and POWER8E to POWER8 Alexey Kardashevskiy
  2014-06-30 14:30 ` [Qemu-devel] [PATCH 1/2] target-ppc: Add pvr_match() callback Alexey Kardashevskiy
@ 2014-06-30 14:30 ` Alexey Kardashevskiy
  2014-07-03 12:46 ` [Qemu-devel] [PATCH 0/2] target-ppc: Merge POWER7+ to POWER7 and POWER8E to POWER8 Alexander Graf
  2014-07-03 14:53 ` Alexander Graf
  3 siblings, 0 replies; 9+ messages in thread
From: Alexey Kardashevskiy @ 2014-06-30 14:30 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, qemu-ppc, Alexander Graf

POWER8E is architecturally equal to POWER8 and POWER7+ is equal to
POWER7. Also no user space tool makes any difference for CPU node name
in the device tree (such as PowerPC,POWER7@0 vs. PowerPC,POWER7+@0).
So there is no point in emulating POWER7+ and POWER8E apart from POWER7
and POWER8. Also, the previos patch implemented multiple PVR mask support
per CPU class so POWER7 class now covers both POWER7 and POWER7+ CPUs,
same is valid for POWER8/8E.

This removes POWER7+ and POWER8E classes. This replaces references
to POWER7P/POWER8E families with POWER7/POWER8 families.

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

diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
index c9112e9..52ac6ec 100644
--- a/target-ppc/cpu-models.c
+++ b/target-ppc/cpu-models.c
@@ -1135,9 +1135,9 @@
 #endif
     POWERPC_DEF("POWER7_v2.3",   CPU_POWERPC_POWER7_v23,             POWER7,
                 "POWER7 v2.3")
-    POWERPC_DEF("POWER7+_v2.1",  CPU_POWERPC_POWER7P_v21,            POWER7P,
+    POWERPC_DEF("POWER7+_v2.1",  CPU_POWERPC_POWER7P_v21,            POWER7,
                 "POWER7+ v2.1")
-    POWERPC_DEF("POWER8E_v1.0",  CPU_POWERPC_POWER8E_v10,            POWER8E,
+    POWERPC_DEF("POWER8E_v1.0",  CPU_POWERPC_POWER8E_v10,            POWER8,
                 "POWER8E v1.0")
     POWERPC_DEF("POWER8_v1.0",   CPU_POWERPC_POWER8_v10,             POWER8,
                 "POWER8 v1.0")
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 5d4fee7..1d38a3a 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8133,66 +8133,6 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
     pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
 }
 
-POWERPC_FAMILY(POWER7P)(ObjectClass *oc, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(oc);
-    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
-
-    dc->fw_name = "PowerPC,POWER7+";
-    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->init_proc = init_proc_POWER7;
-    pcc->check_pow = check_pow_nocheck;
-    pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
-                       PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
-                       PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
-                       PPC_FLOAT_FRSQRTES |
-                       PPC_FLOAT_STFIWX |
-                       PPC_FLOAT_EXT |
-                       PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
-                       PPC_MEM_SYNC | PPC_MEM_EIEIO |
-                       PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
-                       PPC_64B | PPC_ALTIVEC |
-                       PPC_SEGMENT_64B | PPC_SLBI |
-                       PPC_POPCNTB | PPC_POPCNTWD;
-    pcc->insns_flags2 = PPC2_VSX | PPC2_DFP | PPC2_DBRX | PPC2_ISA205 |
-                        PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
-                        PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
-                        PPC2_FP_TST_ISA206;
-    pcc->msr_mask = (1ull << MSR_SF) |
-                    (1ull << MSR_VR) |
-                    (1ull << MSR_VSX) |
-                    (1ull << MSR_EE) |
-                    (1ull << MSR_PR) |
-                    (1ull << MSR_FP) |
-                    (1ull << MSR_ME) |
-                    (1ull << MSR_FE0) |
-                    (1ull << MSR_SE) |
-                    (1ull << MSR_DE) |
-                    (1ull << MSR_FE1) |
-                    (1ull << MSR_IR) |
-                    (1ull << MSR_DR) |
-                    (1ull << MSR_PMM) |
-                    (1ull << MSR_RI) |
-                    (1ull << MSR_LE);
-    pcc->mmu_model = POWERPC_MMU_2_06;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
-#endif
-    pcc->excp_model = POWERPC_EXCP_POWER7;
-    pcc->bus_model = PPC_FLAGS_INPUT_POWER7;
-    pcc->bfd_mach = bfd_mach_ppc64;
-    pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
-                 POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
-                 POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
-                 POWERPC_FLAG_VSX;
-    pcc->l1_dcache_size = 0x8000;
-    pcc->l1_icache_size = 0x8000;
-    pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
-}
-
 static void init_proc_POWER8(CPUPPCState *env)
 {
     init_proc_book3s_64(env, BOOK3S_CPU_POWER8);
@@ -8209,13 +8149,13 @@ static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr)
     return false;
 }
 
-POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
+POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
     PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
 
     dc->fw_name = "PowerPC,POWER8";
-    dc->desc = "POWER8E";
+    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;
@@ -8271,15 +8211,6 @@ POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
     pcc->l1_icache_size = 0x8000;
     pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
 }
-
-POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
-{
-    DeviceClass *dc = DEVICE_CLASS(oc);
-
-    ppc_POWER8E_cpu_family_class_init(oc, data);
-
-    dc->desc = "POWER8";
-}
 #endif /* defined (TARGET_PPC64) */
 
 
-- 
2.0.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] target-ppc: Add pvr_match() callback
  2014-06-30 14:30 ` [Qemu-devel] [PATCH 1/2] target-ppc: Add pvr_match() callback Alexey Kardashevskiy
@ 2014-07-03 12:44   ` Alexander Graf
  2014-07-03 13:32     ` [Qemu-devel] [PATCH v2] " Alexey Kardashevskiy
  0 siblings, 1 reply; 9+ messages in thread
From: Alexander Graf @ 2014-07-03 12:44 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel; +Cc: qemu-ppc


On 30.06.14 16:30, Alexey Kardashevskiy wrote:
> So far it was enough to have a base PVR value and mask per CPU
> family such as POWER7 or POWER8. However there CPUs which are
> completely architecturally compatible but have different PVRs such
> as POWER7/POWER7+ and POWER8/POWER8E. For these CPUs, top 16 bits
> are CPU family and low 16 bits are the version. The families have
> PVR base values different enough so defining a mask which
> would cover both (or potentially more) CPUs within the family is
> not possible.
>
> This adds a pvr_match() callback to PowerPCCPUClass. The default
> handler simply compares PVR defined in the class.
>
> This implements ppc_pvr_match_power7/ppc_pvr_match_power8 callbacks
> for POWER7/8 families. These check for POWER7/POWER7+ and POWER8/POWER8E.
>
> This changes ppc_cpu_compare_class_pvr_mask() not to check masks but
> use the pvr_match() callback.
>
> Since all server CPUs use the same mask, this defines one mask
> value - CPU_POWERPC_POWER_SERVER_MASK - which is used everywhere now.
> This removes other mask definitions.
>
> This removes pvr_mask from PowerPCCPUClass as it is not used anymore.
> This removes pvr initialization for POWER7/8 families as it is not used
> to find the class, the pvr_match() callback is used instead.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>   target-ppc/cpu-models.c     |  1 -
>   target-ppc/cpu-models.h     |  5 +----
>   target-ppc/cpu-qom.h        |  2 +-
>   target-ppc/translate_init.c | 49 ++++++++++++++++++++++++++++++++-------------
>   4 files changed, 37 insertions(+), 20 deletions(-)
>
> diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
> index 9a91af9..c9112e9 100644
> --- a/target-ppc/cpu-models.c
> +++ b/target-ppc/cpu-models.c
> @@ -44,7 +44,6 @@
>           PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);                       \
>                                                                               \
>           pcc->pvr          = _pvr;                                           \
> -        pcc->pvr_mask     = CPU_POWERPC_DEFAULT_MASK;                       \

We don't need the DEFAULT_MASK anymore now, no?

>           pcc->svr          = _svr;                                           \
>           dc->desc          = _desc;                                          \
>       }                                                                       \
> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
> index c39d03a..9d2ee91 100644
> --- a/target-ppc/cpu-models.h
> +++ b/target-ppc/cpu-models.h
> @@ -553,17 +553,14 @@ enum {
>       CPU_POWERPC_POWER6             = 0x003E0000,
>       CPU_POWERPC_POWER6_5           = 0x0F000001, /* POWER6 in POWER5 mode */
>       CPU_POWERPC_POWER6A            = 0x0F000002,
> +    CPU_POWERPC_POWER_SERVER_MASK  = 0xFFFF0000,
>       CPU_POWERPC_POWER7_BASE        = 0x003F0000,
> -    CPU_POWERPC_POWER7_MASK        = 0xFFFF0000,
>       CPU_POWERPC_POWER7_v23         = 0x003F0203,
>       CPU_POWERPC_POWER7P_BASE       = 0x004A0000,
> -    CPU_POWERPC_POWER7P_MASK       = 0xFFFF0000,
>       CPU_POWERPC_POWER7P_v21        = 0x004A0201,
>       CPU_POWERPC_POWER8E_BASE       = 0x004B0000,
> -    CPU_POWERPC_POWER8E_MASK       = 0xFFFF0000,
>       CPU_POWERPC_POWER8E_v10        = 0x004B0100,
>       CPU_POWERPC_POWER8_BASE        = 0x004D0000,
> -    CPU_POWERPC_POWER8_MASK        = 0xFFFF0000,
>       CPU_POWERPC_POWER8_v10         = 0x004D0100,
>       CPU_POWERPC_970                = 0x00390202,
>       CPU_POWERPC_970FX_v10          = 0x00391100,
> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> index f1f0a52..0fee36f 100644
> --- a/target-ppc/cpu-qom.h
> +++ b/target-ppc/cpu-qom.h
> @@ -56,7 +56,7 @@ typedef struct PowerPCCPUClass {
>       void (*parent_reset)(CPUState *cpu);
>   
>       uint32_t pvr;
> -    uint32_t pvr_mask;
> +    bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr);
>       uint64_t pcr_mask;
>       uint32_t svr;
>       uint64_t insns_flags;
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 2ab2810..5d4fee7 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -8062,6 +8062,17 @@ static void init_proc_POWER7 (CPUPPCState *env)
>       init_proc_book3s_64(env, BOOK3S_CPU_POWER7);
>   }
>   
> +static bool ppc_pvr_match_power7(PowerPCCPUClass *pcc, uint32_t pvr)
> +{
> +    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7P_BASE) {
> +        return true;
> +    }
> +    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7_BASE) {
> +        return true;
> +    }
> +    return false;
> +}
> +
>   POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(oc);
> @@ -8070,8 +8081,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->pvr_match = ppc_pvr_match_power7;
>       pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
>       pcc->init_proc = init_proc_POWER7;
>       pcc->check_pow = check_pow_nocheck;
> @@ -8131,8 +8141,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->pvr_match = ppc_pvr_match_power7;
>       pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
>       pcc->init_proc = init_proc_POWER7;
>       pcc->check_pow = check_pow_nocheck;
> @@ -8189,6 +8198,17 @@ static void init_proc_POWER8(CPUPPCState *env)
>       init_proc_book3s_64(env, BOOK3S_CPU_POWER8);
>   }
>   
> +static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr)
> +{
> +    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8E_BASE) {
> +        return true;
> +    }
> +    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8_BASE) {
> +        return true;
> +    }
> +    return false;
> +}
> +
>   POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(oc);
> @@ -8197,8 +8217,7 @@ POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
>       dc->fw_name = "PowerPC,POWER8";
>       dc->desc = "POWER8E";
>       dc->props = powerpc_servercpu_properties;
> -    pcc->pvr = CPU_POWERPC_POWER8E_BASE;
> -    pcc->pvr_mask = CPU_POWERPC_POWER8E_MASK;
> +    pcc->pvr_match = ppc_pvr_match_power8;
>       pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
>       pcc->init_proc = init_proc_POWER8;
>       pcc->check_pow = check_pow_nocheck;
> @@ -8256,13 +8275,10 @@ POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
>   POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(oc);
> -    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
>   
>       ppc_POWER8E_cpu_family_class_init(oc, data);
>   
>       dc->desc = "POWER8";
> -    pcc->pvr = CPU_POWERPC_POWER8_BASE;
> -    pcc->pvr_mask = CPU_POWERPC_POWER8_MASK;
>   }
>   #endif /* defined (TARGET_PPC64) */
>   
> @@ -9245,7 +9261,6 @@ static gint ppc_cpu_compare_class_pvr_mask(gconstpointer a, gconstpointer b)
>       ObjectClass *oc = (ObjectClass *)a;
>       uint32_t pvr = *(uint32_t *)b;
>       PowerPCCPUClass *pcc = (PowerPCCPUClass *)a;
> -    gint ret;
>   
>       /* -cpu host does a PVR lookup during construction */
>       if (unlikely(strcmp(object_class_get_name(oc),
> @@ -9257,9 +9272,11 @@ static gint ppc_cpu_compare_class_pvr_mask(gconstpointer a, gconstpointer b)
>           return -1;
>       }
>   
> -    ret = (((pcc->pvr & pcc->pvr_mask) == (pvr & pcc->pvr_mask)) ? 0 : -1);
> +    if (pcc->pvr_match && pcc->pvr_match(pcc, pvr)) {

We can safely assume that pvr_match() always exists.


Alex

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] target-ppc: Merge POWER7+ to POWER7 and POWER8E to POWER8
  2014-06-30 14:30 [Qemu-devel] [PATCH 0/2] target-ppc: Merge POWER7+ to POWER7 and POWER8E to POWER8 Alexey Kardashevskiy
  2014-06-30 14:30 ` [Qemu-devel] [PATCH 1/2] target-ppc: Add pvr_match() callback Alexey Kardashevskiy
  2014-06-30 14:30 ` [Qemu-devel] [PATCH 2/2] target-ppc: Remove POWER7+ and POWER8E families Alexey Kardashevskiy
@ 2014-07-03 12:46 ` Alexander Graf
  2014-07-03 14:53 ` Alexander Graf
  3 siblings, 0 replies; 9+ messages in thread
From: Alexander Graf @ 2014-07-03 12:46 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel; +Cc: qemu-ppc


On 30.06.14 16:30, Alexey Kardashevskiy wrote:
> Here are 2 patches to simplify handling of the same CPUs with slightly different PVRs.
> Now with a callback instead of additional levels in PPC CPU QOM class tree.
>
> Please comment. Thanks.

Very simply and clean approach to the problem at hand. Please fix the 
minor nits in patch 1 and I'll queue it for 2.1.


Alex

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v2] target-ppc: Add pvr_match() callback
  2014-07-03 12:44   ` Alexander Graf
@ 2014-07-03 13:32     ` Alexey Kardashevskiy
  2014-07-03 13:54       ` Alexander Graf
  0 siblings, 1 reply; 9+ messages in thread
From: Alexey Kardashevskiy @ 2014-07-03 13:32 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, qemu-ppc, Alexander Graf

So far it was enough to have a base PVR value and mask per CPU
family such as POWER7 or POWER8. However there CPUs which are
completely architecturally compatible but have different PVRs such
as POWER7/POWER7+ and POWER8/POWER8E. For these CPUs, top 16 bits
are CPU family and low 16 bits are the version. The families have
PVR base values different enough so defining a mask which
would cover both (or potentially more) CPUs within the family is
not possible.

This adds a pvr_match() callback to PowerPCCPUClass. The default
handler simply compares PVR defined in the class.

This implements ppc_pvr_match_power7/ppc_pvr_match_power8 callbacks
for POWER7/8 families. These check for POWER7/POWER7+ and POWER8/POWER8E.

This changes ppc_cpu_compare_class_pvr_mask() not to check masks but
use the pvr_match() callback.

Since all server CPUs use the same mask, this defines one mask
value - CPU_POWERPC_POWER_SERVER_MASK - which is used everywhere now.
This removes other mask definitions.

This removes pvr_mask from PowerPCCPUClass as it is not used anymore.
This removes pvr initialization for POWER7/8 families as it is not used
to find the class, the pvr_match() callback is used instead.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* do not check pvr_match==NULL as it is always define for PPC
---
 target-ppc/cpu-models.c     |  1 -
 target-ppc/cpu-models.h     |  5 +----
 target-ppc/cpu-qom.h        |  2 +-
 target-ppc/translate_init.c | 49 ++++++++++++++++++++++++++++++++-------------
 4 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
index 9a91af9..c9112e9 100644
--- a/target-ppc/cpu-models.c
+++ b/target-ppc/cpu-models.c
@@ -44,7 +44,6 @@
         PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);                       \
                                                                             \
         pcc->pvr          = _pvr;                                           \
-        pcc->pvr_mask     = CPU_POWERPC_DEFAULT_MASK;                       \
         pcc->svr          = _svr;                                           \
         dc->desc          = _desc;                                          \
     }                                                                       \
diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
index c39d03a..9d2ee91 100644
--- a/target-ppc/cpu-models.h
+++ b/target-ppc/cpu-models.h
@@ -553,17 +553,14 @@ enum {
     CPU_POWERPC_POWER6             = 0x003E0000,
     CPU_POWERPC_POWER6_5           = 0x0F000001, /* POWER6 in POWER5 mode */
     CPU_POWERPC_POWER6A            = 0x0F000002,
+    CPU_POWERPC_POWER_SERVER_MASK  = 0xFFFF0000,
     CPU_POWERPC_POWER7_BASE        = 0x003F0000,
-    CPU_POWERPC_POWER7_MASK        = 0xFFFF0000,
     CPU_POWERPC_POWER7_v23         = 0x003F0203,
     CPU_POWERPC_POWER7P_BASE       = 0x004A0000,
-    CPU_POWERPC_POWER7P_MASK       = 0xFFFF0000,
     CPU_POWERPC_POWER7P_v21        = 0x004A0201,
     CPU_POWERPC_POWER8E_BASE       = 0x004B0000,
-    CPU_POWERPC_POWER8E_MASK       = 0xFFFF0000,
     CPU_POWERPC_POWER8E_v10        = 0x004B0100,
     CPU_POWERPC_POWER8_BASE        = 0x004D0000,
-    CPU_POWERPC_POWER8_MASK        = 0xFFFF0000,
     CPU_POWERPC_POWER8_v10         = 0x004D0100,
     CPU_POWERPC_970                = 0x00390202,
     CPU_POWERPC_970FX_v10          = 0x00391100,
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index f1f0a52..0fee36f 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -56,7 +56,7 @@ typedef struct PowerPCCPUClass {
     void (*parent_reset)(CPUState *cpu);
 
     uint32_t pvr;
-    uint32_t pvr_mask;
+    bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr);
     uint64_t pcr_mask;
     uint32_t svr;
     uint64_t insns_flags;
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 2ab2810..f1684ba 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8062,6 +8062,17 @@ static void init_proc_POWER7 (CPUPPCState *env)
     init_proc_book3s_64(env, BOOK3S_CPU_POWER7);
 }
 
+static bool ppc_pvr_match_power7(PowerPCCPUClass *pcc, uint32_t pvr)
+{
+    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7P_BASE) {
+        return true;
+    }
+    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7_BASE) {
+        return true;
+    }
+    return false;
+}
+
 POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -8070,8 +8081,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->pvr_match = ppc_pvr_match_power7;
     pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
     pcc->init_proc = init_proc_POWER7;
     pcc->check_pow = check_pow_nocheck;
@@ -8131,8 +8141,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->pvr_match = ppc_pvr_match_power7;
     pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
     pcc->init_proc = init_proc_POWER7;
     pcc->check_pow = check_pow_nocheck;
@@ -8189,6 +8198,17 @@ static void init_proc_POWER8(CPUPPCState *env)
     init_proc_book3s_64(env, BOOK3S_CPU_POWER8);
 }
 
+static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr)
+{
+    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8E_BASE) {
+        return true;
+    }
+    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8_BASE) {
+        return true;
+    }
+    return false;
+}
+
 POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -8197,8 +8217,7 @@ POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
     dc->fw_name = "PowerPC,POWER8";
     dc->desc = "POWER8E";
     dc->props = powerpc_servercpu_properties;
-    pcc->pvr = CPU_POWERPC_POWER8E_BASE;
-    pcc->pvr_mask = CPU_POWERPC_POWER8E_MASK;
+    pcc->pvr_match = ppc_pvr_match_power8;
     pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
     pcc->init_proc = init_proc_POWER8;
     pcc->check_pow = check_pow_nocheck;
@@ -8256,13 +8275,10 @@ POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
 POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
-    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
 
     ppc_POWER8E_cpu_family_class_init(oc, data);
 
     dc->desc = "POWER8";
-    pcc->pvr = CPU_POWERPC_POWER8_BASE;
-    pcc->pvr_mask = CPU_POWERPC_POWER8_MASK;
 }
 #endif /* defined (TARGET_PPC64) */
 
@@ -9245,7 +9261,6 @@ static gint ppc_cpu_compare_class_pvr_mask(gconstpointer a, gconstpointer b)
     ObjectClass *oc = (ObjectClass *)a;
     uint32_t pvr = *(uint32_t *)b;
     PowerPCCPUClass *pcc = (PowerPCCPUClass *)a;
-    gint ret;
 
     /* -cpu host does a PVR lookup during construction */
     if (unlikely(strcmp(object_class_get_name(oc),
@@ -9257,9 +9272,11 @@ static gint ppc_cpu_compare_class_pvr_mask(gconstpointer a, gconstpointer b)
         return -1;
     }
 
-    ret = (((pcc->pvr & pcc->pvr_mask) == (pvr & pcc->pvr_mask)) ? 0 : -1);
+    if (pcc->pvr_match(pcc, pvr)) {
+        return 0;
+    }
 
-    return ret;
+    return -1;
 }
 
 PowerPCCPUClass *ppc_cpu_class_by_pvr_mask(uint32_t pvr)
@@ -9656,6 +9673,11 @@ static void ppc_cpu_initfn(Object *obj)
     }
 }
 
+static bool ppc_pvr_match_default(PowerPCCPUClass *pcc, uint32_t pvr)
+{
+    return pcc->pvr == pvr;
+}
+
 static void ppc_cpu_class_init(ObjectClass *oc, void *data)
 {
     PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
@@ -9663,8 +9685,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
 
     pcc->parent_realize = dc->realize;
-    pcc->pvr = CPU_POWERPC_DEFAULT_MASK;
-    pcc->pvr_mask = CPU_POWERPC_DEFAULT_MASK;
+    pcc->pvr_match = ppc_pvr_match_default;
     pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_always;
     dc->realize = ppc_cpu_realizefn;
     dc->unrealize = ppc_cpu_unrealizefn;
-- 
2.0.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH v2] target-ppc: Add pvr_match() callback
  2014-07-03 13:32     ` [Qemu-devel] [PATCH v2] " Alexey Kardashevskiy
@ 2014-07-03 13:54       ` Alexander Graf
  2014-07-03 14:48         ` [Qemu-devel] [PATCH v3] " Alexey Kardashevskiy
  0 siblings, 1 reply; 9+ messages in thread
From: Alexander Graf @ 2014-07-03 13:54 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel; +Cc: qemu-ppc


On 03.07.14 15:32, Alexey Kardashevskiy wrote:
> So far it was enough to have a base PVR value and mask per CPU
> family such as POWER7 or POWER8. However there CPUs which are
> completely architecturally compatible but have different PVRs such
> as POWER7/POWER7+ and POWER8/POWER8E. For these CPUs, top 16 bits
> are CPU family and low 16 bits are the version. The families have
> PVR base values different enough so defining a mask which
> would cover both (or potentially more) CPUs within the family is
> not possible.
>
> This adds a pvr_match() callback to PowerPCCPUClass. The default
> handler simply compares PVR defined in the class.
>
> This implements ppc_pvr_match_power7/ppc_pvr_match_power8 callbacks
> for POWER7/8 families. These check for POWER7/POWER7+ and POWER8/POWER8E.
>
> This changes ppc_cpu_compare_class_pvr_mask() not to check masks but
> use the pvr_match() callback.
>
> Since all server CPUs use the same mask, this defines one mask
> value - CPU_POWERPC_POWER_SERVER_MASK - which is used everywhere now.
> This removes other mask definitions.
>
> This removes pvr_mask from PowerPCCPUClass as it is not used anymore.
> This removes pvr initialization for POWER7/8 families as it is not used
> to find the class, the pvr_match() callback is used instead.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> Changes:
> v2:
> * do not check pvr_match==NULL as it is always define for PPC
> ---
>   target-ppc/cpu-models.c     |  1 -
>   target-ppc/cpu-models.h     |  5 +----
>   target-ppc/cpu-qom.h        |  2 +-
>   target-ppc/translate_init.c | 49 ++++++++++++++++++++++++++++++++-------------
>   4 files changed, 37 insertions(+), 20 deletions(-)
>
> diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
> index 9a91af9..c9112e9 100644
> --- a/target-ppc/cpu-models.c
> +++ b/target-ppc/cpu-models.c
> @@ -44,7 +44,6 @@
>           PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);                       \
>                                                                               \
>           pcc->pvr          = _pvr;                                           \
> -        pcc->pvr_mask     = CPU_POWERPC_DEFAULT_MASK;                       \

I still don't see you removing the definition of CPU_POWERPC_DEFAULT_MASK.


Alex

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Qemu-devel] [PATCH v3] target-ppc: Add pvr_match() callback
  2014-07-03 13:54       ` Alexander Graf
@ 2014-07-03 14:48         ` Alexey Kardashevskiy
  0 siblings, 0 replies; 9+ messages in thread
From: Alexey Kardashevskiy @ 2014-07-03 14:48 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, qemu-ppc, Alexander Graf

So far it was enough to have a base PVR value and mask per CPU
family such as POWER7 or POWER8. However there CPUs which are
completely architecturally compatible but have different PVRs such
as POWER7/POWER7+ and POWER8/POWER8E. For these CPUs, top 16 bits
are CPU family and low 16 bits are the version. The families have
PVR base values different enough so defining a mask which
would cover both (or potentially more) CPUs within the family is
not possible.

This adds a pvr_match() callback to PowerPCCPUClass. The default
handler simply compares PVR defined in the class.

This implements ppc_pvr_match_power7/ppc_pvr_match_power8 callbacks
for POWER7/8 families. These check for POWER7/POWER7+ and POWER8/POWER8E.

This changes ppc_cpu_compare_class_pvr_mask() not to check masks but
use the pvr_match() callback.

Since all server CPUs use the same mask, this defines one mask
value - CPU_POWERPC_POWER_SERVER_MASK - which is used everywhere now.
This removes other mask definitions.

This removes pvr_mask from PowerPCCPUClass as it is not used anymore.
This removes pvr initialization for POWER7/8 families as it is not used
to find the class, the pvr_match() callback is used instead.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v3:
* removed CPU_POWERPC_DEFAULT_MASK (sorry, overlooked this comment)

v2:
* do not check pvr_match==NULL as it is always define for PPC
---
 target-ppc/cpu-models.c     |  1 -
 target-ppc/cpu-models.h     |  6 +-----
 target-ppc/cpu-qom.h        |  2 +-
 target-ppc/translate_init.c | 49 ++++++++++++++++++++++++++++++++-------------
 4 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
index 9a91af9..c9112e9 100644
--- a/target-ppc/cpu-models.c
+++ b/target-ppc/cpu-models.c
@@ -44,7 +44,6 @@
         PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);                       \
                                                                             \
         pcc->pvr          = _pvr;                                           \
-        pcc->pvr_mask     = CPU_POWERPC_DEFAULT_MASK;                       \
         pcc->svr          = _svr;                                           \
         dc->desc          = _desc;                                          \
     }                                                                       \
diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
index c39d03a..290a759 100644
--- a/target-ppc/cpu-models.h
+++ b/target-ppc/cpu-models.h
@@ -39,7 +39,6 @@ extern PowerPCCPUAlias ppc_cpu_aliases[];
 /*****************************************************************************/
 /* PVR definitions for most known PowerPC                                    */
 enum {
-    CPU_POWERPC_DEFAULT_MASK       = 0xFFFFFFFF,
     /* PowerPC 401 family */
     /* Generic PowerPC 401 */
 #define CPU_POWERPC_401              CPU_POWERPC_401G2
@@ -553,17 +552,14 @@ enum {
     CPU_POWERPC_POWER6             = 0x003E0000,
     CPU_POWERPC_POWER6_5           = 0x0F000001, /* POWER6 in POWER5 mode */
     CPU_POWERPC_POWER6A            = 0x0F000002,
+    CPU_POWERPC_POWER_SERVER_MASK  = 0xFFFF0000,
     CPU_POWERPC_POWER7_BASE        = 0x003F0000,
-    CPU_POWERPC_POWER7_MASK        = 0xFFFF0000,
     CPU_POWERPC_POWER7_v23         = 0x003F0203,
     CPU_POWERPC_POWER7P_BASE       = 0x004A0000,
-    CPU_POWERPC_POWER7P_MASK       = 0xFFFF0000,
     CPU_POWERPC_POWER7P_v21        = 0x004A0201,
     CPU_POWERPC_POWER8E_BASE       = 0x004B0000,
-    CPU_POWERPC_POWER8E_MASK       = 0xFFFF0000,
     CPU_POWERPC_POWER8E_v10        = 0x004B0100,
     CPU_POWERPC_POWER8_BASE        = 0x004D0000,
-    CPU_POWERPC_POWER8_MASK        = 0xFFFF0000,
     CPU_POWERPC_POWER8_v10         = 0x004D0100,
     CPU_POWERPC_970                = 0x00390202,
     CPU_POWERPC_970FX_v10          = 0x00391100,
diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
index f1f0a52..0fee36f 100644
--- a/target-ppc/cpu-qom.h
+++ b/target-ppc/cpu-qom.h
@@ -56,7 +56,7 @@ typedef struct PowerPCCPUClass {
     void (*parent_reset)(CPUState *cpu);
 
     uint32_t pvr;
-    uint32_t pvr_mask;
+    bool (*pvr_match)(struct PowerPCCPUClass *pcc, uint32_t pvr);
     uint64_t pcr_mask;
     uint32_t svr;
     uint64_t insns_flags;
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 2ab2810..f1684ba 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -8062,6 +8062,17 @@ static void init_proc_POWER7 (CPUPPCState *env)
     init_proc_book3s_64(env, BOOK3S_CPU_POWER7);
 }
 
+static bool ppc_pvr_match_power7(PowerPCCPUClass *pcc, uint32_t pvr)
+{
+    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7P_BASE) {
+        return true;
+    }
+    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7_BASE) {
+        return true;
+    }
+    return false;
+}
+
 POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -8070,8 +8081,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->pvr_match = ppc_pvr_match_power7;
     pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
     pcc->init_proc = init_proc_POWER7;
     pcc->check_pow = check_pow_nocheck;
@@ -8131,8 +8141,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->pvr_match = ppc_pvr_match_power7;
     pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
     pcc->init_proc = init_proc_POWER7;
     pcc->check_pow = check_pow_nocheck;
@@ -8189,6 +8198,17 @@ static void init_proc_POWER8(CPUPPCState *env)
     init_proc_book3s_64(env, BOOK3S_CPU_POWER8);
 }
 
+static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr)
+{
+    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8E_BASE) {
+        return true;
+    }
+    if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8_BASE) {
+        return true;
+    }
+    return false;
+}
+
 POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
@@ -8197,8 +8217,7 @@ POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
     dc->fw_name = "PowerPC,POWER8";
     dc->desc = "POWER8E";
     dc->props = powerpc_servercpu_properties;
-    pcc->pvr = CPU_POWERPC_POWER8E_BASE;
-    pcc->pvr_mask = CPU_POWERPC_POWER8E_MASK;
+    pcc->pvr_match = ppc_pvr_match_power8;
     pcc->pcr_mask = PCR_COMPAT_2_05 | PCR_COMPAT_2_06;
     pcc->init_proc = init_proc_POWER8;
     pcc->check_pow = check_pow_nocheck;
@@ -8256,13 +8275,10 @@ POWERPC_FAMILY(POWER8E)(ObjectClass *oc, void *data)
 POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(oc);
-    PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
 
     ppc_POWER8E_cpu_family_class_init(oc, data);
 
     dc->desc = "POWER8";
-    pcc->pvr = CPU_POWERPC_POWER8_BASE;
-    pcc->pvr_mask = CPU_POWERPC_POWER8_MASK;
 }
 #endif /* defined (TARGET_PPC64) */
 
@@ -9245,7 +9261,6 @@ static gint ppc_cpu_compare_class_pvr_mask(gconstpointer a, gconstpointer b)
     ObjectClass *oc = (ObjectClass *)a;
     uint32_t pvr = *(uint32_t *)b;
     PowerPCCPUClass *pcc = (PowerPCCPUClass *)a;
-    gint ret;
 
     /* -cpu host does a PVR lookup during construction */
     if (unlikely(strcmp(object_class_get_name(oc),
@@ -9257,9 +9272,11 @@ static gint ppc_cpu_compare_class_pvr_mask(gconstpointer a, gconstpointer b)
         return -1;
     }
 
-    ret = (((pcc->pvr & pcc->pvr_mask) == (pvr & pcc->pvr_mask)) ? 0 : -1);
+    if (pcc->pvr_match(pcc, pvr)) {
+        return 0;
+    }
 
-    return ret;
+    return -1;
 }
 
 PowerPCCPUClass *ppc_cpu_class_by_pvr_mask(uint32_t pvr)
@@ -9656,6 +9673,11 @@ static void ppc_cpu_initfn(Object *obj)
     }
 }
 
+static bool ppc_pvr_match_default(PowerPCCPUClass *pcc, uint32_t pvr)
+{
+    return pcc->pvr == pvr;
+}
+
 static void ppc_cpu_class_init(ObjectClass *oc, void *data)
 {
     PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
@@ -9663,8 +9685,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
 
     pcc->parent_realize = dc->realize;
-    pcc->pvr = CPU_POWERPC_DEFAULT_MASK;
-    pcc->pvr_mask = CPU_POWERPC_DEFAULT_MASK;
+    pcc->pvr_match = ppc_pvr_match_default;
     pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_always;
     dc->realize = ppc_cpu_realizefn;
     dc->unrealize = ppc_cpu_unrealizefn;
-- 
2.0.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] target-ppc: Merge POWER7+ to POWER7 and POWER8E to POWER8
  2014-06-30 14:30 [Qemu-devel] [PATCH 0/2] target-ppc: Merge POWER7+ to POWER7 and POWER8E to POWER8 Alexey Kardashevskiy
                   ` (2 preceding siblings ...)
  2014-07-03 12:46 ` [Qemu-devel] [PATCH 0/2] target-ppc: Merge POWER7+ to POWER7 and POWER8E to POWER8 Alexander Graf
@ 2014-07-03 14:53 ` Alexander Graf
  3 siblings, 0 replies; 9+ messages in thread
From: Alexander Graf @ 2014-07-03 14:53 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel; +Cc: qemu-ppc


On 30.06.14 16:30, Alexey Kardashevskiy wrote:
> Here are 2 patches to simplify handling of the same CPUs with slightly different PVRs.
> Now with a callback instead of additional levels in PPC CPU QOM class tree.
>
> Please comment. Thanks.

Thanks, applied to ppc-next (2.1)


Alex

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-07-03 14:53 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-30 14:30 [Qemu-devel] [PATCH 0/2] target-ppc: Merge POWER7+ to POWER7 and POWER8E to POWER8 Alexey Kardashevskiy
2014-06-30 14:30 ` [Qemu-devel] [PATCH 1/2] target-ppc: Add pvr_match() callback Alexey Kardashevskiy
2014-07-03 12:44   ` Alexander Graf
2014-07-03 13:32     ` [Qemu-devel] [PATCH v2] " Alexey Kardashevskiy
2014-07-03 13:54       ` Alexander Graf
2014-07-03 14:48         ` [Qemu-devel] [PATCH v3] " Alexey Kardashevskiy
2014-06-30 14:30 ` [Qemu-devel] [PATCH 2/2] target-ppc: Remove POWER7+ and POWER8E families Alexey Kardashevskiy
2014-07-03 12:46 ` [Qemu-devel] [PATCH 0/2] target-ppc: Merge POWER7+ to POWER7 and POWER8E to POWER8 Alexander Graf
2014-07-03 14:53 ` Alexander Graf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.