[Qemu-devel] [PATCH 0/5 v5] ppc: Add debug stub support

[Qemu-devel] [PATCH 0/5 v5] ppc: Add debug stub support

[Bharat Bhushan]

This patchset add support for
 - software breakpoint
 - h/w breakpoint
 - h/w watchpoint

Please find description in individual patch.

v4->v5
 - cleanup in kvmppc_hw_debug_points_init()
 - replaced assert in kvm_arch_insert_hw_breakpoint() with return.
   This allows gdb to through warning "You may have requested too many
   hardware breakpoints/watchpoints.". Now user can remove extra breakpint
   and continue.

v3->v4
 - remove hardcoding for size of instruction in software breakpoint
 - remove unnecessary assert in debug handler
 - Corrected assert comparison in insert_breakpoint()

v2->v3
 - Sharing of code for book3s support (which may come in future)
 - Initializing number of hw breakpoint/watchpoints from KVM world
 - Other minor cleanup/fixes

v1->v2:
 - use kvm_get_one_reg() for getting trap instruction
 - factored out e500 specific code based on exception model POWERPC_EXCP_BOOKE.
 - Not supporting ppc440


Bharat Bhushan (5):
  ppc: debug stub: Get trap instruction opcode from KVM
  ppc: Add interface to inject interrupt to guest
  ppc: Add debug interrupt injection handler
  ppc: Add software breakpoint support
  ppc: Add hw breakpoint watchpoint support

 target-ppc/kvm.c | 384 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 356 insertions(+), 28 deletions(-)

-- 
1.9.0

[Qemu-devel] [PATCH 5/5 v5] ppc: Add hw breakpoint watchpoint support

[Bharat Bhushan]

This patch adds hardware breakpoint and hardware watchpoint support
for ppc. If the debug interrupt is not handled then this is injected
to guest.

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v4->v5
 - cleanup in kvmppc_hw_debug_points_init()
 - replaced assert in kvm_arch_insert_hw_breakpoint() with return.
   This allows gdb to through warning "You may have requested too many
   hardware breakpoints/watchpoints.". Now user can remove extra breakpint
   and continue.

 target-ppc/kvm.c | 238 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 223 insertions(+), 15 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 3195491..3347c1b 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -38,6 +38,7 @@
 #include "hw/ppc/ppc.h"
 #include "sysemu/watchdog.h"
 #include "trace.h"
+#include "exec/gdbstub.h"
 
 //#define DEBUG_KVM
 
@@ -410,6 +411,38 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu)
     return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
 }
 
+/* e500 supports 2 h/w breakpoint and 2 watchpoint.
+ * book3s supports only 1 watchpoint, so array size
+ * of 4 is sufficient for now.
+ */
+#define MAX_HW_BKPTS 4
+
+static struct HWBreakpoint {
+    target_ulong addr;
+    int type;
+} hw_debug_points[MAX_HW_BKPTS];
+
+static CPUWatchpoint hw_watchpoint;
+
+/* Default there is no breakpoint and watchpoint supported */
+static int max_hw_breakpoint;
+static int max_hw_watchpoint;
+static int nb_hw_breakpoint;
+static int nb_hw_watchpoint;
+
+static void kvmppc_hw_debug_points_init(CPUPPCState *cenv)
+{
+    if (cenv->excp_model == POWERPC_EXCP_BOOKE) {
+        max_hw_breakpoint = 2;
+        max_hw_watchpoint = 2;
+    }
+
+    if ((max_hw_breakpoint + max_hw_watchpoint) > MAX_HW_BKPTS) {
+        fprintf(stderr, "Error initializing h/w breakpoints\n");
+        return;
+    }
+}
+
 int kvm_arch_init_vcpu(CPUState *cs)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
@@ -437,6 +470,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
     }
 
     kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode);
+    kvmppc_hw_debug_points_init(cenv);
 
     return ret;
 }
@@ -1345,24 +1379,212 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
     return 0;
 }
 
