qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [RFC v2 3/8] virtio: empty check and desc read for pack


From: Jason Wang
Subject: Re: [Qemu-devel] [RFC v2 3/8] virtio: empty check and desc read for packed ring
Date: Wed, 6 Jun 2018 11:09:29 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0



On 2018年06月06日 03:07, address@hidden wrote:
From: Wei Xu <address@hidden>

helper for ring empty check and descriptor read.

Signed-off-by: Wei Xu <address@hidden>
---
  hw/virtio/virtio.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++---
  1 file changed, 59 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index f6c0689..bd669a2 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -24,6 +24,9 @@
  #include "hw/virtio/virtio-access.h"
  #include "sysemu/dma.h"
+#define AVAIL_DESC_PACKED(b) ((b) << 7)
+#define USED_DESC_PACKED(b)  ((b) << 15)
+
  /*
   * The alignment to use between consumer and producer parts of vring.
   * x86 pagesize again. This is the default, used by transports like PCI
@@ -357,10 +360,27 @@ int virtio_queue_ready(VirtQueue *vq)
      return vq->vring.avail != 0;
  }
+static void vring_packed_desc_read(VirtIODevice *vdev, VRingDescPacked *desc,
+                            MemoryRegionCache *cache, int i)
+{
+    address_space_read_cached(cache, i * sizeof(VRingDescPacked),
+                              desc, sizeof(VRingDescPacked));
+    virtio_tswap64s(vdev, &desc->addr);
+    virtio_tswap32s(vdev, &desc->len);
+    virtio_tswap16s(vdev, &desc->id);
+    virtio_tswap16s(vdev, &desc->flags);
+}
+
+static inline bool is_desc_avail(struct VRingDescPacked *desc)
+{
+    return !!(desc->flags & AVAIL_DESC_PACKED(1)) !=
+            !!(desc->flags & USED_DESC_PACKED(1));
+}

See spec and the discussion in the past for vhost patches:

"""
Thus VIRTQ_DESC_F_AVAIL and VIRTQ_DESC_F_USED bits are different for an available descriptor and
equal for a used descriptor.
Note that this observation is mostly useful for sanity-checking as these are necessary but not sufficient conditions - for example, all descriptors are zero-initialized. To detect used and available descriptors it is possible for drivers and devices to keep track of the last observed value of VIRTQ_DESC_F_USED/VIRTQ_- DESC_F_AVAIL. Other techniques to detect VIRTQ_DESC_F_AVAIL/VIRTQ_DESC_F_USED bit changes
might also be possible.
"""

This may not work especially consider we may do migration or savevm/loadvm.

+
  /* Fetch avail_idx from VQ memory only when we really need to know if
   * guest has added some buffers.
   * Called within rcu_read_lock().  */
-static int virtio_queue_empty_rcu(VirtQueue *vq)
+static int virtio_queue_split_empty_rcu(VirtQueue *vq)
  {
      if (unlikely(!vq->vring.avail)) {
          return 1;
@@ -373,7 +393,7 @@ static int virtio_queue_empty_rcu(VirtQueue *vq)
      return vring_avail_idx(vq) == vq->last_avail_idx;
  }
-int virtio_queue_empty(VirtQueue *vq)
+static int virtio_queue_split_empty(VirtQueue *vq)
  {
      bool empty;
@@ -391,6 +411,42 @@ int virtio_queue_empty(VirtQueue *vq)
      return empty;
  }
+static int virtio_queue_packed_empty_rcu(VirtQueue *vq)
+{
+    struct VRingDescPacked desc;
+    VRingMemoryRegionCaches *cache;
+
+    if (unlikely(!vq->vring.desc)) {
+        return 1;
+    }
+
+    cache = vring_get_region_caches(vq);
+    vring_packed_desc_read(vq->vdev, &desc, &cache->desc, vq->last_avail_idx);
+

I believe only we only care flag here.

+    /* Make sure we see the updated flag */
+    smp_mb();

Why need this?

Note there's a smp_rmb() after virtio_queue_empty_rcu() in virtquue_pop().

+    return !is_desc_avail(&desc);
+}
+
+static int virtio_queue_packed_empty(VirtQueue *vq)
+{
+    bool empty;
+
+    rcu_read_lock();
+    empty = virtio_queue_packed_empty_rcu(vq);
+    rcu_read_unlock();
+    return empty;
+}
+
+int virtio_queue_empty(VirtQueue *vq)
+{
+    if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
+        return virtio_queue_packed_empty(vq);
+    } else {
+        return virtio_queue_split_empty(vq);
+    }
+}
+
  static void virtqueue_unmap_sg(VirtQueue *vq, const VirtQueueElement *elem,
                                 unsigned int len)
  {
@@ -862,7 +918,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
          return NULL;
      }
      rcu_read_lock();
-    if (virtio_queue_empty_rcu(vq)) {
+    if (virtio_queue_split_empty_rcu(vq)) {

This is meaningless unless you rename virtqueue_pop to virtqueue_pop_split and call it only for split ring.

Thanks

          goto done;
      }
      /* Needed after virtio_queue_empty(), see comment in




reply via email to

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