vfio: convert to 128 bit arithmetic calculations when adding mem regions

vfio_listener_region_add for a iommu mr results in an overflow assert since vt-d memory region is initialized with UINT64_MAX. Convert calculations to 128 bit arithmetic for such regions and let int128_get64 assert for non iommu regions only.

Suggested-by Alex Williamson <[email protected]>

Signed-off-by Bandan Das <[email protected]>

---
This is v2 of http://patchwork.ozlabs.org/patch/600364/

 hw/vfio/common.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

hw/vfio/common.c +1 -1

index fb588d8..9489ff3 100644
@@ -323,7 +323,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
 {
     VFIOContainer *container = container_of(listener, VFIOContainer, listener);
     hwaddr iova, end;
-    Int128 llend;
+    Int128 llend, endaddr;
     void *vaddr;
     int ret;
 

hw/vfio/common.c +0 -3

-    if ((iova < container->min_iova) || ((end - 1) > container->max_iova)) {
-        error_report("vfio: IOMMU container %p can't map guest IOVA region"
-                     " 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx,

hw/vfio/common.c +1 -1

     }
 
     memory_region_ref(section->mr);

@@ -364,7 +365,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
     if (memory_region_is_iommu(section->mr)) {
         VFIOGuestIOMMU *giommu;
 
-        trace_vfio_listener_region_add_iommu(iova, end - 1);
+        trace_vfio_listener_region_add_iommu(iova, int128_get64(endaddr));
         /*
          * FIXME: We should do some checking to see if the
          * capabilities of the host VFIO IOMMU are adequate to model

Quoted 15 lines

hw/vfio/common.c +1 -1

index fb588d8..9489ff3 100644
@@ -323,7 +323,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
 {
     VFIOContainer *container = container_of(listener, VFIOContainer, listener);
     hwaddr iova, end;
-    Int128 llend;
+    Int128 llend, endaddr;
     void *vaddr;
     int ret;
 

hw/vfio/common.c static void vfio_listener_region_add(MemoryListener *listener, · line 352

Quoted 5 lines

hw/vfio/common.c +1 -1

@@ -349,14 +349,15 @@ static void vfio_listener_region_add(MemoryListener *listener,
     if (int128_ge(int128_make64(iova), llend)) {
         return;
     }
-    end = int128_get64(llend);
+    endaddr = int128_sub(llend, int128_one());

I like where this is headed, but there's maybe a little polish left.

endaddr here can never be greater than UINT64_MAX, so why don't we make it a hwaddr and wrap the whole right side of the equation in an int128_get64(). Then the (end - 1) changes below don't need the extra int128 ops.

Quoted 4 lines

hw/vfio/common.c +0 -3

-    if ((iova < container->min_iova) || ((end - 1) > container->max_iova)) {
-        error_report("vfio: IOMMU container %p can't map guest IOVA region"
-                     " 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx,

hw/vfio/common.c static void vfio_listener_region_add(MemoryListener *listener, · line 360

Quoted 10 lines

hw/vfio/common.c +7 -3

-                     container, iova, end - 1);
-        ret = -EFAULT;
-        goto fail;
+    if ((iova < container->min_iova) || (int128_get64(endaddr) >
+                                         container->max_iova)) {
+            error_report("vfio: IOMMU container %p can't map guest IOVA region"
+                         " 0x%"HWADDR_PRIx"..0x%"HWADDR_PRIx,
+                         container, iova, int128_get64(endaddr));
+            ret = -EFAULT;
+            goto fail;

Bogus whitespace changes

Quoted 13 lines

hw/vfio/common.c +1 -1

     }
 
     memory_region_ref(section->mr);

@@ -364,7 +365,7 @@ static void vfio_listener_region_add(MemoryListener *listener,
     if (memory_region_is_iommu(section->mr)) {
         VFIOGuestIOMMU *giommu;
 
-        trace_vfio_listener_region_add_iommu(iova, end - 1);
+        trace_vfio_listener_region_add_iommu(iova, int128_get64(endaddr));
         /*
          * FIXME: We should do some checking to see if the
          * capabilities of the host VFIO IOMMU are adequate to model

hw/vfio/common.c static void vfio_listener_region_add(MemoryListener *listener, · line 397

Quoted 8 lines

hw/vfio/common.c +2 -0

@@ -389,6 +390,8 @@ static void vfio_listener_region_add(MemoryListener *listener,
         return;
     }
 
+    end = int128_get64(llend);
+
     /* Here we assume that memory_region_is_ram(section->mr)==true */
 
     vaddr = memory_region_get_ram_ptr(section->mr) +

For this last chunk I'd do something like this:

(quoted diff) +4 -2

+    llsize = int128_sub(llend, int128_make64(iova));
 
-    ret = vfio_dma_map(container, iova, end - iova, vaddr, section->readonly);
+    ret = vfio_dma_map(container, iova,
+                       int128_get64(llsize), vaddr, section->readonly);
     if (ret) {
         error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
                      "0x%"HWADDR_PRIx", %p) = %d (%m)",
-                     container, iova, end - iova, vaddr, ret);
+                     container, iova, int128_get64(llsize), vaddr, ret);
         goto fail;
     }

This maintains the 128bit math until exactly the point where it will fall apart when vfio_dma_map can't pass an int128 to the vfio API.

Thanks,

Alex