[PATCH 0/7] Embedded PPC misc clean up and optimisation

[PATCH 0/7] Embedded PPC misc clean up and optimisation

[BALATON Zoltan]

Hello,

This series improves embedded PPC TLB emulation a bit and contains
some misc clean up I've found along the way. Before this patch
ppcemb_tlb_check() shows up in a memory access intensive profile
(running RageMem speed test in AmigaOS on sam460ex) at 11.91%
children, 10.77% self. After this series it does not show up at all.
This is not the biggest bottleneck, that is calling tlb_flush() from
helper_440_tlbwe() excessively but this was simpler to clean up and
still makes a small improvement.

RageMem results on master:
---> RAM <---
READ32:  593 MB/Sec
READ64:  616 MB/Sec
WRITE32: 589 MB/Sec
WRITE64: 621 MB/Sec
WRITE: 518 MB/Sec (Tricky)

---> VIDEO BUS <---
READ:  588 MB/Sec
WRITE: 571 MB/Sec

with this series:
---> RAM <---
READ32:  674 MB/Sec
READ64:  707 MB/Sec
WRITE32: 665 MB/Sec
WRITE64: 714 MB/Sec
WRITE: 580 MB/Sec (Tricky)

---> VIDEO BUS <---
READ:  691 MB/Sec
WRITE: 662 MB/Sec

The results have some jitter but both the higher values and that the
function is gone from the profile can prove the series has an effect.
If nothing else then simplifying the code a bit. For comparison this
is faster than a real sam460ex but much slower than running the same
with -M pegasos2 so embedded PPC TLB emulation still might need some
improvement. I know these are different and PPC440 has software
assisted TLB but the problem with it seems to be too much tlb_flushes
not that it needs more exceptions.

(If somebody is interested to reproduce and experiment with it the
benchmarks and some results are available from here:
https://www.amigans.net/modules/newbb/viewtopic.php?topic_id=9226
some of the tests also have MorphOS versions that's easier to get than
AmigaOS or sources that could be compiled under Linux.)

Regards,
BALATON Zoltan

BALATON Zoltan (7):
  target/ppc: Remove single use function
  target/ppc: Remove "ext" parameter of ppcemb_tlb_check()
  target/ppc: Move ppcemb_tlb_search() to mmu_common.c
  target/ppc: Remove some unneded line breaks
  target/ppc: Simplify ppcemb_tlb_search()
  target/ppc: Change ppcemb_tlb_check() to return bool
  target/ppc: Eliminate goto in mmubooke_check_tlb()

 target/ppc/cpu.h        | 13 ++----
 target/ppc/mmu_common.c | 91 +++++++++++++++++++++++------------------
 target/ppc/mmu_helper.c | 32 +--------------
 3 files changed, 57 insertions(+), 79 deletions(-)

-- 
2.30.9

[PATCH 1/7] target/ppc: Remove single use function

[BALATON Zoltan]

The get_physical_address() function is a trivial wrapper of
get_physical_address_wtlb() that is only used once. Remove it and call
get_physical_address_wtlb() directly instead.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 target/ppc/mmu_helper.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 64e30435f5..c0c71a68ff 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -168,15 +168,6 @@ static void booke206_flush_tlb(CPUPPCState *env, int flags,
     tlb_flush(env_cpu(env));
 }
 
-static int get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
-                                target_ulong eaddr, MMUAccessType access_type,
-                                int type)
-{
-    return get_physical_address_wtlb(env, ctx, eaddr, access_type, type, 0);
-}
-
-
-
 /*****************************************************************************/
 /* BATs management */
 #if !defined(FLUSH_ALL_TLBS)
@@ -643,7 +634,7 @@ target_ulong helper_rac(CPUPPCState *env, target_ulong addr)
      */
     nb_BATs = env->nb_BATs;
     env->nb_BATs = 0;
-    if (get_physical_address(env, &ctx, addr, 0, ACCESS_INT) == 0) {
+    if (get_physical_address_wtlb(env, &ctx, addr, 0, ACCESS_INT, 0) == 0) {
         ret = ctx.raddr;
     }
     env->nb_BATs = nb_BATs;
-- 
2.30.9

[PATCH 2/7] target/ppc: Remove "ext" parameter of ppcemb_tlb_check()

[BALATON Zoltan]

This is only used by one caller so simplify function by removing this
parameter and move the operation to the single place where it's used.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 target/ppc/cpu.h        |  3 +--
 target/ppc/mmu_common.c | 21 +++++++++------------
 target/ppc/mmu_helper.c |  2 +-
 3 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 0f9f2e1a0c..5cd1b442b4 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1430,8 +1430,7 @@ int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb,
                             uint32_t pid);
 int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
                             hwaddr *raddrp,
-                            target_ulong address, uint32_t pid, int ext,
-                            int i);
+                            target_ulong address, uint32_t pid, int i);
 hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
                                         ppcmas_tlb_t *tlb);
 #endif
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index 7235a4befe..21a353c51a 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -491,8 +491,7 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
 /* Generic TLB check function for embedded PowerPC implementations */
 int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
                             hwaddr *raddrp,
