[Qemu-devel] [PATCH] exec: don't return int64_t in address_space_cache_init()

[Jason Wang <jasowang@redhat.com>]

We return int64_t as the length of region cache but accept hwaddr as
the required length. This is wrong and may confuse the caller since
the it can lead comparison between signed and unsigned types. The
caller can not catch the failure in this case. Fixing this by
returning hwaddr and return zero on failure.

Fixes: 5eba0404b9829 ("virtio: use MemoryRegionCache to access descriptors")
Fixes: e45da65322386 ("virtio: validate address space cache during init")
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 exec.c                | 12 ++++++------
 hw/virtio/virtio.c    |  7 +++----
 include/exec/memory.h | 13 ++++++-------
 3 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/exec.c b/exec.c
index e57a8a2..9b71174 100644
--- a/exec.c
+++ b/exec.c
@@ -3230,11 +3230,11 @@ void cpu_physical_memory_unmap(void *buffer, hwaddr len,
 #define RCU_READ_UNLOCK(...)     rcu_read_unlock()
 #include "memory_ldst.inc.c"
 
-int64_t address_space_cache_init(MemoryRegionCache *cache,
-                                 AddressSpace *as,
-                                 hwaddr addr,
-                                 hwaddr len,
-                                 bool is_write)
+hwaddr address_space_cache_init(MemoryRegionCache *cache,
+                                AddressSpace *as,
+                                hwaddr addr,
+                                hwaddr len,
+                                bool is_write)
 {
     hwaddr l, xlat;
     MemoryRegion *mr;
@@ -3245,7 +3245,7 @@ int64_t address_space_cache_init(MemoryRegionCache *cache,
     l = len;
     mr = address_space_translate(as, addr, &xlat, &l, is_write);
     if (!memory_access_is_direct(mr, is_write)) {
-        return -EINVAL;
+        return 0;
     }
 
     l = address_space_extend_translation(as, addr, len, mr, xlat, l, is_write);
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 03592c5..3482be2 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -129,9 +129,8 @@ static void virtio_init_region_cache(VirtIODevice *vdev, int n)
     VirtQueue *vq = &vdev->vq[n];
     VRingMemoryRegionCaches *old = vq->vring.caches;
     VRingMemoryRegionCaches *new;
-    hwaddr addr, size;
+    hwaddr addr, size, len;
     int event_size;
-    int64_t len;
 
     event_size = virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
 
@@ -586,7 +585,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
     unsigned int total_bufs, in_total, out_total;
     VRingMemoryRegionCaches *caches;
     MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
-    int64_t len = 0;
+    hwaddr len = 0;
     int rc;
 
     if (unlikely(!vq->vring.desc)) {
@@ -831,7 +830,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
     VRingMemoryRegionCaches *caches;
     MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
     MemoryRegionCache *desc_cache;
-    int64_t len;
+    hwaddr len;
     VirtIODevice *vdev = vq->vdev;
     VirtQueueElement *elem = NULL;
     unsigned out_num, in_num;
diff --git a/include/exec/memory.h b/include/exec/memory.h
index e39256a..932dd00 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1444,8 +1444,7 @@ struct MemoryRegionCache {
  * @is_write: indicates the transfer direction
  *
  * Will only work with RAM, and may map a subset of the requested range by
- * returning a value that is less than @len.  On failure, return a negative
- * errno value.
+ * returning a value that is less than @len.  On failure, return zero.
  *
  * Because it only works with RAM, this function can be used for
  * read-modify-write operations.  In this case, is_write should be %true.
@@ -1453,11 +1452,11 @@ struct MemoryRegionCache {
  * Note that addresses passed to the address_space_*_cached functions
  * are relative to @addr.
  */
-int64_t address_space_cache_init(MemoryRegionCache *cache,
-                                 AddressSpace *as,
-                                 hwaddr addr,
-                                 hwaddr len,
-                                 bool is_write);
+hwaddr address_space_cache_init(MemoryRegionCache *cache,
+                                AddressSpace *as,
+                                hwaddr addr,
+                                hwaddr len,
+                                bool is_write);
 
 /**
  * address_space_cache_invalidate: complete a write to a #MemoryRegionCache
-- 
2.7.4