Patchwork [09/10] memory: use a MemoryListener for core memory map updates too

login
register
mail settings
Submitter Avi Kivity
Date Feb. 8, 2012, 3:27 p.m.
Message ID <1328714879-18906-10-git-send-email-avi@redhat.com>
Download mbox | patch
Permalink /patch/140171/
State New
Headers show

Comments

Avi Kivity - Feb. 8, 2012, 3:27 p.m.
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(-)
Paolo Bonzini - Feb. 9, 2012, 7:58 a.m.
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
Avi Kivity - Feb. 9, 2012, 9:28 a.m.
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.

Patch

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,