From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42477) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VQhbr-0007Fs-HI for qemu-devel@nongnu.org; Mon, 30 Sep 2013 13:50:22 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VQhbk-0005SK-7A for qemu-devel@nongnu.org; Mon, 30 Sep 2013 13:50:15 -0400 Message-ID: <5249B9CD.2000401@suse.de> Date: Mon, 30 Sep 2013 19:50:05 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1378369007-958-1-git-send-email-aneesh.kumar@linux.vnet.ibm.com> <1378369007-958-3-git-send-email-aneesh.kumar@linux.vnet.ibm.com> In-Reply-To: <1378369007-958-3-git-send-email-aneesh.kumar@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH -V4 2/4] target-ppc: Fix page table lookup with kvm enabled List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Aneesh Kumar K.V" Cc: qemu-ppc@nongnu.org, paulus@samba.org, "qemu-devel@nongnu.org" On 09/05/2013 10:16 AM, Aneesh Kumar K.V wrote: > From: "Aneesh Kumar K.V" > > 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: Cannot access memory at address 0xc000000000098660 > (gdb) > > Signed-off-by: Aneesh Kumar K.V > --- > target-ppc/kvm.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++ > target-ppc/kvm_ppc.h | 12 +++++++++- > target-ppc/mmu-hash64.c | 57 ++++++++++++++++++++++++++++------------------- > 3 files changed, 104 insertions(+), 24 deletions(-) > > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c > index 1838465..05b066c 100644 > --- a/target-ppc/kvm.c > +++ b/target-ppc/kvm.c > @@ -1888,3 +1888,62 @@ int kvm_arch_on_sigbus(int code, void *addr) > void kvm_arch_init_irq_routing(KVMState *s) > { > } > + > +hwaddr kvmppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash, > + bool secondary, target_ulong ptem, > + target_ulong *hpte0, target_ulong *hpte1) > +{ > + int htab_fd; > + uint64_t index; > + hwaddr pte_offset; > + target_ulong pte0, pte1; > + struct kvm_get_htab_fd ghf; > + struct kvm_get_htab_buf { > + struct kvm_get_htab_header header; > + /* > + * Older kernel required one extra byte. Older than what? > + */ > + unsigned long hpte[(HPTES_PER_GROUP * 2) + 1]; > + } hpte_buf; > + > + index = (hash * HPTES_PER_GROUP)& cpu->env.htab_mask; > + *hpte0 = 0; > + *hpte1 = 0; > + if (!cap_htab_fd) { > + return 0; > + } > + > + ghf.flags = 0; > + ghf.start_index = index; > + htab_fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD,&ghf); > + if (htab_fd< 0) { > + goto error_out; > + } > + /* > + * Read the hpte group > + */ > + if (read(htab_fd,&hpte_buf, sizeof(hpte_buf))< 0) { > + goto out; > + } > + > + index = 0; > + pte_offset = (hash * HASH_PTEG_SIZE_64)& cpu->env.htab_mask;; > + while (index< hpte_buf.header.n_valid) { > + pte0 = hpte_buf.hpte[(index * 2)]; > + pte1 = hpte_buf.hpte[(index * 2) + 1]; > + if ((pte0& HPTE64_V_VALID) > +&& (secondary == !!(pte0& HPTE64_V_SECONDARY)) > +&& HPTE64_V_COMPARE(pte0, ptem)) { > + *hpte0 = pte0; > + *hpte1 = pte1; > + close(htab_fd); > + return pte_offset; > + } > + index++; > + pte_offset += HASH_PTE_SIZE_64; > + } > +out: > + close(htab_fd); > +error_out: > + return -1; > +} > diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h > index 4ae7bf2..dad0e57 100644 > --- a/target-ppc/kvm_ppc.h > +++ b/target-ppc/kvm_ppc.h > @@ -42,7 +42,9 @@ 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); > - > +hwaddr kvmppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash, > + bool secondary, target_ulong ptem, > + target_ulong *hpte0, target_ulong *hpte1); > #else > > static inline uint32_t kvmppc_get_tbfreq(void) > @@ -181,6 +183,14 @@ static inline int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index, > abort(); > } > > +static inline hwaddr kvmppc_hash64_pteg_search(PowerPCCPU *cpu, hwaddr hash, > + bool secondary, > + target_ulong ptem, > + target_ulong *hpte0, > + target_ulong *hpte1) > +{ > + abort(); > +} > #endif > > #ifndef CONFIG_KVM > diff --git a/target-ppc/mmu-hash64.c b/target-ppc/mmu-hash64.c > index 67fc1b5..2288fe8 100644 > --- a/target-ppc/mmu-hash64.c > +++ b/target-ppc/mmu-hash64.c > @@ -302,37 +302,50 @@ 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, > +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; > + hwaddr pte_offset; > target_ulong pte0, pte1; > - int i; > - > - for (i = 0; i< HPTES_PER_GROUP; i++) { > - pte0 = ppc_hash64_load_hpte0(env, pte_offset); > - pte1 = ppc_hash64_load_hpte1(env, pte_offset); > - > - if ((pte0& HPTE64_V_VALID) > -&& (secondary == !!(pte0& HPTE64_V_SECONDARY)) > -&& HPTE64_V_COMPARE(pte0, ptem)) { > - pte->pte0 = pte0; > - pte->pte1 = pte1; > - return pte_offset; > + int i, ret = 0; > + > + if (kvm_enabled()) { > + ret = kvmppc_hash64_pteg_search(ppc_env_get_cpu(env), hash, > + secondary, ptem, > +&pte->pte0,&pte->pte1); Instead of duplicating the search, couldn't you just hook yourself into ppc_hash64_load_hpte0/1 and return the respective ptes from there? Just cache the current pteg to ensure things don't become dog slow. Alex > + } > + /* > + * We don't support htab fd, check whether we have a copy of htab > + */ > + if (!ret) { > + pte_offset = (hash * HASH_PTEG_SIZE_64)& env->htab_mask;; > + for (i = 0; i< HPTES_PER_GROUP; i++) { > + pte0 = ppc_hash64_load_hpte0(env, pte_offset); > + pte1 = ppc_hash64_load_hpte1(env, pte_offset); > + > + if ((pte0& HPTE64_V_VALID) > +&& (secondary == !!(pte0& HPTE64_V_SECONDARY)) > +&& HPTE64_V_COMPARE(pte0, ptem)) { > + pte->pte0 = pte0; > + pte->pte1 = pte1; > + return pte_offset; > + } > + pte_offset += HASH_PTE_SIZE_64; > } > - > - pte_offset += HASH_PTE_SIZE_64; > + /* > + * We didn't find a valid entry. > + */ > + ret = -1; > } > - > - return -1; > + return ret; > } > > 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 +380,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 +389,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;