[Qemu-devel] [PATCH 28/45] mmu-hash*: Remove permission checking from find_pte{32, 64}()

[David Gibson]

find_pte{32,64}() are poorly named, since they both find a PTE and do
permissions checking of it.  This patch makes them only locate a matching
PTE, moving the permission checking and other logic to the caller.  We
rename the resulting search functions ppc_hash{32,64}_htab_lookup().

Signed-off-by: David Gibson <address@hidden>
---
 target-ppc/mmu-hash32.c |   45 +++++++++++++++++---------------
 target-ppc/mmu-hash64.c |   65 +++++++++++++++++++++++++----------------------
 2 files changed, 58 insertions(+), 52 deletions(-)

diff --git a/target-ppc/mmu-hash32.c b/target-ppc/mmu-hash32.c
index b7a26a2..8b403fe 100644
--- a/target-ppc/mmu-hash32.c
+++ b/target-ppc/mmu-hash32.c
@@ -374,19 +374,15 @@ static hwaddr ppc_hash32_pteg_search(CPUPPCState *env, 
hwaddr pteg_off,
     return -1;
 }
 
-static int find_pte32(CPUPPCState *env, struct mmu_ctx_hash32 *ctx,
-                      target_ulong sr, target_ulong eaddr, int rwx)
+static hwaddr ppc_hash32_htab_lookup(CPUPPCState *env,
+                                     target_ulong sr, target_ulong eaddr,
+                                     ppc_hash_pte32_t *pte)
 {
     hwaddr pteg_off, pte_offset;
-    ppc_hash_pte32_t pte;
     hwaddr hash;
     uint32_t vsid, pgidx, ptem;
-    int ret;
 
-    ret = -1; /* No entry found */
     vsid = sr & SR32_VSID;
-    ctx->key = (((sr & SR32_KP) && (msr_pr != 0)) ||
-                ((sr & SR32_KS) && (msr_pr == 0))) ? 1 : 0;
     pgidx = (eaddr & ~SEGMENT_MASK_256M) >> TARGET_PAGE_BITS;
     hash = vsid ^ pgidx;
     ptem = (vsid << 7) | (pgidx >> 10);
@@ -402,7 +398,7 @@ static int find_pte32(CPUPPCState *env, struct 
mmu_ctx_hash32 *ctx,
             " hash=" TARGET_FMT_plx "\n",
             env->htab_base, env->htab_mask, vsid, ptem, hash);
     pteg_off = get_pteg_offset32(env, hash);
-    pte_offset = ppc_hash32_pteg_search(env, pteg_off, 0, ptem, &pte);
+    pte_offset = ppc_hash32_pteg_search(env, pteg_off, 0, ptem, pte);
     if (pte_offset == -1) {
         /* Secondary PTEG lookup */
         LOG_MMU("1 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx
@@ -410,20 +406,10 @@ static int find_pte32(CPUPPCState *env, struct 
mmu_ctx_hash32 *ctx,
                 " hash=" TARGET_FMT_plx "\n", env->htab_base,
                 env->htab_mask, vsid, ptem, ~hash);
         pteg_off = get_pteg_offset32(env, ~hash);
-        pte_offset = ppc_hash32_pteg_search(env, pteg_off, 1, ptem, &pte);
+        pte_offset = ppc_hash32_pteg_search(env, pteg_off, 1, ptem, pte);
     }
 
-    if (pte_offset != -1) {
-        ret = pte_check_hash32(ctx, pte.pte0, pte.pte1, rwx);
-        LOG_MMU("found PTE at addr %08" HWADDR_PRIx " prot=%01x ret=%d\n",
-                ctx->raddr, ctx->prot, ret);
-        /* Update page flags */
-        if (ppc_hash32_pte_update_flags(ctx, &pte.pte1, ret, rwx) == 1) {
-            ppc_hash32_store_hpte1(env, pte_offset, pte.pte1);
-        }
-    }
-
-    return ret;
+    return pte_offset;
 }
 
 static int ppc_hash32_translate(CPUPPCState *env, struct mmu_ctx_hash32 *ctx,
@@ -431,6 +417,8 @@ static int ppc_hash32_translate(CPUPPCState *env, struct 
mmu_ctx_hash32 *ctx,
 {
     int ret;
     target_ulong sr;
+    hwaddr pte_offset;
+    ppc_hash_pte32_t pte;
 
     /* 1. Handle real mode accesses */
     if (((rwx == 2) && (msr_ir == 0)) || ((rwx != 2) && (msr_dr == 0))) {
@@ -462,7 +450,22 @@ static int ppc_hash32_translate(CPUPPCState *env, struct 
mmu_ctx_hash32 *ctx,
     if ((rwx == 2) && ctx->nx) {
         return -3;
     }
-    ret = find_pte32(env, ctx, sr, eaddr, rwx);
+
+    /* 6. Locate the PTE in the hash table */
+    pte_offset = ppc_hash32_htab_lookup(env, sr, eaddr, &pte);
+    if (pte_offset == -1) {
+        return -1;
+    }
+    LOG_MMU("found PTE at offset %08" HWADDR_PRIx "\n", pte_offset);
+
+    /* 7. Check access permissions */
+    ctx->key = (((sr & SR32_KP) && (msr_pr != 0)) ||
+                ((sr & SR32_KS) && (msr_pr == 0))) ? 1 : 0;
+    ret = pte_check_hash32(ctx, pte.pte0, pte.pte1, rwx);
+    /* Update page flags */
+    if (ppc_hash32_pte_update_flags(ctx, &pte.pte1, ret, rwx) == 1) {
+        ppc_hash32_store_hpte1(env, pte_offset, pte.pte1);
+    }
 
     return ret;
 }
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 36e2c84..a98e008 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -374,17 +374,14 @@ static hwaddr ppc_hash64_pteg_search(CPUPPCState *env, 
hwaddr pteg_off,
     return -1;
 }
 
-static int find_pte64(CPUPPCState *env, struct mmu_ctx_hash64 *ctx,
-                      ppc_slb_t *slb, target_ulong eaddr, int rwx)
+static hwaddr ppc_hash64_htab_lookup(CPUPPCState *env,
+                                     ppc_slb_t *slb, target_ulong eaddr,
+                                     ppc_hash_pte64_t *pte)
 {
     hwaddr pteg_off, pte_offset;
-    ppc_hash_pte64_t pte;
     uint64_t vsid, pageaddr, ptem;
     hwaddr hash;
     int segment_bits, target_page_bits;
-    int ret;
-
-    ret = -1; /* No entry found */
 
     if (slb->vsid & SLB_VSID_B) {
         vsid = (slb->vsid & SLB_VSID_VSID) >> SLB_VSID_SHIFT_1T;
@@ -396,8 +393,6 @@ static int find_pte64(CPUPPCState *env, struct 
mmu_ctx_hash64 *ctx,
 
     target_page_bits = (slb->vsid & SLB_VSID_L)
         ? TARGET_PAGE_BITS_16M : TARGET_PAGE_BITS;
-    ctx->key = !!(msr_pr ? (slb->vsid & SLB_VSID_KP)
-                  : (slb->vsid & SLB_VSID_KS));
 
     pageaddr = eaddr & ((1ULL << segment_bits)
                             - (1ULL << target_page_bits));
@@ -410,21 +405,19 @@ static int find_pte64(CPUPPCState *env, struct 
mmu_ctx_hash64 *ctx,
     ptem = (slb->vsid & SLB_VSID_PTEM) |
         ((pageaddr >> 16) & ((1ULL << segment_bits) - 0x80));
 
-    ret = -1;
-
     /* Page address translation */
     LOG_MMU("htab_base " TARGET_FMT_plx " htab_mask " TARGET_FMT_plx
             " hash " TARGET_FMT_plx "\n",
             env->htab_base, env->htab_mask, hash);
 
-
     /* Primary PTEG lookup */
     LOG_MMU("0 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx
             " vsid=" TARGET_FMT_lx " ptem=" TARGET_FMT_lx
             " hash=" TARGET_FMT_plx "\n",
             env->htab_base, env->htab_mask, vsid, ptem,  hash);
     pteg_off = (hash * HASH_PTEG_SIZE_64) & env->htab_mask;
-    pte_offset = ppc_hash64_pteg_search(env, pteg_off, 0, ptem, &pte);
+    pte_offset = ppc_hash64_pteg_search(env, pteg_off, 0, ptem, pte);
+
     if (pte_offset == -1) {
         /* Secondary PTEG lookup */
         LOG_MMU("1 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx
@@ -433,26 +426,10 @@ static int find_pte64(CPUPPCState *env, struct 
mmu_ctx_hash64 *ctx,
                 env->htab_mask, vsid, ptem, ~hash);
 
         pteg_off = (~hash * HASH_PTEG_SIZE_64) & env->htab_mask;
-        pte_offset = ppc_hash64_pteg_search(env, pteg_off, 1, ptem, &pte);
+        pte_offset = ppc_hash64_pteg_search(env, pteg_off, 1, ptem, pte);
     }
 
-    if (pte_offset != -1) {
-        ret = pte64_check(ctx, pte.pte0, pte.pte1, rwx);
-        LOG_MMU("found PTE at addr %08" HWADDR_PRIx " prot=%01x ret=%d\n",
-                ctx->raddr, ctx->prot, ret);
-        /* Update page flags */
-        if (ppc_hash64_pte_update_flags(ctx, &pte.pte1, ret, rwx) == 1) {
-            ppc_hash64_store_hpte1(env, pte_offset, pte.pte1);
-        }
-    }
-
-    /* We have a TLB that saves 4K pages, so let's
-     * split a huge page to 4k chunks */
-    if (target_page_bits != TARGET_PAGE_BITS) {
-        ctx->raddr |= (eaddr & ((1 << target_page_bits) - 1))
-                      & TARGET_PAGE_MASK;
-    }
-    return ret;
+    return pte_offset;
 }
 
 static int ppc_hash64_translate(CPUPPCState *env, struct mmu_ctx_hash64 *ctx,
@@ -460,6 +437,9 @@ static int ppc_hash64_translate(CPUPPCState *env, struct 
mmu_ctx_hash64 *ctx,
 {
     int ret;
     ppc_slb_t *slb;
+    hwaddr pte_offset;
+    ppc_hash_pte64_t pte;
+    int target_page_bits;
 
     /* 1. Handle real mode accesses */
     if (((rwx == 2) && (msr_ir == 0)) || ((rwx != 2) && (msr_dr == 0))) {
@@ -482,8 +462,31 @@ static int ppc_hash64_translate(CPUPPCState *env, struct 
mmu_ctx_hash64 *ctx,
         return -3;
     }
 
-    ret = find_pte64(env, ctx, slb, eaddr, rwx);
+    /* 4. Locate the PTE in the hash table */
+    pte_offset = ppc_hash64_htab_lookup(env, slb, eaddr, &pte);
+    if (pte_offset == -1) {
+        return -1;
+    }
+    LOG_MMU("found PTE at offset %08" HWADDR_PRIx "\n", pte_offset);
+
+    /* 5. Check access permissions */
+    ctx->key = !!(msr_pr ? (slb->vsid & SLB_VSID_KP)
+                  : (slb->vsid & SLB_VSID_KS));
+
+    ret = pte64_check(ctx, pte.pte0, pte.pte1, rwx);
+    /* Update page flags */
+    if (ppc_hash64_pte_update_flags(ctx, &pte.pte1, ret, rwx) == 1) {
+        ppc_hash64_store_hpte1(env, pte_offset, pte.pte1);
+    }
 
+    /* We have a TLB that saves 4K pages, so let's
+     * split a huge page to 4k chunks */
+    target_page_bits = (slb->vsid & SLB_VSID_L)
+        ? TARGET_PAGE_BITS_16M : TARGET_PAGE_BITS;
+    if (target_page_bits != TARGET_PAGE_BITS) {
+        ctx->raddr |= (eaddr & ((1 << target_page_bits) - 1))
+                      & TARGET_PAGE_MASK;
+    }
     return ret;
 }
 
-- 
1.7.10.4