[Qemu-devel] [PATCH 5/7] memory: add kvm_mem_flags to MemoryRegion

[Paolo Bonzini]

This patch is ugly; it adds a KVM-specific flag to memory regions that
is used to distinguish SMRAM regions from others.  This of course is a
layering violation, but I have no other good ideas about how to avoid it.

If you let KVM use address_space_memory as it did until now, and add
separate calls to KVM_SET_USER_MEMORY_REGION for SMRAM regions, it's
much harder to cover the case when a RAM or ROM BAR is placed to cover
parts of SMRAM.

Hence, this series uses instead one address space only, including
all of system memory, PCI BARs and SMRAM.  This way, hiding the BARs
is done automatically by the memory core.  This, however, has an
important issue which requires this kind of layering violation.

The issue is that when you close SMRAM there is no change in the memory
map as far as the memory API is concerned.  The only change is in the
flags we pass to KVM, but these are not visible to the memory API.  So
KVM needs to convince the memory API to see a region_del/region_add
sequence from the listener; it needs to convince flatrange_equal to
return false for the "open SMRAM" and "closed SMRAM" cases.  This is
what the kvm_mem_flags do.

Besides the layering violation, there is also the question of how to
handle flags from subregions.  The current code just ORs the parent
region's flags into all subregions.

This being ugly, and hopefully temporary until someone shows me the
light, I haven't yet added nice accessors for the field.  The final
patch accesses the field blindly from target-i386/kvm.c.

One alternative would be to point two different RAM regions to the same
guest memory, using memory_region_init_ram_ptr.  That works, but it's
even more gross.

Signed-off-by: Paolo Bonzini <address@hidden>
---
 include/exec/memory.h |  4 ++++
 kvm-all.c             | 13 +++++++------
 memory.c              | 17 +++++++++++++----
 3 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index b61c84f..cccba59 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -192,6 +192,8 @@ struct MemoryRegion {
     unsigned ioeventfd_nb;
     MemoryRegionIoeventfd *ioeventfds;
     NotifierList iommu_notify;
+
+    int kvm_mem_flags;
 };
 
 /**
@@ -264,6 +266,8 @@ struct MemoryRegionSection {
     Int128 size;
     hwaddr offset_within_address_space;
     bool readonly;
+
+    int kvm_mem_flags;
 };
 
 /**
diff --git a/kvm-all.c b/kvm-all.c
index c70436e..ccb7dd3 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -237,10 +237,11 @@ err:
  * dirty pages logging control
  */
 