-                            target_ulong address, uint32_t pid, int ext,
-                            int i)
+                            target_ulong address, uint32_t pid, int i)
 {
     target_ulong mask;
 
@@ -514,11 +513,6 @@ int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
         return -1;
     }
     *raddrp = (tlb->RPN & mask) | (address & ~mask);
-    if (ext) {
-        /* Extend the physical address to 36 bits */
-        *raddrp |= (uint64_t)(tlb->RPN & 0xF) << 32;
-    }
-
     return 0;
 }
 
@@ -536,7 +530,7 @@ static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
     for (i = 0; i < env->nb_tlb; i++) {
         tlb = &env->tlb.tlbe[i];
         if (ppcemb_tlb_check(env, tlb, &raddr, address,
-                             env->spr[SPR_40x_PID], 0, i) < 0) {
+                             env->spr[SPR_40x_PID], i) < 0) {
             continue;
         }
         zsel = (tlb->attr >> 4) & 0xF;
@@ -598,20 +592,23 @@ static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb,
     int prot2;
 
     if (ppcemb_tlb_check(env, tlb, raddr, address,
-                         env->spr[SPR_BOOKE_PID],
-                         !env->nb_pids, i) >= 0) {
+                         env->spr[SPR_BOOKE_PID], i) >= 0) {
+        if (!env->nb_pids) {
+            /* Extend the physical address to 36 bits */
+            *raddr |= (uint64_t)(tlb->RPN & 0xF) << 32;
+        }
         goto found_tlb;
     }
 
     if (env->spr[SPR_BOOKE_PID1] &&
         ppcemb_tlb_check(env, tlb, raddr, address,
-                         env->spr[SPR_BOOKE_PID1], 0, i) >= 0) {
+                         env->spr[SPR_BOOKE_PID1], i) >= 0) {
         goto found_tlb;
     }
 
     if (env->spr[SPR_BOOKE_PID2] &&
         ppcemb_tlb_check(env, tlb, raddr, address,
-                         env->spr[SPR_BOOKE_PID2], 0, i) >= 0) {
+                         env->spr[SPR_BOOKE_PID2], i) >= 0) {
         goto found_tlb;
     }
 
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index c0c71a68ff..e7275eaec1 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -124,7 +124,7 @@ static int ppcemb_tlb_search(CPUPPCState *env, target_ulong address,
     ret = -1;
     for (i = 0; i < env->nb_tlb; i++) {
         tlb = &env->tlb.tlbe[i];
-        if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, 0, i) == 0) {
+        if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) {
             ret = i;
             break;
         }
-- 
2.30.9

[PATCH 3/7] target/ppc: Move ppcemb_tlb_search() to mmu_common.c

[BALATON Zoltan]

This function is the only reason why ppcemb_tlb_check() is not static
to mmu_common.c but it also better fits in mmu_common.c so move it
there.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 target/ppc/cpu.h        |  4 +---
 target/ppc/mmu_common.c | 22 +++++++++++++++++++++-
 target/ppc/mmu_helper.c | 21 ---------------------
 3 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 5cd1b442b4..77eb5edea2 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1428,9 +1428,7 @@ void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp);
 int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb,
                             hwaddr *raddrp, target_ulong address,
                             uint32_t pid);
-int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
-                            hwaddr *raddrp,
-                            target_ulong address, uint32_t pid, int i);
+int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid);
 hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
                                         ppcmas_tlb_t *tlb);
 #endif
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index 21a353c51a..845eee4c6f 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -489,7 +489,7 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
 }
 
 /* Generic TLB check function for embedded PowerPC implementations */
-int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
+static int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
                             hwaddr *raddrp,
                             target_ulong address, uint32_t pid, int i)
 {
@@ -516,6 +516,26 @@ int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
     return 0;
 }
 
+/* Generic TLB search function for PowerPC embedded implementations */
+int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid)
+{
+    ppcemb_tlb_t *tlb;
+    hwaddr raddr;
+    int i, ret;
+
+    /* Default return value is no match */
+    ret = -1;
+    for (i = 0; i < env->nb_tlb; i++) {
+        tlb = &env->tlb.tlbe[i];
+        if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) {
+            ret = i;
+            break;
+        }
+    }
+
+    return ret;
+}
+
 static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
                                        target_ulong address,
                                        MMUAccessType access_type)
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index e7275eaec1..d3ea7588f9 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -112,27 +112,6 @@ static void ppc6xx_tlb_store(CPUPPCState *env, target_ulong EPN, int way,
     env->last_way = way;
 }
 
