[Qemu-devel] [PATCH -V2 0/4] target-ppc: Add support for dumping guest memory using qemu gdb server

[Qemu-devel] [PATCH -V2 0/4] target-ppc: Add support for dumping guest memory using qemu gdb server

[Aneesh Kumar K.V]

This patch series implement support for dumping guest memory using qemu gdb server.

With this patch series we can now do

(gdb) x/4i htab_call_hpte_insert1
   0xc0000000000470d8 <.htab_call_hpte_insert1>:        bl      0xc0000000000470d8 <.htab_call_hpte_insert1>
   0xc0000000000470dc <.htab_call_hpte_insert1+4>:      cmpdi   r3,0
   0xc0000000000470e0 <.htab_call_hpte_insert1+8>:      bge     0xc000000000047190 <htab_pte_insert_ok>
   0xc0000000000470e4 <.htab_call_hpte_insert1+12>:     cmpdi   r3,-2
(gdb) target remote localhost:1234
Remote debugging using localhost:1234
.plpar_hcall_norets () at arch/powerpc/platforms/pseries/hvCall.S:119
119             HCALL_INST_POSTCALL_NORETS
(gdb) x/4i htab_call_hpte_insert1
   0xc0000000000470d8 <.htab_call_hpte_insert1>:        bl      0xc00000000005f8f0 <pSeries_lpar_hpte_insert>
   0xc0000000000470dc <.htab_call_hpte_insert1+4>:      cmpdi   r3,0
   0xc0000000000470e0 <.htab_call_hpte_insert1+8>:      bge     0xc000000000047190 <htab_pte_insert_ok>
   0xc0000000000470e4 <.htab_call_hpte_insert1+12>:     cmpdi   r3,-2
(gdb)

NOTE: We still don't support inserting breakpoints.

Before Fix:
(qemu) memsave 0xc0000000000470d8 10 memdump
Invalid parameter 'addr'
(qemu)

After fix:

(qemu)  memsave 0xc0000000000470d8 10 memdump
(qemu)

[Qemu-devel] [PATCH -V2 1/4] target-ppc: Update slb array with correct index values.

[Aneesh Kumar K.V]

From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>

Without this, a value of rb=0 and rs=0 results in replacing the 0th
index. This can be observed when using gdb remote debugging support.

(gdb) x/10i do_fork
   0xc000000000085330 <do_fork>:        Cannot access memory at address 0xc000000000085330
(gdb)

This is because when we do the slb sync via kvm_cpu_synchronize_state,
we overwrite the slb entry (0th entry) for 0xc000000000085330

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 target-ppc/kvm.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 30a870e..a629447 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1034,8 +1034,18 @@ int kvm_arch_get_registers(CPUState *cs)
         /* Sync SLB */
 #ifdef TARGET_PPC64
         for (i = 0; i < 64; i++) {
-            ppc_store_slb(env, sregs.u.s.ppc64.slb[i].slbe,
-                               sregs.u.s.ppc64.slb[i].slbv);
+            target_ulong rb = sregs.u.s.ppc64.slb[i].slbe;
+            /*
+             * KVM_GET_SREGS doesn't retun slb entry with slot information
+             * same as index. So don't depend on the slot information in
+             * the returned value.
+             */
+            rb &= ~0xfff;
+            /*
+             * use the array index as the slot
+             */
+            rb |= i;
+            ppc_store_slb(env, rb, sregs.u.s.ppc64.slb[i].slbv);
         }
 #endif
 
-- 
1.8.1.2

Re: [Qemu-devel] [PATCH -V2 1/4] target-ppc: Update slb array with correct index values.

[Andreas Färber]

Am 20.08.2013 12:49, schrieb Aneesh Kumar K.V:
> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
> 
> Without this, a value of rb=0 and rs=0 results in replacing the 0th
> index. This can be observed when using gdb remote debugging support.
> 
> (gdb) x/10i do_fork
>    0xc000000000085330 <do_fork>:        Cannot access memory at address 0xc000000000085330
> (gdb)
> 
> This is because when we do the slb sync via kvm_cpu_synchronize_state,
> we overwrite the slb entry (0th entry) for 0xc000000000085330
> 
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
> ---
>  target-ppc/kvm.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 30a870e..a629447 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1034,8 +1034,18 @@ int kvm_arch_get_registers(CPUState *cs)
>          /* Sync SLB */
>  #ifdef TARGET_PPC64
>          for (i = 0; i < 64; i++) {
> -            ppc_store_slb(env, sregs.u.s.ppc64.slb[i].slbe,
> -                               sregs.u.s.ppc64.slb[i].slbv);
> +            target_ulong rb = sregs.u.s.ppc64.slb[i].slbe;
> +            /*
> +             * KVM_GET_SREGS doesn't retun slb entry with slot information

Still has the "return" typo - maybe Alex can fix if he applies it.

Andreas

> +             * same as index. So don't depend on the slot information in
> +             * the returned value.
> +             */
> +            rb &= ~0xfff;
> +            /*
> +             * use the array index as the slot
> +             */
> +            rb |= i;
> +            ppc_store_slb(env, rb, sregs.u.s.ppc64.slb[i].slbv);
>          }
>  #endif
>  
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH -V2 1/4] target-ppc: Update slb array with correct index values.

