[Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Avi Kivity]

This somewhat controversial patchset converts internal arithmetic in the
memory API to 128 bits.

It has been argued that with careful coding we can make 64-bit work as
well.  I don't think this is true in general - a memory router can adjust
addresses either forwards or backwards, and some buses (PCIe) need the
full 64-bit space - though it's probably the case for all the configurations
we support today.  Regardless, the need for careful coding means subtle bugs,
which I don't want in a core API that is driven by guest supplied values.

Avi Kivity (3):
  Add support for 128-bit arithmetic
  memory: use 128-bit integers for sizes and intermediates
  Adjust system and pci address spaces to full 64-bit

 exec.c          |    2 +-
 hw/pc_piix.c    |    2 +-
 hw/pci_bridge.c |    2 +-
 int128.h        |  116 ++++++++++++++++++++++++++++++++
 memory.c        |  196 ++++++++++++++++++++++++++++++++----------------------
 memory.h        |    3 +-
 6 files changed, 237 insertions(+), 84 deletions(-)
 create mode 100644 int128.h

-- 
1.7.6.3

[Qemu-devel] [PATCH 1/3] Add support for 128-bit arithmetic

[Avi Kivity]

The memory API supports 64-bit buses (e.g. PCI).  A size on such a bus cannot
be represented with a 64-bit data type, if both 0 and the entire address
space size are to be represented.  Futhermore, any address arithemetic may
overflow and return unexpected results.

Introduce a 128-bit signed integer type for use in such cases.  Addition,
subtraction, and comparison are the only operations supported.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 int128.h |  116 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 116 insertions(+), 0 deletions(-)
 create mode 100644 int128.h

diff --git a/int128.h b/int128.h
new file mode 100644
index 0000000..b3864b6
--- /dev/null
+++ b/int128.h
@@ -0,0 +1,116 @@
+#ifndef INT128_H
+#define INT128_H
+
+typedef struct Int128 Int128;
+
+struct Int128 {
+    uint64_t lo;
+    int64_t hi;
+};
+
+static inline Int128 int128_make64(uint64_t a)
+{
+    return (Int128) { a, 0 };
+}
+
+static inline uint64_t int128_get64(Int128 a)
+{
+    assert(!a.hi);
+    return a.lo;
+}
+
+static inline Int128 int128_zero(void)
+{
+    return int128_make64(0);
+}
+
+static inline Int128 int128_one(void)
+{
+    return int128_make64(1);
+}
+
+static inline Int128 int128_2_64(void)
+{
+    return (Int128) { 0, 1 };
+}
+
+static inline Int128 int128_add(Int128 a, Int128 b)
+{
+    Int128 r = { a.lo + b.lo, a.hi + b.hi };
+    r.hi += (r.lo < a.lo) || (r.lo < b.lo);
+    return r;
+}
+
+static inline Int128 int128_neg(Int128 a)
+{
+    a.lo = ~a.lo;
+    a.hi = ~a.hi;
+    return int128_add(a, int128_one());
+}
+
+static inline Int128 int128_sub(Int128 a, Int128 b)
+{
+    return int128_add(a, int128_neg(b));
+}
+
+static inline bool int128_nonneg(Int128 a)
+{
+    return a.hi >= 0;
+}
+
+static inline bool int128_eq(Int128 a, Int128 b)
+{
+    return a.lo == b.lo && a.hi == b.hi;
+}
+
+static inline bool int128_ne(Int128 a, Int128 b)
+{
+    return !int128_eq(a, b);
+}
+
+static inline bool int128_ge(Int128 a, Int128 b)
+{
+    return int128_nonneg(int128_sub(a, b));
+}
+
+static inline bool int128_lt(Int128 a, Int128 b)
+{
+    return !int128_ge(a, b);
+}
+
+static inline bool int128_le(Int128 a, Int128 b)
+{
+    return int128_ge(b, a);
+}
+
+static inline bool int128_gt(Int128 a, Int128 b)
+{
+    return !int128_le(a, b);
+}
+
+static inline bool int128_nz(Int128 a)
+{
+    return a.lo || a.hi;
+}
+
+static inline Int128 int128_min(Int128 a, Int128 b)
+{
+    return int128_le(a, b) ? a : b;
+}
+
+static inline Int128 int128_max(Int128 a, Int128 b)
+{
+    return int128_ge(a, b) ? a : b;
+}
+
+static inline void int128_addto(Int128 *a, Int128 b)
+{
+    *a = int128_add(*a, b);
+}
+
+static inline void int128_subfrom(Int128 *a, Int128 b)
+{
+    *a = int128_sub(*a, b);
+}
+
+#endif
-- 
1.7.6.3

[Qemu-devel] [PATCH 2/3] memory: use 128-bit integers for sizes and intermediates

[Avi Kivity]

Since the memory API supports 64-bit buses, it needs a larger type to represent
intermediate results.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 memory.c |  196 ++++++++++++++++++++++++++++++++++++-------------------------
 memory.h |    3 +-
 2 files changed, 118 insertions(+), 81 deletions(-)

diff --git a/memory.c b/memory.c
index dc5e35d..66142ea 100644
--- a/memory.c
+++ b/memory.c
@@ -28,43 +28,48 @@
  * (large MemoryRegion::alias_offset).
  */
 struct AddrRange {
-    int64_t start;
-    int64_t size;
+    Int128 start;
+    Int128 size;
 };
 
-static AddrRange addrrange_make(int64_t start, int64_t size)
+static AddrRange addrrange_make(Int128 start, Int128 size)
 {
     return (AddrRange) { start, size };
 }
 
 static bool addrrange_equal(AddrRange r1, AddrRange r2)
 {
-    return r1.start == r2.start && r1.size == r2.size;
+    return int128_eq(r1.start, r2.start) && int128_eq(r1.size, r2.size);
 }
 
-static int64_t addrrange_end(AddrRange r)
+static Int128 addrrange_end(AddrRange r)
 {
-    return r.start + r.size;
+    return int128_add(r.start, r.size);
 }
 
-static AddrRange addrrange_shift(AddrRange range, int64_t delta)
+static AddrRange addrrange_shift(AddrRange range, Int128 delta)
 {
-    range.start += delta;
+    int128_addto(&range.start, delta);
     return range;
 }
 