-/* Generic TLB search function for PowerPC embedded implementations */
-static int ppcemb_tlb_search(CPUPPCState *env, target_ulong address,
-                             uint32_t pid)
-{
-    ppcemb_tlb_t *tlb;
-    hwaddr raddr;
-    int i, ret;
-
-    /* Default return value is no match */
-    ret = -1;
-    for (i = 0; i < env->nb_tlb; i++) {
-        tlb = &env->tlb.tlbe[i];
-        if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) {
-            ret = i;
-            break;
-        }
-    }
-
-    return ret;
-}
-
 /* Helpers specific to PowerPC 40x implementations */
 static inline void ppc4xx_tlb_invalidate_all(CPUPPCState *env)
 {
-- 
2.30.9

[PATCH 4/7] target/ppc: Remove some unneded line breaks

[BALATON Zoltan]

Make lines shorter and fix indentation in some functions prototypes.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 target/ppc/cpu.h        | 8 +++-----
 target/ppc/mmu_common.c | 8 +++-----
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 77eb5edea2..4545f74fdd 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1425,12 +1425,10 @@ void store_booke_tsr(CPUPPCState *env, target_ulong val);
 void ppc_tlb_invalidate_all(CPUPPCState *env);
 void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr);
 void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp);
-int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb,
-                            hwaddr *raddrp, target_ulong address,
-                            uint32_t pid);
+int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, hwaddr *raddrp,
+                     target_ulong address, uint32_t pid);
 int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid);
-hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
-                                        ppcmas_tlb_t *tlb);
+hwaddr booke206_tlb_to_page_size(CPUPPCState *env, ppcmas_tlb_t *tlb);
 #endif
 #endif
 
diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index 845eee4c6f..a84bc7de88 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -694,8 +694,7 @@ static int mmubooke_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
     return ret;
 }
 
-hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
-                                        ppcmas_tlb_t *tlb)
+hwaddr booke206_tlb_to_page_size(CPUPPCState *env, ppcmas_tlb_t *tlb)
 {
     int tlbm_size;
 
@@ -705,9 +704,8 @@ hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
 }
 
 /* TLB check function for MAS based SoftTLBs */
-int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb,
-                            hwaddr *raddrp, target_ulong address,
-                            uint32_t pid)
+int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, hwaddr *raddrp,
+                     target_ulong address, uint32_t pid)
 {
     hwaddr mask;
     uint32_t tlb_pid;
-- 
2.30.9

[PATCH 5/7] target/ppc: Simplify ppcemb_tlb_search()

[BALATON Zoltan]

No nead to store return value and break from loop when we can return
directly.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 target/ppc/mmu_common.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index a84bc7de88..ff7f987546 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -521,19 +521,15 @@ int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid)
 {
     ppcemb_tlb_t *tlb;
     hwaddr raddr;
-    int i, ret;
+    int i;
 
-    /* Default return value is no match */
-    ret = -1;
     for (i = 0; i < env->nb_tlb; i++) {
         tlb = &env->tlb.tlbe[i];
         if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) {
-            ret = i;
-            break;
+            return i;
         }
     }
-
-    return ret;
+    return -1;
 }
 
 static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
-- 
2.30.9

[PATCH 6/7] target/ppc: Change ppcemb_tlb_check() to return bool

[BALATON Zoltan]

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 target/ppc/mmu_common.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index ff7f987546..bd7d7d5257 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -489,15 +489,15 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
 }
 
 /* Generic TLB check function for embedded PowerPC implementations */
-static int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
-                            hwaddr *raddrp,
-                            target_ulong address, uint32_t pid, int i)
+static bool ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
+                             hwaddr *raddrp,
+                             target_ulong address, uint32_t pid, int i)
 {
     target_ulong mask;
 
     /* Check valid flag */
     if (!(tlb->prot & PAGE_VALID)) {
-        return -1;
+        return false;
     }
     mask = ~(tlb->size - 1);
     qemu_log_mask(CPU_LOG_MMU, "%s: TLB %d address " TARGET_FMT_lx
@@ -506,14 +506,14 @@ static int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
                   mask, (uint32_t)tlb->PID, tlb->prot);
     /* Check PID */
     if (tlb->PID != 0 && tlb->PID != pid) {
-        return -1;
+        return false;
     }
     /* Check effective address */
     if ((address & mask) != tlb->EPN) {
-        return -1;
+        return false;
     }
     *raddrp = (tlb->RPN & mask) | (address & ~mask);
-    return 0;
+    return true;
 }
 
 /* Generic TLB search function for PowerPC embedded implementations */
@@ -525,7 +525,7 @@ int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid)
 
     for (i = 0; i < env->nb_tlb; i++) {
         tlb = &env->tlb.tlbe[i];
-        if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) {
+        if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i)) {
             return i;
         }
     }
