[Qemu-devel] [PATCH 08/16] exec.c: Have one io_mem_watch per AddressSpace

[Peter Maydell]

The io_mem_watch MemoryRegion's read and write callbacks pass the
accesses through to an underlying address space. Now that that
might be something other than address_space_memory, we need to
pass the correct AddressSpace in via the opaque pointer. That
means we need to have one io_mem_watch MemoryRegion per address
space, rather than sharing one between all address spaces.

This should also fix gdbstub watchpoints in x86 SMRAM, which would
previously not have worked correctly.

Signed-off-by: Peter Maydell <address@hidden>
---
 exec.c                | 40 +++++++++++++++++++++++-----------------
 include/exec/memory.h |  2 ++
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/exec.c b/exec.c
index bc6ab8a..9998fa0 100644
--- a/exec.c
+++ b/exec.c
@@ -163,8 +163,6 @@ static void io_mem_init(void);
 static void memory_map_init(void);
 static void tcg_commit(MemoryListener *listener);
 
-static MemoryRegion io_mem_watch;
-
 /**
  * CPUAddressSpace: all the information a CPU needs about an AddressSpace
  * @cpu: the CPU whose AddressSpace this is
@@ -334,12 +332,6 @@ static MemoryRegionSection *phys_page_find(PhysPageEntry 
lp, hwaddr addr,
     }
 }
 
-bool memory_region_is_unassigned(MemoryRegion *mr)
-{
-    return mr != &io_mem_rom && mr != &io_mem_notdirty && !mr->rom_device
-        && mr != &io_mem_watch;
-}
-
 /* Called from RCU critical section */
 static MemoryRegionSection *address_space_lookup_region(AddressSpaceDispatch 
*d,
                                                         hwaddr addr,
@@ -2045,17 +2037,18 @@ static MemTxResult watch_mem_read(void *opaque, hwaddr 
addr, uint64_t *pdata,
 {
     MemTxResult res;
     uint64_t data;
+    AddressSpace *as = opaque;
 
     check_watchpoint(addr & ~TARGET_PAGE_MASK, size, attrs, BP_MEM_READ);
     switch (size) {
     case 1:
-        data = address_space_ldub(&address_space_memory, addr, attrs, &res);
+        data = address_space_ldub(as, addr, attrs, &res);
         break;
     case 2:
-        data = address_space_lduw(&address_space_memory, addr, attrs, &res);
+        data = address_space_lduw(as, addr, attrs, &res);
         break;
     case 4:
-        data = address_space_ldl(&address_space_memory, addr, attrs, &res);
+        data = address_space_ldl(as, addr, attrs, &res);
         break;
     default: abort();
     }
@@ -2068,17 +2061,18 @@ static MemTxResult watch_mem_write(void *opaque, hwaddr 
addr,
                                    MemTxAttrs attrs)
 {
     MemTxResult res;
+    AddressSpace *as = opaque;
 
     check_watchpoint(addr & ~TARGET_PAGE_MASK, size, attrs, BP_MEM_WRITE);
     switch (size) {
     case 1:
-        address_space_stb(&address_space_memory, addr, val, attrs, &res);
+        address_space_stb(as, addr, val, attrs, &res);
         break;
     case 2:
-        address_space_stw(&address_space_memory, addr, val, attrs, &res);
+        address_space_stw(as, addr, val, attrs, &res);
         break;
     case 4:
-        address_space_stl(&address_space_memory, addr, val, attrs, &res);
+        address_space_stl(as, addr, val, attrs, &res);
         break;
     default: abort();
     }
@@ -2091,6 +2085,15 @@ static const MemoryRegionOps watch_mem_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
+bool memory_region_is_unassigned(MemoryRegion *mr)
+{
+    /* Checking the ops pointer of the MemoryRegion is strictly
+     * speaking looking at private information of the MemoryRegion :-(
+     */
+    return mr != &io_mem_rom && mr != &io_mem_notdirty && !mr->rom_device
+        && mr->ops != &watch_mem_ops;
+}
+
 static MemTxResult subpage_read(void *opaque, hwaddr addr, uint64_t *data,
                                 unsigned len, MemTxAttrs attrs)
 {
@@ -2251,8 +2254,6 @@ static void io_mem_init(void)
                           NULL, UINT64_MAX);
     memory_region_init_io(&io_mem_notdirty, NULL, &notdirty_mem_ops, NULL,
                           NULL, UINT64_MAX);
-    memory_region_init_io(&io_mem_watch, NULL, &watch_mem_ops, NULL,
-                          NULL, UINT64_MAX);
 }
 
 static void mem_begin(MemoryListener *listener)
@@ -2267,7 +2268,7 @@ static void mem_begin(MemoryListener *listener)
     assert(n == PHYS_SECTION_NOTDIRTY);
     n = dummy_section(&d->map, as, &io_mem_rom);
     assert(n == PHYS_SECTION_ROM);
-    n = dummy_section(&d->map, as, &io_mem_watch);
+    n = dummy_section(&d->map, as, as->io_mem_watch);
     assert(n == PHYS_SECTION_WATCH);
 
     d->phys_map  = (PhysPageEntry) { .ptr = PHYS_MAP_NODE_NIL, .skip = 1 };
@@ -2315,6 +2316,10 @@ static void tcg_commit(MemoryListener *listener)
 
 void address_space_init_dispatch(AddressSpace *as)
 {
+    as->io_mem_watch = g_new0(MemoryRegion, 1);
+    memory_region_init_io(as->io_mem_watch, NULL, &watch_mem_ops, as,
+                          NULL, UINT64_MAX);
+
     as->dispatch = NULL;
     as->dispatch_listener = (MemoryListener) {
         .begin = mem_begin,
@@ -2339,6 +2344,7 @@ void address_space_destroy_dispatch(AddressSpace *as)
     if (d) {
         call_rcu(d, address_space_dispatch_free, rcu);
     }
+    memory_region_unref(as->io_mem_watch);
 }
 
 static void memory_map_init(void)
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 0f07159..e5d98f8 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -246,6 +246,8 @@ struct AddressSpace {
     struct AddressSpaceDispatch *next_dispatch;
     MemoryListener dispatch_listener;
 
+    MemoryRegion *io_mem_watch;
+
     QTAILQ_ENTRY(AddressSpace) address_spaces_link;
 };
 
-- 
1.9.1