Re: [Qemu-devel] [RFC PATCH 04/17] target/ppc/POWER9: Add ISAv3.00 MMU definition

[Suraj Jitindar Singh]

On Tue, 2017-01-17 at 08:36 +1100, David Gibson wrote:
> On Fri, Jan 13, 2017 at 05:28:10PM +1100, Suraj Jitindar Singh wrote:
> > 
> > POWER9 processors implement the mmu as defined in version 3.00 of
> > the ISA.
> > 
> > Add a definition for this mmu model and set the POWER9 cpu model to
> > use
> > this mmu model.
> > 
> > Signed-off-by: Suraj Jitindar Singh <address@hidden>
> > ---
> >  target/ppc/cpu-qom.h        | 5 ++++-
> >  target/ppc/mmu_helper.c     | 2 ++
> >  target/ppc/translate_init.c | 3 +--
> >  3 files changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> > index d46c31a..1577cc8 100644
> > --- a/target/ppc/cpu-qom.h
> > +++ b/target/ppc/cpu-qom.h
> > @@ -86,10 +86,13 @@ enum powerpc_mmu_t {
> >      POWERPC_MMU_2_07       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
> >                               | POWERPC_MMU_64K
> >                               | POWERPC_MMU_AMR | 0x00000004,
> > -    /* FIXME Add POWERPC_MMU_3_OO defines */
> >      /* Architecture 2.07 "degraded" (no 1T segments)           */
> >      POWERPC_MMU_2_07a      = POWERPC_MMU_64 | POWERPC_MMU_AMR
> >                               | 0x00000004,
> > +    /* Architecture 3.00 variant                               */
> > +    POWERPC_MMU_3_00       = POWERPC_MMU_64 | POWERPC_MMU_1TSEG
> > +                             | POWERPC_MMU_64K
> > +                             | POWERPC_MMU_AMR | 0x00000005,
> Hmm.  I guess it works for now, but I'm not really sure that having
> this include POWERPC_MMU_64 is a great idea.  The name is kind of
> misleading, but I'm pretty sure a number of places assume that the
> POWERPC_MMU_64 bitindicates a 64-bit *hash* MMU, which is no longer
> really the case.
Good catch, I didn't realise this assumption was made in the code.

There are some cases where this bit needs to be set for correct
behaviour but other places where, while it doesn't break anything, it
leads to incorrect code being called.

Time for me to investigate :)
> 
> > 
> >  };
> >  
> >  /*****************************************************************
> > ************/
> > diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> > index d09fc0a..2ab4562 100644
> > --- a/target/ppc/mmu_helper.c
> > +++ b/target/ppc/mmu_helper.c
> > @@ -1935,6 +1935,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
> >      case POWERPC_MMU_2_06a:
> >      case POWERPC_MMU_2_07:
> >      case POWERPC_MMU_2_07a:
> > +    case POWERPC_MMU_3_00:
> >  #endif /* defined(TARGET_PPC64) */
> >          env->tlb_need_flush = 0;
> >          tlb_flush(CPU(cpu), 1);
> > @@ -1974,6 +1975,7 @@ void ppc_tlb_invalidate_one(CPUPPCState *env,
> > target_ulong addr)
> >      case POWERPC_MMU_2_06a:
> >      case POWERPC_MMU_2_07:
> >      case POWERPC_MMU_2_07a:
> > +    case POWERPC_MMU_3_00:
> >          /* tlbie invalidate TLBs for all segments */
> >          /* XXX: given the fact that there are too many segments to
> > invalidate,
> >           *      and we still don't have a tlb_flush_mask(env, n,
> > mask) in QEMU,
> > diff --git a/target/ppc/translate_init.c
> > b/target/ppc/translate_init.c
> > index bfc1f24..2402eef 100644
> > --- a/target/ppc/translate_init.c
> > +++ b/target/ppc/translate_init.c
> > @@ -8838,8 +8838,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void
> > *data)
> >                      (1ull << MSR_PMM) |
> >                      (1ull << MSR_RI) |
> >                      (1ull << MSR_LE);
> > -    /* Using 2.07 defines until new radix model is added. */
> > -    pcc->mmu_model = POWERPC_MMU_2_07;
> > +    pcc->mmu_model = POWERPC_MMU_3_00;
> >  #if defined(CONFIG_SOFTMMU)
> >      pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
> >      /* segment page size remain the same */