[Qemu-devel] [PATCH 0/6] Make -cpu e500mc useful in TCG

[Qemu-devel] [PATCH 0/6] Make -cpu e500mc useful in TCG

[Alexander Graf]

Thanks to Scott we now have an e500mc CPU description that works great with
KVM, but I have a personal dislike against targets that don't work emulated,
since their test coverage will be very low.

So this patch set implements TCG emulation for -cpu e500mc. I tested that it
works as expected against a recent Linux kernel. Everything looks smooth!

To use it, compile your kernel for an MPC8544DS board with e500mc CPU enabled.

Alexander Graf (6):
  PPC: Add IVOR 38-42
  PPC: e500mc: add missing IVORs to bitmap
  PPC: e500: msync is 440 only, e500 has real sync
  PPC: booke206: allow NULL raddr in ppcmas_tlb_check
  PPC: booke206: Check for min/max TLB entry size
  PPC: booke206: Implement tlbilx

 target-ppc/cpu.h            |    5 +++
 target-ppc/helper.c         |    5 ++-
 target-ppc/helper.h         |    1 +
 target-ppc/op_helper.c      |   70 +++++++++++++++++++++++++++++++++++++++++++
 target-ppc/translate.c      |   28 ++++++++++++++++-
 target-ppc/translate_init.c |   41 ++++++++++++++-----------
 6 files changed, 129 insertions(+), 21 deletions(-)

[Qemu-devel] [PATCH 1/6] PPC: Add IVOR 38-42

[Alexander Graf]

Our code only knows IVORs up to 37. Add the new ones defined in ISA 2.06
from 38 - 42.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target-ppc/cpu.h            |    5 +++++
 target-ppc/translate_init.c |   29 +++++++++++++++--------------
 2 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 2d67d1f..6f4cdde 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1371,6 +1371,11 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 #define SPR_BOOKE_IVOR13      (0x19D)
 #define SPR_BOOKE_IVOR14      (0x19E)
 #define SPR_BOOKE_IVOR15      (0x19F)
+#define SPR_BOOKE_IVOR38      (0x1B0)
+#define SPR_BOOKE_IVOR39      (0x1B1)
+#define SPR_BOOKE_IVOR40      (0x1B2)
+#define SPR_BOOKE_IVOR41      (0x1B3)
+#define SPR_BOOKE_IVOR42      (0x1B4)
 #define SPR_BOOKE_SPEFSCR     (0x200)
 #define SPR_Exxx_BBEAR        (0x201)
 #define SPR_Exxx_BBTAR        (0x202)
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 4d692d0..83348b5 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -526,26 +526,27 @@ static void spr_write_excp_prefix (void *opaque, int sprn, int gprn)
 static void spr_write_excp_vector (void *opaque, int sprn, int gprn)
 {
     DisasContext *ctx = opaque;
+    int sprn_offs;
 
     if (sprn >= SPR_BOOKE_IVOR0 && sprn <= SPR_BOOKE_IVOR15) {
-        TCGv t0 = tcg_temp_new();
-        tcg_gen_ld_tl(t0, cpu_env, offsetof(CPUState, ivor_mask));
-        tcg_gen_and_tl(t0, t0, cpu_gpr[gprn]);
-        tcg_gen_st_tl(t0, cpu_env, offsetof(CPUState, excp_vectors[sprn - SPR_BOOKE_IVOR0]));
-        gen_store_spr(sprn, t0);
-        tcg_temp_free(t0);
+        sprn_offs = sprn - SPR_BOOKE_IVOR0;
     } else if (sprn >= SPR_BOOKE_IVOR32 && sprn <= SPR_BOOKE_IVOR37) {
-        TCGv t0 = tcg_temp_new();
-        tcg_gen_ld_tl(t0, cpu_env, offsetof(CPUState, ivor_mask));
-        tcg_gen_and_tl(t0, t0, cpu_gpr[gprn]);
-        tcg_gen_st_tl(t0, cpu_env, offsetof(CPUState, excp_vectors[sprn - SPR_BOOKE_IVOR32 + 32]));
-        gen_store_spr(sprn, t0);
-        tcg_temp_free(t0);
+        sprn_offs = sprn - SPR_BOOKE_IVOR32 + 32;
+    } else if (sprn >= SPR_BOOKE_IVOR38 && sprn <= SPR_BOOKE_IVOR42) {
+        sprn_offs = sprn - SPR_BOOKE_IVOR38 + 38;
     } else {
         printf("Trying to write an unknown exception vector %d %03x\n",
                sprn, sprn);
         gen_inval_exception(ctx, POWERPC_EXCP_PRIV_REG);
+        return;
     }
+
+    TCGv t0 = tcg_temp_new();
+    tcg_gen_ld_tl(t0, cpu_env, offsetof(CPUState, ivor_mask));
+    tcg_gen_and_tl(t0, t0, cpu_gpr[gprn]);
+    tcg_gen_st_tl(t0, cpu_env, offsetof(CPUState, excp_vectors[sprn_offs]));
+    gen_store_spr(sprn, t0);
+    tcg_temp_free(t0);
 }
 #endif
 
