qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations
@ 2015-11-05 16:18 Pierre Morel
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 1/7] int128: use unsigned 128 bit arithmetic Pierre Morel
                   ` (7 more replies)
  0 siblings, 8 replies; 14+ messages in thread
From: Pierre Morel @ 2015-11-05 16:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: cornelia.huck, pbonzini, pmorel, peter.maydell

The size of a memory area can never be negative.
It follows it must be defined as an unsigned value.
    
Let's modify the memory regions size to unsigned 128 integer
and modify accordingly the 128 bit arithmetic.
    
This makes memory size calculations easier and easier to understand.
    
I fear loud protest but really, it is a broken concept that
obfuscate the code.


Pierre Morel (7):
  int128: use unsigned 128 bit arithmetic
  memory: modify memory size for an unsigned 128bit int
  128bit: adapt core files for unsigned 128bits
  128bit: adapt sparc mmu_helper for UInt128
  128bit: adapt VFIO for UInt128 arithmetic
  128bit: adapt Virtio for UInt128 arithmetic
  128bits: some HW components use 128bit arithmetic.

 exec.c                       |   32 ++++----
 hw/core/loader.c             |    2 +-
 hw/display/exynos4210_fimd.c |    4 +-
 hw/display/framebuffer.c     |    2 +-
 hw/mem/pc-dimm.c             |    6 +-
 hw/pci-host/ppce500.c        |    2 +-
 hw/vfio/common.c             |   20 +++---
 hw/virtio/dataplane/vring.c  |    2 +-
 hw/virtio/vhost.c            |    4 +-
 hw/virtio/virtio-balloon.c   |    2 +-
 include/exec/memory.h        |    4 +-
 include/qemu/int128.h        |   95 ++++++++++-----------
 kvm-all.c                    |   16 ++--
 memory.c                     |  184 +++++++++++++++++++++---------------------
 memory_mapping.c             |    2 +-
 target-sparc/mmu_helper.c    |    2 +-
 16 files changed, 187 insertions(+), 192 deletions(-)

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 1/7] int128: use unsigned 128 bit arithmetic
  2015-11-05 16:18 [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations Pierre Morel
@ 2015-11-05 16:18 ` Pierre Morel
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 2/7] memory: modify memory size for an unsigned 128bit int Pierre Morel
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Pierre Morel @ 2015-11-05 16:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: cornelia.huck, pbonzini, pmorel, peter.maydell

Let's change Int128 definition to UInt128,
modify the internal element to uint64_t and
the arithmetic functions accordingly.

Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
---
 include/qemu/int128.h |   95 +++++++++++++++++++++++--------------------------
 1 files changed, 45 insertions(+), 50 deletions(-)

diff --git a/include/qemu/int128.h b/include/qemu/int128.h
index fb782aa..c0465a2 100644
--- a/include/qemu/int128.h
+++ b/include/qemu/int128.h
@@ -5,64 +5,64 @@
 #include <stdint.h>
 #include <stdbool.h>
 
-typedef struct Int128 Int128;
+typedef struct UInt128 UInt128;
 
