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

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

[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.

v1 -> v2:

  - rename msync to msync_4xx
  - add preliminary TLBnPS handling
  - use mav 2.0 prepared code
  - raise exception on invalid page size
  - remove sas/ts check
  - isize is only valid for mav 2.0
  - drop SPE IVOR for e500mc

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

 target-ppc/cpu.h            |   30 +++++++++++++++++
 target-ppc/helper.c         |    5 ++-
 target-ppc/helper.h         |    1 +
 target-ppc/op_helper.c      |   75 +++++++++++++++++++++++++++++++++++++++++++
 target-ppc/translate.c      |   30 +++++++++++++++--
 target-ppc/translate_init.c |   41 +++++++++++++----------
 6 files changed, 160 insertions(+), 22 deletions(-)

[Qemu-devel] [PATCH 1/8] 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>
Reviewed-by: Andreas Färber <afaerber@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

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

[Alexander Graf]

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

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

---

v1 -> v2:

  - drop SPE IVOR
---
 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..f5fcd1e 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 = 0x000003FE0000FFFFULL;
+    }
+    gen_spr_BookE(env, ivor_mask);
     /* Processor identification */
     spr_register(env, SPR_BOOKE_PIR, "PIR",
                  SPR_NOACCESS, SPR_NOACCESS,
-- 
1.6.0.2

[Qemu-devel] [PATCH 3/8] 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 f5fcd1e..b14a98c 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

[Qemu-devel] [PATCH 4/8] PPC: rename msync to msync_4xx

[Alexander Graf]

The msync instruction as defined today is only valid on 4xx cores, not
on e500 which also supports msync, but treats it the same way as sync.

Rename it to reflect that it's 4xx only.

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

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 18d52a9..adde65b 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -6172,7 +6172,7 @@ static void gen_mbar(DisasContext *ctx)
 }
 
 /* msync replaces sync on 440 */
-static void gen_msync(DisasContext *ctx)
+static void gen_msync_4xx(DisasContext *ctx)
 {
     /* interpreted as no-op */
 }
@@ -8579,7 +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(msync, 0x1F, 0x16, 0x12, 0x03FFF801, PPC_BOOKE),
+GEN_HANDLER(msync_4xx, 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),
-- 
1.6.0.2

[Qemu-devel] [PATCH 5/8] 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 6/8] PPC: booke: add tlbnps handling

[Alexander Graf]

When using MAV 2.0 TLB registers, we have another range of TLB registers
available to read the supported page sizes from.

Add SPR definitions for those and add a helper function that we can use
to receive such a bitmap even when using MAV 1.0.

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

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 6f4cdde..1026254 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1355,6 +1355,10 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
 #define SPR_BOOKE_DVC2        (0x13F)
 #define SPR_BOOKE_TSR         (0x150)
 #define SPR_BOOKE_TCR         (0x154)
+#define SPR_BOOKE_TLB0PS      (0x158)
+#define SPR_BOOKE_TLB1PS      (0x159)
+#define SPR_BOOKE_TLB2PS      (0x15A)
+#define SPR_BOOKE_TLB3PS      (0x15B)
 #define SPR_BOOKE_IVOR0       (0x190)
 #define SPR_BOOKE_IVOR1       (0x191)
 #define SPR_BOOKE_IVOR2       (0x192)
@@ -2116,6 +2120,27 @@ static inline ppcmas_tlb_t *booke206_get_tlbm(CPUState *env, const int tlbn,
     return &env->tlb.tlbm[r];
 }
 
+/* returns bitmap of supported page sizes for a given TLB */
+static inline uint32_t booke206_tlbnps(CPUState *env, const int tlbn)
+{
+    bool mav2 = false;
+    uint32_t ret = 0;
+
+    if (mav2) {
+        ret = env->spr[SPR_BOOKE_TLB0PS + tlbn];
+    } else {
+        uint32_t tlbncfg = env->spr[SPR_BOOKE_TLB0CFG + tlbn];
+        uint32_t min = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT;
+        uint32_t max = (tlbncfg & TLBnCFG_MAXSIZE) >> TLBnCFG_MAXSIZE_SHIFT;
+        int i;
+        for (i = min; i <= max; i++) {
+            ret |= (1 << (i << 1));
+        }
+    }
+
+    return ret;
+}
+
 #endif
 
 extern void (*cpu_ppc_hypercall)(CPUState *);
