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

[David Gibson]

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.

> ---
>  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),

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature