qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Qemu-devel] [PATCH v3 5/5] Vring: use hostmem's RAM safe api


From: Liu Ping Fan
Subject: [Qemu-devel] [PATCH v3 5/5] Vring: use hostmem's RAM safe api
Date: Mon, 6 May 2013 20:49:00 +0800

From: Liu Ping Fan <address@hidden>

Before mm-ops done, we should gurantee the validaion of regions which is
used by Vring self and the chunck pointed by vring desc. We acheive
this goal by inc refcnt of RAM-Device. When finished, we dec this cnt
through the interface of MemoryRegion.

We keep the MemoryRegion's reference info totally in Vring, so the caller
can not be aware of the reference.

Signed-off-by: Liu Ping Fan <address@hidden>
---
 hw/block/dataplane/virtio-blk.c     |    8 ---
 hw/virtio/dataplane/vring.c         |   93 +++++++++++++++++++++++++++-------
 include/hw/virtio/dataplane/vring.h |   15 ++++++
 3 files changed, 89 insertions(+), 27 deletions(-)

diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 0356665..4babda1 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -25,14 +25,6 @@
 #include "block/aio.h"
 #include "hw/virtio/virtio-bus.h"
 
-enum {
-    SEG_MAX = 126,                  /* maximum number of I/O segments */
-    VRING_MAX = SEG_MAX + 2,        /* maximum number of vring descriptors */
-    REQ_MAX = VRING_MAX,            /* maximum number of requests in the vring,
-                                     * is VRING_MAX / 2 with traditional and
-                                     * VRING_MAX with indirect descriptors */
-};
-
 typedef struct {
     struct iocb iocb;               /* Linux AIO control block */
     QEMUIOVector *inhdr;            /* iovecs for virtio_blk_inhdr */
diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c
index e3c3afb..2cfd6d0 100644
--- a/hw/virtio/dataplane/vring.c
+++ b/hw/virtio/dataplane/vring.c
@@ -27,7 +27,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
 
     vring->broken = false;
 
-    vring_ptr = hostmem_lookup(vring_addr, vring_size, NULL, true);
+    vring_ptr = hostmem_lookup(vring_addr, vring_size, &vring->vring_mr, true);
     if (!vring_ptr) {
         error_report("Failed to map vring "
                      "addr %#" HWADDR_PRIx " size %" HWADDR_PRIu,
@@ -50,6 +50,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n)
 
 void vring_teardown(Vring *vring)
 {
+    memory_region_unref(vring->vring_mr);
 }
 
 /* Disable guest->host notifies */
@@ -109,11 +110,14 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring)
 static int get_indirect(Vring *vring,
                         struct iovec iov[], struct iovec *iov_end,
                         unsigned int *out_num, unsigned int *in_num,
-                        struct vring_desc *indirect)
+                        struct vring_desc *indirect,
+                        MemoryRegion ***mrs)
 {
     struct vring_desc desc;
     unsigned int i = 0, count, found = 0;
-
+    MemoryRegion **cur = *mrs;
+    int ret = 0;
+    MemoryRegion *desc_mr;
     /* Sanity check */
     if (unlikely(indirect->len % sizeof(desc))) {
         error_report("Invalid length in indirect descriptor: "
@@ -137,49 +141,58 @@ static int get_indirect(Vring *vring,
 
         /* Translate indirect descriptor */
         desc_ptr = hostmem_lookup(indirect->addr + found * sizeof(desc),
-                                  sizeof(desc), NULL, false);
+                                  sizeof(desc),
+                                  &desc_mr,
+                                  false);
         if (!desc_ptr) {
             error_report("Failed to map indirect descriptor "
                          "addr %#" PRIx64 " len %zu",
                          (uint64_t)indirect->addr + found * sizeof(desc),
                          sizeof(desc));
             vring->broken = true;
-            return -EFAULT;
+            ret = -EFAULT;
+            goto fail;
         }
         desc = *desc_ptr;
 
         /* Ensure descriptor has been loaded before accessing fields */
         barrier(); /* read_barrier_depends(); */
+        memory_region_unref(desc_mr);
 
         if (unlikely(++found > count)) {
             error_report("Loop detected: last one at %u "
                          "indirect size %u", i, count);
             vring->broken = true;
-            return -EFAULT;
+            ret = -EFAULT;
+            goto fail;
         }
 
         if (unlikely(desc.flags & VRING_DESC_F_INDIRECT)) {
             error_report("Nested indirect descriptor");
             vring->broken = true;
-            return -EFAULT;
+            ret = -EFAULT;
+            goto fail;
         }
 
         /* Stop for now if there are not enough iovecs available. */
         if (iov >= iov_end) {
-            return -ENOBUFS;
+            ret = -ENOBUFS;
+            goto fail;
         }
 
-        iov->iov_base = hostmem_lookup(desc.addr, desc.len, NULL,
+        iov->iov_base = hostmem_lookup(desc.addr, desc.len, cur,
                                        desc.flags & VRING_DESC_F_WRITE);
         if (!iov->iov_base) {
             error_report("Failed to map indirect descriptor"
                          "addr %#" PRIx64 " len %u",
                          (uint64_t)desc.addr, desc.len);
             vring->broken = true;
-            return -EFAULT;
+            ret = -EFAULT;
+            goto fail;
         }
         iov->iov_len = desc.len;
         iov++;
+        cur++;
 
         /* If this is an input descriptor, increment that count. */
         if (desc.flags & VRING_DESC_F_WRITE) {
@@ -191,13 +204,23 @@ static int get_indirect(Vring *vring,
                 error_report("Indirect descriptor "
                              "has out after in: idx %u", i);
                 vring->broken = true;
-                return -EFAULT;
+                ret = -EFAULT;
+                goto fail;
             }
             *out_num += 1;
         }
         i = desc.next;
     } while (desc.flags & VRING_DESC_F_NEXT);
+    *mrs = cur;
     return 0;
+
+fail:
+    /* We rely on mrs[] initialized as zero */
+    for (i = 0; *mrs[i] && cur >= *mrs+i; cur--) {
+        memory_region_unref(*mrs[i]);
+    }
+
+    return ret;
 }
 
 /* This looks in the virtqueue and for the first available buffer, and converts
@@ -209,6 +232,8 @@ static int get_indirect(Vring *vring,
  * never a valid descriptor number) if none was found.  A negative code is
  * returned on error.
  *
+ * @mrs should be the same cnt as iov[]
+ *
  * Stolen from linux/drivers/vhost/vhost.c.
  */
 int vring_pop(VirtIODevice *vdev, Vring *vring,
@@ -218,7 +243,10 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
     struct vring_desc desc;
     unsigned int i, head, found = 0, num = vring->vr.num;
     uint16_t avail_idx, last_avail_idx;
+    MemoryRegion **indirect, **cur, **mrs;
+    int ret = 0;
 
+    cur = mrs = g_malloc0((iov_end - iov)*sizeof(void *));
     /* If there was a fatal error then refuse operation */
     if (vring->broken) {
         return -EFAULT;
@@ -267,13 +295,15 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
         if (unlikely(i >= num)) {
             error_report("Desc index is %u > %u, head = %u", i, num, head);
             vring->broken = true;
-            return -EFAULT;
+            ret = -EFAULT;
+            goto fail;
         }
         if (unlikely(++found > num)) {
             error_report("Loop detected: last one at %u vq size %u head %u",
                          i, num, head);
             vring->broken = true;
-            return -EFAULT;
+            ret = -EFAULT;
+            goto fail;
         }
         desc = vring->vr.desc[i];
 
@@ -281,10 +311,13 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
         barrier();
 
         if (desc.flags & VRING_DESC_F_INDIRECT) {
-            int ret = get_indirect(vring, iov, iov_end, out_num, in_num, 
&desc);
+            indirect = cur;
+            int ret = get_indirect(vring, iov, iov_end, out_num, in_num, &desc,
+                        &indirect);
             if (ret < 0) {
-                return ret;
+                goto fail;
             }
+            cur = indirect;
             continue;
         }
 
@@ -293,20 +326,23 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
          * with the current set.
          */
         if (iov >= iov_end) {
-            return -ENOBUFS;
+            ret = -ENOBUFS;
+            goto fail;
         }
 
         /* TODO handle non-contiguous memory across region boundaries */
-        iov->iov_base = hostmem_lookup(desc.addr, desc.len, NULL,
+        iov->iov_base = hostmem_lookup(desc.addr, desc.len, cur,
                                        desc.flags & VRING_DESC_F_WRITE);
         if (!iov->iov_base) {
             error_report("Failed to map vring desc addr %#" PRIx64 " len %u",
                          (uint64_t)desc.addr, desc.len);
             vring->broken = true;
-            return -EFAULT;
+            ret = -EFAULT;
+            goto fail;
         }
         iov->iov_len  = desc.len;
         iov++;
+        cur++;
 
         if (desc.flags & VRING_DESC_F_WRITE) {
             /* If this is an input descriptor,
@@ -318,7 +354,8 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
             if (unlikely(*in_num)) {
                 error_report("Descriptor has out after in: idx %d", i);
                 vring->broken = true;
-                return -EFAULT;
+                ret = -EFAULT;
+                goto fail;
             }
             *out_num += 1;
         }
@@ -327,7 +364,17 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
 
     /* On success, increment avail index. */
     vring->last_avail_idx++;
+    vring->mrs_vect[head].used_cnt = cur - mrs;
+    vring->mrs_vect[head].mrs = mrs;
     return head;
+
+fail:
+    /* We rely on mrs[] initialized as zero */
+    for (i = 0; mrs[i]; i++) {
+        memory_region_unref(mrs[i]);
+    }
+
+    return ret;
 }
 
 /* After we've used one of their buffers, we tell them about it.
@@ -338,6 +385,8 @@ void vring_push(Vring *vring, unsigned int head, int len)
 {
     struct vring_used_elem *used;
     uint16_t new;
+    int i;
+    MrsVect *mrs_vect;
 
     /* Don't touch vring if a fatal error occurred */
     if (vring->broken) {
@@ -357,4 +406,10 @@ void vring_push(Vring *vring, unsigned int head, int len)
     if (unlikely((int16_t)(new - vring->signalled_used) < (uint16_t)1)) {
         vring->signalled_used_valid = false;
     }
+    mrs_vect = &vring->mrs_vect[head];
+    for (i = 0; i < mrs_vect->used_cnt; i++) {
+        memory_region_unref(mrs_vect->mrs[i]);
+    }
+    mrs_vect->used_cnt = 0;
+    g_free(mrs_vect->mrs);
 }
diff --git a/include/hw/virtio/dataplane/vring.h 
b/include/hw/virtio/dataplane/vring.h
index 56acffb..88dac68 100644
--- a/include/hw/virtio/dataplane/vring.h
+++ b/include/hw/virtio/dataplane/vring.h
@@ -22,13 +22,28 @@
 #include "hostmem.h"
 #include "hw/virtio/virtio.h"
 
+enum {
+    SEG_MAX = 126,                  /* maximum number of I/O segments */
+    VRING_MAX = SEG_MAX + 2,        /* maximum number of vring descriptors */
+    REQ_MAX = VRING_MAX,            /* maximum number of requests in the vring,
+                                     * is VRING_MAX / 2 with traditional and
+                                     * VRING_MAX with indirect descriptors */
+};
+
+typedef struct {
+    int used_cnt;
+    MemoryRegion **mrs;
+} MrsVect;
+
 typedef struct {
+    MemoryRegion *vring_mr;   /* RAM's memoryRegion on which this vring sits */
     struct vring vr;                /* virtqueue vring mapped to host memory */
     uint16_t last_avail_idx;        /* last processed avail ring index */
     uint16_t last_used_idx;         /* last processed used ring index */
     uint16_t signalled_used;        /* EVENT_IDX state */
     bool signalled_used_valid;
     bool broken;                    /* was there a fatal error? */
+    MrsVect mrs_vect[VRING_MAX];
 } Vring;
 
 static inline unsigned int vring_get_num(Vring *vring)
-- 
1.7.4.4




reply via email to

[Prev in Thread] Current Thread [Next in Thread]