-- 
1.6.0.2

[Qemu-devel] [PATCH 7/8] 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 can either be redefined to something implementation
dependent or we can raise an illegal opcode exception. We do the latter.

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

---

v1 -> v2:

  - fix min/max check
  - use mav 2.0 prepared code
  - raise exception on invalid page size
---
 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..2c8a96f 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_ps;
 
     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;
     }
 
+    /* check that we support the targeted size */
+    size_tlb = (tlb->mas1 & MAS1_TSIZE_MASK) >> MAS1_TSIZE_SHIFT;
+    size_ps = booke206_tlbnps(env, tlbn);
+    if ((tlb->mas1 & MAS1_VALID) && (tlbncfg & TLBnCFG_AVAIL) &&
+        !(size_ps & (1 << size_tlb))) {
+        helper_raise_exception_err(POWERPC_EXCP_PROGRAM,
+                                   POWERPC_EXCP_INVAL |
+                                   POWERPC_EXCP_INVAL_INVAL);
+    }
+
     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

[Qemu-devel] [PATCH 8/8] 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>

---

v1 -> v2:

  - remove sas/ts check
  - isize is only valid for mav 2.0
---
 target-ppc/helper.h    |    1 +
 target-ppc/op_helper.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++
 target-ppc/translate.c |   25 ++++++++++++++++++
 3 files changed, 90 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 2c8a96f..29c3870 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -4406,6 +4406,70 @@ 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 ind = (env->spr[SPR_BOOKE_MAS6] & MAS6_SIND) ? MAS1_IND : 0;
+    /* XXX check for unsupported isize and raise an invalid opcode then */
+    int size = env->spr[SPR_BOOKE_MAS6] & MAS6_ISIZE_MASK;
+    /* XXX implement MAV2 handling */
+    bool mav2 = false;
+
+    /* 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_IND) != ind) ||
+                    ((tlb->mas8 & MAS8_TGS) != sgs)) {
+                    continue;
+                }
+                if (mav2 && ((tlb->mas1 & MAS1_TSIZE_MASK) != size)) {
+                    /* XXX only check when MMUCFG[TWC] || TLBnCFG[HES] */
+                    continue;
+                }
+                /* XXX e500mc doesn't match SAS, but other cores might */
+                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 adde65b..7ceb210 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 8/8] PPC: booke206: Implement tlbilx

[Blue Swirl]

On Sat, Jan 21, 2012 at 04:15, Alexander Graf <agraf@suse.de> wrote:
> 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>
>
> ---
>
> v1 -> v2:
>
>  - remove sas/ts check
>  - isize is only valid for mav 2.0
> ---
>  target-ppc/helper.h    |    1 +
>  target-ppc/op_helper.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++
>  target-ppc/translate.c |   25 ++++++++++++++++++
>  3 files changed, 90 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 2c8a96f..29c3870 100644
> --- a/target-ppc/op_helper.c
> +++ b/target-ppc/op_helper.c
> @@ -4406,6 +4406,70 @@ 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 ind = (env->spr[SPR_BOOKE_MAS6] & MAS6_SIND) ? MAS1_IND : 0;
> +    /* XXX check for unsupported isize and raise an invalid opcode then */
> +    int size = env->spr[SPR_BOOKE_MAS6] & MAS6_ISIZE_MASK;
> +    /* XXX implement MAV2 handling */
> +    bool mav2 = false;
> +
> +    /* XXX missing LPID handling */
> +    switch (t) {

For better performance, this switch could be pushed to translation
time and helpers introduced for each case.

> +    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_IND) != ind) ||
> +                    ((tlb->mas8 & MAS8_TGS) != sgs)) {
> +                    continue;
> +                }
> +                if (mav2 && ((tlb->mas1 & MAS1_TSIZE_MASK) != size)) {
> +                    /* XXX only check when MMUCFG[TWC] || TLBnCFG[HES] */
> +                    continue;
> +                }
> +                /* XXX e500mc doesn't match SAS, but other cores might */
> +                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 adde65b..7ceb210 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
>
>

