Re: [Qemu-ppc] [PATCH v4 02/29] target-ppc: Merge 970FX and 970MP into a single 970 class

[Alexander Graf]

On 06/03/2014 05:40 PM, Greg Kurz wrote:
> On Tue,  3 Jun 2014 19:27:37 +1000
> Alexey Kardashevskiy <address@hidden> wrote:
>
> > The differences between classes were:
> > 1. SLB size, was 32 for 970 and 64 for others, should be 64 for all;
> > 2. check_pow() callback, HID0 format is the same so should be the same
> > 0x01C00000 which means "deep nap", "doze" and "nap" bits set;
> > 3. LPCR - 970 does not have it but 970MP had one (by mistake).
> >
> > This fixes wrong differences and makes one 970 class.
> >
> > This fixes wrong registration of LPCR which is not present on 970.
> >
> > This does not copy MSR_SHV (Hypervisor State, HV) bit from 970FX to
> > 970 class as we do not emulate hypervisor in QEMU anyway.
> >
> > This does not remove check_pow_970FX now as it is still used by POWER5+
> > class, this will be addressed later.
> >
> > Signed-off-by: Alexey Kardashevskiy <address@hidden>
> > ---
> >   target-ppc/cpu-models.c     |  14 +--
> >   target-ppc/translate_init.c | 222 ++++----------------------------------------
> >   2 files changed, 23 insertions(+), 213 deletions(-)
> >
> > diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
> > index 9a66c03..97a81d8 100644
> > --- a/target-ppc/cpu-models.c
> > +++ b/target-ppc/cpu-models.c
> > @@ -1142,19 +1142,19 @@
> >                   "POWER8 v1.0")
> >       POWERPC_DEF("970",           CPU_POWERPC_970,                    970,
> >                   "PowerPC 970")
> > -    POWERPC_DEF("970fx_v1.0",    CPU_POWERPC_970FX_v10,              970FX,
> > +    POWERPC_DEF("970fx_v1.0",    CPU_POWERPC_970FX_v10,              970,
> >                   "PowerPC 970FX v1.0 (G5)")
> > -    POWERPC_DEF("970fx_v2.0",    CPU_POWERPC_970FX_v20,              970FX,
> > +    POWERPC_DEF("970fx_v2.0",    CPU_POWERPC_970FX_v20,              970,
> >                   "PowerPC 970FX v2.0 (G5)")
> > -    POWERPC_DEF("970fx_v2.1",    CPU_POWERPC_970FX_v21,              970FX,
> > +    POWERPC_DEF("970fx_v2.1",    CPU_POWERPC_970FX_v21,              970,
> >                   "PowerPC 970FX v2.1 (G5)")
> > -    POWERPC_DEF("970fx_v3.0",    CPU_POWERPC_970FX_v30,              970FX,
> > +    POWERPC_DEF("970fx_v3.0",    CPU_POWERPC_970FX_v30,              970,
> >                   "PowerPC 970FX v3.0 (G5)")
> > -    POWERPC_DEF("970fx_v3.1",    CPU_POWERPC_970FX_v31,              970FX,
> > +    POWERPC_DEF("970fx_v3.1",    CPU_POWERPC_970FX_v31,              970,
> >                   "PowerPC 970FX v3.1 (G5)")
> > -    POWERPC_DEF("970mp_v1.0",    CPU_POWERPC_970MP_v10,              970MP,
> > +    POWERPC_DEF("970mp_v1.0",    CPU_POWERPC_970MP_v10,              970,
> >                   "PowerPC 970MP v1.0")
> > -    POWERPC_DEF("970mp_v1.1",    CPU_POWERPC_970MP_v11,              970MP,
> > +    POWERPC_DEF("970mp_v1.1",    CPU_POWERPC_970MP_v11,              970,
> >                   "PowerPC 970MP v1.1")
> >   #if defined(TODO)
> >       POWERPC_DEF("Cell",          CPU_POWERPC_CELL,                   970,
> > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> > index fa137af..2f40d0d 100644
> > --- a/target-ppc/translate_init.c
> > +++ b/target-ppc/translate_init.c
> > @@ -7268,8 +7268,9 @@ POWERPC_FAMILY(e600)(ObjectClass *oc, void *data)
> >
> >   static int check_pow_970 (CPUPPCState *env)
> >   {
> > -    if (env->spr[SPR_HID0] & 0x00600000)
> > +    if (env->spr[SPR_HID0] & 0x01C00000) {
> What about killing magic numbers with something like:
>
> #define HID0_DEEPNAP    (1<<24)
> #define HID0_DOZE       (1<<23)
> #define HID0_NAP        (1<<22)

I like the idea. But IMHO this can easily come as a follow-up patch if 
that's the only nit on this patch set :).


Alex