@@ -545,8 +545,8 @@ static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
     pr = FIELD_EX64(env->msr, MSR, PR);
     for (i = 0; i < env->nb_tlb; i++) {
         tlb = &env->tlb.tlbe[i];
-        if (ppcemb_tlb_check(env, tlb, &raddr, address,
-                             env->spr[SPR_40x_PID], i) < 0) {
+        if (!ppcemb_tlb_check(env, tlb, &raddr, address,
+                              env->spr[SPR_40x_PID], i)) {
             continue;
         }
         zsel = (tlb->attr >> 4) & 0xF;
@@ -608,7 +608,7 @@ static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb,
     int prot2;
 
     if (ppcemb_tlb_check(env, tlb, raddr, address,
-                         env->spr[SPR_BOOKE_PID], i) >= 0) {
+                         env->spr[SPR_BOOKE_PID], i)) {
         if (!env->nb_pids) {
             /* Extend the physical address to 36 bits */
             *raddr |= (uint64_t)(tlb->RPN & 0xF) << 32;
@@ -618,13 +618,13 @@ static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb,
 
     if (env->spr[SPR_BOOKE_PID1] &&
         ppcemb_tlb_check(env, tlb, raddr, address,
-                         env->spr[SPR_BOOKE_PID1], i) >= 0) {
+                         env->spr[SPR_BOOKE_PID1], i)) {
         goto found_tlb;
     }
 
     if (env->spr[SPR_BOOKE_PID2] &&
         ppcemb_tlb_check(env, tlb, raddr, address,
-                         env->spr[SPR_BOOKE_PID2], i) >= 0) {
+                         env->spr[SPR_BOOKE_PID2], i)) {
         goto found_tlb;
     }
 
-- 
2.30.9

[PATCH 7/7] target/ppc: Eliminate goto in mmubooke_check_tlb()

[BALATON Zoltan]

Move out checking PID registers into a separate function which makes
mmubooke_check_tlb() simpler and avoids using goto.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 target/ppc/mmu_common.c | 40 +++++++++++++++++++++-------------------
 1 file changed, 21 insertions(+), 19 deletions(-)

diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
index bd7d7d5257..ae1db6e348 100644
--- a/target/ppc/mmu_common.c
+++ b/target/ppc/mmu_common.c
@@ -601,37 +601,39 @@ static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
     return ret;
 }
 
