* [Qemu-devel] [PATCHv2 1/9] target/ppc: Fix KVM-HV HPTE accessors
2017-02-27 5:12 [Qemu-devel] [PATCHv2 0/9] Cleanups to handling of hash MMU David Gibson
@ 2017-02-27 5:12 ` David Gibson
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 2/9] pseries: Minor cleanups to HPT management hypercalls David Gibson
` (7 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2017-02-27 5:12 UTC (permalink / raw)
To: qemu-ppc, aik, sjitindarsingh, aneesh.kumar
Cc: qemu-devel, thuth, lvivier, agraf, mdroth, David Gibson
When a 'pseries' guest is running with KVM-HV, the guest's hashed page
table (HPT) is stored within the host kernel, so it is not directly
accessible to qemu. Most of the time, qemu doesn't need to access it:
we're using the hardware MMU, and KVM itself implements the guest
hypercalls for manipulating the HPT.
However, qemu does need access to the in-KVM HPT to implement
get_phys_page_debug() for the benefit of the gdbstub, and maybe for
other debug operations.
To allow this, 7c43bca "target-ppc: Fix page table lookup with kvm
enabled" added kvmppc_hash64_read_pteg() to target/ppc/kvm.c to read
in a batch of HPTEs from the KVM table. Unfortunately, there are a
couple of problems with this:
First, the name of the function implies it always reads a whole PTEG
from the HPT, but in fact in some cases it's used to grab individual
HPTEs (which ends up pulling 8 HPTEs, not aligned to a PTEG from the
kernel).
Second, and more importantly, the code to read the HPTEs from KVM is
simply wrong, in general. The data from the fd that KVM provides is
designed mostly for compact migration rather than this sort of one-off
access, and so needs some decoding for this purpose. The current code
will work in some cases, but if there are invalid HPTEs then it will
not get sane results.
This patch rewrite the HPTE reading function to have a simpler
interface (just read n HPTEs into a caller provided buffer), and to
correctly decode the stream from the kernel.
For consistency we also clean up the similar function for altering
HPTEs within KVM (introduced in c138593 "target-ppc: Update
ppc_hash64_store_hpte to support updating in-kernel htab").
Cc: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
target/ppc/cpu.h | 1 +
target/ppc/kvm.c | 126 +++++++++++++++++++++++-------------------------
target/ppc/kvm_ppc.h | 20 ++------
target/ppc/mmu-hash64.c | 8 +--
target/ppc/mmu-hash64.h | 2 +-
5 files changed, 73 insertions(+), 84 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index b559b67..c022984 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -228,6 +228,7 @@ typedef struct DisasContext DisasContext;
typedef struct ppc_spr_t ppc_spr_t;
typedef union ppc_avr_t ppc_avr_t;
typedef union ppc_tlb_t ppc_tlb_t;
+typedef struct ppc_hash_pte64 ppc_hash_pte64_t;
/* SPR access micro-ops generations callbacks */
struct ppc_spr_t {
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 52bbea5..79c1da6 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2596,89 +2596,85 @@ void kvm_arch_init_irq_routing(KVMState *s)
{
}
-struct kvm_get_htab_buf {
- struct kvm_get_htab_header header;
- /*
- * We require one extra byte for read
- */
- target_ulong hpte[(HPTES_PER_GROUP * 2) + 1];
-};
-
-uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index)
+void kvmppc_read_hptes(ppc_hash_pte64_t *hptes, hwaddr ptex, int n)
{
- int htab_fd;
- struct kvm_get_htab_fd ghf;
- struct kvm_get_htab_buf *hpte_buf;
+ struct kvm_get_htab_fd ghf = {
+ .flags = 0,
+ .start_index = ptex,
+ };
+ int fd, rc;
+ int i;
- ghf.flags = 0;
- ghf.start_index = pte_index;
- htab_fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, &ghf);
- if (htab_fd < 0) {
- goto error_out;
+ fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, &ghf);
+ if (fd < 0) {
+ hw_error("kvmppc_read_hptes: Unable to open HPT fd");
}
- hpte_buf = g_malloc0(sizeof(*hpte_buf));
- /*
- * Read the hpte group
- */
- if (read(htab_fd, hpte_buf, sizeof(*hpte_buf)) < 0) {
- goto out_close;
- }
+ i = 0;
+ while (i < n) {
+ struct kvm_get_htab_header *hdr;
+ int m = n < HPTES_PER_GROUP ? n : HPTES_PER_GROUP;
+ char buf[sizeof(*hdr) + m * HASH_PTE_SIZE_64];
- close(htab_fd);
- return (uint64_t)(uintptr_t) hpte_buf->hpte;
+ rc = read(fd, buf, sizeof(buf));
+ if (rc < 0) {
+ hw_error("kvmppc_read_hptes: Unable to read HPTEs");
+ }
-out_close:
- g_free(hpte_buf);
- close(htab_fd);
-error_out:
- return 0;
-}
+ hdr = (struct kvm_get_htab_header *)buf;
+ while ((i < n) && ((char *)hdr < (buf + rc))) {
+ int invalid = hdr->n_invalid;
-void kvmppc_hash64_free_pteg(uint64_t token)
-{
- struct kvm_get_htab_buf *htab_buf;
+ if (hdr->index != (ptex + i)) {
+ hw_error("kvmppc_read_hptes: Unexpected HPTE index %"PRIu32
+ " != (%"HWADDR_PRIu" + %d", hdr->index, ptex, i);
+ }
+
+ memcpy(hptes + i, hdr + 1, HASH_PTE_SIZE_64 * hdr->n_valid);
+ i += hdr->n_valid;
- htab_buf = container_of((void *)(uintptr_t) token, struct kvm_get_htab_buf,
- hpte);
- g_free(htab_buf);
- return;
+ if ((n - i) < invalid) {
+ invalid = n - i;
+ }
+ memset(hptes + i, 0, invalid * HASH_PTE_SIZE_64);
+ i += hdr->n_invalid;
+
+ hdr = (struct kvm_get_htab_header *)
+ ((char *)(hdr + 1) + HASH_PTE_SIZE_64 * hdr->n_valid);
+ }
+ }
+
+ close(fd);
}
-void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
- target_ulong pte0, target_ulong pte1)
+void kvmppc_write_hpte(hwaddr ptex, uint64_t pte0, uint64_t pte1)
{
- int htab_fd;
+ int fd, rc;
struct kvm_get_htab_fd ghf;
- struct kvm_get_htab_buf hpte_buf;
+ struct {
+ struct kvm_get_htab_header hdr;
+ uint64_t pte0;
+ uint64_t pte1;
+ } buf;
ghf.flags = 0;
ghf.start_index = 0; /* Ignored */
- htab_fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, &ghf);
- if (htab_fd < 0) {
- goto error_out;
- }
-
- hpte_buf.header.n_valid = 1;
- hpte_buf.header.n_invalid = 0;
- hpte_buf.header.index = pte_index;
- hpte_buf.hpte[0] = pte0;
- hpte_buf.hpte[1] = pte1;
- /*
- * Write the hpte entry.
- * CAUTION: write() has the warn_unused_result attribute. Hence we
- * need to check the return value, even though we do nothing.
- */
- if (write(htab_fd, &hpte_buf, sizeof(hpte_buf)) < 0) {
- goto out_close;
+ fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, &ghf);
+ if (fd < 0) {
+ hw_error("kvmppc_write_hpte: Unable to open HPT fd");
}
-out_close:
- close(htab_fd);
- return;
+ buf.hdr.n_valid = 1;
+ buf.hdr.n_invalid = 0;
+ buf.hdr.index = ptex;
+ buf.pte0 = cpu_to_be64(pte0);
+ buf.pte1 = cpu_to_be64(pte1);
-error_out:
- return;
+ rc = write(fd, &buf, sizeof(buf));
+ if (rc != sizeof(buf)) {
+ hw_error("kvmppc_write_hpte: Unable to update KVM HPT");
+ }
+ close(fd);
}
int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 8da2ee4..8e9f42d 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -49,11 +49,8 @@ int kvmppc_get_htab_fd(bool write);
int kvmppc_save_htab(QEMUFile *f, int fd, size_t bufsize, int64_t max_ns);
int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
uint16_t n_valid, uint16_t n_invalid);
-uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu, target_ulong pte_index);
-void kvmppc_hash64_free_pteg(uint64_t token);
-
-void kvmppc_hash64_write_pte(CPUPPCState *env, target_ulong pte_index,
- target_ulong pte0, target_ulong pte1);
+void kvmppc_read_hptes(ppc_hash_pte64_t *hptes, hwaddr ptex, int n);
+void kvmppc_write_hpte(hwaddr ptex, uint64_t pte0, uint64_t pte1);
bool kvmppc_has_cap_fixup_hcalls(void);
bool kvmppc_has_cap_htm(void);
int kvmppc_enable_hwrng(void);
@@ -234,20 +231,13 @@ static inline int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
abort();
}
-static inline uint64_t kvmppc_hash64_read_pteg(PowerPCCPU *cpu,
- target_ulong pte_index)
-{
- abort();
-}
-
-static inline void kvmppc_hash64_free_pteg(uint64_t token)
+static inline void kvmppc_read_hptes(ppc_hash_pte64_t *hptes,
+ hwaddr ptex, int n)
{
abort();
}
-static inline void kvmppc_hash64_write_pte(CPUPPCState *env,
- target_ulong pte_index,
- target_ulong pte0, target_ulong pte1)
+static inline void kvmppc_write_hpte(hwaddr ptex, uint64_t pte0, uint64_t pte1)
{
abort();
}
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 76669ed..862e50e 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -438,10 +438,12 @@ uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index)
pte_offset = pte_index * HASH_PTE_SIZE_64;
if (cpu->env.external_htab == MMU_HASH64_KVM_MANAGED_HPT) {
+ ppc_hash_pte64_t *pteg = g_malloc(HASH_PTEG_SIZE_64);
/*
* HTAB is controlled by KVM. Fetch the PTEG into a new buffer.
*/
- token = kvmppc_hash64_read_pteg(cpu, pte_index);
+ kvmppc_read_hptes(pteg, pte_index, HPTES_PER_GROUP);
+ token = (uint64_t)(uintptr_t)pteg;
} else if (cpu->env.external_htab) {
/*
* HTAB is controlled by QEMU. Just point to the internally
@@ -457,7 +459,7 @@ uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index)
void ppc_hash64_stop_access(PowerPCCPU *cpu, uint64_t token)
{
if (cpu->env.external_htab == MMU_HASH64_KVM_MANAGED_HPT) {
- kvmppc_hash64_free_pteg(token);
+ g_free((void *)token);
}
}
@@ -916,7 +918,7 @@ void ppc_hash64_store_hpte(PowerPCCPU *cpu,
CPUPPCState *env = &cpu->env;
if (env->external_htab == MMU_HASH64_KVM_MANAGED_HPT) {
- kvmppc_hash64_write_pte(env, pte_index, pte0, pte1);
+ kvmppc_write_hpte(pte_index, pte0, pte1);
return;
}
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index 7a0b7fc..73aeaa3 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -127,7 +127,7 @@ static inline target_ulong ppc_hash64_load_hpte1(PowerPCCPU *cpu,
}
}
-typedef struct {
+typedef struct ppc_hash_pte64 {
uint64_t pte0, pte1;
} ppc_hash_pte64_t;
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCHv2 2/9] pseries: Minor cleanups to HPT management hypercalls
2017-02-27 5:12 [Qemu-devel] [PATCHv2 0/9] Cleanups to handling of hash MMU David Gibson
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 1/9] target/ppc: Fix KVM-HV HPTE accessors David Gibson
@ 2017-02-27 5:12 ` David Gibson
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 3/9] target/ppc: Merge cpu_ppc_set_vhyp() with cpu_ppc_set_papr() David Gibson
` (6 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2017-02-27 5:12 UTC (permalink / raw)
To: qemu-ppc, aik, sjitindarsingh, aneesh.kumar
Cc: qemu-devel, thuth, lvivier, agraf, mdroth, David Gibson
* Standardize on 'ptex' instead of 'pte_index' for HPTE index variables
for consistency and brevity
* Avoid variables named 'index'; shadowing index(3) from libc can lead to
surprising bugs if the variable is removed, because compiler errors
might not appear for remaining references
* Clarify index calculations in h_enter() - we have two cases, H_EXACT
where the exact HPTE slot is given, and !H_EXACT where we search for
an empty slot within the hash bucket. Make the calculation more
consistent between the cases.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
hw/ppc/spapr_hcall.c | 58 +++++++++++++++++++++++++---------------------------
1 file changed, 28 insertions(+), 30 deletions(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 42d20e0..3298a14 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -47,12 +47,12 @@ static bool has_spr(PowerPCCPU *cpu, int spr)
return cpu->env.spr_cb[spr].name != NULL;
}
-static inline bool valid_pte_index(CPUPPCState *env, target_ulong pte_index)
+static inline bool valid_ptex(PowerPCCPU *cpu, target_ulong ptex)
{
/*
* hash value/pteg group index is normalized by htab_mask
*/
- if (((pte_index & ~7ULL) / HPTES_PER_GROUP) & ~env->htab_mask) {
+ if (((ptex & ~7ULL) / HPTES_PER_GROUP) & ~cpu->env.htab_mask) {
return false;
}
return true;
@@ -77,14 +77,13 @@ static bool is_ram_address(sPAPRMachineState *spapr, hwaddr addr)
static target_ulong h_enter(PowerPCCPU *cpu, sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
{
- CPUPPCState *env = &cpu->env;
target_ulong flags = args[0];
- target_ulong pte_index = args[1];
+ target_ulong ptex = args[1];
target_ulong pteh = args[2];
target_ulong ptel = args[3];
unsigned apshift;
target_ulong raddr;
- target_ulong index;
+ target_ulong slot;
uint64_t token;
apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
@@ -116,25 +115,26 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPRMachineState *spapr,
pteh &= ~0x60ULL;
- if (!valid_pte_index(env, pte_index)) {
+ if (!valid_ptex(cpu, ptex)) {
return H_PARAMETER;
}
- index = 0;
+ slot = ptex & 7ULL;
+ ptex = ptex & ~7ULL;
+
if (likely((flags & H_EXACT) == 0)) {
- pte_index &= ~7ULL;
- token = ppc_hash64_start_access(cpu, pte_index);
- for (; index < 8; index++) {
- if (!(ppc_hash64_load_hpte0(cpu, token, index) & HPTE64_V_VALID)) {
+ token = ppc_hash64_start_access(cpu, ptex);
+ for (slot = 0; slot < 8; slot++) {
+ if (!(ppc_hash64_load_hpte0(cpu, token, slot) & HPTE64_V_VALID)) {
break;
}
}
ppc_hash64_stop_access(cpu, token);
- if (index == 8) {
+ if (slot == 8) {
return H_PTEG_FULL;
}
} else {
- token = ppc_hash64_start_access(cpu, pte_index);
+ token = ppc_hash64_start_access(cpu, ptex);
if (ppc_hash64_load_hpte0(cpu, token, 0) & HPTE64_V_VALID) {
ppc_hash64_stop_access(cpu, token);
return H_PTEG_FULL;
@@ -142,10 +142,9 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPRMachineState *spapr,
ppc_hash64_stop_access(cpu, token);
}
- ppc_hash64_store_hpte(cpu, pte_index + index,
- pteh | HPTE64_V_HPTE_DIRTY, ptel);
+ ppc_hash64_store_hpte(cpu, ptex + slot, pteh | HPTE64_V_HPTE_DIRTY, ptel);
- args[0] = pte_index + index;
+ args[0] = ptex + slot;
return H_SUCCESS;
}
@@ -161,11 +160,10 @@ static RemoveResult remove_hpte(PowerPCCPU *cpu, target_ulong ptex,
target_ulong flags,
target_ulong *vp, target_ulong *rp)
{
- CPUPPCState *env = &cpu->env;
uint64_t token;
target_ulong v, r;
- if (!valid_pte_index(env, ptex)) {
+ if (!valid_ptex(cpu, ptex)) {
return REMOVE_PARM;
}
@@ -191,11 +189,11 @@ static target_ulong h_remove(PowerPCCPU *cpu, sPAPRMachineState *spapr,
{
CPUPPCState *env = &cpu->env;
target_ulong flags = args[0];
- target_ulong pte_index = args[1];
+ target_ulong ptex = args[1];
target_ulong avpn = args[2];
RemoveResult ret;
- ret = remove_hpte(cpu, pte_index, avpn, flags,
+ ret = remove_hpte(cpu, ptex, avpn, flags,
&args[0], &args[1]);
switch (ret) {
@@ -291,16 +289,16 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPRMachineState *spapr,
{
CPUPPCState *env = &cpu->env;
target_ulong flags = args[0];
- target_ulong pte_index = args[1];
+ target_ulong ptex = args[1];
target_ulong avpn = args[2];
uint64_t token;
target_ulong v, r;
- if (!valid_pte_index(env, pte_index)) {
+ if (!valid_ptex(cpu, ptex)) {
return H_PARAMETER;
}
- token = ppc_hash64_start_access(cpu, pte_index);
+ token = ppc_hash64_start_access(cpu, ptex);
v = ppc_hash64_load_hpte0(cpu, token, 0);
r = ppc_hash64_load_hpte1(cpu, token, 0);
ppc_hash64_stop_access(cpu, token);
@@ -315,13 +313,13 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPRMachineState *spapr,
r |= (flags << 55) & HPTE64_R_PP0;
r |= (flags << 48) & HPTE64_R_KEY_HI;
r |= flags & (HPTE64_R_PP | HPTE64_R_N | HPTE64_R_KEY_LO);
- ppc_hash64_store_hpte(cpu, pte_index,
+ ppc_hash64_store_hpte(cpu, ptex,
(v & ~HPTE64_V_VALID) | HPTE64_V_HPTE_DIRTY, 0);
- ppc_hash64_tlb_flush_hpte(cpu, pte_index, v, r);
+ ppc_hash64_tlb_flush_hpte(cpu, ptex, v, r);
/* Flush the tlb */
check_tlb_flush(env, true);
/* Don't need a memory barrier, due to qemu's global lock */
- ppc_hash64_store_hpte(cpu, pte_index, v | HPTE64_V_HPTE_DIRTY, r);
+ ppc_hash64_store_hpte(cpu, ptex, v | HPTE64_V_HPTE_DIRTY, r);
return H_SUCCESS;
}
@@ -330,21 +328,21 @@ static target_ulong h_read(PowerPCCPU *cpu, sPAPRMachineState *spapr,
{
CPUPPCState *env = &cpu->env;
target_ulong flags = args[0];
- target_ulong pte_index = args[1];
+ target_ulong ptex = args[1];
uint8_t *hpte;
int i, ridx, n_entries = 1;
- if (!valid_pte_index(env, pte_index)) {
+ if (!valid_ptex(cpu, ptex)) {
return H_PARAMETER;
}
if (flags & H_READ_4) {
/* Clear the two low order bits */
- pte_index &= ~(3ULL);
+ ptex &= ~(3ULL);
n_entries = 4;
}
- hpte = env->external_htab + (pte_index * HASH_PTE_SIZE_64);
+ hpte = env->external_htab + (ptex * HASH_PTE_SIZE_64);
for (i = 0, ridx = 0; i < n_entries; i++) {
args[ridx++] = ldq_p(hpte);
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCHv2 3/9] target/ppc: Merge cpu_ppc_set_vhyp() with cpu_ppc_set_papr()
2017-02-27 5:12 [Qemu-devel] [PATCHv2 0/9] Cleanups to handling of hash MMU David Gibson
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 1/9] target/ppc: Fix KVM-HV HPTE accessors David Gibson
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 2/9] pseries: Minor cleanups to HPT management hypercalls David Gibson
@ 2017-02-27 5:12 ` David Gibson
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 4/9] target/ppc: SDR1 is a hypervisor resource David Gibson
` (5 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2017-02-27 5:12 UTC (permalink / raw)
To: qemu-ppc, aik, sjitindarsingh, aneesh.kumar
Cc: qemu-devel, thuth, lvivier, agraf, mdroth, David Gibson
cpu_ppc_set_papr() sets up various aspects of CPU state for use with PAPR
paravirtualized guests. However, it doesn't set the virtual hypervisor,
so callers must also call cpu_ppc_set_vhyp() so that PAPR hypercalls are
handled properly. This is a bit silly, so fold setting the virtual
hypervisor into cpu_ppc_set_papr().
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
hw/ppc/spapr_cpu_core.c | 3 +--
target/ppc/cpu.h | 3 +--
target/ppc/translate_init.c | 10 +++-------
3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 55cd045..76563c4 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -57,8 +57,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
/* Enable PAPR mode in TCG or KVM */
- cpu_ppc_set_vhyp(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
- cpu_ppc_set_papr(cpu);
+ cpu_ppc_set_papr(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
if (cpu->max_compat) {
Error *local_err = NULL;
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index c022984..2a94e76 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1301,8 +1301,7 @@ void store_booke_tcr (CPUPPCState *env, target_ulong val);
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);
-void cpu_ppc_set_papr(PowerPCCPU *cpu);
+void cpu_ppc_set_papr(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp);
#endif
#endif
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index be35cbd..a1405e9 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -8835,18 +8835,14 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
}
#if !defined(CONFIG_USER_ONLY)
-
-void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
-{
- cpu->vhyp = vhyp;
-}
-
-void cpu_ppc_set_papr(PowerPCCPU *cpu)
+void cpu_ppc_set_papr(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
{
CPUPPCState *env = &cpu->env;
ppc_spr_t *lpcr = &env->spr_cb[SPR_LPCR];
ppc_spr_t *amor = &env->spr_cb[SPR_AMOR];
+ cpu->vhyp = vhyp;
+
/* PAPR always has exception vectors in RAM not ROM. To ensure this,
* MSR[IP] should never be set.
*
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCHv2 4/9] target/ppc: SDR1 is a hypervisor resource
2017-02-27 5:12 [Qemu-devel] [PATCHv2 0/9] Cleanups to handling of hash MMU David Gibson
` (2 preceding siblings ...)
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 3/9] target/ppc: Merge cpu_ppc_set_vhyp() with cpu_ppc_set_papr() David Gibson
@ 2017-02-27 5:12 ` David Gibson
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 5/9] target/ppc: Cleanup HPTE accessors for 64-bit hash MMU David Gibson
` (4 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2017-02-27 5:12 UTC (permalink / raw)
To: qemu-ppc, aik, sjitindarsingh, aneesh.kumar
Cc: qemu-devel, thuth, lvivier, agraf, mdroth, David Gibson
At present the SDR1 register - the base of the system's hashed page table
(HPT) - is represented as an SPR with supervisor read and write permission.
However, on CPUs which have a hypervisor mode, the SDR1 is a hypervisor
only resource. Change the permission checking on the SPR to reflect this.
Now that this is done, we don't need to check for an external HPT executing
mtsdr1: an external HPT only applies when we're emulating the behaviour of
a hypervisor, rather than modelling the CPU's hypervisor mode internally,
so if we're permitted to execute mtsdr1, we don't have an external HPT.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
target/ppc/misc_helper.c | 8 +++-----
target/ppc/translate_init.c | 20 ++++++++++++++++----
2 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index ab432ba..fa573dd 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -82,11 +82,9 @@ void helper_store_sdr1(CPUPPCState *env, target_ulong val)
{
PowerPCCPU *cpu = ppc_env_get_cpu(env);
- if (!env->external_htab) {
- if (env->spr[SPR_SDR1] != val) {
- ppc_store_sdr1(env, val);
- tlb_flush(CPU(cpu));
- }
+ if (env->spr[SPR_SDR1] != val) {
+ ppc_store_sdr1(env, val);
+ tlb_flush(CPU(cpu));
}
}
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index a1405e9..c92435d 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -740,10 +740,22 @@ static void gen_spr_ne_601 (CPUPPCState *env)
&spr_read_decr, &spr_write_decr,
0x00000000);
/* Memory management */
- spr_register(env, SPR_SDR1, "SDR1",
- SPR_NOACCESS, SPR_NOACCESS,
- &spr_read_generic, &spr_write_sdr1,
- 0x00000000);
+#ifndef CONFIG_USER_ONLY
+ if (env->has_hv_mode) {
+ /* SDR1 is a hypervisor resource on CPUs which have a
+ * hypervisor mode */
+ spr_register_hv(env, SPR_SDR1, "SDR1",
+ SPR_NOACCESS, SPR_NOACCESS,
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_sdr1,
+ 0x00000000);
+ } else {
+ spr_register(env, SPR_SDR1, "SDR1",
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_sdr1,
+ 0x00000000);
+ }
+#endif
}
/* BATs 0-3 */
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCHv2 5/9] target/ppc: Cleanup HPTE accessors for 64-bit hash MMU
2017-02-27 5:12 [Qemu-devel] [PATCHv2 0/9] Cleanups to handling of hash MMU David Gibson
` (3 preceding siblings ...)
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 4/9] target/ppc: SDR1 is a hypervisor resource David Gibson
@ 2017-02-27 5:12 ` David Gibson
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 6/9] target/ppc: Eliminate htab_base and htab_mask variables David Gibson
` (3 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2017-02-27 5:12 UTC (permalink / raw)
To: qemu-ppc, aik, sjitindarsingh, aneesh.kumar
Cc: qemu-devel, thuth, lvivier, agraf, mdroth, David Gibson
Accesses to the hashed page table (HPT) are complicated by the fact that
the HPT could be in one of three places:
1) Within guest memory - when we're emulating a full guest CPU at the
hardware level (e.g. powernv, mac99, g3beige)
2) Within qemu, but outside guest memory - when we're emulating user and
supervisor instructions within TCG, but instead of emulating
the CPU's hypervisor mode, we just emulate a hypervisor's behaviour
(pseries in TCG or KVM-PR)
3) Within the host kernel - a pseries machine using KVM-HV
acceleration. Mostly accesses to the HPT are handled by KVM,
but there are a few cases where qemu needs to access it via a
special fd for the purpose.
In order to batch accesses to the fd in case (3), we use a somewhat awkward
ppc_hash64_start_access() / ppc_hash64_stop_access() pair, which for case
(3) reads / releases several HPTEs from the kernel as a batch (usually a
whole PTEG). For cases (1) & (2) it just returns an address value. The
actual HPTE load helpers then need to interpret the returned token
differently in the 3 cases.
This patch keeps the same basic structure, but simplfiies the details.
First start_access() / stop_access() are renamed to map_hptes() and
unmap_hptes() to make their operation more obvious. Second, map_hptes()
now always returns a qemu pointer, which can always be used in the same way
by the load_hpte() helpers. In case (1) it comes from address_space_map()
in case (2) directly from qemu's HPT buffer and in case (3) from a
temporary buffer read from the KVM fd.
While we're at it, make things a bit more consistent in terms of types and
variable names: avoid variables named 'index' (it shadows index(3) which
can lead to confusing results), use 'hwaddr ptex' for HPTE indices and
uint64_t for each of the HPTE words, use ptex throughout the call stack
instead of pte_offset in some places (we still need that at the bottom
layer, but nowhere else).
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ppc/spapr_hcall.c | 36 ++++++++---------
target/ppc/cpu.h | 2 +-
target/ppc/mmu-hash64.c | 103 +++++++++++++++++++++++++-----------------------
target/ppc/mmu-hash64.h | 46 ++++++++-------------
4 files changed, 89 insertions(+), 98 deletions(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 3298a14..fd961b5 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -84,7 +84,7 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPRMachineState *spapr,
unsigned apshift;
target_ulong raddr;
target_ulong slot;
- uint64_t token;
+ const ppc_hash_pte64_t *hptes;
apshift = ppc_hash64_hpte_page_shift_noslb(cpu, pteh, ptel);
if (!apshift) {
@@ -123,23 +123,23 @@ static target_ulong h_enter(PowerPCCPU *cpu, sPAPRMachineState *spapr,
ptex = ptex & ~7ULL;
if (likely((flags & H_EXACT) == 0)) {
- token = ppc_hash64_start_access(cpu, ptex);
+ hptes = ppc_hash64_map_hptes(cpu, ptex, HPTES_PER_GROUP);
for (slot = 0; slot < 8; slot++) {
- if (!(ppc_hash64_load_hpte0(cpu, token, slot) & HPTE64_V_VALID)) {
+ if (!(ppc_hash64_hpte0(cpu, hptes, slot) & HPTE64_V_VALID)) {
break;
}
}
- ppc_hash64_stop_access(cpu, token);
+ ppc_hash64_unmap_hptes(cpu, hptes, ptex, HPTES_PER_GROUP);
if (slot == 8) {
return H_PTEG_FULL;
}
} else {
- token = ppc_hash64_start_access(cpu, ptex);
- if (ppc_hash64_load_hpte0(cpu, token, 0) & HPTE64_V_VALID) {
- ppc_hash64_stop_access(cpu, token);
+ hptes = ppc_hash64_map_hptes(cpu, ptex + slot, 1);
+ if (ppc_hash64_hpte0(cpu, hptes, 0) & HPTE64_V_VALID) {
+ ppc_hash64_unmap_hptes(cpu, hptes, ptex + slot, 1);
return H_PTEG_FULL;
}
- ppc_hash64_stop_access(cpu, token);
+ ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
}
ppc_hash64_store_hpte(cpu, ptex + slot, pteh | HPTE64_V_HPTE_DIRTY, ptel);
@@ -160,17 +160,17 @@ static RemoveResult remove_hpte(PowerPCCPU *cpu, target_ulong ptex,
target_ulong flags,
target_ulong *vp, target_ulong *rp)
{
- uint64_t token;
+ const ppc_hash_pte64_t *hptes;
target_ulong v, r;
if (!valid_ptex(cpu, ptex)) {
return REMOVE_PARM;
}
- token = ppc_hash64_start_access(cpu, ptex);
- v = ppc_hash64_load_hpte0(cpu, token, 0);
- r = ppc_hash64_load_hpte1(cpu, token, 0);
- ppc_hash64_stop_access(cpu, token);
+ hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
+ v = ppc_hash64_hpte0(cpu, hptes, 0);
+ r = ppc_hash64_hpte1(cpu, hptes, 0);
+ ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
if ((v & HPTE64_V_VALID) == 0 ||
((flags & H_AVPN) && (v & ~0x7fULL) != avpn) ||
@@ -291,17 +291,17 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPRMachineState *spapr,
target_ulong flags = args[0];
target_ulong ptex = args[1];
target_ulong avpn = args[2];
- uint64_t token;
+ const ppc_hash_pte64_t *hptes;
target_ulong v, r;
if (!valid_ptex(cpu, ptex)) {
return H_PARAMETER;
}
- token = ppc_hash64_start_access(cpu, ptex);
- v = ppc_hash64_load_hpte0(cpu, token, 0);
- r = ppc_hash64_load_hpte1(cpu, token, 0);
- ppc_hash64_stop_access(cpu, token);
+ hptes = ppc_hash64_map_hptes(cpu, ptex, 1);
+ v = ppc_hash64_hpte0(cpu, hptes, 0);
+ r = ppc_hash64_hpte1(cpu, hptes, 0);
+ ppc_hash64_unmap_hptes(cpu, hptes, ptex, 1);
if ((v & HPTE64_V_VALID) == 0 ||
((flags & H_AVPN) && (v & ~0x7fULL) != avpn)) {
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 2a94e76..051298b 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -223,7 +223,7 @@ enum {
typedef struct opc_handler_t opc_handler_t;
/*****************************************************************************/
-/* Types used to describe some PowerPC registers */
+/* Types used to describe some PowerPC registers etc. */
typedef struct DisasContext DisasContext;
typedef struct ppc_spr_t ppc_spr_t;
typedef union ppc_avr_t ppc_avr_t;
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 862e50e..f503c69 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -27,6 +27,7 @@
#include "kvm_ppc.h"
#include "mmu-hash64.h"
#include "exec/log.h"
+#include "hw/hw.h"
//#define DEBUG_SLB
@@ -431,35 +432,43 @@ static int ppc_hash64_amr_prot(PowerPCCPU *cpu, ppc_hash_pte64_t pte)
return prot;
}
-uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index)
+const ppc_hash_pte64_t *ppc_hash64_map_hptes(PowerPCCPU *cpu,
+ hwaddr ptex, int n)
{
- uint64_t token = 0;
- hwaddr pte_offset;
+ ppc_hash_pte64_t *hptes = NULL;
+ hwaddr pte_offset = ptex * HASH_PTE_SIZE_64;
- pte_offset = pte_index * HASH_PTE_SIZE_64;
if (cpu->env.external_htab == MMU_HASH64_KVM_MANAGED_HPT) {
- ppc_hash_pte64_t *pteg = g_malloc(HASH_PTEG_SIZE_64);
/*
- * HTAB is controlled by KVM. Fetch the PTEG into a new buffer.
+ * HTAB is controlled by KVM. Fetch into temporary buffer
*/
- kvmppc_read_hptes(pteg, pte_index, HPTES_PER_GROUP);
- token = (uint64_t)(uintptr_t)pteg;
+ hptes = g_malloc(HASH_PTEG_SIZE_64);
+ kvmppc_read_hptes(hptes, ptex, n);
} else if (cpu->env.external_htab) {
/*
* HTAB is controlled by QEMU. Just point to the internally
* accessible PTEG.
*/
- token = (uint64_t)(uintptr_t) cpu->env.external_htab + pte_offset;
+ hptes = (ppc_hash_pte64_t *)(cpu->env.external_htab + pte_offset);
} else if (cpu->env.htab_base) {
- token = cpu->env.htab_base + pte_offset;
+ hwaddr plen = n * HASH_PTE_SIZE_64;
+ hptes = address_space_map(CPU(cpu)->as, cpu->env.htab_base + pte_offset,
+ &plen, false);
+ if (plen < (n * HASH_PTE_SIZE_64)) {
+ hw_error("%s: Unable to map all requested HPTEs\n", __func__);
+ }
}
- return token;
+ return hptes;
}
-void ppc_hash64_stop_access(PowerPCCPU *cpu, uint64_t token)
+void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const ppc_hash_pte64_t *hptes,
+ hwaddr ptex, int n)
{
if (cpu->env.external_htab == MMU_HASH64_KVM_MANAGED_HPT) {
- g_free((void *)token);
+ g_free((void *)hptes);
+ } else if (!cpu->env.external_htab) {
+ address_space_unmap(CPU(cpu)->as, (void *)hptes, n * HASH_PTE_SIZE_64,
+ false, n * HASH_PTE_SIZE_64);
}
}
@@ -507,18 +516,18 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
{
CPUPPCState *env = &cpu->env;
int i;
- uint64_t token;
+ const ppc_hash_pte64_t *pteg;
target_ulong pte0, pte1;
- target_ulong pte_index;
+ target_ulong ptex;
- pte_index = (hash & env->htab_mask) * HPTES_PER_GROUP;
- token = ppc_hash64_start_access(cpu, pte_index);
- if (!token) {
+ ptex = (hash & env->htab_mask) * HPTES_PER_GROUP;
+ pteg = ppc_hash64_map_hptes(cpu, ptex, HPTES_PER_GROUP);
+ if (!pteg) {
return -1;
}
for (i = 0; i < HPTES_PER_GROUP; i++) {
- pte0 = ppc_hash64_load_hpte0(cpu, token, i);
- pte1 = ppc_hash64_load_hpte1(cpu, token, i);
+ pte0 = ppc_hash64_hpte0(cpu, pteg, i);
+ pte1 = ppc_hash64_hpte1(cpu, pteg, i);
/* This compares V, B, H (secondary) and the AVPN */
if (HPTE64_V_COMPARE(pte0, ptem)) {
@@ -538,11 +547,11 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
*/
pte->pte0 = pte0;
pte->pte1 = pte1;
- ppc_hash64_stop_access(cpu, token);
- return (pte_index + i) * HASH_PTE_SIZE_64;
+ ppc_hash64_unmap_hptes(cpu, pteg, ptex, HPTES_PER_GROUP);
+ return ptex + i;
}
}
- ppc_hash64_stop_access(cpu, token);
+ ppc_hash64_unmap_hptes(cpu, pteg, ptex, HPTES_PER_GROUP);
/*
* We didn't find a valid entry.
*/
@@ -554,8 +563,7 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
ppc_hash_pte64_t *pte, unsigned *pshift)
{
CPUPPCState *env = &cpu->env;
- hwaddr pte_offset;
- hwaddr hash;
+ hwaddr hash, ptex;
uint64_t vsid, epnmask, epn, ptem;
const struct ppc_one_seg_page_size *sps = slb->sps;
@@ -598,9 +606,9 @@ 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, sps, ptem, pte, pshift);
+ ptex = ppc_hash64_pteg_search(cpu, hash, sps, ptem, pte, pshift);
- if (pte_offset == -1) {
+ if (ptex == -1) {
/* Secondary PTEG lookup */
ptem |= HPTE64_V_SECONDARY;
qemu_log_mask(CPU_LOG_MMU,
@@ -609,10 +617,10 @@ 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, sps, ptem, pte, pshift);
+ ptex = ppc_hash64_pteg_search(cpu, ~hash, sps, ptem, pte, pshift);
}
- return pte_offset;
+ return ptex;
}
unsigned ppc_hash64_hpte_page_shift_noslb(PowerPCCPU *cpu,
@@ -710,7 +718,7 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
CPUPPCState *env = &cpu->env;
ppc_slb_t *slb;
unsigned apshift;
- hwaddr pte_offset;
+ hwaddr ptex;
ppc_hash_pte64_t pte;
int pp_prot, amr_prot, prot;
uint64_t new_pte1, dsisr;
@@ -794,8 +802,8 @@ skip_slb_search:
}
/* 4. Locate the PTE in the hash table */
- pte_offset = ppc_hash64_htab_lookup(cpu, slb, eaddr, &pte, &apshift);
- if (pte_offset == -1) {
+ ptex = ppc_hash64_htab_lookup(cpu, slb, eaddr, &pte, &apshift);
+ if (ptex == -1) {
dsisr = 0x40000000;
if (rwx == 2) {
ppc_hash64_set_isi(cs, env, dsisr);
@@ -808,7 +816,7 @@ skip_slb_search:
return 1;
}
qemu_log_mask(CPU_LOG_MMU,
- "found PTE at offset %08" HWADDR_PRIx "\n", pte_offset);
+ "found PTE at index %08" HWADDR_PRIx "\n", ptex);
/* 5. Check access permissions */
@@ -851,8 +859,7 @@ skip_slb_search:
}
if (new_pte1 != pte.pte1) {
- ppc_hash64_store_hpte(cpu, pte_offset / HASH_PTE_SIZE_64,
- pte.pte0, new_pte1);
+ ppc_hash64_store_hpte(cpu, ptex, pte.pte0, new_pte1);
}
/* 7. Determine the real address from the PTE */
@@ -869,7 +876,7 @@ hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr)
{
CPUPPCState *env = &cpu->env;
ppc_slb_t *slb;
- hwaddr pte_offset, raddr;
+ hwaddr ptex, raddr;
ppc_hash_pte64_t pte;
unsigned apshift;
@@ -902,8 +909,8 @@ hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr)
}
}
- pte_offset = ppc_hash64_htab_lookup(cpu, slb, addr, &pte, &apshift);
- if (pte_offset == -1) {
+ ptex = ppc_hash64_htab_lookup(cpu, slb, addr, &pte, &apshift);
+ if (ptex == -1) {
return -1;
}
@@ -911,30 +918,28 @@ hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr)
& TARGET_PAGE_MASK;
}
-void ppc_hash64_store_hpte(PowerPCCPU *cpu,
- target_ulong pte_index,
- target_ulong pte0, target_ulong pte1)
+void ppc_hash64_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
+ uint64_t pte0, uint64_t pte1)
{
CPUPPCState *env = &cpu->env;
+ hwaddr offset = ptex * HASH_PTE_SIZE_64;
if (env->external_htab == MMU_HASH64_KVM_MANAGED_HPT) {
- kvmppc_write_hpte(pte_index, pte0, pte1);
+ kvmppc_write_hpte(ptex, pte0, pte1);
return;
}
- pte_index *= HASH_PTE_SIZE_64;
if (env->external_htab) {
- stq_p(env->external_htab + pte_index, pte0);
- stq_p(env->external_htab + pte_index + HASH_PTE_SIZE_64 / 2, pte1);
+ stq_p(env->external_htab + offset, pte0);
+ stq_p(env->external_htab + offset + HASH_PTE_SIZE_64 / 2, pte1);
} else {
- stq_phys(CPU(cpu)->as, env->htab_base + pte_index, pte0);
+ stq_phys(CPU(cpu)->as, env->htab_base + offset, pte0);
stq_phys(CPU(cpu)->as,
- env->htab_base + pte_index + HASH_PTE_SIZE_64 / 2, pte1);
+ env->htab_base + offset + HASH_PTE_SIZE_64 / 2, pte1);
}
}
-void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
- target_ulong pte_index,
+void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu, target_ulong ptex,
target_ulong pte0, target_ulong pte1)
{
/*
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index 73aeaa3..edd687b 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -10,8 +10,8 @@ int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr address, int rw,
int mmu_idx);
-void ppc_hash64_store_hpte(PowerPCCPU *cpu, target_ulong index,
- target_ulong pte0, target_ulong pte1);
+void ppc_hash64_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
+ uint64_t pte0, uint64_t pte1);
void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
target_ulong pte_index,
target_ulong pte0, target_ulong pte1);
@@ -96,41 +96,27 @@ void ppc_hash64_set_sdr1(PowerPCCPU *cpu, target_ulong value,
void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void *hpt, int shift,
Error **errp);
-uint64_t ppc_hash64_start_access(PowerPCCPU *cpu, target_ulong pte_index);
-void ppc_hash64_stop_access(PowerPCCPU *cpu, uint64_t token);
+struct ppc_hash_pte64 {
+ uint64_t pte0, pte1;
+};
+
+const ppc_hash_pte64_t *ppc_hash64_map_hptes(PowerPCCPU *cpu,
+ hwaddr ptex, int n);
+void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const ppc_hash_pte64_t *hptes,
+ hwaddr ptex, int n);
-static inline target_ulong ppc_hash64_load_hpte0(PowerPCCPU *cpu,
- uint64_t token, int index)
+static inline uint64_t ppc_hash64_hpte0(PowerPCCPU *cpu,
+ const ppc_hash_pte64_t *hptes, int i)
{
- CPUPPCState *env = &cpu->env;
- uint64_t addr;
-
- addr = token + (index * HASH_PTE_SIZE_64);
- if (env->external_htab) {
- return ldq_p((const void *)(uintptr_t)addr);
- } else {
- return ldq_phys(CPU(cpu)->as, addr);
- }
+ return ldq_p(&(hptes[i].pte0));
}
-static inline target_ulong ppc_hash64_load_hpte1(PowerPCCPU *cpu,
- uint64_t token, int index)
+static inline uint64_t ppc_hash64_hpte1(PowerPCCPU *cpu,
+ const ppc_hash_pte64_t *hptes, int i)
{
- CPUPPCState *env = &cpu->env;
- uint64_t addr;
-
- addr = token + (index * HASH_PTE_SIZE_64) + HASH_PTE_SIZE_64/2;
- if (env->external_htab) {
- return ldq_p((const void *)(uintptr_t)addr);
- } else {
- return ldq_phys(CPU(cpu)->as, addr);
- }
+ return ldq_p(&(hptes[i].pte1));
}
-typedef struct ppc_hash_pte64 {
- uint64_t pte0, pte1;
-} ppc_hash_pte64_t;
-
#endif /* CONFIG_USER_ONLY */
#endif /* MMU_HASH64_H */
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCHv2 6/9] target/ppc: Eliminate htab_base and htab_mask variables
2017-02-27 5:12 [Qemu-devel] [PATCHv2 0/9] Cleanups to handling of hash MMU David Gibson
` (4 preceding siblings ...)
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 5/9] target/ppc: Cleanup HPTE accessors for 64-bit hash MMU David Gibson
@ 2017-02-27 5:12 ` David Gibson
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 7/9] target/ppc: Manage external HPT via virtual hypervisor David Gibson
` (2 subsequent siblings)
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2017-02-27 5:12 UTC (permalink / raw)
To: qemu-ppc, aik, sjitindarsingh, aneesh.kumar
Cc: qemu-devel, thuth, lvivier, agraf, mdroth, David Gibson
CPUPPCState includes fields htab_base and htab_mask which store the base
address (GPA) and size (as a mask) of the guest's hashed page table (HPT).
These are set when the SDR1 register is updated.
Keeping these in sync with the SDR1 is actually a little bit fiddly, and
probably not useful for performance, since keeping them expands the size of
CPUPPCState. It also makes some upcoming changes harder to implement.
This patch removes these fields, in favour of calculating them directly
from the SDR1 contents when necessary.
This does make a change to the behaviour of attempting to write a bad value
(invalid HPT size) to the SDR1 with an mtspr instruction. Previously, the
bad value would be stored in SDR1 and could be retrieved with a later
mfspr, but the HPT size as used by the softmmu would be, clamped to the
allowed values. Now, writing a bad value is treated as a no-op. An error
message is printed in both new and old versions.
I'm not sure which behaviour, if either, matches real hardware. I don't
think it matters that much, since it's pretty clear that if an OS writes
a bad value to SDR1, it's not going to boot.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/ppc/spapr_hcall.c | 4 ++--
target/ppc/cpu.h | 11 -----------
target/ppc/machine.c | 1 -
target/ppc/mmu-hash32.c | 14 +++++++-------
target/ppc/mmu-hash32.h | 26 ++++++++++++++++++++------
target/ppc/mmu-hash64.c | 35 +++++++++++++++--------------------
target/ppc/mmu-hash64.h | 13 +++++++++++++
target/ppc/mmu_helper.c | 31 ++++++++++++++++---------------
8 files changed, 73 insertions(+), 62 deletions(-)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index fd961b5..85d96f6 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -50,9 +50,9 @@ static bool has_spr(PowerPCCPU *cpu, int spr)
static inline bool valid_ptex(PowerPCCPU *cpu, target_ulong ptex)
{
/*
- * hash value/pteg group index is normalized by htab_mask
+ * hash value/pteg group index is normalized by HPT mask
*/
- if (((ptex & ~7ULL) / HPTES_PER_GROUP) & ~cpu->env.htab_mask) {
+ if (((ptex & ~7ULL) / HPTES_PER_GROUP) & ~ppc_hash64_hpt_mask(cpu)) {
return false;
}
return true;
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 051298b..aaf79f5 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -306,14 +306,6 @@ union ppc_tlb_t {
#define TLB_MAS 3
#endif
-#define SDR_32_HTABORG 0xFFFF0000UL
-#define SDR_32_HTABMASK 0x000001FFUL
-
-#if defined(TARGET_PPC64)
-#define SDR_64_HTABORG 0xFFFFFFFFFFFC0000ULL
-#define SDR_64_HTABSIZE 0x000000000000001FULL
-#endif /* defined(TARGET_PPC64 */
-
typedef struct ppc_slb_t ppc_slb_t;
struct ppc_slb_t {
uint64_t esid;
@@ -1006,9 +998,6 @@ struct CPUPPCState {
/* tcg TLB needs flush (deferred slb inval instruction typically) */
#endif
/* segment registers */
- hwaddr htab_base;
- /* mask used to normalize hash value to PTEG index */
- hwaddr htab_mask;
target_ulong sr[32];
/* externally stored hash table */
uint8_t *external_htab;
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index df9f7a4..1ccbc8a 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -229,7 +229,6 @@ static int cpu_post_load(void *opaque, int version_id)
}
if (!env->external_htab) {
- /* Restore htab_base and htab_mask variables */
ppc_store_sdr1(env, env->spr[SPR_SDR1]);
}
diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
index 29bace6..03ae3c1 100644
--- a/target/ppc/mmu-hash32.c
+++ b/target/ppc/mmu-hash32.c
@@ -304,9 +304,9 @@ static int ppc_hash32_direct_store(PowerPCCPU *cpu, target_ulong sr,
hwaddr get_pteg_offset32(PowerPCCPU *cpu, hwaddr hash)
{
- CPUPPCState *env = &cpu->env;
+ target_ulong mask = ppc_hash32_hpt_mask(cpu);
- return (hash * HASH_PTEG_SIZE_32) & env->htab_mask;
+ return (hash * HASH_PTEG_SIZE_32) & mask;
}
static hwaddr ppc_hash32_pteg_search(PowerPCCPU *cpu, hwaddr pteg_off,
@@ -339,7 +339,6 @@ static hwaddr ppc_hash32_htab_lookup(PowerPCCPU *cpu,
target_ulong sr, target_ulong eaddr,
ppc_hash_pte32_t *pte)
{
- CPUPPCState *env = &cpu->env;
hwaddr pteg_off, pte_offset;
hwaddr hash;
uint32_t vsid, pgidx, ptem;
@@ -353,21 +352,22 @@ static hwaddr ppc_hash32_htab_lookup(PowerPCCPU *cpu,
qemu_log_mask(CPU_LOG_MMU, "htab_base " TARGET_FMT_plx
" htab_mask " TARGET_FMT_plx
" hash " TARGET_FMT_plx "\n",
- env->htab_base, env->htab_mask, hash);
+ ppc_hash32_hpt_base(cpu), ppc_hash32_hpt_mask(cpu), hash);
/* Primary PTEG lookup */
qemu_log_mask(CPU_LOG_MMU, "0 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx
" vsid=%" PRIx32 " ptem=%" PRIx32
" hash=" TARGET_FMT_plx "\n",
- env->htab_base, env->htab_mask, vsid, ptem, hash);
+ ppc_hash32_hpt_base(cpu), ppc_hash32_hpt_mask(cpu),
+ vsid, ptem, hash);
pteg_off = get_pteg_offset32(cpu, hash);
pte_offset = ppc_hash32_pteg_search(cpu, pteg_off, 0, ptem, pte);
if (pte_offset == -1) {
/* Secondary PTEG lookup */
qemu_log_mask(CPU_LOG_MMU, "1 htab=" TARGET_FMT_plx "/" TARGET_FMT_plx
" vsid=%" PRIx32 " api=%" PRIx32
- " hash=" TARGET_FMT_plx "\n", env->htab_base,
- env->htab_mask, vsid, ptem, ~hash);
+ " hash=" TARGET_FMT_plx "\n", ppc_hash32_hpt_base(cpu),
+ ppc_hash32_hpt_mask(cpu), vsid, ptem, ~hash);
pteg_off = get_pteg_offset32(cpu, ~hash);
pte_offset = ppc_hash32_pteg_search(cpu, pteg_off, 1, ptem, pte);
}
diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h
index 5b9fb08..2ed4888 100644
--- a/target/ppc/mmu-hash32.h
+++ b/target/ppc/mmu-hash32.h
@@ -44,6 +44,8 @@ int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr address, int rw,
/*
* Hash page table definitions
*/
+#define SDR_32_HTABORG 0xFFFF0000UL
+#define SDR_32_HTABMASK 0x000001FFUL
#define HPTES_PER_GROUP 8
#define HASH_PTE_SIZE_32 8
@@ -65,42 +67,54 @@ int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr address, int rw,
#define HPTE32_R_WIMG 0x00000078
#define HPTE32_R_PP 0x00000003
+static inline hwaddr ppc_hash32_hpt_base(PowerPCCPU *cpu)
+{
+ return cpu->env.spr[SPR_SDR1] & SDR_32_HTABORG;
+}
+
+static inline hwaddr ppc_hash32_hpt_mask(PowerPCCPU *cpu)
+{
+ return ((cpu->env.spr[SPR_SDR1] & SDR_32_HTABMASK) << 16) | 0xFFFF;
+}
+
static inline target_ulong ppc_hash32_load_hpte0(PowerPCCPU *cpu,
hwaddr pte_offset)
{
CPUPPCState *env = &cpu->env;
+ target_ulong base = ppc_hash32_hpt_base(cpu);
assert(!env->external_htab); /* Not supported on 32-bit for now */
- return ldl_phys(CPU(cpu)->as, env->htab_base + pte_offset);
+ return ldl_phys(CPU(cpu)->as, base + pte_offset);
}
static inline target_ulong ppc_hash32_load_hpte1(PowerPCCPU *cpu,
hwaddr pte_offset)
{
+ target_ulong base = ppc_hash32_hpt_base(cpu);
CPUPPCState *env = &cpu->env;
assert(!env->external_htab); /* Not supported on 32-bit for now */
- return ldl_phys(CPU(cpu)->as,
- env->htab_base + pte_offset + HASH_PTE_SIZE_32 / 2);
+ return ldl_phys(CPU(cpu)->as, base + pte_offset + HASH_PTE_SIZE_32 / 2);
}
static inline void ppc_hash32_store_hpte0(PowerPCCPU *cpu,
hwaddr pte_offset, target_ulong pte0)
{
CPUPPCState *env = &cpu->env;
+ target_ulong base = ppc_hash32_hpt_base(cpu);
assert(!env->external_htab); /* Not supported on 32-bit for now */
- stl_phys(CPU(cpu)->as, env->htab_base + pte_offset, pte0);
+ stl_phys(CPU(cpu)->as, base + pte_offset, pte0);
}
static inline void ppc_hash32_store_hpte1(PowerPCCPU *cpu,
hwaddr pte_offset, target_ulong pte1)
{
CPUPPCState *env = &cpu->env;
+ target_ulong base = ppc_hash32_hpt_base(cpu);
assert(!env->external_htab); /* Not supported on 32-bit for now */
- stl_phys(CPU(cpu)->as,
- env->htab_base + pte_offset + HASH_PTE_SIZE_32 / 2, pte1);
+ stl_phys(CPU(cpu)->as, base + pte_offset + HASH_PTE_SIZE_32 / 2, pte1);
}
typedef struct {
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index f503c69..38a6912 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -304,15 +304,13 @@ void ppc_hash64_set_sdr1(PowerPCCPU *cpu, target_ulong value,
CPUPPCState *env = &cpu->env;
target_ulong htabsize = value & SDR_64_HTABSIZE;
- env->spr[SPR_SDR1] = value;
if (htabsize > 28) {
error_setg(errp,
"Invalid HTABSIZE 0x" TARGET_FMT_lx" stored in SDR1",
htabsize);
- htabsize = 28;
+ return;
}
- env->htab_mask = (1ULL << (htabsize + 18 - 7)) - 1;
- env->htab_base = value & SDR_64_HTABORG;
+ env->spr[SPR_SDR1] = value;
}
void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void *hpt, int shift,
@@ -333,10 +331,6 @@ void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void *hpt, int shift,
return;
}
- /* Not strictly necessary, but makes it clearer that an external
- * htab is in use when debugging */
- env->htab_base = -1;
-
if (kvm_enabled()) {
if (kvmppc_put_books_sregs(cpu) < 0) {
error_setg(errp, "Unable to update SDR1 in KVM");
@@ -450,10 +444,11 @@ const ppc_hash_pte64_t *ppc_hash64_map_hptes(PowerPCCPU *cpu,
* accessible PTEG.
*/
hptes = (ppc_hash_pte64_t *)(cpu->env.external_htab + pte_offset);
- } else if (cpu->env.htab_base) {
+ } else if (ppc_hash64_hpt_base(cpu)) {
+ hwaddr base = ppc_hash64_hpt_base(cpu);
hwaddr plen = n * HASH_PTE_SIZE_64;
- hptes = address_space_map(CPU(cpu)->as, cpu->env.htab_base + pte_offset,
- &plen, false);
+ hptes = address_space_map(CPU(cpu)->as, base + pte_offset,
+ &plen, false);
if (plen < (n * HASH_PTE_SIZE_64)) {
hw_error("%s: Unable to map all requested HPTEs\n", __func__);
}
@@ -514,13 +509,12 @@ static hwaddr ppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash,
target_ulong ptem,
ppc_hash_pte64_t *pte, unsigned *pshift)
{
- CPUPPCState *env = &cpu->env;
int i;
const ppc_hash_pte64_t *pteg;
target_ulong pte0, pte1;
target_ulong ptex;
- ptex = (hash & env->htab_mask) * HPTES_PER_GROUP;
+ ptex = (hash & ppc_hash64_hpt_mask(cpu)) * HPTES_PER_GROUP;
pteg = ppc_hash64_map_hptes(cpu, ptex, HPTES_PER_GROUP);
if (!pteg) {
return -1;
@@ -598,14 +592,15 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
qemu_log_mask(CPU_LOG_MMU,
"htab_base " TARGET_FMT_plx " htab_mask " TARGET_FMT_plx
" hash " TARGET_FMT_plx "\n",
- env->htab_base, env->htab_mask, hash);
+ ppc_hash64_hpt_base(cpu), ppc_hash64_hpt_mask(cpu), hash);
/* Primary PTEG lookup */
qemu_log_mask(CPU_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);
+ ppc_hash64_hpt_base(cpu), ppc_hash64_hpt_mask(cpu),
+ vsid, ptem, hash);
ptex = ppc_hash64_pteg_search(cpu, hash, sps, ptem, pte, pshift);
if (ptex == -1) {
@@ -614,8 +609,8 @@ static hwaddr ppc_hash64_htab_lookup(PowerPCCPU *cpu,
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);
+ " hash=" TARGET_FMT_plx "\n", ppc_hash64_hpt_base(cpu),
+ ppc_hash64_hpt_mask(cpu), vsid, ptem, ~hash);
ptex = ppc_hash64_pteg_search(cpu, ~hash, sps, ptem, pte, pshift);
}
@@ -933,9 +928,9 @@ void ppc_hash64_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
stq_p(env->external_htab + offset, pte0);
stq_p(env->external_htab + offset + HASH_PTE_SIZE_64 / 2, pte1);
} else {
- stq_phys(CPU(cpu)->as, env->htab_base + offset, pte0);
- stq_phys(CPU(cpu)->as,
- env->htab_base + offset + HASH_PTE_SIZE_64 / 2, pte1);
+ hwaddr base = ppc_hash64_hpt_base(cpu);
+ stq_phys(CPU(cpu)->as, base + offset, pte0);
+ stq_phys(CPU(cpu)->as, base + offset + HASH_PTE_SIZE_64 / 2, pte1);
}
}
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index edd687b..5914a08 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -56,6 +56,9 @@ void ppc_hash64_update_rmls(CPUPPCState *env);
* Hash page table definitions
*/
+#define SDR_64_HTABORG 0xFFFFFFFFFFFC0000ULL
+#define SDR_64_HTABSIZE 0x000000000000001FULL
+
#define HPTES_PER_GROUP 8
#define HASH_PTE_SIZE_64 16
#define HASH_PTEG_SIZE_64 (HASH_PTE_SIZE_64 * HPTES_PER_GROUP)
@@ -91,6 +94,16 @@ void ppc_hash64_update_rmls(CPUPPCState *env);
#define HPTE64_V_1TB_SEG 0x4000000000000000ULL
#define HPTE64_V_VRMA_MASK 0x4001ffffff000000ULL
+static inline hwaddr ppc_hash64_hpt_base(PowerPCCPU *cpu)
+{
+ return cpu->env.spr[SPR_SDR1] & SDR_64_HTABORG;
+}
+
+static inline hwaddr ppc_hash64_hpt_mask(PowerPCCPU *cpu)
+{
+ return (1ULL << ((cpu->env.spr[SPR_SDR1] & SDR_64_HTABSIZE) + 18 - 7)) - 1;
+}
+
void ppc_hash64_set_sdr1(PowerPCCPU *cpu, target_ulong value,
Error **errp);
void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void *hpt, int shift,
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index eb2d482..1381635 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -466,6 +466,7 @@ static int get_bat_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
static inline int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
target_ulong eaddr, int rw, int type)
{
+ PowerPCCPU *cpu = ppc_env_get_cpu(env);
hwaddr hash;
target_ulong vsid;
int ds, pr, target_page_bits;
@@ -503,7 +504,7 @@ static inline int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
qemu_log_mask(CPU_LOG_MMU, "htab_base " TARGET_FMT_plx
" htab_mask " TARGET_FMT_plx
" hash " TARGET_FMT_plx "\n",
- env->htab_base, env->htab_mask, hash);
+ ppc_hash32_hpt_base(cpu), ppc_hash32_hpt_mask(cpu), hash);
ctx->hash[0] = hash;
ctx->hash[1] = ~hash;
@@ -518,9 +519,11 @@ static inline int get_segment_6xx_tlb(CPUPPCState *env, mmu_ctx_t *ctx,
uint32_t a0, a1, a2, a3;
qemu_log("Page table: " TARGET_FMT_plx " len " TARGET_FMT_plx
- "\n", env->htab_base, env->htab_mask + 0x80);
- for (curaddr = env->htab_base;
- curaddr < (env->htab_base + env->htab_mask + 0x80);
+ "\n", ppc_hash32_hpt_base(cpu),
+ ppc_hash32_hpt_mask(env) + 0x80);
+ for (curaddr = ppc_hash32_hpt_base(cpu);
+ curaddr < (ppc_hash32_hpt_base(cpu)
+ + ppc_hash32_hpt_mask(cpu) + 0x80);
curaddr += 16) {
a0 = ldl_phys(cs->as, curaddr);
a1 = ldl_phys(cs->as, curaddr + 4);
@@ -1205,12 +1208,13 @@ static void mmu6xx_dump_BATs(FILE *f, fprintf_function cpu_fprintf,
static void mmu6xx_dump_mmu(FILE *f, fprintf_function cpu_fprintf,
CPUPPCState *env)
{
+ PowerPCCPU *cpu = ppc_env_get_cpu(env);
ppc6xx_tlb_t *tlb;
target_ulong sr;
int type, way, entry, i;
- cpu_fprintf(f, "HTAB base = 0x%"HWADDR_PRIx"\n", env->htab_base);
- cpu_fprintf(f, "HTAB mask = 0x%"HWADDR_PRIx"\n", env->htab_mask);
+ cpu_fprintf(f, "HTAB base = 0x%"HWADDR_PRIx"\n", ppc_hash32_hpt_base(cpu));
+ cpu_fprintf(f, "HTAB mask = 0x%"HWADDR_PRIx"\n", ppc_hash32_hpt_mask(cpu));
cpu_fprintf(f, "\nSegment registers:\n");
for (i = 0; i < 32; i++) {
@@ -1592,9 +1596,9 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
env->spr[SPR_DCMP] = 0x80000000 | ctx.ptem;
tlb_miss:
env->error_code |= ctx.key << 19;
- env->spr[SPR_HASH1] = env->htab_base +
+ env->spr[SPR_HASH1] = ppc_hash32_hpt_base(cpu) +
get_pteg_offset32(cpu, ctx.hash[0]);
- env->spr[SPR_HASH2] = env->htab_base +
+ env->spr[SPR_HASH2] = ppc_hash32_hpt_base(cpu) +
get_pteg_offset32(cpu, ctx.hash[1]);
break;
case POWERPC_MMU_SOFT_74xx:
@@ -1999,7 +2003,6 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
{
qemu_log_mask(CPU_LOG_MMU, "%s: " TARGET_FMT_lx "\n", __func__, value);
assert(!env->external_htab);
- env->spr[SPR_SDR1] = value;
#if defined(TARGET_PPC64)
if (env->mmu_model & POWERPC_MMU_64) {
PowerPCCPU *cpu = ppc_env_get_cpu(env);
@@ -2009,14 +2012,12 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
if (local_err) {
error_report_err(local_err);
error_free(local_err);
+ return;
}
- } else
-#endif /* defined(TARGET_PPC64) */
- {
- /* FIXME: Should check for valid HTABMASK values */
- env->htab_mask = ((value & SDR_32_HTABMASK) << 16) | 0xFFFF;
- env->htab_base = value & SDR_32_HTABORG;
}
+#endif /* defined(TARGET_PPC64) */
+ /* FIXME: Should check for valid HTABMASK values in 32-bit case */
+ env->spr[SPR_SDR1] = value;
}
/* Segment registers load and store */
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCHv2 7/9] target/ppc: Manage external HPT via virtual hypervisor
2017-02-27 5:12 [Qemu-devel] [PATCHv2 0/9] Cleanups to handling of hash MMU David Gibson
` (5 preceding siblings ...)
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 6/9] target/ppc: Eliminate htab_base and htab_mask variables David Gibson
@ 2017-02-27 5:12 ` David Gibson
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 8/9] target/ppc: Remove the function ppc_hash64_set_sdr1() David Gibson
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 9/9] target/ppc: Correct SDR1 masking David Gibson
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2017-02-27 5:12 UTC (permalink / raw)
To: qemu-ppc, aik, sjitindarsingh, aneesh.kumar
Cc: qemu-devel, thuth, lvivier, agraf, mdroth, David Gibson
The pseries machine type implements the behaviour of a PAPR compliant
hypervisor, without actually executing such a hypervisor on the virtual
CPU. To do this we need some hooks in the CPU code to make hypervisor
facilities get redirected to the machine instead of emulated internally.
For hypercalls this is managed through the cpu->vhyp field, which points
to a QOM interface with a method implementing the hypercall.
For the hashed page table (HPT) - also a hypervisor resource - we use an
older hack. CPUPPCState has an 'external_htab' field which when non-NULL
indicates that the HPT is stored in qemu memory, rather than within the
guest's address space.
For consistency - and to make some future extensions easier - this merges
the external HPT mechanism into the vhyp mechanism. Methods are added
to vhyp for the basic operations the core hash MMU code needs: map_hptes()
and unmap_hptes() for reading the HPT, store_hpte() for updating it and
hpt_mask() to retrieve its size.
To match this, the pseries machine now sets these vhyp fields in its
existing vhyp class, rather than reaching into the cpu object to set the
external_htab field.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
hw/ppc/spapr.c | 60 ++++++++++++++++++++++++++++++
hw/ppc/spapr_cpu_core.c | 17 ++++++++-
hw/ppc/spapr_hcall.c | 3 +-
target/ppc/cpu.h | 10 ++++-
target/ppc/kvm.c | 2 +-
target/ppc/machine.c | 4 +-
target/ppc/mmu-hash32.h | 8 ----
target/ppc/mmu-hash64.c | 99 ++++++++++++++++---------------------------------
target/ppc/mmu-hash64.h | 7 +++-
target/ppc/mmu_helper.c | 3 +-
10 files changed, 125 insertions(+), 88 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 0b57aad..e0bb9bc 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1053,6 +1053,62 @@ static void close_htab_fd(sPAPRMachineState *spapr)
spapr->htab_fd = -1;
}
+static hwaddr spapr_hpt_mask(PPCVirtualHypervisor *vhyp)
+{
+ sPAPRMachineState *spapr = SPAPR_MACHINE(vhyp);
+
+ return HTAB_SIZE(spapr) / HASH_PTEG_SIZE_64 - 1;
+}
+
+static const ppc_hash_pte64_t *spapr_map_hptes(PPCVirtualHypervisor *vhyp,
+ hwaddr ptex, int n)
+{
+ sPAPRMachineState *spapr = SPAPR_MACHINE(vhyp);
+ hwaddr pte_offset = ptex * HASH_PTE_SIZE_64;
+
+ if (!spapr->htab) {
+ /*
+ * HTAB is controlled by KVM. Fetch into temporary buffer
+ */
+ ppc_hash_pte64_t *hptes = g_malloc(n * HASH_PTE_SIZE_64);
+ kvmppc_read_hptes(hptes, ptex, n);
+ return hptes;
+ }
+
+ /*
+ * HTAB is controlled by QEMU. Just point to the internally
+ * accessible PTEG.
+ */
+ return (const ppc_hash_pte64_t *)(spapr->htab + pte_offset);
+}
+
+static void spapr_unmap_hptes(PPCVirtualHypervisor *vhyp,
+ const ppc_hash_pte64_t *hptes,
+ hwaddr ptex, int n)
+{
+ sPAPRMachineState *spapr = SPAPR_MACHINE(vhyp);
+
+ if (!spapr->htab) {
+ g_free((void *)hptes);
+ }
+
+ /* Nothing to do for qemu managed HPT */
+}
+
+static void spapr_store_hpte(PPCVirtualHypervisor *vhyp, hwaddr ptex,
+ uint64_t pte0, uint64_t pte1)
+{
+ sPAPRMachineState *spapr = SPAPR_MACHINE(vhyp);
+ hwaddr offset = ptex * HASH_PTE_SIZE_64;
+
+ if (!spapr->htab) {
+ kvmppc_write_hpte(ptex, pte0, pte1);
+ } else {
+ stq_p(spapr->htab + offset, pte0);
+ stq_p(spapr->htab + offset + HASH_PTE_SIZE_64 / 2, pte1);
+ }
+}
+
static int spapr_hpt_shift_for_ramsize(uint64_t ramsize)
{
int shift;
@@ -2913,6 +2969,10 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
nc->nmi_monitor_handler = spapr_nmi;
smc->phb_placement = spapr_phb_placement;
vhc->hypercall = emulate_spapr_hypercall;
+ vhc->hpt_mask = spapr_hpt_mask;
+ vhc->map_hptes = spapr_map_hptes;
+ vhc->unmap_hptes = spapr_unmap_hptes;
+ vhc->store_hpte = spapr_store_hpte;
}
static const TypeInfo spapr_machine_info = {
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index 76563c4..ddb130f 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -13,10 +13,12 @@
#include "hw/boards.h"
#include "qapi/error.h"
#include "sysemu/cpus.h"
+#include "sysemu/kvm.h"
#include "target/ppc/kvm_ppc.h"
#include "hw/ppc/ppc.h"
#include "target/ppc/mmu-hash64.h"
#include "sysemu/numa.h"
+#include "qemu/error-report.h"
static void spapr_cpu_reset(void *opaque)
{
@@ -34,8 +36,19 @@ static void spapr_cpu_reset(void *opaque)
env->spr[SPR_HIOR] = 0;
- ppc_hash64_set_external_hpt(cpu, spapr->htab, spapr->htab_shift,
- &error_fatal);
+ /*
+ * This is a hack for the benefit of KVM PR - it abuses the SDR1
+ * slot in kvm_sregs to communicate the userspace address of the
+ * HPT
+ */
+ if (kvm_enabled()) {
+ env->spr[SPR_SDR1] = (target_ulong)(uintptr_t)spapr->htab
+ | (spapr->htab_shift - 18);
+ if (kvmppc_put_books_sregs(cpu) < 0) {
+ error_report("Unable to update SDR1 in KVM");
+ exit(1);
+ }
+ }
}
static void spapr_cpu_destroy(PowerPCCPU *cpu)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 85d96f6..f05a90e 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -326,7 +326,6 @@ static target_ulong h_protect(PowerPCCPU *cpu, sPAPRMachineState *spapr,
static target_ulong h_read(PowerPCCPU *cpu, sPAPRMachineState *spapr,
target_ulong opcode, target_ulong *args)
{
- CPUPPCState *env = &cpu->env;
target_ulong flags = args[0];
target_ulong ptex = args[1];
uint8_t *hpte;
@@ -342,7 +341,7 @@ static target_ulong h_read(PowerPCCPU *cpu, sPAPRMachineState *spapr,
n_entries = 4;
}
- hpte = env->external_htab + (ptex * HASH_PTE_SIZE_64);
+ hpte = spapr->htab + (ptex * HASH_PTE_SIZE_64);
for (i = 0, ridx = 0; i < n_entries; i++) {
args[ridx++] = ldq_p(hpte);
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index aaf79f5..4e7fc2c 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -999,8 +999,6 @@ struct CPUPPCState {
#endif
/* segment registers */
target_ulong sr[32];
- /* externally stored hash table */
- uint8_t *external_htab;
/* BATs */
uint32_t nb_BATs;
target_ulong DBAT[2][8];
@@ -1208,6 +1206,14 @@ struct PPCVirtualHypervisor {
struct PPCVirtualHypervisorClass {
InterfaceClass parent;
void (*hypercall)(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu);
+ hwaddr (*hpt_mask)(PPCVirtualHypervisor *vhyp);
+ const ppc_hash_pte64_t *(*map_hptes)(PPCVirtualHypervisor *vhyp,
+ hwaddr ptex, int n);
+ void (*unmap_hptes)(PPCVirtualHypervisor *vhyp,
+ const ppc_hash_pte64_t *hptes,
+ hwaddr ptex, int n);
+ void (*store_hpte)(PPCVirtualHypervisor *vhyp, hwaddr ptex,
+ uint64_t pte0, uint64_t pte1);
};
#define TYPE_PPC_VIRTUAL_HYPERVISOR "ppc-virtual-hypervisor"
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index 79c1da6..acc40ec 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -1251,7 +1251,7 @@ static int kvmppc_get_books_sregs(PowerPCCPU *cpu)
return ret;
}
- if (!env->external_htab) {
+ if (!cpu->vhyp) {
ppc_store_sdr1(env, sregs.u.s.sdr1);
}
diff --git a/target/ppc/machine.c b/target/ppc/machine.c
index 1ccbc8a..6cb3a48 100644
--- a/target/ppc/machine.c
+++ b/target/ppc/machine.c
@@ -76,7 +76,7 @@ static int cpu_load_old(QEMUFile *f, void *opaque, int version_id)
qemu_get_betls(f, &env->pb[i]);
for (i = 0; i < 1024; i++)
qemu_get_betls(f, &env->spr[i]);
- if (!env->external_htab) {
+ if (!cpu->vhyp) {
ppc_store_sdr1(env, sdr1);
}
qemu_get_be32s(f, &env->vscr);
@@ -228,7 +228,7 @@ static int cpu_post_load(void *opaque, int version_id)
env->IBAT[1][i+4] = env->spr[SPR_IBAT4U + 2*i + 1];
}
- if (!env->external_htab) {
+ if (!cpu->vhyp) {
ppc_store_sdr1(env, env->spr[SPR_SDR1]);
}
diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h
index 2ed4888..898021f 100644
--- a/target/ppc/mmu-hash32.h
+++ b/target/ppc/mmu-hash32.h
@@ -80,10 +80,8 @@ static inline hwaddr ppc_hash32_hpt_mask(PowerPCCPU *cpu)
static inline target_ulong ppc_hash32_load_hpte0(PowerPCCPU *cpu,
hwaddr pte_offset)
{
- CPUPPCState *env = &cpu->env;
target_ulong base = ppc_hash32_hpt_base(cpu);
- assert(!env->external_htab); /* Not supported on 32-bit for now */
return ldl_phys(CPU(cpu)->as, base + pte_offset);
}
@@ -91,29 +89,23 @@ static inline target_ulong ppc_hash32_load_hpte1(PowerPCCPU *cpu,
hwaddr pte_offset)
{
target_ulong base = ppc_hash32_hpt_base(cpu);
- CPUPPCState *env = &cpu->env;
- assert(!env->external_htab); /* Not supported on 32-bit for now */
return ldl_phys(CPU(cpu)->as, base + pte_offset + HASH_PTE_SIZE_32 / 2);
}
static inline void ppc_hash32_store_hpte0(PowerPCCPU *cpu,
hwaddr pte_offset, target_ulong pte0)
{
- CPUPPCState *env = &cpu->env;
target_ulong base = ppc_hash32_hpt_base(cpu);
- assert(!env->external_htab); /* Not supported on 32-bit for now */
stl_phys(CPU(cpu)->as, base + pte_offset, pte0);
}
static inline void ppc_hash32_store_hpte1(PowerPCCPU *cpu,
hwaddr pte_offset, target_ulong pte1)
{
- CPUPPCState *env = &cpu->env;
target_ulong base = ppc_hash32_hpt_base(cpu);
- assert(!env->external_htab); /* Not supported on 32-bit for now */
stl_phys(CPU(cpu)->as, base + pte_offset + HASH_PTE_SIZE_32 / 2, pte1);
}
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 38a6912..8fe0e78 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -38,12 +38,6 @@
#endif
/*
- * Used to indicate that a CPU has its hash page table (HPT) managed
- * within the host kernel
- */
-#define MMU_HASH64_KVM_MANAGED_HPT ((void *)-1)
-
-/*
* SLB handling
*/
@@ -313,31 +307,6 @@ void ppc_hash64_set_sdr1(PowerPCCPU *cpu, target_ulong value,
env->spr[SPR_SDR1] = value;
}
-void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void *hpt, int shift,
- Error **errp)
-{
- CPUPPCState *env = &cpu->env;
- Error *local_err = NULL;
-
- if (hpt) {
- env->external_htab = hpt;
- } else {
- env->external_htab = MMU_HASH64_KVM_MANAGED_HPT;
- }
- ppc_hash64_set_sdr1(cpu, (target_ulong)(uintptr_t)hpt | (shift - 18),
- &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- return;
- }
-
- if (kvm_enabled()) {
- if (kvmppc_put_books_sregs(cpu) < 0) {
- error_setg(errp, "Unable to update SDR1 in KVM");
- }
- }
-}
-
static int ppc_hash64_pte_prot(PowerPCCPU *cpu,
ppc_slb_t *slb, ppc_hash_pte64_t pte)
{
@@ -429,29 +398,24 @@ static int ppc_hash64_amr_prot(PowerPCCPU *cpu, ppc_hash_pte64_t pte)
const ppc_hash_pte64_t *ppc_hash64_map_hptes(PowerPCCPU *cpu,
hwaddr ptex, int n)
{
- ppc_hash_pte64_t *hptes = NULL;
hwaddr pte_offset = ptex * HASH_PTE_SIZE_64;
+ hwaddr base = ppc_hash64_hpt_base(cpu);
+ hwaddr plen = n * HASH_PTE_SIZE_64;
+ const ppc_hash_pte64_t *hptes;
+
+ if (cpu->vhyp) {
+ PPCVirtualHypervisorClass *vhc =
+ PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+ return vhc->map_hptes(cpu->vhyp, ptex, n);
+ }
- if (cpu->env.external_htab == MMU_HASH64_KVM_MANAGED_HPT) {
- /*
- * HTAB is controlled by KVM. Fetch into temporary buffer
- */
- hptes = g_malloc(HASH_PTEG_SIZE_64);
- kvmppc_read_hptes(hptes, ptex, n);
- } else if (cpu->env.external_htab) {
- /*
- * HTAB is controlled by QEMU. Just point to the internally
- * accessible PTEG.
- */
- hptes = (ppc_hash_pte64_t *)(cpu->env.external_htab + pte_offset);
- } else if (ppc_hash64_hpt_base(cpu)) {
- hwaddr base = ppc_hash64_hpt_base(cpu);
- hwaddr plen = n * HASH_PTE_SIZE_64;
- hptes = address_space_map(CPU(cpu)->as, base + pte_offset,
- &plen, false);
- if (plen < (n * HASH_PTE_SIZE_64)) {
- hw_error("%s: Unable to map all requested HPTEs\n", __func__);
- }
+ if (!base) {
+ return NULL;
+ }
+
+ hptes = address_space_map(CPU(cpu)->as, base + pte_offset, &plen, false);
+ if (plen < (n * HASH_PTE_SIZE_64)) {
+ hw_error("%s: Unable to map all requested HPTEs\n", __func__);
}
return hptes;
}
@@ -459,12 +423,15 @@ const ppc_hash_pte64_t *ppc_hash64_map_hptes(PowerPCCPU *cpu,
void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const ppc_hash_pte64_t *hptes,
hwaddr ptex, int n)
{
- if (cpu->env.external_htab == MMU_HASH64_KVM_MANAGED_HPT) {
- g_free((void *)hptes);
- } else if (!cpu->env.external_htab) {
- address_space_unmap(CPU(cpu)->as, (void *)hptes, n * HASH_PTE_SIZE_64,
- false, n * HASH_PTE_SIZE_64);
+ if (cpu->vhyp) {
+ PPCVirtualHypervisorClass *vhc =
+ PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+ vhc->unmap_hptes(cpu->vhyp, hptes, ptex, n);
+ return;
}
+
+ address_space_unmap(CPU(cpu)->as, (void *)hptes, n * HASH_PTE_SIZE_64,
+ false, n * HASH_PTE_SIZE_64);
}
static unsigned hpte_page_shift(const struct ppc_one_seg_page_size *sps,
@@ -916,22 +883,18 @@ hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr)
void ppc_hash64_store_hpte(PowerPCCPU *cpu, hwaddr ptex,
uint64_t pte0, uint64_t pte1)
{
- CPUPPCState *env = &cpu->env;
+ hwaddr base = ppc_hash64_hpt_base(cpu);
hwaddr offset = ptex * HASH_PTE_SIZE_64;
- if (env->external_htab == MMU_HASH64_KVM_MANAGED_HPT) {
- kvmppc_write_hpte(ptex, pte0, pte1);
+ if (cpu->vhyp) {
+ PPCVirtualHypervisorClass *vhc =
+ PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+ vhc->store_hpte(cpu->vhyp, ptex, pte0, pte1);
return;
}
- if (env->external_htab) {
- stq_p(env->external_htab + offset, pte0);
- stq_p(env->external_htab + offset + HASH_PTE_SIZE_64 / 2, pte1);
- } else {
- hwaddr base = ppc_hash64_hpt_base(cpu);
- stq_phys(CPU(cpu)->as, base + offset, pte0);
- stq_phys(CPU(cpu)->as, base + offset + HASH_PTE_SIZE_64 / 2, pte1);
- }
+ stq_phys(CPU(cpu)->as, base + offset, pte0);
+ stq_phys(CPU(cpu)->as, base + offset + HASH_PTE_SIZE_64 / 2, pte1);
}
void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu, target_ulong ptex,
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index 5914a08..c8d3458 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -101,13 +101,16 @@ static inline hwaddr ppc_hash64_hpt_base(PowerPCCPU *cpu)
static inline hwaddr ppc_hash64_hpt_mask(PowerPCCPU *cpu)
{
+ if (cpu->vhyp) {
+ PPCVirtualHypervisorClass *vhc =
+ PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+ return vhc->hpt_mask(cpu->vhyp);
+ }
return (1ULL << ((cpu->env.spr[SPR_SDR1] & SDR_64_HTABSIZE) + 18 - 7)) - 1;
}
void ppc_hash64_set_sdr1(PowerPCCPU *cpu, target_ulong value,
Error **errp);
-void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void *hpt, int shift,
- Error **errp);
struct ppc_hash_pte64 {
uint64_t pte0, pte1;
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 1381635..0176ab6 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -2001,8 +2001,9 @@ void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr)
/* Special registers manipulation */
void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
{
+ PowerPCCPU *cpu = ppc_env_get_cpu(env);
qemu_log_mask(CPU_LOG_MMU, "%s: " TARGET_FMT_lx "\n", __func__, value);
- assert(!env->external_htab);
+ assert(!cpu->vhyp);
#if defined(TARGET_PPC64)
if (env->mmu_model & POWERPC_MMU_64) {
PowerPCCPU *cpu = ppc_env_get_cpu(env);
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCHv2 8/9] target/ppc: Remove the function ppc_hash64_set_sdr1()
2017-02-27 5:12 [Qemu-devel] [PATCHv2 0/9] Cleanups to handling of hash MMU David Gibson
` (6 preceding siblings ...)
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 7/9] target/ppc: Manage external HPT via virtual hypervisor David Gibson
@ 2017-02-27 5:12 ` David Gibson
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 9/9] target/ppc: Correct SDR1 masking David Gibson
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2017-02-27 5:12 UTC (permalink / raw)
To: qemu-ppc, aik, sjitindarsingh, aneesh.kumar
Cc: qemu-devel, thuth, lvivier, agraf, mdroth, David Gibson
From: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
The function ppc_hash64_set_sdr1 basically checked the htabsize and set an
error if it was too big, otherwise it just stored the value in SPR_SDR1.
Given that the only function which calls ppc_hash64_set_sdr1() is
ppc_store_sdr1(), why not handle the checking in ppc_store_sdr1() to avoid
the extra function call. Note that ppc_store_sdr1() already stores the
value in SPR_SDR1 anyway, so we were doing it twice.
Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
[dwg: Remove unnecessary error temporary]
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
target/ppc/mmu-hash64.c | 18 ------------------
target/ppc/mmu-hash64.h | 3 ---
target/ppc/mmu_helper.c | 11 +++++------
3 files changed, 5 insertions(+), 27 deletions(-)
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 8fe0e78..d44f2bb 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -289,24 +289,6 @@ target_ulong helper_load_slb_vsid(CPUPPCState *env, target_ulong rb)
return rt;
}
-/*
- * 64-bit hash table MMU handling
- */
-void ppc_hash64_set_sdr1(PowerPCCPU *cpu, target_ulong value,
- Error **errp)
-{
- CPUPPCState *env = &cpu->env;
- target_ulong htabsize = value & SDR_64_HTABSIZE;
-
- if (htabsize > 28) {
- error_setg(errp,
- "Invalid HTABSIZE 0x" TARGET_FMT_lx" stored in SDR1",
- htabsize);
- return;
- }
- env->spr[SPR_SDR1] = value;
-}
-
static int ppc_hash64_pte_prot(PowerPCCPU *cpu,
ppc_slb_t *slb, ppc_hash_pte64_t pte)
{
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index c8d3458..9c74823 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -109,9 +109,6 @@ static inline hwaddr ppc_hash64_hpt_mask(PowerPCCPU *cpu)
return (1ULL << ((cpu->env.spr[SPR_SDR1] & SDR_64_HTABSIZE) + 18 - 7)) - 1;
}
-void ppc_hash64_set_sdr1(PowerPCCPU *cpu, target_ulong value,
- Error **errp);
-
struct ppc_hash_pte64 {
uint64_t pte0, pte1;
};
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 0176ab6..3bc8030 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -28,6 +28,7 @@
#include "exec/cpu_ldst.h"
#include "exec/log.h"
#include "helper_regs.h"
+#include "qemu/error-report.h"
//#define DEBUG_MMU
//#define DEBUG_BATS
@@ -2006,13 +2007,11 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
assert(!cpu->vhyp);
#if defined(TARGET_PPC64)
if (env->mmu_model & POWERPC_MMU_64) {
- PowerPCCPU *cpu = ppc_env_get_cpu(env);
- Error *local_err = NULL;
+ target_ulong htabsize = value & SDR_64_HTABSIZE;
- ppc_hash64_set_sdr1(cpu, value, &local_err);
- if (local_err) {
- error_report_err(local_err);
- error_free(local_err);
+ if (htabsize > 28) {
+ error_report("Invalid HTABSIZE 0x" TARGET_FMT_lx" stored in SDR1",
+ htabsize);
return;
}
}
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread* [Qemu-devel] [PATCHv2 9/9] target/ppc: Correct SDR1 masking
2017-02-27 5:12 [Qemu-devel] [PATCHv2 0/9] Cleanups to handling of hash MMU David Gibson
` (7 preceding siblings ...)
2017-02-27 5:12 ` [Qemu-devel] [PATCHv2 8/9] target/ppc: Remove the function ppc_hash64_set_sdr1() David Gibson
@ 2017-02-27 5:12 ` David Gibson
8 siblings, 0 replies; 10+ messages in thread
From: David Gibson @ 2017-02-27 5:12 UTC (permalink / raw)
To: qemu-ppc, aik, sjitindarsingh, aneesh.kumar
Cc: qemu-devel, thuth, lvivier, agraf, mdroth, David Gibson
SDR_64_HTABORG, which indicates the bits of the SDR1 register to use for
the base of a 64-bit machine's hashed page table (HPT) isn't correct. It
includes the top 46 bits of the register, but in fact the top 4 bits must
be zero (according to the ISA v2.07). No actual implementation has
supported close to 2^60 bytes of physical address space, so it's kind of
irrelevant, but we might as well correct this.
In addition, although we checked for bad size values in SDR1, we never
reported an error if entirely invalid bits were set there. Add this check
to ppc_store_sdr1().
Reported-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
target/ppc/mmu-hash64.h | 2 +-
target/ppc/mmu_helper.c | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index 9c74823..54f1e37 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -56,7 +56,7 @@ void ppc_hash64_update_rmls(CPUPPCState *env);
* Hash page table definitions
*/
-#define SDR_64_HTABORG 0xFFFFFFFFFFFC0000ULL
+#define SDR_64_HTABORG 0x0FFFFFFFFFFC0000ULL
#define SDR_64_HTABSIZE 0x000000000000001FULL
#define HPTES_PER_GROUP 8
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 3bc8030..a1af3d6 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -2007,8 +2007,14 @@ void ppc_store_sdr1(CPUPPCState *env, target_ulong value)
assert(!cpu->vhyp);
#if defined(TARGET_PPC64)
if (env->mmu_model & POWERPC_MMU_64) {
+ target_ulong sdr_mask = SDR_64_HTABORG | SDR_64_HTABSIZE;
target_ulong htabsize = value & SDR_64_HTABSIZE;
+ if (value & ~sdr_mask) {
+ error_report("Invalid bits 0x"TARGET_FMT_lx" set in SDR1",
+ value & ~sdr_mask);
+ value &= sdr_mask;
+ }
if (htabsize > 28) {
error_report("Invalid HTABSIZE 0x" TARGET_FMT_lx" stored in SDR1",
htabsize);
--
2.9.3
^ permalink raw reply related [flat|nested] 10+ messages in thread