[Qemu-devel] [PATCH 3/5] Add read watchpoint support

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

This patch adds read access detection to the watchpoint subsystem. It
adds a few additional instructions to the io_read path even when
watchpoints are disabled. Me feeling is that this does not cause a
relevant slowdown and can be justified by the enhanced feature set.

But I must say that the softmmu subsystem of QEMU is not yet my home. So
parts of this patch were not really designed, but engineered ;).
Nevertheless, things work smoothly for me, no regressions found so far.

Signed-off-by: Jan Kiszka <jan.kiszka@web.de>

---
 cpu-defs.h         |    8 +++---
 exec.c             |   70 ++++++++++++++++++++++++++++-------------------------
 softmmu_template.h |   15 +++++++----
 3 files changed, 52 insertions(+), 41 deletions(-)

Index: b/exec.c
===================================================================
--- a/exec.c
+++ b/exec.c
@@ -808,9 +808,9 @@ void tb_invalidate_phys_page_range(targe
             if (current_tb_not_found) {
                 current_tb_not_found = 0;
                 current_tb = NULL;
-                if (env->mem_write_pc) {
+                if (env->iomem_access_pc) {
                     /* now we have a real cpu fault */
-                    current_tb = tb_find_pc(env->mem_write_pc);
+                    current_tb = tb_find_pc(env->iomem_access_pc);
                 }
             }
             if (current_tb == tb &&
@@ -823,7 +823,7 @@ void tb_invalidate_phys_page_range(targe
 
                 current_tb_modified = 1;
                 cpu_restore_state(current_tb, env,
-                                  env->mem_write_pc, NULL);
+                                  env->iomem_access_pc, NULL);
 #if defined(TARGET_I386)
                 current_flags = env->hflags;
                 current_flags |= (env->eflags & (IOPL_MASK | TF_MASK | VM_MASK));
@@ -855,7 +855,7 @@ void tb_invalidate_phys_page_range(targe
     if (!p->first_tb) {
         invalidate_page_bitmap(p);
         if (is_cpu_write_access) {
-            tlb_unprotect_code_phys(env, start, env->mem_write_vaddr);
+            tlb_unprotect_code_phys(env, start, env->iomem_access_vaddr);
         }
     }
 #endif
@@ -881,7 +881,7 @@ static inline void tb_invalidate_phys_pa
     if (1) {
         if (loglevel) {
             fprintf(logfile, "modifying code at 0x%x size=%d EIP=%x PC=%08x\n",
-                   cpu_single_env->mem_write_vaddr, len,
+                   cpu_single_env->iomem_access_vaddr, len,
                    cpu_single_env->eip,
                    cpu_single_env->eip + (long)cpu_single_env->segs[R_CS].base);
         }
@@ -1769,7 +1769,7 @@ int tlb_set_page_exec(CPUState *env, tar
     PhysPageDesc *p;
     unsigned long pd;
     unsigned int index;
-    target_ulong address;
+    target_ulong address, address_read;
     target_phys_addr_t addend;
     int ret;
     CPUTLBEntry *te;
@@ -1801,18 +1801,18 @@ int tlb_set_page_exec(CPUState *env, tar
             addend = (unsigned long)phys_ram_base + (pd & TARGET_PAGE_MASK);
         }
 
+        address_read = address;
         /* Make accesses to pages with watchpoints go via the
            watchpoint trap routines.  */
         for (i = 0; i < env->nb_watchpoints; i++) {
             if (vaddr == (env->watchpoint[i].vaddr & TARGET_PAGE_MASK)) {
                 if (address & ~TARGET_PAGE_MASK) {
                     env->watchpoint[i].addend = 0;
-                    address = vaddr | io_mem_watch;
+                    address_read = address = vaddr | io_mem_watch;
                 } else {
                     env->watchpoint[i].addend = pd - paddr +
                         (unsigned long) phys_ram_base;
-                    /* TODO: Figure out how to make read watchpoints coexist
-                       with code.  */
+                    address_read = vaddr | io_mem_watch;
                     pd = (pd & TARGET_PAGE_MASK) | io_mem_watch | IO_MEM_ROMD;
                 }
             }
@@ -1823,7 +1823,7 @@ int tlb_set_page_exec(CPUState *env, tar
         te = &env->tlb_table[mmu_idx][index];
         te->addend = addend;
         if (prot & PAGE_READ) {
-            te->addr_read = address;
+            te->addr_read = address_read;
         } else {
             te->addr_read = -1;
         }
@@ -2304,7 +2304,8 @@ static void notdirty_mem_writeb(void *op
     /* we remove the notdirty callback only if the code has been
        flushed */
     if (dirty_flags == 0xff)
-        tlb_set_dirty(cpu_single_env, addr, cpu_single_env->mem_write_vaddr);
+        tlb_set_dirty(cpu_single_env, addr,
+                      cpu_single_env->iomem_access_vaddr);
 }
 
 static void notdirty_mem_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
@@ -2330,7 +2331,8 @@ static void notdirty_mem_writew(void *op
     /* we remove the notdirty callback only if the code has been
        flushed */
     if (dirty_flags == 0xff)
-        tlb_set_dirty(cpu_single_env, addr, cpu_single_env->mem_write_vaddr);
+        tlb_set_dirty(cpu_single_env, addr,
+                      cpu_single_env->iomem_access_vaddr);
 }
 
 static void notdirty_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
@@ -2356,7 +2358,8 @@ static void notdirty_mem_writel(void *op
     /* we remove the notdirty callback only if the code has been
        flushed */
     if (dirty_flags == 0xff)
-        tlb_set_dirty(cpu_single_env, addr, cpu_single_env->mem_write_vaddr);
+        tlb_set_dirty(cpu_single_env, addr,
+                      cpu_single_env->iomem_access_vaddr);
 }
 
 static CPUReadMemoryFunc *error_mem_read[3] = {
@@ -2372,24 +2375,6 @@ static CPUWriteMemoryFunc *notdirty_mem_
 };
 
 #if defined(CONFIG_SOFTMMU)
-/* Watchpoint access routines.  Watchpoints are inserted using TLB tricks,
-   so these check for a hit then pass through to the normal out-of-line
-   phys routines.  */
-static uint32_t watch_mem_readb(void *opaque, target_phys_addr_t addr)
-{
-    return ldub_phys(addr);
-}
-
-static uint32_t watch_mem_readw(void *opaque, target_phys_addr_t addr)
-{
-    return lduw_phys(addr);
-}
-
-static uint32_t watch_mem_readl(void *opaque, target_phys_addr_t addr)
-{
-    return ldl_phys(addr);
-}
-
 /* 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.  */
@@ -2404,7 +2389,7 @@ static target_ulong check_watchpoint(tar
     retaddr = addr;
     for (i = 0; i < env->nb_watchpoints; i++) {
         watch = env->watchpoint[i].vaddr;
-        if (((env->mem_write_vaddr ^ watch) & TARGET_PAGE_MASK) == 0) {
+        if (((env->iomem_access_vaddr ^ watch) & TARGET_PAGE_MASK) == 0) {
             retaddr = addr - env->watchpoint[i].addend;
             if (((addr ^ watch) & (~TARGET_PAGE_MASK - (len - 1))) == 0 &&
                 (env->watchpoint[i].type == type ||
@@ -2418,6 +2403,27 @@ static target_ulong check_watchpoint(tar
     return retaddr;
 }
 
+
+/* Watchpoint access routines.  Watchpoints are inserted using TLB tricks,
+   so these check for a hit then pass through to the normal out-of-line
+   phys routines.  */
+static uint32_t watch_mem_readb(void *opaque, target_phys_addr_t addr)
+{
+    addr = check_watchpoint(addr, 1, GDB_WATCHPOINT_READ);
+    return ldub_phys(addr);
+}
+
+static uint32_t watch_mem_readw(void *opaque, target_phys_addr_t addr)
+{
+    addr = check_watchpoint(addr, 2, GDB_WATCHPOINT_READ);
+    return lduw_phys(addr);
+}
+
+static uint32_t watch_mem_readl(void *opaque, target_phys_addr_t addr)
+{
+    addr = check_watchpoint(addr, 4, GDB_WATCHPOINT_READ);
+    return ldl_phys(addr);
+}
 static void watch_mem_writeb(void *opaque, target_phys_addr_t addr,
                              uint32_t val)
 {
Index: b/cpu-defs.h
===================================================================
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -136,10 +136,10 @@ typedef struct CPUTLBEntry {
     /* in order to avoid passing too many arguments to the memory       \
        write helpers, we store some rarely used information in the CPU  \
        context) */                                                      \
-    unsigned long mem_write_pc; /* host pc at which the memory was      \
-                                   written */                           \
-    target_ulong mem_write_vaddr; /* target virtual addr at which the   \
-                                     memory was written */              \
+    unsigned long iomem_access_pc; /* host pc at which the io-memory    \
+                                      was accessed */                   \
+    target_ulong iomem_access_vaddr; /* target virtual addr at which    \
+                                        the io-memory was accessed */   \
     int halted; /* TRUE if the CPU is in suspend state */               \
     /* The meaning of the MMU modes is defined in the target code. */   \
     CPUTLBEntry tlb_table[NB_MMU_MODES][CPU_TLB_SIZE];                  \
Index: b/softmmu_template.h
===================================================================
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -51,12 +51,15 @@ static DATA_TYPE glue(glue(slow_ld, SUFF
                                                         int mmu_idx,
                                                         void *retaddr);
 static inline DATA_TYPE glue(io_read, SUFFIX)(target_phys_addr_t physaddr,
-                                              target_ulong tlb_addr)
+                                              target_ulong tlb_addr,
+                                              void *retaddr)
 {
     DATA_TYPE res;
     int index;
 
     index = (tlb_addr >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
+    env->iomem_access_vaddr = tlb_addr;
+    env->iomem_access_pc = (unsigned long)retaddr;
 #if SHIFT <= 2
     res = io_mem_read[index][SHIFT](io_mem_opaque[index], physaddr);
 #else
@@ -95,7 +98,8 @@ DATA_TYPE REGPARM glue(glue(__ld, SUFFIX
             /* IO access */
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
-            res = glue(io_read, SUFFIX)(physaddr, tlb_addr);
+            retaddr = GETPC();
+            res = glue(io_read, SUFFIX)(physaddr, tlb_addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
             /* slow unaligned access (it spans two pages or IO) */
         do_unaligned_access:
@@ -147,7 +151,8 @@ static DATA_TYPE glue(glue(slow_ld, SUFF
             /* IO access */
             if ((addr & (DATA_SIZE - 1)) != 0)
                 goto do_unaligned_access;
-            res = glue(io_read, SUFFIX)(physaddr, tlb_addr);
+            retaddr = GETPC();
+            res = glue(io_read, SUFFIX)(physaddr, tlb_addr, retaddr);
         } else if (((addr & ~TARGET_PAGE_MASK) + DATA_SIZE - 1) >= TARGET_PAGE_SIZE) {
         do_unaligned_access:
             /* slow unaligned access (it spans two pages) */
@@ -191,8 +196,8 @@ static inline void glue(io_write, SUFFIX
     int index;
 
     index = (tlb_addr >> IO_MEM_SHIFT) & (IO_MEM_NB_ENTRIES - 1);
-    env->mem_write_vaddr = tlb_addr;
-    env->mem_write_pc = (unsigned long)retaddr;
+    env->iomem_access_vaddr = tlb_addr;
+    env->iomem_access_pc = (unsigned long)retaddr;
 #if SHIFT <= 2
     io_mem_write[index][SHIFT](io_mem_opaque[index], physaddr, val);
 #else