[Qemu-devel] [PATCH] 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>

---

v1 -> v2:

  - remove sas/ts check
  - isize is only valid for mav 2.0

v2 -> v3:

  - move tlbilx variants into their own helpers
---
 target-ppc/helper.h    |    3 ++
 target-ppc/op_helper.c |   64 ++++++++++++++++++++++++++++++++++++++++++++++++
 target-ppc/translate.c |   35 ++++++++++++++++++++++++++
 3 files changed, 102 insertions(+), 0 deletions(-)

diff --git a/target-ppc/helper.h b/target-ppc/helper.h
index 470e42f..4798fd5 100644
--- a/target-ppc/helper.h
+++ b/target-ppc/helper.h
@@ -336,6 +336,9 @@ 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_1(booke206_tlbilx0, void, tl)
+DEF_HELPER_1(booke206_tlbilx1, void, tl)
+DEF_HELPER_1(booke206_tlbilx3, void, tl)
 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 2c8a96f..3e83b64 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -4406,6 +4406,70 @@ void helper_booke206_tlbivax(target_ulong address)
     }
 }
 
+void helper_booke206_tlbilx0(target_ulong address)
+{
+    /* XXX missing LPID handling */
+    booke206_flush_tlb(env, -1, 1);
+}
+
+void helper_booke206_tlbilx1(target_ulong address)
+{
+    int i, j;
+    int tid = (env->spr[SPR_BOOKE_MAS6] & MAS6_SPID);
+    ppcmas_tlb_t *tlb = env->tlb.tlbm;
+    int tlb_size;
+
+    /* XXX missing LPID handling */
+    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);
+}
+
+void helper_booke206_tlbilx3(target_ulong address)
+{
+    int i, j;
+    ppcmas_tlb_t *tlb;
+    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 ind = (env->spr[SPR_BOOKE_MAS6] & MAS6_SIND) ? MAS1_IND : 0;
+    /* XXX check for unsupported isize and raise an invalid opcode then */
+    int size = env->spr[SPR_BOOKE_MAS6] & MAS6_ISIZE_MASK;
+    /* XXX implement MAV2 handling */
+    bool mav2 = false;
+
+    /* XXX missing LPID handling */
+    /* 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_IND) != ind) ||
+                ((tlb->mas8 & MAS8_TGS) != sgs)) {
+                continue;
+            }
+            if (mav2 && ((tlb->mas1 & MAS1_TSIZE_MASK) != size)) {
+                /* XXX only check when MMUCFG[TWC] || TLBnCFG[HES] */
+                continue;
+            }
+            /* XXX e500mc doesn't match SAS, but other cores might */
+            tlb->mas1 &= ~MAS1_VALID;
+        }
+    }
+    tlb_flush(env, 1);
+}
+
 void helper_booke206_tlbflush(uint32_t type)
 {
     int flags = 0;
diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index adde65b..d8ef719 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -6110,6 +6110,39 @@ 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;
+    if (unlikely(!ctx->mem_idx)) {
+        gen_inval_exception(ctx, POWERPC_EXCP_PRIV_OPC);
+        return;
+    }
+
+    t0 = tcg_temp_new();
+    gen_addr_reg_index(ctx, t0);
+
+    switch((ctx->opcode >> 21) & 0x3) {
+    case 0:
+        gen_helper_booke206_tlbilx0(t0);
+        break;
+    case 1:
+        gen_helper_booke206_tlbilx1(t0);
+        break;
+    case 3:
+        gen_helper_booke206_tlbilx3(t0);
+        break;
+    default:
+        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);
+        break;
+    }
+
+    tcg_temp_free(t0);
+#endif
+}
+
 
 /* wrtee */
 static void gen_wrtee(DisasContext *ctx)
@@ -8574,6 +8607,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/8] PPC: booke: add tlbnps handling

[Scott Wood]

