[Qemu-devel] [PATCH v2] memory: use signed arithmetic

[Avi Kivity <avi@redhat.com>]

When trying to map an alias of a ram region, where the alias starts at
address A and we map it into address B, and A > B, we had an arithmetic
underflow.  Because we use unsigned arithmetic, the underflow converted
into a large number which failed addrrange_intersects() tests.

The concrete example which triggered this was cirrus vga mapping
the framebuffer at offsets 0xc0000-0xc7fff (relative to the start of
the framebuffer) into offsets 0xa0000 (relative to system addres space
start).

With our favorite analogy of a windowing system, this is equivalent to
dragging a subwindow off the left edge of the screen, and failing to clip
it into its parent window which is on screen.

Fix by switching to signed arithmetic.

Signed-off-by: Avi Kivity <avi@redhat.com>
---

v2: add comment about physical address width limitation to 63 bits

 exec.c   |    2 +-
 memory.c |   23 ++++++++++++++---------
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/exec.c b/exec.c
index 476b507..751fd89 100644
--- a/exec.c
+++ b/exec.c
@@ -3818,7 +3818,7 @@ static void io_mem_init(void)
 static void memory_map_init(void)
 {
     system_memory = qemu_malloc(sizeof(*system_memory));
-    memory_region_init(system_memory, "system", UINT64_MAX);
+    memory_region_init(system_memory, "system", INT64_MAX);
     set_system_memory_map(system_memory);
 }
 
diff --git a/memory.c b/memory.c
index 5f20320..be891c6 100644
--- a/memory.c
+++ b/memory.c
@@ -22,12 +22,17 @@ unsigned memory_region_transaction_depth = 0;
 
 typedef struct AddrRange AddrRange;
 
+/*
+ * Note using signed integers limits us to physical addresses at most
+ * 63 bits wide.  They are needed for negative offsetting in aliases
+ * (large MemoryRegion::alias_offset).
+ */
 struct AddrRange {
-    uint64_t start;
-    uint64_t size;
+    int64_t start;
+    int64_t size;
 };
 
-static AddrRange addrrange_make(uint64_t start, uint64_t size)
+static AddrRange addrrange_make(int64_t start, int64_t size)
 {
     return (AddrRange) { start, size };
 }
@@ -37,7 +42,7 @@ static bool addrrange_equal(AddrRange r1, AddrRange r2)
     return r1.start == r2.start && r1.size == r2.size;
 }
 
-static uint64_t addrrange_end(AddrRange r)
+static int64_t addrrange_end(AddrRange r)
 {
     return r.start + r.size;
 }
@@ -56,9 +61,9 @@ static bool addrrange_intersects(AddrRange r1, AddrRange r2)
 
 static AddrRange addrrange_intersection(AddrRange r1, AddrRange r2)
 {
-    uint64_t start = MAX(r1.start, r2.start);
+    int64_t start = MAX(r1.start, r2.start);
     /* off-by-one arithmetic to prevent overflow */
-    uint64_t end = MIN(addrrange_end(r1) - 1, addrrange_end(r2) - 1);
+    int64_t end = MIN(addrrange_end(r1) - 1, addrrange_end(r2) - 1);
     return addrrange_make(start, end - start + 1);
 }
 
@@ -411,8 +416,8 @@ static void render_memory_region(FlatView *view,
     MemoryRegion *subregion;
     unsigned i;
     target_phys_addr_t offset_in_region;
-    uint64_t remain;
-    uint64_t now;
+    int64_t remain;
+    int64_t now;
     FlatRange fr;
     AddrRange tmp;
 
@@ -486,7 +491,7 @@ static FlatView generate_memory_topology(MemoryRegion *mr)
 
     flatview_init(&view);
 
-    render_memory_region(&view, mr, 0, addrrange_make(0, UINT64_MAX));
+    render_memory_region(&view, mr, 0, addrrange_make(0, INT64_MAX));
     flatview_simplify(&view);
 
     return view;
-- 
1.7.5.3