@@ -1434,8 +1435,8 @@ static void gen_spr_BookE (CPUPPCState *env, uint64_t ivor_mask)
         SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx,
         SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx,
         SPR_BOOKE_IVOR32, SPR_BOOKE_IVOR33, SPR_BOOKE_IVOR34, SPR_BOOKE_IVOR35,
-        SPR_BOOKE_IVOR36, SPR_BOOKE_IVOR37, SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx,
-        SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx,
+        SPR_BOOKE_IVOR36, SPR_BOOKE_IVOR37, SPR_BOOKE_IVOR38, SPR_BOOKE_IVOR39,
+        SPR_BOOKE_IVOR40, SPR_BOOKE_IVOR41, SPR_BOOKE_IVOR42, SPR_BOOKE_IVORxx,
         SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx,
         SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx,
         SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx, SPR_BOOKE_IVORxx,
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 1/6] PPC: Add IVOR 38-42

[Andreas Färber]

Am 20.01.2012 04:17, schrieb Alexander Graf:
> Our code only knows IVORs up to 37. Add the new ones defined in ISA 2.06
> from 38 - 42.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Reviewed-by: Andreas Färber <afaerber@suse.de>

Didn't check against the ISA but the TCG code changes look right.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH 2/6] PPC: e500mc: add missing IVORs to bitmap

[Alexander Graf]

