Re: [PATCH v3 1/2] target/ppc: Move tlbie[l] to decode tree

[Leandro Lupori]

On 7/14/22 15:45, Daniel Henrique Barboza wrote:
> On 7/12/22 16:37, Leandro Lupori wrote:
> > Also decode RIC, PRS and R operands.
> >
> > Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
> > ---
> >   target/ppc/cpu_init.c                        |  4 +-
> >   target/ppc/insn32.decode                     |  8 ++
> >   target/ppc/translate.c                       | 64 +-------------
> >   target/ppc/translate/storage-ctrl-impl.c.inc | 87 ++++++++++++++++++++
> >   4 files changed, 99 insertions(+), 64 deletions(-)
> >   create mode 100644 target/ppc/translate/storage-ctrl-impl.c.inc
> >
> > diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> > index c16cb8dbe7..8d7e77f778 100644
> > --- a/target/ppc/cpu_init.c
> > +++ b/target/ppc/cpu_init.c
> > @@ -6368,7 +6368,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
> >                          PPC_FLOAT_EXT |
> >                          PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
> >                          PPC_MEM_SYNC | PPC_MEM_EIEIO |
> > -                       PPC_MEM_TLBSYNC |
> > +                       PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
> >                          PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
> >                          PPC_SEGMENT_64B | PPC_SLBI |
> >                          PPC_POPCNTB | PPC_POPCNTWD |
> > @@ -6585,7 +6585,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void 
> > *data)
> >                          PPC_FLOAT_EXT |
> >                          PPC_CACHE | PPC_CACHE_ICBI | PPC_CACHE_DCBZ |
> >                          PPC_MEM_SYNC | PPC_MEM_EIEIO |
> > -                       PPC_MEM_TLBSYNC |
> > +                       PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
> >                          PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
> >                          PPC_SEGMENT_64B | PPC_SLBI |
> >                          PPC_POPCNTB | PPC_POPCNTWD |
> > diff --git a/target/ppc/insn32.decode b/target/ppc/insn32.decode
> > index 6ea48d5163..2b985249b8 100644
> > --- a/target/ppc/insn32.decode
> > +++ b/target/ppc/insn32.decode
> > @@ -809,3 +809,11 @@ VMODSD          000100 ..... ..... ..... 
> > 11111001011    @VX
> >   VMODUD          000100 ..... ..... ..... 11011001011    @VX
> >   VMODSQ          000100 ..... ..... ..... 11100001011    @VX
> >   VMODUQ          000100 ..... ..... ..... 11000001011    @VX
> > +
> > +## TLB Management Instructions
> > +
> > +&X_tlbie        rb rs ric prs:bool r:bool
> > +@X_tlbie        ...... rs:5 - ric:2 prs:1 r:1 rb:5 .......... .     
> > &X_tlbie
>
> You're marking bit 11 as ignored but you're not marking 31 as ignored. 
> The way
> the argument patterns are made in this file seems to be either not mark the
> ignored bits (e.g. most of args from the start of the file) or mark all 
> ignore
> bits (e.g. @XL_S from RFEBB).
>
> I am being petty, yes. This makes no functional change in the 
> instruction, but
> I'd rather mark bit 31 as ignored in @X_tlbie as well.
>
> I did that in my tree and it seems to work fine. If you're ok with this 
> change,
>
>
>
> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

Right, the @X_tlbie pattern ended up inconsistent with regard to ignored 
bits. I'm ok with changing bit 31 of it to ignored.

Talking with the guys here, they've explained me that it is usually 
better to use '.' with format definitions ('@'), to make it easier to 
reuse them for more instructions, some of which may ignore a given bit 
while others may not. But for @X_tlbie it's ok to use dots or dashes for 
bits 11 and 31, as it's used only by TLBIE and TLBIEL.

Thanks,
Leandro

> > +
> > +TLBIE           011111 ..... - .. . . ..... 0100110010 -            
> > @X_tlbie
> > +TLBIEL          011111 ..... - .. . . ..... 0100010010 -            
> > @X_tlbie