* [Qemu-devel] [PATCH 1/3] target-ppc: Correct page size decoding in ppc_hash64_pteg_search()
2016-07-05 2:33 [Qemu-devel] [PATCH 0/3] Fixes and cleanups to HPTE lookup and page size decoding David Gibson
@ 2016-07-05 2:33 ` David Gibson
2016-07-05 2:33 ` [Qemu-devel] [PATCH 2/3] target-ppc: Simplify HPTE matching David Gibson
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: David Gibson @ 2016-07-05 2:33 UTC (permalink / raw)
To: benh; +Cc: agraf, qemu-ppc, qemu-devel, David Gibson
The architecture specifies that when searching a PTEG for PTEs, entries
with a page size encoding that's not valid for the current segment should
be ignored, continuing the search.
The current implementation does this with ppc_hash64_pte_size_decode()
which is a very incomplete implementation of this check. We already have
code to do a full and correct page size decode in hpte_page_shift().
This patch moves hpte_page_shift() so it can be used in
ppc_hash64_pteg_search() and adjusts the latter's parameters to include
a full SLBE instead of just a segment page shift.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
target-ppc/mmu-hash64.c | 99 ++++++++++++++++++++-----------------------------
1 file changed, 41 insertions(+), 58 deletions(-)
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 3b1357a..cbc5c0a 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -450,30 +450,45 @@ void ppc_hash64_stop_access(PowerPCCPU *cpu, uint64_t token)
}
}
-/* Returns the effective page shift or 0. MPSS isn't supported yet so
- * this will always be the slb_pshift or 0
- */
-static uint32_t ppc_hash64_pte_size_decode(uint64_t pte1, uint32_t slb_pshift)
+static unsigned hpte_page_shift(const struct ppc_one_seg_page_size *sps,
+ uint64_t pte0, uint64_t pte1)
{
- switch (slb_pshift) {
- case 12:
+ int i;
+
+ if (!(pte0 & HPTE64_V_LARGE)) {
+ if (sps->page_shift != 12) {
+ /* 4kiB page in a non 4kiB segment */
+ return 0;
+ }
+ /* Normal 4kiB page */
return 12;
- case 16:
- if ((pte1 & 0xf000) == 0x1000) {
- return 16;
+ }
+
+ for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
+ const struct ppc_one_page_size *ps = &sps->enc[i];
+ uint64_t mask;
+
+ if (!ps->page_shift) {
+ break;
}
- return 0;
- case 24:
- if ((pte1 & 0xff000) == 0) {
- return 24;
+
+ if (ps->page_shift == 12) {
+ /* L bit is set so this can't be a 4kiB page */
+ continue;
+ }
+
+ mask = ((1ULL << ps->page_shift) - 1) & HPTE64_R_RPN;
+
+ if ((pte1 & mask) == (ps->pte_enc << HPTE64_R_RPN_SHIFT)) {
+ return ps->page_shift;
}
- return 0;
}
- return 0;
+
+ return 0; /* Bad page size encoding */
}
static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
- uint32_t slb_pshift, bool secondary,
+ ppc_slb_t *slb, bool secondary,
target_ulong ptem, ppc_hash_pte64_t *pte)
{
CPUPPCState *env = &cpu->env;
@@ -494,7 +509,14 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
if ((pte0 & HPTE64_V_VALID)
&& (secondary == !!(pte0 & HPTE64_V_SECONDARY))
&& HPTE64_V_COMPARE(pte0, ptem)) {
- uint32_t pshift = ppc_hash64_pte_size_decode(pte1, slb_pshift);
+ unsigned pshift = hpte_page_shift(slb->sps, pte0, pte1);
+ /*
+ * If there is no match, ignore the PTE, it could simply
+ * be for a different segment size encoding and the
+ * architecture specifies we should not match. Linux will
+ * potentially leave behind PTEs for the wrong base page
+ * size when demoting segments.
+ */
if (pshift == 0) {
continue;
}
@@ -554,8 +576,7 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
" vsid=" TARGET_FMT_lx " ptem=" TARGET_FMT_lx
" hash=" TARGET_FMT_plx "\n",
env->htab_base, env->htab_mask, vsid, ptem, hash);
- pte_offset = ppc_hash64_pteg_search(cpu, hash, slb->sps->page_shift,
- 0, ptem, pte);
+ pte_offset = ppc_hash64_pteg_search(cpu, hash, slb, 0, ptem, pte);
if (pte_offset == -1) {
/* Secondary PTEG lookup */
@@ -565,50 +586,12 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
" hash=" TARGET_FMT_plx "\n", env->htab_base,
env->htab_mask, vsid, ptem, ~hash);
- pte_offset = ppc_hash64_pteg_search(cpu, ~hash, slb->sps->page_shift, 1,
- ptem, pte);
+ pte_offset = ppc_hash64_pteg_search(cpu, ~hash, slb, 1, ptem, pte);
}
return pte_offset;
}
-static unsigned hpte_page_shift(const struct ppc_one_seg_page_size *sps,
- uint64_t pte0, uint64_t pte1)
-{
- int i;
-
- if (!(pte0 & HPTE64_V_LARGE)) {
- if (sps->page_shift != 12) {
- /* 4kiB page in a non 4kiB segment */
- return 0;
- }
- /* Normal 4kiB page */
- return 12;
- }
-
- for (i = 0; i < PPC_PAGE_SIZES_MAX_SZ; i++) {
- const struct ppc_one_page_size *ps = &sps->enc[i];
- uint64_t mask;
-
- if (!ps->page_shift) {
- break;
- }
-
- if (ps->page_shift == 12) {
- /* L bit is set so this can't be a 4kiB page */
- continue;
- }
-
- mask = ((1ULL << ps->page_shift) - 1) & HPTE64_R_RPN;
-
- if ((pte1 & mask) == (ps->pte_enc << HPTE64_R_RPN_SHIFT)) {
- return ps->page_shift;
- }
- }
-
- return 0; /* Bad page size encoding */
-}
-
unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
uint64_t pte0, uint64_t pte1,
unsigned *seg_page_shift)
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PATCH 2/3] target-ppc: Simplify HPTE matching
2016-07-05 2:33 [Qemu-devel] [PATCH 0/3] Fixes and cleanups to HPTE lookup and page size decoding David Gibson
2016-07-05 2:33 ` [Qemu-devel] [PATCH 1/3] target-ppc: Correct page size decoding in ppc_hash64_pteg_search() David Gibson
@ 2016-07-05 2:33 ` David Gibson
2016-07-05 2:33 ` [Qemu-devel] [PATCH 3/3] target-ppc: Return page shift from PTEG search David Gibson
2016-07-05 3:13 ` [Qemu-devel] [PATCH 0/3] Fixes and cleanups to HPTE lookup and page size decoding Benjamin Herrenschmidt
3 siblings, 0 replies; 5+ messages in thread
From: David Gibson @ 2016-07-05 2:33 UTC (permalink / raw)
To: benh; +Cc: agraf, qemu-ppc, qemu-devel, David Gibson
ppc_hash64_pteg_search() explicitly checks each HPTE's VALID and
SECONDARY bits, then uses the HPTE64_V_COMPARE() macro to check the B field
and AVPN. However, a small tweak to HPTE64_V_COMPARE() means we can check
all of these bits at once with a suitable ptem value. So, consolidate all
the comparisons for simplicity.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
target-ppc/mmu-hash64.c | 15 ++++++++-------
target-ppc/mmu-hash64.h | 2 +-
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index cbc5c0a..e4a561e 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -488,8 +488,8 @@ static unsigned hpte_page_shift(const struct ppc_one_seg_page_size *sps,
}
static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
- ppc_slb_t *slb, bool secondary,
- target_ulong ptem, ppc_hash_pte64_t *pte)
+ ppc_slb_t *slb, target_ulong ptem,
+ ppc_hash_pte64_t *pte)
{
CPUPPCState *env = &cpu->env;
int i;
@@ -506,9 +506,8 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
pte0 = ppc_hash64_load_hpte0(cpu, token, i);
pte1 = ppc_hash64_load_hpte1(cpu, token, i);
- if ((pte0 & HPTE64_V_VALID)
- && (secondary == !!(pte0 & HPTE64_V_SECONDARY))
- && HPTE64_V_COMPARE(pte0, ptem)) {
+ /* This compares V, B, H (secondary) and the AVPN */
+ if (HPTE64_V_COMPARE(pte0, ptem)) {
unsigned pshift = hpte_page_shift(slb->sps, pte0, pte1);
/*
* If there is no match, ignore the PTE, it could simply
@@ -563,6 +562,7 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
hash = vsid ^ (epn >> slb->sps->page_shift);
}
ptem = (slb->vsid & SLB_VSID_PTEM) | ((epn >> 16) & HPTE64_V_AVPN);
+ ptem |= HPTE64_V_VALID;
/* Page address translation */
qemu_log_mask(CPU_LOG_MMU,
@@ -576,17 +576,18 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
" vsid=" TARGET_FMT_lx " ptem=" TARGET_FMT_lx
" hash=" TARGET_FMT_plx "\n",
env->htab_base, env->htab_mask, vsid, ptem, hash);
- pte_offset = ppc_hash64_pteg_search(cpu, hash, slb, 0, ptem, pte);
+ pte_offset = ppc_hash64_pteg_search(cpu, hash, slb, ptem, pte);
if (pte_offset == -1) {
/* Secondary PTEG lookup */
+ ptem |= HPTE64_V_SECONDARY;
qemu_log_mask(CPU_LOG_MMU,
"1 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx
" vsid=" TARGET_FMT_lx " api=" TARGET_FMT_lx
" hash=" TARGET_FMT_plx "\n", env->htab_base,
env->htab_mask, vsid, ptem, ~hash);
- pte_offset = ppc_hash64_pteg_search(cpu, ~hash, slb, 1, ptem, pte);
+ pte_offset = ppc_hash64_pteg_search(cpu, ~hash, slb, ptem, pte);
}
return pte_offset;
diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
index 6423b9f..693980c 100644
--- a/target-ppc/mmu-hash64.h
+++ b/target-ppc/mmu-hash64.h
@@ -63,7 +63,7 @@ unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
#define HPTE64_V_AVPN_SHIFT 7
#define HPTE64_V_AVPN 0x3fffffffffffff80ULL
#define HPTE64_V_AVPN_VAL(x) (((x) & HPTE64_V_AVPN) >> HPTE64_V_AVPN_SHIFT)
-#define HPTE64_V_COMPARE(x, y) (!(((x) ^ (y)) & 0xffffffffffffff80ULL))
+#define HPTE64_V_COMPARE(x, y) (!(((x) ^ (y)) & 0xffffffffffffff83ULL))
#define HPTE64_V_LARGE 0x0000000000000004ULL
#define HPTE64_V_SECONDARY 0x0000000000000002ULL
#define HPTE64_V_VALID 0x0000000000000001ULL
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* [Qemu-devel] [PATCH 3/3] target-ppc: Return page shift from PTEG search
2016-07-05 2:33 [Qemu-devel] [PATCH 0/3] Fixes and cleanups to HPTE lookup and page size decoding David Gibson
2016-07-05 2:33 ` [Qemu-devel] [PATCH 1/3] target-ppc: Correct page size decoding in ppc_hash64_pteg_search() David Gibson
2016-07-05 2:33 ` [Qemu-devel] [PATCH 2/3] target-ppc: Simplify HPTE matching David Gibson
@ 2016-07-05 2:33 ` David Gibson
2016-07-05 3:13 ` [Qemu-devel] [PATCH 0/3] Fixes and cleanups to HPTE lookup and page size decoding Benjamin Herrenschmidt
3 siblings, 0 replies; 5+ messages in thread
From: David Gibson @ 2016-07-05 2:33 UTC (permalink / raw)
To: benh; +Cc: agraf, qemu-ppc, qemu-devel, David Gibson
ppc_hash64_pteg_search() now decodes a PTEs page size encoding, which it
didn't previously do. This means we're now double decoding the page size
because we check it int he fault path after ppc64_hash64_htab_lookup()
returns.
To avoid this duplication have ppc_hash64_pteg_search() and
ppc_hash64_htab_lookup() return the page size from the PTE and use that in
the callers instead of decoding again.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
target-ppc/mmu-hash64.c | 33 ++++++++-------------------------
1 file changed, 8 insertions(+), 25 deletions(-)
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index e4a561e..08bc80d 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -489,7 +489,7 @@ static unsigned hpte_page_shift(const struct ppc_one_seg_page_size *sps,
static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
ppc_slb_t *slb, target_ulong ptem,
- ppc_hash_pte64_t *pte)
+ ppc_hash_pte64_t *pte, unsigned *pshift)
{
CPUPPCState *env = &cpu->env;
int i;
@@ -508,7 +508,7 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
/* This compares V, B, H (secondary) and the AVPN */
if (HPTE64_V_COMPARE(pte0, ptem)) {
- unsigned pshift = hpte_page_shift(slb->sps, pte0, pte1);
+ *pshift = hpte_page_shift(slb->sps, pte0, pte1);
/*
* If there is no match, ignore the PTE, it could simply
* be for a different segment size encoding and the
@@ -516,7 +516,7 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
* potentially leave behind PTEs for the wrong base page
* size when demoting segments.
*/
- if (pshift == 0) {
+ if (*pshift == 0) {
continue;
}
/* We don't do anything with pshift yet as qemu TLB only deals
@@ -537,7 +537,7 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
ppc_slb_t *slb, target_ulong eaddr,
- ppc_hash_pte64_t *pte)
+ ppc_hash_pte64_t *pte, unsigned *pshift)
{
CPUPPCState *env = &cpu->env;
hwaddr pte_offset;
@@ -576,7 +576,7 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
" vsid=" TARGET_FMT_lx " ptem=" TARGET_FMT_lx
" hash=" TARGET_FMT_plx "\n",
env->htab_base, env->htab_mask, vsid, ptem, hash);
- pte_offset = ppc_hash64_pteg_search(cpu, hash, slb, ptem, pte);
+ pte_offset = ppc_hash64_pteg_search(cpu, hash, slb, ptem, pte, pshift);
if (pte_offset == -1) {
/* Secondary PTEG lookup */
@@ -587,7 +587,7 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
" hash=" TARGET_FMT_plx "\n", env->htab_base,
env->htab_mask, vsid, ptem, ~hash);
- pte_offset = ppc_hash64_pteg_search(cpu, ~hash, slb, ptem, pte);
+ pte_offset = ppc_hash64_pteg_search(cpu, ~hash, slb, ptem, pte, pshift);
}
return pte_offset;
@@ -718,7 +718,7 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
}
/* 4. Locate the PTE in the hash table */
- pte_offset = ppc_hash64_htab_lookup(cpu, slb, eaddr, &pte);
+ pte_offset = ppc_hash64_htab_lookup(cpu, slb, eaddr, &pte, &apshift);
if (pte_offset == -1) {
dsisr = 0x40000000;
if (rwx == 2) {
@@ -734,18 +734,6 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
qemu_log_mask(CPU_LOG_MMU,
"found PTE at offset %08" HWADDR_PRIx "\n", pte_offset);
- /* Validate page size encoding */
- apshift = hpte_page_shift(slb->sps, pte.pte0, pte.pte1);
- if (!apshift) {
- error_report("Bad page size encoding in HPTE 0x%"PRIx64" - 0x%"PRIx64
- " @ 0x%"HWADDR_PRIx, pte.pte0, pte.pte1, pte_offset);
- /* Not entirely sure what the right action here, but machine
- * check seems reasonable */
- cs->exception_index = POWERPC_EXCP_MCHECK;
- env->error_code = 0;
- return 1;
- }
-
/* 5. Check access permissions */
pp_prot = ppc_hash64_pte_prot(cpu, slb, pte);
@@ -819,16 +807,11 @@ hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr)
return -1;
}
- pte_offset = ppc_hash64_htab_lookup(cpu, slb, addr, &pte);
+ pte_offset = ppc_hash64_htab_lookup(cpu, slb, addr, &pte, &apshift);
if (pte_offset == -1) {
return -1;
}
- apshift = hpte_page_shift(slb->sps, pte.pte0, pte.pte1);
- if (!apshift) {
- return -1;
- }
-
return deposit64(pte.pte1 & HPTE64_R_RPN, 0, apshift, addr)
& TARGET_PAGE_MASK;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH 0/3] Fixes and cleanups to HPTE lookup and page size decoding
2016-07-05 2:33 [Qemu-devel] [PATCH 0/3] Fixes and cleanups to HPTE lookup and page size decoding David Gibson
` (2 preceding siblings ...)
2016-07-05 2:33 ` [Qemu-devel] [PATCH 3/3] target-ppc: Return page shift from PTEG search David Gibson
@ 2016-07-05 3:13 ` Benjamin Herrenschmidt
3 siblings, 0 replies; 5+ messages in thread
From: Benjamin Herrenschmidt @ 2016-07-05 3:13 UTC (permalink / raw)
To: David Gibson; +Cc: agraf, qemu-ppc, qemu-devel
On Tue, 2016-07-05 at 12:33 +1000, David Gibson wrote:
> Here are 3 fixes and cleanups to the path to look up hashed PTEs and
> decode their page size. This series is functionally equivalent to
> BenH's earlier posted "ppc/hash64: Various fixes in PTE search in the
> hash" but split up for clarity and with some unnnecessary renames
> removed.
>
> David Gibson (3):
> target-ppc: Correct page size decoding in ppc_hash64_pteg_search()
> target-ppc: Simplify HPTE matching
> target-ppc: Return page shift from PTEG search
All 3:
Reviewed-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> target-ppc/mmu-hash64.c | 133 ++++++++++++++++++------------------
> ------------
> target-ppc/mmu-hash64.h | 2 +-
> 2 files changed, 51 insertions(+), 84 deletions(-)
>
^ permalink raw reply [flat|nested] 5+ messages in thread