-struct Int128 {
+struct UInt128 {
     uint64_t lo;
-    int64_t hi;
+    uint64_t hi;
 };
 
-static inline Int128 int128_make64(uint64_t a)
+static inline UInt128 uint128_from_64(uint64_t a)
 {
-    return (Int128) { a, 0 };
+    return (UInt128) { a, 0 };
 }
 
-static inline uint64_t int128_get64(Int128 a)
+static inline uint64_t uint128_to_64(UInt128 a)
 {
     assert(!a.hi);
     return a.lo;
 }
 
-static inline Int128 int128_zero(void)
+static inline UInt128 uint128_zero(void)
 {
-    return int128_make64(0);
+    return uint128_from_64(0);
 }
 
-static inline Int128 int128_one(void)
+static inline UInt128 uint128_one(void)
 {
-    return int128_make64(1);
+    return uint128_from_64(1);
 }
 
-static inline Int128 int128_2_64(void)
+static inline UInt128 uint128_2_64(void)
 {
-    return (Int128) { 0, 1 };
+    return (UInt128) { UINT64_MAX, 0 };
 }
 
-static inline Int128 int128_exts64(int64_t a)
+static inline UInt128 uint128_exts64(int64_t a)
 {
-    return (Int128) { .lo = a, .hi = (a < 0) ? -1 : 0 };
+    return (UInt128) { .lo = a, .hi = (a < 0) ? -1 : 0 };
 }
 
-static inline Int128 int128_and(Int128 a, Int128 b)
+static inline UInt128 uint128_and(UInt128 a, UInt128 b)
 {
-    return (Int128) { a.lo & b.lo, a.hi & b.hi };
+    return (UInt128) { a.lo & b.lo, a.hi & b.hi };
 }
 
-static inline Int128 int128_rshift(Int128 a, int n)
+static inline UInt128 uint128_rshift(UInt128 a, unsigned int n)
 {
-    int64_t h;
+    uint64_t h;
     if (!n) {
         return a;
     }
     h = a.hi >> (n & 63);
     if (n >= 64) {
-        return (Int128) { h, h >> 63 };
+        return (UInt128) { h, h >> 63 };
     } else {
-        return (Int128) { (a.lo >> n) | ((uint64_t)a.hi << (64 - n)), h };
+        return (UInt128) { (a.lo >> n) | ((uint64_t)a.hi << (64 - n)), h };
     }
 }
 
-static inline Int128 int128_add(Int128 a, Int128 b)
+static inline UInt128 uint128_add(UInt128 a, UInt128 b)
 {
     uint64_t lo = a.lo + b.lo;
 
@@ -72,78 +72,73 @@ static inline Int128 int128_add(Int128 a, Int128 b)
      *
      * So the carry is lo < a.lo.
      */
-    return (Int128) { lo, (uint64_t)a.hi + b.hi + (lo < a.lo) };
+    return (UInt128) { lo, a.hi + b.hi + (lo < a.lo) };
 }
 
-static inline Int128 int128_neg(Int128 a)
+static inline UInt128 uint128_neg(UInt128 a)
 {
     uint64_t lo = -a.lo;
-    return (Int128) { lo, ~(uint64_t)a.hi + !lo };
+    return (UInt128) { lo, ~a.hi + !lo };
 }
 
-static inline Int128 int128_sub(Int128 a, Int128 b)
+static inline UInt128 uint128_sub(UInt128 a, UInt128 b)
 {
-    return (Int128){ a.lo - b.lo, (uint64_t)a.hi - b.hi - (a.lo < b.lo) };
+    return (UInt128){ a.lo - b.lo, a.hi - b.hi - (a.lo < b.lo) };
 }
 
-static inline bool int128_nonneg(Int128 a)
-{
-    return a.hi >= 0;
-}
-
-static inline bool int128_eq(Int128 a, Int128 b)
+static inline bool uint128_eq(UInt128 a, UInt128 b)
 {
     return a.lo == b.lo && a.hi == b.hi;
 }
 
-static inline bool int128_ne(Int128 a, Int128 b)
+static inline bool uint128_ne(UInt128 a, UInt128 b)
 {
-    return !int128_eq(a, b);
+    return !uint128_eq(a, b);
 }
 
-static inline bool int128_ge(Int128 a, Int128 b)
+static inline bool uint128_ge(UInt128 a, UInt128 b)
 {
     return a.hi > b.hi || (a.hi == b.hi && a.lo >= b.lo);
 }
 
-static inline bool int128_lt(Int128 a, Int128 b)
+static inline bool uint128_lt(UInt128 a, UInt128 b)
 {
-    return !int128_ge(a, b);
+    return !uint128_ge(a, b);
 }
 
-static inline bool int128_le(Int128 a, Int128 b)
+static inline bool uint128_le(UInt128 a, UInt128 b)
 {
-    return int128_ge(b, a);
+    return uint128_ge(b, a);
 }
 
-static inline bool int128_gt(Int128 a, Int128 b)
+static inline bool uint128_gt(UInt128 a, UInt128 b)
 {
-    return !int128_le(a, b);
+    return !uint128_le(a, b);
 }
 
-static inline bool int128_nz(Int128 a)
+static inline bool uint128_nz(UInt128 a)
 {
     return a.lo || a.hi;
 }
 
-static inline Int128 int128_min(Int128 a, Int128 b)
+static inline UInt128 uint128_min(UInt128 a, UInt128 b)
 {
-    return int128_le(a, b) ? a : b;
+    return uint128_le(a, b) ? a : b;
 }
 
-static inline Int128 int128_max(Int128 a, Int128 b)
+static inline UInt128 uint128_max(UInt128 a, UInt128 b)
 {
-    return int128_ge(a, b) ? a : b;
+    return uint128_ge(a, b) ? a : b;
 }
 
-static inline void int128_addto(Int128 *a, Int128 b)
+static inline void uint128_addto(UInt128 *a, UInt128 b)
 {
-    *a = int128_add(*a, b);
+    *a = uint128_add(*a, b);
 }
 
-static inline void int128_subfrom(Int128 *a, Int128 b)
+static inline void uint128_subfrom(UInt128 *a, UInt128 b)
 {
-    *a = int128_sub(*a, b);
+    *a = uint128_sub(*a, b);
 }
 
 #endif
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 2/7] memory: modify memory size for an unsigned 128bit int
  2015-11-05 16:18 [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations Pierre Morel
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 1/7] int128: use unsigned 128 bit arithmetic Pierre Morel
@ 2015-11-05 16:18 ` Pierre Morel
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 3/7] 128bit: adapt core files for unsigned 128bits Pierre Morel
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Pierre Morel @ 2015-11-05 16:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: cornelia.huck, pbonzini, pmorel, peter.maydell

The size of a memory area can never be negative.
It follows it must be defined as an unsigned value.

Let's modify the memory regions size to unsigned 128 integer
and accordingly use the unsigned 128 bit arithmetic.

Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
---
 include/exec/memory.h |    4 +-
 memory.c              |  184 ++++++++++++++++++++++++------------------------
 memory_mapping.c      |    2 +-
 3 files changed, 95 insertions(+), 95 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 94d20ea..42457f5 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -164,7 +164,7 @@ struct MemoryRegion {
     const MemoryRegionIOMMUOps *iommu_ops;
     void *opaque;
     MemoryRegion *container;
-    Int128 size;
+    UInt128 size;
     hwaddr addr;
     void (*destructor)(MemoryRegion *mr);
     ram_addr_t ram_addr;
@@ -264,7 +264,7 @@ struct MemoryRegionSection {
     MemoryRegion *mr;
     AddressSpace *address_space;
     hwaddr offset_within_region;
-    Int128 size;
+    UInt128 size;
     hwaddr offset_within_address_space;
     bool readonly;
 };
diff --git a/memory.c b/memory.c
index 10c1df5..d676f13 100644
--- a/memory.c
+++ b/memory.c
@@ -48,35 +48,35 @@ typedef struct AddrRange AddrRange;
  * (large MemoryRegion::alias_offset).
  */
 struct AddrRange {
-    Int128 start;
-    Int128 size;
+    UInt128 start;
+    UInt128 size;
 };
 
-static AddrRange addrrange_make(Int128 start, Int128 size)
+static AddrRange addrrange_make(UInt128 start, UInt128 size)
 {
     return (AddrRange) { start, size };
 }
 
 static bool addrrange_equal(AddrRange r1, AddrRange r2)
 {
-    return int128_eq(r1.start, r2.start) && int128_eq(r1.size, r2.size);
+    return uint128_eq(r1.start, r2.start) && uint128_eq(r1.size, r2.size);
 }
 
-static Int128 addrrange_end(AddrRange r)
+static UInt128 addrrange_end(AddrRange r)
 {
-    return int128_add(r.start, r.size);
+    return uint128_add(r.start, r.size);
 }
 
-static AddrRange addrrange_shift(AddrRange range, Int128 delta)
+static AddrRange addrrange_shift(AddrRange range, UInt128 delta)
 {
-    int128_addto(&range.start, delta);
+    uint128_addto(&range.start, delta);
     return range;
 }
 
-static bool addrrange_contains(AddrRange range, Int128 addr)
+static bool addrrange_contains(AddrRange range, UInt128 addr)
 {
-    return int128_ge(addr, range.start)
-        && int128_lt(addr, addrrange_end(range));
+    return uint128_ge(addr, range.start)
+        && uint128_lt(addr, addrrange_end(range));
 }
 
 static bool addrrange_intersects(AddrRange r1, AddrRange r2)
@@ -87,9 +87,9 @@ static bool addrrange_intersects(AddrRange r1, AddrRange r2)
 
 static AddrRange addrrange_intersection(AddrRange r1, AddrRange r2)
 {
-    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));
+    UInt128 start = uint128_max(r1.start, r2.start);
+    UInt128 end = uint128_min(addrrange_end(r1), addrrange_end(r2));
+    return addrrange_make(start, uint128_sub(end, start));
 }
 
 enum ListenerDirection { Forward, Reverse };
@@ -160,7 +160,7 @@ static bool memory_listener_match(MemoryListener *listener,
         .address_space = (as),                                          \
         .offset_within_region = (fr)->offset_in_region,                 \
         .size = (fr)->addr.size,                                        \
-        .offset_within_address_space = int128_get64((fr)->addr.start),  \
+        .offset_within_address_space = uint128_to_64((fr)->addr.start),  \
         .readonly = (fr)->readonly,                                     \
               }), ##_args)
 
@@ -179,13 +179,13 @@ struct MemoryRegionIoeventfd {
 static bool memory_region_ioeventfd_before(MemoryRegionIoeventfd a,
                                            MemoryRegionIoeventfd b)
 {
-    if (int128_lt(a.addr.start, b.addr.start)) {
+    if (uint128_lt(a.addr.start, b.addr.start)) {
         return true;
-    } else if (int128_gt(a.addr.start, b.addr.start)) {
+    } else if (uint128_gt(a.addr.start, b.addr.start)) {
         return false;
-    } else if (int128_lt(a.addr.size, b.addr.size)) {
+    } else if (uint128_lt(a.addr.size, b.addr.size)) {
         return true;
-    } else if (int128_gt(a.addr.size, b.addr.size)) {
+    } else if (uint128_gt(a.addr.size, b.addr.size)) {
         return false;
     } else if (a.match_data < b.match_data) {
         return true;
@@ -301,11 +301,11 @@ static void flatview_unref(FlatView *view)
 
 static bool can_merge(FlatRange *r1, FlatRange *r2)
 {
-    return int128_eq(addrrange_end(r1->addr), r2->addr.start)
+    return uint128_eq(addrrange_end(r1->addr), r2->addr.start)
         && r1->mr == r2->mr
-        && int128_eq(int128_add(int128_make64(r1->offset_in_region),
+        && uint128_eq(uint128_add(uint128_from_64(r1->offset_in_region),
                                 r1->addr.size),
-                     int128_make64(r2->offset_in_region))
+                     uint128_from_64(r2->offset_in_region))
         && r1->dirty_log_mask == r2->dirty_log_mask
         && r1->romd_mode == r2->romd_mode
         && r1->readonly == r2->readonly;
@@ -321,7 +321,7 @@ static void flatview_simplify(FlatView *view)
         j = i + 1;
         while (j < view->nr
                && can_merge(&view->ranges[j-1], &view->ranges[j])) {
-            int128_addto(&view->ranges[i].addr.size, view->ranges[j].addr.size);
+            uint128_addto(&view->ranges[i].addr.size, view->ranges[j].addr.size);
             ++j;
         }
         ++i;
@@ -530,15 +530,15 @@ static AddressSpace *memory_region_to_address_space(MemoryRegion *mr)
  */
 static void render_memory_region(FlatView *view,
                                  MemoryRegion *mr,
-                                 Int128 base,
+                                 UInt128 base,
                                  AddrRange clip,
                                  bool readonly)
 {
     MemoryRegion *subregion;
     unsigned i;
     hwaddr offset_in_region;
-    Int128 remain;
-    Int128 now;
+    UInt128 remain;
+    UInt128 now;
     FlatRange fr;
     AddrRange tmp;
 
@@ -546,7 +546,7 @@ static void render_memory_region(FlatView *view,
         return;
     }
 
-    int128_addto(&base, int128_make64(mr->addr));
+    uint128_addto(&base, uint128_from_64(mr->addr));
     readonly |= mr->readonly;
 
     tmp = addrrange_make(base, mr->size);
@@ -558,8 +558,8 @@ static void render_memory_region(FlatView *view,
     clip = addrrange_intersection(tmp, clip);
 
     if (mr->alias) {
-        int128_subfrom(&base, int128_make64(mr->alias->addr));
-        int128_subfrom(&base, int128_make64(mr->alias_offset));
+        uint128_subfrom(&base, uint128_from_64(mr->alias->addr));
+        uint128_subfrom(&base, uint128_from_64(mr->alias_offset));
         render_memory_region(view, mr->alias, base, clip, readonly);
         return;
     }
@@ -573,7 +573,7 @@ static void render_memory_region(FlatView *view,
         return;
     }
 
-    offset_in_region = int128_get64(int128_sub(clip.start, base));
+    offset_in_region = uint128_to_64(uint128_sub(clip.start, base));
     base = clip.start;
     remain = clip.size;
 
@@ -583,29 +583,29 @@ static void render_memory_region(FlatView *view,
     fr.readonly = readonly;
 
     /* Render the region itself into any gaps left by the current view. */
-    for (i = 0; i < view->nr && int128_nz(remain); ++i) {
-        if (int128_ge(base, addrrange_end(view->ranges[i].addr))) {
+    for (i = 0; i < view->nr && uint128_nz(remain); ++i) {
+        if (uint128_ge(base, addrrange_end(view->ranges[i].addr))) {
             continue;
         }
-        if (int128_lt(base, view->ranges[i].addr.start)) {
-            now = int128_min(remain,
-                             int128_sub(view->ranges[i].addr.start, base));
+        if (uint128_lt(base, view->ranges[i].addr.start)) {
+            now = uint128_min(remain,
+                             uint128_sub(view->ranges[i].addr.start, base));
             fr.offset_in_region = offset_in_region;
             fr.addr = addrrange_make(base, now);
             flatview_insert(view, i, &fr);
             ++i;
-            int128_addto(&base, now);
-            offset_in_region += int128_get64(now);
-            int128_subfrom(&remain, now);
+            uint128_addto(&base, now);
+            offset_in_region += uint128_to_64(now);
+            uint128_subfrom(&remain, now);
         }
-        now = int128_sub(int128_min(int128_add(base, remain),
+        now = uint128_sub(uint128_min(uint128_add(base, remain),
                                     addrrange_end(view->ranges[i].addr)),
                          base);
-        int128_addto(&base, now);
-        offset_in_region += int128_get64(now);
-        int128_subfrom(&remain, now);
+        uint128_addto(&base, now);
+        offset_in_region += uint128_to_64(now);
+        uint128_subfrom(&remain, now);
     }
-    if (int128_nz(remain)) {
+    if (uint128_nz(remain)) {
         fr.offset_in_region = offset_in_region;
         fr.addr = addrrange_make(base, remain);
         flatview_insert(view, i, &fr);
@@ -621,8 +621,8 @@ static FlatView *generate_memory_topology(MemoryRegion *mr)
     flatview_init(view);
 
     if (mr) {
-        render_memory_region(view, mr, int128_zero(),
-                             addrrange_make(int128_zero(), int128_2_64()), false);
+        render_memory_region(view, mr, uint128_zero(),
+                             addrrange_make(uint128_zero(), uint128_2_64()), false);
     }
     flatview_simplify(view);
 
@@ -652,7 +652,7 @@ static void address_space_add_del_ioeventfds(AddressSpace *as,
             fd = &fds_old[iold];
             section = (MemoryRegionSection) {
                 .address_space = as,
-                .offset_within_address_space = int128_get64(fd->addr.start),
+                .offset_within_address_space = uint128_to_64(fd->addr.start),
                 .size = fd->addr.size,
             };
             MEMORY_LISTENER_CALL(eventfd_del, Forward, &section,
@@ -665,7 +665,7 @@ static void address_space_add_del_ioeventfds(AddressSpace *as,
             fd = &fds_new[inew];
             section = (MemoryRegionSection) {
                 .address_space = as,
-                .offset_within_address_space = int128_get64(fd->addr.start),
+                .offset_within_address_space = uint128_to_64(fd->addr.start),
                 .size = fd->addr.size,
             };
             MEMORY_LISTENER_CALL(eventfd_add, Reverse, &section,
@@ -702,8 +702,8 @@ static void address_space_update_ioeventfds(AddressSpace *as)
     FOR_EACH_FLAT_RANGE(fr, view) {
         for (i = 0; i < fr->mr->ioeventfd_nb; ++i) {
             tmp = addrrange_shift(fr->mr->ioeventfds[i].addr,
-                                  int128_sub(fr->addr.start,
-                                             int128_make64(fr->offset_in_region)));
+                                  uint128_sub(fr->addr.start,
+                                             uint128_from_64(fr->offset_in_region)));
             if (addrrange_intersects(fr->addr, tmp)) {
                 ++ioeventfd_nb;
                 ioeventfds = g_realloc(ioeventfds,
@@ -749,8 +749,8 @@ static void address_space_update_topology_pass(AddressSpace *as,
 
         if (frold
             && (!frnew
-                || int128_lt(frold->addr.start, frnew->addr.start)
-                || (int128_eq(frold->addr.start, frnew->addr.start)
+                || uint128_lt(frold->addr.start, frnew->addr.start)
+                || (uint128_eq(frold->addr.start, frnew->addr.start)
                     && !flatrange_equal(frold, frnew)))) {
             /* In old but not in new, or in both but attributes changed. */
 
@@ -913,9 +913,9 @@ void memory_region_init(MemoryRegion *mr,
     }
 
     object_initialize(mr, sizeof(*mr), TYPE_MEMORY_REGION);
-    mr->size = int128_make64(size);
+    mr->size = uint128_from_64(size);
     if (size == UINT64_MAX) {
-        mr->size = int128_2_64();
+        mr->size = uint128_2_64();
     }
     mr->name = g_strdup(name);
 
@@ -1372,10 +1372,10 @@ void memory_region_unref(MemoryRegion *mr)
 
 uint64_t memory_region_size(MemoryRegion *mr)
 {
-    if (int128_eq(mr->size, int128_2_64())) {
+    if (uint128_eq(mr->size, uint128_2_64())) {
         return UINT64_MAX;
     }
-    return int128_get64(mr->size);
+    return uint128_to_64(mr->size);
 }
 
 const char *memory_region_name(const MemoryRegion *mr)
@@ -1566,24 +1566,24 @@ static void memory_region_update_coalesced_range_as(MemoryRegion *mr, AddressSpa
         if (fr->mr == mr) {
             section = (MemoryRegionSection) {
                 .address_space = as,
-                .offset_within_address_space = int128_get64(fr->addr.start),
+                .offset_within_address_space = uint128_to_64(fr->addr.start),
                 .size = fr->addr.size,
             };
 
             MEMORY_LISTENER_CALL(coalesced_mmio_del, Reverse, &section,
-                                 int128_get64(fr->addr.start),
-                                 int128_get64(fr->addr.size));
+                                 uint128_to_64(fr->addr.start),
+                                 uint128_to_64(fr->addr.size));
             QTAILQ_FOREACH(cmr, &mr->coalesced, link) {
                 tmp = addrrange_shift(cmr->addr,
-                                      int128_sub(fr->addr.start,
-                                                 int128_make64(fr->offset_in_region)));
+                                      uint128_sub(fr->addr.start,
+                                                 uint128_from_64(fr->offset_in_region)));
                 if (!addrrange_intersects(tmp, fr->addr)) {
                     continue;
                 }
                 tmp = addrrange_intersection(tmp, fr->addr);
                 MEMORY_LISTENER_CALL(coalesced_mmio_add, Forward, &section,
-                                     int128_get64(tmp.start),
-                                     int128_get64(tmp.size));
+                                     uint128_to_64(tmp.start),
+                                     uint128_to_64(tmp.size));
             }
         }
     }
@@ -1602,7 +1602,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, int128_get64(mr->size));
+    memory_region_add_coalescing(mr, 0, uint128_to_64(mr->size));
 }
 
 void memory_region_add_coalescing(MemoryRegion *mr,
@@ -1611,7 +1611,7 @@ void memory_region_add_coalescing(MemoryRegion *mr,
 {
     CoalescedMemoryRange *cmr = g_malloc(sizeof(*cmr));
 
-    cmr->addr = addrrange_make(int128_make64(offset), int128_make64(size));
+    cmr->addr = addrrange_make(uint128_from_64(offset), uint128_from_64(size));
     QTAILQ_INSERT_TAIL(&mr->coalesced, cmr, link);
     memory_region_update_coalesced_range(mr);
     memory_region_set_flush_coalesced(mr);
@@ -1668,8 +1668,8 @@ void memory_region_add_eventfd(MemoryRegion *mr,
                                EventNotifier *e)
 {
     MemoryRegionIoeventfd mrfd = {
-        .addr.start = int128_make64(addr),
-        .addr.size = int128_make64(size),
+        .addr.start = uint128_from_64(addr),
+        .addr.size = uint128_from_64(size),
         .match_data = match_data,
         .data = data,
         .e = e,
@@ -1701,8 +1701,8 @@ void memory_region_del_eventfd(MemoryRegion *mr,
                                EventNotifier *e)
 {
     MemoryRegionIoeventfd mrfd = {
-        .addr.start = int128_make64(addr),
-        .addr.size = int128_make64(size),
+        .addr.start = uint128_from_64(addr),
+        .addr.size = uint128_from_64(size),
         .match_data = match_data,
         .data = data,
         .e = e,
@@ -1739,20 +1739,20 @@ static void memory_region_update_container_subregions(MemoryRegion *subregion)
         if (subregion->may_overlap || other->may_overlap) {
             continue;
         }
-        if (int128_ge(int128_make64(offset),
-                      int128_add(int128_make64(other->addr), other->size))
-            || int128_le(int128_add(int128_make64(offset), subregion->size),
-                         int128_make64(other->addr))) {
+        if (uint128_ge(uint128_from_64(offset),
+                      uint128_add(uint128_from_64(other->addr), other->size))
+            || uint128_le(uint128_add(uint128_from_64(offset), subregion->size),
+                         uint128_from_64(other->addr))) {
             continue;
         }
 #if 0
         printf("warning: subregion collision %llx/%llx (%s) "
                "vs %llx/%llx (%s)\n",
                (unsigned long long)offset,
-               (unsigned long long)int128_get64(subregion->size),
+               (unsigned long long)uint128_to_64(subregion->size),
                subregion->name,
                (unsigned long long)other->addr,
-               (unsigned long long)int128_get64(other->size),
+               (unsigned long long)uint128_to_64(other->size),
                other->name);
 #endif
     }
@@ -1822,12 +1822,12 @@ void memory_region_set_enabled(MemoryRegion *mr, bool enabled)
 
 void memory_region_set_size(MemoryRegion *mr, uint64_t size)
 {
-    Int128 s = int128_make64(size);
+    UInt128 s = uint128_from_64(size);
 
     if (size == UINT64_MAX) {
-        s = int128_2_64();
+        s = uint128_2_64();
     }
-    if (int128_eq(s, mr->size)) {
+    if (uint128_eq(s, mr->size)) {
         return;
     }
     memory_region_transaction_begin();
@@ -1888,9 +1888,9 @@ static int cmp_flatrange_addr(const void *addr_, const void *fr_)
     const AddrRange *addr = addr_;
     const FlatRange *fr = fr_;
 
-    if (int128_le(addrrange_end(*addr), fr->addr.start)) {
+    if (uint128_le(addrrange_end(*addr), fr->addr.start)) {
         return -1;
-    } else if (int128_ge(addr->start, addrrange_end(fr->addr))) {
+    } else if (uint128_ge(addr->start, addrrange_end(fr->addr))) {
         return 1;
     }
     return 0;
@@ -1930,7 +1930,7 @@ static MemoryRegionSection memory_region_find_rcu(MemoryRegion *mr,
     if (!as) {
         return ret;
     }
-    range = addrrange_make(int128_make64(addr), int128_make64(size));
+    range = addrrange_make(uint128_from_64(addr), uint128_from_64(size));
 
     view = atomic_rcu_read(&as->current_map);
     fr = flatview_lookup(view, range);
@@ -1946,10 +1946,10 @@ static MemoryRegionSection memory_region_find_rcu(MemoryRegion *mr,
     ret.address_space = as;
     range = addrrange_intersection(range, fr->addr);
     ret.offset_within_region = fr->offset_in_region;
-    ret.offset_within_region += int128_get64(int128_sub(range.start,
+    ret.offset_within_region += uint128_to_64(uint128_sub(range.start,
                                                         fr->addr.start));
     ret.size = range.size;
-    ret.offset_within_address_space = int128_get64(range.start);
+    ret.offset_within_address_space = uint128_to_64(range.start);
     ret.readonly = fr->readonly;
     return ret;
 }
@@ -2037,7 +2037,7 @@ static void listener_add_address_space(MemoryListener *listener,
             .address_space = as,
             .offset_within_region = fr->offset_in_region,
             .size = fr->addr.size,
-            .offset_within_address_space = int128_get64(fr->addr.start),
+            .offset_within_address_space = uint128_to_64(fr->addr.start),
             .readonly = fr->readonly,
         };
         if (listener->region_add) {
@@ -2175,9 +2175,9 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
                    "-" TARGET_FMT_plx "%s\n",
                    base + mr->addr,
                    base + mr->addr
-                   + (int128_nz(mr->size) ?
-                      (hwaddr)int128_get64(int128_sub(mr->size,
-                                                      int128_one())) : 0),
+                   + (uint128_nz(mr->size) ?
+                      (hwaddr)uint128_to_64(uint128_sub(mr->size,
+                                                      uint128_one())) : 0),
                    mr->priority,
                    mr->romd_mode ? 'R' : '-',
                    !mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W'
@@ -2186,18 +2186,18 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f,
                    memory_region_name(mr->alias),
                    mr->alias_offset,
                    mr->alias_offset
-                   + (int128_nz(mr->size) ?
-                      (hwaddr)int128_get64(int128_sub(mr->size,
-                                                      int128_one())) : 0),
+                   + (uint128_nz(mr->size) ?
+                      (hwaddr)uint128_to_64(uint128_sub(mr->size,
+                                                      uint128_one())) : 0),
                    mr->enabled ? "" : " [disabled]");
     } else {
         mon_printf(f,
                    TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %c%c): %s%s\n",
                    base + mr->addr,
                    base + mr->addr
-                   + (int128_nz(mr->size) ?
-                      (hwaddr)int128_get64(int128_sub(mr->size,
-                                                      int128_one())) : 0),
+                   + (uint128_nz(mr->size) ?
+                      (hwaddr)uint128_to_64(uint128_sub(mr->size,
+                                                      uint128_one())) : 0),
                    mr->priority,
                    mr->romd_mode ? 'R' : '-',
                    !mr->readonly && !(mr->rom_device && mr->romd_mode) ? 'W'
diff --git a/memory_mapping.c b/memory_mapping.c
index 36d6b26..ce87c6b 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -209,7 +209,7 @@ static void guest_phys_blocks_region_add(MemoryListener *listener,
     }
 
     g            = container_of(listener, GuestPhysListener, listener);
-    section_size = int128_get64(section->size);
+    section_size = uint128_to_64(section->size);
     target_start = section->offset_within_address_space;
     target_end   = target_start + section_size;
     host_addr    = memory_region_get_ram_ptr(section->mr) +
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 3/7] 128bit: adapt core files for unsigned 128bits
  2015-11-05 16:18 [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations Pierre Morel
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 1/7] int128: use unsigned 128 bit arithmetic Pierre Morel
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 2/7] memory: modify memory size for an unsigned 128bit int Pierre Morel
@ 2015-11-05 16:18 ` Pierre Morel
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 4/7] 128bit: adapt sparc mmu_helper for UInt128 Pierre Morel
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Pierre Morel @ 2015-11-05 16:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: cornelia.huck, pbonzini, pmorel, peter.maydell

