qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 5/5 v3][RESEND] ppc: Add hw breakpoint watchpoin


From: Alexander Graf
Subject: Re: [Qemu-devel] [PATCH 5/5 v3][RESEND] ppc: Add hw breakpoint watchpoint support
Date: Tue, 24 Jun 2014 15:19:52 +0200
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.6.0


On 24.06.14 14:10, 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 <address@hidden>
---
v2->v3
  - Shared as much code as much possible for futuristic book3s support
  - Initializing number of hw breakpoint/watchpoints from KVM world
  - Other minor cleanup/fixes

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

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 8e2dbb3..4fb0efd 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,44 @@ 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)
+{
+    static bool initialize = true;
+
+    if (initialize) {
+        if (cenv->excp_model == POWERPC_EXCP_BOOKE) {
+            max_hw_breakpoint = 2;
+            max_hw_watchpoint = 2;
+        }
+
+        initialize = false;
+    }
+
+    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 +476,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;
  }
@@ -1343,24 +1383,216 @@ 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)
+{
+    assert((nb_hw_breakpoint + nb_hw_watchpoint)
+           <= ARRAY_SIZE(hw_debug_points));
+
+    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].addr = addr;
+    hw_debug_points[nb_hw_breakpoint + nb_hw_watchpoint].type = type;

Imagine the following:

  nb_hw_breakpoint = 2
  nb_hw_watchpoint = 2

The assert above succeeds, because 4 <= 4. However, the array shuffling below accesses memory that is out of bounds: hw_debug_points[4].

+
+    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;
+
+    cpu_synchronize_state(cs);
+    env->spr[SPR_BOOKE_DBSR] = 0;

I don't see how this would take any effect with KVM? I don't see where we synchonize DBSR.

  }
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) {
+        assert((nb_hw_breakpoint + nb_hw_watchpoint)
+               <= ARRAY_SIZE(hw_debug_points));

I don't think this assert needs to be here :). You already assert() properly in the actual find function.


Alex

+
+        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;
  }
@@ -2101,20 +2333,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;
      /*




reply via email to

[Prev in Thread] Current Thread [Next in Thread]