+static int find_hw_breakpoint(target_ulong addr, int type)
+{
+    int n;
+
+    assert((nb_hw_breakpoint + nb_hw_watchpoint)
+           <= ARRAY_SIZE(hw_debug_points));
+
+    for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) {
+        if (hw_debug_points[n].addr == addr && hw_debug_points[n].type == type) {
+            return n;
+        }
+    }
+
+    return -1;
+}
+
+static int find_hw_watchpoint(target_ulong addr, int *flag)
+{
+    int n;
+
+    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_ACCESS);
+    if (n >= 0) {
+        *flag = BP_MEM_ACCESS;
+        return n;
+    }
+
+    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_WRITE);
+    if (n >= 0) {
+        *flag = BP_MEM_WRITE;
+        return n;
+    }
+
+    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_READ);
+    if (n >= 0) {
+        *flag = BP_MEM_READ;
+        return n;
+    }
+
+    return -1;
+}
+
+int kvm_arch_insert_hw_breakpoint(target_ulong addr,
+                                  target_ulong len, int type)
+{
+    if ((nb_hw_breakpoint + nb_hw_watchpoint) >= ARRAY_SIZE(hw_debug_points))
+	return -ENOBUFS;
+
+    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].addr = addr;
+    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].type = type;
+
+    switch (type) {
+    case GDB_BREAKPOINT_HW:
+        if (nb_hw_breakpoint >= max_hw_breakpoint) {
+            return -ENOBUFS;
+        }
+
+        if (find_hw_breakpoint(addr, type) >= 0) {
+            return -EEXIST;
+        }
+
+        nb_hw_breakpoint++;
+        break;
+
+    case GDB_WATCHPOINT_WRITE:
+    case GDB_WATCHPOINT_READ:
+    case GDB_WATCHPOINT_ACCESS:
+        if (nb_hw_watchpoint >= max_hw_watchpoint) {
+            return -ENOBUFS;
+        }
+
+        if (find_hw_breakpoint(addr, type) >= 0) {
+            return -EEXIST;
+        }
+
+        nb_hw_watchpoint++;
+        break;
+
+    default:
+        return -ENOSYS;
+    }
+
+    return 0;
+}
+
+int kvm_arch_remove_hw_breakpoint(target_ulong addr,
+                                  target_ulong len, int type)
+{
+    int n;
+
+    n = find_hw_breakpoint(addr, type);
+    if (n < 0) {
+        return -ENOENT;
+    }
+
+    switch (type) {
+    case GDB_BREAKPOINT_HW:
+        nb_hw_breakpoint--;
+        break;
+
+    case GDB_WATCHPOINT_WRITE:
+    case GDB_WATCHPOINT_READ:
+    case GDB_WATCHPOINT_ACCESS:
+        nb_hw_watchpoint--;
+        break;
+
+    default:
+        return -ENOSYS;
+    }
+    hw_debug_points[n] = hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint];
+
+    return 0;
+}
+
+void kvm_arch_remove_all_hw_breakpoints(void)
+{
+    nb_hw_breakpoint = nb_hw_watchpoint = 0;
+}
+
 void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
 {
+    int n;
+
     /* Software Breakpoint updates */
     if (kvm_sw_breakpoints_active(cs)) {
         dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
     }
+
+    assert((nb_hw_breakpoint + nb_hw_watchpoint)
+           <= ARRAY_SIZE(hw_debug_points));
+    assert((nb_hw_breakpoint + nb_hw_watchpoint) <= ARRAY_SIZE(dbg->arch.bp));
+
+    if (nb_hw_breakpoint + nb_hw_watchpoint > 0) {
+        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP;
+        memset(dbg->arch.bp, 0, sizeof(dbg->arch.bp));
+        for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) {
+            switch (hw_debug_points[n].type) {
+            case GDB_BREAKPOINT_HW:
+                dbg->arch.bp[n].type = KVMPPC_DEBUG_BREAKPOINT;
+                break;
+            case GDB_WATCHPOINT_WRITE:
+                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE;
+                break;
+            case GDB_WATCHPOINT_READ:
+                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_READ;
+                break;
+            case GDB_WATCHPOINT_ACCESS:
+                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE |
+                                        KVMPPC_DEBUG_WATCH_READ;
+                break;
+            default:
+                cpu_abort(cs, "Unsupported breakpoint type\n");
+            }
+            dbg->arch.bp[n].addr = hw_debug_points[n].addr;
+        }
+    }
+}
+
+static void kvm_e500_handle_debug(CPUState *cs, int handle)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+
+    env->spr[SPR_BOOKE_DBSR] = 0;
 }
 
 static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run)
 {
     CPUState *cs = CPU(cpu);
+    CPUPPCState *env = &cpu->env;
     struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
     int handle = 0;
+    int n;
+    int flag = 0;
 
-    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
+    if (cs->singlestep_enabled) {
+        handle = 1;
+    } else if (arch_info->status) {
+        if (nb_hw_breakpoint + nb_hw_watchpoint > 0) {
+            if (arch_info->status & KVMPPC_DEBUG_BREAKPOINT) {
+                n = find_hw_breakpoint(arch_info->address, GDB_BREAKPOINT_HW);
+                if (n >= 0) {
+                    handle = 1;
+                }
+            } else if (arch_info->status & (KVMPPC_DEBUG_WATCH_READ |
+                                            KVMPPC_DEBUG_WATCH_WRITE)) {
+                n = find_hw_watchpoint(arch_info->address,  &flag);
+                if (n >= 0) {
+                    handle = 1;
+                    cs->watchpoint_hit = &hw_watchpoint;
+                    hw_watchpoint.vaddr = hw_debug_points[n].addr;
+                    hw_watchpoint.flags = flag;
+                }
+            }
+        }
+    } else if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
         handle = 1;
     }
 
+    if (handle) {
+        if (env->excp_model == POWERPC_EXCP_BOOKE) {
+            kvm_e500_handle_debug(cs, handle);
+        }
+    } else {
+       /* inject debug exception into guest */
+       env->pending_interrupts |=  1 << PPC_INTERRUPT_DEBUG;
+    }
     return handle;
 }
 
@@ -2103,20 +2325,6 @@ void kvm_arch_init_irq_routing(KVMState *s)
 {
 }
 