Adapt the core files for unsigned 128 bit arithmetic.

Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
---
 exec.c    |   32 ++++++++++++++++----------------
 kvm-all.c |   16 ++++++++--------
 2 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/exec.c b/exec.c
index 0a4a0c5..7828f0b 100644
--- a/exec.c
+++ b/exec.c
@@ -343,7 +343,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
 {
     MemoryRegionSection *section;
     MemoryRegion *mr;
-    Int128 diff;
+    UInt128 diff;
 
     section = address_space_lookup_region(d, addr, resolve_subpage);
     /* Compute offset within MemoryRegionSection */
@@ -366,8 +366,8 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
      * the caller really has to do the clamping through memory_access_size.
      */
     if (memory_region_is_ram(mr)) {
-        diff = int128_sub(section->size, int128_make64(addr));
-        *plen = int128_get64(int128_min(diff, int128_make64(*plen)));
+        diff = uint128_sub(section->size, uint128_from_64(addr));
+        *plen = uint128_to_64(uint128_min(diff, uint128_from_64(*plen)));
     }
     return section;
 }
@@ -1046,7 +1046,7 @@ static void register_subpage(AddressSpaceDispatch *d, MemoryRegionSection *secti
                                                    d->map.nodes, d->map.sections);
     MemoryRegionSection subsection = {
         .offset_within_address_space = base,
-        .size = int128_make64(TARGET_PAGE_SIZE),
+        .size = uint128_from_64(TARGET_PAGE_SIZE),
     };
     hwaddr start, end;
 
