On 12/21/2015 01:41 PM, David Gibson wrote:
The h_enter() hypercall implementation in spapr_hcall.c contains some code
to determine the page size of the newly inserted hash PTE. This is
surprisingly complex in the general case, because POWER CPUs can have
different implementation dependent ways of encoding page sizes. The
current version assumes POWER7 or POWER8 and doesn't support all the
s/and doesn't/doesn't/ ?
possibilities of even those (but this is advertised correctly in the device
tree and so works with guests).
To support the possibility of future CPUs with different options, however,
this really belongs in the core ppc mmu code, rather than the spapr
("pseries" machine type) specific code.
So, move this code to a helper in target-ppc.
At the same time, uncomment some code to allow 64kiB pages. At the time
that was added, I believe other parts of the ppc mmu code didn't handle
64kiB pages, but that's since been fixed.
Signed-off-by: David Gibson <da...@gibson.dropbear.id.au>
---
hw/ppc/spapr_hcall.c | 26 ++++++++------------------
target-ppc/mmu-hash64.c | 31 +++++++++++++++++++++++++++++++
target-ppc/mmu-hash64.h | 3 +++
3 files changed, 42 insertions(+), 18 deletions(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index cebceea..c346e97 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -93,28 +93,18 @@ static target_ulong h_enter(PowerPCCPU *cpu,
sPAPRMachineState *spapr,
target_ulong pte_index = args[1];
target_ulong pteh = args[2];
target_ulong ptel = args[3];
- target_ulong page_shift = 12;
+ int page_shift;
+ uint64_t avpnm;
target_ulong raddr;
target_ulong index;
uint64_t token;
- /* only handle 4k and 16M pages for now */
- if (pteh & HPTE64_V_LARGE) {
-#if 0 /* We don't support 64k pages yet */
- if ((ptel & 0xf000) == 0x1000) {
- /* 64k page */
- } else
-#endif
- if ((ptel & 0xff000) == 0) {
- /* 16M page */
- page_shift = 24;
- /* lowest AVA bit must be 0 for 16M pages */
- if (pteh & 0x80) {
- return H_PARAMETER;
- }
- } else {
- return H_PARAMETER;
- }
+ if (ppc_hash64_hpte_shift(pteh, ptel, &page_shift, &avpnm) < 0) {
+ return H_PARAMETER;
+ }
+
+ if (HPTE64_V_AVPN_VAL(pteh) & avpnm) {
HPTE64_V_AVPN_VAL(pteh) & ((1ULL << page_shift) >> 23)
should do the same thing, no?
+ return H_PARAMETER;
}
raddr = (ptel & HPTE64_R_RPN) & ~((1ULL << page_shift) - 1);
diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c
index 34e20fa..d3b895d 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -399,6 +399,37 @@ static uint64_t ppc_hash64_page_shift(ppc_slb_t *slb)
return epnshift;
}
+/* Returns the page shift of the given hpte. This is the "base"
+ * shift, used for hashing. Although we don't implement it yet, the
+ * architecture allows the actuall mapped page to be larger, but
s/actuall/actual/
+ * determining that size requires information from the slb. */
+int ppc_hash64_hpte_shift(uint64_t pte0, uint64_t pte1,
+ int *shift, uint64_t *avpnm)
+{
+ *shift = 12; /* 4kiB default */
+
+ /* only handle 4k and 16M pages for now */
+ if (pte0 & HPTE64_V_LARGE) {
+ if ((pte1 & 0xf000) == 0x1000) {
Nit: it would improve readability of this code if HPTE64_R_KEY()&co was
used. What does define these "key" bits meaning?
+ *shift = 16; /* 64 kiB */
+ } else if ((pte1 & 0xff000) == 0) {
+ *shift = 24; /* 16MiB */
+ } else {
+ return -EINVAL;
+ }
+ }
+
+ if (avpnm) {
+ if (*shift <= 23) {
+ *avpnm = 0;
+ } else {
+ *avpnm = (1ULL << (*shift - 23)) - 1;
+ }
+ }
What does "avpnm" stand for? Abbreviated virtual page <something> mask?
If you dropped "if(avpnm)", you could move "*avpnm = 0;" to the beginning
and "*avpnm = (1ULL << (*shift - 23)) - 1;" next to "*shift = 24;".
+
+ return 0;
+}
+
static hwaddr ppc_hash64_htab_lookup(CPUPPCState *env,
ppc_slb_t *slb, target_ulong eaddr,
ppc_hash_pte64_t *pte)
diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h
index 291750f..71d2532 100644
--- a/target-ppc/mmu-hash64.h
+++ b/target-ppc/mmu-hash64.h
@@ -117,6 +117,9 @@ typedef struct {
uint64_t pte0, pte1;
} ppc_hash_pte64_t;
+int ppc_hash64_hpte_shift(uint64_t pte0, uint64_t pte1,
+ int *shift, uint64_t *avpnm);
+
#endif /* CONFIG_USER_ONLY */
#endif /* !defined (__MMU_HASH64_H__) */
--
Alexey