E500mc supports IVORs 36-41. Add them to the support mask.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target-ppc/translate_init.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 83348b5..7d1c6a3 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -4433,6 +4433,7 @@ enum fsl_e500_version {
 static void init_proc_e500 (CPUPPCState *env, int version)
 {
     uint32_t tlbncfg[2];
+    uint64_t ivor_mask = 0x0000000F0000FFFFULL;
 #if !defined(CONFIG_USER_ONLY)
     int i;
 #endif
@@ -4444,7 +4445,10 @@ static void init_proc_e500 (CPUPPCState *env, int version)
      *     complain when accessing them.
      * gen_spr_BookE(env, 0x0000000F0000FD7FULL);
      */
-    gen_spr_BookE(env, 0x0000000F0000FFFFULL);
+    if (version == fsl_e500mc) {
+        ivor_mask |= 0x3F000000000ULL;
+    }
+    gen_spr_BookE(env, ivor_mask);
     /* Processor identification */
     spr_register(env, SPR_BOOKE_PIR, "PIR",
                  SPR_NOACCESS, SPR_NOACCESS,
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 2/6] PPC: e500mc: add missing IVORs to bitmap

[Scott Wood]

On 01/19/2012 09:17 PM, Alexander Graf wrote:
> E500mc supports IVORs 36-41. Add them to the support mask.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  target-ppc/translate_init.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 83348b5..7d1c6a3 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -4433,6 +4433,7 @@ enum fsl_e500_version {
>  static void init_proc_e500 (CPUPPCState *env, int version)
>  {
>      uint32_t tlbncfg[2];
> +    uint64_t ivor_mask = 0x0000000F0000FFFFULL;
>  #if !defined(CONFIG_USER_ONLY)
>      int i;
>  #endif
> @@ -4444,7 +4445,10 @@ static void init_proc_e500 (CPUPPCState *env, int version)
>       *     complain when accessing them.
>       * gen_spr_BookE(env, 0x0000000F0000FD7FULL);
>       */
> -    gen_spr_BookE(env, 0x0000000F0000FFFFULL);
> +    if (version == fsl_e500mc) {
> +        ivor_mask |= 0x3F000000000ULL;
> +    }
> +    gen_spr_BookE(env, ivor_mask);
>      /* Processor identification */
>      spr_register(env, SPR_BOOKE_PIR, "PIR",
>                   SPR_NOACCESS, SPR_NOACCESS,

What happens when we add e5500 and future chips?  We should probably
move variant-specific data into a table instead of ad-hocking it.

Also, e500mc doesn't just add new IVORs, it drops the SPE IVORs.

-Scott

Re: [Qemu-devel] [PATCH 2/6] PPC: e500mc: add missing IVORs to bitmap

[Alexander Graf]

On 20.01.2012, at 20:16, Scott Wood wrote:

> On 01/19/2012 09:17 PM, Alexander Graf wrote:
>> E500mc supports IVORs 36-41. Add them to the support mask.
>> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> target-ppc/translate_init.c |    6 +++++-
>> 1 files changed, 5 insertions(+), 1 deletions(-)
>> 
>> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
>> index 83348b5..7d1c6a3 100644
>> --- a/target-ppc/translate_init.c
>> +++ b/target-ppc/translate_init.c
>> @@ -4433,6 +4433,7 @@ enum fsl_e500_version {
>> static void init_proc_e500 (CPUPPCState *env, int version)
>> {
>>     uint32_t tlbncfg[2];
>> +    uint64_t ivor_mask = 0x0000000F0000FFFFULL;
>> #if !defined(CONFIG_USER_ONLY)
>>     int i;
>> #endif
>> @@ -4444,7 +4445,10 @@ static void init_proc_e500 (CPUPPCState *env, int version)
>>      *     complain when accessing them.
>>      * gen_spr_BookE(env, 0x0000000F0000FD7FULL);
>>      */
>> -    gen_spr_BookE(env, 0x0000000F0000FFFFULL);
>> +    if (version == fsl_e500mc) {
>> +        ivor_mask |= 0x3F000000000ULL;
>> +    }
>> +    gen_spr_BookE(env, ivor_mask);
>>     /* Processor identification */
>>     spr_register(env, SPR_BOOKE_PIR, "PIR",
>>                  SPR_NOACCESS, SPR_NOACCESS,
> 
> What happens when we add e5500 and future chips?  We should probably
> move variant-specific data into a table instead of ad-hocking it.

Yup, but I think it makes sense to cross that bridge when we do add e5500 support, since then we'll actually know how to construct the tables. Today things aren't quite clear to me how much of this code we should reuse.

And in fact, a lot of the translate_init.c code is pretty redundant, so maybe the actual cleanup should rather be to commonize all of that. Maybe qdev'ify the CPUs with supported IVORs becoming simply a qdev property.

> Also, e500mc doesn't just add new IVORs, it drops the SPE IVORs.

Oops. Will fix :)


Alex

[Qemu-devel] [PATCH 3/6] PPC: e500: msync is 440 only, e500 has real sync

[Alexander Graf]

The e500 CPUs don't use 440's msync which falls on the same opcode IDs,
but instead use the real powerpc sync instruction. This is important,
since the invalid mask differs between the two.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target-ppc/translate.c      |    3 +--
 target-ppc/translate_init.c |    6 +++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 66eae30..18d52a9 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -8579,8 +8579,7 @@ GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01, PPC_WRTEE),
 GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x00000000, PPC_440_SPEC),
 GEN_HANDLER_E(mbar, 0x1F, 0x16, 0x1a, 0x001FF801,
               PPC_BOOKE, PPC2_BOOKE206),
-GEN_HANDLER_E(msync, 0x1F, 0x16, 0x12, 0x03FFF801,
-              PPC_BOOKE, PPC2_BOOKE206),
+GEN_HANDLER(msync, 0x1F, 0x16, 0x12, 0x03FFF801, PPC_BOOKE),
 GEN_HANDLER2_E(icbt_440, "icbt", 0x1F, 0x16, 0x00, 0x03E00001,
                PPC_BOOKE, PPC2_BOOKE206),
 GEN_HANDLER(lvsl, 0x1f, 0x06, 0x00, 0x00000001, PPC_ALTIVEC),
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 7d1c6a3..91e9e98 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -4371,7 +4371,7 @@ static void init_proc_e300 (CPUPPCState *env)
                                 PPC_WRTEE | PPC_RFDI |                  \
                                 PPC_CACHE | PPC_CACHE_LOCK | PPC_CACHE_ICBI | \
                                 PPC_CACHE_DCBZ | PPC_CACHE_DCBA |       \
-                                PPC_MEM_TLBSYNC | PPC_TLBIVAX)
+                                PPC_MEM_TLBSYNC | PPC_TLBIVAX | PPC_MEM_SYNC)
 #define POWERPC_INSNS2_e500v1  (PPC2_BOOKE206)
 #define POWERPC_MSRM_e500v1    (0x000000000606FF30ULL)
 #define POWERPC_MMU_e500v1     (POWERPC_MMU_BOOKE206)
@@ -4390,7 +4390,7 @@ static void init_proc_e300 (CPUPPCState *env)
                                 PPC_WRTEE | PPC_RFDI |                  \
                                 PPC_CACHE | PPC_CACHE_LOCK | PPC_CACHE_ICBI | \
                                 PPC_CACHE_DCBZ | PPC_CACHE_DCBA |       \
-                                PPC_MEM_TLBSYNC | PPC_TLBIVAX)
+                                PPC_MEM_TLBSYNC | PPC_TLBIVAX | PPC_MEM_SYNC)
 #define POWERPC_INSNS2_e500v2  (PPC2_BOOKE206)
 #define POWERPC_MSRM_e500v2    (0x000000000606FF30ULL)
 #define POWERPC_MMU_e500v2     (POWERPC_MMU_BOOKE206)
@@ -4411,7 +4411,7 @@ static void init_proc_e300 (CPUPPCState *env)
                                 PPC_FLOAT | PPC_FLOAT_FRES |                \
                                 PPC_FLOAT_FRSQRTE | PPC_FLOAT_FSEL |        \
                                 PPC_FLOAT_STFIWX | PPC_WAIT |               \
-                                PPC_MEM_TLBSYNC | PPC_TLBIVAX)
+                                PPC_MEM_TLBSYNC | PPC_TLBIVAX | PPC_MEM_SYNC)
 #define POWERPC_INSNS2_e500mc  (PPC2_BOOKE206)
 #define POWERPC_MSRM_e500mc    (0x000000001402FB36ULL)
 #define POWERPC_MMU_e500mc     (POWERPC_MMU_BOOKE206)
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 3/6] PPC: e500: msync is 440 only, e500 has real sync

[Scott Wood]

On 01/19/2012 09:17 PM, Alexander Graf wrote:
> The e500 CPUs don't use 440's msync which falls on the same opcode IDs,
> but instead use the real powerpc sync instruction. This is important,
> since the invalid mask differs between the two.

Could you rename 4xx msync to explicitly be 4xx msync, instead of
pretending that e500 doesn't have something called msync?

