On 11.10.2013, at 13:13, Aneesh Kumar K.V <aneesh.ku...@linux.vnet.ibm.com> wrote:
> From: "Aneesh Kumar K.V" <aneesh.ku...@linux.vnet.ibm.com> > > With kvm enabled, we store the hash page table information in the hypervisor. > Use ioctl to read the htab contents. Without this we get the below error when > trying to read the guest address > > (gdb) x/10 do_fork > 0xc000000000098660 <do_fork>: Cannot access memory at address > 0xc000000000098660 > (gdb) > > Signed-off-by: Aneesh Kumar K.V <aneesh.ku...@linux.vnet.ibm.com> > --- > Changes from V4: > * Rewrite to avoid two code paths for doing hash lookups > > hw/ppc/spapr_hcall.c | 44 ++++++++++++++++++++----------- > target-ppc/kvm.c | 47 +++++++++++++++++++++++++++++++++ > target-ppc/kvm_ppc.h | 16 +++++++++++ > target-ppc/mmu-hash64.c | 70 +++++++++++++++++++++++++++++++++++++++---------- > target-ppc/mmu-hash64.h | 33 ++++++++++++++++------- > 5 files changed, 170 insertions(+), 40 deletions(-) > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index f10ba8a..96348e3 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -52,6 +52,7 @@ static target_ulong h_enter(PowerPCCPU *cpu, > sPAPREnvironment *spapr, > target_ulong raddr; > target_ulong i; > hwaddr hpte; > + struct ppc_hash64_hpte_token *token; > > /* only handle 4k and 16M pages for now */ > if (pteh & HPTE64_V_LARGE) { > @@ -94,25 +95,32 @@ static target_ulong h_enter(PowerPCCPU *cpu, > sPAPREnvironment *spapr, > if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) { > return H_PARAMETER; > } > + > + i = 0; > + hpte = pte_index * HASH_PTE_SIZE_64; > if (likely((flags & H_EXACT) == 0)) { > pte_index &= ~7ULL; > - hpte = pte_index * HASH_PTE_SIZE_64; > - for (i = 0; ; ++i) { > + token = ppc_hash64_start_access(ppc_env_get_cpu(env), pte_index); > + do { Why convert this into a while loop? > if (i == 8) { > + ppc_hash64_stop_access(token); > return H_PTEG_FULL; > } > - if ((ppc_hash64_load_hpte0(env, hpte) & HPTE64_V_VALID) == 0) { > + if ((ppc_hash64_load_hpte0(token, i) & HPTE64_V_VALID) == 0) { > break; > } > - hpte += HASH_PTE_SIZE_64; > - } > + } while (i++); > + ppc_hash64_stop_access(token); > } else { > - i = 0; > - hpte = pte_index * HASH_PTE_SIZE_64; > - if (ppc_hash64_load_hpte0(env, hpte) & HPTE64_V_VALID) { > + token = ppc_hash64_start_access(ppc_env_get_cpu(env), pte_index); > + if (ppc_hash64_load_hpte0(token, 0) & HPTE64_V_VALID) { > + ppc_hash64_stop_access(token); > return H_PTEG_FULL; > } > + ppc_hash64_stop_access(token); > } > + hpte += i * HASH_PTE_SIZE_64; > + > ppc_hash64_store_hpte1(env, hpte, ptel); > /* eieio(); FIXME: need some sort of barrier for smp? */ > ppc_hash64_store_hpte0(env, hpte, pteh | HPTE64_V_HPTE_DIRTY); > @@ -135,15 +143,16 @@ static RemoveResult remove_hpte(CPUPPCState *env, > target_ulong ptex, > { > hwaddr hpte; > target_ulong v, r, rb; > + struct ppc_hash64_hpte_token *token; > > if ((ptex * HASH_PTE_SIZE_64) & ~env->htab_mask) { > return REMOVE_PARM; > } > > - hpte = ptex * HASH_PTE_SIZE_64; > - > - v = ppc_hash64_load_hpte0(env, hpte); > - r = ppc_hash64_load_hpte1(env, hpte); > + token = ppc_hash64_start_access(ppc_env_get_cpu(env), ptex); > + v = ppc_hash64_load_hpte0(token, 0); > + r = ppc_hash64_load_hpte1(token, 0); > + ppc_hash64_stop_access(token); > > if ((v & HPTE64_V_VALID) == 0 || > ((flags & H_AVPN) && (v & ~0x7fULL) != avpn) || > @@ -152,6 +161,7 @@ static RemoveResult remove_hpte(CPUPPCState *env, > target_ulong ptex, > } > *vp = v; > *rp = r; > + hpte = ptex * HASH_PTE_SIZE_64; > ppc_hash64_store_hpte0(env, hpte, HPTE64_V_HPTE_DIRTY); > rb = compute_tlbie_rb(v, r, ptex); > ppc_tlb_invalidate_one(env, rb); > @@ -261,15 +271,16 @@ static target_ulong h_protect(PowerPCCPU *cpu, > sPAPREnvironment *spapr, > target_ulong avpn = args[2]; > hwaddr hpte; > target_ulong v, r, rb; > + struct ppc_hash64_hpte_token *token; > > if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) { > return H_PARAMETER; > } > > - hpte = pte_index * HASH_PTE_SIZE_64; > - > - v = ppc_hash64_load_hpte0(env, hpte); > - r = ppc_hash64_load_hpte1(env, hpte); > + token = ppc_hash64_start_access(ppc_env_get_cpu(env), pte_index); > + v = ppc_hash64_load_hpte0(token, 0); > + r = ppc_hash64_load_hpte1(token, 0); > + ppc_hash64_stop_access(token); > > if ((v & HPTE64_V_VALID) == 0 || > ((flags & H_AVPN) && (v & ~0x7fULL) != avpn)) { > @@ -282,6 +293,7 @@ static target_ulong h_protect(PowerPCCPU *cpu, > sPAPREnvironment *spapr, > r |= (flags << 48) & HPTE64_R_KEY_HI; > r |= flags & (HPTE64_R_PP | HPTE64_R_N | HPTE64_R_KEY_LO); > rb = compute_tlbie_rb(v, r, pte_index); > + hpte = pte_index * HASH_PTE_SIZE_64; > ppc_hash64_store_hpte0(env, hpte, (v & ~HPTE64_V_VALID) | > HPTE64_V_HPTE_DIRTY); > ppc_tlb_invalidate_one(env, rb); > ppc_hash64_store_hpte1(env, hpte, r); > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index ac70efbf..3ad3283 100644 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -1783,6 +1783,11 @@ bool kvmppc_has_cap_epr(void) > return cap_epr; > } > > +bool kvmppc_has_cap_htab_fd(void) > +{ > + return cap_htab_fd; > +} > + > static int kvm_ppc_register_host_cpu_type(void) > { > TypeInfo type_info = { > @@ -1888,3 +1893,45 @@ int kvm_arch_on_sigbus(int code, void *addr) > void kvm_arch_init_irq_routing(KVMState *s) > { > } > + > +int kvm_ppc_hash64_start_access(PowerPCCPU *cpu, unsigned long pte_index, > + struct ppc_hash64_hpte_token *token) > +{ > + int htab_fd; > + int hpte_group_size; > + struct kvm_get_htab_fd ghf; > + struct kvm_get_htab_buf { > + struct kvm_get_htab_header header; > + /* > + * We required one extra byte for read > + */ > + unsigned long hpte[(HPTES_PER_GROUP * 2) + 1]; > + } hpte_buf;; Double semicolon? > + > + 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; > + } > + memset(&hpte_buf, 0, sizeof(hpte_buf)); > + /* > + * Read the hpte group > + */ > + if (read(htab_fd, &hpte_buf, sizeof(hpte_buf)) < 0) { > + goto out_close; > + } > + > + hpte_group_size = sizeof(unsigned long) * 2 * HPTES_PER_GROUP; > + token->hpte_group = g_malloc(hpte_group_size); > + token->free_hpte_group = 1; > + token->external = 1; > + memcpy(token->hpte_group, hpte_buf.hpte, hpte_group_size); > + close(htab_fd); > + return 0; > + > +out_close: > + close(htab_fd); > +error_out: > + return -1; > +} > diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h > index 4ae7bf2..31df330 100644 > --- a/target-ppc/kvm_ppc.h > +++ b/target-ppc/kvm_ppc.h > @@ -12,6 +12,7 @@ > #define TYPE_HOST_POWERPC_CPU "host-" TYPE_POWERPC_CPU > > void kvmppc_init(void); > +struct ppc_hash64_hpte_token; > > #ifdef CONFIG_KVM > > @@ -38,10 +39,13 @@ uint64_t kvmppc_rma_size(uint64_t current_size, unsigned > int hash_shift); > #endif /* !CONFIG_USER_ONLY */ > int kvmppc_fixup_cpu(PowerPCCPU *cpu); > bool kvmppc_has_cap_epr(void); > +bool kvmppc_has_cap_htab_fd(void); > 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); > +int kvm_ppc_hash64_start_access(PowerPCCPU *cpu, hwaddr hash, > + struct ppc_hash64_hpte_token *token); > > #else > > @@ -164,6 +168,11 @@ static inline bool kvmppc_has_cap_epr(void) > return false; > } > > +static inline bool kvmppc_has_cap_htab_fd(void) > +{ > + return false; > +} > + > static inline int kvmppc_get_htab_fd(bool write) > { > return -1; > @@ -181,6 +190,13 @@ static inline int kvmppc_load_htab_chunk(QEMUFile *f, > int fd, uint32_t index, > abort(); > } > > +static inline int kvm_ppc_hash64_start_access(PowerPCCPU *cpu, > + unsigned long pte_index, > + struct ppc_hash64_hpte_token *token) > +{ > + abort(); > +} > + > #endif > > #ifndef CONFIG_KVM > diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c > index 67fc1b5..aeb4593 100644 > --- a/target-ppc/mmu-hash64.c > +++ b/target-ppc/mmu-hash64.c > @@ -302,29 +302,73 @@ static int ppc_hash64_amr_prot(CPUPPCState *env, > ppc_hash_pte64_t pte) > return prot; > } > > -static hwaddr ppc_hash64_pteg_search(CPUPPCState *env, hwaddr pteg_off, > +struct ppc_hash64_hpte_token *ppc_hash64_start_access(PowerPCCPU *cpu, > + unsigned long > pte_index) How about you also pass in the number of PTEs you want to access? Let's call it "pte_num" for now. Then if you only care about one PTE you can indicate so, otherwise it's clear that you want to access 8 PTEs beginning from the one you're pointing at. > +{ > + hwaddr pte_offset; > + struct ppc_hash64_hpte_token *token; void *token = NULL; if (kvmppc_uses_htab_fd(cpu)) { /* HTAB is controlled by KVM. Fetch the PTEG into a new buffer. */ int hpte_group_size = sizeof(unsigned long) * 2 * pte_num; token = g_malloc(hpte_group_size); if (kvm_ppc_hash64_read_pteg(cpu, pte_index, token)) { free(token); return NULL; } } else { /* HTAB is controlled by QEMU. Just point to the internally accessible PTEG. */ hwaddr pte_offset; pte_offset = pte_index * HASH_PTE_SIZE_64; if (cpu->env.external_htab) { token = cpu->env.external_htab + pte_offset; } else { token = (uint8_t *) cpu->env.htab_base + pte_offset; } } return token; This way it's more obvious which path the "normal code flow" would be. We also only clearly choose what to do depending on in-kernel HTAB or now. As a big plus we don't need a struct that we need to dynamically allocate even in the TCG case (malloc is slow). You also want to cache kvmppc_uses_htab_fd(cpu) so it doesn't have to call an ioctl every time we try to read a PTE. I guess the best spot for it would be to put it into env. > + > + token = g_malloc(sizeof(struct ppc_hash64_hpte_token)); > + > + pte_offset = pte_index * HASH_PTE_SIZE_64; > + if (!kvmppc_has_cap_htab_fd()) { > + token->free_hpte_group = 0; > + token->external = !!cpu->env.external_htab; > + if (token->external) { > + token->hpte_group = cpu->env.external_htab + pte_offset; > + } else { > + token->hpte_group = (uint8_t *) cpu->env.htab_base + pte_offset; > + } > + return token; > + } > + > + if (!kvm_ppc_hash64_start_access(cpu, pte_index, token)) { > + return token; > + } > + free(token); > + return NULL; > +} > + > +void ppc_hash64_stop_access(struct ppc_hash64_hpte_token *token) > +{ Pass in cpu as well, then simply only do if (kvmppc_uses_htab_fd(cpu)) { g_free(token); } > + if (token->free_hpte_group) { > + free(token->hpte_group); > + } > + free(token); > + return; > +} > + > +static hwaddr ppc_hash64_pteg_search(CPUPPCState *env, hwaddr hash, > bool secondary, target_ulong ptem, > ppc_hash_pte64_t *pte) > { > - hwaddr pte_offset = pteg_off; > - target_ulong pte0, pte1; > int i; > + target_ulong pte0, pte1; > + unsigned long pte_index; > + struct ppc_hash64_hpte_token *token; > > + pte_index = (hash * HPTES_PER_GROUP) & env->htab_mask; > + token = ppc_hash64_start_access(ppc_env_get_cpu(env), pte_index); > + if (!token) { > + return -1; > + } > for (i = 0; i < HPTES_PER_GROUP; i++) { > - pte0 = ppc_hash64_load_hpte0(env, pte_offset); > - pte1 = ppc_hash64_load_hpte1(env, pte_offset); > + pte0 = ppc_hash64_load_hpte0(token, i); > + pte1 = ppc_hash64_load_hpte1(token, i); > > if ((pte0 & HPTE64_V_VALID) > && (secondary == !!(pte0 & HPTE64_V_SECONDARY)) > && HPTE64_V_COMPARE(pte0, ptem)) { > pte->pte0 = pte0; > pte->pte1 = pte1; > - return pte_offset; > + ppc_hash64_stop_access(token); > + return (pte_index + i) * HASH_PTE_SIZE_64; > } > - > - pte_offset += HASH_PTE_SIZE_64; > } > - > + ppc_hash64_stop_access(token); > + /* > + * We didn't find a valid entry. > + */ > return -1; > } > > @@ -332,7 +376,7 @@ static hwaddr ppc_hash64_htab_lookup(CPUPPCState *env, > ppc_slb_t *slb, target_ulong eaddr, > ppc_hash_pte64_t *pte) > { > - hwaddr pteg_off, pte_offset; > + hwaddr pte_offset; > hwaddr hash; > uint64_t vsid, epnshift, epnmask, epn, ptem; > > @@ -367,8 +411,7 @@ static hwaddr ppc_hash64_htab_lookup(CPUPPCState *env, > " vsid=" TARGET_FMT_lx " ptem=" TARGET_FMT_lx > " hash=" TARGET_FMT_plx "\n", > env->htab_base, env->htab_mask, vsid, ptem, hash); > - pteg_off = (hash * HASH_PTEG_SIZE_64) & env->htab_mask; > - pte_offset = ppc_hash64_pteg_search(env, pteg_off, 0, ptem, pte); > + pte_offset = ppc_hash64_pteg_search(env, hash, 0, ptem, pte); > > if (pte_offset == -1) { > /* Secondary PTEG lookup */ > @@ -377,8 +420,7 @@ static hwaddr ppc_hash64_htab_lookup(CPUPPCState *env, > " hash=" TARGET_FMT_plx "\n", env->htab_base, > env->htab_mask, vsid, ptem, ~hash); > > - pteg_off = (~hash * HASH_PTEG_SIZE_64) & env->htab_mask; > - pte_offset = ppc_hash64_pteg_search(env, pteg_off, 1, ptem, pte); > + pte_offset = ppc_hash64_pteg_search(env, ~hash, 1, ptem, pte); > } > > return pte_offset; > diff --git a/target-ppc/mmu-hash64.h b/target-ppc/mmu-hash64.h > index 55f5a23..0b44321 100644 > --- a/target-ppc/mmu-hash64.h > +++ b/target-ppc/mmu-hash64.h > @@ -75,23 +75,36 @@ int ppc_hash64_handle_mmu_fault(CPUPPCState *env, > target_ulong address, int rw, > #define HPTE64_V_1TB_SEG 0x4000000000000000ULL > #define HPTE64_V_VRMA_MASK 0x4001ffffff000000ULL > > -static inline target_ulong ppc_hash64_load_hpte0(CPUPPCState *env, > - hwaddr pte_offset) > + > +struct ppc_hash64_hpte_token { > + bool free_hpte_group; > + bool external; > + uint8_t *hpte_group; > +}; > + > +struct ppc_hash64_hpte_token *ppc_hash64_start_access(PowerPCCPU *cpu, > + unsigned long > pte_index); > +void ppc_hash64_stop_access(struct ppc_hash64_hpte_token *token); > + > +static inline target_ulong ppc_hash64_load_hpte0(struct > ppc_hash64_hpte_token *token, > + int index) If you pass in env or cpu in here as well you can do the decision whether to use ldq_p or ldq_phys based on cpu->env.external_htab. Whenever the HTAB is controlled by KVM, external_htab is true as well. Alex > { > - if (env->external_htab) { > - return ldq_p(env->external_htab + pte_offset); > + index *= HASH_PTE_SIZE_64; > + if (token->external) { > + return ldq_p(token->hpte_group + index); > } else { > - return ldq_phys(env->htab_base + pte_offset); > + return ldq_phys((uint64_t)(token->hpte_group + index)); > } > } > > -static inline target_ulong ppc_hash64_load_hpte1(CPUPPCState *env, > - hwaddr pte_offset) > +static inline target_ulong ppc_hash64_load_hpte1(struct > ppc_hash64_hpte_token *token, > + int index) > { > - if (env->external_htab) { > - return ldq_p(env->external_htab + pte_offset + HASH_PTE_SIZE_64/2); > + index = index * HASH_PTE_SIZE_64; > + if (token->external) { > + return ldq_p(token->hpte_group + index + HASH_PTE_SIZE_64/2); > } else { > - return ldq_phys(env->htab_base + pte_offset + HASH_PTE_SIZE_64/2); > + return ldq_phys((uint64_t)(token->hpte_group + index + > HASH_PTE_SIZE_64/2)); > } > } > > -- > 1.8.1.2 >