-int kvm_arch_insert_hw_breakpoint(target_ulong addr, target_ulong len, int type)
-{
-    return -EINVAL;
-}
-
-int kvm_arch_remove_hw_breakpoint(target_ulong addr, target_ulong len, int type)
-{
-    return -EINVAL;
-}
-
-void kvm_arch_remove_all_hw_breakpoints(void)
-{
-}
-
 struct kvm_get_htab_buf {
     struct kvm_get_htab_header header;
     /*
-- 
1.9.0

Re: [Qemu-devel] [PATCH 5/5 v5] ppc: Add hw breakpoint watchpoint support

[Alexander Graf]

On 26.06.14 09:23, Bharat Bhushan wrote:
> This patch adds hardware breakpoint and hardware watchpoint support
> for ppc. If the debug interrupt is not handled then this is injected
> to guest.
>
> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> ---
> v4->v5
>   - cleanup in kvmppc_hw_debug_points_init()
>   - replaced assert in kvm_arch_insert_hw_breakpoint() with return.
>     This allows gdb to through warning "You may have requested too many
>     hardware breakpoints/watchpoints.". Now user can remove extra breakpint
>     and continue.
>
>   target-ppc/kvm.c | 238 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 223 insertions(+), 15 deletions(-)
>
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 3195491..3347c1b 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -38,6 +38,7 @@
>   #include "hw/ppc/ppc.h"
>   #include "sysemu/watchdog.h"
>   #include "trace.h"
> +#include "exec/gdbstub.h"
>   
>   //#define DEBUG_KVM
>   
> @@ -410,6 +411,38 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu)
>       return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>   }
>   
> +/* e500 supports 2 h/w breakpoint and 2 watchpoint.
> + * book3s supports only 1 watchpoint, so array size
> + * of 4 is sufficient for now.
> + */
> +#define MAX_HW_BKPTS 4
> +
> +static struct HWBreakpoint {
> +    target_ulong addr;
> +    int type;
> +} hw_debug_points[MAX_HW_BKPTS];
> +
> +static CPUWatchpoint hw_watchpoint;
> +
> +/* Default there is no breakpoint and watchpoint supported */
> +static int max_hw_breakpoint;
> +static int max_hw_watchpoint;
> +static int nb_hw_breakpoint;
> +static int nb_hw_watchpoint;
> +
> +static void kvmppc_hw_debug_points_init(CPUPPCState *cenv)
> +{
> +    if (cenv->excp_model == POWERPC_EXCP_BOOKE) {
> +        max_hw_breakpoint = 2;
> +        max_hw_watchpoint = 2;
> +    }
> +
> +    if ((max_hw_breakpoint + max_hw_watchpoint) > MAX_HW_BKPTS) {
> +        fprintf(stderr, "Error initializing h/w breakpoints\n");
> +        return;
> +    }
> +}
> +
>   int kvm_arch_init_vcpu(CPUState *cs)
>   {
>       PowerPCCPU *cpu = POWERPC_CPU(cs);
> @@ -437,6 +470,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>       }
>   
>       kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode);
> +    kvmppc_hw_debug_points_init(cenv);
>   
>       return ret;
>   }
> @@ -1345,24 +1379,212 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
>       return 0;
>   }
>   
> +static int find_hw_breakpoint(target_ulong addr, int type)
> +{
> +    int n;
> +
> +    assert((nb_hw_breakpoint + nb_hw_watchpoint)
> +           <= ARRAY_SIZE(hw_debug_points));
> +
> +    for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) {
> +        if (hw_debug_points[n].addr == addr && hw_debug_points[n].type == type) {
> +            return n;
> +        }
> +    }
> +
> +    return -1;
> +}
> +
> +static int find_hw_watchpoint(target_ulong addr, int *flag)
> +{
> +    int n;
> +
> +    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_ACCESS);
> +    if (n >= 0) {
> +        *flag = BP_MEM_ACCESS;
> +        return n;
> +    }
> +
> +    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_WRITE);
> +    if (n >= 0) {
> +        *flag = BP_MEM_WRITE;
> +        return n;
> +    }
> +
> +    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_READ);
> +    if (n >= 0) {
> +        *flag = BP_MEM_READ;
> +        return n;
> +    }
> +
> +    return -1;
> +}
> +
> +int kvm_arch_insert_hw_breakpoint(target_ulong addr,
> +                                  target_ulong len, int type)
> +{
> +    if ((nb_hw_breakpoint + nb_hw_watchpoint) >= ARRAY_SIZE(hw_debug_points))
> +	return -ENOBUFS;
> +
> +    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].addr = addr;
> +    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].type = type;
> +
> +    switch (type) {
> +    case GDB_BREAKPOINT_HW:
> +        if (nb_hw_breakpoint >= max_hw_breakpoint) {
> +            return -ENOBUFS;
> +        }
> +
> +        if (find_hw_breakpoint(addr, type) >= 0) {
> +            return -EEXIST;
> +        }
> +
> +        nb_hw_breakpoint++;
> +        break;
> +
> +    case GDB_WATCHPOINT_WRITE:
> +    case GDB_WATCHPOINT_READ:
> +    case GDB_WATCHPOINT_ACCESS:
> +        if (nb_hw_watchpoint >= max_hw_watchpoint) {
> +            return -ENOBUFS;
> +        }
> +
> +        if (find_hw_breakpoint(addr, type) >= 0) {
> +            return -EEXIST;
> +        }
> +
> +        nb_hw_watchpoint++;
> +        break;
> +
> +    default:
> +        return -ENOSYS;
> +    }
> +
> +    return 0;
> +}
> +
> +int kvm_arch_remove_hw_breakpoint(target_ulong addr,
> +                                  target_ulong len, int type)
> +{
> +    int n;
> +
> +    n = find_hw_breakpoint(addr, type);
> +    if (n < 0) {
> +        return -ENOENT;
> +    }
> +
> +    switch (type) {
> +    case GDB_BREAKPOINT_HW:
> +        nb_hw_breakpoint--;
> +        break;
> +
> +    case GDB_WATCHPOINT_WRITE:
> +    case GDB_WATCHPOINT_READ:
> +    case GDB_WATCHPOINT_ACCESS:
> +        nb_hw_watchpoint--;
> +        break;
> +
> +    default:
> +        return -ENOSYS;
> +    }
> +    hw_debug_points[n] = hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint];
> +
> +    return 0;
> +}
> +
> +void kvm_arch_remove_all_hw_breakpoints(void)
> +{
> +    nb_hw_breakpoint = nb_hw_watchpoint = 0;
> +}
> +
>   void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
>   {
> +    int n;
> +
>       /* Software Breakpoint updates */
>       if (kvm_sw_breakpoints_active(cs)) {
>           dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
>       }
> +
> +    assert((nb_hw_breakpoint + nb_hw_watchpoint)
> +           <= ARRAY_SIZE(hw_debug_points));
> +    assert((nb_hw_breakpoint + nb_hw_watchpoint) <= ARRAY_SIZE(dbg->arch.bp));
> +
> +    if (nb_hw_breakpoint + nb_hw_watchpoint > 0) {
> +        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP;
> +        memset(dbg->arch.bp, 0, sizeof(dbg->arch.bp));
> +        for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) {
> +            switch (hw_debug_points[n].type) {
> +            case GDB_BREAKPOINT_HW:
> +                dbg->arch.bp[n].type = KVMPPC_DEBUG_BREAKPOINT;
> +                break;
> +            case GDB_WATCHPOINT_WRITE:
> +                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE;
> +                break;
> +            case GDB_WATCHPOINT_READ:
> +                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_READ;
> +                break;
> +            case GDB_WATCHPOINT_ACCESS:
> +                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE |
> +                                        KVMPPC_DEBUG_WATCH_READ;
> +                break;
> +            default:
> +                cpu_abort(cs, "Unsupported breakpoint type\n");
> +            }
> +            dbg->arch.bp[n].addr = hw_debug_points[n].addr;
> +        }
> +    }
> +}
> +
> +static void kvm_e500_handle_debug(CPUState *cs, int handle)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +
> +    env->spr[SPR_BOOKE_DBSR] = 0;

How would KVM ever know that DBSR is now 0? And why do we need this in 
the first place? The guest's DBSR value doesn't get set on debug 
interrupts unless we call inject_debug_exception(). So there's no need 
to clear it either, no?


Alex