+static bool addrrange_contains(AddrRange range, Int128 addr)
+{
+    return int128_ge(addr, range.start)
+        && int128_lt(addr, addrrange_end(range));
+}
+
 static bool addrrange_intersects(AddrRange r1, AddrRange r2)
 {
-    return (r1.start >= r2.start && (r1.start - r2.start) < r2.size)
-        || (r2.start >= r1.start && (r2.start - r1.start) < r1.size);
+    return addrrange_contains(r1, r2.start)
+        || addrrange_contains(r2, r1.start);
 }
 
 static AddrRange addrrange_intersection(AddrRange r1, AddrRange r2)
 {
-    int64_t start = MAX(r1.start, r2.start);
-    /* off-by-one arithmetic to prevent overflow */
-    int64_t end = MIN(addrrange_end(r1) - 1, addrrange_end(r2) - 1);
-    return addrrange_make(start, end - start + 1);
+    Int128 start = int128_max(r1.start, r2.start);
+    Int128 end = int128_min(addrrange_end(r1), addrrange_end(r2));
+    return addrrange_make(start, int128_sub(end, start));
 }
 
 struct CoalescedMemoryRange {
@@ -82,13 +87,13 @@ struct MemoryRegionIoeventfd {
 static bool memory_region_ioeventfd_before(MemoryRegionIoeventfd a,
                                            MemoryRegionIoeventfd b)
 {
-    if (a.addr.start < b.addr.start) {
+    if (int128_lt(a.addr.start, b.addr.start)) {
         return true;
-    } else if (a.addr.start > b.addr.start) {
+    } else if (int128_gt(a.addr.start, b.addr.start)) {
         return false;
-    } else if (a.addr.size < b.addr.size) {
+    } else if (int128_lt(a.addr.size, b.addr.size)) {
         return true;
-    } else if (a.addr.size > b.addr.size) {
+    } else if (int128_gt(a.addr.size, b.addr.size)) {
         return false;
     } else if (a.match_data < b.match_data) {
         return true;
@@ -201,9 +206,11 @@ static void flatview_destroy(FlatView *view)
 
 static bool can_merge(FlatRange *r1, FlatRange *r2)
 {
-    return addrrange_end(r1->addr) == r2->addr.start
+    return int128_eq(addrrange_end(r1->addr), r2->addr.start)
         && r1->mr == r2->mr
-        && r1->offset_in_region + r1->addr.size == r2->offset_in_region
+        && int128_eq(int128_add(int128_make64(r1->offset_in_region),
+                                r1->addr.size),
+                     int128_make64(r2->offset_in_region))
         && r1->dirty_log_mask == r2->dirty_log_mask
         && r1->readable == r2->readable
         && r1->readonly == r2->readonly;
@@ -219,7 +226,7 @@ static void flatview_simplify(FlatView *view)
         j = i + 1;
         while (j < view->nr
                && can_merge(&view->ranges[j-1], &view->ranges[j])) {
-            view->ranges[i].addr.size += view->ranges[j].addr.size;
+            int128_addto(&view->ranges[i].addr.size, view->ranges[j].addr.size);
             ++j;
         }
         ++i;
@@ -314,8 +321,8 @@ static void as_memory_range_add(AddressSpace *as, FlatRange *fr)
         phys_offset |= IO_MEM_ROM;
     }
 
-    cpu_register_physical_memory_log(fr->addr.start,
-                                     fr->addr.size,
+    cpu_register_physical_memory_log(int128_get64(fr->addr.start),
+                                     int128_get64(fr->addr.size),
                                      phys_offset,
                                      region_offset,
                                      fr->dirty_log_mask);
@@ -324,30 +331,35 @@ static void as_memory_range_add(AddressSpace *as, FlatRange *fr)
 static void as_memory_range_del(AddressSpace *as, FlatRange *fr)
 {
     if (fr->dirty_log_mask) {
-        cpu_physical_sync_dirty_bitmap(fr->addr.start,
-                                       fr->addr.start + fr->addr.size);
+        Int128 end = addrrange_end(fr->addr);
+        cpu_physical_sync_dirty_bitmap(int128_get64(fr->addr.start),
+                                       int128_get64(end));
     }
-    cpu_register_physical_memory(fr->addr.start, fr->addr.size,
+    cpu_register_physical_memory(int128_get64(fr->addr.start),
+                                 int128_get64(fr->addr.size),
                                  IO_MEM_UNASSIGNED);
 }
 
 static void as_memory_log_start(AddressSpace *as, FlatRange *fr)
 {
-    cpu_physical_log_start(fr->addr.start, fr->addr.size);
+    cpu_physical_log_start(int128_get64(fr->addr.start),
+                           int128_get64(fr->addr.size));
 }
 
 static void as_memory_log_stop(AddressSpace *as, FlatRange *fr)
 {
-    cpu_physical_log_stop(fr->addr.start, fr->addr.size);
+    cpu_physical_log_stop(int128_get64(fr->addr.start),
+                          int128_get64(fr->addr.size));
 }
 
 static void as_memory_ioeventfd_add(AddressSpace *as, MemoryRegionIoeventfd *fd)
 {
     int r;
 
-    assert(fd->match_data && fd->addr.size == 4);
+    assert(fd->match_data && int128_get64(fd->addr.size) == 4);
 
-    r = kvm_set_ioeventfd_mmio_long(fd->fd, fd->addr.start, fd->data, true);
+    r = kvm_set_ioeventfd_mmio_long(fd->fd, int128_get64(fd->addr.start),
+                                    fd->data, true);
     if (r < 0) {
         abort();
     }
@@ -357,7 +369,8 @@ static void as_memory_ioeventfd_del(AddressSpace *as, MemoryRegionIoeventfd *fd)
 {
     int r;
 
-    r = kvm_set_ioeventfd_mmio_long(fd->fd, fd->addr.start, fd->data, false);
+    r = kvm_set_ioeventfd_mmio_long(fd->fd, int128_get64(fd->addr.start),
+                                    fd->data, false);
     if (r < 0) {
         abort();
     }
@@ -453,22 +466,24 @@ static void memory_region_iorange_write(IORange *iorange,
 static void as_io_range_add(AddressSpace *as, FlatRange *fr)
 {
     iorange_init(&fr->mr->iorange, &memory_region_iorange_ops,
-                 fr->addr.start,fr->addr.size);
+                 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(fr->addr.start, fr->addr.size);
+    isa_unassign_ioport(int128_get64(fr->addr.start),
+                        int128_get64(fr->addr.size));
 }
 
 static void as_io_ioeventfd_add(AddressSpace *as, MemoryRegionIoeventfd *fd)
 {
     int r;
 
-    assert(fd->match_data && fd->addr.size == 2);
+    assert(fd->match_data && int128_get64(fd->addr.size) == 2);
 
-    r = kvm_set_ioeventfd_pio_word(fd->fd, fd->addr.start, fd->data, true);
+    r = kvm_set_ioeventfd_pio_word(fd->fd, int128_get64(fd->addr.start),
+                                   fd->data, true);
     if (r < 0) {
         abort();
     }
@@ -478,7 +493,8 @@ static void as_io_ioeventfd_del(AddressSpace *as, MemoryRegionIoeventfd *fd)
 {
     int r;
 
-    r = kvm_set_ioeventfd_pio_word(fd->fd, fd->addr.start, fd->data, false);
+    r = kvm_set_ioeventfd_pio_word(fd->fd, int128_get64(fd->addr.start),
+                                   fd->data, false);
     if (r < 0) {
         abort();
     }
@@ -500,19 +516,19 @@ static void as_io_ioeventfd_del(AddressSpace *as, MemoryRegionIoeventfd *fd)
  */
 static void render_memory_region(FlatView *view,
                                  MemoryRegion *mr,
-                                 target_phys_addr_t base,
+                                 Int128 base,
                                  AddrRange clip,
                                  bool readonly)
 {
     MemoryRegion *subregion;
     unsigned i;
     target_phys_addr_t offset_in_region;
-    int64_t remain;
-    int64_t now;
+    Int128 remain;
+    Int128 now;
     FlatRange fr;
     AddrRange tmp;
 
-    base += mr->addr;
+    int128_addto(&base, int128_make64(mr->addr));
     readonly |= mr->readonly;
 
     tmp = addrrange_make(base, mr->size);
@@ -524,8 +540,8 @@ static void render_memory_region(FlatView *view,
     clip = addrrange_intersection(tmp, clip);
 
     if (mr->alias) {
-        base -= mr->alias->addr;
-        base -= mr->alias_offset;
+        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);
         return;
     }
@@ -539,17 +555,18 @@ static void render_memory_region(FlatView *view,
         return;
     }
 
-    offset_in_region = clip.start - base;
+    offset_in_region = int128_get64(int128_sub(clip.start, base));
     base = clip.start;
     remain = clip.size;
 
     /* Render the region itself into any gaps left by the current view. */
-    for (i = 0; i < view->nr && remain; ++i) {
-        if (base >= addrrange_end(view->ranges[i].addr)) {
+    for (i = 0; i < view->nr && int128_nz(remain); ++i) {
+        if (int128_ge(base, addrrange_end(view->ranges[i].addr))) {
             continue;
         }
-        if (base < view->ranges[i].addr.start) {
-            now = MIN(remain, view->ranges[i].addr.start - base);
+        if (int128_lt(base, view->ranges[i].addr.start)) {
+            now = int128_min(remain,
+                             int128_sub(view->ranges[i].addr.start, base));
             fr.mr = mr;
             fr.offset_in_region = offset_in_region;
             fr.addr = addrrange_make(base, now);
@@ -558,18 +575,18 @@ static void render_memory_region(FlatView *view,
             fr.readonly = readonly;
             flatview_insert(view, i, &fr);
             ++i;
-            base += now;
-            offset_in_region += now;
-            remain -= now;
+            int128_addto(&base, now);
+            offset_in_region += int128_get64(now);
+            int128_subfrom(&remain, now);
         }
-        if (base == view->ranges[i].addr.start) {
-            now = MIN(remain, view->ranges[i].addr.size);
-            base += now;
-            offset_in_region += now;
-            remain -= now;
+        if (int128_eq(base, view->ranges[i].addr.start)) {
+            now = int128_min(remain, view->ranges[i].addr.size);
+            int128_addto(&base, now);
+            offset_in_region += int128_get64(now);
+            int128_subfrom(&remain, now);
         }
     }
-    if (remain) {
+    if (int128_nz(remain)) {
         fr.mr = mr;
         fr.offset_in_region = offset_in_region;
         fr.addr = addrrange_make(base, remain);
@@ -587,7 +604,8 @@ static FlatView generate_memory_topology(MemoryRegion *mr)
 
     flatview_init(&view);
 
-    render_memory_region(&view, mr, 0, addrrange_make(0, INT64_MAX), false);
+    render_memory_region(&view, mr, int128_zero(),
+                         addrrange_make(int128_zero(), int128_2_64()), false);
     flatview_simplify(&view);
 
     return view;
@@ -637,7 +655,8 @@ static void address_space_update_ioeventfds(AddressSpace *as)
     FOR_EACH_FLAT_RANGE(fr, &as->current_map) {
         for (i = 0; i < fr->mr->ioeventfd_nb; ++i) {
             tmp = addrrange_shift(fr->mr->ioeventfds[i].addr,
-                                  fr->addr.start - fr->offset_in_region);
+                                  int128_sub(fr->addr.start,
+                                             int128_make64(fr->offset_in_region)));
             if (addrrange_intersects(fr->addr, tmp)) {
                 ++ioeventfd_nb;
                 ioeventfds = g_realloc(ioeventfds,
@@ -682,8 +701,8 @@ static void address_space_update_topology_pass(AddressSpace *as,
 
         if (frold
             && (!frnew
-                || frold->addr.start < frnew->addr.start
-                || (frold->addr.start == frnew->addr.start
+                || int128_lt(frold->addr.start, frnew->addr.start)
+                || (int128_eq(frold->addr.start, frnew->addr.start)
                     && !flatrange_equal(frold, frnew)))) {
             /* In old, but (not in new, or in new but attributes changed). */
 
@@ -788,7 +807,10 @@ void memory_region_init(MemoryRegion *mr,
 {
     mr->ops = NULL;
     mr->parent = NULL;
-    mr->size = size;
+    mr->size = int128_make64(size);
+    if (size == UINT64_MAX) {
+        mr->size = int128_2_64();
+    }
     mr->addr = 0;
     mr->offset = 0;
     mr->terminates = false;
@@ -1014,7 +1036,10 @@ void memory_region_destroy(MemoryRegion *mr)
 
 uint64_t memory_region_size(MemoryRegion *mr)
 {
-    return mr->size;
+    if (int128_eq(mr->size, int128_2_64())) {
+        return UINT64_MAX;
+    }
+    return int128_get64(mr->size);
 }
 
 void memory_region_set_offset(MemoryRegion *mr, target_phys_addr_t offset)
@@ -1049,8 +1074,8 @@ void memory_region_sync_dirty_bitmap(MemoryRegion *mr)
 
     FOR_EACH_FLAT_RANGE(fr, &address_space_memory.current_map) {
         if (fr->mr == mr) {
-            cpu_physical_sync_dirty_bitmap(fr->addr.start,
-                                           fr->addr.start + fr->addr.size);
+            cpu_physical_sync_dirty_bitmap(int128_get64(fr->addr.start),
+                                           int128_get64(addrrange_end(fr->addr)));
         }
     }
 }
@@ -1099,15 +1124,18 @@ static void memory_region_update_coalesced_range(MemoryRegion *mr)
 
     FOR_EACH_FLAT_RANGE(fr, &address_space_memory.current_map) {
         if (fr->mr == mr) {
-            qemu_unregister_coalesced_mmio(fr->addr.start, fr->addr.size);
+            qemu_unregister_coalesced_mmio(int128_get64(fr->addr.start),
+                                           int128_get64(fr->addr.size));
             QTAILQ_FOREACH(cmr, &mr->coalesced, link) {
                 tmp = addrrange_shift(cmr->addr,
-                                      fr->addr.start - fr->offset_in_region);
+                                      int128_sub(fr->addr.start,
+                                                 int128_make64(fr->offset_in_region)));
                 if (!addrrange_intersects(tmp, fr->addr)) {
                     continue;
                 }
                 tmp = addrrange_intersection(tmp, fr->addr);
-                qemu_register_coalesced_mmio(tmp.start, tmp.size);
+                qemu_register_coalesced_mmio(int128_get64(tmp.start),
+                                             int128_get64(tmp.size));
             }
         }
     }
@@ -1116,7 +1144,7 @@ static void memory_region_update_coalesced_range(MemoryRegion *mr)
 void memory_region_set_coalescing(MemoryRegion *mr)
 {
     memory_region_clear_coalescing(mr);
-    memory_region_add_coalescing(mr, 0, mr->size);
+    memory_region_add_coalescing(mr, 0, int128_get64(mr->size));
 }
 
 void memory_region_add_coalescing(MemoryRegion *mr,
@@ -1125,7 +1153,7 @@ void memory_region_add_coalescing(MemoryRegion *mr,
 {
     CoalescedMemoryRange *cmr = g_malloc(sizeof(*cmr));
 
-    cmr->addr = addrrange_make(offset, size);
+    cmr->addr = addrrange_make(int128_make64(offset), int128_make64(size));
     QTAILQ_INSERT_TAIL(&mr->coalesced, cmr, link);
     memory_region_update_coalesced_range(mr);
 }
@@ -1150,8 +1178,8 @@ void memory_region_add_eventfd(MemoryRegion *mr,
                                int fd)
 {
     MemoryRegionIoeventfd mrfd = {
-        .addr.start = addr,
-        .addr.size = size,
+        .addr.start = int128_make64(addr),
+        .addr.size = int128_make64(size),
         .match_data = match_data,
         .data = data,
         .fd = fd,
@@ -1180,8 +1208,8 @@ void memory_region_del_eventfd(MemoryRegion *mr,
                                int fd)
 {
     MemoryRegionIoeventfd mrfd = {
-        .addr.start = addr,
-        .addr.size = size,
+        .addr.start = int128_make64(addr),
+        .addr.size = int128_make64(size),
         .match_data = match_data,
         .data = data,
         .fd = fd,
@@ -1215,18 +1243,20 @@ static void memory_region_add_subregion_common(MemoryRegion *mr,
         if (subregion->may_overlap || other->may_overlap) {
             continue;
         }
-        if (offset >= other->addr + other->size
-            || offset + subregion->size <= other->addr) {
+        if (int128_gt(int128_make64(offset),
+                      int128_add(int128_make64(other->addr), other->size))
+            || int128_le(int128_add(int128_make64(offset), subregion->size),
+                         int128_make64(other->addr))) {
             continue;
         }
 #if 0
         printf("warning: subregion collision %llx/%llx (%s) "
                "vs %llx/%llx (%s)\n",
                (unsigned long long)offset,
-               (unsigned long long)subregion->size,
+               (unsigned long long)int128_get64(subregion->size),
                subregion->name,
                (unsigned long long)other->addr,
-               (unsigned long long)other->size,
+               (unsigned long long)int128_get64(other->size),
                other->name);
 #endif
     }
@@ -1330,16 +1360,22 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
         mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): alias %s @%s "
                    TARGET_FMT_plx "-" TARGET_FMT_plx "\n",
                    base + mr->addr,
-                   base + mr->addr + (target_phys_addr_t)mr->size - 1,
+                   base + mr->addr
+                   + (target_phys_addr_t)int128_get64(int128_sub(mr->size,
+                                                                 int128_one())),
                    mr->priority,
                    mr->name,
                    mr->alias->name,
                    mr->alias_offset,
-                   mr->alias_offset + (target_phys_addr_t)mr->size - 1);
+                   mr->alias_offset
+                   + (target_phys_addr_t)int128_get64(int128_sub(mr->size,
+                                                                 int128_one())));
     } else {
         mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d): %s\n",
                    base + mr->addr,
-                   base + mr->addr + (target_phys_addr_t)mr->size - 1,
+                   base + mr->addr
+                   + (target_phys_addr_t)int128_get64(int128_sub(mr->size,
+                                                                 int128_one())),
                    mr->priority,
                    mr->name);
     }
diff --git a/memory.h b/memory.h
index d5b47da..7fb36d1 100644
--- a/memory.h
+++ b/memory.h
@@ -24,6 +24,7 @@
 #include "qemu-queue.h"
 #include "iorange.h"
 #include "ioport.h"
+#include "int128.h"
 
 typedef struct MemoryRegionOps MemoryRegionOps;
 typedef struct MemoryRegion MemoryRegion;
@@ -105,7 +106,7 @@ struct MemoryRegion {
     const MemoryRegionOps *ops;
     void *opaque;
     MemoryRegion *parent;
-    uint64_t size;
+    Int128 size;
     target_phys_addr_t addr;
     target_phys_addr_t offset;
     bool backend_registered;
-- 
1.7.6.3

[Qemu-devel] [PATCH 3/3] Adjust system and pci address spaces to full 64-bit

[Avi Kivity]

Now that the memory API supports full 64-bit buses, adjust the relevant
callers to take advantage of it.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 exec.c          |    2 +-
 hw/pc_piix.c    |    2 +-
 hw/pci_bridge.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/exec.c b/exec.c
index d0cbf15..16e37a7 100644
--- a/exec.c
+++ b/exec.c
@@ -3825,7 +3825,7 @@ static void io_mem_init(void)
 static void memory_map_init(void)
 {
     system_memory = g_malloc(sizeof(*system_memory));
-    memory_region_init(system_memory, "system", INT64_MAX);
+    memory_region_init(system_memory, "system", UINT64_MAX);
     set_system_memory_map(system_memory);
 
     system_io = g_malloc(sizeof(*system_io));
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index ce1c87f..45540e5 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -115,7 +115,7 @@ static void pc_init1(MemoryRegion *system_memory,
 
     if (pci_enabled) {
         pci_memory = g_new(MemoryRegion, 1);
-        memory_region_init(pci_memory, "pci", INT64_MAX);
+        memory_region_init(pci_memory, "pci", UINT64_MAX);
         rom_memory = pci_memory;
     } else {
         pci_memory = NULL;
diff --git a/hw/pci_bridge.c b/hw/pci_bridge.c
index b6287cd..3b786aa 100644
--- a/hw/pci_bridge.c
+++ b/hw/pci_bridge.c
@@ -319,7 +319,7 @@ int pci_bridge_initfn(PCIDevice *dev)
     sec_bus->parent_dev = dev;
     sec_bus->map_irq = br->map_irq;
     sec_bus->address_space_mem = &br->address_space_mem;
-    memory_region_init(&br->address_space_mem, "pci_pridge_pci", INT64_MAX);
+    memory_region_init(&br->address_space_mem, "pci_pridge_pci", UINT64_MAX);
     sec_bus->address_space_io = &br->address_space_io;
     memory_region_init(&br->address_space_io, "pci_bridge_io", 65536);
     pci_bridge_region_init(br);
-- 
1.7.6.3

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Anthony Liguori]

On 10/30/2011 09:02 AM, Avi Kivity wrote:
> This somewhat controversial patchset converts internal arithmetic in the
> memory API to 128 bits.
>
> It has been argued that with careful coding we can make 64-bit work as
> well.  I don't think this is true in general - a memory router can adjust
> addresses either forwards or backwards, and some buses (PCIe) need the
> full 64-bit space - though it's probably the case for all the configurations
> we support today.  Regardless, the need for careful coding means subtle bugs,
> which I don't want in a core API that is driven by guest supplied values.

The primary need for signed arithmetic is aliases, correct?

Where do we actually make use of this in practice?   I think having negative 
address spaces is a weird aspect of the memory api and wonder if refactoring it 
away is a better solution tot he problem.

Regards,

Anthony Liguori

>
> Avi Kivity (3):
>    Add support for 128-bit arithmetic
>    memory: use 128-bit integers for sizes and intermediates
>    Adjust system and pci address spaces to full 64-bit
>
>   exec.c          |    2 +-
>   hw/pc_piix.c    |    2 +-
>   hw/pci_bridge.c |    2 +-
>   int128.h        |  116 ++++++++++++++++++++++++++++++++
>   memory.c        |  196 ++++++++++++++++++++++++++++++++----------------------
>   memory.h        |    3 +-
>   6 files changed, 237 insertions(+), 84 deletions(-)
>   create mode 100644 int128.h
>

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Avi Kivity]

On 10/30/2011 04:12 PM, Anthony Liguori wrote:
> On 10/30/2011 09:02 AM, Avi Kivity wrote:
>> This somewhat controversial patchset converts internal arithmetic in the
>> memory API to 128 bits.
>>
>> It has been argued that with careful coding we can make 64-bit work as
>> well.  I don't think this is true in general - a memory router can
>> adjust
>> addresses either forwards or backwards, and some buses (PCIe) need the
>> full 64-bit space - though it's probably the case for all the
>> configurations
>> we support today.  Regardless, the need for careful coding means
>> subtle bugs,
>> which I don't want in a core API that is driven by guest supplied
>> values.
>
> The primary need for signed arithmetic is aliases, correct?

> Where do we actually make use of this in practice?   I think having
> negative address spaces is a weird aspect of the memory api and wonder
> if refactoring it away is a better solution tot he problem.
>

There is no direct use of signed arithmetic in the API (just in the
implementation).  Aliases can cause a region to move in either the
positive or negative direction, and this requires either signed
arithmetic or special casing the two directions.

Signed arithmetic is not the only motivation - overflow is another. 
Nothing prevents a user from placing a 64-bit 4k BAR at address
ffff_ffff_ffff_f000; we could move to base/limit representation, but
that will likely cause its own bugs.  Finally, we should be able to
represent both a 0-sized region and a 2^64 sized region.

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

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Blue Swirl]

On Sun, Oct 30, 2011 at 14:19, Avi Kivity <avi@redhat.com> wrote:
> On 10/30/2011 04:12 PM, Anthony Liguori wrote:
>> On 10/30/2011 09:02 AM, Avi Kivity wrote:
>>> This somewhat controversial patchset converts internal arithmetic in the
>>> memory API to 128 bits.
>>>
>>> It has been argued that with careful coding we can make 64-bit work as
>>> well.  I don't think this is true in general - a memory router can
>>> adjust
>>> addresses either forwards or backwards, and some buses (PCIe) need the
>>> full 64-bit space - though it's probably the case for all the
>>> configurations
>>> we support today.  Regardless, the need for careful coding means
>>> subtle bugs,
>>> which I don't want in a core API that is driven by guest supplied
>>> values.
>>
>> The primary need for signed arithmetic is aliases, correct?
>
>> Where do we actually make use of this in practice?   I think having
>> negative address spaces is a weird aspect of the memory api and wonder
>> if refactoring it away is a better solution tot he problem.
>>
>
> There is no direct use of signed arithmetic in the API (just in the
> implementation).  Aliases can cause a region to move in either the
> positive or negative direction, and this requires either signed
> arithmetic or special casing the two directions.
>
> Signed arithmetic is not the only motivation - overflow is another.
> Nothing prevents a user from placing a 64-bit 4k BAR at address
> ffff_ffff_ffff_f000; we could move to base/limit representation, but
> that will likely cause its own bugs.  Finally, we should be able to
> represent both a 0-sized region and a 2^64 sized region.

It looks like 64 bit saturating arithmetic could also work. It should
also be possible to work only with (start, end) address pairs and
never with start + size, then 2^64 shouldn't be an issue.

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Avi Kivity]

On 10/30/2011 04:59 PM, Blue Swirl wrote:
> >
> > There is no direct use of signed arithmetic in the API (just in the
> > implementation).  Aliases can cause a region to move in either the
> > positive or negative direction, and this requires either signed
> > arithmetic or special casing the two directions.
> >
> > Signed arithmetic is not the only motivation - overflow is another.
> > Nothing prevents a user from placing a 64-bit 4k BAR at address
> > ffff_ffff_ffff_f000; we could move to base/limit representation, but
> > that will likely cause its own bugs.  Finally, we should be able to
> > represent both a 0-sized region and a 2^64 sized region.
>
> It looks like 64 bit saturating arithmetic could also work. 

Depends when you saturate.  The standard example (vga) takes a alias of
(say) 0xe01a0000 and aliases it into 0x000a0000.  So you need to adjust
addresses downwards by -0xe0100000.  That brings the start of the real
region (0xe0000000) into the negative territory.  Saturating it there
would break it.

> It should
> also be possible to work only with (start, end) address pairs and
> never with start + size, then 2^64 shouldn't be an issue.

Then 0 becomes an issue (end < start).

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

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[David Gibson]

On Sun, Oct 30, 2011 at 04:19:51PM +0200, Avi Kivity wrote:
> On 10/30/2011 04:12 PM, Anthony Liguori wrote:
> > On 10/30/2011 09:02 AM, Avi Kivity wrote:
> >> This somewhat controversial patchset converts internal arithmetic in the
> >> memory API to 128 bits.
> >>
> >> It has been argued that with careful coding we can make 64-bit work as
> >> well.  I don't think this is true in general - a memory router can
> >> adjust
> >> addresses either forwards or backwards, and some buses (PCIe) need the
> >> full 64-bit space - though it's probably the case for all the
> >> configurations
> >> we support today.  Regardless, the need for careful coding means
> >> subtle bugs,
> >> which I don't want in a core API that is driven by guest supplied
> >> values.
> >
> > The primary need for signed arithmetic is aliases, correct?
> 
> > Where do we actually make use of this in practice?   I think having
> > negative address spaces is a weird aspect of the memory api and wonder
> > if refactoring it away is a better solution tot he problem.
> 
> There is no direct use of signed arithmetic in the API (just in the
> implementation).  Aliases can cause a region to move in either the
> positive or negative direction, and this requires either signed
> arithmetic or special casing the two directions.

You keep saying we need signed arithmetic for this, but it's not
really true.  *If* you see aliases as shifting the entire aliases
address space w.r.t., then just allowing a window to show through, you
get negative offsets, yes, but that's by no means the only way t think
about it.

It's basically one spot - the alias handling in render_memory_region()
- that generates a negative start intermediate.  I'm convinced it's
pretty straightforward to remove this - making a patch for it just
hasn't bubbled to the top of my priority queue, though.

> Signed arithmetic is not the only motivation - overflow is another. 
> Nothing prevents a user from placing a 64-bit 4k BAR at address
> ffff_ffff_ffff_f000; we could move to base/limit representation, but
> that will likely cause its own bugs.  Finally, we should be able to
> represent both a 0-sized region and a 2^64 sized region.

Note that an (inclusive) start/end representation also cannot
represent a 0 sized region.
-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Avi Kivity]

On 10/31/2011 02:36 AM, David Gibson wrote:
> > 
> > There is no direct use of signed arithmetic in the API (just in the
> > implementation).  Aliases can cause a region to move in either the
> > positive or negative direction, and this requires either signed
> > arithmetic or special casing the two directions.
>
> You keep saying we need signed arithmetic for this, but it's not
> really true.  *If* you see aliases as shifting the entire aliases
> address space w.r.t., then just allowing a window to show through, you
> get negative offsets, yes, but that's by no means the only way t think
> about it.

Obviously it's not the only way.  We could insert checks for the
direction, and for overflow/underflow.  But I am looking for the most
reliable way to prevent similar issues from popping up.  There have been
at least three bugs in this area.

If we can use a heavy hammer here, it is worthwhile IMO.  Sorry for
being a little trollish, but I much prefer replacing function calls with
infix operators, than getting a CVE for some overflow.

> It's basically one spot - the alias handling in render_memory_region()
> - that generates a negative start intermediate.  I'm convinced it's
> pretty straightforward to remove this - making a patch for it just
> hasn't bubbled to the top of my priority queue, though.

We keep adding, subtracting, and comparing stuff everywhere.  I am
fairly certain that you are right and there are no other trouble spot,
but I am not absolutely sure, and I would like to be.

> > Signed arithmetic is not the only motivation - overflow is another. 
> > Nothing prevents a user from placing a 64-bit 4k BAR at address
> > ffff_ffff_ffff_f000; we could move to base/limit representation, but
> > that will likely cause its own bugs.  Finally, we should be able to
> > represent both a 0-sized region and a 2^64 sized region.
>
> Note that an (inclusive) start/end representation also cannot
> represent a 0 sized region.

Right.  In theory we shouldn't generate zero sized regions, but can we
trust call device code not to do that?

Also, start/end or off-by-one size are easy to get wrong since C
programmers assume half-inclusive regions.

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

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Anthony Liguori]

On 10/30/2011 09:02 AM, Avi Kivity wrote:
> This somewhat controversial patchset converts internal arithmetic in the
> memory API to 128 bits.

Given the level of controversy, what do you think about deferring this to 1.1?

Regards,

Anthony Liguori

>
> It has been argued that with careful coding we can make 64-bit work as
> well.  I don't think this is true in general - a memory router can adjust
> addresses either forwards or backwards, and some buses (PCIe) need the
> full 64-bit space - though it's probably the case for all the configurations
> we support today.  Regardless, the need for careful coding means subtle bugs,
> which I don't want in a core API that is driven by guest supplied values.
>
> Avi Kivity (3):
>    Add support for 128-bit arithmetic
>    memory: use 128-bit integers for sizes and intermediates
>    Adjust system and pci address spaces to full 64-bit
>
>   exec.c          |    2 +-
>   hw/pc_piix.c    |    2 +-
>   hw/pci_bridge.c |    2 +-
>   int128.h        |  116 ++++++++++++++++++++++++++++++++
>   memory.c        |  196 ++++++++++++++++++++++++++++++++----------------------
>   memory.h        |    3 +-
>   6 files changed, 237 insertions(+), 84 deletions(-)
>   create mode 100644 int128.h
>

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[David Gibson]

On Mon, Oct 31, 2011 at 11:05:47AM -0500, Anthony Liguori wrote:
> On 10/30/2011 09:02 AM, Avi Kivity wrote:
> >This somewhat controversial patchset converts internal arithmetic in the
> >memory API to 128 bits.
> 
> Given the level of controversy, what do you think about deferring
> this to 1.1?

If it's deferred then one of my rearrangements for the arithmetic must
go in instead.  These patches fix real bugs, that bite us on pseries.
It's not the only way to fix those bugs, and probably not even my
personally preferred way to fix them, but they need to be fixed
_somehow_ for 1.0.


> 
> Regards,
> 
> Anthony Liguori
> 
> >
> >It has been argued that with careful coding we can make 64-bit work as
> >well.  I don't think this is true in general - a memory router can adjust
> >addresses either forwards or backwards, and some buses (PCIe) need the
> >full 64-bit space - though it's probably the case for all the configurations
> >we support today.  Regardless, the need for careful coding means subtle bugs,
> >which I don't want in a core API that is driven by guest supplied values.
> >
> >Avi Kivity (3):
> >   Add support for 128-bit arithmetic
> >   memory: use 128-bit integers for sizes and intermediates
> >   Adjust system and pci address spaces to full 64-bit
> >
> >  exec.c          |    2 +-
> >  hw/pc_piix.c    |    2 +-
> >  hw/pci_bridge.c |    2 +-
> >  int128.h        |  116 ++++++++++++++++++++++++++++++++
> >  memory.c        |  196 ++++++++++++++++++++++++++++++++----------------------
> >  memory.h        |    3 +-
> >  6 files changed, 237 insertions(+), 84 deletions(-)
> >  create mode 100644 int128.h
> >
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Avi Kivity]

On 11/01/2011 02:54 AM, David Gibson wrote:
> On Mon, Oct 31, 2011 at 11:05:47AM -0500, Anthony Liguori wrote:
> > On 10/30/2011 09:02 AM, Avi Kivity wrote:
> > >This somewhat controversial patchset converts internal arithmetic in the
> > >memory API to 128 bits.
> > 
> > Given the level of controversy, what do you think about deferring
> > this to 1.1?
>
> If it's deferred then one of my rearrangements for the arithmetic must
> go in instead.  These patches fix real bugs, that bite us on pseries.
> It's not the only way to fix those bugs, and probably not even my
> personally preferred way to fix them, but they need to be fixed
> _somehow_ for 1.0.

Yes, plus if one of them is exploitable, then it's certainly a must for 1.0.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Anthony Liguori]

On 11/01/2011 03:43 AM, Avi Kivity wrote:
> On 11/01/2011 02:54 AM, David Gibson wrote:
>> On Mon, Oct 31, 2011 at 11:05:47AM -0500, Anthony Liguori wrote:
>>> On 10/30/2011 09:02 AM, Avi Kivity wrote:
>>>> This somewhat controversial patchset converts internal arithmetic in the
>>>> memory API to 128 bits.
>>>
>>> Given the level of controversy, what do you think about deferring
>>> this to 1.1?
>>
>> If it's deferred then one of my rearrangements for the arithmetic must
>> go in instead.  These patches fix real bugs, that bite us on pseries.
>> It's not the only way to fix those bugs, and probably not even my
>> personally preferred way to fix them, but they need to be fixed
>> _somehow_ for 1.0.
>
> Yes, plus if one of them is exploitable, then it's certainly a must for 1.0.

Since it's just internal, I'll just pull this series and if we want to change it 
post 1.0, we can.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Andreas Färber]

Am 01.11.2011 13:59, schrieb Anthony Liguori:
> On 11/01/2011 03:43 AM, Avi Kivity wrote:
>> On 11/01/2011 02:54 AM, David Gibson wrote:
>>> On Mon, Oct 31, 2011 at 11:05:47AM -0500, Anthony Liguori wrote:
>>>> On 10/30/2011 09:02 AM, Avi Kivity wrote:
>>>>> This somewhat controversial patchset converts internal arithmetic
>>>>> in the
>>>>> memory API to 128 bits.
>>>>
>>>> Given the level of controversy, what do you think about deferring
>>>> this to 1.1?
>>>
>>> If it's deferred then one of my rearrangements for the arithmetic must
>>> go in instead.  These patches fix real bugs, that bite us on pseries.
>>> It's not the only way to fix those bugs, and probably not even my
>>> personally preferred way to fix them, but they need to be fixed
>>> _somehow_ for 1.0.
>>
>> Yes, plus if one of them is exploitable, then it's certainly a must
>> for 1.0.
> 
> Since it's just internal, I'll just pull this series and if we want to
> change it post 1.0, we can.

FWIW I must say I don't like where this is heading... iiuc just because
of a zero-or-full-64-bits issue with start+end we're doubling the
internal storage format for all memory ranges. If having the size
unsigned would eliminate the overflow issue at hand, can't we move the
signedness to some flag field instead?
I don't see a problem with using macros/inlines, just with the seemingly
unnecessary 128-bitness. In particular I'm thinking of ARM.

Since this seems to be addressing an overflow bug in ppc64, the
hard-freeze date shouldn't make us rush this IMO.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Anthony Liguori]

On 10/30/2011 09:02 AM, Avi Kivity wrote:
> This somewhat controversial patchset converts internal arithmetic in the
> memory API to 128 bits.
>
> It has been argued that with careful coding we can make 64-bit work as
> well.  I don't think this is true in general - a memory router can adjust
> addresses either forwards or backwards, and some buses (PCIe) need the
> full 64-bit space - though it's probably the case for all the configurations
> we support today.  Regardless, the need for careful coding means subtle bugs,
> which I don't want in a core API that is driven by guest supplied values.
>
> Avi Kivity (3):
>    Add support for 128-bit arithmetic
>    memory: use 128-bit integers for sizes and intermediates
>    Adjust system and pci address spaces to full 64-bit

I now notice that this is not a PULL request...  Did you mess up the subject or 
the pull request?

Regards,

Anthony Liguori

>
>   exec.c          |    2 +-
>   hw/pc_piix.c    |    2 +-
>   hw/pci_bridge.c |    2 +-
>   int128.h        |  116 ++++++++++++++++++++++++++++++++
>   memory.c        |  196 ++++++++++++++++++++++++++++++++----------------------
>   memory.h        |    3 +-
>   6 files changed, 237 insertions(+), 84 deletions(-)
>   create mode 100644 int128.h
>

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Avi Kivity]

On 11/01/2011 08:08 PM, Anthony Liguori wrote:
> On 10/30/2011 09:02 AM, Avi Kivity wrote:
>> This somewhat controversial patchset converts internal arithmetic in the
>> memory API to 128 bits.
>>
>> It has been argued that with careful coding we can make 64-bit work as
>> well.  I don't think this is true in general - a memory router can
>> adjust
>> addresses either forwards or backwards, and some buses (PCIe) need the
>> full 64-bit space - though it's probably the case for all the
>> configurations
>> we support today.  Regardless, the need for careful coding means
>> subtle bugs,
>> which I don't want in a core API that is driven by guest supplied
>> values.
>>
>> Avi Kivity (3):
>>    Add support for 128-bit arithmetic
>>    memory: use 128-bit integers for sizes and intermediates
>>    Adjust system and pci address spaces to full 64-bit
>
> I now notice that this is not a PULL request...  Did you mess up the
> subject or the pull request?
>

I forgot to supply the pull info, which is:

  git://github.com/avikivity/qemu.git memory/int128

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

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Avi Kivity]

On 11/01/2011 03:48 PM, Andreas Färber wrote:
> > 
> > Since it's just internal, I'll just pull this series and if we want to
> > change it post 1.0, we can.
>
> FWIW I must say I don't like where this is heading... iiuc just because
> of a zero-or-full-64-bits issue with start+end 

It's not just that issue.

> we're doubling the
> internal storage format for all memory ranges. 

It's not doubled, since the MemoryRegion structure has many other
members.  And anyway there are too few such structures to matter.

On the other hand, when we get to rewrite the core we can eliminate the
PageDesc array at 16 bytes per guest page, that is a really significant
saving.

> If having the size
> unsigned would eliminate the overflow issue at hand, can't we move the
> signedness to some flag field instead?

You mean a 65-bit integer?

> I don't see a problem with using macros/inlines, just with the seemingly
> unnecessary 128-bitness. In particular I'm thinking of ARM.

We could rename Int128 something like AddrArith and make it 64-bit for
archs with 32-bit target_phys_addr_t.  However I don't think there's
much point.  This code is executed very rarely, at initialization time
and when the physical address map changes (like when mapping a BAR). 
There's no point in optimizing it (and every point for making it robust).

> Since this seems to be addressing an overflow bug in ppc64, the
> hard-freeze date shouldn't make us rush this IMO.

The bug it fixes is actually in code not yet merged.  But I'm certain
there are more lurking in there, we're taking guest supplied 64-bit
values and using 64-bit arithmetic on them.  It has to overflow.

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

Re: [Qemu-devel] [PULL 0/3] 128-bit support for the memory API

[Anthony Liguori]

On 11/02/2011 05:10 AM, Avi Kivity wrote:
> On 11/01/2011 08:08 PM, Anthony Liguori wrote:
>> On 10/30/2011 09:02 AM, Avi Kivity wrote:
>>> This somewhat controversial patchset converts internal arithmetic in the
>>> memory API to 128 bits.
>>>
>>> It has been argued that with careful coding we can make 64-bit work as
>>> well.  I don't think this is true in general - a memory router can
>>> adjust
>>> addresses either forwards or backwards, and some buses (PCIe) need the
>>> full 64-bit space - though it's probably the case for all the
>>> configurations
>>> we support today.  Regardless, the need for careful coding means
>>> subtle bugs,
>>> which I don't want in a core API that is driven by guest supplied
>>> values.
>>>
>>> Avi Kivity (3):
>>>     Add support for 128-bit arithmetic
>>>     memory: use 128-bit integers for sizes and intermediates
>>>     Adjust system and pci address spaces to full 64-bit
>>
>> I now notice that this is not a PULL request...  Did you mess up the
>> subject or the pull request?
>>
>
> I forgot to supply the pull info, which is:
>
>    git://github.com/avikivity/qemu.git memory/int128

Pulled.  Thanks.

Regards,

Anthony Liguori

>