-static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb,
-                              hwaddr *raddr, int *prot, target_ulong address,
-                              MMUAccessType access_type, int i)
+static bool mmubooke_check_pid(CPUPPCState *env, ppcemb_tlb_t *tlb,
+                               hwaddr *raddr, target_ulong addr, int i)
 {
-    int prot2;
-
-    if (ppcemb_tlb_check(env, tlb, raddr, address,
-                         env->spr[SPR_BOOKE_PID], i)) {
+    if (ppcemb_tlb_check(env, tlb, raddr, addr, env->spr[SPR_BOOKE_PID], i)) {
         if (!env->nb_pids) {
             /* Extend the physical address to 36 bits */
             *raddr |= (uint64_t)(tlb->RPN & 0xF) << 32;
         }
-        goto found_tlb;
+        return true;
+    } else if (!env->nb_pids) {
+        return false;
     }
-
     if (env->spr[SPR_BOOKE_PID1] &&
-        ppcemb_tlb_check(env, tlb, raddr, address,
-                         env->spr[SPR_BOOKE_PID1], i)) {
-        goto found_tlb;
+        ppcemb_tlb_check(env, tlb, raddr, addr, env->spr[SPR_BOOKE_PID1], i)) {
+        return true;
     }
-
     if (env->spr[SPR_BOOKE_PID2] &&
-        ppcemb_tlb_check(env, tlb, raddr, address,
-                         env->spr[SPR_BOOKE_PID2], i)) {
-        goto found_tlb;
+        ppcemb_tlb_check(env, tlb, raddr, addr, env->spr[SPR_BOOKE_PID2], i)) {
+        return true;
     }
+    return false;
+}
 
-     qemu_log_mask(CPU_LOG_MMU, "%s: TLB entry not found\n", __func__);
-    return -1;
+static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb,
+                              hwaddr *raddr, int *prot, target_ulong address,
+                              MMUAccessType access_type, int i)
+{
+    int prot2;
 
-found_tlb:
+    if (!mmubooke_check_pid(env, tlb, raddr, address, i)) {
+        qemu_log_mask(CPU_LOG_MMU, "%s: TLB entry not found\n", __func__);
+        return -1;
+    }
 
     if (FIELD_EX64(env->msr, MSR, PR)) {
         prot2 = tlb->prot & 0xF;
-- 
2.30.9

Re: [PATCH 1/7] target/ppc: Remove single use function

[Cédric Le Goater]

On 5/30/23 15:28, BALATON Zoltan wrote:
> The get_physical_address() function is a trivial wrapper of
> get_physical_address_wtlb() that is only used once. Remove it and call
> get_physical_address_wtlb() directly instead.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   target/ppc/mmu_helper.c | 11 +----------
>   1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 64e30435f5..c0c71a68ff 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -168,15 +168,6 @@ static void booke206_flush_tlb(CPUPPCState *env, int flags,
>       tlb_flush(env_cpu(env));
>   }
>   
> -static int get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
> -                                target_ulong eaddr, MMUAccessType access_type,
> -                                int type)
> -{
> -    return get_physical_address_wtlb(env, ctx, eaddr, access_type, type, 0);
> -}
> -
> -
> -
>   /*****************************************************************************/
>   /* BATs management */
>   #if !defined(FLUSH_ALL_TLBS)
> @@ -643,7 +634,7 @@ target_ulong helper_rac(CPUPPCState *env, target_ulong addr)
>        */
>       nb_BATs = env->nb_BATs;
>       env->nb_BATs = 0;
> -    if (get_physical_address(env, &ctx, addr, 0, ACCESS_INT) == 0) {
> +    if (get_physical_address_wtlb(env, &ctx, addr, 0, ACCESS_INT, 0) == 0) {
>           ret = ctx.raddr;
>       }
>       env->nb_BATs = nb_BATs;

Re: [PATCH 2/7] target/ppc: Remove "ext" parameter of ppcemb_tlb_check()

[Cédric Le Goater]

On 5/30/23 15:28, BALATON Zoltan wrote:
> This is only used by one caller so simplify function by removing this
> parameter and move the operation to the single place where it's used.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   target/ppc/cpu.h        |  3 +--
>   target/ppc/mmu_common.c | 21 +++++++++------------
>   target/ppc/mmu_helper.c |  2 +-
>   3 files changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 0f9f2e1a0c..5cd1b442b4 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1430,8 +1430,7 @@ int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb,
>                               uint32_t pid);
>   int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
>                               hwaddr *raddrp,
> -                            target_ulong address, uint32_t pid, int ext,
> -                            int i);
> +                            target_ulong address, uint32_t pid, int i);
>   hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
>                                           ppcmas_tlb_t *tlb);
>   #endif
> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
> index 7235a4befe..21a353c51a 100644
> --- a/target/ppc/mmu_common.c
> +++ b/target/ppc/mmu_common.c
> @@ -491,8 +491,7 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
>   /* Generic TLB check function for embedded PowerPC implementations */
>   int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
>                               hwaddr *raddrp,
> -                            target_ulong address, uint32_t pid, int ext,
> -                            int i)
> +                            target_ulong address, uint32_t pid, int i)
>   {
>       target_ulong mask;
>   
> @@ -514,11 +513,6 @@ int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
>           return -1;
>       }
>       *raddrp = (tlb->RPN & mask) | (address & ~mask);
> -    if (ext) {
> -        /* Extend the physical address to 36 bits */
> -        *raddrp |= (uint64_t)(tlb->RPN & 0xF) << 32;
> -    }
> -
>       return 0;
>   }
>   
> @@ -536,7 +530,7 @@ static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
>       for (i = 0; i < env->nb_tlb; i++) {
>           tlb = &env->tlb.tlbe[i];
>           if (ppcemb_tlb_check(env, tlb, &raddr, address,
> -                             env->spr[SPR_40x_PID], 0, i) < 0) {
> +                             env->spr[SPR_40x_PID], i) < 0) {
>               continue;
>           }
>           zsel = (tlb->attr >> 4) & 0xF;
> @@ -598,20 +592,23 @@ static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb,
>       int prot2;
>   
>       if (ppcemb_tlb_check(env, tlb, raddr, address,
> -                         env->spr[SPR_BOOKE_PID],
> -                         !env->nb_pids, i) >= 0) {
> +                         env->spr[SPR_BOOKE_PID], i) >= 0) {
> +        if (!env->nb_pids) {
> +            /* Extend the physical address to 36 bits */
> +            *raddr |= (uint64_t)(tlb->RPN & 0xF) << 32;
> +        }
>           goto found_tlb;
>       }
>   
>       if (env->spr[SPR_BOOKE_PID1] &&
>           ppcemb_tlb_check(env, tlb, raddr, address,
> -                         env->spr[SPR_BOOKE_PID1], 0, i) >= 0) {
> +                         env->spr[SPR_BOOKE_PID1], i) >= 0) {
>           goto found_tlb;
>       }
>   
>       if (env->spr[SPR_BOOKE_PID2] &&
>           ppcemb_tlb_check(env, tlb, raddr, address,
> -                         env->spr[SPR_BOOKE_PID2], 0, i) >= 0) {
> +                         env->spr[SPR_BOOKE_PID2], i) >= 0) {
>           goto found_tlb;
>       }
>   
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index c0c71a68ff..e7275eaec1 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -124,7 +124,7 @@ static int ppcemb_tlb_search(CPUPPCState *env, target_ulong address,
>       ret = -1;
>       for (i = 0; i < env->nb_tlb; i++) {
>           tlb = &env->tlb.tlbe[i];
> -        if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, 0, i) == 0) {
> +        if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) {
>               ret = i;
>               break;
>           }

Re: [PATCH 3/7] target/ppc: Move ppcemb_tlb_search() to mmu_common.c

[Cédric Le Goater]

