Re: [PATCH v5 03/11] net/virtio: add packed virtqueue helpers
From: Maxime Coquelin <hidden>
Date: 2018-09-12 08:26:02
On 09/06/2018 08:19 PM, Jens Freimann wrote:
quoted hunk ↗ jump to hunk
Add helper functions to set/clear and check descriptor flags. Signed-off-by: Jens Freimann <redacted> --- drivers/net/virtio/virtio_ring.h | 26 ++++++++++++++++++++++++++ drivers/net/virtio/virtqueue.h | 19 +++++++++++++++++++ 2 files changed, 45 insertions(+)diff --git a/drivers/net/virtio/virtio_ring.h b/drivers/net/virtio/virtio_ring.h index e2c597434..f3b23f419 100644 --- a/drivers/net/virtio/virtio_ring.h +++ b/drivers/net/virtio/virtio_ring.h@@ -78,6 +78,8 @@ struct vring_packed_desc_event { struct vring { unsigned int num; + unsigned int avail_wrap_counter; + unsigned int used_wrap_counter; union { struct vring_desc_packed *desc_packed; struct vring_desc *desc;@@ -92,6 +94,30 @@ struct vring { }; }; +static inline void +_set_desc_avail(struct vring_desc_packed *desc, int wrap_counter) +{ + desc->flags |= VRING_DESC_F_AVAIL(wrap_counter) | + VRING_DESC_F_USED(!wrap_counter);
It implies the avail and used bits to be cleared beforehand. Maybe it would be safer to clear them in the helper?
quoted hunk ↗ jump to hunk
+} + +static inline void +set_desc_avail(struct vring *vr, struct vring_desc_packed *desc) +{ + _set_desc_avail(desc, vr->avail_wrap_counter); +} + +static inline int +desc_is_used(struct vring_desc_packed *desc, struct vring *vr) +{ + uint16_t used, avail; + + used = !!(desc->flags & VRING_DESC_F_USED(1)); + avail = !!(desc->flags & VRING_DESC_F_AVAIL(1)); + + return used == avail && used == vr->used_wrap_counter; +} + /* The standard layout for the ring is a continuous chunk of memory which * looks like this. We assume num is a power of 2. *diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h index d2a0b651a..53fce61b4 100644 --- a/drivers/net/virtio/virtqueue.h +++ b/drivers/net/virtio/virtqueue.h@@ -245,6 +245,25 @@ struct virtio_tx_region { __attribute__((__aligned__(16))); }; +static inline uint16_t +increment_pq_index(uint16_t idx, size_t ring_size) +{ + return ++idx >= ring_size ? 0 : idx; +}
Not sure this helper is really useful, as it ends up doing two checks in a row.
+
+static inline uint16_t
+update_pq_avail_index(struct virtqueue *vq)
+{
+ uint16_t idx;
+
+ idx = increment_pq_index(vq->vq_avail_idx, vq->vq_nentries);
+ if (idx == 0)
+ vq->vq_ring.avail_wrap_counter ^= 1;+ vq->vq_avail_idx = idx; + + return vq->vq_avail_idx; +}
So it could be simplified to:
{
if (++vq->vq_avail_idx >= vq->vq_entries) {
vq->vq_avail_idx = 0;
vq->vq_ring.avail_wrap_counter ^= 1;
}
return vq->vq_avail_idx;
}
+
static inline void
vring_desc_init_packed(struct vring *vr, int n)
{