@@ -1062,7 +1062,7 @@ static void register_subpage(AddressSpaceDispatch *d, MemoryRegionSection *secti
         subpage = container_of(existing->mr, subpage_t, iomem);
     }
     start = section->offset_within_address_space & ~TARGET_PAGE_MASK;
-    end = start + int128_get64(section->size) - 1;
+    end = start + uint128_to_64(section->size) - 1;
     subpage_register(subpage, start, end,
                      phys_section_add(&d->map, section));
 }
@@ -1073,7 +1073,7 @@ static void register_multipage(AddressSpaceDispatch *d,
 {
     hwaddr start_addr = section->offset_within_address_space;
     uint16_t section_index = phys_section_add(&d->map, section);
-    uint64_t num_pages = int128_get64(int128_rshift(section->size,
+    uint64_t num_pages = uint128_to_64(uint128_rshift(section->size,
                                                     TARGET_PAGE_BITS));
 
     assert(num_pages);
@@ -1085,29 +1085,29 @@ static void mem_add(MemoryListener *listener, MemoryRegionSection *section)
     AddressSpace *as = container_of(listener, AddressSpace, dispatch_listener);
     AddressSpaceDispatch *d = as->next_dispatch;
     MemoryRegionSection now = *section, remain = *section;
-    Int128 page_size = int128_make64(TARGET_PAGE_SIZE);
+    UInt128 page_size = uint128_from_64(TARGET_PAGE_SIZE);
 
     if (now.offset_within_address_space & ~TARGET_PAGE_MASK) {
         uint64_t left = TARGET_PAGE_ALIGN(now.offset_within_address_space)
                        - now.offset_within_address_space;
 
-        now.size = int128_min(int128_make64(left), now.size);
+        now.size = uint128_min(uint128_from_64(left), now.size);
         register_subpage(d, &now);
     } else {
-        now.size = int128_zero();
+        now.size = uint128_zero();
     }
-    while (int128_ne(remain.size, now.size)) {
-        remain.size = int128_sub(remain.size, now.size);
-        remain.offset_within_address_space += int128_get64(now.size);
-        remain.offset_within_region += int128_get64(now.size);
+    while (uint128_ne(remain.size, now.size)) {
+        remain.size = uint128_sub(remain.size, now.size);
+        remain.offset_within_address_space += uint128_to_64(now.size);
+        remain.offset_within_region += uint128_to_64(now.size);
         now = remain;
-        if (int128_lt(remain.size, page_size)) {
+        if (uint128_lt(remain.size, page_size)) {
             register_subpage(d, &now);
         } else if (remain.offset_within_address_space & ~TARGET_PAGE_MASK) {
             now.size = page_size;
             register_subpage(d, &now);
         } else {
-            now.size = int128_and(now.size, int128_neg(page_size));
+            now.size = uint128_and(now.size, uint128_neg(page_size));
             register_multipage(d, &now);
         }
     }
@@ -2155,7 +2155,7 @@ static uint16_t dummy_section(PhysPageMap *map, AddressSpace *as,
         .mr = mr,
         .offset_within_address_space = 0,
         .offset_within_region = 0,
-        .size = int128_2_64(),
+        .size = uint128_2_64(),
     };
 
     return phys_section_add(map, &section);
diff --git a/kvm-all.c b/kvm-all.c
index 06e06f2..6a155a9 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -316,7 +316,7 @@ static int kvm_section_update_flags(KVMMemoryListener *kml,
                                     MemoryRegionSection *section)
 {
     hwaddr phys_addr = section->offset_within_address_space;
-    ram_addr_t size = int128_get64(section->size);
+    ram_addr_t size = uint128_to_64(section->size);
     KVMSlot *mem = kvm_lookup_matching_slot(kml, phys_addr, phys_addr + size);
 
     if (mem == NULL)  {
@@ -365,7 +365,7 @@ static int kvm_get_dirty_pages_log_range(MemoryRegionSection *section,
                                          unsigned long *bitmap)
 {
     ram_addr_t start = section->offset_within_region + section->mr->ram_addr;
-    ram_addr_t pages = int128_get64(section->size) / getpagesize();
+    ram_addr_t pages = uint128_to_64(section->size) / getpagesize();
 
     cpu_physical_memory_set_dirty_lebitmap(bitmap, start, pages);
     return 0;
@@ -391,7 +391,7 @@ static int kvm_physical_sync_dirty_bitmap(KVMMemoryListener *kml,
     KVMSlot *mem;
     int ret = 0;
     hwaddr start_addr = section->offset_within_address_space;
-    hwaddr end_addr = start_addr + int128_get64(section->size);
+    hwaddr end_addr = start_addr + uint128_to_64(section->size);
 
     d.dirty_bitmap = NULL;
     while (start_addr < end_addr) {
@@ -634,7 +634,7 @@ static void kvm_set_phys_mem(KVMMemoryListener *kml,
     MemoryRegion *mr = section->mr;
     bool writeable = !mr->readonly && !mr->rom_device;
     hwaddr start_addr = section->offset_within_address_space;
-    ram_addr_t size = int128_get64(section->size);
+    ram_addr_t size = uint128_to_64(section->size);
     void *ram = NULL;
     unsigned delta;
 
@@ -825,7 +825,7 @@ static void kvm_mem_ioeventfd_add(MemoryListener *listener,
     int r;
 
     r = kvm_set_ioeventfd_mmio(fd, section->offset_within_address_space,
-                               data, true, int128_get64(section->size),
+                               data, true, uint128_to_64(section->size),
                                match_data);
     if (r < 0) {
         fprintf(stderr, "%s: error adding ioeventfd: %s\n",
@@ -843,7 +843,7 @@ static void kvm_mem_ioeventfd_del(MemoryListener *listener,
     int r;
 
     r = kvm_set_ioeventfd_mmio(fd, section->offset_within_address_space,
-                               data, false, int128_get64(section->size),
+                               data, false, uint128_to_64(section->size),
                                match_data);
     if (r < 0) {
         abort();
@@ -859,7 +859,7 @@ static void kvm_io_ioeventfd_add(MemoryListener *listener,
     int r;
 
     r = kvm_set_ioeventfd_pio(fd, section->offset_within_address_space,
-                              data, true, int128_get64(section->size),
+                              data, true, uint128_to_64(section->size),
                               match_data);
     if (r < 0) {
         fprintf(stderr, "%s: error adding ioeventfd: %s\n",
@@ -878,7 +878,7 @@ static void kvm_io_ioeventfd_del(MemoryListener *listener,
     int r;
 
     r = kvm_set_ioeventfd_pio(fd, section->offset_within_address_space,
-                              data, false, int128_get64(section->size),
+                              data, false, uint128_to_64(section->size),
                               match_data);
     if (r < 0) {
         abort();
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 4/7] 128bit: adapt sparc mmu_helper for UInt128
  2015-11-05 16:18 [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations Pierre Morel
                   ` (2 preceding siblings ...)
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 3/7] 128bit: adapt core files for unsigned 128bits Pierre Morel
@ 2015-11-05 16:18 ` Pierre Morel
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 5/7] 128bit: adapt VFIO for UInt128 arithmetic Pierre Morel
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Pierre Morel @ 2015-11-05 16:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: cornelia.huck, pbonzini, pmorel, peter.maydell

Adaptation for target-sparc/mmu_helper.c to
the 128 bit arithmetic.

Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
---
 target-sparc/mmu_helper.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-sparc/mmu_helper.c b/target-sparc/mmu_helper.c
index 2a0c6f0..2c1156e 100644
--- a/target-sparc/mmu_helper.c
+++ b/target-sparc/mmu_helper.c
@@ -859,7 +859,7 @@ hwaddr sparc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
     }
     section = memory_region_find(get_system_memory(), phys_addr, 1);
     memory_region_unref(section.mr);
-    if (!int128_nz(section.size)) {
+    if (!uint128_nz(section.size)) {
         return -1;
     }
     return phys_addr;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 5/7] 128bit: adapt VFIO for UInt128 arithmetic
  2015-11-05 16:18 [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations Pierre Morel
                   ` (3 preceding siblings ...)
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 4/7] 128bit: adapt sparc mmu_helper for UInt128 Pierre Morel
@ 2015-11-05 16:18 ` Pierre Morel
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 6/7] 128bit: adapt Virtio " Pierre Morel
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 14+ messages in thread
From: Pierre Morel @ 2015-11-05 16:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: cornelia.huck, pbonzini, pmorel, peter.maydell

VFIO is using 128bit arithmetic for memory areas.
Let's adapt to unsigned 128bit arithmetic.

Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
---
 hw/vfio/common.c |   20 ++++++++++----------
 1 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 85ee9b0..f819cd5 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -318,7 +318,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
     VFIOContainer *container = container_of(listener, VFIOContainer,
                                             iommu_data.type1.listener);
     hwaddr iova, end;
-    Int128 llend;
+    UInt128 llend;
     void *vaddr;
     int ret;
 
@@ -326,7 +326,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
         trace_vfio_listener_region_add_skip(
                 section->offset_within_address_space,
                 section->offset_within_address_space +
-                int128_get64(int128_sub(section->size, int128_one())));
+                uint128_to_64(uint128_sub(section->size, uint128_one())));
         return;
     }
 
@@ -337,11 +337,11 @@ static void vfio_listener_region_add(MemoryListener *listener,
     }
 
     iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
-    llend = int128_make64(section->offset_within_address_space);
-    llend = int128_add(llend, section->size);
-    llend = int128_and(llend, int128_exts64(TARGET_PAGE_MASK));
+    llend = uint128_from_64(section->offset_within_address_space);
+    llend = uint128_add(llend, section->size);
+    llend = uint128_and(llend, uint128_exts64(TARGET_PAGE_MASK));
 
-    if (int128_ge(int128_make64(iova), llend)) {
+    if (uint128_ge(uint128_from_64(iova), llend)) {
         return;
     }
 
@@ -351,7 +351,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
         VFIOGuestIOMMU *giommu;
 
         trace_vfio_listener_region_add_iommu(iova,
-                    int128_get64(int128_sub(llend, int128_one())));
+                    uint128_to_64(uint128_sub(llend, uint128_one())));
         /*
          * FIXME: We should do some checking to see if the
          * capabilities of the host VFIO IOMMU are adequate to model
@@ -388,7 +388,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
 
     /* Here we assume that memory_region_is_ram(section->mr)==true */
 
-    end = int128_get64(llend);
+    end = uint128_to_64(llend);
     vaddr = memory_region_get_ram_ptr(section->mr) +
             section->offset_within_region +
             (iova - section->offset_within_address_space);
@@ -428,7 +428,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
         trace_vfio_listener_region_del_skip(
                 section->offset_within_address_space,
                 section->offset_within_address_space +
-                int128_get64(int128_sub(section->size, int128_one())));
+                uint128_to_64(uint128_sub(section->size, uint128_one())));
         return;
     }
 
@@ -460,7 +460,7 @@ static void vfio_listener_region_del(MemoryListener *listener,
     }
 
     iova = TARGET_PAGE_ALIGN(section->offset_within_address_space);
-    end = (section->offset_within_address_space + int128_get64(section->size)) &
+    end = (section->offset_within_address_space + uint128_to_64(section->size)) &
           TARGET_PAGE_MASK;
 
     if (iova >= end) {
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 6/7] 128bit: adapt Virtio for UInt128 arithmetic
  2015-11-05 16:18 [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations Pierre Morel
                   ` (4 preceding siblings ...)
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 5/7] 128bit: adapt VFIO for UInt128 arithmetic Pierre Morel
@ 2015-11-05 16:18 ` Pierre Morel
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 7/7] 128bits: some HW components use 128bit arithmetic Pierre Morel
  2015-11-05 16:32 ` [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations Paolo Bonzini
  7 siblings, 0 replies; 14+ messages in thread
From: Pierre Morel @ 2015-11-05 16:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: cornelia.huck, pbonzini, pmorel, peter.maydell

Virtio uses 128 bit arithmetic for memory areas.
Let's adapt to UInt128 bit arithmetic.

Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
---
 hw/virtio/dataplane/vring.c |    2 +-
 hw/virtio/vhost.c           |    4 ++--
 hw/virtio/virtio-balloon.c  |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
index 68f1994..2551a42 100644
--- a/hw/virtio/dataplane/vring.c
+++ b/hw/virtio/dataplane/vring.c
@@ -31,7 +31,7 @@ static void *vring_map(MemoryRegion **mr, hwaddr phys, hwaddr len,
 {
     MemoryRegionSection section = memory_region_find(get_system_memory(), phys, len);
 
-    if (!section.mr || int128_get64(section.size) < len) {
+    if (!section.mr || uint128_to_64(section.size) < len) {
         goto out;
     }
     if (is_write && section.readonly) {
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index a08c36b..143ea6f 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -84,7 +84,7 @@ static int vhost_sync_dirty_bitmap(struct vhost_dev *dev,
         return 0;
     }
     start_addr = section->offset_within_address_space;
-    end_addr = range_get_last(start_addr, int128_get64(section->size));
+    end_addr = range_get_last(start_addr, uint128_to_64(section->size));
     start_addr = MAX(first, start_addr);
     end_addr = MIN(last, end_addr);
 
@@ -417,7 +417,7 @@ static void vhost_set_memory(MemoryListener *listener,
     struct vhost_dev *dev = container_of(listener, struct vhost_dev,
                                          memory_listener);
     hwaddr start_addr = section->offset_within_address_space;
-    ram_addr_t size = int128_get64(section->size);
+    ram_addr_t size = uint128_to_64(section->size);
     bool log_dirty =
         memory_region_get_dirty_log_mask(section->mr) & ~(1 << DIRTY_MEMORY_MIGRATION);
     int s = offsetof(struct vhost_memory, regions) +
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index c419b17..d0db511 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -220,7 +220,7 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 
             /* FIXME: remove get_system_memory(), but how? */
             section = memory_region_find(get_system_memory(), pa, 1);
-            if (!int128_nz(section.size) || !memory_region_is_ram(section.mr))
+            if (!uint128_nz(section.size) || !memory_region_is_ram(section.mr))
                 continue;
 
             trace_virtio_balloon_handle_output(memory_region_name(section.mr),
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH 7/7] 128bits: some HW components use 128bit arithmetic.
  2015-11-05 16:18 [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations Pierre Morel
                   ` (5 preceding siblings ...)
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 6/7] 128bit: adapt Virtio " Pierre Morel
@ 2015-11-05 16:18 ` Pierre Morel
  2015-11-05 16:32 ` [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations Paolo Bonzini
  7 siblings, 0 replies; 14+ messages in thread
From: Pierre Morel @ 2015-11-05 16:18 UTC (permalink / raw)
  To: qemu-devel; +Cc: cornelia.huck, pbonzini, pmorel, peter.maydell

Some HW components use 128bit arithmetic.
Let's adapt to unsigned 128 bit calculations.

Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
---
 hw/core/loader.c             |    2 +-
 hw/display/exynos4210_fimd.c |    4 ++--
 hw/display/framebuffer.c     |    2 +-
 hw/mem/pc-dimm.c             |    6 +++---
 hw/pci-host/ppce500.c        |    2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 216eeeb..97c7b54 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -953,7 +953,7 @@ int rom_check_and_register_reset(void)
         addr  = rom->addr;
         addr += rom->romsize;
         section = memory_region_find(get_system_memory(), rom->addr, 1);
-        rom->isrom = int128_nz(section.size) && memory_region_is_rom(section.mr);
+        rom->isrom = uint128_nz(section.size) && memory_region_is_rom(section.mr);
         memory_region_unref(section.mr);
     }
     qemu_register_reset(rom_reset, NULL);
diff --git a/hw/display/exynos4210_fimd.c b/hw/display/exynos4210_fimd.c
index 603ef50..dbedb45 100644
--- a/hw/display/exynos4210_fimd.c
+++ b/hw/display/exynos4210_fimd.c
@@ -1154,7 +1154,7 @@ static void fimd_update_memory_section(Exynos4210fimdState *s, unsigned win)
     DPRINT_TRACE("Window %u framebuffer changed: address=0x%08x, len=0x%x\n",
             win, fb_start_addr, w->fb_len);
 
-    if (int128_get64(w->mem_section.size) != w->fb_len ||
+    if (uint128_to_64(w->mem_section.size) != w->fb_len ||
             !memory_region_is_ram(w->mem_section.mr)) {
         DPRINT_ERROR("Failed to find window %u framebuffer region\n", win);
         goto error_return;
@@ -1179,7 +1179,7 @@ static void fimd_update_memory_section(Exynos4210fimdState *s, unsigned win)
 error_return:
     memory_region_unref(w->mem_section.mr);
     w->mem_section.mr = NULL;
-    w->mem_section.size = int128_zero();
+    w->mem_section.size = uint128_zero();
     w->host_fb_addr = NULL;
     w->fb_len = 0;
 }
diff --git a/hw/display/framebuffer.c b/hw/display/framebuffer.c
index 7f075ce..19e3d39 100644
--- a/hw/display/framebuffer.c
+++ b/hw/display/framebuffer.c
@@ -41,7 +41,7 @@ void framebuffer_update_memory_section(
         return;
     }
 
-    if (int128_get64(mem_section->size) < src_len ||
+    if (uint128_to_64(mem_section->size) < src_len ||
             !memory_region_is_ram(mem_section->mr)) {
         memory_region_unref(mem_section->mr);
         mem_section->mr = NULL;
diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index bb04862..0be2a18 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -260,11 +260,11 @@ static gint pc_dimm_addr_sort(gconstpointer a, gconstpointer b)
 {
     PCDIMMDevice *x = PC_DIMM(a);
     PCDIMMDevice *y = PC_DIMM(b);
-    Int128 diff = int128_sub(int128_make64(x->addr), int128_make64(y->addr));
+    UInt128 diff = uint128_sub(uint128_from_64(x->addr), uint128_from_64(y->addr));
 
-    if (int128_lt(diff, int128_zero())) {
+    if (uint128_lt(diff, uint128_zero())) {
         return -1;
-    } else if (int128_gt(diff, int128_zero())) {
+    } else if (uint128_gt(diff, uint128_zero())) {
         return 1;
     }
     return 0;
diff --git a/hw/pci-host/ppce500.c b/hw/pci-host/ppce500.c
index 613ba73..9002c51 100644
--- a/hw/pci-host/ppce500.c
+++ b/hw/pci-host/ppce500.c
@@ -428,7 +428,7 @@ static void e500_pcihost_bridge_realize(PCIDevice *d, Error **errp)
         PCI_HEADER_TYPE_BRIDGE;
 
     memory_region_init_alias(&b->bar0, OBJECT(ccsr), "e500-pci-bar0", &ccsr->ccsr_space,
-                             0, int128_get64(ccsr->ccsr_space.size));
+                             0, uint128_to_64(ccsr->ccsr_space.size));
     pci_register_bar(d, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &b->bar0);
 }
 
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations
  2015-11-05 16:18 [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations Pierre Morel
                   ` (6 preceding siblings ...)
  2015-11-05 16:18 ` [Qemu-devel] [PATCH 7/7] 128bits: some HW components use 128bit arithmetic Pierre Morel
@ 2015-11-05 16:32 ` Paolo Bonzini
  2015-11-06  8:36   ` Pierre Morel
  7 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2015-11-05 16:32 UTC (permalink / raw)
  To: Pierre Morel, qemu-devel; +Cc: cornelia.huck, peter.maydell



On 05/11/2015 17:18, Pierre Morel wrote:
> The size of a memory area can never be negative.
> It follows it must be defined as an unsigned value.
>     
> Let's modify the memory regions size to unsigned 128 integer
> and modify accordingly the 128 bit arithmetic.
>     
> This makes memory size calculations easier and easier to understand.
>     
> I fear loud protest but really, it is a broken concept that
> obfuscate the code.

You are right in fearing loud protest, though the protest is for the
lack of explanation of what is broken.

Since the values are never going to be > 2^65, there is no chance of
overflow.  On the other hand there are cases where we compute
start+size-1, and size-1 *could* overflow if you use unsigned integers.

So I am not sure... why?

Paolo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations
  2015-11-05 16:32 ` [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations Paolo Bonzini
@ 2015-11-06  8:36   ` Pierre Morel
  2015-11-06 16:33     ` Paolo Bonzini
  0 siblings, 1 reply; 14+ messages in thread
From: Pierre Morel @ 2015-11-06  8:36 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: cornelia.huck, peter.maydell



On 11/05/2015 05:32 PM, Paolo Bonzini wrote:
>
> On 05/11/2015 17:18, Pierre Morel wrote:
>> The size of a memory area can never be negative.
>> It follows it must be defined as an unsigned value.
>>      
>> Let's modify the memory regions size to unsigned 128 integer
>> and modify accordingly the 128 bit arithmetic.
>>      
>> This makes memory size calculations easier and easier to understand.
>>      
>> I fear loud protest but really, it is a broken concept that
>> obfuscate the code.
> You are right in fearing loud protest, though the protest is for the
> lack of explanation of what is broken.
>
> Since the values are never going to be > 2^65, there is no chance of
> overflow.  On the other hand there are cases where we compute
> start+size-1, and size-1 *could* overflow if you use unsigned integers.
>
> So I am not sure... why?
>
> Paolo
>
Paolo,

The calculation are not broken and it works for actual usage but:

For me, it is the design that is broken, as it uses an integer to represent
something that is fundamentally unsigned like the size of a memory area.

This leads to have UINT64_MAX represented with {1, 0} instead of {0, 
UINT64_MAX}
while {1, 0} is 2^64.
This again leads to have unnecessary and obfuscating transformations 
with int128_2_64()
to test for UINT64_MAX and return {1,0} in memory_region_init() while using
inverse translation test{1,0} and return UINT64_MAX in memory_region_size()

Another concern is that pushing the 31th bit to the 32nd bit may be 
confusing when
used, if ever, with IOMMU or other hardware components using 128bits 
registers.
Considering some Intel IOMMU for example.

The modifications I proposed are also not perfect, I just seen I forgot 
to change a -1 to UINT64_MAX
and I forgot some unnecessary cast. At least.

Pierre

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations
  2015-11-06  8:36   ` Pierre Morel
@ 2015-11-06 16:33     ` Paolo Bonzini
       [not found]       ` <56408B28.8070408@linux.vnet.ibm.com>
  0 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2015-11-06 16:33 UTC (permalink / raw)
  To: Pierre Morel, qemu-devel; +Cc: cornelia.huck, peter.maydell



On 06/11/2015 09:36, Pierre Morel wrote:
> The calculation are not broken and it works for actual usage

Thanks for confirming.

> For me, it is the design that is broken, as it uses an integer to represent
> something that is fundamentally unsigned like the size of a memory area.

But it uses a very large integer. :)

Consider that every time you do math on uint16_t, C is actually using a
32-bit signed integer.  This is roughly the same.

> This leads to have UINT64_MAX represented with {1, 0} instead of {0,
> UINT64_MAX}
> while {1, 0} is 2^64.
> This again leads to have unnecessary and obfuscating transformations
> with int128_2_64()
> to test for UINT64_MAX and return {1,0} in memory_region_init() while using
> inverse translation test{1,0} and return UINT64_MAX in memory_region_size()

Yes, the use of UINT64_MAX for 2^64 is a hack, but it is unrelated to
the signedness of Int128.

Paolo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations
       [not found]       ` <56408B28.8070408@linux.vnet.ibm.com>
@ 2015-11-09 12:20         ` Paolo Bonzini
  2015-11-10  9:08           ` Pierre Morel
  0 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2015-11-09 12:20 UTC (permalink / raw)
  To: Pierre Morel, qemu-devel



On 09/11/2015 13:01, Pierre Morel wrote:
>>> This leads to have UINT64_MAX represented with {1, 0} instead of
>>> {0, UINT64_MAX} while {1, 0} is 2^64. This again leads to have
>>> unnecessary and obfuscating transformations with int128_2_64() to
>>> test for UINT64_MAX and return {1,0} in memory_region_init()
>>> while using inverse translation test{1,0} and return UINT64_MAX
>>> in memory_region_size()>>
>>
>> Yes, the use of UINT64_MAX for 2^64 is a hack, but it is unrelated to
>> the signedness of Int128.
> 
> OK, we agree it is a hack,
> but sorry, I should have missed something,
> because I do not understand what this hack is useful for.

It's used in the size argument of memory_region_init*, so that it can
remain an uint64_t.  The size is usually small (up to 2^40, say) unless
it is 2^64 meaning "the whole address space".  The latter case is
covered by UINT64_MAX.

Paolo

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations
  2015-11-09 12:20         ` Paolo Bonzini
@ 2015-11-10  9:08           ` Pierre Morel
  2015-11-10 12:12             ` Pierre Morel
  0 siblings, 1 reply; 14+ messages in thread
From: Pierre Morel @ 2015-11-10  9:08 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel



On 11/09/2015 01:20 PM, Paolo Bonzini wrote:
>
> On 09/11/2015 13:01, Pierre Morel wrote:
>>>> This leads to have UINT64_MAX represented with {1, 0} instead of
>>>> {0, UINT64_MAX} while {1, 0} is 2^64. This again leads to have
>>>> unnecessary and obfuscating transformations with int128_2_64() to
>>>> test for UINT64_MAX and return {1,0} in memory_region_init()
>>>> while using inverse translation test{1,0} and return UINT64_MAX
>>>> in memory_region_size()>>
>>> Yes, the use of UINT64_MAX for 2^64 is a hack, but it is unrelated to
>>> the signedness of Int128.
>> OK, we agree it is a hack,
>> but sorry, I should have missed something,
>> because I do not understand what this hack is useful for.
> It's used in the size argument of memory_region_init*, so that it can
> remain an uint64_t.  The size is usually small (up to 2^40, say) unless
> it is 2^64 meaning "the whole address space".  The latter case is
> covered by UINT64_MAX.
>
> Paolo
>

OK, I understand, thanks for having taking time for me.

To sum-up size is a size :-) and not an offset in memory.

Size of UINT64_MAX does not exist but we can live without it, having
a description for "whole address space", 2^64, can be useful.

Even there may be other solutions like taking 0 for 2^64,
if a memory size of 0 has no meaning,
but it could be misleading too.

So I do not see better solution for this interesting problematic.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations
  2015-11-10  9:08           ` Pierre Morel
@ 2015-11-10 12:12             ` Pierre Morel
  0 siblings, 0 replies; 14+ messages in thread
From: Pierre Morel @ 2015-11-10 12:12 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel



On 11/10/2015 10:08 AM, Pierre Morel wrote:
>
>
> On 11/09/2015 01:20 PM, Paolo Bonzini wrote:
>>
>> On 09/11/2015 13:01, Pierre Morel wrote:
>>>>> This leads to have UINT64_MAX represented with {1, 0} instead of
>>>>> {0, UINT64_MAX} while {1, 0} is 2^64. This again leads to have
>>>>> unnecessary and obfuscating transformations with int128_2_64() to
>>>>> test for UINT64_MAX and return {1,0} in memory_region_init()
>>>>> while using inverse translation test{1,0} and return UINT64_MAX
>>>>> in memory_region_size()>>
>>>> Yes, the use of UINT64_MAX for 2^64 is a hack, but it is unrelated to
>>>> the signedness of Int128.
>>> OK, we agree it is a hack,
>>> but sorry, I should have missed something,
>>> because I do not understand what this hack is useful for.
>> It's used in the size argument of memory_region_init*, so that it can
>> remain an uint64_t.  The size is usually small (up to 2^40, say) unless
>> it is 2^64 meaning "the whole address space".  The latter case is
>> covered by UINT64_MAX.
>>
>> Paolo
>>
>
> OK, I understand, thanks for having taking time for me.
>
> To sum-up size is a size :-) and not an offset in memory.
>
> Size of UINT64_MAX does not exist but we can live without it, having
> a description for "whole address space", 2^64, can be useful.
>
> Even there may be other solutions like taking 0 for 2^64,
> if a memory size of 0 has no meaning,
> but it could be misleading too.
>
> So I do not see better solution for this interesting problematic.
>
>
>

My problem with this came because usually, on hardware, region are easier
described by start/end rather than by start/size.
But  changing this in the actual implementation would be too much.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-11-10 12:13 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-05 16:18 [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations Pierre Morel
2015-11-05 16:18 ` [Qemu-devel] [PATCH 1/7] int128: use unsigned 128 bit arithmetic Pierre Morel
2015-11-05 16:18 ` [Qemu-devel] [PATCH 2/7] memory: modify memory size for an unsigned 128bit int Pierre Morel
2015-11-05 16:18 ` [Qemu-devel] [PATCH 3/7] 128bit: adapt core files for unsigned 128bits Pierre Morel
2015-11-05 16:18 ` [Qemu-devel] [PATCH 4/7] 128bit: adapt sparc mmu_helper for UInt128 Pierre Morel
2015-11-05 16:18 ` [Qemu-devel] [PATCH 5/7] 128bit: adapt VFIO for UInt128 arithmetic Pierre Morel
2015-11-05 16:18 ` [Qemu-devel] [PATCH 6/7] 128bit: adapt Virtio " Pierre Morel
2015-11-05 16:18 ` [Qemu-devel] [PATCH 7/7] 128bits: some HW components use 128bit arithmetic Pierre Morel
2015-11-05 16:32 ` [Qemu-devel] [PATCH 0/7] int128: reparing broken 128 bit memory calculations Paolo Bonzini
2015-11-06  8:36   ` Pierre Morel
2015-11-06 16:33     ` Paolo Bonzini
     [not found]       ` <56408B28.8070408@linux.vnet.ibm.com>
2015-11-09 12:20         ` Paolo Bonzini
2015-11-10  9:08           ` Pierre Morel
2015-11-10 12:12             ` Pierre Morel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).