[Qemu-devel] [PATCH 00/10] Remove AddressSpaceOps

[Qemu-devel] [PATCH 00/10] Remove AddressSpaceOps

[Avi Kivity]

This patchset makes the memory core (memory.c) talk to the backend (in exec.c)
via a MemoryListener instead of named functions.

While the motivation for this is to simplify the memory core, it also enables
optimizing accelerators some more (by having a tcg MemoryListener to do tcg
specific core) and allows unit testing of memory.c (by adding a testing
MemoryListener and seeing what it outputs as various inputs are fed into the
core).

Avi Kivity (10):
  ioport: change portio_list not to use memory_region_set_offset()
  memory: remove memory_region_set_offset()
  memory: add shorthand for invoking a callback on all listeners
  memory: switch memory listeners to a QTAILQ
  memory: code motion: move MEMORY_LISTENER_CALL()
  memory: move ioeventfd ops to MemoryListener
  memory: add a readonly attribute to MemoryRegionSection
  memory: don't pass ->readable attribute to
    cpu_register_physical_memory_log
  memory: use a MemoryListener for core memory map updates too
  memory: drop AddressSpaceOps

 exec-obsolete.h |    5 +-
 exec.c          |   77 +++++++++++++++-
 hw/vhost.c      |   15 +++
 ioport.c        |   25 ++++-
 ioport.h        |    1 +
 kvm-all.c       |   79 +++++++++++++++
 memory.c        |  288 ++++++++++++++++++-------------------------------------
 memory.h        |   19 ++--
 xen-all.c       |   15 +++
 9 files changed, 310 insertions(+), 214 deletions(-)

-- 
1.7.9

[Qemu-devel] [PATCH 01/10] ioport: change portio_list not to use memory_region_set_offset()

[Avi Kivity]

