Re: [Qemu-devel] [RFC PATCH] powerpc: add PVR mask support

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Thu, 2013-08-15 at 07:21 +0200, Alexander Graf wrote:
> 
> Am 15.08.2013 um 05:35 schrieb Alexey Kardashevskiy <aik@ozlabs.ru>:
> 
> > 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. POWER7, POWER7+ and POWER8 CPUs
> > definitions converted to use masks.
> 
> How does the user select that he wants a v2.3 p7 cpu with this patch?

It's a tradeoff. We are trading a useless feature (which you describe
above) for a functional system that doesn't require to have an entry for
every possible chip revision ever designed and to be designed in the
future for things to work.

I know the qemu folks is generally uninterested in making things
actually work but heh .. :-)

Cheers,
Ben.

> Alex
> 
> > 
> > Cc: Andreas Färber <afaerber@suse.de>
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> > 
> > The patch does basically what the kernel does in arch/powerpc/kernel/cputable.c.
> > 
> > ---
> > target-ppc/cpu-models.c     | 26 +++++++++++++++-----------
> > target-ppc/cpu-models.h     | 12 +++++++-----
> > target-ppc/cpu-qom.h        |  1 +
> > target-ppc/translate_init.c |  2 +-
> > 4 files changed, 24 insertions(+), 17 deletions(-)
> > 
> > diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
> > index 8dea560..c40cf04 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)    \
> >     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;                                          \
> >     }                                                                       \
> > @@ -66,9 +67,16 @@
> >     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)
> > +
> > #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)
> > +
> >     /* Embedded PowerPC                                                      */
> >     /* PowerPC 401 family                                                    */
> >     POWERPC_DEF("401",           CPU_POWERPC_401,                    401,
> > @@ -1133,16 +1141,12 @@
> >     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_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,
> > diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
> > index d9145d1..b322063 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
> > @@ -552,11 +553,12 @@ enum {
> >     CPU_POWERPC_POWER6             = 0x003E0000,
> >     CPU_POWERPC_POWER6_5           = 0x0F000001, /* POWER6 in POWER5 mode */
> >     CPU_POWERPC_POWER6A            = 0x0F000002,
> > -    CPU_POWERPC_POWER7_v20         = 0x003F0200,
> > -    CPU_POWERPC_POWER7_v21         = 0x003F0201,
> > -    CPU_POWERPC_POWER7_v23         = 0x003F0203,
> > -    CPU_POWERPC_POWER7P_v21        = 0x004A0201,
> > -    CPU_POWERPC_POWER8_v10         = 0x004B0100,
> > +    CPU_POWERPC_POWER7             = 0x003F0000,
> > +    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
> >