On 01/20/2012 10:15 PM, Alexander Graf wrote:
> +/* returns bitmap of supported page sizes for a given TLB */
> +static inline uint32_t booke206_tlbnps(CPUState *env, const int tlbn)
> +{
> +    bool mav2 = false;
> +    uint32_t ret = 0;
> +
> +    if (mav2) {
> +        ret = env->spr[SPR_BOOKE_TLB0PS + tlbn];
> +    } else {
> +        uint32_t tlbncfg = env->spr[SPR_BOOKE_TLB0CFG + tlbn];
> +        uint32_t min = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT;
> +        uint32_t max = (tlbncfg & TLBnCFG_MAXSIZE) >> TLBnCFG_MAXSIZE_SHIFT;
> +        int i;
> +        for (i = min; i <= max; i++) {
> +            ret |= (1 << (i << 1));
> +        }
> +    }

For mav1 only the even sizes are supported.

-Scott

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

[Scott Wood]

On 01/20/2012 10:15 PM, Alexander Graf wrote:
> @@ -4273,6 +4274,16 @@ void helper_booke206_tlbwe(void)
>          tlb->mas1 &= ~MAS1_IPROT;
>      }
>  
> +    /* check that we support the targeted size */
> +    size_tlb = (tlb->mas1 & MAS1_TSIZE_MASK) >> MAS1_TSIZE_SHIFT;
> +    size_ps = booke206_tlbnps(env, tlbn);
> +    if ((tlb->mas1 & MAS1_VALID) && (tlbncfg & TLBnCFG_AVAIL) &&
> +        !(size_ps & (1 << size_tlb))) {
> +        helper_raise_exception_err(POWERPC_EXCP_PROGRAM,
> +                                   POWERPC_EXCP_INVAL |
> +                                   POWERPC_EXCP_INVAL_INVAL);
> +    }
> +
>      if (booke206_tlb_to_page_size(env, tlb) == TARGET_PAGE_SIZE) {
>          tlb_flush_page(env, tlb->mas2 & MAS2_EPN_MASK);
>      } else {

For tlb0 on e500 and derivatives, tsize is explicitly documented as
ignored.  Software may rely on this.

-Scott

Re: [Qemu-devel] [PATCH 6/8] PPC: booke: add tlbnps handling

[Alexander Graf]

On 01/23/2012 06:29 PM, Scott Wood wrote:
> On 01/20/2012 10:15 PM, Alexander Graf wrote:
>> +/* returns bitmap of supported page sizes for a given TLB */
>> +static inline uint32_t booke206_tlbnps(CPUState *env, const int tlbn)
>> +{
>> +    bool mav2 = false;
>> +    uint32_t ret = 0;
>> +
>> +    if (mav2) {
>> +        ret = env->spr[SPR_BOOKE_TLB0PS + tlbn];
>> +    } else {
>> +        uint32_t tlbncfg = env->spr[SPR_BOOKE_TLB0CFG + tlbn];
>> +        uint32_t min = (tlbncfg&  TLBnCFG_MINSIZE)>>  TLBnCFG_MINSIZE_SHIFT;
>> +        uint32_t max = (tlbncfg&  TLBnCFG_MAXSIZE)>>  TLBnCFG_MAXSIZE_SHIFT;
>> +        int i;
>> +        for (i = min; i<= max; i++) {
>> +            ret |= (1<<  (i<<  1));
>> +        }
>> +    }
> For mav1 only the even sizes are supported.

Yes, which is why min and max are >> 1 compared to the MAV2 values.

Alex

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

[Alexander Graf]

On 01/23/2012 06:32 PM, Scott Wood wrote:
> On 01/20/2012 10:15 PM, Alexander Graf wrote:
>> @@ -4273,6 +4274,16 @@ void helper_booke206_tlbwe(void)
>>           tlb->mas1&= ~MAS1_IPROT;
>>       }
>>
>> +    /* check that we support the targeted size */
>> +    size_tlb = (tlb->mas1&  MAS1_TSIZE_MASK)>>  MAS1_TSIZE_SHIFT;
>> +    size_ps = booke206_tlbnps(env, tlbn);
>> +    if ((tlb->mas1&  MAS1_VALID)&&  (tlbncfg&  TLBnCFG_AVAIL)&&
>> +        !(size_ps&  (1<<  size_tlb))) {
>> +        helper_raise_exception_err(POWERPC_EXCP_PROGRAM,
>> +                                   POWERPC_EXCP_INVAL |
>> +                                   POWERPC_EXCP_INVAL_INVAL);
>> +    }
>> +
>>       if (booke206_tlb_to_page_size(env, tlb) == TARGET_PAGE_SIZE) {
>>           tlb_flush_page(env, tlb->mas2&  MAS2_EPN_MASK);
>>       } else {
> For tlb0 on e500 and derivatives, tsize is explicitly documented as
> ignored.  Software may rely on this.

Yup, that's why there's the check for TLBnCG_AVAIL, which indicates that 
a TLB has dynamic page size capabilities, which TLB0 does not have.


Alex

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

[Scott Wood]

On 01/23/2012 11:33 AM, Alexander Graf wrote:
> On 01/23/2012 06:32 PM, Scott Wood wrote:
>> On 01/20/2012 10:15 PM, Alexander Graf wrote:
>>> @@ -4273,6 +4274,16 @@ void helper_booke206_tlbwe(void)
>>>           tlb->mas1&= ~MAS1_IPROT;
>>>       }
>>>
>>> +    /* check that we support the targeted size */
>>> +    size_tlb = (tlb->mas1&  MAS1_TSIZE_MASK)>>  MAS1_TSIZE_SHIFT;
>>> +    size_ps = booke206_tlbnps(env, tlbn);
>>> +    if ((tlb->mas1&  MAS1_VALID)&&  (tlbncfg&  TLBnCFG_AVAIL)&&
>>> +        !(size_ps&  (1<<  size_tlb))) {
>>> +        helper_raise_exception_err(POWERPC_EXCP_PROGRAM,
>>> +                                   POWERPC_EXCP_INVAL |
>>> +                                   POWERPC_EXCP_INVAL_INVAL);
>>> +    }
>>> +
>>>       if (booke206_tlb_to_page_size(env, tlb) == TARGET_PAGE_SIZE) {
>>>           tlb_flush_page(env, tlb->mas2&  MAS2_EPN_MASK);
>>>       } else {
>> For tlb0 on e500 and derivatives, tsize is explicitly documented as
>> ignored.  Software may rely on this.
> 
> Yup, that's why there's the check for TLBnCG_AVAIL, which indicates that
> a TLB has dynamic page size capabilities, which TLB0 does not have.

Silly me, thinking "avail" meant "this TLB is available" instead of
looking up the actual meaning. :-P

Where do we check whether the TLB exists at all?

-Scott

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

[Alexander Graf]

On 01/23/2012 07:19 PM, Scott Wood wrote:
> On 01/23/2012 11:33 AM, Alexander Graf wrote:
>> On 01/23/2012 06:32 PM, Scott Wood wrote:
>>> On 01/20/2012 10:15 PM, Alexander Graf wrote:
>>>> @@ -4273,6 +4274,16 @@ void helper_booke206_tlbwe(void)
>>>>            tlb->mas1&= ~MAS1_IPROT;
>>>>        }
>>>>
>>>> +    /* check that we support the targeted size */
>>>> +    size_tlb = (tlb->mas1&   MAS1_TSIZE_MASK)>>   MAS1_TSIZE_SHIFT;
>>>> +    size_ps = booke206_tlbnps(env, tlbn);
>>>> +    if ((tlb->mas1&   MAS1_VALID)&&   (tlbncfg&   TLBnCFG_AVAIL)&&
>>>> +        !(size_ps&   (1<<   size_tlb))) {
>>>> +        helper_raise_exception_err(POWERPC_EXCP_PROGRAM,
>>>> +                                   POWERPC_EXCP_INVAL |
>>>> +                                   POWERPC_EXCP_INVAL_INVAL);
>>>> +    }
>>>> +
>>>>        if (booke206_tlb_to_page_size(env, tlb) == TARGET_PAGE_SIZE) {
>>>>            tlb_flush_page(env, tlb->mas2&   MAS2_EPN_MASK);
>>>>        } else {
>>> For tlb0 on e500 and derivatives, tsize is explicitly documented as
>>> ignored.  Software may rely on this.
>> Yup, that's why there's the check for TLBnCG_AVAIL, which indicates that
>> a TLB has dynamic page size capabilities, which TLB0 does not have.
> Silly me, thinking "avail" meant "this TLB is available" instead of
> looking up the actual meaning. :-P
>
> Where do we check whether the TLB exists at all?

We don't. Eventually TLB access goes through:

static inline ppcmas_tlb_t *booke206_get_tlbm(CPUState *env, const int tlbn,
                                               target_ulong ea, int way)
{
     int r;
     uint32_t ways = booke206_tlb_ways(env, tlbn);
     int ways_bits = ffs(ways) - 1;
     int tlb_bits = ffs(booke206_tlb_size(env, tlbn)) - 1;
     int i;

     way &= ways - 1;
     ea >>= MAS2_EPN_SHIFT;
     ea &= (1 << (tlb_bits - ways_bits)) - 1;
     r = (ea << ways_bits) | way;

     /* bump up to tlbn index */
     for (i = 0; i < tlbn; i++) {
         r += booke206_tlb_size(env, i);
     }

     return &env->tlb.tlbm[r];
}

Since unavailable TLBs have ways set to 0 and tlb_size is 0, we always 
end up with the last TLB entry that's available.

So if you do a tlbwe on tlbn=5 on TLB2, you write to the last entry of 
TLB1. Which actually is fine according to the spec:

If an invalid value is specified for MAS0TLBSEL
MAS0ESEL or MAS2EPN, either no TLB entry is written
by the tlbwe, or the tlbwe is performed as if some
implementation-dependent, valid value were substi-
tuted for the invalid value, or an Illegal Instruction
exception occurs.

We substitute it with a valid value :)


Alex

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

[Scott Wood]

On 01/23/2012 12:41 PM, Alexander Graf wrote:
>>> For tlb0 on e500 and derivatives, tsize is explicitly documented as
>>> ignored.  Software may rely on this.
>> Yup, that's why there's the check for TLBnCG_AVAIL, which indicates that
>> a TLB has dynamic page size capabilities, which TLB0 does not have.
> Silly me, thinking "avail" meant "this TLB is available" instead of
> looking up the actual meaning. :-P

But where do we fill in the size if TLBnCFG_AVAIL is not set?  If this
is TLB0 on e500, we can't trust that the target code provided a valid
size -- we need to force to 4K.

>> Where do we check whether the TLB exists at all?
> 
> We don't. Eventually TLB access goes through:
> 
> static inline ppcmas_tlb_t *booke206_get_tlbm(CPUState *env, const int
> tlbn,
>                                               target_ulong ea, int way)
> {
>     int r;
>     uint32_t ways = booke206_tlb_ways(env, tlbn);
>     int ways_bits = ffs(ways) - 1;
>     int tlb_bits = ffs(booke206_tlb_size(env, tlbn)) - 1;
>     int i;
> 
>     way &= ways - 1;
>     ea >>= MAS2_EPN_SHIFT;
>     ea &= (1 << (tlb_bits - ways_bits)) - 1;
>     r = (ea << ways_bits) | way;
> 
>     /* bump up to tlbn index */
>     for (i = 0; i < tlbn; i++) {
>         r += booke206_tlb_size(env, i);
>     }
> 
>     return &env->tlb.tlbm[r];
> }
> 
> Since unavailable TLBs have ways set to 0 and tlb_size is 0, we always
> end up with the last TLB entry that's available.

I think you end up with the first entry beyond the end of the array,
actually.

> So if you do a tlbwe on tlbn=5 on TLB2, you write to the last entry of
> TLB1. Which actually is fine according to the spec:
> 
> If an invalid value is specified for MAS0TLBSEL
> MAS0ESEL or MAS2EPN, either no TLB entry is written
> by the tlbwe, or the tlbwe is performed as if some
> implementation-dependent, valid value were substi-
> tuted for the invalid value, or an Illegal Instruction
> exception occurs.
> 
> We substitute it with a valid value :)