-Scott

[Qemu-devel] [PATCH 4/6] PPC: booke206: allow NULL raddr in ppcmas_tlb_check

[Alexander Graf]

We might want to call the tlb check function without actually caring about
the real address resolution. Check if we really should write the value
back.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target-ppc/helper.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index 5847453..2ce2d92 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -1338,7 +1338,10 @@ int ppcmas_tlb_check(CPUState *env, ppcmas_tlb_t *tlb,
     if ((address & mask) != (tlb->mas2 & MAS2_EPN_MASK)) {
         return -1;
     }
-    *raddrp = (tlb->mas7_3 & mask) | (address & ~mask);
+
+    if (raddrp) {
+        *raddrp = (tlb->mas7_3 & mask) | (address & ~mask);
+    }
 
     return 0;
 }
-- 
1.6.0.2

[Qemu-devel] [PATCH 5/6] PPC: booke206: Check for min/max TLB entry size

[Alexander Graf]

When setting a TLB entry, we need to check if the TLB we're putting it in
actually supports the given size. According to the 2.06 PowerPC ISA, a
value that's out of range results in the minimum page size for the TLB
to be used.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target-ppc/op_helper.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index 6339c95..0a88bf4 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -4228,6 +4228,7 @@ void helper_booke206_tlbwe(void)
 {
     uint32_t tlbncfg, tlbn;
     ppcmas_tlb_t *tlb;
+    uint32_t size_tlb, size_min, size_max;
 
     switch (env->spr[SPR_BOOKE_MAS0] & MAS0_WQ_MASK) {
     case MAS0_WQ_ALWAYS:
@@ -4273,6 +4274,16 @@ void helper_booke206_tlbwe(void)
         tlb->mas1 &= ~MAS1_IPROT;
     }
 
+    /* XXX only applies for MAV 1.0 */
+    size_tlb = (tlb->mas1 & MAS1_TSIZE_MASK) >> (MAS1_TSIZE_SHIFT + 1);
+    size_min = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT;
+    size_max = (tlbncfg & TLBnCFG_MAXSIZE) >> TLBnCFG_MAXSIZE_SHIFT;
+    if ((size_tlb > size_max) || (size_tlb < size_max)) {
+        /* set to min size */
+        tlb->mas1 &= ~MAS1_TSIZE_MASK;
+        tlb->mas1 |= size_min << (MAS1_TSIZE_SHIFT + 1);
+    }
+
     if (booke206_tlb_to_page_size(env, tlb) == TARGET_PAGE_SIZE) {
         tlb_flush_page(env, tlb->mas2 & MAS2_EPN_MASK);
     } else {
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 5/6] PPC: booke206: Check for min/max TLB entry size

[Paolo Bonzini]

On 01/20/2012 04:17 AM, Alexander Graf wrote:
> +    if ((size_tlb > size_max) || (size_tlb < size_max)) {

You want < size_min, and the extra parentheses look odd.

> +        /* set to min size */
> +        tlb->mas1&= ~MAS1_TSIZE_MASK;
> +        tlb->mas1 |= size_min<<  (MAS1_TSIZE_SHIFT + 1);
> +    }

Re: [Qemu-devel] [PATCH 5/6] PPC: booke206: Check for min/max TLB entry size

[Andreas Färber]

Am 20.01.2012 04:17, schrieb Alexander Graf:
> When setting a TLB entry, we need to check if the TLB we're putting it in
> actually supports the given size. According to the 2.06 PowerPC ISA, a
> value that's out of range results in the minimum page size for the TLB
> to be used.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  target-ppc/op_helper.c |   11 +++++++++++
>  1 files changed, 11 insertions(+), 0 deletions(-)
> 
> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
> index 6339c95..0a88bf4 100644
> --- a/target-ppc/op_helper.c
> +++ b/target-ppc/op_helper.c
> @@ -4228,6 +4228,7 @@ void helper_booke206_tlbwe(void)
>  {
>      uint32_t tlbncfg, tlbn;
>      ppcmas_tlb_t *tlb;
> +    uint32_t size_tlb, size_min, size_max;
>  
>      switch (env->spr[SPR_BOOKE_MAS0] & MAS0_WQ_MASK) {
>      case MAS0_WQ_ALWAYS:
> @@ -4273,6 +4274,16 @@ void helper_booke206_tlbwe(void)
>          tlb->mas1 &= ~MAS1_IPROT;
>      }
>  
> +    /* XXX only applies for MAV 1.0 */
> +    size_tlb = (tlb->mas1 & MAS1_TSIZE_MASK) >> (MAS1_TSIZE_SHIFT + 1);
> +    size_min = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT;
> +    size_max = (tlbncfg & TLBnCFG_MAXSIZE) >> TLBnCFG_MAXSIZE_SHIFT;
> +    if ((size_tlb > size_max) || (size_tlb < size_max)) {

This looks wrong...?

Andreas

> +        /* set to min size */
> +        tlb->mas1 &= ~MAS1_TSIZE_MASK;
> +        tlb->mas1 |= size_min << (MAS1_TSIZE_SHIFT + 1);
> +    }
> +
>      if (booke206_tlb_to_page_size(env, tlb) == TARGET_PAGE_SIZE) {
>          tlb_flush_page(env, tlb->mas2 & MAS2_EPN_MASK);
>      } else {

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH] PPC: booke206: Check for min/max TLB entry size

[Alexander Graf]

When setting a TLB entry, we need to check if the TLB we're putting it in
actually supports the given size. According to the 2.06 PowerPC ISA, a
value that's out of range results in the minimum page size for the TLB
to be used.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - fix min/max check

diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index 6339c95..8cd0224 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -4228,6 +4228,7 @@ void helper_booke206_tlbwe(void)
 {
     uint32_t tlbncfg, tlbn;
     ppcmas_tlb_t *tlb;
+    uint32_t size_tlb, size_min, size_max;
 
     switch (env->spr[SPR_BOOKE_MAS0] & MAS0_WQ_MASK) {
     case MAS0_WQ_ALWAYS:
@@ -4273,6 +4274,16 @@ void helper_booke206_tlbwe(void)
         tlb->mas1 &= ~MAS1_IPROT;
     }
 
+    /* XXX only applies for MAV 1.0 */
+    size_tlb = (tlb->mas1 & MAS1_TSIZE_MASK) >> (MAS1_TSIZE_SHIFT + 1);
+    size_min = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT;
+    size_max = (tlbncfg & TLBnCFG_MAXSIZE) >> TLBnCFG_MAXSIZE_SHIFT;
+    if ((size_tlb > size_max) || (size_tlb < size_min)) {
+        /* set to min size */
+        tlb->mas1 &= ~MAS1_TSIZE_MASK;
+        tlb->mas1 |= size_min << (MAS1_TSIZE_SHIFT + 1);
+    }
+
     if (booke206_tlb_to_page_size(env, tlb) == TARGET_PAGE_SIZE) {
         tlb_flush_page(env, tlb->mas2 & MAS2_EPN_MASK);
     } else {

Re: [Qemu-devel] [PATCH] PPC: booke206: Check for min/max TLB entry size

[Scott Wood]

On 01/20/2012 07:21 AM, Alexander Graf wrote:
> When setting a TLB entry, we need to check if the TLB we're putting it in
> actually supports the given size. According to the 2.06 PowerPC ISA, a
> value that's out of range results in the minimum page size for the TLB
> to be used.

The ISA says, "If the page size specified by
MAS1TSIZE is not supported by the specified array, the
tlbwe may be performed as if TSIZE were some imple-
mentation-dependent value, or an Illegal Instruction
exception occurs."

In practice, what this means on e500 is that TLB0 (which only supports
one page size) ignores TSIZE.  I'm not sure what happens when you write
an entry to TLB1 with an invalid TSIZE.

> +    /* XXX only applies for MAV 1.0 */
> +    size_tlb = (tlb->mas1 & MAS1_TSIZE_MASK) >> (MAS1_TSIZE_SHIFT + 1);
> +    size_min = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT;
> +    size_max = (tlbncfg & TLBnCFG_MAXSIZE) >> TLBnCFG_MAXSIZE_SHIFT;
> +    if ((size_tlb > size_max) || (size_tlb < size_min)) {
> +        /* set to min size */
> +        tlb->mas1 &= ~MAS1_TSIZE_MASK;
> +        tlb->mas1 |= size_min << (MAS1_TSIZE_SHIFT + 1);
> +    }

You could just implement a bitmap now, which will work for MAV 2.0 as well.

Especially since we're using the MAV 2.0 definition of tsize already, so
min/max isn't an accurate way to describe what we support.

-Scott

Re: [Qemu-devel] [PATCH] PPC: booke206: Check for min/max TLB entry size

[Alexander Graf]

Am 20.01.2012 um 21:01 schrieb Scott Wood <scottwood@freescale.com>:

> On 01/20/2012 07:21 AM, Alexander Graf wrote:
>> When setting a TLB entry, we need to check if the TLB we're putting it in
>> actually supports the given size. According to the 2.06 PowerPC ISA, a
>> value that's out of range results in the minimum page size for the TLB
>> to be used.
> 
> The ISA says, "If the page size specified by
> MAS1TSIZE is not supported by the specified array, the
> tlbwe may be performed as if TSIZE were some imple-
> mentation-dependent value, or an Illegal Instruction
> exception occurs."

Ah, I looked at MMUCSR0 behavior (page 955 in 2.06B) where it stated that invalid sizes mean MINSIZE. Apparently it's not defined for tlbwe though.

However, if we declare the implementation dependent value to be MINSIZE for us, we're good. If you have any idea what hardware does, that would be great. Otherwise when I get the chance I could of course. Also give it a try myself :).

> 
> In practice, what this means on e500 is that TLB0 (which only supports
> one page size) ignores TSIZE.  

Yes, that part is actually written out

> I'm not sure what happens when you write
> an entry to TLB1 with an invalid TSIZE.

What it says, the ISA means it's implementation dependent. What e500mc actually implements is an different question. Which still needs to be answered.

However for now I think wde 're ok by not modeling every odd corner case.

> 
>> +    /* XXX only applies for MAV 1.0 */
>> +    size_tlb = (tlb->mas1 & MAS1_TSIZE_MASK) >> (MAS1_TSIZE_SHIFT + 1);
>> +    size_min = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT;
>> +    size_max = (tlbncfg & TLBnCFG_MAXSIZE) >> TLBnCFG_MAXSIZE_SHIFT;
>> +    if ((size_tlb > size_max) || (size_tlb < size_min)) {
>> +        /* set to min size */
>> +        tlb->mas1 &= ~MAS1_TSIZE_MASK;
>> +        tlb->mas1 |= size_min << (MAS1_TSIZE_SHIFT + 1);
>> +    }
> 
> You could just implement a bitmap now, which will work for MAV 2.0 as well.
> 
> Especially since we're using the MAV 2.0 definition of tsize already, so
> min/max isn't an accurate way to describe what we support.

Not sure I follow. In MAV 1.0 the size constraints are defined in TLBnCFG, while for MAV 2.0 they are defned in their own bitmap registers (TLBnPS)

Would you like to have a function called that returns a bitmap of supported sizes for the TLB depending on its MAV value based on either TLBnCFG or TLBnPS? We could then check if that size value bit is set.

Alex

> 
> -Scott
> 

Re: [Qemu-devel] [PATCH] PPC: booke206: Check for min/max TLB entry size

[Scott Wood]

On 01/20/2012 08:43 PM, Alexander Graf wrote:
> 
> 
> Am 20.01.2012 um 21:01 schrieb Scott Wood <scottwood@freescale.com>:
>> I'm not sure what happens when you write
>> an entry to TLB1 with an invalid TSIZE.
> 
> What it says, the ISA means it's implementation dependent. What e500mc actually implements is an different question. Which still needs to be answered.

AFAIK it's not documented what e500mc does for invalid sizes in TLB1, so
I think anything that complies with the architecture's statement of any
supported size is OK.

> However for now I think wde 're ok by not modeling every odd corner case.

Sure.  I was just curious about the architectural statement.

>>> +    /* XXX only applies for MAV 1.0 */
>>> +    size_tlb = (tlb->mas1 & MAS1_TSIZE_MASK) >> (MAS1_TSIZE_SHIFT + 1);
>>> +    size_min = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT;
>>> +    size_max = (tlbncfg & TLBnCFG_MAXSIZE) >> TLBnCFG_MAXSIZE_SHIFT;
>>> +    if ((size_tlb > size_max) || (size_tlb < size_min)) {
>>> +        /* set to min size */
>>> +        tlb->mas1 &= ~MAS1_TSIZE_MASK;
>>> +        tlb->mas1 |= size_min << (MAS1_TSIZE_SHIFT + 1);
>>> +    }
>>
>> You could just implement a bitmap now, which will work for MAV 2.0 as well.
>>
>> Especially since we're using the MAV 2.0 definition of tsize already, so
>> min/max isn't an accurate way to describe what we support.
> 
> Not sure I follow. In MAV 1.0 the size constraints are defined in TLBnCFG, while for MAV 2.0 they are defned in their own bitmap registers (TLBnPS)
> 
> Would you like to have a function called that returns a bitmap of
> supported sizes for the TLB depending on its MAV value based on
> either TLBnCFG or TLBnPS? We could then check if that size value bit
> is set.

Yes, use a bitmap internally regardless of how the programming model
says we convey the information to the target code.

-Scott

Re: [Qemu-devel] [PATCH] PPC: booke206: Check for min/max TLB entry size

[Alexander Graf]

On 01/23/2012 06:28 PM, Scott Wood wrote:
> On 01/20/2012 08:43 PM, Alexander Graf wrote:
>>
>> Am 20.01.2012 um 21:01 schrieb Scott Wood<scottwood@freescale.com>:
>>> I'm not sure what happens when you write
>>> an entry to TLB1 with an invalid TSIZE.
>> What it says, the ISA means it's implementation dependent. What e500mc actually implements is an different question. Which still needs to be answered.
> AFAIK it's not documented what e500mc does for invalid sizes in TLB1, so
> I think anything that complies with the architecture's statement of any
> supported size is OK.
>
>> However for now I think wde 're ok by not modeling every odd corner case.
> Sure.  I was just curious about the architectural statement.
>
>>>> +    /* XXX only applies for MAV 1.0 */
>>>> +    size_tlb = (tlb->mas1&  MAS1_TSIZE_MASK)>>  (MAS1_TSIZE_SHIFT + 1);
>>>> +    size_min = (tlbncfg&  TLBnCFG_MINSIZE)>>  TLBnCFG_MINSIZE_SHIFT;
>>>> +    size_max = (tlbncfg&  TLBnCFG_MAXSIZE)>>  TLBnCFG_MAXSIZE_SHIFT;
>>>> +    if ((size_tlb>  size_max) || (size_tlb<  size_min)) {
>>>> +        /* set to min size */
>>>> +        tlb->mas1&= ~MAS1_TSIZE_MASK;
>>>> +        tlb->mas1 |= size_min<<  (MAS1_TSIZE_SHIFT + 1);
>>>> +    }
>>> You could just implement a bitmap now, which will work for MAV 2.0 as well.
>>>
>>> Especially since we're using the MAV 2.0 definition of tsize already, so
>>> min/max isn't an accurate way to describe what we support.
>> Not sure I follow. In MAV 1.0 the size constraints are defined in TLBnCFG, while for MAV 2.0 they are defned in their own bitmap registers (TLBnPS)
>>
>> Would you like to have a function called that returns a bitmap of
>> supported sizes for the TLB depending on its MAV value based on
>> either TLBnCFG or TLBnPS? We could then check if that size value bit
>> is set.
> Yes, use a bitmap internally regardless of how the programming model
> says we convey the information to the target code.

Already done :).


Alex

[Qemu-devel] [PATCH 6/6] PPC: booke206: Implement tlbilx

[Alexander Graf]

The PowerPC 2.06 BookE ISA defines an opcode called "tlbilx" which is used
to flush TLB entries. It's the recommended way of flushing in virtualized
environments.

So far we got away without implementing it, but Linux for e500mc uses this
instruction, so we better add it :).

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target-ppc/helper.h    |    1 +
 target-ppc/op_helper.c |   59 ++++++++++++++++++++++++++++++++++++++++++++++++
 target-ppc/translate.c |   25 ++++++++++++++++++++
 3 files changed, 85 insertions(+), 0 deletions(-)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 470e42f..1635767 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -336,6 +336,7 @@ DEF_HELPER_0(booke206_tlbre, void)
 DEF_HELPER_0(booke206_tlbwe, void)
 DEF_HELPER_1(booke206_tlbsx, void, tl)
 DEF_HELPER_1(booke206_tlbivax, void, tl)
+DEF_HELPER_2(booke206_tlbilx, void, tl, i32)
 DEF_HELPER_1(booke206_tlbflush, void, i32)
 DEF_HELPER_2(booke_setpid, void, i32, tl)
 DEF_HELPER_1(6xx_tlbd, void, tl)
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index 0a88bf4..7ae920a 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -4406,6 +4406,65 @@ void helper_booke206_tlbivax(target_ulong address)
     }
 }
 