[Alexander Graf]

Am 22.08.2013 um 18:58 schrieb Andreas Färber <afaerber@suse.de>:

> Am 20.08.2013 12:49, schrieb Aneesh Kumar K.V:
>> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>
>> 
>> Without this, a value of rb=0 and rs=0 results in replacing the 0th
>> index. This can be observed when using gdb remote debugging support.
>> 
>> (gdb) x/10i do_fork
>>   0xc000000000085330 <do_fork>:        Cannot access memory at address 0xc000000000085330
>> (gdb)
>> 
>> This is because when we do the slb sync via kvm_cpu_synchronize_state,
>> we overwrite the slb entry (0th entry) for 0xc000000000085330
>> 
>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
>> ---
>> target-ppc/kvm.c | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>> 
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index 30a870e..a629447 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -1034,8 +1034,18 @@ int kvm_arch_get_registers(CPUState *cs)
>>         /* Sync SLB */
>> #ifdef TARGET_PPC64
>>         for (i = 0; i < 64; i++) {
>> -            ppc_store_slb(env, sregs.u.s.ppc64.slb[i].slbe,
>> -                               sregs.u.s.ppc64.slb[i].slbv);
>> +            target_ulong rb = sregs.u.s.ppc64.slb[i].slbe;
>> +            /*
>> +             * KVM_GET_SREGS doesn't retun slb entry with slot information
> 
> Still has the "return" typo - maybe Alex can fix if he applies it.

No, this patch is wrong. I merely picked that one trivial patch for now :)

Alex

> 
> Andreas
> 
>> +             * same as index. So don't depend on the slot information in
>> +             * the returned value.
>> +             */
>> +            rb &= ~0xfff;
>> +            /*
>> +             * use the array index as the slot
>> +             */
>> +            rb |= i;
>> +            ppc_store_slb(env, rb, sregs.u.s.ppc64.slb[i].slbv);
>>         }
>> #endif
> 
> 
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH -V2 2/4] target-ppc: Use #define instead of opencoding SLB valid bit

[Aneesh Kumar K.V]

From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>

Use SLB_ESID_V instead of (1 << 27) in the code

Reviewed-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 target-ppc/mmu_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
index 5dd4e05..9c9132e 100644
--- a/target-ppc/mmu_helper.c
+++ b/target-ppc/mmu_helper.c
@@ -2061,7 +2061,7 @@ void helper_store_sr(CPUPPCState *env, target_ulong srnum, target_ulong value)
         /* ESID = srnum */
         rb |= ((uint32_t)srnum & 0xf) << 28;
         /* Set the valid bit */
-        rb |= 1 << 27;
+        rb |= SLB_ESID_V;
         /* Index = ESID */
         rb |= (uint32_t)srnum;
 
-- 
1.8.1.2

Re: [Qemu-devel] [PATCH -V2 2/4] target-ppc: Use #define instead of opencoding SLB valid bit

[Alexander Graf]

On 20.08.2013, at 11:49, Aneesh Kumar K.V wrote:

> From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>

Please fix your patch generation ;)

> 
> Use SLB_ESID_V instead of (1 << 27) in the code
> 
> Reviewed-by: Andreas Färber <afaerber@suse.de>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>

Thanks, applied to ppc-next.


Alex

> ---
> target-ppc/mmu_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
> index 5dd4e05..9c9132e 100644
> --- a/target-ppc/mmu_helper.c
> +++ b/target-ppc/mmu_helper.c
> @@ -2061,7 +2061,7 @@ void helper_store_sr(CPUPPCState *env, target_ulong srnum, target_ulong value)
>         /* ESID = srnum */
>         rb |= ((uint32_t)srnum & 0xf) << 28;
>         /* Set the valid bit */
> -        rb |= 1 << 27;
> +        rb |= SLB_ESID_V;
>         /* Index = ESID */
>         rb |= (uint32_t)srnum;
> 
> -- 
> 1.8.1.2
> 

[Qemu-devel] [PATCH -V2 3/4] target-ppc: Fix page table lookup with kvm enabled