-static int kvm_mem_flags(MemoryRegion *mr)
+static int kvm_mem_flags(MemoryRegionSection *section)
 {
+    MemoryRegion *mr = section->mr;
     bool readonly = mr->readonly || memory_region_is_romd(mr);
-    int flags = 0;
+    int flags = section->kvm_mem_flags;
 
     if (memory_region_is_logging(mr)) {
         flags |= KVM_MEM_LOG_DIRTY_PAGES;
@@ -689,7 +690,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, 
bool add)
             mem->memory_size = old.memory_size;
             mem->start_addr = old.start_addr;
             mem->ram = old.ram;
-            mem->flags = kvm_mem_flags(mr);
+            mem->flags = kvm_mem_flags(section);
 
             err = kvm_set_user_memory_region(s, mem);
             if (err) {
@@ -710,7 +711,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, 
bool add)
             mem->memory_size = start_addr - old.start_addr;
             mem->start_addr = old.start_addr;
             mem->ram = old.ram;
-            mem->flags =  kvm_mem_flags(mr);
+            mem->flags =  kvm_mem_flags(section);
 
             err = kvm_set_user_memory_region(s, mem);
             if (err) {
@@ -734,7 +735,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, 
bool add)
             size_delta = mem->start_addr - old.start_addr;
             mem->memory_size = old.memory_size - size_delta;
             mem->ram = old.ram + size_delta;
-            mem->flags = kvm_mem_flags(mr);
+            mem->flags = kvm_mem_flags(section);
 
             err = kvm_set_user_memory_region(s, mem);
             if (err) {
@@ -756,7 +757,7 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, 
bool add)
     mem->memory_size = size;
     mem->start_addr = start_addr;
     mem->ram = ram;
-    mem->flags = kvm_mem_flags(mr);
+    mem->flags = kvm_mem_flags(section);
 
     err = kvm_set_user_memory_region(s, mem);
     if (err) {
diff --git a/memory.c b/memory.c
index 03c536b..cf810f1 100644
--- a/memory.c
+++ b/memory.c
@@ -160,6 +160,7 @@ static bool memory_listener_match(MemoryListener *listener,
         .size = (fr)->addr.size,                                        \
         .offset_within_address_space = int128_get64((fr)->addr.start),  \
         .readonly = (fr)->readonly,                                     \
+        .kvm_mem_flags = (fr)->kvm_mem_flags,                           \
               }))
 
 struct CoalescedMemoryRange {
@@ -219,6 +220,7 @@ struct FlatRange {
     MemoryRegion *mr;
     hwaddr offset_in_region;
     AddrRange addr;
+    int kvm_mem_flags;
     uint8_t dirty_log_mask;
     bool romd_mode;
     bool readonly;
@@ -246,6 +248,7 @@ static bool flatrange_equal(FlatRange *a, FlatRange *b)
         && addrrange_equal(a->addr, b->addr)
         && a->offset_in_region == b->offset_in_region
         && a->romd_mode == b->romd_mode
+        && a->kvm_mem_flags == b->kvm_mem_flags
         && a->readonly == b->readonly;
 }
 
@@ -306,7 +309,8 @@ static bool can_merge(FlatRange *r1, FlatRange *r2)
                      int128_make64(r2->offset_in_region))
         && r1->dirty_log_mask == r2->dirty_log_mask
         && r1->romd_mode == r2->romd_mode
-        && r1->readonly == r2->readonly;
+        && r1->readonly == r2->readonly
+        && r1->kvm_mem_flags == r2->kvm_mem_flags;
 }
 
 /* Attempt to simplify a view by merging adjacent ranges */
@@ -542,6 +546,7 @@ static void render_memory_region(FlatView *view,
                                  MemoryRegion *mr,
                                  Int128 base,
                                  AddrRange clip,
+                                 int kvm_mem_flags,
                                  bool readonly)
 {
     MemoryRegion *subregion;
@@ -556,6 +561,7 @@ static void render_memory_region(FlatView *view,
         return;
     }
 
+    kvm_mem_flags |= mr->kvm_mem_flags;
     int128_addto(&base, int128_make64(mr->addr));
     readonly |= mr->readonly;
 
@@ -570,13 +576,15 @@ static void render_memory_region(FlatView *view,
     if (mr->alias) {
         int128_subfrom(&base, int128_make64(mr->alias->addr));
         int128_subfrom(&base, int128_make64(mr->alias_offset));
-        render_memory_region(view, mr->alias, base, clip, readonly);
+        render_memory_region(view, mr->alias, base, clip,
+                            kvm_mem_flags, readonly);
         return;
     }
 
     /* Render subregions in priority order. */
     QTAILQ_FOREACH(subregion, &mr->subregions, subregions_link) {
-        render_memory_region(view, subregion, base, clip, readonly);
+        render_memory_region(view, subregion, base, clip, 
+                             kvm_mem_flags, readonly);
     }
 
     if (!mr->terminates) {
@@ -591,6 +599,7 @@ static void render_memory_region(FlatView *view,
     fr.dirty_log_mask = mr->dirty_log_mask;
     fr.romd_mode = mr->romd_mode;
     fr.readonly = readonly;
+    fr.kvm_mem_flags = kvm_mem_flags;
 
     /* Render the region itself into any gaps left by the current view. */
     for (i = 0; i < view->nr && int128_nz(remain); ++i) {
@@ -632,7 +641,7 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
 
     if (mr) {
         render_memory_region(view, mr, int128_zero(),
-                             addrrange_make(int128_zero(), int128_2_64()), 
false);
+                             addrrange_make(int128_zero(), int128_2_64()), 0, 
false);
     }
     flatview_simplify(view);
 
-- 
1.8.3.1