+void helper_booke206_tlbilx(target_ulong address, uint32_t t)
+{
+    int tlb_size;
+    int i, j;
+    ppcmas_tlb_t *tlb = env->tlb.tlbm;
+    int tid = (env->spr[SPR_BOOKE_MAS6] & MAS6_SPID);
+    int pid = tid >> MAS6_SPID_SHIFT;
+    int sgs = env->spr[SPR_BOOKE_MAS5] & MAS5_SGS;
+    int ts = (env->spr[SPR_BOOKE_MAS6] & MAS6_SAS) ? MAS1_TS : 0;
+    int ind = (env->spr[SPR_BOOKE_MAS6] & MAS6_SIND) ? MAS1_IND : 0;
+    int size = env->spr[SPR_BOOKE_MAS6] & MAS6_ISIZE_MASK;
+
+    /* XXX missing LPID handling */
+    switch (t) {
+    case 0:
+        /* flush all */
+        booke206_flush_tlb(env, -1, 1);
+        break;
+    case 1:
+        /* flush by pid */
+        for (i = 0; i < BOOKE206_MAX_TLBN; i++) {
+            tlb_size = booke206_tlb_size(env, i);
+            for (j = 0; j < tlb_size; j++) {
+                if (!(tlb[j].mas1 & MAS1_IPROT) &&
+                    ((tlb[j].mas1 & MAS1_TID_MASK) == tid)) {
+                    tlb[j].mas1 &= ~MAS1_VALID;
+                }
+            }
+            tlb += booke206_tlb_size(env, i);
+        }
+        tlb_flush(env, 1);
+        break;
+    case 3:
+        /* flush by pid and ea */
+        for (i = 0; i < BOOKE206_MAX_TLBN; i++) {
+            int ways = booke206_tlb_ways(env, i);
+
+            for (j = 0; j < ways; j++) {
+                tlb = booke206_get_tlbm(env, i, address, j);
+                if ((ppcmas_tlb_check(env, tlb, NULL, address, pid) != 0) ||
+                    (tlb->mas1 & MAS1_IPROT) ||
+                    ((tlb->mas1 & MAS1_TS) != ts) ||
+                    ((tlb->mas1 & MAS1_IND) != ind) ||
+                    ((tlb->mas1 & MAS1_TSIZE_MASK) != size) ||
+                    ((tlb->mas8 & MAS8_TGS) != sgs)) {
+                        continue;
+                }
+                tlb->mas1 &= ~MAS1_VALID;
+            }
+        }
+        tlb_flush(env, 1);
+        break;
+    default:
+        helper_raise_exception_err(POWERPC_EXCP_PROGRAM,
+                                   POWERPC_EXCP_INVAL |
+                                   POWERPC_EXCP_INVAL_INVAL);
+    }
+}
+
 void helper_booke206_tlbflush(uint32_t type)
 {
     int flags = 0;
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 18d52a9..3cf4cce 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -6110,6 +6110,29 @@ static void gen_tlbivax_booke206(DisasContext *ctx)
 #endif
 }
 