Even if I'm reading it wrong and you do somehow end up with the last
element of the array, how do you know it's valid to write this entry
there?  You haven't been checking that array's page size restrictions,
or way/set geometry.

-Scott

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

[Alexander Graf]

On 23.01.2012, at 19:49, Scott Wood <scottwood@freescale.com> wrote:

> On 01/23/2012 12:41 PM, Alexander Graf wrote:
>>>> For tlb0 on e500 and derivatives, tsize is explicitly documented as
>>>> ignored.  Software may rely on this.
>>> Yup, that's why there's the check for TLBnCG_AVAIL, which indicates that
>>> a TLB has dynamic page size capabilities, which TLB0 does not have.
>> Silly me, thinking "avail" meant "this TLB is available" instead of
>> looking up the actual meaning. :-P
> 
> But where do we fill in the size if TLBnCFG_AVAIL is not set?  If this
> is TLB0 on e500, we can't trust that the target code provided a valid
> size -- we need to force to 4K.

TLB0 has min=max=4k :)

> 
>>> Where do we check whether the TLB exists at all?
>> 
>> We don't. Eventually TLB access goes through:
>> 
>> static inline ppcmas_tlb_t *booke206_get_tlbm(CPUState *env, const int
>> tlbn,
>>                                              target_ulong ea, int way)
>> {
>>    int r;
>>    uint32_t ways = booke206_tlb_ways(env, tlbn);
>>    int ways_bits = ffs(ways) - 1;
>>    int tlb_bits = ffs(booke206_tlb_size(env, tlbn)) - 1;
>>    int i;
>> 
>>    way &= ways - 1;
>>    ea >>= MAS2_EPN_SHIFT;
>>    ea &= (1 << (tlb_bits - ways_bits)) - 1;
>>    r = (ea << ways_bits) | way;
>> 
>>    /* bump up to tlbn index */
>>    for (i = 0; i < tlbn; i++) {
>>        r += booke206_tlb_size(env, i);
>>    }
>> 
>>    return &env->tlb.tlbm[r];
>> }
>> 
>> Since unavailable TLBs have ways set to 0 and tlb_size is 0, we always
>> end up with the last TLB entry that's available.
> 
> I think you end up with the first entry beyond the end of the array,
> actually.

