Re: [Qemu-ppc] [_R_F_C_ PATCH v2] powerpc: add PVR mask support

[Alexander Graf]

On 15.08.2013, at 08:43, Alexey Kardashevskiy wrote:

> IBM POWERPC processors encode PVR as a CPU family in higher 16 bits and
> a CPU version in lower 16 bits. Since there is no significant change
> in behavior between versions, there is no point to add every single CPU
> version in QEMU's CPU list. Also, new CPU versions of already supported
> CPU won't break the existing code.
> 
> This adds a PVR mask support which means that aliases are replaced with
> another layer in POWERPC CPU class hierarchy. The patch adds intermediate
> POWER7, POWER7+ and POWER8 CPU classes and makes use of those in
> specific versioned POWERPC CPUs.

A lot better :)

> 
> Cc: Andreas Färber <address@hidden>
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> 
> ---
> Changes:
> v2:
> * aliases are replaced with another level in class hierarchy
> ---
> target-ppc/cpu-models.c     | 46 ++++++++++++++++++++++++++++++---------------
> target-ppc/cpu-models.h     |  7 +++++++
> target-ppc/cpu-qom.h        |  1 +
> target-ppc/translate_init.c |  2 +-
> 4 files changed, 40 insertions(+), 16 deletions(-)
> 
> diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
> index 8dea560..dfa6673 100644
> --- a/target-ppc/cpu-models.c
> +++ b/target-ppc/cpu-models.c
> @@ -35,7 +35,7 @@
> /* PowerPC CPU definitions                                                 */
> #define POWERPC_DEF_PREFIX(pvr, svr, type)                                  \
>     glue(glue(glue(glue(pvr, _), svr), _), type)
> -#define POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type)                    \
> +#define POWERPC_DEF_SVR_MASK(_name, _desc, _pvr, _pvr_mask, _svr, _type, 
> _parent)    \
>     static void                                                             \
>     glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_class_init)            \
>     (ObjectClass *oc, void *data)                                           \
> @@ -44,6 +44,7 @@
>         PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);                       \
>                                                                             \
>         pcc->pvr          = _pvr;                                           \
> +        pcc->pvr_mask     = _pvr_mask;                                      \
>         pcc->svr          = _svr;                                           \
>         dc->desc          = _desc;                                          \
>     }                                                                       \
> @@ -51,7 +52,7 @@
>     static const TypeInfo                                                   \
>     glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_type_info) = {         \
>         .name       = _name "-" TYPE_POWERPC_CPU,                           \
> -        .parent     = stringify(_type) "-family-" TYPE_POWERPC_CPU,         \
> +        .parent     = _parent,                                              \
>         .class_init =                                                       \
>             glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_class_init),   \
>     };                                                                      \
> @@ -66,9 +67,21 @@
>     type_init(                                                              \
>         glue(POWERPC_DEF_PREFIX(_pvr, _svr, _type), _cpu_register_types))
> 
> +#define POWERPC_DEF_SVR(_name, _desc, _pvr, _svr, _type)                    \
> +    POWERPC_DEF_SVR_MASK(_name, _desc, _pvr, CPU_POWERPC_DEFAULT_MASK,      \
> +                         _svr, _type, stringify(_type) "-family-" 
> TYPE_POWERPC_CPU)
> +
> #define POWERPC_DEF(_name, _pvr, _type, _desc)                              \
>     POWERPC_DEF_SVR(_name, _desc, _pvr, POWERPC_SVR_NONE, _type)
> 
> +#define POWERPC_DEF_MASK(_name, _pvr, _pvr_mask, _type, _desc)              \
> +    POWERPC_DEF_SVR_MASK(_name, _desc, _pvr, _pvr_mask, POWERPC_SVR_NONE, 
> _type, \
> +                        stringify(_type) "-family-" TYPE_POWERPC_CPU)
> +
> +#define POWERPC_DEF_2ND(_name, _pvr, _type, _desc, _parent)                 \
> +    POWERPC_DEF_SVR_MASK(_name, _desc, _pvr, CPU_POWERPC_DEFAULT_MASK,      \
> +                            POWERPC_SVR_NONE, _type, _parent)
> +
>     /* Embedded PowerPC                                                      
> */
>     /* PowerPC 401 family                                                    
> */
>     POWERPC_DEF("401",           CPU_POWERPC_401,                    401,
> @@ -1133,16 +1146,22 @@
>     POWERPC_DEF("POWER6A",       CPU_POWERPC_POWER6A,                POWER6,
>                 "POWER6A")
> #endif
> -    POWERPC_DEF("POWER7_v2.0",   CPU_POWERPC_POWER7_v20,             POWER7,
> -                "POWER7 v2.0")
> -    POWERPC_DEF("POWER7_v2.1",   CPU_POWERPC_POWER7_v21,             POWER7,
> -                "POWER7 v2.1")
> -    POWERPC_DEF("POWER7_v2.3",   CPU_POWERPC_POWER7_v23,             POWER7,
> -                "POWER7 v2.3")
> -    POWERPC_DEF("POWER7+_v2.1",  CPU_POWERPC_POWER7P_v21,            POWER7,
> -                "POWER7+ v2.1")
> -    POWERPC_DEF("POWER8_v1.0",   CPU_POWERPC_POWER8_v10,             POWER8,
> -                "POWER8 v1.0")
> +    POWERPC_DEF_2ND("POWER7_v2.0",   CPU_POWERPC_POWER7_v20,             
> POWER7,
> +                "POWER7 v2.0", "POWER7-" TYPE_POWERPC_CPU)
> +    POWERPC_DEF_2ND("POWER7_v2.1",   CPU_POWERPC_POWER7_v21,             
> POWER7,
> +                "POWER7 v2.1", "POWER7-" TYPE_POWERPC_CPU)
> +    POWERPC_DEF_2ND("POWER7_v2.3",   CPU_POWERPC_POWER7_v23,             
> POWER7,
> +                "POWER7 v2.3", "POWER7-" TYPE_POWERPC_CPU)
> +    POWERPC_DEF_2ND("POWER7+_v2.1",  CPU_POWERPC_POWER7P_v21,            
> POWER7,
> +                "POWER7+ v2.1", "POWER7+-" TYPE_POWERPC_CPU)
> +    POWERPC_DEF_2ND("POWER8_v1.0",   CPU_POWERPC_POWER8_v10,             
> POWER8,
> +                "POWER8 v1.0", "POWER8-" TYPE_POWERPC_CPU)
> +    POWERPC_DEF_MASK("POWER7", CPU_POWERPC_POWER7, CPU_POWERPC_POWER7_MASK,
> +                     POWER7, "POWER7")
> +    POWERPC_DEF_MASK("POWER7+", CPU_POWERPC_POWER7P, 
> CPU_POWERPC_POWER7P_MASK,
> +                     POWER7, "POWER7")
> +    POWERPC_DEF_MASK("POWER8", CPU_POWERPC_POWER8, CPU_POWERPC_POWER8_MASK,
> +                     POWER8, "POWER8")
>     POWERPC_DEF("970",           CPU_POWERPC_970,                    970,
>                 "PowerPC 970")
>     POWERPC_DEF("970fx_v1.0",    CPU_POWERPC_970FX_v10,              970FX,
> @@ -1389,9 +1408,6 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
>     { "POWER3+", "631" },
>     { "POWER5gr", "POWER5" },
>     { "POWER5gs", "POWER5+" },
> -    { "POWER7", "POWER7_v2.3" },
> -    { "POWER7+", "POWER7+_v2.1" },
> -    { "POWER8", "POWER8_v1.0" },

So here we had explicit versions for -cpu POWERx

>     { "970fx", "970fx_v3.1" },
>     { "970mp", "970mp_v1.1" },
>     { "Apache", "RS64" },
> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
> index d9145d1..2233053 100644
> --- a/target-ppc/cpu-models.h
> +++ b/target-ppc/cpu-models.h
> @@ -39,6 +39,7 @@ 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
> @@ -557,6 +558,12 @@ enum {
>     CPU_POWERPC_POWER7_v23         = 0x003F0203,
>     CPU_POWERPC_POWER7P_v21        = 0x004A0201,
>     CPU_POWERPC_POWER8_v10         = 0x004B0100,
> +    CPU_POWERPC_POWER7             = 0x003F0000,

But because we define this as the default PVR value we lose those. So when the 
user just says -cpu POWER7 he will get a POWER7 v0.0 chip rather than a POWER7 
v2.3 chip, which guests might not like.

I'm not sure what the best way to solve this is. Maybe we shouldn't pass in 
CPU_POWERPC_POWER7 but instead CPU_POWERPC_POWER7_V23 and have the mask work 
its magic?


Alex

> +    CPU_POWERPC_POWER7_MASK        = 0xFFFF0000,
> +    CPU_POWERPC_POWER7P            = 0x004A0000,
> +    CPU_POWERPC_POWER7P_MASK       = 0xFFFF0000,
> +    CPU_POWERPC_POWER8             = 0x004B0000,
> +    CPU_POWERPC_POWER8_MASK        = 0xFFFF0000,
>     CPU_POWERPC_970                = 0x00390202,
>     CPU_POWERPC_970FX_v10          = 0x00391100,
>     CPU_POWERPC_970FX_v20          = 0x003C0200,
> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> index f3c710a..0ae8b09 100644
> --- a/target-ppc/cpu-qom.h
> +++ b/target-ppc/cpu-qom.h
> @@ -54,6 +54,7 @@ typedef struct PowerPCCPUClass {
>     void (*parent_reset)(CPUState *cpu);
> 
>     uint32_t pvr;
> +    uint32_t pvr_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 13b290c..01dffa6 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -8161,7 +8161,7 @@ static gint ppc_cpu_compare_class_pvr(gconstpointer a, 
> gconstpointer b)
>     }
> #endif
> 
> -    return pcc->pvr == pvr ? 0 : -1;
> +    return pcc->pvr == (pvr & pcc->pvr_mask) ? 0 : -1;
> }
> 
> PowerPCCPUClass *ppc_cpu_class_by_pvr(uint32_t pvr)
> -- 
> 1.8.3.2
>