+static void gen_tlbilx_booke206(DisasContext *ctx)
+{
+#if defined(CONFIG_USER_ONLY)
+    gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
+#else
+    TCGv t0;
+    TCGv_i32 t1;
+    if (unlikely(!ctx->mem_idx)) {
+        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
+        return;
+    }
+
+    t0 = tcg_temp_new();
+    t1 = tcg_const_i32((ctx->opcode >> 21) & 0x3);
+    gen_addr_reg_index(ctx, t0);
+
+    gen_helper_booke206_tlbilx(t0, t1);
+
+    tcg_temp_free(t0);
+    tcg_temp_free_i32(t1);
+#endif
+}
+
 
 /* wrtee */
 static void gen_wrtee(DisasContext *ctx)
@@ -8574,6 +8597,8 @@ GEN_HANDLER2_E(tlbwe_booke206, "tlbwe", 0x1F, 0x12, 0x1E, 0x00000001,
                PPC_NONE, PPC2_BOOKE206),
 GEN_HANDLER2_E(tlbivax_booke206, "tlbivax", 0x1F, 0x12, 0x18, 0x00000001,
                PPC_NONE, PPC2_BOOKE206),
+GEN_HANDLER2_E(tlbilx_booke206, "tlbilx", 0x1F, 0x12, 0x00, 0x03800001,
+               PPC_NONE, PPC2_BOOKE206),
 GEN_HANDLER(wrtee, 0x1F, 0x03, 0x04, 0x000FFC01, PPC_WRTEE),
 GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01, PPC_WRTEE),
 GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x00000000, PPC_440_SPEC),
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH 6/6] PPC: booke206: Implement tlbilx