Yikes. Yeah :(

> 
>> So if you do a tlbwe on tlbn=5 on TLB2, you write to the last entry of
>> TLB1. Which actually is fine according to the spec:
>> 
>> If an invalid value is specified for MAS0TLBSEL
>> MAS0ESEL or MAS2EPN, either no TLB entry is written
>> by the tlbwe, or the tlbwe is performed as if some
>> implementation-dependent, valid value were substi-
>> tuted for the invalid value, or an Illegal Instruction
>> exception occurs.
>> 
>> We substitute it with a valid value :)
> 
> Even if I'm reading it wrong and you do somehow end up with the last
> element of the array, how do you know it's valid to write this entry
> there?  You haven't been checking that array's page size restrictions,
> or way/set geometry.

True. Maybe we should just always reserve a surplus TLB entry and have the current code work, basically making it be a nop?

Or we could add checks everywhere...

Alex

> 
> -Scott
> 

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

[Scott Wood]

On 01/23/2012 02:03 PM, Alexander Graf wrote:
> 
> 
> On 23.01.2012, at 19:49, Scott Wood <scottwood@freescale.com> wrote:
> 
>> On 01/23/2012 12:41 PM, Alexander Graf wrote:
>>>>> For tlb0 on e500 and derivatives, tsize is explicitly documented as
>>>>> ignored.  Software may rely on this.
>>>> Yup, that's why there's the check for TLBnCG_AVAIL, which indicates that
>>>> a TLB has dynamic page size capabilities, which TLB0 does not have.
>>> Silly me, thinking "avail" meant "this TLB is available" instead of
>>> looking up the actual meaning. :-P
>>
>> But where do we fill in the size if TLBnCFG_AVAIL is not set?  If this
>> is TLB0 on e500, we can't trust that the target code provided a valid
>> size -- we need to force to 4K.
> 
> TLB0 has min=max=4k :)