memory_region_set_offset() will be going away soon, so don't use it.
Use an alias instead.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 ioport.c |   25 +++++++++++++++++++------
 ioport.h |    1 +
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/ioport.c b/ioport.c
index 36fa3a4..505b252 100644
--- a/ioport.c
+++ b/ioport.c
@@ -328,6 +328,7 @@ void portio_list_init(PortioList *piolist,
     piolist->ports = callbacks;
     piolist->nr = 0;
     piolist->regions = g_new0(MemoryRegion *, n);
+    piolist->aliases = g_new0(MemoryRegion *, n);
     piolist->address_space = NULL;
     piolist->opaque = opaque;
     piolist->name = name;
@@ -336,6 +337,7 @@ void portio_list_init(PortioList *piolist,
 void portio_list_destroy(PortioList *piolist)
 {
     g_free(piolist->regions);
+    g_free(piolist->aliases);
 }
 
 static void portio_list_add_1(PortioList *piolist,
@@ -345,7 +347,7 @@ static void portio_list_add_1(PortioList *piolist,
 {
     MemoryRegionPortio *pio;
     MemoryRegionOps *ops;
-    MemoryRegion *region;
+    MemoryRegion *region, *alias;
     unsigned i;
 
     /* Copy the sub-list and null-terminate it.  */
@@ -362,12 +364,19 @@ static void portio_list_add_1(PortioList *piolist,
     ops->old_portio = pio;
 
     region = g_new(MemoryRegion, 1);
+    alias = g_new(MemoryRegion, 1);
+    /*
+     * Use an alias so that the callback is called with an absolute address,
+     * rather than an offset relative to to start + off_low.
+     */
     memory_region_init_io(region, ops, piolist->opaque, piolist->name,
-                          off_high - off_low);
-    memory_region_set_offset(region, start + off_low);
+                          UINT64_MAX);
+    memory_region_init_alias(alias, piolist->name,
+                             region, start + off_low, off_high - off_low);
     memory_region_add_subregion(piolist->address_space,
-                                start + off_low, region);
+                                start + off_low, alias);
     piolist->regions[piolist->nr++] = region;
+    piolist->aliases[piolist->nr++] = alias;
 }
 
 void portio_list_add(PortioList *piolist,
@@ -409,15 +418,19 @@ void portio_list_add(PortioList *piolist,
 
 void portio_list_del(PortioList *piolist)
 {
-    MemoryRegion *mr;
+    MemoryRegion *mr, *alias;
     unsigned i;
 
     for (i = 0; i < piolist->nr; ++i) {
         mr = piolist->regions[i];
-        memory_region_del_subregion(piolist->address_space, mr);
+        alias = piolist->aliases[i];
+        memory_region_del_subregion(piolist->address_space, alias);
+        memory_region_destroy(alias);
         memory_region_destroy(mr);
         g_free((MemoryRegionOps *)mr->ops);
         g_free(mr);
+        g_free(alias);
         piolist->regions[i] = NULL;
+        piolist->aliases[i] = NULL;
     }
 }
diff --git a/ioport.h b/ioport.h
index ae3e9da..ab29c89 100644
--- a/ioport.h
+++ b/ioport.h
@@ -60,6 +60,7 @@ typedef struct PortioList {
     struct MemoryRegion *address_space;
     unsigned nr;
     struct MemoryRegion **regions;
+    struct MemoryRegion **aliases;
     void *opaque;
     const char *name;
 } PortioList;
-- 
1.7.9

[Qemu-devel] [PATCH 02/10] memory: remove memory_region_set_offset()

[Avi Kivity]

memory_region_set_offset() complicates the API, and has been deprecated
since its introduction.  Now that it is no longer used, remove it.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 memory.c |   26 ++++++++++----------------
 memory.h |    9 ---------
 2 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/memory.c b/memory.c
index 5e77d8a..382dded 100644
--- a/memory.c
+++ b/memory.c
@@ -401,17 +401,17 @@ static void memory_region_iorange_read(IORange *iorange,
 
         *data = ((uint64_t)1 << (width * 8)) - 1;
         if (mrp) {
-            *data = mrp->read(mr->opaque, offset + mr->offset);
+            *data = mrp->read(mr->opaque, offset);
         } else if (width == 2) {
             mrp = find_portio(mr, offset, 1, false);
             assert(mrp);
-            *data = mrp->read(mr->opaque, offset + mr->offset) |
-                    (mrp->read(mr->opaque, offset + mr->offset + 1) << 8);
+            *data = mrp->read(mr->opaque, offset) |
+                    (mrp->read(mr->opaque, offset + 1) << 8);
         }
         return;
     }
     *data = 0;
-    access_with_adjusted_size(offset + mr->offset, data, width,
+    access_with_adjusted_size(offset, data, width,
                               mr->ops->impl.min_access_size,
                               mr->ops->impl.max_access_size,
                               memory_region_read_accessor, mr);
@@ -428,16 +428,16 @@ static void memory_region_iorange_write(IORange *iorange,
         const MemoryRegionPortio *mrp = find_portio(mr, offset, width, true);
 
         if (mrp) {
-            mrp->write(mr->opaque, offset + mr->offset, data);
+            mrp->write(mr->opaque, offset, data);
         } else if (width == 2) {
             mrp = find_portio(mr, offset, 1, false);
             assert(mrp);
-            mrp->write(mr->opaque, offset + mr->offset, data & 0xff);
-            mrp->write(mr->opaque, offset + mr->offset + 1, data >> 8);
+            mrp->write(mr->opaque, offset, data & 0xff);
+            mrp->write(mr->opaque, offset + 1, data >> 8);
         }
         return;
     }
-    access_with_adjusted_size(offset + mr->offset, &data, width,
+    access_with_adjusted_size(offset, &data, width,
                               mr->ops->impl.min_access_size,
                               mr->ops->impl.max_access_size,
                               memory_region_write_accessor, mr);
@@ -863,7 +863,6 @@ void memory_region_init(MemoryRegion *mr,
         mr->size = int128_2_64();
     }
     mr->addr = 0;
-    mr->offset = 0;
     mr->subpage = false;
     mr->enabled = true;
     mr->terminates = false;
@@ -925,7 +924,7 @@ static uint64_t memory_region_dispatch_read1(MemoryRegion *mr,
     }
 
     /* FIXME: support unaligned access */
-    access_with_adjusted_size(addr + mr->offset, &data, size,
+    access_with_adjusted_size(addr, &data, size,
                               mr->ops->impl.min_access_size,
                               mr->ops->impl.max_access_size,
                               memory_region_read_accessor, mr);
@@ -979,7 +978,7 @@ static void memory_region_dispatch_write(MemoryRegion *mr,
     }
 
     /* FIXME: support unaligned access */
-    access_with_adjusted_size(addr + mr->offset, &data, size,
+    access_with_adjusted_size(addr, &data, size,
                               mr->ops->impl.min_access_size,
                               mr->ops->impl.max_access_size,
                               memory_region_write_accessor, mr);
@@ -1122,11 +1121,6 @@ bool memory_region_is_rom(MemoryRegion *mr)
     return mr->ram && mr->readonly;
 }
 
-void memory_region_set_offset(MemoryRegion *mr, target_phys_addr_t offset)
-{
-    mr->offset = offset;
-}
-
 void memory_region_set_log(MemoryRegion *mr, bool log, unsigned client)
 {
     uint8_t mask = 1 << client;
diff --git a/memory.h b/memory.h
index 4cf8d2f..4763286 100644
--- a/memory.h
+++ b/memory.h
@@ -115,7 +115,6 @@ struct MemoryRegion {
     MemoryRegion *parent;
     Int128 size;
     target_phys_addr_t addr;
-    target_phys_addr_t offset;
     void (*destructor)(MemoryRegion *mr);
     ram_addr_t ram_addr;
     IORange iorange;
@@ -359,14 +358,6 @@ bool memory_region_is_rom(MemoryRegion *mr);
 void *memory_region_get_ram_ptr(MemoryRegion *mr);
 
 /**
- * memory_region_set_offset: Sets an offset to be added to MemoryRegionOps
- *                           callbacks.
- *
- * This function is deprecated and should not be used in new code.
- */
-void memory_region_set_offset(MemoryRegion *mr, target_phys_addr_t offset);
-
-/**
  * memory_region_set_log: Turn dirty logging on or off for a region.
  *
  * Turns dirty logging on or off for a specified client (display, migration).
-- 
1.7.9

[Qemu-devel] [PATCH 03/10] memory: add shorthand for invoking a callback on all listeners

[Avi Kivity]

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 memory.c |   54 +++++++++++++++++++-----------------------------------
 1 files changed, 19 insertions(+), 35 deletions(-)

diff --git a/memory.c b/memory.c
index 382dded..6afe414 100644
--- a/memory.c
+++ b/memory.c
@@ -678,31 +678,23 @@ static void address_space_update_ioeventfds(AddressSpace *as)
     as->ioeventfd_nb = ioeventfd_nb;
 }
 
-typedef void ListenerCallback(MemoryListener *listener,
-                              MemoryRegionSection *mrs);
-
-/* Want "void (&MemoryListener::*callback)(const MemoryRegionSection& s)" */
-static void memory_listener_update_region(FlatRange *fr, AddressSpace *as,
-                                          size_t callback_offset)
-{
-    MemoryRegionSection section = {
-        .mr = fr->mr,
-        .address_space = as->root,
-        .offset_within_region = fr->offset_in_region,
-        .size = int128_get64(fr->addr.size),
-        .offset_within_address_space = int128_get64(fr->addr.start),
-    };
-    MemoryListener *listener;
-
-    QLIST_FOREACH(listener, &memory_listeners, link) {
-        ListenerCallback *callback
-            = *(ListenerCallback **)((void *)listener + callback_offset);
-        callback(listener, &section);
-    }
-}
-
-#define MEMORY_LISTENER_UPDATE_REGION(fr, as, callback) \
-    memory_listener_update_region(fr, as, offsetof(MemoryListener, callback))
+#define MEMORY_LISTENER_CALL(_callback, _args...)               \
+    do {                                                        \
+        MemoryListener *_listener;                              \
+                                                                \
+        QLIST_FOREACH(_listener, &memory_listeners, link) {     \
+            _listener->_callback(_listener, ##_args);           \
+        }                                                       \
+    } while (0)
+
+#define MEMORY_LISTENER_UPDATE_REGION(fr, as, callback)                 \
+    MEMORY_LISTENER_CALL(callback, &(MemoryRegionSection) {             \
+        .mr = (fr)->mr,                                                 \
+        .address_space = (as)->root,                                    \
+        .offset_within_region = (fr)->offset_in_region,                 \
+        .size = int128_get64((fr)->addr.size),                          \
+        .offset_within_address_space = int128_get64((fr)->addr.start),  \
+                })
 
 static void address_space_update_topology_pass(AddressSpace *as,
                                                FlatView old_view,
@@ -1483,23 +1475,15 @@ void memory_global_sync_dirty_bitmap(MemoryRegion *address_space)
 
 void memory_global_dirty_log_start(void)
 {
-    MemoryListener *listener;
-
     cpu_physical_memory_set_dirty_tracking(1);
     global_dirty_log = true;
-    QLIST_FOREACH(listener, &memory_listeners, link) {
-        listener->log_global_start(listener);
-    }
+    MEMORY_LISTENER_CALL(log_global_start);
 }
 
 void memory_global_dirty_log_stop(void)
 {
-    MemoryListener *listener;
-
     global_dirty_log = false;
-    QLIST_FOREACH(listener, &memory_listeners, link) {
-        listener->log_global_stop(listener);
-    }
+    MEMORY_LISTENER_CALL(log_global_stop);
     cpu_physical_memory_set_dirty_tracking(0);
 }
 
-- 
1.7.9

[Qemu-devel] [PATCH 04/10] memory: switch memory listeners to a QTAILQ

[Avi Kivity]

This allows reverse iteration, which in turns allows consistent ordering
among multiple listeners:

  l1->add
  l2->add
  l2->del
  l1->del

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/vhost.c |    1 +
 kvm-all.c  |    1 +
 memory.c   |   70 ++++++++++++++++++++++++++++++++++++++++++------------------
 memory.h   |    4 ++-
 xen-all.c  |    1 +
 5 files changed, 55 insertions(+), 22 deletions(-)

diff --git a/hw/vhost.c b/hw/vhost.c
index 5ece659..4737145 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -751,6 +751,7 @@ int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force)
         .log_sync = vhost_log_sync,
         .log_global_start = vhost_log_global_start,
         .log_global_stop = vhost_log_global_stop,
+        .priority = 10
     };
     hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions));
     hdev->n_mem_sections = 0;
diff --git a/kvm-all.c b/kvm-all.c
index 0b87658..6835fd4 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -726,6 +726,7 @@ static void kvm_log_global_stop(struct MemoryListener *listener)
     .log_sync = kvm_log_sync,
     .log_global_start = kvm_log_global_start,
     .log_global_stop = kvm_log_global_stop,
+    .priority = 10,
 };
 
 static void kvm_handle_interrupt(CPUState *env, int mask)
diff --git a/memory.c b/memory.c
index 6afe414..cb2b4f1 100644
--- a/memory.c
+++ b/memory.c
@@ -27,8 +27,8 @@
 static bool memory_region_update_pending = false;
 static bool global_dirty_log = false;
 
-static QLIST_HEAD(, MemoryListener) memory_listeners
-    = QLIST_HEAD_INITIALIZER(memory_listeners);
+static QTAILQ_HEAD(memory_listeners, MemoryListener) memory_listeners
+    = QTAILQ_HEAD_INITIALIZER(memory_listeners);
 
 typedef struct AddrRange AddrRange;
 
@@ -678,17 +678,31 @@ static void address_space_update_ioeventfds(AddressSpace *as)
     as->ioeventfd_nb = ioeventfd_nb;
 }
 
-#define MEMORY_LISTENER_CALL(_callback, _args...)               \
-    do {                                                        \
-        MemoryListener *_listener;                              \
-                                                                \
-        QLIST_FOREACH(_listener, &memory_listeners, link) {     \
-            _listener->_callback(_listener, ##_args);           \
-        }                                                       \
+enum ListenerDirection { Forward, Reverse };
+
+#define MEMORY_LISTENER_CALL(_callback, _direction, _args...)           \
+    do {                                                                \
+        MemoryListener *_listener;                                      \
+                                                                        \
+        switch (_direction) {                                           \
+        case Forward:                                                   \
+            QTAILQ_FOREACH(_listener, &memory_listeners, link) {        \
+                _listener->_callback(_listener, ##_args);               \
+            }                                                           \
+            break;                                                      \
+        case Reverse:                                                   \
+            QTAILQ_FOREACH_REVERSE(_listener, &memory_listeners,        \
+                                   memory_listeners, link) {            \
+                _listener->_callback(_listener, ##_args);               \
+            }                                                           \
+            break;                                                      \
+        default:                                                        \
+            abort();                                                    \
+        }                                                               \
     } while (0)
 
-#define MEMORY_LISTENER_UPDATE_REGION(fr, as, callback)                 \
-    MEMORY_LISTENER_CALL(callback, &(MemoryRegionSection) {             \
+#define MEMORY_LISTENER_UPDATE_REGION(fr, as, dir, callback)            \
+    MEMORY_LISTENER_CALL(callback, dir, &(MemoryRegionSection) {        \
         .mr = (fr)->mr,                                                 \
         .address_space = (as)->root,                                    \
         .offset_within_region = (fr)->offset_in_region,                 \
@@ -728,7 +742,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
             /* In old, but (not in new, or in new but attributes changed). */
 
             if (!adding) {
-                MEMORY_LISTENER_UPDATE_REGION(frold, as, region_del);
+                MEMORY_LISTENER_UPDATE_REGION(frold, as, Reverse, region_del);
                 as->ops->range_del(as, frold);
             }
 
@@ -738,11 +752,11 @@ static void address_space_update_topology_pass(AddressSpace *as,
 
             if (adding) {
                 if (frold->dirty_log_mask && !frnew->dirty_log_mask) {
-                    MEMORY_LISTENER_UPDATE_REGION(frnew, as, log_stop);
+                    MEMORY_LISTENER_UPDATE_REGION(frnew, as, Reverse, log_stop);
                     as->ops->log_stop(as, frnew);
                 } else if (frnew->dirty_log_mask && !frold->dirty_log_mask) {
                     as->ops->log_start(as, frnew);
-                    MEMORY_LISTENER_UPDATE_REGION(frnew, as, log_start);
+                    MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, log_start);
                 }
             }
 
@@ -753,7 +767,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
 
             if (adding) {
                 as->ops->range_add(as, frnew);
-                MEMORY_LISTENER_UPDATE_REGION(frnew, as, region_add);
+                MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, region_add);
             }
 
             ++inew;
@@ -1142,7 +1156,8 @@ void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
 
     FOR_EACH_FLAT_RANGE(fr, &address_space_memory.current_map) {
         if (fr->mr == mr) {
-            MEMORY_LISTENER_UPDATE_REGION(fr, &address_space_memory, log_sync);
+            MEMORY_LISTENER_UPDATE_REGION(fr, &address_space_memory,
+                                          Forward, log_sync);
         }
     }
 }
@@ -1469,7 +1484,7 @@ void memory_global_sync_dirty_bitmap(MemoryRegion *address_space)
     FlatRange *fr;
 
     FOR_EACH_FLAT_RANGE(fr, &as->current_map) {
-        MEMORY_LISTENER_UPDATE_REGION(fr, as, log_sync);
+        MEMORY_LISTENER_UPDATE_REGION(fr, as, Forward, log_sync);
     }
 }
 
@@ -1477,13 +1492,13 @@ void memory_global_dirty_log_start(void)
 {
     cpu_physical_memory_set_dirty_tracking(1);
     global_dirty_log = true;
-    MEMORY_LISTENER_CALL(log_global_start);
+    MEMORY_LISTENER_CALL(log_global_start, Forward);
 }
 
 void memory_global_dirty_log_stop(void)
 {
     global_dirty_log = false;
-    MEMORY_LISTENER_CALL(log_global_stop);
+    MEMORY_LISTENER_CALL(log_global_stop, Reverse);
     cpu_physical_memory_set_dirty_tracking(0);
 }
 
@@ -1509,14 +1524,27 @@ static void listener_add_address_space(MemoryListener *listener,
 
 void memory_listener_register(MemoryListener *listener)
 {
-    QLIST_INSERT_HEAD(&memory_listeners, listener, link);
+    MemoryListener *other = NULL;
+
+    if (QTAILQ_EMPTY(&memory_listeners)
+        || listener->priority >= QTAILQ_LAST(&memory_listeners,
+                                             memory_listeners)->priority) {
+        QTAILQ_INSERT_TAIL(&memory_listeners, listener, link);
+    } else {
+        QTAILQ_FOREACH(other, &memory_listeners, link) {
+            if (listener->priority < other->priority) {
+                break;
+            }
+        }
+        QTAILQ_INSERT_BEFORE(other, listener, link);
+    }
     listener_add_address_space(listener, &address_space_memory);
     listener_add_address_space(listener, &address_space_io);
 }
 
 void memory_listener_unregister(MemoryListener *listener)
 {
-    QLIST_REMOVE(listener, link);
+    QTAILQ_REMOVE(&memory_listeners, listener, link);
 }
 
 void set_system_memory_map(MemoryRegion *mr)
diff --git a/memory.h b/memory.h
index 4763286..954dc86 100644
--- a/memory.h
+++ b/memory.h
@@ -185,7 +185,9 @@ struct MemoryListener {
     void (*log_sync)(MemoryListener *listener, MemoryRegionSection *section);
     void (*log_global_start)(MemoryListener *listener);
     void (*log_global_stop)(MemoryListener *listener);
-    QLIST_ENTRY(MemoryListener) link;
+    /* Lower = earlier (during add), later (during del) */
+    unsigned priority;
+    QTAILQ_ENTRY(MemoryListener) link;
 };
 
 /**
diff --git a/xen-all.c b/xen-all.c
index fd39168..8cb84ef 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -495,6 +495,7 @@ static void xen_log_global_stop(MemoryListener *listener)
     .log_sync = xen_log_sync,
     .log_global_start = xen_log_global_start,
     .log_global_stop = xen_log_global_stop,
+    .priority = 10,
 };
 
 /* VCPU Operations, MMIO, IO ring ... */
-- 
1.7.9

[Qemu-devel] [PATCH 05/10] memory: code motion: move MEMORY_LISTENER_CALL()

[Avi Kivity]

So it can be used in earlier code.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 memory.c |   64 +++++++++++++++++++++++++++++++-------------------------------
 1 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/memory.c b/memory.c
index cb2b4f1..a1013bc 100644
--- a/memory.c
+++ b/memory.c
@@ -82,6 +82,38 @@ static AddrRange addrrange_intersection(AddrRange r1, AddrRange r2)
     return addrrange_make(start, int128_sub(end, start));
 }
 
+enum ListenerDirection { Forward, Reverse };
+
+#define MEMORY_LISTENER_CALL(_callback, _direction, _args...)           \
+    do {                                                                \
+        MemoryListener *_listener;                                      \
+                                                                        \
+        switch (_direction) {                                           \
+        case Forward:                                                   \
+            QTAILQ_FOREACH(_listener, &memory_listeners, link) {        \
+                _listener->_callback(_listener, ##_args);               \
+            }                                                           \
+            break;                                                      \
+        case Reverse:                                                   \
+            QTAILQ_FOREACH_REVERSE(_listener, &memory_listeners,        \
+                                   memory_listeners, link) {            \
+                _listener->_callback(_listener, ##_args);               \
+            }                                                           \
+            break;                                                      \
+        default:                                                        \
+            abort();                                                    \
+        }                                                               \
+    } while (0)
+
+#define MEMORY_LISTENER_UPDATE_REGION(fr, as, dir, callback)            \
+    MEMORY_LISTENER_CALL(callback, dir, &(MemoryRegionSection) {        \
+        .mr = (fr)->mr,                                                 \
+        .address_space = (as)->root,                                    \
+        .offset_within_region = (fr)->offset_in_region,                 \
+        .size = int128_get64((fr)->addr.size),                          \
+        .offset_within_address_space = int128_get64((fr)->addr.start),  \
+                })
+
 struct CoalescedMemoryRange {
     AddrRange addr;
     QTAILQ_ENTRY(CoalescedMemoryRange) link;
@@ -678,38 +710,6 @@ static void address_space_update_ioeventfds(AddressSpace *as)
     as->ioeventfd_nb = ioeventfd_nb;
 }
 
-enum ListenerDirection { Forward, Reverse };
-
-#define MEMORY_LISTENER_CALL(_callback, _direction, _args...)           \
-    do {                                                                \
-        MemoryListener *_listener;                                      \
-                                                                        \
-        switch (_direction) {                                           \
-        case Forward:                                                   \
-            QTAILQ_FOREACH(_listener, &memory_listeners, link) {        \
-                _listener->_callback(_listener, ##_args);               \
-            }                                                           \
-            break;                                                      \
-        case Reverse:                                                   \
-            QTAILQ_FOREACH_REVERSE(_listener, &memory_listeners,        \
-                                   memory_listeners, link) {            \
-                _listener->_callback(_listener, ##_args);               \
-            }                                                           \
-            break;                                                      \
-        default:                                                        \
-            abort();                                                    \
-        }                                                               \
-    } while (0)
-
-#define MEMORY_LISTENER_UPDATE_REGION(fr, as, dir, callback)            \
-    MEMORY_LISTENER_CALL(callback, dir, &(MemoryRegionSection) {        \
-        .mr = (fr)->mr,                                                 \
-        .address_space = (as)->root,                                    \
-        .offset_within_region = (fr)->offset_in_region,                 \
-        .size = int128_get64((fr)->addr.size),                          \
-        .offset_within_address_space = int128_get64((fr)->addr.start),  \
-                })
-
 static void address_space_update_topology_pass(AddressSpace *as,
                                                FlatView old_view,
                                                FlatView new_view,
-- 
1.7.9

[Qemu-devel] [PATCH 06/10] memory: move ioeventfd ops to MemoryListener

[Avi Kivity]

This way the accelerator (kvm) can handle them directly.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/vhost.c |   14 ++++++++++
 kvm-all.c  |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory.c   |   74 ++++++++++++++-------------------------------------------
 memory.h   |    4 +++
 xen-all.c  |   14 ++++++++++
 5 files changed, 128 insertions(+), 56 deletions(-)

diff --git a/hw/vhost.c b/hw/vhost.c
index 4737145..e1e7e01 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -720,6 +720,18 @@ static void vhost_virtqueue_cleanup(struct vhost_dev *dev,
                               0, virtio_queue_get_desc_size(vdev, idx));
 }
 
+static void vhost_eventfd_add(MemoryListener *listener,
+                              MemoryRegionSection *section,
+                              bool match_data, uint64_t data, int fd)
+{
+}
+
+static void vhost_eventfd_del(MemoryListener *listener,
+                              MemoryRegionSection *section,
+                              bool match_data, uint64_t data, int fd)
+{
+}
+
 int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force)
 {
     uint64_t features;
@@ -751,6 +763,8 @@ int vhost_dev_init(struct vhost_dev *hdev, int devfd, bool force)
         .log_sync = vhost_log_sync,
         .log_global_start = vhost_log_global_start,
         .log_global_stop = vhost_log_global_stop,
+        .eventfd_add = vhost_eventfd_add,
+        .eventfd_del = vhost_eventfd_del,
         .priority = 10
     };
     hdev->mem = g_malloc0(offsetof(struct vhost_memory, regions));
diff --git a/kvm-all.c b/kvm-all.c
index 6835fd4..a05e591 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -28,6 +28,7 @@
 #include "kvm.h"
 #include "bswap.h"
 #include "memory.h"
+#include "exec-memory.h"
 
 /* This check must be after config-host.h is included */
 #ifdef CONFIG_EVENTFD
@@ -718,6 +719,81 @@ static void kvm_log_global_stop(struct MemoryListener *listener)
     assert(r >= 0);
 }
 
+static void kvm_mem_ioeventfd_add(MemoryRegionSection *section,
+                                  bool match_data, uint64_t data, int fd)
+{
+    int r;
+
+    assert(match_data && section->size == 4);
+
+    r = kvm_set_ioeventfd_mmio_long(fd, section->offset_within_address_space,
+                                    data, true);
+    if (r < 0) {
+        abort();
+    }
+}
+
+static void kvm_mem_ioeventfd_del(MemoryRegionSection *section,
+                                  bool match_data, uint64_t data, int fd)
+{
+    int r;
+
+    r = kvm_set_ioeventfd_mmio_long(fd, section->offset_within_address_space,
+                                    data, false);
+    if (r < 0) {
+        abort();
+    }
+}
+
+static void kvm_io_ioeventfd_add(MemoryRegionSection *section,
+                                 bool match_data, uint64_t data, int fd)
+{
+    int r;
+
+    assert(match_data && section->size == 2);
+
+    r = kvm_set_ioeventfd_pio_word(fd, section->offset_within_address_space,
+                                   data, true);
+    if (r < 0) {
+        abort();
+    }
+}
+
+static void kvm_io_ioeventfd_del(MemoryRegionSection *section,
+                                 bool match_data, uint64_t data, int fd)
+
+{
+    int r;
+
+    r = kvm_set_ioeventfd_pio_word(fd, section->offset_within_address_space,
+                                   data, false);
+    if (r < 0) {
+        abort();
+    }
+}
+
+static void kvm_eventfd_add(MemoryListener *listener,
+                            MemoryRegionSection *section,
+                            bool match_data, uint64_t data, int fd)
+{
+    if (section->address_space == get_system_memory()) {
+        kvm_mem_ioeventfd_add(section, match_data, data, fd);
+    } else {
+        kvm_io_ioeventfd_add(section, match_data, data, fd);
+    }
+}
+
+static void kvm_eventfd_del(MemoryListener *listener,
+                            MemoryRegionSection *section,
+                            bool match_data, uint64_t data, int fd)
+{
+    if (section->address_space == get_system_memory()) {
+        kvm_mem_ioeventfd_del(section, match_data, data, fd);
+    } else {
+        kvm_io_ioeventfd_del(section, match_data, data, fd);
+    }
+}
+
 static MemoryListener kvm_memory_listener = {
     .region_add = kvm_region_add,
     .region_del = kvm_region_del,
@@ -726,6 +802,8 @@ static void kvm_log_global_stop(struct MemoryListener *listener)
     .log_sync = kvm_log_sync,
     .log_global_start = kvm_log_global_start,
     .log_global_stop = kvm_log_global_stop,
+    .eventfd_add = kvm_eventfd_add,
+    .eventfd_del = kvm_eventfd_del,
     .priority = 10,
 };
 
diff --git a/memory.c b/memory.c
index a1013bc..e1a31d4 100644
--- a/memory.c
+++ b/memory.c
@@ -202,8 +202,6 @@ struct AddressSpaceOps {
     void (*range_del)(AddressSpace *as, FlatRange *fr);
     void (*log_start)(AddressSpace *as, FlatRange *fr);
     void (*log_stop)(AddressSpace *as, FlatRange *fr);
-    void (*ioeventfd_add)(AddressSpace *as, MemoryRegionIoeventfd *fd);
-    void (*ioeventfd_del)(AddressSpace *as, MemoryRegionIoeventfd *fd);
 };
 
 #define FOR_EACH_FLAT_RANGE(var, view)          \
@@ -369,37 +367,11 @@ static void as_memory_log_stop(AddressSpace *as, FlatRange *fr)
 {
 }
 
-static void as_memory_ioeventfd_add(AddressSpace *as, MemoryRegionIoeventfd *fd)
-{
-    int r;
-
-    assert(fd->match_data && int128_get64(fd->addr.size) == 4);
-
-    r = kvm_set_ioeventfd_mmio_long(fd->fd, int128_get64(fd->addr.start),
-                                    fd->data, true);
-    if (r < 0) {
-        abort();
-    }
-}
-
-static void as_memory_ioeventfd_del(AddressSpace *as, MemoryRegionIoeventfd *fd)
-{
-    int r;
-
-    r = kvm_set_ioeventfd_mmio_long(fd->fd, int128_get64(fd->addr.start),
-                                    fd->data, false);
-    if (r < 0) {
-        abort();
-    }
-}
-
 static const AddressSpaceOps address_space_ops_memory = {
     .range_add = as_memory_range_add,
     .range_del = as_memory_range_del,
     .log_start = as_memory_log_start,
     .log_stop = as_memory_log_stop,
-    .ioeventfd_add = as_memory_ioeventfd_add,
-    .ioeventfd_del = as_memory_ioeventfd_del,
 };
 
 static AddressSpace address_space_memory = {
@@ -493,35 +465,9 @@ static void as_io_range_del(AddressSpace *as, FlatRange *fr)
                         int128_get64(fr->addr.size));
 }
 
-static void as_io_ioeventfd_add(AddressSpace *as, MemoryRegionIoeventfd *fd)
-{
-    int r;
-
-    assert(fd->match_data && int128_get64(fd->addr.size) == 2);
-
-    r = kvm_set_ioeventfd_pio_word(fd->fd, int128_get64(fd->addr.start),
-                                   fd->data, true);
-    if (r < 0) {
-        abort();
-    }
-}
-
-static void as_io_ioeventfd_del(AddressSpace *as, MemoryRegionIoeventfd *fd)
-{
-    int r;
-
-    r = kvm_set_ioeventfd_pio_word(fd->fd, int128_get64(fd->addr.start),
-                                   fd->data, false);
-    if (r < 0) {
-        abort();
-    }
-}
-
 static const AddressSpaceOps address_space_ops_io = {
     .range_add = as_io_range_add,
     .range_del = as_io_range_del,
-    .ioeventfd_add = as_io_ioeventfd_add,
-    .ioeventfd_del = as_io_ioeventfd_del,
 };
 
 static AddressSpace address_space_io = {
@@ -653,6 +599,8 @@ static void address_space_add_del_ioeventfds(AddressSpace *as,
                                              unsigned fds_old_nb)
 {
     unsigned iold, inew;
+    MemoryRegionIoeventfd *fd;
+    MemoryRegionSection section;
 
     /* Generate a symmetric difference of the old and new fd sets, adding
      * and deleting as necessary.
@@ -664,13 +612,27 @@ static void address_space_add_del_ioeventfds(AddressSpace *as,
             && (inew == fds_new_nb
                 || memory_region_ioeventfd_before(fds_old[iold],
                                                   fds_new[inew]))) {
-            as->ops->ioeventfd_del(as, &fds_old[iold]);
+            fd = &fds_old[iold];
+            section = (MemoryRegionSection) {
+                .address_space = as->root,
+                .offset_within_address_space = int128_get64(fd->addr.start),
+                .size = int128_get64(fd->addr.size),
+            };
+            MEMORY_LISTENER_CALL(eventfd_del, Forward, &section,
+                                 fd->match_data, fd->data, fd->fd);
             ++iold;
         } else if (inew < fds_new_nb
                    && (iold == fds_old_nb
                        || memory_region_ioeventfd_before(fds_new[inew],
                                                          fds_old[iold]))) {
-            as->ops->ioeventfd_add(as, &fds_new[inew]);
+            fd = &fds_new[inew];
+            section = (MemoryRegionSection) {
+                .address_space = as->root,
+                .offset_within_address_space = int128_get64(fd->addr.start),
+                .size = int128_get64(fd->addr.size),
+            };
+            MEMORY_LISTENER_CALL(eventfd_add, Reverse, &section,
+                                 fd->match_data, fd->data, fd->fd);
             ++inew;
         } else {
             ++iold;
diff --git a/memory.h b/memory.h
index 954dc86..84bb67c 100644
--- a/memory.h
+++ b/memory.h
@@ -185,6 +185,10 @@ struct MemoryListener {
     void (*log_sync)(MemoryListener *listener, MemoryRegionSection *section);
     void (*log_global_start)(MemoryListener *listener);
     void (*log_global_stop)(MemoryListener *listener);
+    void (*eventfd_add)(MemoryListener *listener, MemoryRegionSection *section,
+                        bool match_data, uint64_t data, int fd);
+    void (*eventfd_del)(MemoryListener *listener, MemoryRegionSection *section,
+                        bool match_data, uint64_t data, int fd);
     /* Lower = earlier (during add), later (during del) */
     unsigned priority;
     QTAILQ_ENTRY(MemoryListener) link;
diff --git a/xen-all.c b/xen-all.c
index 8cb84ef..e005b63 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -487,6 +487,18 @@ static void xen_log_global_stop(MemoryListener *listener)
 {
 }
 
+static void xen_eventfd_add(MemoryListener *listener,
+                            MemoryRegionSection *section,
+                            bool match_data, uint64_t data, int fd)
+{
+}
+
+static void xen_eventfd_del(MemoryListener *listener,
+                            MemoryRegionSection *section,
+                            bool match_data, uint64_t data, int fd)
+{
+}
+
 static MemoryListener xen_memory_listener = {
     .region_add = xen_region_add,
     .region_del = xen_region_del,
@@ -495,6 +507,8 @@ static void xen_log_global_stop(MemoryListener *listener)
     .log_sync = xen_log_sync,
     .log_global_start = xen_log_global_start,
     .log_global_stop = xen_log_global_stop,
+    .eventfd_add = xen_eventfd_add,
+    .eventfd_del = xen_eventfd_del,
     .priority = 10,
 };
 
-- 
1.7.9

[Qemu-devel] [PATCH 07/10] memory: add a readonly attribute to MemoryRegionSection

[Avi Kivity]

.readonly cannot be obtained from the MemoryRegion, since it is
inherited from aliases (so you can have a MemoryRegion mapped RW
at one address and RO at another).  Record it in a MemoryRegionSection
for listeners.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 memory.c |    5 +++++
 memory.h |    2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/memory.c b/memory.c
index e1a31d4..4f37152 100644
--- a/memory.c
+++ b/memory.c
@@ -112,6 +112,7 @@ static AddrRange addrrange_intersection(AddrRange r1, AddrRange r2)
         .offset_within_region = (fr)->offset_in_region,                 \
         .size = int128_get64((fr)->addr.size),                          \
         .offset_within_address_space = int128_get64((fr)->addr.start),  \
+        .readonly = (fr)->readonly,                                     \
                 })
 
 struct CoalescedMemoryRange {
@@ -342,6 +343,7 @@ static void as_memory_range_add(AddressSpace *as, FlatRange *fr)
         .offset_within_address_space = int128_get64(fr->addr.start),
         .offset_within_region = fr->offset_in_region,
         .size = int128_get64(fr->addr.size),
+        .readonly = fr->readonly,
     };
 
     cpu_register_physical_memory_log(&section, fr->readable, fr->readonly);
@@ -354,6 +356,7 @@ static void as_memory_range_del(AddressSpace *as, FlatRange *fr)
         .offset_within_address_space = int128_get64(fr->addr.start),
         .offset_within_region = int128_get64(fr->addr.start),
         .size = int128_get64(fr->addr.size),
+        .readonly = fr->readonly,
     };
 
     cpu_register_physical_memory_log(&section, true, false);
@@ -1437,6 +1440,7 @@ MemoryRegionSection memory_region_find(MemoryRegion *address_space,
                                                         fr->addr.start));
     ret.size = int128_get64(range.size);
     ret.offset_within_address_space = int128_get64(range.start);
+    ret.readonly = fr->readonly;
     return ret;
 }
 
@@ -1479,6 +1483,7 @@ static void listener_add_address_space(MemoryListener *listener,
             .offset_within_region = fr->offset_in_region,
             .size = int128_get64(fr->addr.size),
             .offset_within_address_space = int128_get64(fr->addr.start),
+            .readonly = fr->readonly,
         };
         listener->region_add(listener, &section);
     }
diff --git a/memory.h b/memory.h
index 84bb67c..1d99cee 100644
--- a/memory.h
+++ b/memory.h
@@ -160,6 +160,7 @@ typedef struct MemoryRegionSection MemoryRegionSection;
  * @size: the size of the section; will not exceed @mr's boundaries
  * @offset_within_address_space: the address of the first byte of the section
  *     relative to the region's address space
+ * @readonly: writes to this section are ignored
  */
 struct MemoryRegionSection {
     MemoryRegion *mr;
@@ -167,6 +168,7 @@ struct MemoryRegionSection {
     target_phys_addr_t offset_within_region;
     uint64_t size;
     target_phys_addr_t offset_within_address_space;
+    bool readonly;
 };
 
 typedef struct MemoryListener MemoryListener;
-- 
1.7.9

[Qemu-devel] [PATCH 08/10] memory: don't pass ->readable attribute to cpu_register_physical_memory_log

[Avi Kivity]

It can be derived from the MemoryRegion itself (which is why it is not
used there).

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 exec-obsolete.h |    2 +-
 exec.c          |    2 +-
 memory.c        |    4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/exec-obsolete.h b/exec-obsolete.h
index 94c23d0..23ffbaa 100644
--- a/exec-obsolete.h
+++ b/exec-obsolete.h
@@ -37,7 +37,7 @@ void cpu_unregister_io_memory(int table_address);
 
 struct MemoryRegionSection;
 void cpu_register_physical_memory_log(struct MemoryRegionSection *section,
-                                      bool readable, bool readonly);
+                                      bool readonly);
 
 void qemu_register_coalesced_mmio(target_phys_addr_t addr, ram_addr_t size);
 void qemu_unregister_coalesced_mmio(target_phys_addr_t addr, ram_addr_t size);
diff --git a/exec.c b/exec.c
index b81677a..d3020ab 100644
--- a/exec.c
+++ b/exec.c
@@ -2529,7 +2529,7 @@ static subpage_t *subpage_init (target_phys_addr_t base, ram_addr_t *phys,
    before calculating this offset.  This should not be a problem unless
    the low bits of start_addr and region_offset differ.  */
 void cpu_register_physical_memory_log(MemoryRegionSection *section,
-                                      bool readable, bool readonly)
+                                      bool readonly)
 {
     target_phys_addr_t start_addr = section->offset_within_address_space;
     ram_addr_t size = section->size;
diff --git a/memory.c b/memory.c
index 4f37152..71039c4 100644
--- a/memory.c
+++ b/memory.c
@@ -346,7 +346,7 @@ static void as_memory_range_add(AddressSpace *as, FlatRange *fr)
         .readonly = fr->readonly,
     };
 
-    cpu_register_physical_memory_log(&section, fr->readable, fr->readonly);
+    cpu_register_physical_memory_log(&section, fr->readonly);
 }
 
 static void as_memory_range_del(AddressSpace *as, FlatRange *fr)
@@ -359,7 +359,7 @@ static void as_memory_range_del(AddressSpace *as, FlatRange *fr)
         .readonly = fr->readonly,
     };
 
-    cpu_register_physical_memory_log(&section, true, false);
+    cpu_register_physical_memory_log(&section, false);
 }
 
 static void as_memory_log_start(AddressSpace *as, FlatRange *fr)
-- 
1.7.9

[Qemu-devel] [PATCH 09/10] memory: use a MemoryListener for core memory map updates too

[Avi Kivity]

This transforms memory.c into a library which can then be unit tested
easily, by feeding it inputs and listening to its outputs.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 exec-obsolete.h |    3 ++
 exec.c          |   75 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory.c        |   27 +-------------------
 3 files changed, 79 insertions(+), 26 deletions(-)

diff --git a/exec-obsolete.h b/exec-obsolete.h
index 23ffbaa..4dbe476 100644
--- a/exec-obsolete.h
+++ b/exec-obsolete.h
@@ -121,6 +121,9 @@ static inline void cpu_physical_memory_mask_dirty_range(ram_addr_t start,
 
 void cpu_physical_memory_reset_dirty(ram_addr_t start, ram_addr_t end,
                                      int dirty_flags);
+
+extern const IORangeOps memory_region_iorange_ops;
+
 #endif
 
 #endif
diff --git a/exec.c b/exec.c
index d3020ab..7fb5d4e 100644
--- a/exec.c
+++ b/exec.c
@@ -3488,6 +3488,79 @@ static void io_mem_init(void)
                           "watch", UINT64_MAX);
 }
 
+static void core_region_add(MemoryListener *listener,
+                            MemoryRegionSection *section)
+{
+    if (section->address_space == get_system_memory()) {
+        cpu_register_physical_memory_log(section, section->readonly);
+    } else {
+        iorange_init(&section->mr->iorange, &memory_region_iorange_ops,
+                     section->offset_within_address_space, section->size);
+        ioport_register(&section->mr->iorange);
+    }
+}
+
+static void core_region_del(MemoryListener *listener,
+                            MemoryRegionSection *section)
+{
+    if (section->address_space == get_system_memory()) {
+        cpu_register_physical_memory_log(section, false);
+    } else {
+        isa_unassign_ioport(section->offset_within_address_space,
+                            section->size);
+    }
+}
+
+static void core_log_start(MemoryListener *listener,
+                           MemoryRegionSection *section)
+{
+}
+
+static void core_log_stop(MemoryListener *listener,
+                          MemoryRegionSection *section)
+{
+}
+
+static void core_log_sync(MemoryListener *listener,
+                          MemoryRegionSection *section)
+{
+}
+
+static void core_log_global_start(MemoryListener *listener)
+{
+    cpu_physical_memory_set_dirty_tracking(1);
+}
+
+static void core_log_global_stop(MemoryListener *listener)
+{
+    cpu_physical_memory_set_dirty_tracking(0);
+}
+
+static void core_eventfd_add(MemoryListener *listener,
+                             MemoryRegionSection *section,
+                             bool match_data, uint64_t data, int fd)
+{
+}
+
+static void core_eventfd_del(MemoryListener *listener,
+                             MemoryRegionSection *section,
+                             bool match_data, uint64_t data, int fd)
+{
+}
+
+static MemoryListener core_memory_listener = {
+    .region_add = core_region_add,
+    .region_del = core_region_del,
+    .log_start = core_log_start,
+    .log_stop = core_log_stop,
+    .log_sync = core_log_sync,
+    .log_global_start = core_log_global_start,
+    .log_global_stop = core_log_global_stop,
+    .eventfd_add = core_eventfd_add,
+    .eventfd_del = core_eventfd_del,
+    .priority = 0,
+};
+
 static void memory_map_init(void)
 {
     system_memory = g_malloc(sizeof(*system_memory));
@@ -3497,6 +3570,8 @@ static void memory_map_init(void)
     system_io = g_malloc(sizeof(*system_io));
     memory_region_init(system_io, "io", 65536);
     set_system_io_map(system_io);
+
+    memory_listener_register(&core_memory_listener);
 }
 
 MemoryRegion *get_system_memory(void)
diff --git a/memory.c b/memory.c
index 71039c4..4e7a90b 100644
--- a/memory.c
+++ b/memory.c
@@ -338,28 +338,10 @@ static void access_with_adjusted_size(target_phys_addr_t addr,
 
 static void as_memory_range_add(AddressSpace *as, FlatRange *fr)
 {
-    MemoryRegionSection section = {
-        .mr = fr->mr,
-        .offset_within_address_space = int128_get64(fr->addr.start),
-        .offset_within_region = fr->offset_in_region,
-        .size = int128_get64(fr->addr.size),
-        .readonly = fr->readonly,
-    };
-
-    cpu_register_physical_memory_log(&section, fr->readonly);
 }
 
 static void as_memory_range_del(AddressSpace *as, FlatRange *fr)
 {
-    MemoryRegionSection section = {
-        .mr = &io_mem_unassigned,
-        .offset_within_address_space = int128_get64(fr->addr.start),
-        .offset_within_region = int128_get64(fr->addr.start),
-        .size = int128_get64(fr->addr.size),
-        .readonly = fr->readonly,
-    };
-
-    cpu_register_physical_memory_log(&section, false);
 }
 
 static void as_memory_log_start(AddressSpace *as, FlatRange *fr)
@@ -450,22 +432,17 @@ static void memory_region_iorange_write(IORange *iorange,
                               memory_region_write_accessor, mr);
 }
 
-static const IORangeOps memory_region_iorange_ops = {
+const IORangeOps memory_region_iorange_ops = {
     .read = memory_region_iorange_read,
     .write = memory_region_iorange_write,
 };
 
 static void as_io_range_add(AddressSpace *as, FlatRange *fr)
 {
-    iorange_init(&fr->mr->iorange, &memory_region_iorange_ops,
-                 int128_get64(fr->addr.start), int128_get64(fr->addr.size));
-    ioport_register(&fr->mr->iorange);
 }
 
 static void as_io_range_del(AddressSpace *as, FlatRange *fr)
 {
-    isa_unassign_ioport(int128_get64(fr->addr.start),
-                        int128_get64(fr->addr.size));
 }
 
 static const AddressSpaceOps address_space_ops_io = {
@@ -1456,7 +1433,6 @@ void memory_global_sync_dirty_bitmap(MemoryRegion *address_space)
 
 void memory_global_dirty_log_start(void)
 {
-    cpu_physical_memory_set_dirty_tracking(1);
     global_dirty_log = true;
     MEMORY_LISTENER_CALL(log_global_start, Forward);
 }
@@ -1465,7 +1441,6 @@ void memory_global_dirty_log_stop(void)
 {
     global_dirty_log = false;
     MEMORY_LISTENER_CALL(log_global_stop, Reverse);
-    cpu_physical_memory_set_dirty_tracking(0);
 }
 
 static void listener_add_address_space(MemoryListener *listener,
-- 
1.7.9

[Qemu-devel] [PATCH 10/10] memory: drop AddressSpaceOps

[Avi Kivity]

All functionality has been moved to various MemoryListeners.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 memory.c |   56 ++------------------------------------------------------
 1 files changed, 2 insertions(+), 54 deletions(-)

diff --git a/memory.c b/memory.c
index 4e7a90b..4f854d4 100644
--- a/memory.c
+++ b/memory.c
@@ -191,20 +191,12 @@ struct FlatView {
 
 /* A system address space - I/O, memory, etc. */
 struct AddressSpace {
-    const AddressSpaceOps *ops;
     MemoryRegion *root;
     FlatView current_map;
     int ioeventfd_nb;
     MemoryRegionIoeventfd *ioeventfds;
 };
 
-struct AddressSpaceOps {
-    void (*range_add)(AddressSpace *as, FlatRange *fr);
-    void (*range_del)(AddressSpace *as, FlatRange *fr);
-    void (*log_start)(AddressSpace *as, FlatRange *fr);
-    void (*log_stop)(AddressSpace *as, FlatRange *fr);
-};
-
 #define FOR_EACH_FLAT_RANGE(var, view)          \
     for (var = (view)->ranges; var < (view)->ranges + (view)->nr; ++var)
 
@@ -336,32 +328,7 @@ static void access_with_adjusted_size(target_phys_addr_t addr,
     }
 }
 
-static void as_memory_range_add(AddressSpace *as, FlatRange *fr)
-{
-}
-
-static void as_memory_range_del(AddressSpace *as, FlatRange *fr)
-{
-}
-
-static void as_memory_log_start(AddressSpace *as, FlatRange *fr)
-{
-}
-
-static void as_memory_log_stop(AddressSpace *as, FlatRange *fr)
-{
-}
-
-static const AddressSpaceOps address_space_ops_memory = {
-    .range_add = as_memory_range_add,
-    .range_del = as_memory_range_del,
-    .log_start = as_memory_log_start,
-    .log_stop = as_memory_log_stop,
-};
-
-static AddressSpace address_space_memory = {
-    .ops = &address_space_ops_memory,
-};
+static AddressSpace address_space_memory;
 
 static const MemoryRegionPortio *find_portio(MemoryRegion *mr, uint64_t offset,
                                              unsigned width, bool write)
@@ -437,22 +404,7 @@ static void memory_region_iorange_write(IORange *iorange,
     .write = memory_region_iorange_write,
 };
 
-static void as_io_range_add(AddressSpace *as, FlatRange *fr)
-{
-}
-
-static void as_io_range_del(AddressSpace *as, FlatRange *fr)
-{
-}
-
-static const AddressSpaceOps address_space_ops_io = {
-    .range_add = as_io_range_add,
-    .range_del = as_io_range_del,
-};
-
-static AddressSpace address_space_io = {
-    .ops = &address_space_ops_io,
-};
+static AddressSpace address_space_io;
 
 static AddressSpace *memory_region_to_address_space(MemoryRegion *mr)
 {
@@ -685,7 +637,6 @@ static void address_space_update_topology_pass(AddressSpace *as,
 
             if (!adding) {
                 MEMORY_LISTENER_UPDATE_REGION(frold, as, Reverse, region_del);
-                as->ops->range_del(as, frold);
             }
 
             ++iold;
@@ -695,9 +646,7 @@ static void address_space_update_topology_pass(AddressSpace *as,
             if (adding) {
                 if (frold->dirty_log_mask && !frnew->dirty_log_mask) {
                     MEMORY_LISTENER_UPDATE_REGION(frnew, as, Reverse, log_stop);
-                    as->ops->log_stop(as, frnew);
                 } else if (frnew->dirty_log_mask && !frold->dirty_log_mask) {
-                    as->ops->log_start(as, frnew);
                     MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, log_start);
                 }
             }
@@ -708,7 +657,6 @@ static void address_space_update_topology_pass(AddressSpace *as,
             /* In new */
 
             if (adding) {
-                as->ops->range_add(as, frnew);
                 MEMORY_LISTENER_UPDATE_REGION(frnew, as, Forward, region_add);
             }
 
-- 
1.7.9

Re: [Qemu-devel] [PATCH 00/10] Remove AddressSpaceOps

[Avi Kivity]

On 02/08/2012 05:27 PM, Avi Kivity wrote:
> This patchset makes the memory core (memory.c) talk to the backend (in exec.c)
> via a MemoryListener instead of named functions.
>
> While the motivation for this is to simplify the memory core, it also enables
> optimizing accelerators some more (by having a tcg MemoryListener to do tcg
> specific core) and allows unit testing of memory.c (by adding a testing
> MemoryListener and seeing what it outputs as various inputs are fed into the
> core).
>

Something that is very visible here is that MemoryListeners switch quite
a lot on the address space.  This suggests that we need to allow
observing a specific address space.  I'll address that in a future patch.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 00/10] Remove AddressSpaceOps

[Richard Henderson]

On 02/08/2012 07:27 AM, Avi Kivity wrote:
> This patchset makes the memory core (memory.c) talk to the backend (in exec.c)
> via a MemoryListener instead of named functions.
> 
> While the motivation for this is to simplify the memory core, it also enables
> optimizing accelerators some more (by having a tcg MemoryListener to do tcg
> specific core) and allows unit testing of memory.c (by adding a testing
> MemoryListener and seeing what it outputs as various inputs are fed into the
> core).
> 
> Avi Kivity (10):
>   ioport: change portio_list not to use memory_region_set_offset()
>   memory: remove memory_region_set_offset()
>   memory: add shorthand for invoking a callback on all listeners
>   memory: switch memory listeners to a QTAILQ
>   memory: code motion: move MEMORY_LISTENER_CALL()
>   memory: move ioeventfd ops to MemoryListener
>   memory: add a readonly attribute to MemoryRegionSection
>   memory: don't pass ->readable attribute to
>     cpu_register_physical_memory_log
>   memory: use a MemoryListener for core memory map updates too
>   memory: drop AddressSpaceOps

Looks good.

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

Re: [Qemu-devel] [PATCH 09/10] memory: use a MemoryListener for core memory map updates too

[Paolo Bonzini]

On 02/08/2012 04:27 PM, Avi Kivity wrote:
> +static void core_log_stop(MemoryListener *listener,
> +                          MemoryRegionSection *section)
> +{
> +}
> +
> +static void core_log_sync(MemoryListener *listener,
> +                          MemoryRegionSection *section)
> +{
> +}
> +

Why not wrapping the calls inside an "if" so that these dummy functions 
need not be there?  Can be done in a follow-up, though.

Paolo

Re: [Qemu-devel] [PATCH 09/10] memory: use a MemoryListener for core memory map updates too

[Avi Kivity]

On 02/09/2012 09:58 AM, Paolo Bonzini wrote:
> On 02/08/2012 04:27 PM, Avi Kivity wrote:
>> +static void core_log_stop(MemoryListener *listener,
>> +                          MemoryRegionSection *section)
>> +{
>> +}
>> +
>> +static void core_log_sync(MemoryListener *listener,
>> +                          MemoryRegionSection *section)
>> +{
>> +}
>> +
>
> Why not wrapping the calls inside an "if" so that these dummy
> functions need not be there?  Can be done in a follow-up, though.

I dislike that style.  We could have memory.c provide default versions
for users to put into their listener structures.

-- 
error compiling committee.c: too many arguments to function