>   }
>   
>   static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run)
>   {
>       CPUState *cs = CPU(cpu);
> +    CPUPPCState *env = &cpu->env;
>       struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
>       int handle = 0;
> +    int n;
> +    int flag = 0;
>   
> -    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
> +    if (cs->singlestep_enabled) {
> +        handle = 1;
> +    } else if (arch_info->status) {
> +        if (nb_hw_breakpoint + nb_hw_watchpoint > 0) {
> +            if (arch_info->status & KVMPPC_DEBUG_BREAKPOINT) {
> +                n = find_hw_breakpoint(arch_info->address, GDB_BREAKPOINT_HW);
> +                if (n >= 0) {
> +                    handle = 1;
> +                }
> +            } else if (arch_info->status & (KVMPPC_DEBUG_WATCH_READ |
> +                                            KVMPPC_DEBUG_WATCH_WRITE)) {
> +                n = find_hw_watchpoint(arch_info->address,  &flag);
> +                if (n >= 0) {
> +                    handle = 1;
> +                    cs->watchpoint_hit = &hw_watchpoint;
> +                    hw_watchpoint.vaddr = hw_debug_points[n].addr;
> +                    hw_watchpoint.flags = flag;
> +                }
> +            }
> +        }
> +    } else if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
>           handle = 1;
>       }
>   
> +    if (handle) {
> +        if (env->excp_model == POWERPC_EXCP_BOOKE) {
> +            kvm_e500_handle_debug(cs, handle);
> +        }
> +    } else {
> +       /* inject debug exception into guest */
> +       env->pending_interrupts |=  1 << PPC_INTERRUPT_DEBUG;
> +    }
>       return handle;
>   }
>   
> @@ -2103,20 +2325,6 @@ void kvm_arch_init_irq_routing(KVMState *s)
>   {
>   }
>   
> -int kvm_arch_insert_hw_breakpoint(target_ulong addr, target_ulong len, int type)
> -{
> -    return -EINVAL;
> -}
> -
> -int kvm_arch_remove_hw_breakpoint(target_ulong addr, target_ulong len, int type)
> -{
> -    return -EINVAL;
> -}
> -
> -void kvm_arch_remove_all_hw_breakpoints(void)
> -{
> -}
> -
>   struct kvm_get_htab_buf {
>       struct kvm_get_htab_header header;
>       /*

Re: [Qemu-devel] [PATCH 5/5 v5] ppc: Add hw breakpoint watchpoint support

[Bharat.Bhushan]

> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Thursday, June 26, 2014 3:52 PM
> To: Bhushan Bharat-R65777
> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org; maddy@linux.vnet.ibm.com
> Subject: Re: [PATCH 5/5 v5] ppc: Add hw breakpoint watchpoint support
> 
> 
> On 26.06.14 09:23, Bharat Bhushan wrote:
> > This patch adds hardware breakpoint and hardware watchpoint support
> > for ppc. If the debug interrupt is not handled then this is injected
> > to guest.
> >
> > Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
> > ---
> > v4->v5
> >   - cleanup in kvmppc_hw_debug_points_init()
> >   - replaced assert in kvm_arch_insert_hw_breakpoint() with return.
> >     This allows gdb to through warning "You may have requested too many
> >     hardware breakpoints/watchpoints.". Now user can remove extra breakpint
> >     and continue.
> >
> >   target-ppc/kvm.c | 238 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> --
> >   1 file changed, 223 insertions(+), 15 deletions(-)
> >
> > diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index
> > 3195491..3347c1b 100644
> > --- a/target-ppc/kvm.c
> > +++ b/target-ppc/kvm.c
> > @@ -38,6 +38,7 @@
> >   #include "hw/ppc/ppc.h"
> >   #include "sysemu/watchdog.h"
> >   #include "trace.h"
> > +#include "exec/gdbstub.h"
> >
> >   //#define DEBUG_KVM
> >
> > @@ -410,6 +411,38 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu)
> >       return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
> >   }
> >
> > +/* e500 supports 2 h/w breakpoint and 2 watchpoint.
> > + * book3s supports only 1 watchpoint, so array size
> > + * of 4 is sufficient for now.
> > + */
> > +#define MAX_HW_BKPTS 4
> > +
> > +static struct HWBreakpoint {
> > +    target_ulong addr;
> > +    int type;
> > +} hw_debug_points[MAX_HW_BKPTS];
> > +
> > +static CPUWatchpoint hw_watchpoint;
> > +
> > +/* Default there is no breakpoint and watchpoint supported */ static
> > +int max_hw_breakpoint; static int max_hw_watchpoint; static int
> > +nb_hw_breakpoint; static int nb_hw_watchpoint;
> > +
> > +static void kvmppc_hw_debug_points_init(CPUPPCState *cenv) {
> > +    if (cenv->excp_model == POWERPC_EXCP_BOOKE) {
> > +        max_hw_breakpoint = 2;
> > +        max_hw_watchpoint = 2;
> > +    }
> > +
> > +    if ((max_hw_breakpoint + max_hw_watchpoint) > MAX_HW_BKPTS) {
> > +        fprintf(stderr, "Error initializing h/w breakpoints\n");
> > +        return;
> > +    }
> > +}
> > +
> >   int kvm_arch_init_vcpu(CPUState *cs)
> >   {
> >       PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -437,6 +470,7 @@ int
> > kvm_arch_init_vcpu(CPUState *cs)
> >       }
> >
> >       kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode);
> > +    kvmppc_hw_debug_points_init(cenv);
> >
> >       return ret;
> >   }
> > @@ -1345,24 +1379,212 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs,
> struct kvm_sw_breakpoint *bp)
> >       return 0;
> >   }
> >
> > +static int find_hw_breakpoint(target_ulong addr, int type) {
> > +    int n;
> > +
> > +    assert((nb_hw_breakpoint + nb_hw_watchpoint)
> > +           <= ARRAY_SIZE(hw_debug_points));
> > +
> > +    for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) {
> > +        if (hw_debug_points[n].addr == addr && hw_debug_points[n].type ==
> type) {
> > +            return n;
> > +        }
> > +    }
> > +
> > +    return -1;
> > +}
> > +
> > +static int find_hw_watchpoint(target_ulong addr, int *flag) {
> > +    int n;
> > +
> > +    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_ACCESS);
> > +    if (n >= 0) {
> > +        *flag = BP_MEM_ACCESS;
> > +        return n;
> > +    }
> > +
> > +    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_WRITE);
> > +    if (n >= 0) {
> > +        *flag = BP_MEM_WRITE;
> > +        return n;
> > +    }
> > +
> > +    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_READ);
> > +    if (n >= 0) {
> > +        *flag = BP_MEM_READ;
> > +        return n;
> > +    }
> > +
> > +    return -1;
> > +}
> > +
> > +int kvm_arch_insert_hw_breakpoint(target_ulong addr,
> > +                                  target_ulong len, int type) {
> > +    if ((nb_hw_breakpoint + nb_hw_watchpoint) >= ARRAY_SIZE(hw_debug_points))
> > +	return -ENOBUFS;
> > +
> > +    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].addr = addr;
> > +    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].type = type;
> > +
> > +    switch (type) {
> > +    case GDB_BREAKPOINT_HW:
> > +        if (nb_hw_breakpoint >= max_hw_breakpoint) {
> > +            return -ENOBUFS;
> > +        }
> > +
> > +        if (find_hw_breakpoint(addr, type) >= 0) {
> > +            return -EEXIST;
> > +        }
> > +
> > +        nb_hw_breakpoint++;
> > +        break;
> > +
> > +    case GDB_WATCHPOINT_WRITE:
> > +    case GDB_WATCHPOINT_READ:
> > +    case GDB_WATCHPOINT_ACCESS:
> > +        if (nb_hw_watchpoint >= max_hw_watchpoint) {
> > +            return -ENOBUFS;
> > +        }
> > +
> > +        if (find_hw_breakpoint(addr, type) >= 0) {
> > +            return -EEXIST;
> > +        }
> > +
> > +        nb_hw_watchpoint++;
> > +        break;
> > +
> > +    default:
> > +        return -ENOSYS;
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +int kvm_arch_remove_hw_breakpoint(target_ulong addr,
> > +                                  target_ulong len, int type) {
> > +    int n;
> > +
> > +    n = find_hw_breakpoint(addr, type);
> > +    if (n < 0) {
> > +        return -ENOENT;
> > +    }
> > +
> > +    switch (type) {
> > +    case GDB_BREAKPOINT_HW:
> > +        nb_hw_breakpoint--;
> > +        break;
> > +
> > +    case GDB_WATCHPOINT_WRITE:
> > +    case GDB_WATCHPOINT_READ:
> > +    case GDB_WATCHPOINT_ACCESS:
> > +        nb_hw_watchpoint--;
> > +        break;
> > +
> > +    default:
> > +        return -ENOSYS;
> > +    }
> > +    hw_debug_points[n] = hw_debug_points[nb_hw_breakpoint +
> > + nb_hw_watchpoint];
> > +
> > +    return 0;
> > +}
> > +
> > +void kvm_arch_remove_all_hw_breakpoints(void)
> > +{
> > +    nb_hw_breakpoint = nb_hw_watchpoint = 0; }
> > +
> >   void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
> >   {
> > +    int n;
> > +
> >       /* Software Breakpoint updates */
> >       if (kvm_sw_breakpoints_active(cs)) {
> >           dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
> >       }
> > +
> > +    assert((nb_hw_breakpoint + nb_hw_watchpoint)
> > +           <= ARRAY_SIZE(hw_debug_points));
> > +    assert((nb_hw_breakpoint + nb_hw_watchpoint) <=
> > + ARRAY_SIZE(dbg->arch.bp));
> > +
> > +    if (nb_hw_breakpoint + nb_hw_watchpoint > 0) {
> > +        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP;
> > +        memset(dbg->arch.bp, 0, sizeof(dbg->arch.bp));
> > +        for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) {
> > +            switch (hw_debug_points[n].type) {
> > +            case GDB_BREAKPOINT_HW:
> > +                dbg->arch.bp[n].type = KVMPPC_DEBUG_BREAKPOINT;
> > +                break;
> > +            case GDB_WATCHPOINT_WRITE:
> > +                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE;
> > +                break;
> > +            case GDB_WATCHPOINT_READ:
> > +                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_READ;
> > +                break;
> > +            case GDB_WATCHPOINT_ACCESS:
> > +                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE |
> > +                                        KVMPPC_DEBUG_WATCH_READ;
> > +                break;
> > +            default:
> > +                cpu_abort(cs, "Unsupported breakpoint type\n");
> > +            }
> > +            dbg->arch.bp[n].addr = hw_debug_points[n].addr;
> > +        }
> > +    }
> > +}
> > +
> > +static void kvm_e500_handle_debug(CPUState *cs, int handle) {
> > +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> > +    CPUPPCState *env = &cpu->env;
> > +
> > +    env->spr[SPR_BOOKE_DBSR] = 0;
> 
> How would KVM ever know that DBSR is now 0?

Yes, guest will not come to know of this "0" value. I was wrong, thinking about that this will be used in inject_debug_exception(), but inject_debug_exception() is not called in this flow.

> And why do we need this in the first
> place? The guest's DBSR value doesn't get set on debug interrupts unless we call
> inject_debug_exception().

vcpu->arch.dbsr is set on debug exception in KVM and that is what guest sees, so we should clear DBSR.

> So there's no need to clear it either, no?

So I think I need a one_reg interface to set/clear DBSR.

Also I do not need to explicitly set "sregs.u.e.dbsr" in inject_debug_exception(). 

Thanks
-Bharat

> 
> 
> Alex
> 
> >   }
> >
> >   static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run)
> >   {
> >       CPUState *cs = CPU(cpu);
> > +    CPUPPCState *env = &cpu->env;
> >       struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
> >       int handle = 0;
> > +    int n;
> > +    int flag = 0;
> >
> > -    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
> > +    if (cs->singlestep_enabled) {
> > +        handle = 1;
> > +    } else if (arch_info->status) {
> > +        if (nb_hw_breakpoint + nb_hw_watchpoint > 0) {
> > +            if (arch_info->status & KVMPPC_DEBUG_BREAKPOINT) {
> > +                n = find_hw_breakpoint(arch_info->address,
> GDB_BREAKPOINT_HW);
> > +                if (n >= 0) {
> > +                    handle = 1;
> > +                }
> > +            } else if (arch_info->status & (KVMPPC_DEBUG_WATCH_READ |
> > +                                            KVMPPC_DEBUG_WATCH_WRITE)) {
> > +                n = find_hw_watchpoint(arch_info->address,  &flag);
> > +                if (n >= 0) {
> > +                    handle = 1;
> > +                    cs->watchpoint_hit = &hw_watchpoint;
> > +                    hw_watchpoint.vaddr = hw_debug_points[n].addr;
> > +                    hw_watchpoint.flags = flag;
> > +                }
> > +            }
> > +        }
> > +    } else if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
> >           handle = 1;
> >       }
> >
> > +    if (handle) {
> > +        if (env->excp_model == POWERPC_EXCP_BOOKE) {
> > +            kvm_e500_handle_debug(cs, handle);
> > +        }
> > +    } else {
> > +       /* inject debug exception into guest */
> > +       env->pending_interrupts |=  1 << PPC_INTERRUPT_DEBUG;
> > +    }
> >       return handle;
> >   }
> >
> > @@ -2103,20 +2325,6 @@ void kvm_arch_init_irq_routing(KVMState *s)
> >   {
> >   }
> >
> > -int kvm_arch_insert_hw_breakpoint(target_ulong addr, target_ulong
> > len, int type) -{
> > -    return -EINVAL;
> > -}
> > -
> > -int kvm_arch_remove_hw_breakpoint(target_ulong addr, target_ulong
> > len, int type) -{
> > -    return -EINVAL;
> > -}
> > -
> > -void kvm_arch_remove_all_hw_breakpoints(void)
> > -{
> > -}
> > -
> >   struct kvm_get_htab_buf {
> >       struct kvm_get_htab_header header;
> >       /*

Re: [Qemu-devel] [PATCH 5/5 v5] ppc: Add hw breakpoint watchpoint support

[Alexander Graf]

On 27.06.14 07:51, Bharat.Bhushan@freescale.com wrote:
>
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de]
>> Sent: Thursday, June 26, 2014 3:52 PM
>> To: Bhushan Bharat-R65777
>> Cc: qemu-ppc@nongnu.org; qemu-devel@nongnu.org; maddy@linux.vnet.ibm.com
>> Subject: Re: [PATCH 5/5 v5] ppc: Add hw breakpoint watchpoint support
>>
>>
>> On 26.06.14 09:23, Bharat Bhushan wrote:
>>> This patch adds hardware breakpoint and hardware watchpoint support
>>> for ppc. If the debug interrupt is not handled then this is injected
>>> to guest.
>>>
>>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
>>> ---
>>> v4->v5
>>>    - cleanup in kvmppc_hw_debug_points_init()
>>>    - replaced assert in kvm_arch_insert_hw_breakpoint() with return.
>>>      This allows gdb to through warning "You may have requested too many
>>>      hardware breakpoints/watchpoints.". Now user can remove extra breakpint
>>>      and continue.
>>>
>>>    target-ppc/kvm.c | 238 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>> --
>>>    1 file changed, 223 insertions(+), 15 deletions(-)
>>>
>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c index
>>> 3195491..3347c1b 100644
>>> --- a/target-ppc/kvm.c
>>> +++ b/target-ppc/kvm.c
>>> @@ -38,6 +38,7 @@
>>>    #include "hw/ppc/ppc.h"
>>>    #include "sysemu/watchdog.h"
>>>    #include "trace.h"
>>> +#include "exec/gdbstub.h"
>>>
>>>    //#define DEBUG_KVM
>>>
>>> @@ -410,6 +411,38 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu)
>>>        return ppc_get_vcpu_dt_id(POWERPC_CPU(cpu));
>>>    }
>>>
>>> +/* e500 supports 2 h/w breakpoint and 2 watchpoint.
>>> + * book3s supports only 1 watchpoint, so array size
>>> + * of 4 is sufficient for now.
>>> + */
>>> +#define MAX_HW_BKPTS 4
>>> +
>>> +static struct HWBreakpoint {
>>> +    target_ulong addr;
>>> +    int type;
>>> +} hw_debug_points[MAX_HW_BKPTS];
>>> +
>>> +static CPUWatchpoint hw_watchpoint;
>>> +
>>> +/* Default there is no breakpoint and watchpoint supported */ static
>>> +int max_hw_breakpoint; static int max_hw_watchpoint; static int
>>> +nb_hw_breakpoint; static int nb_hw_watchpoint;
>>> +
>>> +static void kvmppc_hw_debug_points_init(CPUPPCState *cenv) {
>>> +    if (cenv->excp_model == POWERPC_EXCP_BOOKE) {
>>> +        max_hw_breakpoint = 2;
>>> +        max_hw_watchpoint = 2;
>>> +    }
>>> +
>>> +    if ((max_hw_breakpoint + max_hw_watchpoint) > MAX_HW_BKPTS) {
>>> +        fprintf(stderr, "Error initializing h/w breakpoints\n");
>>> +        return;
>>> +    }
>>> +}
>>> +
>>>    int kvm_arch_init_vcpu(CPUState *cs)
>>>    {
>>>        PowerPCCPU *cpu = POWERPC_CPU(cs); @@ -437,6 +470,7 @@ int
>>> kvm_arch_init_vcpu(CPUState *cs)
>>>        }
>>>
>>>        kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode);
>>> +    kvmppc_hw_debug_points_init(cenv);
>>>
>>>        return ret;
>>>    }
>>> @@ -1345,24 +1379,212 @@ int kvm_arch_remove_sw_breakpoint(CPUState *cs,
>> struct kvm_sw_breakpoint *bp)
>>>        return 0;
>>>    }
>>>
>>> +static int find_hw_breakpoint(target_ulong addr, int type) {
>>> +    int n;
>>> +
>>> +    assert((nb_hw_breakpoint + nb_hw_watchpoint)
>>> +           <= ARRAY_SIZE(hw_debug_points));
>>> +
>>> +    for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) {
>>> +        if (hw_debug_points[n].addr == addr && hw_debug_points[n].type ==
>> type) {
>>> +            return n;
>>> +        }
>>> +    }
>>> +
>>> +    return -1;
>>> +}
>>> +
>>> +static int find_hw_watchpoint(target_ulong addr, int *flag) {
>>> +    int n;
>>> +
>>> +    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_ACCESS);
>>> +    if (n >= 0) {
>>> +        *flag = BP_MEM_ACCESS;
>>> +        return n;
>>> +    }
>>> +
>>> +    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_WRITE);
>>> +    if (n >= 0) {
>>> +        *flag = BP_MEM_WRITE;
>>> +        return n;
>>> +    }
>>> +
>>> +    n = find_hw_breakpoint(addr, GDB_WATCHPOINT_READ);
>>> +    if (n >= 0) {
>>> +        *flag = BP_MEM_READ;
>>> +        return n;
>>> +    }
>>> +
>>> +    return -1;
>>> +}
>>> +
>>> +int kvm_arch_insert_hw_breakpoint(target_ulong addr,
>>> +                                  target_ulong len, int type) {
>>> +    if ((nb_hw_breakpoint + nb_hw_watchpoint) >= ARRAY_SIZE(hw_debug_points))
>>> +	return -ENOBUFS;
>>> +
>>> +    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].addr = addr;
>>> +    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].type = type;
>>> +
>>> +    switch (type) {
>>> +    case GDB_BREAKPOINT_HW:
>>> +        if (nb_hw_breakpoint >= max_hw_breakpoint) {
>>> +            return -ENOBUFS;
>>> +        }
>>> +
>>> +        if (find_hw_breakpoint(addr, type) >= 0) {
>>> +            return -EEXIST;
>>> +        }
>>> +
>>> +        nb_hw_breakpoint++;
>>> +        break;
>>> +
>>> +    case GDB_WATCHPOINT_WRITE:
>>> +    case GDB_WATCHPOINT_READ:
>>> +    case GDB_WATCHPOINT_ACCESS:
>>> +        if (nb_hw_watchpoint >= max_hw_watchpoint) {
>>> +            return -ENOBUFS;
>>> +        }
>>> +
>>> +        if (find_hw_breakpoint(addr, type) >= 0) {
>>> +            return -EEXIST;
>>> +        }
>>> +
>>> +        nb_hw_watchpoint++;
>>> +        break;
>>> +
>>> +    default:
>>> +        return -ENOSYS;
>>> +    }
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +int kvm_arch_remove_hw_breakpoint(target_ulong addr,
>>> +                                  target_ulong len, int type) {
>>> +    int n;
>>> +
>>> +    n = find_hw_breakpoint(addr, type);
>>> +    if (n < 0) {
>>> +        return -ENOENT;
>>> +    }
>>> +
>>> +    switch (type) {
>>> +    case GDB_BREAKPOINT_HW:
>>> +        nb_hw_breakpoint--;
>>> +        break;
>>> +
>>> +    case GDB_WATCHPOINT_WRITE:
>>> +    case GDB_WATCHPOINT_READ:
>>> +    case GDB_WATCHPOINT_ACCESS:
>>> +        nb_hw_watchpoint--;
>>> +        break;
>>> +
>>> +    default:
>>> +        return -ENOSYS;
>>> +    }
>>> +    hw_debug_points[n] = hw_debug_points[nb_hw_breakpoint +
>>> + nb_hw_watchpoint];
>>> +
>>> +    return 0;
>>> +}
>>> +
>>> +void kvm_arch_remove_all_hw_breakpoints(void)
>>> +{
>>> +    nb_hw_breakpoint = nb_hw_watchpoint = 0; }
>>> +
>>>    void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
>>>    {
>>> +    int n;
>>> +
>>>        /* Software Breakpoint updates */
>>>        if (kvm_sw_breakpoints_active(cs)) {
>>>            dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
>>>        }
>>> +
>>> +    assert((nb_hw_breakpoint + nb_hw_watchpoint)
>>> +           <= ARRAY_SIZE(hw_debug_points));
>>> +    assert((nb_hw_breakpoint + nb_hw_watchpoint) <=
>>> + ARRAY_SIZE(dbg->arch.bp));
>>> +
>>> +    if (nb_hw_breakpoint + nb_hw_watchpoint > 0) {
>>> +        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_HW_BP;
>>> +        memset(dbg->arch.bp, 0, sizeof(dbg->arch.bp));
>>> +        for (n = 0; n < nb_hw_breakpoint + nb_hw_watchpoint; n++) {
>>> +            switch (hw_debug_points[n].type) {
>>> +            case GDB_BREAKPOINT_HW:
>>> +                dbg->arch.bp[n].type = KVMPPC_DEBUG_BREAKPOINT;
>>> +                break;
>>> +            case GDB_WATCHPOINT_WRITE:
>>> +                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE;
>>> +                break;
>>> +            case GDB_WATCHPOINT_READ:
>>> +                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_READ;
>>> +                break;
>>> +            case GDB_WATCHPOINT_ACCESS:
>>> +                dbg->arch.bp[n].type = KVMPPC_DEBUG_WATCH_WRITE |
>>> +                                        KVMPPC_DEBUG_WATCH_READ;
>>> +                break;
>>> +            default:
>>> +                cpu_abort(cs, "Unsupported breakpoint type\n");
>>> +            }
>>> +            dbg->arch.bp[n].addr = hw_debug_points[n].addr;
>>> +        }
>>> +    }
>>> +}
>>> +
>>> +static void kvm_e500_handle_debug(CPUState *cs, int handle) {
>>> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
>>> +    CPUPPCState *env = &cpu->env;
>>> +
>>> +    env->spr[SPR_BOOKE_DBSR] = 0;
>> How would KVM ever know that DBSR is now 0?
> Yes, guest will not come to know of this "0" value. I was wrong, thinking about that this will be used in inject_debug_exception(), but inject_debug_exception() is not called in this flow.
>
>> And why do we need this in the first
>> place? The guest's DBSR value doesn't get set on debug interrupts unless we call
>> inject_debug_exception().
> vcpu->arch.dbsr is set on debug exception in KVM and that is what guest sees, so we should clear DBSR.

Ah, ok. So do we actually need inject_debug_exception()?

>
>> So there's no need to clear it either, no?
> So I think I need a one_reg interface to set/clear DBSR.

The sregs interface should be good enough for now, no?


Alex

Re: [Qemu-devel] [PATCH 5/5 v5] ppc: Add hw breakpoint watchpoint support

[Bharat.Bhushan]

> >>> +
> >>> +static void kvm_e500_handle_debug(CPUState *cs, int handle) {
> >>> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> >>> +    CPUPPCState *env = &cpu->env;
> >>> +
> >>> +    env->spr[SPR_BOOKE_DBSR] = 0;
> >> How would KVM ever know that DBSR is now 0?
> > Yes, guest will not come to know of this "0" value. I was wrong, thinking
> about that this will be used in inject_debug_exception(), but
> inject_debug_exception() is not called in this flow.
> >
> >> And why do we need this in the first
> >> place? The guest's DBSR value doesn't get set on debug interrupts
> >> unless we call inject_debug_exception().
> > vcpu->arch.dbsr is set on debug exception in KVM and that is what guest sees,
> so we should clear DBSR.
> 
> Ah, ok. So do we actually need inject_debug_exception()?

If not then how we to inject interrupt to guest ?

> 
> >
> >> So there's no need to clear it either, no?
> > So I think I need a one_reg interface to set/clear DBSR.
> 
> The sregs interface should be good enough for now, no?

So basically we want two things
1) Just clear DBSR in KVM when debug interrupts are handled by QEMU
2) Set DSRR0/1 or CSRR0/1 and set DBSR when interrupt are not handled by QEMU


