Re: [Qemu-devel] [PATCH 01/13] mips_jazz: convert to memory API

[Avi Kivity <avi@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 518 bytes --]

On 09/25/2011 12:18 PM, Avi Kivity wrote:
> On 09/24/2011 11:29 AM, Hervé Poussineau wrote:
>>
>> This part breaks Magnum emulation, where firmware writes to 
>> 0x60000000, and then checks if value has been modified.
>> This is not working as expected (value is modified) because 
>> memory_region_set_readonly() is unimplemented...
>
> Whoops!  Will provide a fix asap.
>

Please test the attached patch (works for me with 440FX PAM)

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


[-- Attachment #2: 0001-memory-implement-memory_region_set_readonly.patch --]
[-- Type: text/x-patch, Size: 5152 bytes --]

>From fb1cd6f998fe7dba889e5c77962f94bb1a15d202 Mon Sep 17 00:00:00 2001
From: Avi Kivity <avi@redhat.com>
Date: Sun, 25 Sep 2011 14:48:47 +0300
Subject: [PATCH] memory: implement memory_region_set_readonly()

The property is inheritable, but only if set to true.  This is so
that memory routers can mark sections of RAM as read-only via aliases.

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

diff --git a/memory.c b/memory.c
index ba74435..71e769e 100644
--- a/memory.c
+++ b/memory.c
@@ -126,6 +126,7 @@ struct FlatRange {
     AddrRange addr;
     uint8_t dirty_log_mask;
     bool readable;
+    bool readonly;
 };
 
 /* Flattened global view of current active memory hierarchy.  Kept in sorted
@@ -166,7 +167,8 @@ static bool flatrange_equal(FlatRange *a, FlatRange *b)
     return a->mr == b->mr
         && addrrange_equal(a->addr, b->addr)
         && a->offset_in_region == b->offset_in_region
-        && a->readable == b->readable;
+        && a->readable == b->readable
+        && a->readonly == b->readonly;
 }
 
 static void flatview_init(FlatView *view)
@@ -203,7 +205,8 @@ static bool can_merge(FlatRange *r1, FlatRange *r2)
         && r1->mr == r2->mr
         && r1->offset_in_region + r1->addr.size == r2->offset_in_region
         && r1->dirty_log_mask == r2->dirty_log_mask
-        && r1->readable == r2->readable;
+        && r1->readable == r2->readable
+        && r1->readonly == r2->readonly;
 }
 
 /* Attempt to simplify a view by merging ajacent ranges */
@@ -307,6 +310,10 @@ static void as_memory_range_add(AddressSpace *as, FlatRange *fr)
         phys_offset &= ~TARGET_PAGE_MASK & ~IO_MEM_ROMD;
     }
 
+    if (fr->readonly) {
+        phys_offset |= IO_MEM_ROM;
+    }
+
     cpu_register_physical_memory_log(fr->addr.start,
                                      fr->addr.size,
                                      phys_offset,
@@ -484,7 +491,8 @@ static void as_io_ioeventfd_del(AddressSpace *as, MemoryRegionIoeventfd *fd)
 static void render_memory_region(FlatView *view,
                                  MemoryRegion *mr,
                                  target_phys_addr_t base,
-                                 AddrRange clip)
+                                 AddrRange clip,
+                                 bool readonly)
 {
     MemoryRegion *subregion;
     unsigned i;
@@ -495,6 +503,7 @@ static void render_memory_region(FlatView *view,
     AddrRange tmp;
 
     base += mr->addr;
+    readonly |= mr->readonly;
 
     tmp = addrrange_make(base, mr->size);
 
@@ -507,13 +516,13 @@ static void render_memory_region(FlatView *view,
     if (mr->alias) {
         base -= mr->alias->addr;
         base -= mr->alias_offset;
-        render_memory_region(view, mr->alias, base, clip);
+        render_memory_region(view, mr->alias, base, clip, readonly);
         return;
     }
 
     /* Render subregions in priority order. */
     QTAILQ_FOREACH(subregion, &mr->subregions, subregions_link) {
-        render_memory_region(view, subregion, base, clip);
+        render_memory_region(view, subregion, base, clip, readonly);
     }
 
     if (!mr->terminates) {
@@ -536,6 +545,7 @@ static void render_memory_region(FlatView *view,
             fr.addr = addrrange_make(base, now);
             fr.dirty_log_mask = mr->dirty_log_mask;
             fr.readable = mr->readable;
+            fr.readonly = readonly;
             flatview_insert(view, i, &fr);
             ++i;
             base += now;
@@ -555,6 +565,7 @@ static void render_memory_region(FlatView *view,
         fr.addr = addrrange_make(base, remain);
         fr.dirty_log_mask = mr->dirty_log_mask;
         fr.readable = mr->readable;
+        fr.readonly = readonly;
         flatview_insert(view, i, &fr);
     }
 }
@@ -566,7 +577,7 @@ static FlatView generate_memory_topology(MemoryRegion *mr)
 
     flatview_init(&view);
 
-    render_memory_region(&view, mr, 0, addrrange_make(0, INT64_MAX));
+    render_memory_region(&view, mr, 0, addrrange_make(0, INT64_MAX), false);
     flatview_simplify(&view);
 
     return view;
@@ -772,6 +783,7 @@ void memory_region_init(MemoryRegion *mr,
     mr->offset = 0;
     mr->terminates = false;
     mr->readable = true;
+    mr->readonly = false;
     mr->destructor = memory_region_destructor_none;
     mr->priority = 0;
     mr->may_overlap = false;
@@ -1035,7 +1047,10 @@ void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
 
 void memory_region_set_readonly(MemoryRegion *mr, bool readonly)
 {
-    /* FIXME */
+    if (mr->readonly != readonly) {
+        mr->readonly = readonly;
+        memory_region_update_topology();
+    }
 }
 
 void memory_region_rom_device_set_readable(MemoryRegion *mr, bool readable)
diff --git a/memory.h b/memory.h
index 06b83ae..e93e65a 100644
--- a/memory.h
+++ b/memory.h
@@ -114,6 +114,7 @@ struct MemoryRegion {
     IORange iorange;
     bool terminates;
     bool readable;
+    bool readonly; /* For RAM regions */
     MemoryRegion *alias;
     target_phys_addr_t alias_offset;
     unsigned priority;
-- 
1.7.6.3