[RFC][PATCH 1/4] qemu: refactor cpu_watch/breakpoint API

[Jan Kiszka <jan.kiszka@web.de>]

[ Should apply against vanilla QEMU, but not ATM due to ongoing
constructions in gdbstub. ]

This patch prepares the QEMU cpu_watchpoint/breakpoint API to allow us
hooking in with KVM and doing guest debugging differently (maybe
QEMUAccel should provide appropriate callbacks for this, too). But it
also allows to extend QEMU's debugging features one day, specifically
/wrt different watchpoint types.

---
 qemu/cpu-all.h |   14 ++++++++++----
 qemu/exec.c    |   30 ++++++++++++++++++++----------
 qemu/gdbstub.c |   50 +++++++++++++++++++++++++++++---------------------
 3 files changed, 59 insertions(+), 35 deletions(-)

Index: b/qemu/cpu-all.h
===================================================================
--- a/qemu/cpu-all.h
+++ b/qemu/cpu-all.h
@@ -758,10 +758,16 @@ extern int code_copy_enabled;
 void cpu_interrupt(CPUState *s, int mask);
 void cpu_reset_interrupt(CPUState *env, int mask);
 
-int cpu_watchpoint_insert(CPUState *env, target_ulong addr);
-int cpu_watchpoint_remove(CPUState *env, target_ulong addr);
-int cpu_breakpoint_insert(CPUState *env, target_ulong pc);
-int cpu_breakpoint_remove(CPUState *env, target_ulong pc);
+#define GDB_BREAKPOINT_SW       0
+#define GDB_BREAKPOINT_HW       1
+#define GDB_WATCHPOINT_WRITE    2
+#define GDB_WATCHPOINT_READ     3
+#define GDB_WATCHPOINT_ACCESS   4
+
+int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ulong len, int type);
+int cpu_watchpoint_remove(CPUState *env, target_ulong addr, target_ulong len, int type);
+int cpu_breakpoint_insert(CPUState *env, target_ulong pc, target_ulong len, int type);
+int cpu_breakpoint_remove(CPUState *env, target_ulong pc, target_ulong len, int type);
 void cpu_single_step(CPUState *env, int enabled);
 void cpu_reset(CPUState *s);
 