For (1); We can add a function which will use SREGS interface to clear DBSR.
For (2); we will use inject_debug_exception().


Thanks
-Bharat

> 
> 
> Alex

[Qemu-devel] [PATCH 4/5 v5] ppc: Add software breakpoint support

[Bharat Bhushan]

This patch allow insert/remove software breakpoint

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v4->v5
 - No change

 target-ppc/kvm.c | 73 +++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 59 insertions(+), 14 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 5238de7..3195491 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -1317,6 +1317,55 @@ static int kvmppc_handle_dcr_write(CPUPPCState *env, uint32_t dcrn, uint32_t dat
     return 0;
 }
 
+int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+{
+    /* Mixed endian case is not handled */
+    uint32_t sc = debug_inst_opcode;
+
+    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
+                            sizeof(sc), 0) ||
+        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, sizeof(sc), 1)) {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+int kvm_arch_remove_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
+{
+    uint32_t sc;
+
+    if (cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&sc, sizeof(sc), 0) ||
+        sc != debug_inst_opcode ||
+        cpu_memory_rw_debug(cs, bp->pc, (uint8_t *)&bp->saved_insn,
+                            sizeof(sc), 1)) {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+void kvm_arch_update_guest_debug(CPUState *cs, struct kvm_guest_debug *dbg)
+{
+    /* Software Breakpoint updates */
+    if (kvm_sw_breakpoints_active(cs)) {
+        dbg->control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_USE_SW_BP;
+    }
+}
+
+static int kvm_handle_debug(PowerPCCPU *cpu, struct kvm_run *run)
+{
+    CPUState *cs = CPU(cpu);
+    struct kvm_debug_exit_arch *arch_info = &run->debug.arch;
+    int handle = 0;
+
+    if (kvm_find_sw_breakpoint(cs, arch_info->address)) {
+        handle = 1;
+    }
+
+    return handle;
+}
+
 int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
@@ -1357,6 +1406,16 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run *run)
         ret = 0;
         break;
 
+    case KVM_EXIT_DEBUG:
+        DPRINTF("handle debug exception\n");
+        if (kvm_handle_debug(cpu, run)) {
+            ret = EXCP_DEBUG;
+            break;
+        }
+        /* re-enter, this exception was guest-internal */
+        ret = 0;
+        break;
+
     default:
         fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
         ret = -1;
@@ -2044,16 +2103,6 @@ void kvm_arch_init_irq_routing(KVMState *s)
 {
 }
 
-int kvm_arch_insert_sw_breakpoint(CPUState *cpu, struct kvm_sw_breakpoint *bp)
-{
-    return -EINVAL;
-}
-
-int kvm_arch_remove_sw_breakpoint(CPUState *cpu, struct kvm_sw_breakpoint *bp)
-{
-    return -EINVAL;
-}
-
 int kvm_arch_insert_hw_breakpoint(target_ulong addr, target_ulong len, int type)
 {
     return -EINVAL;
@@ -2068,10 +2117,6 @@ void kvm_arch_remove_all_hw_breakpoints(void)
 {
 }
 
-void kvm_arch_update_guest_debug(CPUState *cpu, struct kvm_guest_debug *dbg)
-{
-}
-
 struct kvm_get_htab_buf {
     struct kvm_get_htab_header header;
     /*
-- 
1.9.0

[Qemu-devel] [PATCH 3/5 v5] ppc: Add debug interrupt injection handler

[Bharat Bhushan]

With this patch a debug interrupt can be injected to guest.
Follow up patch will use this interface to inject debug
interrupt to guest if qemu will not be able to handle.

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v4->v5
 - No change

 target-ppc/kvm.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 51 insertions(+), 2 deletions(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 70f77d1..5238de7 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -759,11 +759,59 @@ static int kvm_put_vpa(CPUState *cs)
 }
 #endif /* TARGET_PPC64 */
 
-static int kvmppc_inject_debug_exception(CPUState *cs)
+static int kvmppc_e500_inject_debug_exception(CPUState *cs)
 {
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+    struct kvm_sregs sregs;
+    int ret;
+
+    if (!cap_booke_sregs) {
+        return -1;
+    }
+
+    ret = kvm_vcpu_ioctl(cs, KVM_GET_SREGS, &sregs);
+    if (ret < 0) {
+        return -1;
+    }
+
+    if (sregs.u.e.features & KVM_SREGS_E_ED) {
+        sregs.u.e.dsrr0 = env->nip;
+        sregs.u.e.dsrr1 = env->msr;
+    } else {
+        sregs.u.e.csrr0 = env->nip;
+        sregs.u.e.csrr1 = env->msr;
+    }
+
+    sregs.u.e.update_special = KVM_SREGS_E_UPDATE_DBSR;
+    sregs.u.e.dbsr = env->spr[SPR_BOOKE_DBSR];
+
+    ret = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, &sregs);
+    if (ret < 0) {
+        return -1;
+    }
+
     return 0;
 }
 
+static int kvmppc_inject_debug_exception(CPUState *cs)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+    int ret = -1;
+
+    switch (env->excp_model) {
+    case POWERPC_EXCP_BOOKE:
+        ret = kvmppc_e500_inject_debug_exception(cs);
+        break;
+    default:
+        fprintf(stderr, "%s: Invalid exception model %d\n",
+                __func__, env->excp_model);
+        break;
+    }
+    return ret;
+}
+
 static void kvmppc_inject_exception(CPUState *cs)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
@@ -772,8 +820,9 @@ static void kvmppc_inject_exception(CPUState *cs)
     if (env->pending_interrupts & (1 << PPC_INTERRUPT_DEBUG)) {
         if (kvmppc_inject_debug_exception(cs)) {
             fprintf(stderr, "%s: Debug exception injection failed\n", __func__);
+            return;
         }
-        return;
+        env->pending_interrupts &= ~(1 << PPC_INTERRUPT_DEBUG);
     }
 }
 