If TLB0 has TLBnCFG[AVAIL] set, then with this patch you'll be raising
an exception rather than setting the size to the minimum.

If TLB0 does not have TLBnCFG[AVAIL] set, you'll be letting the user set
whatever size they want.

In either case, you seem to be letting the user write whatever the want
to the TLB array, and only afterward check whether to send an exception.

> True. Maybe we should just always reserve a surplus TLB entry and have the current code work, basically making it be a nop?
> 
> Or we could add checks everywhere...

I'd have booke206_get_tlbm() check and return NULL, with callers
checking for that.  Optimization can come later, if/when it's shown to
be a bottleneck.

-Scott

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

[Alexander Graf]

On 23.01.2012, at 21:10, Scott Wood <scottwood@freescale.com> wrote:

> On 01/23/2012 02:03 PM, Alexander Graf wrote:
>> 
>> 
>> On 23.01.2012, at 19:49, Scott Wood <scottwood@freescale.com> wrote:
>> 
>>> On 01/23/2012 12:41 PM, Alexander Graf wrote:
>>>>>> For tlb0 on e500 and derivatives, tsize is explicitly documented as
>>>>>> ignored.  Software may rely on this.
>>>>> Yup, that's why there's the check for TLBnCG_AVAIL, which indicates that
>>>>> a TLB has dynamic page size capabilities, which TLB0 does not have.
>>>> Silly me, thinking "avail" meant "this TLB is available" instead of
>>>> looking up the actual meaning. :-P
>>> 
>>> But where do we fill in the size if TLBnCFG_AVAIL is not set?  If this
>>> is TLB0 on e500, we can't trust that the target code provided a valid
>>> size -- we need to force to 4K.
>> 
>> TLB0 has min=max=4k :)
> 
> If TLB0 has TLBnCFG[AVAIL] set, then with this patch you'll be raising
> an exception rather than setting the size to the minimum.
> 
> If TLB0 does not have TLBnCFG[AVAIL] set, you'll be letting the user set
> whatever size they want.
> 
> In either case, you seem to be letting the user write whatever the want
> to the TLB array, and only afterward check whether to send an exception.