[Aneesh Kumar K.V]

From: "Aneesh Kumar K.V" <aneesh.kumar@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.kumar@linux.vnet.ibm.com>
---
 target-ppc/kvm.c        | 45 +++++++++++++++++++++++++++++++++++++++++++++
 target-ppc/kvm_ppc.h    |  9 ++++++++-
 target-ppc/mmu-hash64.c | 25 ++++++++++++++++---------
 3 files changed, 69 insertions(+), 10 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index a629447..b590808 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1885,3 +1885,48 @@ int kvm_arch_on_sigbus(int code, void *addr)
 void kvm_arch_init_irq_routing(KVMState *s)
 {
 }
+
+int kvmppc_hash64_load_hpte(PowerPCCPU *cpu, uint64_t index,
+                            target_ulong *hpte0, target_ulong *hpte1)
+{
+    int htab_fd;
+    struct kvm_get_htab_fd ghf;
+    struct kvm_get_htab_buf {
+        struct kvm_get_htab_header header;
+        /*
+         * Older kernel required one extra byte.
+         */
+        unsigned long hpte[3];
+    } hpte_buf;
+
+    *hpte0 = 0;
+    *hpte1 = 0;
+    if (!cap_htab_fd) {
+        return 0;
+    }
+    /*
+     * At this point we are only interested in reading only bolted entries
+     */
+    ghf.flags = KVM_GET_HTAB_BOLTED_ONLY;
+    ghf.start_index = index;
+    htab_fd = kvm_vm_ioctl(kvm_state, KVM_PPC_GET_HTAB_FD, &ghf);
+    if (htab_fd < 0) {
+        return htab_fd;
+    }
+
+    if (read(htab_fd, &hpte_buf, sizeof(hpte_buf)) < 0) {
+        goto out;
+    }
+    /*
+     * We only requested for one entry, So we should get only 1
+     * valid entry at the same index
+     */
+    if (hpte_buf.header.n_valid != 1 || hpte_buf.header.index != index) {
+        goto out;
+    }
+    *hpte0 = hpte_buf.hpte[0];
+    *hpte1 = hpte_buf.hpte[1];
+out:
+    close(htab_fd);
+    return 0;
+}
diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
index 4ae7bf2..e25373a 100644
--- a/target-ppc/kvm_ppc.h
+++ b/target-ppc/kvm_ppc.h
@@ -42,7 +42,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);
-
+int kvmppc_hash64_load_hpte(PowerPCCPU *cpu, uint64_t index,
+                            target_ulong *hpte0, target_ulong *hpte1);
 #else
 
 static inline uint32_t kvmppc_get_tbfreq(void)
@@ -181,6 +182,12 @@ static inline int kvmppc_load_htab_chunk(QEMUFile *f, int fd, uint32_t index,
     abort();
 }
 
+static inline int kvmppc_hash64_load_hpte(PowerPCCPU *cpu, uint64_t index,
+                                          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..4d8120c 100644
--- a/target-ppc/mmu-hash64.c
+++ b/target-ppc/mmu-hash64.c
@@ -302,17 +302,26 @@ 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;
+    uint64_t index;
+    hwaddr pte_offset;
     target_ulong pte0, pte1;
     int i;
 
+    pte_offset = (hash * HASH_PTEG_SIZE_64) & env->htab_mask;;
+    index = (hash * HPTES_PER_GROUP) & 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 (kvm_enabled()) {
+            index += i;
+            kvmppc_hash64_load_hpte(ppc_env_get_cpu(env), index, &pte0, &pte1);
+        } else {
+            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))
@@ -332,7 +341,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 +376,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 +385,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;
-- 
1.8.1.2

[Qemu-devel] [PATCH -V2 4/4] target-ppc: Check for error on address translation in memsave command

[Aneesh Kumar K.V]

From: "Aneesh Kumar K.V" <aneesh.kumar@linux.vnet.ibm.com>

When we translate the virtual address to physical check for error.

Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
 cpus.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index 0f65e76..658366d 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1309,7 +1309,10 @@ void qmp_memsave(int64_t addr, int64_t size, const char *filename,
         l = sizeof(buf);
         if (l > size)
             l = size;
-        cpu_memory_rw_debug(cpu, addr, buf, l, 0);
+        if (cpu_memory_rw_debug(cpu, addr, buf, l, 0) != 0) {
+            error_set(errp, QERR_INVALID_PARAMETER, "addr");
+            goto exit;
+        }
         if (fwrite(buf, 1, l, f) != l) {
             error_set(errp, QERR_IO_ERROR);
             goto exit;
-- 
1.8.1.2