Index: b/qemu/exec.c
===================================================================
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -1104,16 +1104,20 @@ static void breakpoint_invalidate(CPUSta
 #endif
 
 /* Add a watchpoint.  */
-int  cpu_watchpoint_insert(CPUState *env, target_ulong addr)
+int cpu_watchpoint_insert(CPUState *env, target_ulong addr, target_ulong len,
+                          int type)
 {
     int i;
 
+    if (type != GDB_WATCHPOINT_WRITE)
+        return -ENOSYS;
+
     for (i = 0; i < env->nb_watchpoints; i++) {
         if (addr == env->watchpoint[i].vaddr)
             return 0;
     }
     if (env->nb_watchpoints >= MAX_WATCHPOINTS)
-        return -1;
+        return -ENOBUFS;
 
     i = env->nb_watchpoints++;
     env->watchpoint[i].vaddr = addr;
@@ -1126,10 +1130,14 @@ int  cpu_watchpoint_insert(CPUState *env
 }
 
 /* Remove a watchpoint.  */
-int cpu_watchpoint_remove(CPUState *env, target_ulong addr)
+int cpu_watchpoint_remove(CPUState *env, target_ulong addr, target_ulong len,
+                          int type)
 {
     int i;
 
+    if (type != GDB_WATCHPOINT_WRITE)
+        return -ENOSYS;
+
     for (i = 0; i < env->nb_watchpoints; i++) {
         if (addr == env->watchpoint[i].vaddr) {
             env->nb_watchpoints--;
@@ -1138,12 +1146,13 @@ int cpu_watchpoint_remove(CPUState *env,
             return 0;
         }
     }
-    return -1;
+    return -ENOENT;
 }
 
 /* add a breakpoint. EXCP_DEBUG is returned by the CPU loop if a
    breakpoint is reached */
-int cpu_breakpoint_insert(CPUState *env, target_ulong pc)
+int cpu_breakpoint_insert(CPUState *env, target_ulong pc, target_ulong len,
+                          int type)
 {
 #if defined(TARGET_HAS_ICE)
     int i;
@@ -1154,7 +1163,7 @@ int cpu_breakpoint_insert(CPUState *env,
     }
 
     if (env->nb_breakpoints >= MAX_BREAKPOINTS)
-        return -1;
+        return -ENOBUFS;
     env->breakpoints[env->nb_breakpoints++] = pc;
 
     if (kvm_enabled())
@@ -1163,12 +1172,13 @@ int cpu_breakpoint_insert(CPUState *env,
     breakpoint_invalidate(env, pc);
     return 0;
 #else
-    return -1;
+    return -ENOSYS;
 #endif
 }
 
 /* remove a breakpoint */
-int cpu_breakpoint_remove(CPUState *env, target_ulong pc)
+int cpu_breakpoint_remove(CPUState *env, target_ulong pc, target_ulong len,
+                          int type)
 {
 #if defined(TARGET_HAS_ICE)
     int i;
@@ -1176,7 +1186,7 @@ int cpu_breakpoint_remove(CPUState *env,
         if (env->breakpoints[i] == pc)
             goto found;
     }
-    return -1;
+    return -ENOENT;
  found:
     env->nb_breakpoints--;
     if (i < env->nb_breakpoints)
@@ -1188,7 +1198,7 @@ int cpu_breakpoint_remove(CPUState *env,
     breakpoint_invalidate(env, pc);
     return 0;
 #else
-    return -1;
+    return -ENOSYS;
 #endif
 }
 
Index: b/qemu/gdbstub.c
===================================================================
--- a/qemu/gdbstub.c
+++ b/qemu/gdbstub.c
@@ -882,7 +882,7 @@ static void cpu_gdb_write_registers(CPUS
 static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf)
 {
     const char *p;
-    int ch, reg_size, type;
+    int ch, reg_size, type, res;
     char buf[4096];
     uint8_t mem_buf[4096];
     uint32_t *registers;
@@ -1017,21 +1017,20 @@ static int gdb_handle_packet(GDBState *s
         if (*p == ',')
             p++;
         len = strtoull(p, (char **)&p, 16);
-        if (type == 0 || type == 1) {
-            if (cpu_breakpoint_insert(env, addr) < 0)
-                goto breakpoint_error;
-            put_packet(s, "OK");
+        switch (type) {
+        case GDB_BREAKPOINT_SW ... GDB_BREAKPOINT_HW:
+            res = cpu_breakpoint_insert(env, addr, len, type);
+            break;
 #ifndef CONFIG_USER_ONLY
-        } else if (type == 2) {
-            if (cpu_watchpoint_insert(env, addr) < 0)
-                goto breakpoint_error;
-            put_packet(s, "OK");
+        case GDB_WATCHPOINT_WRITE ... GDB_WATCHPOINT_ACCESS:
+            res = cpu_watchpoint_insert(env, addr, len, type);
+            break;
 #endif
-        } else {
-        breakpoint_error:
-            put_packet(s, "E22");
+        default:
+            res = -ENOSYS;
+            break;
         }
-        break;
+        goto answer_bp_packet;
     case 'z':
         type = strtoul(p, (char **)&p, 16);
         if (*p == ',')
@@ -1040,17 +1039,26 @@ static int gdb_handle_packet(GDBState *s
         if (*p == ',')
             p++;
         len = strtoull(p, (char **)&p, 16);
-        if (type == 0 || type == 1) {
-            cpu_breakpoint_remove(env, addr);
-            put_packet(s, "OK");
+        switch (type) {
+        case GDB_BREAKPOINT_SW ... GDB_BREAKPOINT_HW:
+            res = cpu_breakpoint_remove(env, addr, len, type);
+            break;
 #ifndef CONFIG_USER_ONLY
-        } else if (type == 2) {
-            cpu_watchpoint_remove(env, addr);
-            put_packet(s, "OK");
+        case GDB_WATCHPOINT_WRITE ... GDB_WATCHPOINT_ACCESS:
+            res = cpu_watchpoint_remove(env, addr, len, type);
+            break;
 #endif
-        } else {
-            goto breakpoint_error;
+        default:
+            res = -ENOSYS;
+            break;
         }
+    answer_bp_packet:
+        if (res >= 0)
+            put_packet(s, "OK");
+        else if (res == -ENOSYS)
+            put_packet(s, "");
+        else
+            put_packet(s, "E22");
         break;
 #ifdef CONFIG_LINUX_USER
     case 'q':


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft 
Defy all challenges. Microsoft(R) Visual Studio 2008. 
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/