Yes, for !AVAIL we simply override the page size on qemu tlb miss iirc.

Is that wrong? Does tlbwe;tlbre result in different tsize values?

> 
>> True. Maybe we should just always reserve a surplus TLB entry and have the current code work, basically making it be a nop?
>> 
>> Or we could add checks everywhere...
> 
> I'd have booke206_get_tlbm() check and return NULL, with callers
> checking for that.  Optimization can come later, if/when it's shown to
> be a bottleneck.

It's more about not missing any cases :). But yeah, it's probably best to just change the semantics.

Alex

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

[Scott Wood]

On 01/23/2012 03:29 PM, Alexander Graf wrote:
> 
> 
> On 23.01.2012, at 21:10, Scott Wood <scottwood@freescale.com> wrote:
> 
>> If TLB0 has TLBnCFG[AVAIL] set, then with this patch you'll be raising
>> an exception rather than setting the size to the minimum.
>>
>> If TLB0 does not have TLBnCFG[AVAIL] set, you'll be letting the user set
>> whatever size they want.
>>
>> In either case, you seem to be letting the user write whatever the want
>> to the TLB array, and only afterward check whether to send an exception.
> 
> Yes, for !AVAIL we simply override the page size on qemu tlb miss iirc.

Ah.  That seems like a hotter path than tlbwe, and you could still
insert an invalid entry into tlb1 (you'd get an exception, but the entry
would be there).

> Is that wrong? Does tlbwe;tlbre result in different tsize values?

e500mc manual (table 6-6, "MMU Assist Register Field Updates") says
tlbre returns a tsize of 1 for tlb0 -- it doesn't store tsize.  The KVM
MMU API also requires that tsize be stored as a valid value, to simplify
the code that operates on the TLB.  The TLB dump code depends on this
(could be fixed of course, but simpler to fix it once in tlbwe).

>>> True. Maybe we should just always reserve a surplus TLB entry and have the current code work, basically making it be a nop?
>>>
>>> Or we could add checks everywhere...
>>
>> I'd have booke206_get_tlbm() check and return NULL, with callers
>> checking for that.  Optimization can come later, if/when it's shown to
>> be a bottleneck.
> 
> It's more about not missing any cases :). But yeah, it's probably best to just change the semantics.

At least a NULL deference will be more noticeable than an array overrun...

-Scott