[Qemu-devel] [PATCH 2/5] Watchpoint length and type awareness

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

Extend QEMU's memory watchpoint with length and type awareness. The
length parameter is no correctly accounted for. gdbstub is enhanced to
report the correct watchpoint type back on hits. To keep things simple,
I decided to reject unaligned watchpoints instead of mapping them on
multiple instances.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>
---
 cpu-defs.h |    2 ++
 exec.c     |   33 ++++++++++++++++++---------------
 gdbstub.c  |   16 ++++++++++++++--
 3 files changed, 34 insertions(+), 17 deletions(-)

Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -1180,13 +1180,13 @@ int cpu_watchpoint_insert(CPUState *env,
 {
     int i;
 
-    if (type != GDB_WATCHPOINT_WRITE)
-        return -ENOSYS;
-    if (len != 1 && len != 2 && len != 4)
+    /* sanity checks: allow power-of-2 lengths, deny unaligned breakpoints */
+    if ((len != 1 && len != 2 && len != 4) || (addr & (len-1)))
         return -EINVAL;
 
     for (i = 0; i < env->nb_watchpoints; i++) {
-        if (addr == env->watchpoint[i].vaddr)
+        if (addr == env->watchpoint[i].vaddr &&
+            len == env->watchpoint[i].len && type == env->watchpoint[i].type)
             return 0;
     }
     if (env->nb_watchpoints >= MAX_WATCHPOINTS)
@@ -1194,6 +1194,8 @@ int cpu_watchpoint_insert(CPUState *env,
 
     i = env->nb_watchpoints++;
     env->watchpoint[i].vaddr = addr;
+    env->watchpoint[i].len = len;
+    env->watchpoint[i].type = type;
     tlb_flush_page(env, addr);
     /* FIXME: This flush is needed because of the hack to make memory ops
        terminate the TB.  It can be removed once the proper IO trap and
@@ -1208,11 +1210,9 @@ int cpu_watchpoint_remove(CPUState *env,
 {
     int i;
 
-    if (type != GDB_WATCHPOINT_WRITE)
-        return -ENOSYS;
-
     for (i = 0; i < env->nb_watchpoints; i++) {
-        if (addr == env->watchpoint[i].vaddr) {
+        if (addr == env->watchpoint[i].vaddr &&
+            len == env->watchpoint[i].len && type == env->watchpoint[i].type) {
             env->nb_watchpoints--;
             env->watchpoint[i] = env->watchpoint[env->nb_watchpoints];
             tlb_flush_page(env, addr);
@@ -2393,7 +2393,8 @@ static uint32_t watch_mem_readl(void *op
 /* Generate a debug exception if a watchpoint has been hit.
    Returns the real physical address of the access.  addr will be a host
    address in case of a RAM location.  */
-static target_ulong check_watchpoint(target_phys_addr_t addr)
+static target_ulong check_watchpoint(target_phys_addr_t addr,
+                                     int len, int type)
 {
     CPUState *env = cpu_single_env;
     target_ulong watch;
@@ -2405,9 +2406,11 @@ static target_ulong check_watchpoint(tar
         watch = env->watchpoint[i].vaddr;
         if (((env->mem_write_vaddr ^ watch) & TARGET_PAGE_MASK) == 0) {
             retaddr = addr - env->watchpoint[i].addend;
-            if (((addr ^ watch) & ~TARGET_PAGE_MASK) == 0) {
-                cpu_single_env->watchpoint_hit = i + 1;
-                cpu_interrupt(cpu_single_env, CPU_INTERRUPT_DEBUG);
+            if (((addr ^ watch) & (~TARGET_PAGE_MASK - (len - 1))) == 0 &&
+                (env->watchpoint[i].type == type ||
+                 env->watchpoint[i].type == GDB_WATCHPOINT_ACCESS)) {
+                env->watchpoint_hit = i + 1;
+                cpu_interrupt(env, CPU_INTERRUPT_DEBUG);
                 break;
             }
         }
@@ -2418,21 +2421,21 @@ static target_ulong check_watchpoint(tar
 static void watch_mem_writeb(void *opaque, target_phys_addr_t addr,
                              uint32_t val)
 {
-    addr = check_watchpoint(addr);
+    addr = check_watchpoint(addr, 1, GDB_WATCHPOINT_WRITE);
     stb_phys(addr, val);
 }
 
 static void watch_mem_writew(void *opaque, target_phys_addr_t addr,
                              uint32_t val)
 {
-    addr = check_watchpoint(addr);
+    addr = check_watchpoint(addr, 2, GDB_WATCHPOINT_WRITE);
     stw_phys(addr, val);
 }
 
 static void watch_mem_writel(void *opaque, target_phys_addr_t addr,
                              uint32_t val)
 {
-    addr = check_watchpoint(addr);
+    addr = check_watchpoint(addr, 4, GDB_WATCHPOINT_WRITE);
     stl_phys(addr, val);
 }
 
Index: b/cpu-defs.h
===================================================================
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -156,6 +156,8 @@ typedef struct CPUTLBEntry {
     struct {                                                            \
         target_ulong vaddr;                                             \
         target_phys_addr_t addend;                                      \
+        target_ulong len;                                               \
+        int type;                                                       \
     } watchpoint[MAX_WATCHPOINTS];                                      \
     int nb_watchpoints;                                                 \
     int watchpoint_hit;                                                 \
Index: b/gdbstub.c
===================================================================
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1203,6 +1203,7 @@ static void gdb_vm_stopped(void *opaque,
 {
     GDBState *s = opaque;
     char buf[256];
+    char *type;
     int ret;
 
     if (s->state == RS_SYSCALL)
@@ -1213,8 +1214,19 @@ static void gdb_vm_stopped(void *opaque,
 
     if (reason == EXCP_DEBUG) {
         if (s->env->watchpoint_hit) {
-            snprintf(buf, sizeof(buf), "T%02xwatch:" TARGET_FMT_lx ";",
-                     SIGTRAP,
+            switch (s->env->watchpoint[s->env->watchpoint_hit - 1].type) {
+            case GDB_WATCHPOINT_READ:
+                type = "r";
+                break;
+            case GDB_WATCHPOINT_ACCESS:
+                type = "a";
+                break;
+            default:
+                type = "";
+                break;
+            }
+            snprintf(buf, sizeof(buf), "T%02x%swatch:" TARGET_FMT_lx ";",
+                     SIGTRAP, type,
                      s->env->watchpoint[s->env->watchpoint_hit - 1].vaddr);
             put_packet(s, buf);
             s->env->watchpoint_hit = 0;