diff mbox series

[[RFC,v3,05/12] virtio: init and desc empty check for packed ring

Message ID 1539266915-15216-6-git-send-email-wexu@redhat.com (mailing list archive)
State New, archived
Headers show
Series packed ring virtio-net userspace backend support | expand

Commit Message

Wei Xu Oct. 11, 2018, 2:08 p.m. UTC 
From: Wei Xu <wexu@redhat.com>

Basic initialization and helpers for packed ring.

Signed-off-by: Wei Xu <wexu@redhat.com>
---
 hw/virtio/virtio.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

Comments

Jason Wang Oct. 15, 2018, 3:18 a.m. UTC | #1 
On 2018年10月11日 22:08, wexu@redhat.com wrote:
> From: Wei Xu <wexu@redhat.com>
>
> Basic initialization and helpers for packed ring.
>
> Signed-off-by: Wei Xu <wexu@redhat.com>
> ---
>   hw/virtio/virtio.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 56 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 9185efb..86f88da 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
> @@ -372,6 +375,23 @@ int virtio_queue_ready(VirtQueue *vq)
>       return vq->vring.avail != 0;
>   }
>   
> +static void vring_packed_desc_read_flags(VirtIODevice *vdev,
> +                    VRingPackedDesc *desc, MemoryRegionCache *cache, int i)
> +{
> +    address_space_read_cached(cache,
> +              i * sizeof(VRingPackedDesc) + offsetof(VRingPackedDesc, flags),
> +              &desc->flags, sizeof(desc->flags));
> +}
> +
> +static inline bool is_desc_avail(struct VRingPackedDesc *desc, bool wc)
> +{

I think it's better use wrap_counter instead of wc here (unless you want 
to use wc everywhere which is a even worse idea).

Thanks

> +    bool avail, used;
> +
> +    avail = !!(desc->flags & AVAIL_DESC_PACKED(1));
> +    used = !!(desc->flags & USED_DESC_PACKED(1));
> +    return (avail != used) && (avail == wc);
> +}
> +
>   /* Fetch avail_idx from VQ memory only when we really need to know if
>    * guest has added some buffers.
>    * Called within rcu_read_lock().  */
> @@ -392,7 +412,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;
>   
> @@ -414,6 +434,41 @@ int virtio_queue_empty(VirtQueue *vq)
>       return empty;
>   }
>   
> +static int virtio_queue_packed_empty_rcu(VirtQueue *vq)
> +{
> +    struct VRingPackedDesc desc;
> +    VRingMemoryRegionCaches *cache;
> +
> +    if (unlikely(!vq->vring.desc)) {
> +        return 1;
> +    }
> +
> +    cache = vring_get_region_caches(vq);
> +    vring_packed_desc_read_flags(vq->vdev, &desc, &cache->desc,
> +                                vq->last_avail_idx);
> +
> +    return !is_desc_avail(&desc, vq->avail_wrap_counter);
> +}
> +
> +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)
>   {
Wei Xu Oct. 15, 2018, 7:04 a.m. UTC | #2 
On Mon, Oct 15, 2018 at 11:18:05AM +0800, Jason Wang wrote:
> 
> 
> On 2018年10月11日 22:08, wexu@redhat.com wrote:
> >From: Wei Xu <wexu@redhat.com>
> >
> >Basic initialization and helpers for packed ring.
> >
> >Signed-off-by: Wei Xu <wexu@redhat.com>
> >---
> >  hw/virtio/virtio.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 56 insertions(+), 1 deletion(-)
> >
> >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >index 9185efb..86f88da 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
> >@@ -372,6 +375,23 @@ int virtio_queue_ready(VirtQueue *vq)
> >      return vq->vring.avail != 0;
> >  }
> >+static void vring_packed_desc_read_flags(VirtIODevice *vdev,
> >+                    VRingPackedDesc *desc, MemoryRegionCache *cache, int i)
> >+{
> >+    address_space_read_cached(cache,
> >+              i * sizeof(VRingPackedDesc) + offsetof(VRingPackedDesc, flags),
> >+              &desc->flags, sizeof(desc->flags));
> >+}
> >+
> >+static inline bool is_desc_avail(struct VRingPackedDesc *desc, bool wc)
> >+{
> 
> I think it's better use wrap_counter instead of wc here (unless you want to
> use wc everywhere which is a even worse idea).

It was to avoid a new line for a parameter since this is a mini function,
I will take it back.

Wei

> 
> Thanks
> 
> >+    bool avail, used;
> >+
> >+    avail = !!(desc->flags & AVAIL_DESC_PACKED(1));
> >+    used = !!(desc->flags & USED_DESC_PACKED(1));
> >+    return (avail != used) && (avail == wc);
> >+}
> >+
> >  /* Fetch avail_idx from VQ memory only when we really need to know if
> >   * guest has added some buffers.
> >   * Called within rcu_read_lock().  */
> >@@ -392,7 +412,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;
> >@@ -414,6 +434,41 @@ int virtio_queue_empty(VirtQueue *vq)
> >      return empty;
> >  }
> >+static int virtio_queue_packed_empty_rcu(VirtQueue *vq)
> >+{
> >+    struct VRingPackedDesc desc;
> >+    VRingMemoryRegionCaches *cache;
> >+
> >+    if (unlikely(!vq->vring.desc)) {
> >+        return 1;
> >+    }
> >+
> >+    cache = vring_get_region_caches(vq);
> >+    vring_packed_desc_read_flags(vq->vdev, &desc, &cache->desc,
> >+                                vq->last_avail_idx);
> >+
> >+    return !is_desc_avail(&desc, vq->avail_wrap_counter);
> >+}
> >+
> >+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)
> >  {
>
diff mbox series

Patch

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 9185efb..86f88da 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
@@ -372,6 +375,23 @@  int virtio_queue_ready(VirtQueue *vq)
     return vq->vring.avail != 0;
 }
 
+static void vring_packed_desc_read_flags(VirtIODevice *vdev,
+                    VRingPackedDesc *desc, MemoryRegionCache *cache, int i)
+{
+    address_space_read_cached(cache,
+              i * sizeof(VRingPackedDesc) + offsetof(VRingPackedDesc, flags),
+              &desc->flags, sizeof(desc->flags));
+}
+
+static inline bool is_desc_avail(struct VRingPackedDesc *desc, bool wc)
+{
+    bool avail, used;
+
+    avail = !!(desc->flags & AVAIL_DESC_PACKED(1));
+    used = !!(desc->flags & USED_DESC_PACKED(1));
+    return (avail != used) && (avail == wc);
+}
+
 /* Fetch avail_idx from VQ memory only when we really need to know if
  * guest has added some buffers.
  * Called within rcu_read_lock().  */
@@ -392,7 +412,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;
 
@@ -414,6 +434,41 @@  int virtio_queue_empty(VirtQueue *vq)
     return empty;
 }
 
+static int virtio_queue_packed_empty_rcu(VirtQueue *vq)
+{
+    struct VRingPackedDesc desc;
+    VRingMemoryRegionCaches *cache;
+
+    if (unlikely(!vq->vring.desc)) {
+        return 1;
+    }
+
+    cache = vring_get_region_caches(vq);
+    vring_packed_desc_read_flags(vq->vdev, &desc, &cache->desc,
+                                vq->last_avail_idx);
+
+    return !is_desc_avail(&desc, vq->avail_wrap_counter);
+}
+
+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)
 {