[Qemu-devel] [PATCH 28/66] exec: reorganize address_space_map

[Paolo Bonzini]

First of all, rename "todo" to "done".

Second, clearly separate the case of done == 0 with the case of done != 0.
This will help handling reference counting in the next patch.

Third, this test:

             if (memory_region_get_ram_addr(mr) + xlat != raddr + todo) {

does not guarantee that the memory region is the same across two iterations
of the while loop.  For example, you could have two blocks:

A) size 640 K, mapped at physical address 0, ram_addr_t 0
B) size 64 K, mapped at physical address 0xa0000, ram_addr_t 0xa0000

then mapping 1 M starting at physical address zero will erroneously treat
B as the continuation of block A.  qemu_ram_ptr_length ensures that no
invalid memory is accessed, but it is still a pointless complication of
the algorithm.  The patch makes the logic clearer with an explicit test
that the memory region is the same.

Reviewed-by: Jan Kiszka <address@hidden>
Signed-off-by: Paolo Bonzini <address@hidden>
---
 exec.c | 71 +++++++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 38 insertions(+), 33 deletions(-)

diff --git a/exec.c b/exec.c
index 307efea..a994bc8 100644
--- a/exec.c
+++ b/exec.c
@@ -2065,47 +2065,52 @@ void *address_space_map(AddressSpace *as,
                         bool is_write)
 {
     hwaddr len = *plen;
-    hwaddr todo = 0;
-    hwaddr l, xlat;
-    MemoryRegion *mr;
-    ram_addr_t raddr = RAM_ADDR_MAX;
-    ram_addr_t rlen;
-    void *ret;
+    hwaddr done = 0;
+    hwaddr l, xlat, base;
+    MemoryRegion *mr, *this_mr;
+    ram_addr_t raddr;
 
-    while (len > 0) {
-        l = len;
-        mr = address_space_translate(as, addr, &xlat, &l, is_write);
-
-        if (!memory_access_is_direct(mr, is_write)) {
-            if (todo || bounce.buffer) {
-                break;
-            }
-            bounce.buffer = qemu_memalign(TARGET_PAGE_SIZE, TARGET_PAGE_SIZE);
-            bounce.addr = addr;
-            bounce.len = l;
-            if (!is_write) {
-                address_space_read(as, addr, bounce.buffer, l);
-            }
+    if (len == 0) {
+        return NULL;
+    }
 
-            *plen = l;
-            return bounce.buffer;
+    l = len;
+    mr = address_space_translate(as, addr, &xlat, &l, is_write);
+    if (!memory_access_is_direct(mr, is_write)) {
+        if (bounce.buffer) {
+            return NULL;
         }
-        if (!todo) {
-            raddr = memory_region_get_ram_addr(mr) + xlat;
-        } else {
-            if (memory_region_get_ram_addr(mr) + xlat != raddr + todo) {
-                break;
-            }
+        bounce.buffer = qemu_memalign(TARGET_PAGE_SIZE, TARGET_PAGE_SIZE);
+        bounce.addr = addr;
+        bounce.len = l;
+        if (!is_write) {
+            address_space_read(as, addr, bounce.buffer, l);
         }
 
+        *plen = l;
+        return bounce.buffer;
+    }
+
+    base = xlat;
+    raddr = memory_region_get_ram_addr(mr);
+
+    for (;;) {
         len -= l;
         addr += l;
-        todo += l;
+        done += l;
+        if (len == 0) {
+            break;
+        }
+
+        l = len;
+        this_mr = address_space_translate(as, addr, &xlat, &l, is_write);
+        if (this_mr != mr || xlat != base + done) {
+            break;
+        }
     }
-    rlen = todo;
-    ret = qemu_ram_ptr_length(raddr, &rlen);
-    *plen = rlen;
-    return ret;
+
+    *plen = done;
+    return qemu_ram_ptr_length(raddr + base, plen);
 }
 
 /* Unmaps a memory region previously mapped by address_space_map().
-- 
1.8.1.4