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

[Suraj Jitindar Singh]

On Tue, 2017-01-17 at 11:33 +1100, Suraj Jitindar Singh wrote:
> 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 :)
Actually, having looked through the code, I think we want this set. As
far as I can tell it doesn't lead to anything we don't want happening.
> > 
> > 
> > > 
> > > 
> > >  };
> > >  
> > >  /***************************************************************
> > > **
> > > ************/
> > > 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 */