Re: [Qemu-ppc] [Qemu-ppc for-2.10] [PATCH V3 3/5] target/ppc: Adapt tlbie[l] for ISAv3.00 Support

[Suraj Jitindar Singh]

On Thu, 2017-03-30 at 16:01 +1100, David Gibson wrote:
> On Wed, Mar 29, 2017 at 04:43:47PM +1100, Suraj Jitindar Singh wrote:
> > 
> > The tlbie[l] instructions are used to invalidate TLB entries used
> > to cache
> > address translations.
> > 
> > ISAv3.00 changes these instructions to take additional arguments to
> > change
> > the behaviour of the instruction which allow the caller to limit
> > what is
> > invalidated. Since the majority of things which a real POWER9
> > processor
> > will cache aren't cached in QEMU for tcg, in fact the only thing
> > QEMU
> > actually caches is the effective to real address translation (or
> > the ERAT),
> > we don't actually really care about these new instruction argument
> > fields.
> > 
> > So adapt the handler gen functions to accept the new instuction
> > argument
> > fields so an illegal instruction exception isn't raised when they
> > are
> > called. Note that we ignore the new arguments anyway and simply
> > call the
> > same handlers as before, this is fine since they just invalidate
> > the entire
> > TLB anyway which is the only translation related cache which QEMU
> > keeps.
> > 
> > Signed-off-by: Suraj Jitindar Singh <address@hidden>
> > Reviewed-by: David Gibson <address@hidden>
> I know I reviewed this before, but I think this is not quite right.
> It does the right thing for POWER9, since those extra bits are
> ignored.  However, AFAICT this will also enable them for POWER8 and
> earlier, which doesn't seem right.  Seems like an explicit test for
> the reserved fields triggering a 0x700 on pre-POWER9 belongs with
> this.

Correct, I'll fix this up.

> 
> > 
> > ---
> >  target/ppc/translate.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> > index f40b5a1..c3abb40 100644
> > --- a/target/ppc/translate.c
> > +++ b/target/ppc/translate.c
> > @@ -6545,8 +6545,8 @@ GEN_HANDLER2(slbfee_, "slbfee.", 0x1F, 0x13,
> > 0x1E, 0x001F0000, PPC_SEGMENT_64B),
> >  GEN_HANDLER(tlbia, 0x1F, 0x12, 0x0B, 0x03FFFC01, PPC_MEM_TLBIA),
> >  /* XXX Those instructions will need to be handled differently for
> >   * different ISA versions */
> > -GEN_HANDLER(tlbiel, 0x1F, 0x12, 0x08, 0x001F0001, PPC_MEM_TLBIE),
> > -GEN_HANDLER(tlbie, 0x1F, 0x12, 0x09, 0x001F0001, PPC_MEM_TLBIE),
> > +GEN_HANDLER(tlbiel, 0x1F, 0x12, 0x08, 0x00100001, PPC_MEM_TLBIE),
> > +GEN_HANDLER(tlbie, 0x1F, 0x12, 0x09, 0x00100001, PPC_MEM_TLBIE),
> >  GEN_HANDLER(tlbsync, 0x1F, 0x16, 0x11, 0x03FFF801,
> > PPC_MEM_TLBSYNC),
> >  #if defined(TARGET_PPC64)
> >  GEN_HANDLER(slbia, 0x1F, 0x12, 0x0F, 0x031FFC01, PPC_SLBI),