On 5/30/23 15:28, BALATON Zoltan wrote:
> This function is the only reason why ppcemb_tlb_check() is not static
> to mmu_common.c but it also better fits in mmu_common.c so move it
> there.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   target/ppc/cpu.h        |  4 +---
>   target/ppc/mmu_common.c | 22 +++++++++++++++++++++-
>   target/ppc/mmu_helper.c | 21 ---------------------
>   3 files changed, 22 insertions(+), 25 deletions(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 5cd1b442b4..77eb5edea2 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1428,9 +1428,7 @@ void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp);
>   int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb,
>                               hwaddr *raddrp, target_ulong address,
>                               uint32_t pid);
> -int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
> -                            hwaddr *raddrp,
> -                            target_ulong address, uint32_t pid, int i);
> +int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid);
>   hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
>                                           ppcmas_tlb_t *tlb);
>   #endif
> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
> index 21a353c51a..845eee4c6f 100644
> --- a/target/ppc/mmu_common.c
> +++ b/target/ppc/mmu_common.c
> @@ -489,7 +489,7 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
>   }
>   
>   /* Generic TLB check function for embedded PowerPC implementations */
> -int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
> +static int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
>                               hwaddr *raddrp,
>                               target_ulong address, uint32_t pid, int i)
>   {
> @@ -516,6 +516,26 @@ int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
>       return 0;
>   }
>   
> +/* Generic TLB search function for PowerPC embedded implementations */
> +int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid)
> +{
> +    ppcemb_tlb_t *tlb;
> +    hwaddr raddr;
> +    int i, ret;
> +
> +    /* Default return value is no match */
> +    ret = -1;
> +    for (i = 0; i < env->nb_tlb; i++) {
> +        tlb = &env->tlb.tlbe[i];
> +        if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) {
> +            ret = i;
> +            break;
> +        }
> +    }
> +
> +    return ret;
> +}
> +
>   static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
>                                          target_ulong address,
>                                          MMUAccessType access_type)
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index e7275eaec1..d3ea7588f9 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -112,27 +112,6 @@ static void ppc6xx_tlb_store(CPUPPCState *env, target_ulong EPN, int way,
>       env->last_way = way;
>   }
>   
> -/* Generic TLB search function for PowerPC embedded implementations */
> -static int ppcemb_tlb_search(CPUPPCState *env, target_ulong address,
> -                             uint32_t pid)
> -{
> -    ppcemb_tlb_t *tlb;
> -    hwaddr raddr;
> -    int i, ret;
> -
> -    /* Default return value is no match */
> -    ret = -1;
> -    for (i = 0; i < env->nb_tlb; i++) {
> -        tlb = &env->tlb.tlbe[i];
> -        if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) {
> -            ret = i;
> -            break;
> -        }
> -    }
> -
> -    return ret;
> -}
> -
>   /* Helpers specific to PowerPC 40x implementations */
>   static inline void ppc4xx_tlb_invalidate_all(CPUPPCState *env)
>   {

Re: [PATCH 4/7] target/ppc: Remove some unneded line breaks

[Cédric Le Goater]

On 5/30/23 15:28, BALATON Zoltan wrote:
> Make lines shorter and fix indentation in some functions prototypes.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   target/ppc/cpu.h        | 8 +++-----
>   target/ppc/mmu_common.c | 8 +++-----
>   2 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 77eb5edea2..4545f74fdd 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1425,12 +1425,10 @@ void store_booke_tsr(CPUPPCState *env, target_ulong val);
>   void ppc_tlb_invalidate_all(CPUPPCState *env);
>   void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr);
>   void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp);
> -int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb,
> -                            hwaddr *raddrp, target_ulong address,
> -                            uint32_t pid);
> +int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, hwaddr *raddrp,
> +                     target_ulong address, uint32_t pid);
>   int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid);
> -hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
> -                                        ppcmas_tlb_t *tlb);
> +hwaddr booke206_tlb_to_page_size(CPUPPCState *env, ppcmas_tlb_t *tlb);
>   #endif
>   #endif
>   
> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
> index 845eee4c6f..a84bc7de88 100644
> --- a/target/ppc/mmu_common.c
> +++ b/target/ppc/mmu_common.c
> @@ -694,8 +694,7 @@ static int mmubooke_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
>       return ret;
>   }
>   
> -hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
> -                                        ppcmas_tlb_t *tlb)
> +hwaddr booke206_tlb_to_page_size(CPUPPCState *env, ppcmas_tlb_t *tlb)
>   {
>       int tlbm_size;
>   
> @@ -705,9 +704,8 @@ hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
>   }
>   
>   /* TLB check function for MAS based SoftTLBs */
> -int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb,
> -                            hwaddr *raddrp, target_ulong address,
> -                            uint32_t pid)
> +int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, hwaddr *raddrp,
> +                     target_ulong address, uint32_t pid)
>   {
>       hwaddr mask;
>       uint32_t tlb_pid;

Re: [PATCH 5/7] target/ppc: Simplify ppcemb_tlb_search()

[Cédric Le Goater]

On 5/30/23 15:28, BALATON Zoltan wrote:
> No nead to store return value and break from loop when we can return
> directly.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   target/ppc/mmu_common.c | 10 +++-------
>   1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
> index a84bc7de88..ff7f987546 100644
> --- a/target/ppc/mmu_common.c
> +++ b/target/ppc/mmu_common.c
> @@ -521,19 +521,15 @@ int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid)
>   {
>       ppcemb_tlb_t *tlb;
>       hwaddr raddr;
> -    int i, ret;
> +    int i;
>   
> -    /* Default return value is no match */
> -    ret = -1;
>       for (i = 0; i < env->nb_tlb; i++) {
>           tlb = &env->tlb.tlbe[i];
>           if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) {
> -            ret = i;
> -            break;
> +            return i;
>           }
>       }
> -
> -    return ret;
> +    return -1;
>   }
>   
>   static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,

Re: [PATCH 6/7] target/ppc: Change ppcemb_tlb_check() to return bool

