mmu-hash*: Don't keep looking for PTEs after we find a match BEHAVIOUR CHANGE The ppc hash mmu hashes each virtual address to a primary and secondary possible hash bucket (aka PTE group or PTEG) each with 8 PTEs. Then we need a linear search through the PTEs to find the correct one for the virtual address we're translating. It is a programming error for the guest to insert multiple PTEs mapping the same virtual address into a PTEG - in this case the ppc architecture says the MMU can either act as if just one was present, or give a machine check. Currently our code takes the first matching PTE in a PTEG if it finds a successful translation. But if a matching PTE is found, but permission bits don't allow the access, we keep looking through the PTEG, checking that any other matching PTEs contain an identical translation. That behaviour is perhaps not exactly wrong, but it's certainly not useful. This patch changes it to always just find the first matching PTE in a PTEG. In addition, if we get a permissions problem on the primary PTEG, we then search the secondary PTEG. This is incorrect - a permission denying PTE in the primary PTEG should not be overwritten by an access granting PTE in the secondary (although again, it would be a programming error for the guest to set up such a situation anyway). So additionally we update the code to only search the secondary PTEG if no matching PTE is found in the primary at all. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> Signed-off-by: Alexander Graf <agraf@suse.de>
diff --git a/target-ppc/mmu-hash32.c b/target-ppc/mmu-hash32.c index d812adb..3be1002 100644 --- a/target-ppc/mmu-hash32.c +++ b/target-ppc/mmu-hash32.c
@@ -50,8 +50,6 @@ int nx; /* Non-execute area */ }; -#define PTE_CHECK_MASK (TARGET_PAGE_MASK | 0x7B) - static int ppc_hash32_pp_check(int key, int pp, int nx) { int access; @@ -299,40 +297,32 @@ } } -static int pte_check_hash32(struct mmu_ctx_hash32 *ctx, target_ulong pte0, - target_ulong pte1, int h, int rwx) +static bool pte32_match(target_ulong pte0, target_ulong pte1, + bool secondary, target_ulong ptem) { - target_ulong mmask; + return (pte0 & HPTE32_V_VALID) + && (secondary == !!(pte0 & HPTE32_V_SECONDARY)) + && HPTE32_V_COMPARE(pte0, ptem); +} + +static int pte_check_hash32(struct mmu_ctx_hash32 *ctx, target_ulong pte0, + target_ulong pte1, int rwx) +{ int access, ret, pp; - ret = -1; - /* Check validity and table match */ - if ((pte0 & HPTE32_V_VALID) && (h == !!(pte0 & HPTE32_V_SECONDARY))) { - /* Check vsid & api */ - mmask = PTE_CHECK_MASK; - pp = pte1 & HPTE32_R_PP; - if (HPTE32_V_COMPARE(pte0, ctx->ptem)) { - if (ctx->raddr != (hwaddr)-1ULL) { - /* all matches should have equal RPN, WIMG & PP */ - if ((ctx->raddr & mmask) != (pte1 & mmask)) { - qemu_log("Bad RPN/WIMG/PP\n"); - return -3; - } - } - /* Compute access rights */ - access = ppc_hash32_pp_check(ctx->key, pp, ctx->nx); - /* Keep the matching PTE informations */ - ctx->raddr = pte1; - ctx->prot = access; - ret = ppc_hash32_check_prot(ctx->prot, rwx); - if (ret == 0) { - /* Access granted */ - LOG_MMU("PTE access granted !\n"); - } else { - /* Access right violation */ - LOG_MMU("PTE access rejected\n"); - } - } + pp = pte1 & HPTE32_R_PP; + /* Compute access rights */ + access = ppc_hash32_pp_check(ctx->key, pp, ctx->nx); + /* Keep the matching PTE informations */ + ctx->raddr = pte1; + ctx->prot = access; + ret = ppc_hash32_check_prot(ctx->prot, rwx); + if (ret == 0) { + /* Access granted */ + LOG_MMU("PTE access granted !\n"); + } else { + /* Access right violation */ + LOG_MMU("PTE access rejected\n"); } return ret; @@ -375,44 +365,26 @@ hwaddr pteg_off; target_ulong pte0, pte1; int i, good = -1; - int ret, r; + int ret; ret = -1; /* No entry found */ pteg_off = get_pteg_offset32(env, ctx->hash[h]); for (i = 0; i < HPTES_PER_GROUP; i++) { pte0 = ppc_hash32_load_hpte0(env, pteg_off + i*HASH_PTE_SIZE_32); pte1 = ppc_hash32_load_hpte1(env, pteg_off + i*HASH_PTE_SIZE_32); - r = pte_check_hash32(ctx, pte0, pte1, h, rwx); + LOG_MMU("Load pte from %08" HWADDR_PRIx " => " TARGET_FMT_lx " " TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n", pteg_off + (i * 8), pte0, pte1, (int)(pte0 >> 31), h, (int)((pte0 >> 6) & 1), ctx->ptem); - switch (r) { - case -3: - /* PTE inconsistency */ - return -1; - case -2: - /* Access violation */ - ret = -2; + + if (pte32_match(pte0, pte1, h, ctx->ptem)) { good = i; break; - case -1: - default: - /* No PTE match */ - break; - case 0: - /* access granted */ - /* XXX: we should go on looping to check all PTEs consistency - * but if we can speed-up the whole thing as the - * result would be undefined if PTEs are not consistent. - */ - ret = 0; - good = i; - goto done; } } if (good != -1) { - done: + ret = pte_check_hash32(ctx, pte0, pte1, rwx); LOG_MMU("found PTE at addr %08" HWADDR_PRIx " prot=%01x ret=%d\n", ctx->raddr, ctx->prot, ret); /* Update page flags */ @@ -498,8 +470,6 @@ ctx->hash[0] = hash; ctx->hash[1] = ~hash; - /* Initialize real address with an invalid value */ - ctx->raddr = (hwaddr)-1ULL; LOG_MMU("0 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx " vsid=" TARGET_FMT_lx " ptem=" TARGET_FMT_lx " hash=" TARGET_FMT_plx "\n", @@ -507,7 +477,7 @@ ctx->hash[0]); /* Primary table lookup */ ret = find_pte32(env, ctx, eaddr, 0, rwx, target_page_bits); - if (ret < 0) { + if (ret == -1) { /* Secondary table lookup */ LOG_MMU("1 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx " vsid=" TARGET_FMT_lx " api=" TARGET_FMT_lx
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c index 4fb7ecd..8664116 100644 --- a/target-ppc/mmu-hash64.c +++ b/target-ppc/mmu-hash64.c
@@ -231,8 +231,6 @@ * 64-bit hash table MMU handling */ -#define PTE64_CHECK_MASK (TARGET_PAGE_MASK | 0x7F) - static int ppc_hash64_pp_check(int key, int pp, bool nx) { int access; @@ -301,44 +299,35 @@ return ret; } -static int pte64_check(struct mmu_ctx_hash64 *ctx, target_ulong pte0, - target_ulong pte1, int h, int rwx) +static bool pte64_match(target_ulong pte0, target_ulong pte1, + bool secondary, target_ulong ptem) { - target_ulong mmask; + return (pte0 & HPTE64_V_VALID) + && (secondary == !!(pte0 & HPTE64_V_SECONDARY)) + && HPTE64_V_COMPARE(pte0, ptem); +} + +static int pte64_check(struct mmu_ctx_hash64 *ctx, target_ulong pte0, + target_ulong pte1, int rwx) +{ int access, ret, pp; + bool nx; - ret = -1; - /* Check validity and table match */ - if ((pte0 & HPTE64_V_VALID) && (h == !!(pte0 & HPTE64_V_SECONDARY))) { - bool nx; - - /* Check vsid & api */ - mmask = PTE64_CHECK_MASK; - pp = (pte1 & HPTE64_R_PP) | ((pte1 & HPTE64_R_PP0) >> 61); - /* No execute if either noexec or guarded bits set */ - nx = (pte1 & HPTE64_R_N) || (pte1 & HPTE64_R_G); - if (HPTE64_V_COMPARE(pte0, ctx->ptem)) { - if (ctx->raddr != (hwaddr)-1ULL) { - /* all matches should have equal RPN, WIMG & PP */ - if ((ctx->raddr & mmask) != (pte1 & mmask)) { - qemu_log("Bad RPN/WIMG/PP\n"); - return -3; - } - } - /* Compute access rights */ - access = ppc_hash64_pp_check(ctx->key, pp, nx); - /* Keep the matching PTE informations */ - ctx->raddr = pte1; - ctx->prot = access; - ret = ppc_hash64_check_prot(ctx->prot, rwx); - if (ret == 0) { - /* Access granted */ - LOG_MMU("PTE access granted !\n"); - } else { - /* Access right violation */ - LOG_MMU("PTE access rejected\n"); - } - } + pp = (pte1 & HPTE64_R_PP) | ((pte1 & HPTE64_R_PP0) >> 61); + /* No execute if either noexec or guarded bits set */ + nx = (pte1 & HPTE64_R_N) || (pte1 & HPTE64_R_G); + /* Compute access rights */ + access = ppc_hash64_pp_check(ctx->key, pp, nx); + /* Keep the matching PTE informations */ + ctx->raddr = pte1; + ctx->prot = access; + ret = ppc_hash64_check_prot(ctx->prot, rwx); + if (ret == 0) { + /* Access granted */ + LOG_MMU("PTE access granted !\n"); + } else { + /* Access right violation */ + LOG_MMU("PTE access rejected\n"); } return ret; @@ -377,7 +366,7 @@ hwaddr pteg_off; target_ulong pte0, pte1; int i, good = -1; - int ret, r; + int ret; ret = -1; /* No entry found */ pteg_off = (ctx->hash[h] * HASH_PTEG_SIZE_64) & env->htab_mask; @@ -385,37 +374,18 @@ pte0 = ppc_hash64_load_hpte0(env, pteg_off + i*HASH_PTE_SIZE_64); pte1 = ppc_hash64_load_hpte1(env, pteg_off + i*HASH_PTE_SIZE_64); - r = pte64_check(ctx, pte0, pte1, h, rwx); LOG_MMU("Load pte from %016" HWADDR_PRIx " => " TARGET_FMT_lx " " TARGET_FMT_lx " %d %d %d " TARGET_FMT_lx "\n", - pteg_off + (i * 16), pte0, pte1, (int)(pte0 & 1), h, - (int)((pte0 >> 1) & 1), ctx->ptem); - switch (r) { - case -3: - /* PTE inconsistency */ - return -1; - case -2: - /* Access violation */ - ret = -2; + pteg_off + (i * 16), pte0, pte1, !!(pte0 & HPTE64_V_VALID), + h, !!(pte0 & HPTE64_V_SECONDARY), ctx->ptem); + + if (pte64_match(pte0, pte1, h, ctx->ptem)) { good = i; break; - case -1: - default: - /* No PTE match */ - break; - case 0: - /* access granted */ - /* XXX: we should go on looping to check all PTEs consistency - * but if we can speed-up the whole thing as the - * result would be undefined if PTEs are not consistent. - */ - ret = 0; - good = i; - goto done; } } if (good != -1) { - done: + ret = pte64_check(ctx, pte0, pte1, rwx); LOG_MMU("found PTE at addr %08" HWADDR_PRIx " prot=%01x ret=%d\n", ctx->raddr, ctx->prot, ret); /* Update page flags */ @@ -503,8 +473,6 @@ ctx->hash[0] = hash; ctx->hash[1] = ~hash; - /* Initialize real address with an invalid value */ - ctx->raddr = (hwaddr)-1ULL; LOG_MMU("0 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx " vsid=" TARGET_FMT_lx " ptem=" TARGET_FMT_lx " hash=" TARGET_FMT_plx "\n", @@ -512,7 +480,7 @@ ctx->hash[0]); /* Primary table lookup */ ret = find_pte64(env, ctx, eaddr, 0, rwx, target_page_bits); - if (ret < 0) { + if (ret == -1) { /* Secondary table lookup */ LOG_MMU("1 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx " vsid=" TARGET_FMT_lx " api=" TARGET_FMT_lx