[Scott Wood]

On 01/19/2012 09:17 PM, Alexander Graf wrote:
> +    case 3:
> +        /* flush by pid and ea */
> +        for (i = 0; i < BOOKE206_MAX_TLBN; i++) {
> +            int ways = booke206_tlb_ways(env, i);
> +
> +            for (j = 0; j < ways; j++) {
> +                tlb = booke206_get_tlbm(env, i, address, j);
> +                if ((ppcmas_tlb_check(env, tlb, NULL, address, pid) != 0) ||
> +                    (tlb->mas1 & MAS1_IPROT) ||
> +                    ((tlb->mas1 & MAS1_TS) != ts) ||
> +                    ((tlb->mas1 & MAS1_IND) != ind) ||
> +                    ((tlb->mas1 & MAS1_TSIZE_MASK) != size) ||
> +                    ((tlb->mas8 & MAS8_TGS) != sgs)) {
> +                        continue;
> +                }
> +                tlb->mas1 &= ~MAS1_VALID;
> +            }

ISIZE is only supported on MAV=2.0, and then only if TLB write
conditional or Hardware Entry Select is supported.

Also, I don't know to what extent you want to emulate particular cores
versus a generic implementation of the architecture, but e500mc does not
filter on MAS6[SAS].  This is permitted as noted in 6.7.1's Programming
Note allowing generous TLB invalidations, and is documented this way in
the e500mc manual so software could be relying on it.

-Scott

Re: [Qemu-devel] [PATCH 6/6] PPC: booke206: Implement tlbilx

[Alexander Graf]

On 20.01.2012, at 21:40, Scott Wood wrote:

> On 01/19/2012 09:17 PM, Alexander Graf wrote:
>> +    case 3:
>> +        /* flush by pid and ea */
>> +        for (i = 0; i < BOOKE206_MAX_TLBN; i++) {
>> +            int ways = booke206_tlb_ways(env, i);
>> +
>> +            for (j = 0; j < ways; j++) {
>> +                tlb = booke206_get_tlbm(env, i, address, j);
>> +                if ((ppcmas_tlb_check(env, tlb, NULL, address, pid) != 0) ||
>> +                    (tlb->mas1 & MAS1_IPROT) ||
>> +                    ((tlb->mas1 & MAS1_TS) != ts) ||
>> +                    ((tlb->mas1 & MAS1_IND) != ind) ||
>> +                    ((tlb->mas1 & MAS1_TSIZE_MASK) != size) ||
>> +                    ((tlb->mas8 & MAS8_TGS) != sgs)) {
>> +                        continue;
>> +                }
>> +                tlb->mas1 &= ~MAS1_VALID;
>> +            }
> 
> ISIZE is only supported on MAV=2.0, and then only if TLB write
> conditional or Hardware Entry Select is supported.

Actually ISIZE is even more tricky than that. According to ISA 2.06 the instruction is treated as invalid instruction if ISIZE doesn't work for the TLB it's invalidating in:

If T = 3 and the implementation requires the page size to be specified by MAS6ISIZE (MMUCFG[TWC] = 1 or, for any TLB array, TLBnCFG[HES] = 1) and the page size specified by MAS6ISIZE is not supported by the implementation, the instruction is treated as if the instruction form is invalid.

But since it's MAV 2.0 only we can leave it out for now. I'll try to make the code obvious enough on where to put it back in later.

> Also, I don't know to what extent you want to emulate particular cores
> versus a generic implementation of the architecture, but e500mc does not
> filter on MAS6[SAS].  This is permitted as noted in 6.7.1's Programming
> Note allowing generous TLB invalidations, and is documented this way in
> the e500mc manual so software could be relying on it.

We can leave SAS matching out and just always conform to the spec that way. If any guest OS later needs specific core behavior to actually match on SAS, we can still add it back in.


Alex