[Cédric Le Goater]

On 5/30/23 15:28, BALATON Zoltan wrote:
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   target/ppc/mmu_common.c | 26 +++++++++++++-------------
>   1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
> index ff7f987546..bd7d7d5257 100644
> --- a/target/ppc/mmu_common.c
> +++ b/target/ppc/mmu_common.c
> @@ -489,15 +489,15 @@ static int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
>   }
>   
>   /* Generic TLB check function for embedded PowerPC implementations */
> -static int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
> -                            hwaddr *raddrp,
> -                            target_ulong address, uint32_t pid, int i)
> +static bool ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
> +                             hwaddr *raddrp,
> +                             target_ulong address, uint32_t pid, int i)
>   {
>       target_ulong mask;
>   
>       /* Check valid flag */
>       if (!(tlb->prot & PAGE_VALID)) {
> -        return -1;
> +        return false;
>       }
>       mask = ~(tlb->size - 1);
>       qemu_log_mask(CPU_LOG_MMU, "%s: TLB %d address " TARGET_FMT_lx
> @@ -506,14 +506,14 @@ static int ppcemb_tlb_check(CPUPPCState *env, ppcemb_tlb_t *tlb,
>                     mask, (uint32_t)tlb->PID, tlb->prot);
>       /* Check PID */
>       if (tlb->PID != 0 && tlb->PID != pid) {
> -        return -1;
> +        return false;
>       }
>       /* Check effective address */
>       if ((address & mask) != tlb->EPN) {
> -        return -1;
> +        return false;
>       }
>       *raddrp = (tlb->RPN & mask) | (address & ~mask);
> -    return 0;
> +    return true;
>   }
>   
>   /* Generic TLB search function for PowerPC embedded implementations */
> @@ -525,7 +525,7 @@ int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid)
>   
>       for (i = 0; i < env->nb_tlb; i++) {
>           tlb = &env->tlb.tlbe[i];
> -        if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i) == 0) {
> +        if (ppcemb_tlb_check(env, tlb, &raddr, address, pid, i)) {
>               return i;
>           }
>       }
> @@ -545,8 +545,8 @@ static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
>       pr = FIELD_EX64(env->msr, MSR, PR);
>       for (i = 0; i < env->nb_tlb; i++) {
>           tlb = &env->tlb.tlbe[i];
> -        if (ppcemb_tlb_check(env, tlb, &raddr, address,
> -                             env->spr[SPR_40x_PID], i) < 0) {
> +        if (!ppcemb_tlb_check(env, tlb, &raddr, address,
> +                              env->spr[SPR_40x_PID], i)) {
>               continue;
>           }
>           zsel = (tlb->attr >> 4) & 0xF;
> @@ -608,7 +608,7 @@ static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb,
>       int prot2;
>   
>       if (ppcemb_tlb_check(env, tlb, raddr, address,
> -                         env->spr[SPR_BOOKE_PID], i) >= 0) {
> +                         env->spr[SPR_BOOKE_PID], i)) {
>           if (!env->nb_pids) {
>               /* Extend the physical address to 36 bits */
>               *raddr |= (uint64_t)(tlb->RPN & 0xF) << 32;
> @@ -618,13 +618,13 @@ static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb,
>   
>       if (env->spr[SPR_BOOKE_PID1] &&
>           ppcemb_tlb_check(env, tlb, raddr, address,
> -                         env->spr[SPR_BOOKE_PID1], i) >= 0) {
> +                         env->spr[SPR_BOOKE_PID1], i)) {
>           goto found_tlb;
>       }
>   
>       if (env->spr[SPR_BOOKE_PID2] &&
>           ppcemb_tlb_check(env, tlb, raddr, address,
> -                         env->spr[SPR_BOOKE_PID2], i) >= 0) {
> +                         env->spr[SPR_BOOKE_PID2], i)) {
>           goto found_tlb;
>       }
>   

Re: [PATCH 7/7] target/ppc: Eliminate goto in mmubooke_check_tlb()

[Daniel Henrique Barboza]

On 5/30/23 10:28, BALATON Zoltan wrote:
> Move out checking PID registers into a separate function which makes
> mmubooke_check_tlb() simpler and avoids using goto.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   target/ppc/mmu_common.c | 40 +++++++++++++++++++++-------------------
>   1 file changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/target/ppc/mmu_common.c b/target/ppc/mmu_common.c
> index bd7d7d5257..ae1db6e348 100644
> --- a/target/ppc/mmu_common.c
> +++ b/target/ppc/mmu_common.c
> @@ -601,37 +601,39 @@ static int mmu40x_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
>       return ret;
>   }
>   
> -static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb,
> -                              hwaddr *raddr, int *prot, target_ulong address,
> -                              MMUAccessType access_type, int i)
> +static bool mmubooke_check_pid(CPUPPCState *env, ppcemb_tlb_t *tlb,
> +                               hwaddr *raddr, target_ulong addr, int i)
>   {
> -    int prot2;
> -
> -    if (ppcemb_tlb_check(env, tlb, raddr, address,
> -                         env->spr[SPR_BOOKE_PID], i)) {
> +    if (ppcemb_tlb_check(env, tlb, raddr, addr, env->spr[SPR_BOOKE_PID], i)) {
>           if (!env->nb_pids) {
>               /* Extend the physical address to 36 bits */
>               *raddr |= (uint64_t)(tlb->RPN & 0xF) << 32;
>           }
> -        goto found_tlb;
> +        return true;
> +    } else if (!env->nb_pids) {
> +        return false;
>       }
> -
>       if (env->spr[SPR_BOOKE_PID1] &&
> -        ppcemb_tlb_check(env, tlb, raddr, address,
> -                         env->spr[SPR_BOOKE_PID1], i)) {
> -        goto found_tlb;
> +        ppcemb_tlb_check(env, tlb, raddr, addr, env->spr[SPR_BOOKE_PID1], i)) {
> +        return true;
>       }
> -
>       if (env->spr[SPR_BOOKE_PID2] &&
> -        ppcemb_tlb_check(env, tlb, raddr, address,
> -                         env->spr[SPR_BOOKE_PID2], i)) {
> -        goto found_tlb;
> +        ppcemb_tlb_check(env, tlb, raddr, addr, env->spr[SPR_BOOKE_PID2], i)) {
> +        return true;
>       }
> +    return false;
> +}
>   
> -     qemu_log_mask(CPU_LOG_MMU, "%s: TLB entry not found\n", __func__);
> -    return -1;
> +static int mmubooke_check_tlb(CPUPPCState *env, ppcemb_tlb_t *tlb,
> +                              hwaddr *raddr, int *prot, target_ulong address,
> +                              MMUAccessType access_type, int i)
> +{
> +    int prot2;
>   
> -found_tlb:
> +    if (!mmubooke_check_pid(env, tlb, raddr, address, i)) {
> +        qemu_log_mask(CPU_LOG_MMU, "%s: TLB entry not found\n", __func__);
> +        return -1;
> +    }
>   
>       if (FIELD_EX64(env->msr, MSR, PR)) {
>           prot2 = tlb->prot & 0xF;

Re: [PATCH 0/7] Embedded PPC misc clean up and optimisation

[Daniel Henrique Barboza]

Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks,


Daniel

On 5/30/23 10:28, BALATON Zoltan wrote:
> Hello,
> 
> This series improves embedded PPC TLB emulation a bit and contains
> some misc clean up I've found along the way. Before this patch
> ppcemb_tlb_check() shows up in a memory access intensive profile
> (running RageMem speed test in AmigaOS on sam460ex) at 11.91%
> children, 10.77% self. After this series it does not show up at all.
> This is not the biggest bottleneck, that is calling tlb_flush() from
> helper_440_tlbwe() excessively but this was simpler to clean up and
> still makes a small improvement.
> 
> RageMem results on master:
> ---> RAM <---
> READ32:  593 MB/Sec
> READ64:  616 MB/Sec
> WRITE32: 589 MB/Sec
> WRITE64: 621 MB/Sec
> WRITE: 518 MB/Sec (Tricky)
> 
> ---> VIDEO BUS <---
> READ:  588 MB/Sec
> WRITE: 571 MB/Sec
> 
> with this series:
> ---> RAM <---
> READ32:  674 MB/Sec
> READ64:  707 MB/Sec
> WRITE32: 665 MB/Sec
> WRITE64: 714 MB/Sec
> WRITE: 580 MB/Sec (Tricky)
> 
> ---> VIDEO BUS <---
> READ:  691 MB/Sec
> WRITE: 662 MB/Sec
> 
> The results have some jitter but both the higher values and that the
> function is gone from the profile can prove the series has an effect.
> If nothing else then simplifying the code a bit. For comparison this
> is faster than a real sam460ex but much slower than running the same
> with -M pegasos2 so embedded PPC TLB emulation still might need some
> improvement. I know these are different and PPC440 has software
> assisted TLB but the problem with it seems to be too much tlb_flushes
> not that it needs more exceptions.
> 
> (If somebody is interested to reproduce and experiment with it the
> benchmarks and some results are available from here:
> https://www.amigans.net/modules/newbb/viewtopic.php?topic_id=9226
> some of the tests also have MorphOS versions that's easier to get than
> AmigaOS or sources that could be compiled under Linux.)
> 
> Regards,
> BALATON Zoltan
> 
> BALATON Zoltan (7):
>    target/ppc: Remove single use function
>    target/ppc: Remove "ext" parameter of ppcemb_tlb_check()
>    target/ppc: Move ppcemb_tlb_search() to mmu_common.c
>    target/ppc: Remove some unneded line breaks
>    target/ppc: Simplify ppcemb_tlb_search()
>    target/ppc: Change ppcemb_tlb_check() to return bool
>    target/ppc: Eliminate goto in mmubooke_check_tlb()
> 
>   target/ppc/cpu.h        | 13 ++----
>   target/ppc/mmu_common.c | 91 +++++++++++++++++++++++------------------
>   target/ppc/mmu_helper.c | 32 +--------------
>   3 files changed, 57 insertions(+), 79 deletions(-)
>