-- 
1.9.0

[Qemu-devel] [PATCH 2/5 v5] ppc: Add interface to inject interrupt to guest

[Bharat Bhushan]

This patch adds interface to inject interrupt to guest.
Currently a void debug exception function added.
Follow up patch will use this interface to inject debug
interrupt to guest

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v4->v5
 - No change

 target-ppc/kvm.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 1f78cd1..70f77d1 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -759,6 +759,24 @@ static int kvm_put_vpa(CPUState *cs)
 }
 #endif /* TARGET_PPC64 */
 
+static int kvmppc_inject_debug_exception(CPUState *cs)
+{
+    return 0;
+}
+
+static void kvmppc_inject_exception(CPUState *cs)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+
+    if (env->pending_interrupts & (1 << PPC_INTERRUPT_DEBUG)) {
+        if (kvmppc_inject_debug_exception(cs)) {
+            fprintf(stderr, "%s: Debug exception injection failed\n", __func__);
+        }
+        return;
+    }
+}
+
 int kvm_arch_put_registers(CPUState *cs, int level)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
@@ -772,6 +790,10 @@ int kvm_arch_put_registers(CPUState *cs, int level)
         return ret;
     }
 
+    if (env->pending_interrupts) {
+        kvmppc_inject_exception(cs);
+    }
+
     regs.ctr = env->ctr;
     regs.lr  = env->lr;
     regs.xer = cpu_read_xer(env);
-- 
1.9.0

[Qemu-devel] [PATCH 1/5 v5] ppc: debug stub: Get trap instruction opcode from KVM

[Bharat Bhushan]

Get trap instruction opcode from KVM and this opcode will
be used for setting software breakpoint in following patch

Signed-off-by: Bharat Bhushan <Bharat.Bhushan@freescale.com>
---
v4->v5
 - No change

 target-ppc/kvm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index dfa5a26..1f78cd1 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -71,6 +71,8 @@ static int cap_papr;
 static int cap_htab_fd;
 static int cap_fixup_hcalls;
 
+static uint32_t debug_inst_opcode;
+
 /* XXX We have a race condition where we actually have a level triggered
  *     interrupt, but the infrastructure can't expose that yet, so the guest
  *     takes but ignores it, goes to sleep and never gets notified that there's
@@ -434,6 +436,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
         break;
     }
 
+    kvm_get_one_reg(cs, KVM_REG_PPC_DEBUG_INST, &debug_inst_opcode);
+
     return ret;
 }
 
-- 
1.9.0