* [Qemu-devel] [[RFC v3 01/12] virtio: introduce packed ring definitions
2018-10-11 14:08 [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support wexu
@ 2018-10-11 14:08 ` wexu
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 02/12] virtio: redefine structure & memory cache for packed ring wexu
` (11 subsequent siblings)
12 siblings, 0 replies; 36+ messages in thread
From: wexu @ 2018-10-11 14:08 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, tiwei.bie, wexu, jfreimann, maxime.coquelin
From: Wei Xu <wexu@redhat.com>
sync from 1.1 spec
Signed-off-by: Wei Xu <wexu@redhat.com>
---
include/standard-headers/linux/virtio_config.h | 15 +++++++++
include/standard-headers/linux/virtio_ring.h | 43 ++++++++++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/include/standard-headers/linux/virtio_config.h b/include/standard-headers/linux/virtio_config.h
index 0b19436..9f450fd 100644
--- a/include/standard-headers/linux/virtio_config.h
+++ b/include/standard-headers/linux/virtio_config.h
@@ -75,6 +75,21 @@
*/
#define VIRTIO_F_IOMMU_PLATFORM 33
+/* This feature indicates support for the packed virtqueue layout. */
+#define VIRTIO_F_RING_PACKED 34
+
+/* Enable events */
+#define RING_EVENT_FLAGS_ENABLE 0x0
+/* Disable events */
+#define RING_EVENT_FLAGS_DISABLE 0x1
+/*
+ * * Enable events for a specific descriptor
+ * * (as specified by Descriptor Ring Change Event Offset/Wrap Counter).
+ * * Only valid if VIRTIO_F_RING_EVENT_IDX has been negotiated.
+ * */
+#define RING_EVENT_FLAGS_DESC 0x2
+/* The value 0x3 is reserved */
+
/*
* Does the device support Single Root I/O Virtualization?
*/
diff --git a/include/standard-headers/linux/virtio_ring.h b/include/standard-headers/linux/virtio_ring.h
index d26e72b..1719c6f 100644
--- a/include/standard-headers/linux/virtio_ring.h
+++ b/include/standard-headers/linux/virtio_ring.h
@@ -42,6 +42,10 @@
/* This means the buffer contains a list of buffer descriptors. */
#define VRING_DESC_F_INDIRECT 4
+/* Mark a descriptor as available or used. */
+#define VRING_DESC_F_AVAIL (1ul << 7)
+#define VRING_DESC_F_USED (1ul << 15)
+
/* The Host uses this in used->flags to advise the Guest: don't kick me when
* you add a buffer. It's unreliable, so it's simply an optimization. Guest
* will still kick if it's out of buffers. */
@@ -51,6 +55,17 @@
* optimization. */
#define VRING_AVAIL_F_NO_INTERRUPT 1
+/* Enable events. */
+#define VRING_EVENT_F_ENABLE 0x0
+/* Disable events. */
+#define VRING_EVENT_F_DISABLE 0x1
+/*
+ * Enable events for a specific descriptor
+ * (as specified by Descriptor Ring Change Event Offset/Wrap Counter).
+ * Only valid if VIRTIO_RING_F_EVENT_IDX has been negotiated.
+ */
+#define VRING_EVENT_F_DESC 0x2
+
/* We support indirect buffer descriptors */
#define VIRTIO_RING_F_INDIRECT_DESC 28
@@ -169,4 +184,32 @@ static inline int vring_need_event(uint16_t event_idx, uint16_t new_idx, uint16_
return (uint16_t)(new_idx - event_idx - 1) < (uint16_t)(new_idx - old);
}
+struct vring_packed_desc_event {
+ /* Descriptor Ring Change Event Offset/Wrap Counter. */
+ __virtio16 off_wrap;
+ /* Descriptor Ring Change Event Flags. */
+ __virtio16 flags;
+};
+
+struct vring_packed_desc {
+ /* Buffer Address. */
+ __virtio64 addr;
+ /* Buffer Length. */
+ __virtio32 len;
+ /* Buffer ID. */
+ __virtio16 id;
+ /* The flags depending on descriptor type. */
+ __virtio16 flags;
+};
+
+struct vring_packed {
+ unsigned int num;
+
+ struct vring_packed_desc *desc;
+
+ struct vring_packed_desc_event *driver;
+
+ struct vring_packed_desc_event *device;
+};
+
#endif /* _LINUX_VIRTIO_RING_H */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [[RFC v3 02/12] virtio: redefine structure & memory cache for packed ring
2018-10-11 14:08 [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support wexu
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 01/12] virtio: introduce packed ring definitions wexu
@ 2018-10-11 14:08 ` wexu
2018-10-15 3:03 ` Jason Wang
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 03/12] virtio: init " wexu
` (10 subsequent siblings)
12 siblings, 1 reply; 36+ messages in thread
From: wexu @ 2018-10-11 14:08 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, tiwei.bie, wexu, jfreimann, maxime.coquelin
From: Wei Xu <wexu@redhat.com>
Redefine packed ring structure according to qemu nomenclature,
also supported data(event index, wrap counter, etc) are introduced.
Signed-off-by: Wei Xu <wexu@redhat.com>
---
hw/virtio/virtio.c | 26 ++++++++++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 94f5c8e..500eecf 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -39,6 +39,13 @@ typedef struct VRingDesc
uint16_t next;
} VRingDesc;
+typedef struct VRingPackedDesc {
+ uint64_t addr;
+ uint32_t len;
+ uint16_t id;
+ uint16_t flags;
+} VRingPackedDesc;
+
typedef struct VRingAvail
{
uint16_t flags;
@@ -62,8 +69,14 @@ typedef struct VRingUsed
typedef struct VRingMemoryRegionCaches {
struct rcu_head rcu;
MemoryRegionCache desc;
- MemoryRegionCache avail;
- MemoryRegionCache used;
+ union {
+ MemoryRegionCache avail;
+ MemoryRegionCache driver;
+ };
+ union {
+ MemoryRegionCache used;
+ MemoryRegionCache device;
+ };
} VRingMemoryRegionCaches;
typedef struct VRing
@@ -77,6 +90,11 @@ typedef struct VRing
VRingMemoryRegionCaches *caches;
} VRing;
+typedef struct VRingPackedDescEvent {
+ uint16_t off_wrap;
+ uint16_t flags;
+} VRingPackedDescEvent ;
+
struct VirtQueue
{
VRing vring;
@@ -87,6 +105,10 @@ struct VirtQueue
/* Last avail_idx read from VQ. */
uint16_t shadow_avail_idx;
+ uint16_t event_idx;
+ bool event_wrap_counter;
+ bool avail_wrap_counter;
+
uint16_t used_idx;
/* Last used index value we have signalled on */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 02/12] virtio: redefine structure & memory cache for packed ring
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 02/12] virtio: redefine structure & memory cache for packed ring wexu
@ 2018-10-15 3:03 ` Jason Wang
2018-10-15 7:26 ` Wei Xu
0 siblings, 1 reply; 36+ messages in thread
From: Jason Wang @ 2018-10-15 3:03 UTC (permalink / raw)
To: wexu, qemu-devel; +Cc: tiwei.bie, jfreimann, maxime.coquelin
On 2018年10月11日 22:08, wexu@redhat.com wrote:
> From: Wei Xu <wexu@redhat.com>
>
> Redefine packed ring structure according to qemu nomenclature,
> also supported data(event index, wrap counter, etc) are introduced.
>
> Signed-off-by: Wei Xu <wexu@redhat.com>
> ---
> hw/virtio/virtio.c | 26 ++++++++++++++++++++++++--
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 94f5c8e..500eecf 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -39,6 +39,13 @@ typedef struct VRingDesc
> uint16_t next;
> } VRingDesc;
>
> +typedef struct VRingPackedDesc {
> + uint64_t addr;
> + uint32_t len;
> + uint16_t id;
> + uint16_t flags;
> +} VRingPackedDesc;
> +
> typedef struct VRingAvail
> {
> uint16_t flags;
> @@ -62,8 +69,14 @@ typedef struct VRingUsed
> typedef struct VRingMemoryRegionCaches {
> struct rcu_head rcu;
> MemoryRegionCache desc;
> - MemoryRegionCache avail;
> - MemoryRegionCache used;
> + union {
> + MemoryRegionCache avail;
> + MemoryRegionCache driver;
> + };
Can we reuse avail and used?
> + union {
> + MemoryRegionCache used;
> + MemoryRegionCache device;
> + };
> } VRingMemoryRegionCaches;
>
> typedef struct VRing
> @@ -77,6 +90,11 @@ typedef struct VRing
> VRingMemoryRegionCaches *caches;
> } VRing;
>
> +typedef struct VRingPackedDescEvent {
> + uint16_t off_wrap;
> + uint16_t flags;
> +} VRingPackedDescEvent ;
> +
> struct VirtQueue
> {
> VRing vring;
> @@ -87,6 +105,10 @@ struct VirtQueue
> /* Last avail_idx read from VQ. */
> uint16_t shadow_avail_idx;
>
> + uint16_t event_idx;
Need a comment to explain this field.
Thanks
> + bool event_wrap_counter;
> + bool avail_wrap_counter;
> +
> uint16_t used_idx;
>
> /* Last used index value we have signalled on */
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 02/12] virtio: redefine structure & memory cache for packed ring
2018-10-15 3:03 ` Jason Wang
@ 2018-10-15 7:26 ` Wei Xu
2018-10-15 8:03 ` Jason Wang
0 siblings, 1 reply; 36+ messages in thread
From: Wei Xu @ 2018-10-15 7:26 UTC (permalink / raw)
To: Jason Wang; +Cc: qemu-devel, maxime.coquelin, jfreimann, tiwei.bie
On Mon, Oct 15, 2018 at 11:03:52AM +0800, Jason Wang wrote:
>
>
> On 2018年10月11日 22:08, wexu@redhat.com wrote:
> >From: Wei Xu <wexu@redhat.com>
> >
> >Redefine packed ring structure according to qemu nomenclature,
> >also supported data(event index, wrap counter, etc) are introduced.
> >
> >Signed-off-by: Wei Xu <wexu@redhat.com>
> >---
> > hw/virtio/virtio.c | 26 ++++++++++++++++++++++++--
> > 1 file changed, 24 insertions(+), 2 deletions(-)
> >
> >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >index 94f5c8e..500eecf 100644
> >--- a/hw/virtio/virtio.c
> >+++ b/hw/virtio/virtio.c
> >@@ -39,6 +39,13 @@ typedef struct VRingDesc
> > uint16_t next;
> > } VRingDesc;
> >+typedef struct VRingPackedDesc {
> >+ uint64_t addr;
> >+ uint32_t len;
> >+ uint16_t id;
> >+ uint16_t flags;
> >+} VRingPackedDesc;
> >+
> > typedef struct VRingAvail
> > {
> > uint16_t flags;
> >@@ -62,8 +69,14 @@ typedef struct VRingUsed
> > typedef struct VRingMemoryRegionCaches {
> > struct rcu_head rcu;
> > MemoryRegionCache desc;
> >- MemoryRegionCache avail;
> >- MemoryRegionCache used;
> >+ union {
> >+ MemoryRegionCache avail;
> >+ MemoryRegionCache driver;
> >+ };
>
> Can we reuse avail and used?
Sure.
>
> >+ union {
> >+ MemoryRegionCache used;
> >+ MemoryRegionCache device;
> >+ };
> > } VRingMemoryRegionCaches;
> > typedef struct VRing
> >@@ -77,6 +90,11 @@ typedef struct VRing
> > VRingMemoryRegionCaches *caches;
> > } VRing;
> >+typedef struct VRingPackedDescEvent {
> >+ uint16_t off_wrap;
> >+ uint16_t flags;
> >+} VRingPackedDescEvent ;
> >+
> > struct VirtQueue
> > {
> > VRing vring;
> >@@ -87,6 +105,10 @@ struct VirtQueue
> > /* Last avail_idx read from VQ. */
> > uint16_t shadow_avail_idx;
> >+ uint16_t event_idx;
>
> Need a comment to explain this field.
Yes, it is the unified name for interrupt which is what I want to see
if we can reuse 'shadow' and 'used' index in current code, for Tx
queue, it should be the 'used' index after finished sending the last
desc. For Rx queue, it should be the 'shadow' index when no enough
descriptors which might be a few descriptors ahead of the 'used' index,
there are a few indexes already so this makes code a bit redundant.
Will see if I can remove this in next version, any comments?
Wei
>
> Thanks
>
> >+ bool event_wrap_counter;
> >+ bool avail_wrap_counter;
> >+
> > uint16_t used_idx;
> > /* Last used index value we have signalled on */
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 02/12] virtio: redefine structure & memory cache for packed ring
2018-10-15 7:26 ` Wei Xu
@ 2018-10-15 8:03 ` Jason Wang
0 siblings, 0 replies; 36+ messages in thread
From: Jason Wang @ 2018-10-15 8:03 UTC (permalink / raw)
To: Wei Xu; +Cc: qemu-devel, maxime.coquelin, jfreimann, tiwei.bie
On 2018年10月15日 15:26, Wei Xu wrote:
> On Mon, Oct 15, 2018 at 11:03:52AM +0800, Jason Wang wrote:
>>
>> On 2018年10月11日 22:08, wexu@redhat.com wrote:
>>> From: Wei Xu <wexu@redhat.com>
>>>
>>> Redefine packed ring structure according to qemu nomenclature,
>>> also supported data(event index, wrap counter, etc) are introduced.
>>>
>>> Signed-off-by: Wei Xu <wexu@redhat.com>
>>> ---
>>> hw/virtio/virtio.c | 26 ++++++++++++++++++++++++--
>>> 1 file changed, 24 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>>> index 94f5c8e..500eecf 100644
>>> --- a/hw/virtio/virtio.c
>>> +++ b/hw/virtio/virtio.c
>>> @@ -39,6 +39,13 @@ typedef struct VRingDesc
>>> uint16_t next;
>>> } VRingDesc;
>>> +typedef struct VRingPackedDesc {
>>> + uint64_t addr;
>>> + uint32_t len;
>>> + uint16_t id;
>>> + uint16_t flags;
>>> +} VRingPackedDesc;
>>> +
>>> typedef struct VRingAvail
>>> {
>>> uint16_t flags;
>>> @@ -62,8 +69,14 @@ typedef struct VRingUsed
>>> typedef struct VRingMemoryRegionCaches {
>>> struct rcu_head rcu;
>>> MemoryRegionCache desc;
>>> - MemoryRegionCache avail;
>>> - MemoryRegionCache used;
>>> + union {
>>> + MemoryRegionCache avail;
>>> + MemoryRegionCache driver;
>>> + };
>> Can we reuse avail and used?
> Sure.
>
>>> + union {
>>> + MemoryRegionCache used;
>>> + MemoryRegionCache device;
>>> + };
>>> } VRingMemoryRegionCaches;
>>> typedef struct VRing
>>> @@ -77,6 +90,11 @@ typedef struct VRing
>>> VRingMemoryRegionCaches *caches;
>>> } VRing;
>>> +typedef struct VRingPackedDescEvent {
>>> + uint16_t off_wrap;
>>> + uint16_t flags;
>>> +} VRingPackedDescEvent ;
>>> +
>>> struct VirtQueue
>>> {
>>> VRing vring;
>>> @@ -87,6 +105,10 @@ struct VirtQueue
>>> /* Last avail_idx read from VQ. */
>>> uint16_t shadow_avail_idx;
>>> + uint16_t event_idx;
>> Need a comment to explain this field.
> Yes, it is the unified name for interrupt which is what I want to see
> if we can reuse 'shadow' and 'used' index in current code, for Tx
> queue, it should be the 'used' index after finished sending the last
> desc. For Rx queue, it should be the 'shadow' index when no enough
> descriptors which might be a few descriptors ahead of the 'used' index,
> there are a few indexes already so this makes code a bit redundant.
If I understand your meaning correctly, you can refer vhost codes:
https://github.com/jasowang/net/commit/9b7b0d75d88d980a3c8954db0aa754b124facd0e
It maintains both last_wrap_counter and avail_wrap_counter/avail_idx.
avail_wrap_counter/avail_idx is guaranteed to be increased
monotonically, this is useful when we need to move avail index backwards.
Besides this, for qemu, I think you need a better name e.g
last_avail_idx here.
Thanks
>
> Will see if I can remove this in next version, any comments?
>
> Wei
>
>
>> Thanks
>>
>>> + bool event_wrap_counter;
>>> + bool avail_wrap_counter;
>>> +
>>> uint16_t used_idx;
>>> /* Last used index value we have signalled on */
>>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [[RFC v3 03/12] virtio: init memory cache for packed ring
2018-10-11 14:08 [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support wexu
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 01/12] virtio: introduce packed ring definitions wexu
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 02/12] virtio: redefine structure & memory cache for packed ring wexu
@ 2018-10-11 14:08 ` wexu
2018-10-15 3:10 ` Jason Wang
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 04/12] virtio: init wrap counter " wexu
` (9 subsequent siblings)
12 siblings, 1 reply; 36+ messages in thread
From: wexu @ 2018-10-11 14:08 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, tiwei.bie, wexu, jfreimann, maxime.coquelin
From: Wei Xu <wexu@redhat.com>
Expand 1.0 by adding offset calculation accordingly.
Signed-off-by: Wei Xu <wexu@redhat.com>
---
hw/virtio/vhost.c | 16 ++++++++--------
hw/virtio/virtio.c | 35 +++++++++++++++++++++++------------
include/hw/virtio/virtio.h | 4 ++--
3 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 569c405..9df2da3 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -996,14 +996,14 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
r = -ENOMEM;
goto fail_alloc_desc;
}
- vq->avail_size = s = l = virtio_queue_get_avail_size(vdev, idx);
+ vq->avail_size = s = l = virtio_queue_get_driver_size(vdev, idx);
vq->avail_phys = a = virtio_queue_get_avail_addr(vdev, idx);
vq->avail = vhost_memory_map(dev, a, &l, 0);
if (!vq->avail || l != s) {
r = -ENOMEM;
goto fail_alloc_avail;
}
- vq->used_size = s = l = virtio_queue_get_used_size(vdev, idx);
+ vq->used_size = s = l = virtio_queue_get_device_size(vdev, idx);
vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
vq->used = vhost_memory_map(dev, a, &l, 1);
if (!vq->used || l != s) {
@@ -1051,10 +1051,10 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
fail_vector:
fail_kick:
fail_alloc:
- vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
+ vhost_memory_unmap(dev, vq->used, virtio_queue_get_device_size(vdev, idx),
0, 0);
fail_alloc_used:
- vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
+ vhost_memory_unmap(dev, vq->avail, virtio_queue_get_driver_size(vdev, idx),
0, 0);
fail_alloc_avail:
vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
@@ -1101,10 +1101,10 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
vhost_vq_index);
}
- vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
- 1, virtio_queue_get_used_size(vdev, idx));
- vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
- 0, virtio_queue_get_avail_size(vdev, idx));
+ vhost_memory_unmap(dev, vq->used, virtio_queue_get_device_size(vdev, idx),
+ 1, virtio_queue_get_device_size(vdev, idx));
+ vhost_memory_unmap(dev, vq->avail, virtio_queue_get_driver_size(vdev, idx),
+ 0, virtio_queue_get_driver_size(vdev, idx));
vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
0, virtio_queue_get_desc_size(vdev, idx));
}
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 500eecf..bfb3364 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -162,11 +162,8 @@ static void virtio_init_region_cache(VirtIODevice *vdev, int n)
VRingMemoryRegionCaches *old = vq->vring.caches;
VRingMemoryRegionCaches *new = NULL;
hwaddr addr, size;
- int event_size;
int64_t len;
- event_size = virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
-
addr = vq->vring.desc;
if (!addr) {
goto out_no_cache;
@@ -174,13 +171,13 @@ static void virtio_init_region_cache(VirtIODevice *vdev, int n)
new = g_new0(VRingMemoryRegionCaches, 1);
size = virtio_queue_get_desc_size(vdev, n);
len = address_space_cache_init(&new->desc, vdev->dma_as,
- addr, size, false);
+ addr, size, true);
if (len < size) {
virtio_error(vdev, "Cannot map desc");
goto err_desc;
}
- size = virtio_queue_get_used_size(vdev, n) + event_size;
+ size = virtio_queue_get_device_size(vdev, n);
len = address_space_cache_init(&new->used, vdev->dma_as,
vq->vring.used, size, true);
if (len < size) {
@@ -188,7 +185,7 @@ static void virtio_init_region_cache(VirtIODevice *vdev, int n)
goto err_used;
}
- size = virtio_queue_get_avail_size(vdev, n) + event_size;
+ size = virtio_queue_get_driver_size(vdev, n);
len = address_space_cache_init(&new->avail, vdev->dma_as,
vq->vring.avail, size, false);
if (len < size) {
@@ -2339,16 +2336,30 @@ hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n)
return sizeof(VRingDesc) * vdev->vq[n].vring.num;
}
-hwaddr virtio_queue_get_avail_size(VirtIODevice *vdev, int n)
+hwaddr virtio_queue_get_driver_size(VirtIODevice *vdev, int n)
{
- return offsetof(VRingAvail, ring) +
- sizeof(uint16_t) * vdev->vq[n].vring.num;
+ int s;
+
+ if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
+ return sizeof(struct VRingPackedDescEvent);
+ } else {
+ s = virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
+ return offsetof(VRingAvail, ring) +
+ sizeof(uint16_t) * vdev->vq[n].vring.num + s;
+ }
}
-hwaddr virtio_queue_get_used_size(VirtIODevice *vdev, int n)
+hwaddr virtio_queue_get_device_size(VirtIODevice *vdev, int n)
{
- return offsetof(VRingUsed, ring) +
- sizeof(VRingUsedElem) * vdev->vq[n].vring.num;
+ int s;
+
+ if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
+ return sizeof(struct VRingPackedDescEvent);
+ } else {
+ s = virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
+ return offsetof(VRingUsed, ring) +
+ sizeof(VRingUsedElem) * vdev->vq[n].vring.num + s;
+ }
}
uint16_t virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 9c1fa07..e323e76 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -270,8 +270,8 @@ hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n);
-hwaddr virtio_queue_get_avail_size(VirtIODevice *vdev, int n);
-hwaddr virtio_queue_get_used_size(VirtIODevice *vdev, int n);
+hwaddr virtio_queue_get_driver_size(VirtIODevice *vdev, int n);
+hwaddr virtio_queue_get_device_size(VirtIODevice *vdev, int n);
uint16_t virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n);
void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx);
void virtio_queue_restore_last_avail_idx(VirtIODevice *vdev, int n);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 03/12] virtio: init memory cache for packed ring
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 03/12] virtio: init " wexu
@ 2018-10-15 3:10 ` Jason Wang
2018-10-15 7:09 ` Wei Xu
0 siblings, 1 reply; 36+ messages in thread
From: Jason Wang @ 2018-10-15 3:10 UTC (permalink / raw)
To: wexu, qemu-devel; +Cc: tiwei.bie, jfreimann, maxime.coquelin
On 2018年10月11日 22:08, wexu@redhat.com wrote:
> From: Wei Xu <wexu@redhat.com>
>
> Expand 1.0 by adding offset calculation accordingly.
This is only part of what this patch did and I suggest to another patch
to do this.
>
> Signed-off-by: Wei Xu <wexu@redhat.com>
> ---
> hw/virtio/vhost.c | 16 ++++++++--------
> hw/virtio/virtio.c | 35 +++++++++++++++++++++++------------
> include/hw/virtio/virtio.h | 4 ++--
> 3 files changed, 33 insertions(+), 22 deletions(-)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 569c405..9df2da3 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -996,14 +996,14 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
> r = -ENOMEM;
> goto fail_alloc_desc;
> }
> - vq->avail_size = s = l = virtio_queue_get_avail_size(vdev, idx);
> + vq->avail_size = s = l = virtio_queue_get_driver_size(vdev, idx);
Let's try to use a more consistent name. E.g either use avail/used or
driver/device.
I prefer to use avail/used, it can save lots of unnecessary changes.
> vq->avail_phys = a = virtio_queue_get_avail_addr(vdev, idx);
> vq->avail = vhost_memory_map(dev, a, &l, 0);
> if (!vq->avail || l != s) {
> r = -ENOMEM;
> goto fail_alloc_avail;
> }
> - vq->used_size = s = l = virtio_queue_get_used_size(vdev, idx);
> + vq->used_size = s = l = virtio_queue_get_device_size(vdev, idx);
> vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
> vq->used = vhost_memory_map(dev, a, &l, 1);
> if (!vq->used || l != s) {
> @@ -1051,10 +1051,10 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
> fail_vector:
> fail_kick:
> fail_alloc:
> - vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
> + vhost_memory_unmap(dev, vq->used, virtio_queue_get_device_size(vdev, idx),
> 0, 0);
> fail_alloc_used:
> - vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
> + vhost_memory_unmap(dev, vq->avail, virtio_queue_get_driver_size(vdev, idx),
> 0, 0);
> fail_alloc_avail:
> vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
> @@ -1101,10 +1101,10 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
> vhost_vq_index);
> }
>
> - vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
> - 1, virtio_queue_get_used_size(vdev, idx));
> - vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
> - 0, virtio_queue_get_avail_size(vdev, idx));
> + vhost_memory_unmap(dev, vq->used, virtio_queue_get_device_size(vdev, idx),
> + 1, virtio_queue_get_device_size(vdev, idx));
> + vhost_memory_unmap(dev, vq->avail, virtio_queue_get_driver_size(vdev, idx),
> + 0, virtio_queue_get_driver_size(vdev, idx));
> vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
> 0, virtio_queue_get_desc_size(vdev, idx));
> }
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 500eecf..bfb3364 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -162,11 +162,8 @@ static void virtio_init_region_cache(VirtIODevice *vdev, int n)
> VRingMemoryRegionCaches *old = vq->vring.caches;
> VRingMemoryRegionCaches *new = NULL;
> hwaddr addr, size;
> - int event_size;
> int64_t len;
>
> - event_size = virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
> -
> addr = vq->vring.desc;
> if (!addr) {
> goto out_no_cache;
> @@ -174,13 +171,13 @@ static void virtio_init_region_cache(VirtIODevice *vdev, int n)
> new = g_new0(VRingMemoryRegionCaches, 1);
> size = virtio_queue_get_desc_size(vdev, n);
> len = address_space_cache_init(&new->desc, vdev->dma_as,
> - addr, size, false);
> + addr, size, true);
This looks wrong, for split virtqueue, descriptor ring is read only.
> if (len < size) {
> virtio_error(vdev, "Cannot map desc");
> goto err_desc;
> }
>
> - size = virtio_queue_get_used_size(vdev, n) + event_size;
> + size = virtio_queue_get_device_size(vdev, n);
> len = address_space_cache_init(&new->used, vdev->dma_as,
> vq->vring.used, size, true);
> if (len < size) {
> @@ -188,7 +185,7 @@ static void virtio_init_region_cache(VirtIODevice *vdev, int n)
> goto err_used;
> }
>
> - size = virtio_queue_get_avail_size(vdev, n) + event_size;
> + size = virtio_queue_get_driver_size(vdev, n);
> len = address_space_cache_init(&new->avail, vdev->dma_as,
> vq->vring.avail, size, false);
> if (len < size) {
> @@ -2339,16 +2336,30 @@ hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n)
> return sizeof(VRingDesc) * vdev->vq[n].vring.num;
> }
>
> -hwaddr virtio_queue_get_avail_size(VirtIODevice *vdev, int n)
> +hwaddr virtio_queue_get_driver_size(VirtIODevice *vdev, int n)
> {
> - return offsetof(VRingAvail, ring) +
> - sizeof(uint16_t) * vdev->vq[n].vring.num;
> + int s;
> +
> + if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
> + return sizeof(struct VRingPackedDescEvent);
> + } else {
> + s = virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
> + return offsetof(VRingAvail, ring) +
> + sizeof(uint16_t) * vdev->vq[n].vring.num + s;
I tend to move this to an independent patch.
Thanks
> + }
> }
>
> -hwaddr virtio_queue_get_used_size(VirtIODevice *vdev, int n)
> +hwaddr virtio_queue_get_device_size(VirtIODevice *vdev, int n)
> {
> - return offsetof(VRingUsed, ring) +
> - sizeof(VRingUsedElem) * vdev->vq[n].vring.num;
> + int s;
> +
> + if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
> + return sizeof(struct VRingPackedDescEvent);
> + } else {
> + s = virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
> + return offsetof(VRingUsed, ring) +
> + sizeof(VRingUsedElem) * vdev->vq[n].vring.num + s;
> + }
> }
>
> uint16_t virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n)
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 9c1fa07..e323e76 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -270,8 +270,8 @@ hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
> hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n);
> -hwaddr virtio_queue_get_avail_size(VirtIODevice *vdev, int n);
> -hwaddr virtio_queue_get_used_size(VirtIODevice *vdev, int n);
> +hwaddr virtio_queue_get_driver_size(VirtIODevice *vdev, int n);
> +hwaddr virtio_queue_get_device_size(VirtIODevice *vdev, int n);
> uint16_t virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n);
> void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx);
> void virtio_queue_restore_last_avail_idx(VirtIODevice *vdev, int n);
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 03/12] virtio: init memory cache for packed ring
2018-10-15 3:10 ` Jason Wang
@ 2018-10-15 7:09 ` Wei Xu
2018-10-15 7:54 ` Jason Wang
0 siblings, 1 reply; 36+ messages in thread
From: Wei Xu @ 2018-10-15 7:09 UTC (permalink / raw)
To: Jason Wang; +Cc: qemu-devel, maxime.coquelin, jfreimann, tiwei.bie
On Mon, Oct 15, 2018 at 11:10:12AM +0800, Jason Wang wrote:
>
>
> On 2018年10月11日 22:08, wexu@redhat.com wrote:
> >From: Wei Xu <wexu@redhat.com>
> >
> >Expand 1.0 by adding offset calculation accordingly.
>
> This is only part of what this patch did and I suggest to another patch to
> do this.
>
> >
> >Signed-off-by: Wei Xu <wexu@redhat.com>
> >---
> > hw/virtio/vhost.c | 16 ++++++++--------
> > hw/virtio/virtio.c | 35 +++++++++++++++++++++++------------
> > include/hw/virtio/virtio.h | 4 ++--
> > 3 files changed, 33 insertions(+), 22 deletions(-)
> >
> >diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> >index 569c405..9df2da3 100644
> >--- a/hw/virtio/vhost.c
> >+++ b/hw/virtio/vhost.c
> >@@ -996,14 +996,14 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
> > r = -ENOMEM;
> > goto fail_alloc_desc;
> > }
> >- vq->avail_size = s = l = virtio_queue_get_avail_size(vdev, idx);
> >+ vq->avail_size = s = l = virtio_queue_get_driver_size(vdev, idx);
>
> Let's try to use a more consistent name. E.g either use avail/used or
> driver/device.
>
> I prefer to use avail/used, it can save lots of unnecessary changes.
OK.
>
> > vq->avail_phys = a = virtio_queue_get_avail_addr(vdev, idx);
> > vq->avail = vhost_memory_map(dev, a, &l, 0);
> > if (!vq->avail || l != s) {
> > r = -ENOMEM;
> > goto fail_alloc_avail;
> > }
> >- vq->used_size = s = l = virtio_queue_get_used_size(vdev, idx);
> >+ vq->used_size = s = l = virtio_queue_get_device_size(vdev, idx);
> > vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
> > vq->used = vhost_memory_map(dev, a, &l, 1);
> > if (!vq->used || l != s) {
> >@@ -1051,10 +1051,10 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
> > fail_vector:
> > fail_kick:
> > fail_alloc:
> >- vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
> >+ vhost_memory_unmap(dev, vq->used, virtio_queue_get_device_size(vdev, idx),
> > 0, 0);
> > fail_alloc_used:
> >- vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
> >+ vhost_memory_unmap(dev, vq->avail, virtio_queue_get_driver_size(vdev, idx),
> > 0, 0);
> > fail_alloc_avail:
> > vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
> >@@ -1101,10 +1101,10 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
> > vhost_vq_index);
> > }
> >- vhost_memory_unmap(dev, vq->used, virtio_queue_get_used_size(vdev, idx),
> >- 1, virtio_queue_get_used_size(vdev, idx));
> >- vhost_memory_unmap(dev, vq->avail, virtio_queue_get_avail_size(vdev, idx),
> >- 0, virtio_queue_get_avail_size(vdev, idx));
> >+ vhost_memory_unmap(dev, vq->used, virtio_queue_get_device_size(vdev, idx),
> >+ 1, virtio_queue_get_device_size(vdev, idx));
> >+ vhost_memory_unmap(dev, vq->avail, virtio_queue_get_driver_size(vdev, idx),
> >+ 0, virtio_queue_get_driver_size(vdev, idx));
> > vhost_memory_unmap(dev, vq->desc, virtio_queue_get_desc_size(vdev, idx),
> > 0, virtio_queue_get_desc_size(vdev, idx));
> > }
> >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >index 500eecf..bfb3364 100644
> >--- a/hw/virtio/virtio.c
> >+++ b/hw/virtio/virtio.c
> >@@ -162,11 +162,8 @@ static void virtio_init_region_cache(VirtIODevice *vdev, int n)
> > VRingMemoryRegionCaches *old = vq->vring.caches;
> > VRingMemoryRegionCaches *new = NULL;
> > hwaddr addr, size;
> >- int event_size;
> > int64_t len;
> >- event_size = virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
> >-
> > addr = vq->vring.desc;
> > if (!addr) {
> > goto out_no_cache;
> >@@ -174,13 +171,13 @@ static void virtio_init_region_cache(VirtIODevice *vdev, int n)
> > new = g_new0(VRingMemoryRegionCaches, 1);
> > size = virtio_queue_get_desc_size(vdev, n);
> > len = address_space_cache_init(&new->desc, vdev->dma_as,
> >- addr, size, false);
> >+ addr, size, true);
>
> This looks wrong, for split virtqueue, descriptor ring is read only.
Did know it, I got a segment fault with 'false' case for packed ring,
will add a feature check here, thanks for the tip.
>
> > if (len < size) {
> > virtio_error(vdev, "Cannot map desc");
> > goto err_desc;
> > }
> >- size = virtio_queue_get_used_size(vdev, n) + event_size;
> >+ size = virtio_queue_get_device_size(vdev, n);
> > len = address_space_cache_init(&new->used, vdev->dma_as,
> > vq->vring.used, size, true);
> > if (len < size) {
> >@@ -188,7 +185,7 @@ static void virtio_init_region_cache(VirtIODevice *vdev, int n)
> > goto err_used;
> > }
> >- size = virtio_queue_get_avail_size(vdev, n) + event_size;
> >+ size = virtio_queue_get_driver_size(vdev, n);
> > len = address_space_cache_init(&new->avail, vdev->dma_as,
> > vq->vring.avail, size, false);
> > if (len < size) {
> >@@ -2339,16 +2336,30 @@ hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n)
> > return sizeof(VRingDesc) * vdev->vq[n].vring.num;
> > }
> >-hwaddr virtio_queue_get_avail_size(VirtIODevice *vdev, int n)
> >+hwaddr virtio_queue_get_driver_size(VirtIODevice *vdev, int n)
> > {
> >- return offsetof(VRingAvail, ring) +
> >- sizeof(uint16_t) * vdev->vq[n].vring.num;
> >+ int s;
> >+
> >+ if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
> >+ return sizeof(struct VRingPackedDescEvent);
> >+ } else {
> >+ s = virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
> >+ return offsetof(VRingAvail, ring) +
> >+ sizeof(uint16_t) * vdev->vq[n].vring.num + s;
>
> I tend to move this to an independent patch.
You mean this two functions?
Wei
>
> Thanks
>
> >+ }
> > }
> >-hwaddr virtio_queue_get_used_size(VirtIODevice *vdev, int n)
> >+hwaddr virtio_queue_get_device_size(VirtIODevice *vdev, int n)
> > {
> >- return offsetof(VRingUsed, ring) +
> >- sizeof(VRingUsedElem) * vdev->vq[n].vring.num;
> >+ int s;
> >+
> >+ if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
> >+ return sizeof(struct VRingPackedDescEvent);
> >+ } else {
> >+ s = virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
> >+ return offsetof(VRingUsed, ring) +
> >+ sizeof(VRingUsedElem) * vdev->vq[n].vring.num + s;
> >+ }
> > }
> > uint16_t virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n)
> >diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> >index 9c1fa07..e323e76 100644
> >--- a/include/hw/virtio/virtio.h
> >+++ b/include/hw/virtio/virtio.h
> >@@ -270,8 +270,8 @@ hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
> > hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
> > hwaddr virtio_queue_get_used_addr(VirtIODevice *vdev, int n);
> > hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n);
> >-hwaddr virtio_queue_get_avail_size(VirtIODevice *vdev, int n);
> >-hwaddr virtio_queue_get_used_size(VirtIODevice *vdev, int n);
> >+hwaddr virtio_queue_get_driver_size(VirtIODevice *vdev, int n);
> >+hwaddr virtio_queue_get_device_size(VirtIODevice *vdev, int n);
> > uint16_t virtio_queue_get_last_avail_idx(VirtIODevice *vdev, int n);
> > void virtio_queue_set_last_avail_idx(VirtIODevice *vdev, int n, uint16_t idx);
> > void virtio_queue_restore_last_avail_idx(VirtIODevice *vdev, int n);
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 03/12] virtio: init memory cache for packed ring
2018-10-15 7:09 ` Wei Xu
@ 2018-10-15 7:54 ` Jason Wang
0 siblings, 0 replies; 36+ messages in thread
From: Jason Wang @ 2018-10-15 7:54 UTC (permalink / raw)
To: Wei Xu; +Cc: maxime.coquelin, jfreimann, qemu-devel, tiwei.bie
On 2018年10月15日 15:09, Wei Xu wrote:
>>> -hwaddr virtio_queue_get_avail_size(VirtIODevice *vdev, int n)
>>> +hwaddr virtio_queue_get_driver_size(VirtIODevice *vdev, int n)
>>> {
>>> - return offsetof(VRingAvail, ring) +
>>> - sizeof(uint16_t) * vdev->vq[n].vring.num;
>>> + int s;
>>> +
>>> + if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
>>> + return sizeof(struct VRingPackedDescEvent);
>>> + } else {
>>> + s = virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
>>> + return offsetof(VRingAvail, ring) +
>>> + sizeof(uint16_t) * vdev->vq[n].vring.num + s;
>> I tend to move this to an independent patch.
> You mean this two functions?
>
> Wei
>
I mean moving the adding of event to get_avail()/get_used().
Thanks
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [[RFC v3 04/12] virtio: init wrap counter for packed ring
2018-10-11 14:08 [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support wexu
` (2 preceding siblings ...)
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 03/12] virtio: init " wexu
@ 2018-10-11 14:08 ` wexu
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 05/12] virtio: init and desc empty check " wexu
` (8 subsequent siblings)
12 siblings, 0 replies; 36+ messages in thread
From: wexu @ 2018-10-11 14:08 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, tiwei.bie, wexu, jfreimann, maxime.coquelin
From: Wei Xu <wexu@redhat.com>
Signed-off-by: Wei Xu <wexu@redhat.com>
---
hw/virtio/virtio.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index bfb3364..9185efb 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1243,6 +1243,9 @@ void virtio_reset(void *opaque)
vdev->vq[i].last_avail_idx = 0;
vdev->vq[i].shadow_avail_idx = 0;
vdev->vq[i].used_idx = 0;
+ vdev->vq[i].avail_wrap_counter = true;
+ vdev->vq[i].event_idx = 0;
+ vdev->vq[i].event_wrap_counter = true;
virtio_queue_set_vector(vdev, i, VIRTIO_NO_VECTOR);
vdev->vq[i].signalled_used = 0;
vdev->vq[i].signalled_used_valid = false;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [[RFC v3 05/12] virtio: init and desc empty check for packed ring
2018-10-11 14:08 [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support wexu
` (3 preceding siblings ...)
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 04/12] virtio: init wrap counter " wexu
@ 2018-10-11 14:08 ` wexu
2018-10-15 3:18 ` Jason Wang
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 06/12] virtio: get avail bytes " wexu
` (7 subsequent siblings)
12 siblings, 1 reply; 36+ messages in thread
From: wexu @ 2018-10-11 14:08 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, tiwei.bie, wexu, jfreimann, maxime.coquelin
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)
+{
+ 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)
{
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 05/12] virtio: init and desc empty check for packed ring
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 05/12] virtio: init and desc empty check " wexu
@ 2018-10-15 3:18 ` Jason Wang
2018-10-15 7:04 ` Wei Xu
0 siblings, 1 reply; 36+ messages in thread
From: Jason Wang @ 2018-10-15 3:18 UTC (permalink / raw)
To: wexu, qemu-devel; +Cc: tiwei.bie, jfreimann, maxime.coquelin
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)
> {
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 05/12] virtio: init and desc empty check for packed ring
2018-10-15 3:18 ` Jason Wang
@ 2018-10-15 7:04 ` Wei Xu
0 siblings, 0 replies; 36+ messages in thread
From: Wei Xu @ 2018-10-15 7:04 UTC (permalink / raw)
To: Jason Wang; +Cc: qemu-devel, tiwei.bie, jfreimann, maxime.coquelin
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)
> > {
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [[RFC v3 06/12] virtio: get avail bytes check for packed ring
2018-10-11 14:08 [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support wexu
` (4 preceding siblings ...)
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 05/12] virtio: init and desc empty check " wexu
@ 2018-10-11 14:08 ` wexu
2018-10-15 3:47 ` Jason Wang
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 07/12] virtio: fill/flush/pop " wexu
` (6 subsequent siblings)
12 siblings, 1 reply; 36+ messages in thread
From: wexu @ 2018-10-11 14:08 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, tiwei.bie, wexu, jfreimann, maxime.coquelin
From: Wei Xu <wexu@redhat.com>
Same thought as 1.0 except a bit confused when trying to reuse
'shadow_avail_idx', so the interrelated new event_idx and the wrap
counter for notifications has been introduced in previous patch.
Signed-off-by: Wei Xu <wexu@redhat.com>
---
hw/virtio/virtio.c | 176 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 173 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 86f88da..13c6c98 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -375,6 +375,17 @@ int virtio_queue_ready(VirtQueue *vq)
return vq->vring.avail != 0;
}
+static void vring_packed_desc_read(VirtIODevice *vdev, VRingPackedDesc *desc,
+ MemoryRegionCache *cache, int i)
+{
+ address_space_read_cached(cache, i * sizeof(VRingPackedDesc),
+ desc, sizeof(VRingPackedDesc));
+ virtio_tswap16s(vdev, &desc->flags);
+ virtio_tswap64s(vdev, &desc->addr);
+ virtio_tswap32s(vdev, &desc->len);
+ virtio_tswap16s(vdev, &desc->id);
+}
+
static void vring_packed_desc_read_flags(VirtIODevice *vdev,
VRingPackedDesc *desc, MemoryRegionCache *cache, int i)
{
@@ -672,9 +683,9 @@ static int virtqueue_read_next_desc(VirtIODevice *vdev, VRingDesc *desc,
return VIRTQUEUE_READ_DESC_MORE;
}
-void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
- unsigned int *out_bytes,
- unsigned max_in_bytes, unsigned max_out_bytes)
+static void virtqueue_split_get_avail_bytes(VirtQueue *vq,
+ unsigned int *in_bytes, unsigned int *out_bytes,
+ unsigned max_in_bytes, unsigned max_out_bytes)
{
VirtIODevice *vdev = vq->vdev;
unsigned int max, idx;
@@ -797,6 +808,165 @@ err:
goto done;
}
+static void virtqueue_packed_get_avail_bytes(VirtQueue *vq,
+ unsigned int *in_bytes, unsigned int *out_bytes,
+ unsigned max_in_bytes, unsigned max_out_bytes)
+{
+ VirtIODevice *vdev = vq->vdev;
+ unsigned int max, idx;
+ unsigned int total_bufs, in_total, out_total;
+ MemoryRegionCache *desc_cache;
+ VRingMemoryRegionCaches *caches;
+ MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
+ int64_t len = 0;
+ VRingPackedDesc desc;
+ bool wrap_counter;
+
+ if (unlikely(!vq->vring.desc)) {
+ if (in_bytes) {
+ *in_bytes = 0;
+ }
+ if (out_bytes) {
+ *out_bytes = 0;
+ }
+ return;
+ }
+
+ rcu_read_lock();
+ idx = vq->last_avail_idx;
+ wrap_counter = vq->avail_wrap_counter;
+ total_bufs = in_total = out_total = 0;
+
+ max = vq->vring.num;
+ caches = vring_get_region_caches(vq);
+ if (caches->desc.len < max * sizeof(VRingPackedDesc)) {
+ virtio_error(vdev, "Cannot map descriptor ring");
+ goto err;
+ }
+
+ desc_cache = &caches->desc;
+ vring_packed_desc_read(vdev, &desc, desc_cache, idx);
+ /* Make sure we see all the fields*/
+ smp_rmb();
+ while (is_desc_avail(&desc, wrap_counter)) {
+ unsigned int num_bufs;
+ unsigned int i = 0;
+
+ num_bufs = total_bufs;
+
+ if (desc.flags & VRING_DESC_F_INDIRECT) {
+ if (desc.len % sizeof(VRingPackedDesc)) {
+ virtio_error(vdev, "Invalid size for indirect buffer table");
+ goto err;
+ }
+
+ /* If we've got too many, that implies a descriptor loop. */
+ if (num_bufs >= max) {
+ virtio_error(vdev, "Looped descriptor");
+ goto err;
+ }
+
+ /* loop over the indirect descriptor table */
+ len = address_space_cache_init(&indirect_desc_cache,
+ vdev->dma_as,
+ desc.addr, desc.len, false);
+ desc_cache = &indirect_desc_cache;
+ if (len < desc.len) {
+ virtio_error(vdev, "Cannot map indirect buffer");
+ goto err;
+ }
+
+ max = desc.len / sizeof(VRingPackedDesc);
+ num_bufs = i = 0;
+ vring_packed_desc_read(vdev, &desc, desc_cache, i);
+ /* Make sure we see all the fields*/
+ smp_rmb();
+ }
+
+ do {
+ /* If we've got too many, that implies a descriptor loop. */
+ if (++num_bufs > max) {
+ virtio_error(vdev, "Looped descriptor");
+ goto err;
+ }
+
+ if (desc.flags & VRING_DESC_F_WRITE) {
+ in_total += desc.len;
+ } else {
+ out_total += desc.len;
+ }
+ if (in_total >= max_in_bytes && out_total >= max_out_bytes) {
+ goto done;
+ }
+
+ if (desc_cache == &indirect_desc_cache) {
+ if (++i > vq->vring.num) {
+ virtio_error(vdev, "Looped descriptor");
+ goto err;
+ }
+ vring_packed_desc_read(vdev, &desc, desc_cache, i);
+ } else {
+ if (++idx >= vq->vring.num) {
+ idx -= vq->vring.num;
+ wrap_counter = !wrap_counter;
+ }
+ vring_packed_desc_read(vdev, &desc, desc_cache, idx);
+ }
+ /* Make sure we see the flags */
+ smp_rmb();
+ } while (desc.flags & VRING_DESC_F_NEXT);
+
+ if (desc_cache == &indirect_desc_cache) {
+ address_space_cache_destroy(&indirect_desc_cache);
+ total_bufs++;
+ /* We missed one step on for indirect desc */
+ idx++;
+ if (++idx >= vq->vring.num) {
+ idx -= vq->vring.num;
+ wrap_counter = !wrap_counter;
+ }
+ } else {
+ total_bufs = num_bufs;
+ }
+
+ desc_cache = &caches->desc;
+ vring_packed_desc_read(vdev, &desc, desc_cache, idx);
+ /* Make sure we see all the fields */
+ smp_rmb();
+ }
+
+ /* Set up index and wrap counter for an interrupt when no enough desc */
+ vq->event_idx = idx;
+ vq->event_wrap_counter = wrap_counter;
+done:
+ address_space_cache_destroy(&indirect_desc_cache);
+ if (in_bytes) {
+ *in_bytes = in_total;
+ }
+ if (out_bytes) {
+ *out_bytes = out_total;
+ }
+ rcu_read_unlock();
+ return;
+
+err:
+ in_total = out_total = 0;
+ goto done;
+}
+
+void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
+ unsigned int *out_bytes,
+ unsigned max_in_bytes, unsigned max_out_bytes)
+{
+ if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
+ virtqueue_packed_get_avail_bytes(vq, in_bytes, out_bytes,
+ max_in_bytes, max_out_bytes);
+ } else {
+ virtqueue_split_get_avail_bytes(vq, in_bytes, out_bytes,
+ max_in_bytes, max_out_bytes);
+ }
+}
+
int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes,
unsigned int out_bytes)
{
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 06/12] virtio: get avail bytes check for packed ring
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 06/12] virtio: get avail bytes " wexu
@ 2018-10-15 3:47 ` Jason Wang
0 siblings, 0 replies; 36+ messages in thread
From: Jason Wang @ 2018-10-15 3:47 UTC (permalink / raw)
To: wexu, qemu-devel; +Cc: tiwei.bie, jfreimann, maxime.coquelin
On 2018年10月11日 22:08, wexu@redhat.com wrote:
> From: Wei Xu <wexu@redhat.com>
>
> Same thought as 1.0 except a bit confused when trying to reuse
> 'shadow_avail_idx', so the interrelated new event_idx and the wrap
> counter for notifications has been introduced in previous patch.
>
> Signed-off-by: Wei Xu <wexu@redhat.com>
> ---
> hw/virtio/virtio.c | 176 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 173 insertions(+), 3 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 86f88da..13c6c98 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -375,6 +375,17 @@ int virtio_queue_ready(VirtQueue *vq)
> return vq->vring.avail != 0;
> }
>
> +static void vring_packed_desc_read(VirtIODevice *vdev, VRingPackedDesc *desc,
> + MemoryRegionCache *cache, int i)
> +{
> + address_space_read_cached(cache, i * sizeof(VRingPackedDesc),
> + desc, sizeof(VRingPackedDesc));
> + virtio_tswap16s(vdev, &desc->flags);
> + virtio_tswap64s(vdev, &desc->addr);
> + virtio_tswap32s(vdev, &desc->len);
> + virtio_tswap16s(vdev, &desc->id);
> +}
> +
> static void vring_packed_desc_read_flags(VirtIODevice *vdev,
> VRingPackedDesc *desc, MemoryRegionCache *cache, int i)
> {
> @@ -672,9 +683,9 @@ static int virtqueue_read_next_desc(VirtIODevice *vdev, VRingDesc *desc,
> return VIRTQUEUE_READ_DESC_MORE;
> }
>
> -void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
> - unsigned int *out_bytes,
> - unsigned max_in_bytes, unsigned max_out_bytes)
> +static void virtqueue_split_get_avail_bytes(VirtQueue *vq,
> + unsigned int *in_bytes, unsigned int *out_bytes,
> + unsigned max_in_bytes, unsigned max_out_bytes)
> {
> VirtIODevice *vdev = vq->vdev;
> unsigned int max, idx;
> @@ -797,6 +808,165 @@ err:
> goto done;
> }
>
> +static void virtqueue_packed_get_avail_bytes(VirtQueue *vq,
> + unsigned int *in_bytes, unsigned int *out_bytes,
> + unsigned max_in_bytes, unsigned max_out_bytes)
> +{
> + VirtIODevice *vdev = vq->vdev;
> + unsigned int max, idx;
> + unsigned int total_bufs, in_total, out_total;
> + MemoryRegionCache *desc_cache;
> + VRingMemoryRegionCaches *caches;
> + MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
> + int64_t len = 0;
> + VRingPackedDesc desc;
> + bool wrap_counter;
> +
> + if (unlikely(!vq->vring.desc)) {
> + if (in_bytes) {
> + *in_bytes = 0;
> + }
> + if (out_bytes) {
> + *out_bytes = 0;
> + }
> + return;
> + }
> +
> + rcu_read_lock();
> + idx = vq->last_avail_idx;
> + wrap_counter = vq->avail_wrap_counter;
> + total_bufs = in_total = out_total = 0;
> +
> + max = vq->vring.num;
> + caches = vring_get_region_caches(vq);
> + if (caches->desc.len < max * sizeof(VRingPackedDesc)) {
> + virtio_error(vdev, "Cannot map descriptor ring");
> + goto err;
> + }
The above is mostly duplicated with split version. Can we unify them and
start the different version here?
> +
> + desc_cache = &caches->desc;
> + vring_packed_desc_read(vdev, &desc, desc_cache, idx);
> + /* Make sure we see all the fields*/
> + smp_rmb();
This looks strange. Do you want to make sure the flags were read before
other fields of descriptor?
You probably need a helper which did:
vring_packed_desc_read_flags(&desc)
if (is_desc_avail(&desc) {
smp_rmb();
return true;
}
return false;
> + while (is_desc_avail(&desc, wrap_counter)) {
> + unsigned int num_bufs;
> + unsigned int i = 0;
> +
> + num_bufs = total_bufs;
> +
> + if (desc.flags & VRING_DESC_F_INDIRECT) {
> + if (desc.len % sizeof(VRingPackedDesc)) {
> + virtio_error(vdev, "Invalid size for indirect buffer table");
> + goto err;
> + }
> +
> + /* If we've got too many, that implies a descriptor loop. */
> + if (num_bufs >= max) {
> + virtio_error(vdev, "Looped descriptor");
> + goto err;
> + }
> +
> + /* loop over the indirect descriptor table */
> + len = address_space_cache_init(&indirect_desc_cache,
> + vdev->dma_as,
> + desc.addr, desc.len, false);
> + desc_cache = &indirect_desc_cache;
> + if (len < desc.len) {
> + virtio_error(vdev, "Cannot map indirect buffer");
> + goto err;
Do we need to destroy desc cache here?
> + }
> +
> + max = desc.len / sizeof(VRingPackedDesc);
> + num_bufs = i = 0;
> + vring_packed_desc_read(vdev, &desc, desc_cache, i);
> + /* Make sure we see all the fields*/
> + smp_rmb();
All fields have already been read by us, why need this barrier?
> + }
> +
> + do {
> + /* If we've got too many, that implies a descriptor loop. */
> + if (++num_bufs > max) {
> + virtio_error(vdev, "Looped descriptor");
> + goto err;
> + }
> +
> + if (desc.flags & VRING_DESC_F_WRITE) {
> + in_total += desc.len;
> + } else {
> + out_total += desc.len;
> + }
> + if (in_total >= max_in_bytes && out_total >= max_out_bytes) {
> + goto done;
> + }
> +
> + if (desc_cache == &indirect_desc_cache) {
> + if (++i > vq->vring.num) {
> + virtio_error(vdev, "Looped descriptor");
> + goto err;
> + }
This duplicates with the above num_bufs check I think? And it's better
to check them before the avail bytes calculation.
> + vring_packed_desc_read(vdev, &desc, desc_cache, i);
> + } else {
> + if (++idx >= vq->vring.num) {
> + idx -= vq->vring.num;
> + wrap_counter = !wrap_counter;
> + }
> + vring_packed_desc_read(vdev, &desc, desc_cache, idx);
> + }
> + /* Make sure we see the flags */
> + smp_rmb();
This is also suspicious. For commenting, we usually mention the order we
want like
"make sure XXX is read/write before YYY."
> + } while (desc.flags & VRING_DESC_F_NEXT);
Why not implement a split version of virtqueue_read_next_desc() and hide
all e.g wrap counter & barrier stuffs there?
> +
> + if (desc_cache == &indirect_desc_cache) {
> + address_space_cache_destroy(&indirect_desc_cache);
> + total_bufs++;
> + /* We missed one step on for indirect desc */
> + idx++;
> + if (++idx >= vq->vring.num) {
> + idx -= vq->vring.num;
> + wrap_counter = !wrap_counter;
> + }
> + } else {
> + total_bufs = num_bufs;
> + }
> +
> + desc_cache = &caches->desc;
> + vring_packed_desc_read(vdev, &desc, desc_cache, idx);
> + /* Make sure we see all the fields */
> + smp_rmb();
Need to better comment for explaining this barrier as well.
> + }
> +
> + /* Set up index and wrap counter for an interrupt when no enough desc */
I don't get what did this mean?
Thanks
> + vq->event_idx = idx;
> + vq->event_wrap_counter = wrap_counter;
> +done:
> + address_space_cache_destroy(&indirect_desc_cache);
> + if (in_bytes) {
> + *in_bytes = in_total;
> + }
> + if (out_bytes) {
> + *out_bytes = out_total;
> + }
> + rcu_read_unlock();
> + return;
> +
> +err:
> + in_total = out_total = 0;
> + goto done;
> +}
> +
> +void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
> + unsigned int *out_bytes,
> + unsigned max_in_bytes, unsigned max_out_bytes)
> +{
> + if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> + virtqueue_packed_get_avail_bytes(vq, in_bytes, out_bytes,
> + max_in_bytes, max_out_bytes);
> + } else {
> + virtqueue_split_get_avail_bytes(vq, in_bytes, out_bytes,
> + max_in_bytes, max_out_bytes);
> + }
> +}
> +
> int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes,
> unsigned int out_bytes)
> {
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [[RFC v3 07/12] virtio: fill/flush/pop for packed ring
2018-10-11 14:08 [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support wexu
` (5 preceding siblings ...)
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 06/12] virtio: get avail bytes " wexu
@ 2018-10-11 14:08 ` wexu
2018-10-15 6:14 ` Jason Wang
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 08/12] virtio: event suppression support " wexu
` (5 subsequent siblings)
12 siblings, 1 reply; 36+ messages in thread
From: wexu @ 2018-10-11 14:08 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, tiwei.bie, wexu, jfreimann, maxime.coquelin
From: Wei Xu <wexu@redhat.com>
Signed-off-by: Wei Xu <wexu@redhat.com>
---
hw/virtio/virtio.c | 258 ++++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 244 insertions(+), 14 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 13c6c98..d12a7e3 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -386,6 +386,21 @@ static void vring_packed_desc_read(VirtIODevice *vdev, VRingPackedDesc *desc,
virtio_tswap16s(vdev, &desc->id);
}
+static void vring_packed_desc_write(VirtIODevice *vdev, VRingPackedDesc *desc,
+ MemoryRegionCache *cache, int i)
+{
+ virtio_tswap64s(vdev, &desc->addr);
+ virtio_tswap32s(vdev, &desc->len);
+ virtio_tswap16s(vdev, &desc->id);
+ virtio_tswap16s(vdev, &desc->flags);
+ address_space_write_cached(cache,
+ sizeof(VRingPackedDesc) * i, desc,
+ sizeof(VRingPackedDesc));
+ address_space_cache_invalidate(cache,
+ sizeof(VRingPackedDesc) * i,
+ sizeof(VRingPackedDesc));
+}
+
static void vring_packed_desc_read_flags(VirtIODevice *vdev,
VRingPackedDesc *desc, MemoryRegionCache *cache, int i)
{
@@ -559,19 +574,11 @@ bool virtqueue_rewind(VirtQueue *vq, unsigned int num)
}
/* Called within rcu_read_lock(). */
-void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
+static void virtqueue_split_fill(VirtQueue *vq, const VirtQueueElement *elem,
unsigned int len, unsigned int idx)
{
VRingUsedElem uelem;
- trace_virtqueue_fill(vq, elem, len, idx);
-
- virtqueue_unmap_sg(vq, elem, len);
-
- if (unlikely(vq->vdev->broken)) {
- return;
- }
-
if (unlikely(!vq->vring.used)) {
return;
}
@@ -583,16 +590,64 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
vring_used_write(vq, &uelem, idx);
}
-/* Called within rcu_read_lock(). */
-void virtqueue_flush(VirtQueue *vq, unsigned int count)
+static void virtqueue_packed_fill(VirtQueue *vq, const VirtQueueElement *elem,
+ unsigned int len, unsigned int idx)
{
- uint16_t old, new;
+ uint16_t w, head;
+ VRingMemoryRegionCaches *caches;
+ VRingPackedDesc desc = {
+ .addr = 0,
+ .flags = 0,
+ };
+
+ if (unlikely(!vq->vring.desc)) {
+ return;
+ }
+
+ caches = vring_get_region_caches(vq);
+ head = vq->used_idx + idx;
+ head = head >= vq->vring.num ? (head - vq->vring.num) : head;
+ vring_packed_desc_read(vq->vdev, &desc, &caches->desc, head);
+
+ w = (desc.flags & AVAIL_DESC_PACKED(1)) >> 7;
+ desc.flags &= ~(AVAIL_DESC_PACKED(1) | USED_DESC_PACKED(1));
+ desc.flags |= AVAIL_DESC_PACKED(w) | USED_DESC_PACKED(w);
+ if (!(desc.flags & VRING_DESC_F_INDIRECT)) {
+ if (!(desc.flags & VRING_DESC_F_WRITE)) {
+ desc.len = 0;
+ } else {
+ desc.len = len;
+ }
+ }
+ vring_packed_desc_write(vq->vdev, &desc, &caches->desc, head);
+
+ /* Make sure flags has been updated */
+ smp_mb();
+}
+
+void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
+ unsigned int len, unsigned int idx)
+{
+ trace_virtqueue_fill(vq, elem, len, idx);
+
+ virtqueue_unmap_sg(vq, elem, len);
if (unlikely(vq->vdev->broken)) {
- vq->inuse -= count;
return;
}
+ if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
+ virtqueue_packed_fill(vq, elem, len, idx);
+ } else {
+ virtqueue_split_fill(vq, elem, len, idx);
+ }
+}
+
+/* Called within rcu_read_lock(). */
+static void virtqueue_split_flush(VirtQueue *vq, unsigned int count)
+{
+ uint16_t old, new;
+
if (unlikely(!vq->vring.used)) {
return;
}
@@ -608,6 +663,33 @@ void virtqueue_flush(VirtQueue *vq, unsigned int count)
vq->signalled_used_valid = false;
}
+static void virtqueue_packed_flush(VirtQueue *vq, unsigned int count)
+{
+ if (unlikely(!vq->vring.desc)) {
+ return;
+ }
+
+ vq->inuse -= count;
+ vq->used_idx += count;
+ if (vq->used_idx >= vq->vring.num) {
+ vq->used_idx -= vq->vring.num;
+ }
+}
+
+void virtqueue_flush(VirtQueue *vq, unsigned int count)
+{
+ if (unlikely(vq->vdev->broken)) {
+ vq->inuse -= count;
+ return;
+ }
+
+ if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
+ virtqueue_packed_flush(vq, count);
+ } else {
+ virtqueue_split_flush(vq, count);
+ }
+}
+
void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
unsigned int len)
{
@@ -1091,7 +1173,7 @@ static void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_nu
return elem;
}
-void *virtqueue_pop(VirtQueue *vq, size_t sz)
+static void *virtqueue_split_pop(VirtQueue *vq, size_t sz)
{
unsigned int i, head, max;
VRingMemoryRegionCaches *caches;
@@ -1226,6 +1308,154 @@ err_undo_map:
goto done;
}
+static void *virtqueue_packed_pop(VirtQueue *vq, size_t sz)
+{
+ unsigned int i, head, max;
+ VRingMemoryRegionCaches *caches;
+ MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
+ MemoryRegionCache *cache;
+ int64_t len;
+ VirtIODevice *vdev = vq->vdev;
+ VirtQueueElement *elem = NULL;
+ unsigned out_num, in_num, elem_entries;
+ hwaddr addr[VIRTQUEUE_MAX_SIZE];
+ struct iovec iov[VIRTQUEUE_MAX_SIZE];
+ VRingPackedDesc desc;
+
+ if (unlikely(vdev->broken)) {
+ return NULL;
+ }
+
+ rcu_read_lock();
+ if (virtio_queue_packed_empty_rcu(vq)) {
+ goto done;
+ }
+
+ /* When we start there are none of either input nor output. */
+ out_num = in_num = elem_entries = 0;
+
+ max = vq->vring.num;
+
+ if (vq->inuse >= vq->vring.num) {
+ virtio_error(vdev, "Virtqueue size exceeded");
+ goto done;
+ }
+
+ head = vq->last_avail_idx;
+ i = head;
+
+ caches = vring_get_region_caches(vq);
+ cache = &caches->desc;
+ vring_packed_desc_read(vdev, &desc, cache, i);
+ /* Make sure we see all the fields*/
+ smp_rmb();
+ if (desc.flags & VRING_DESC_F_INDIRECT) {
+ if (desc.len % sizeof(VRingPackedDesc)) {
+ virtio_error(vdev, "Invalid size for indirect buffer table");
+ goto done;
+ }
+
+ /* loop over the indirect descriptor table */
+ len = address_space_cache_init(&indirect_desc_cache, vdev->dma_as,
+ desc.addr, desc.len, false);
+ cache = &indirect_desc_cache;
+ if (len < desc.len) {
+ virtio_error(vdev, "Cannot map indirect buffer");
+ goto done;
+ }
+
+ max = desc.len / sizeof(VRingPackedDesc);
+ i = 0;
+ vring_packed_desc_read(vdev, &desc, cache, i);
+ /* Make sure we see all the fields*/
+ smp_rmb();
+ }
+
+ /* Collect all the descriptors */
+ while (1) {
+ bool map_ok;
+
+ if (desc.flags & VRING_DESC_F_WRITE) {
+ map_ok = virtqueue_map_desc(vdev, &in_num, addr + out_num,
+ iov + out_num,
+ VIRTQUEUE_MAX_SIZE - out_num, true,
+ desc.addr, desc.len);
+ } else {
+ if (in_num) {
+ virtio_error(vdev, "Incorrect order for descriptors");
+ goto err_undo_map;
+ }
+ map_ok = virtqueue_map_desc(vdev, &out_num, addr, iov,
+ VIRTQUEUE_MAX_SIZE, false,
+ desc.addr, desc.len);
+ }
+ if (!map_ok) {
+ goto err_undo_map;
+ }
+
+ /* If we've got too many, that implies a descriptor loop. */
+ if (++elem_entries > max) {
+ virtio_error(vdev, "Looped descriptor");
+ goto err_undo_map;
+ }
+
+ if (++i >= vq->vring.num) {
+ i -= vq->vring.num;
+ }
+
+ if (desc.flags & VRING_DESC_F_NEXT) {
+ vring_packed_desc_read(vq->vdev, &desc, cache, i);
+ /* Make sure we see all the fields*/
+ smp_rmb();
+ } else {
+ break;
+ }
+ }
+
+ /* Now copy what we have collected and mapped */
+ elem = virtqueue_alloc_element(sz, out_num, in_num);
+ for (i = 0; i < out_num; i++) {
+ elem->out_addr[i] = addr[i];
+ elem->out_sg[i] = iov[i];
+ }
+ for (i = 0; i < in_num; i++) {
+ elem->in_addr[i] = addr[head + out_num + i];
+ elem->in_sg[i] = iov[out_num + i];
+ }
+
+ vq->last_avail_idx += (cache == &indirect_desc_cache) ?
+ 1 : out_num + in_num;
+ if (vq->last_avail_idx >= vq->vring.num) {
+ vq->last_avail_idx -= vq->vring.num;
+ vq->avail_wrap_counter = !vq->avail_wrap_counter;
+ }
+ vq->inuse++;
+
+ vq->event_idx = vq->last_avail_idx;
+ vq->event_wrap_counter = vq->avail_wrap_counter;
+
+ trace_virtqueue_pop(vq, elem, elem->in_num, elem->out_num);
+done:
+ address_space_cache_destroy(&indirect_desc_cache);
+ rcu_read_unlock();
+
+ return elem;
+
+err_undo_map:
+ virtqueue_undo_map_desc(out_num, in_num, iov);
+ g_free(elem);
+ goto done;
+}
+
+void *virtqueue_pop(VirtQueue *vq, size_t sz)
+{
+ if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
+ return virtqueue_packed_pop(vq, sz);
+ } else {
+ return virtqueue_split_pop(vq, sz);
+ }
+}
+
/* virtqueue_drop_all:
* @vq: The #VirtQueue
* Drops all queued buffers and indicates them to the guest
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 07/12] virtio: fill/flush/pop for packed ring
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 07/12] virtio: fill/flush/pop " wexu
@ 2018-10-15 6:14 ` Jason Wang
0 siblings, 0 replies; 36+ messages in thread
From: Jason Wang @ 2018-10-15 6:14 UTC (permalink / raw)
To: wexu, qemu-devel; +Cc: tiwei.bie, jfreimann, maxime.coquelin
On 2018年10月11日 22:08, wexu@redhat.com wrote:
> From: Wei Xu <wexu@redhat.com>
>
> Signed-off-by: Wei Xu <wexu@redhat.com>
> ---
> hw/virtio/virtio.c | 258 ++++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 244 insertions(+), 14 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 13c6c98..d12a7e3 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -386,6 +386,21 @@ static void vring_packed_desc_read(VirtIODevice *vdev, VRingPackedDesc *desc,
> virtio_tswap16s(vdev, &desc->id);
> }
>
> +static void vring_packed_desc_write(VirtIODevice *vdev, VRingPackedDesc *desc,
> + MemoryRegionCache *cache, int i)
> +{
> + virtio_tswap64s(vdev, &desc->addr);
> + virtio_tswap32s(vdev, &desc->len);
> + virtio_tswap16s(vdev, &desc->id);
> + virtio_tswap16s(vdev, &desc->flags);
> + address_space_write_cached(cache,
> + sizeof(VRingPackedDesc) * i, desc,
> + sizeof(VRingPackedDesc));
> + address_space_cache_invalidate(cache,
> + sizeof(VRingPackedDesc) * i,
> + sizeof(VRingPackedDesc));
> +}
> +
> static void vring_packed_desc_read_flags(VirtIODevice *vdev,
> VRingPackedDesc *desc, MemoryRegionCache *cache, int i)
> {
> @@ -559,19 +574,11 @@ bool virtqueue_rewind(VirtQueue *vq, unsigned int num)
> }
>
> /* Called within rcu_read_lock(). */
> -void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
> +static void virtqueue_split_fill(VirtQueue *vq, const VirtQueueElement *elem,
> unsigned int len, unsigned int idx)
> {
> VRingUsedElem uelem;
>
> - trace_virtqueue_fill(vq, elem, len, idx);
> -
> - virtqueue_unmap_sg(vq, elem, len);
> -
> - if (unlikely(vq->vdev->broken)) {
> - return;
> - }
> -
> if (unlikely(!vq->vring.used)) {
> return;
> }
> @@ -583,16 +590,64 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
> vring_used_write(vq, &uelem, idx);
> }
>
> -/* Called within rcu_read_lock(). */
> -void virtqueue_flush(VirtQueue *vq, unsigned int count)
> +static void virtqueue_packed_fill(VirtQueue *vq, const VirtQueueElement *elem,
> + unsigned int len, unsigned int idx)
> {
> - uint16_t old, new;
> + uint16_t w, head;
> + VRingMemoryRegionCaches *caches;
> + VRingPackedDesc desc = {
> + .addr = 0,
> + .flags = 0,
> + };
> +
> + if (unlikely(!vq->vring.desc)) {
> + return;
> + }
> +
> + caches = vring_get_region_caches(vq);
> + head = vq->used_idx + idx;
> + head = head >= vq->vring.num ? (head - vq->vring.num) : head;
> + vring_packed_desc_read(vq->vdev, &desc, &caches->desc, head);
Is this a must for reading descriptor once more? Shouldn't device
maintain its own used_wrap_counter?
> +
> + w = (desc.flags & AVAIL_DESC_PACKED(1)) >> 7;
> + desc.flags &= ~(AVAIL_DESC_PACKED(1) | USED_DESC_PACKED(1));
> + desc.flags |= AVAIL_DESC_PACKED(w) | USED_DESC_PACKED(w);
> + if (!(desc.flags & VRING_DESC_F_INDIRECT)) {
> + if (!(desc.flags & VRING_DESC_F_WRITE)) {
> + desc.len = 0;
> + } else {
> + desc.len = len;
> + }
> + }
> + vring_packed_desc_write(vq->vdev, &desc, &caches->desc, head);
> +
> + /* Make sure flags has been updated */
> + smp_mb();
This is wrong in two places:
- What you want is to make sure flag was updated after all other fields,
you can do it in vring_packed_desc_write()
- A write barrier instead of a full barrier is needed.
> +}
> +
> +void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
> + unsigned int len, unsigned int idx)
> +{
> + trace_virtqueue_fill(vq, elem, len, idx);
> +
> + virtqueue_unmap_sg(vq, elem, len);
>
> if (unlikely(vq->vdev->broken)) {
> - vq->inuse -= count;
> return;
> }
>
> + if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> + virtqueue_packed_fill(vq, elem, len, idx);
> + } else {
> + virtqueue_split_fill(vq, elem, len, idx);
> + }
> +}
> +
> +/* Called within rcu_read_lock(). */
> +static void virtqueue_split_flush(VirtQueue *vq, unsigned int count)
> +{
> + uint16_t old, new;
> +
> if (unlikely(!vq->vring.used)) {
> return;
> }
> @@ -608,6 +663,33 @@ void virtqueue_flush(VirtQueue *vq, unsigned int count)
> vq->signalled_used_valid = false;
> }
>
> +static void virtqueue_packed_flush(VirtQueue *vq, unsigned int count)
> +{
> + if (unlikely(!vq->vring.desc)) {
> + return;
> + }
> +
> + vq->inuse -= count;
> + vq->used_idx += count;
> + if (vq->used_idx >= vq->vring.num) {
> + vq->used_idx -= vq->vring.num;
> + }
> +}
> +
> +void virtqueue_flush(VirtQueue *vq, unsigned int count)
> +{
> + if (unlikely(vq->vdev->broken)) {
> + vq->inuse -= count;
> + return;
> + }
> +
> + if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> + virtqueue_packed_flush(vq, count);
> + } else {
> + virtqueue_split_flush(vq, count);
> + }
> +}
> +
> void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
> unsigned int len)
> {
> @@ -1091,7 +1173,7 @@ static void *virtqueue_alloc_element(size_t sz, unsigned out_num, unsigned in_nu
> return elem;
> }
>
> -void *virtqueue_pop(VirtQueue *vq, size_t sz)
> +static void *virtqueue_split_pop(VirtQueue *vq, size_t sz)
> {
> unsigned int i, head, max;
> VRingMemoryRegionCaches *caches;
> @@ -1226,6 +1308,154 @@ err_undo_map:
> goto done;
> }
>
> +static void *virtqueue_packed_pop(VirtQueue *vq, size_t sz)
> +{
> + unsigned int i, head, max;
> + VRingMemoryRegionCaches *caches;
> + MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
> + MemoryRegionCache *cache;
> + int64_t len;
> + VirtIODevice *vdev = vq->vdev;
> + VirtQueueElement *elem = NULL;
> + unsigned out_num, in_num, elem_entries;
> + hwaddr addr[VIRTQUEUE_MAX_SIZE];
> + struct iovec iov[VIRTQUEUE_MAX_SIZE];
> + VRingPackedDesc desc;
> +
> + if (unlikely(vdev->broken)) {
> + return NULL;
> + }
> +
> + rcu_read_lock();
> + if (virtio_queue_packed_empty_rcu(vq)) {
> + goto done;
> + }
> +
> + /* When we start there are none of either input nor output. */
> + out_num = in_num = elem_entries = 0;
> +
> + max = vq->vring.num;
> +
> + if (vq->inuse >= vq->vring.num) {
> + virtio_error(vdev, "Virtqueue size exceeded");
> + goto done;
> + }
The above is duplicated with split version, please unify them.
> +
> + head = vq->last_avail_idx;
> + i = head;
> +
> + caches = vring_get_region_caches(vq);
> + cache = &caches->desc;
> + vring_packed_desc_read(vdev, &desc, cache, i);
> + /* Make sure we see all the fields*/
> + smp_rmb();
This is wrong, as I've pointed out.
> + if (desc.flags & VRING_DESC_F_INDIRECT) {
> + if (desc.len % sizeof(VRingPackedDesc)) {
> + virtio_error(vdev, "Invalid size for indirect buffer table");
> + goto done;
> + }
> +
> + /* loop over the indirect descriptor table */
> + len = address_space_cache_init(&indirect_desc_cache, vdev->dma_as,
> + desc.addr, desc.len, false);
> + cache = &indirect_desc_cache;
> + if (len < desc.len) {
> + virtio_error(vdev, "Cannot map indirect buffer");
> + goto done;
> + }
> +
> + max = desc.len / sizeof(VRingPackedDesc);
> + i = 0;
> + vring_packed_desc_read(vdev, &desc, cache, i);
> + /* Make sure we see all the fields*/
> + smp_rmb();
> + }
> +
> + /* Collect all the descriptors */
> + while (1) {
> + bool map_ok;
> +
> + if (desc.flags & VRING_DESC_F_WRITE) {
> + map_ok = virtqueue_map_desc(vdev, &in_num, addr + out_num,
> + iov + out_num,
> + VIRTQUEUE_MAX_SIZE - out_num, true,
> + desc.addr, desc.len);
> + } else {
> + if (in_num) {
> + virtio_error(vdev, "Incorrect order for descriptors");
> + goto err_undo_map;
> + }
> + map_ok = virtqueue_map_desc(vdev, &out_num, addr, iov,
> + VIRTQUEUE_MAX_SIZE, false,
> + desc.addr, desc.len);
> + }
> + if (!map_ok) {
> + goto err_undo_map;
> + }
> +
> + /* If we've got too many, that implies a descriptor loop. */
> + if (++elem_entries > max) {
> + virtio_error(vdev, "Looped descriptor");
> + goto err_undo_map;
> + }
> +
> + if (++i >= vq->vring.num) {
> + i -= vq->vring.num;
> + }
> +
> + if (desc.flags & VRING_DESC_F_NEXT) {
> + vring_packed_desc_read(vq->vdev, &desc, cache, i);
> + /* Make sure we see all the fields*/
> + smp_rmb();
This looks unnecessary.
Thanks
> + } else {
> + break;
> + }
> + }
> +
> + /* Now copy what we have collected and mapped */
> + elem = virtqueue_alloc_element(sz, out_num, in_num);
> + for (i = 0; i < out_num; i++) {
> + elem->out_addr[i] = addr[i];
> + elem->out_sg[i] = iov[i];
> + }
> + for (i = 0; i < in_num; i++) {
> + elem->in_addr[i] = addr[head + out_num + i];
> + elem->in_sg[i] = iov[out_num + i];
> + }
> +
> + vq->last_avail_idx += (cache == &indirect_desc_cache) ?
> + 1 : out_num + in_num;
> + if (vq->last_avail_idx >= vq->vring.num) {
> + vq->last_avail_idx -= vq->vring.num;
> + vq->avail_wrap_counter = !vq->avail_wrap_counter;
> + }
> + vq->inuse++;
> +
> + vq->event_idx = vq->last_avail_idx;
> + vq->event_wrap_counter = vq->avail_wrap_counter;
> +
> + trace_virtqueue_pop(vq, elem, elem->in_num, elem->out_num);
> +done:
> + address_space_cache_destroy(&indirect_desc_cache);
> + rcu_read_unlock();
> +
> + return elem;
> +
> +err_undo_map:
> + virtqueue_undo_map_desc(out_num, in_num, iov);
> + g_free(elem);
> + goto done;
> +}
> +
> +void *virtqueue_pop(VirtQueue *vq, size_t sz)
> +{
> + if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> + return virtqueue_packed_pop(vq, sz);
> + } else {
> + return virtqueue_split_pop(vq, sz);
> + }
> +}
> +
> /* virtqueue_drop_all:
> * @vq: The #VirtQueue
> * Drops all queued buffers and indicates them to the guest
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [[RFC v3 08/12] virtio: event suppression support for packed ring
2018-10-11 14:08 [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support wexu
` (6 preceding siblings ...)
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 07/12] virtio: fill/flush/pop " wexu
@ 2018-10-11 14:08 ` wexu
2018-10-15 6:55 ` Jason Wang
2018-10-15 6:59 ` Jason Wang
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 09/12] virtio-net: fill head desc after done all in a chain wexu
` (4 subsequent siblings)
12 siblings, 2 replies; 36+ messages in thread
From: wexu @ 2018-10-11 14:08 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, tiwei.bie, wexu, jfreimann, maxime.coquelin
From: Wei Xu <wexu@redhat.com>
Signed-off-by: Wei Xu <wexu@redhat.com>
---
hw/virtio/virtio.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 123 insertions(+), 3 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index d12a7e3..1d25776 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -241,6 +241,30 @@ static void vring_desc_read(VirtIODevice *vdev, VRingDesc *desc,
virtio_tswap16s(vdev, &desc->next);
}
+static void vring_packed_event_read(VirtIODevice *vdev,
+ MemoryRegionCache *cache, VRingPackedDescEvent *e)
+{
+ address_space_read_cached(cache, 0, e, sizeof(*e));
+ virtio_tswap16s(vdev, &e->off_wrap);
+ virtio_tswap16s(vdev, &e->flags);
+}
+
+static void vring_packed_off_wrap_write(VirtIODevice *vdev,
+ MemoryRegionCache *cache, uint16_t off_wrap)
+{
+ virtio_tswap16s(vdev, &off_wrap);
+ address_space_write_cached(cache, 0, &off_wrap, sizeof(off_wrap));
+ address_space_cache_invalidate(cache, 0, sizeof(off_wrap));
+}
+
+static void vring_packed_flags_write(VirtIODevice *vdev,
+ MemoryRegionCache *cache, uint16_t flags)
+{
+ virtio_tswap16s(vdev, &flags);
+ address_space_write_cached(cache, sizeof(uint16_t), &flags, sizeof(flags));
+ address_space_cache_invalidate(cache, sizeof(uint16_t), sizeof(flags));
+}
+
static VRingMemoryRegionCaches *vring_get_region_caches(struct VirtQueue *vq)
{
VRingMemoryRegionCaches *caches = atomic_rcu_read(&vq->vring.caches);
@@ -347,7 +371,7 @@ static inline void vring_set_avail_event(VirtQueue *vq, uint16_t val)
address_space_cache_invalidate(&caches->used, pa, sizeof(val));
}
-void virtio_queue_set_notification(VirtQueue *vq, int enable)
+static void virtio_queue_set_notification_split(VirtQueue *vq, int enable)
{
vq->notification = enable;
@@ -370,6 +394,51 @@ void virtio_queue_set_notification(VirtQueue *vq, int enable)
rcu_read_unlock();
}
+static void virtio_queue_set_notification_packed(VirtQueue *vq, int enable)
+{
+ VRingPackedDescEvent e;
+ VRingMemoryRegionCaches *caches;
+
+ rcu_read_lock();
+ caches = vring_get_region_caches(vq);
+ vring_packed_event_read(vq->vdev, &caches->device, &e);
+
+ if (!enable) {
+ e.flags = RING_EVENT_FLAGS_DISABLE;
+ goto out;
+ }
+
+ e.flags = RING_EVENT_FLAGS_ENABLE;
+ if (virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
+ uint16_t off_wrap = vq->event_idx | vq->event_wrap_counter << 15;
+
+ vring_packed_off_wrap_write(vq->vdev, &caches->device, off_wrap);
+ /* Make sure off_wrap is wrote before flags */
+ smp_wmb();
+
+ e.flags = RING_EVENT_FLAGS_DESC;
+ }
+
+out:
+ vring_packed_flags_write(vq->vdev, &caches->device, e.flags);
+ rcu_read_unlock();
+}
+
+void virtio_queue_set_notification(VirtQueue *vq, int enable)
+{
+ vq->notification = enable;
+
+ if (!vq->vring.desc) {
+ return;
+ }
+
+ if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
+ virtio_queue_set_notification_packed(vq, enable);
+ } else {
+ virtio_queue_set_notification_split(vq, enable);
+ }
+}
+
int virtio_queue_ready(VirtQueue *vq)
{
return vq->vring.avail != 0;
@@ -2103,8 +2172,7 @@ static void virtio_set_isr(VirtIODevice *vdev, int value)
}
}
-/* Called within rcu_read_lock(). */
-static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq)
+static bool virtio_split_should_notify(VirtIODevice *vdev, VirtQueue *vq)
{
uint16_t old, new;
bool v;
@@ -2127,6 +2195,58 @@ static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq)
return !v || vring_need_event(vring_get_used_event(vq), new, old);
}
+static bool vring_packed_need_event(VirtQueue *vq, uint16_t off_wrap,
+ uint16_t new, uint16_t old)
+{
+ bool wrap = vq->event_wrap_counter;
+ int off = off_wrap & ~(1 << 15);
+
+ if (new < old) {
+ new += vq->vring.num;
+ wrap ^= 1;
+ }
+
+ if (wrap != off_wrap >> 15) {
+ off += vq->vring.num;
+ }
+
+ return vring_need_event(off, new, old);
+}
+
+static bool virtio_packed_should_notify(VirtIODevice *vdev, VirtQueue *vq)
+{
+ VRingPackedDescEvent e;
+ uint16_t old, new;
+ bool v;
+ VRingMemoryRegionCaches *caches;
+
+ caches = vring_get_region_caches(vq);
+ vring_packed_event_read(vdev, &caches->driver, &e);
+
+ old = vq->signalled_used;
+ new = vq->signalled_used = vq->used_idx;
+ v = vq->signalled_used_valid;
+ vq->signalled_used_valid = true;
+
+ if (e.flags == RING_EVENT_FLAGS_DISABLE) {
+ return false;
+ } else if (e.flags == RING_EVENT_FLAGS_ENABLE) {
+ return true;
+ }
+
+ return !v || vring_packed_need_event(vq, e.off_wrap, new, old);
+}
+
+/* Called within rcu_read_lock(). */
+static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq)
+{
+ if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
+ return virtio_packed_should_notify(vdev, vq);
+ } else {
+ return virtio_split_should_notify(vdev, vq);
+ }
+}
+
void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq)
{
bool should_notify;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 08/12] virtio: event suppression support for packed ring
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 08/12] virtio: event suppression support " wexu
@ 2018-10-15 6:55 ` Jason Wang
2018-10-15 6:59 ` Jason Wang
1 sibling, 0 replies; 36+ messages in thread
From: Jason Wang @ 2018-10-15 6:55 UTC (permalink / raw)
To: wexu, qemu-devel; +Cc: tiwei.bie, jfreimann, maxime.coquelin
On 2018年10月11日 22:08, wexu@redhat.com wrote:
> From: Wei Xu <wexu@redhat.com>
>
> Signed-off-by: Wei Xu <wexu@redhat.com>
> ---
> hw/virtio/virtio.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 123 insertions(+), 3 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index d12a7e3..1d25776 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -241,6 +241,30 @@ static void vring_desc_read(VirtIODevice *vdev, VRingDesc *desc,
> virtio_tswap16s(vdev, &desc->next);
> }
>
> +static void vring_packed_event_read(VirtIODevice *vdev,
> + MemoryRegionCache *cache, VRingPackedDescEvent *e)
> +{
> + address_space_read_cached(cache, 0, e, sizeof(*e));
> + virtio_tswap16s(vdev, &e->off_wrap);
> + virtio_tswap16s(vdev, &e->flags);
> +}
> +
> +static void vring_packed_off_wrap_write(VirtIODevice *vdev,
> + MemoryRegionCache *cache, uint16_t off_wrap)
> +{
> + virtio_tswap16s(vdev, &off_wrap);
> + address_space_write_cached(cache, 0, &off_wrap, sizeof(off_wrap));
> + address_space_cache_invalidate(cache, 0, sizeof(off_wrap));
> +}
> +
> +static void vring_packed_flags_write(VirtIODevice *vdev,
> + MemoryRegionCache *cache, uint16_t flags)
> +{
> + virtio_tswap16s(vdev, &flags);
> + address_space_write_cached(cache, sizeof(uint16_t), &flags, sizeof(flags));
> + address_space_cache_invalidate(cache, sizeof(uint16_t), sizeof(flags));
> +}
> +
> static VRingMemoryRegionCaches *vring_get_region_caches(struct VirtQueue *vq)
> {
> VRingMemoryRegionCaches *caches = atomic_rcu_read(&vq->vring.caches);
> @@ -347,7 +371,7 @@ static inline void vring_set_avail_event(VirtQueue *vq, uint16_t val)
> address_space_cache_invalidate(&caches->used, pa, sizeof(val));
> }
>
> -void virtio_queue_set_notification(VirtQueue *vq, int enable)
> +static void virtio_queue_set_notification_split(VirtQueue *vq, int enable)
> {
> vq->notification = enable;
>
> @@ -370,6 +394,51 @@ void virtio_queue_set_notification(VirtQueue *vq, int enable)
> rcu_read_unlock();
> }
>
> +static void virtio_queue_set_notification_packed(VirtQueue *vq, int enable)
> +{
> + VRingPackedDescEvent e;
> + VRingMemoryRegionCaches *caches;
> +
> + rcu_read_lock();
> + caches = vring_get_region_caches(vq);
> + vring_packed_event_read(vq->vdev, &caches->device, &e);
> +
> + if (!enable) {
> + e.flags = RING_EVENT_FLAGS_DISABLE;
> + goto out;
> + }
> +
> + e.flags = RING_EVENT_FLAGS_ENABLE;
> + if (virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
> + uint16_t off_wrap = vq->event_idx | vq->event_wrap_counter << 15;
> +
> + vring_packed_off_wrap_write(vq->vdev, &caches->device, off_wrap);
> + /* Make sure off_wrap is wrote before flags */
> + smp_wmb();
> +
> + e.flags = RING_EVENT_FLAGS_DESC;
> + }
> +
> +out:
> + vring_packed_flags_write(vq->vdev, &caches->device, e.flags);
> + rcu_read_unlock();
> +}
> +
> +void virtio_queue_set_notification(VirtQueue *vq, int enable)
> +{
> + vq->notification = enable;
> +
> + if (!vq->vring.desc) {
> + return;
> + }
> +
> + if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> + virtio_queue_set_notification_packed(vq, enable);
> + } else {
> + virtio_queue_set_notification_split(vq, enable);
> + }
> +}
> +
> int virtio_queue_ready(VirtQueue *vq)
> {
> return vq->vring.avail != 0;
> @@ -2103,8 +2172,7 @@ static void virtio_set_isr(VirtIODevice *vdev, int value)
> }
> }
>
> -/* Called within rcu_read_lock(). */
> -static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> +static bool virtio_split_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> {
> uint16_t old, new;
> bool v;
> @@ -2127,6 +2195,58 @@ static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> return !v || vring_need_event(vring_get_used_event(vq), new, old);
> }
>
> +static bool vring_packed_need_event(VirtQueue *vq, uint16_t off_wrap,
> + uint16_t new, uint16_t old)
> +{
> + bool wrap = vq->event_wrap_counter;
> + int off = off_wrap & ~(1 << 15);
> +
> + if (new < old) {
> + new += vq->vring.num;
> + wrap ^= 1;
> + }
> +
> + if (wrap != off_wrap >> 15) {
> + off += vq->vring.num;
> + }
Let's use a more compact and verified version from dpdk:
static bool vhost_vring_packed_need_event(struct vhost_virtqueue *vq,
bool wrap, __u16 off_wrap, __u16 new,
__u16 old)
{
int off = off_wrap & ~(1 << 15);
if (wrap != off_wrap >> 15)
off -= vq->num;
return vring_need_event(off, new, old);
}
Thanks
> +
> + return vring_need_event(off, new, old);
> +}
> +
> +static bool virtio_packed_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> +{
> + VRingPackedDescEvent e;
> + uint16_t old, new;
> + bool v;
> + VRingMemoryRegionCaches *caches;
> +
> + caches = vring_get_region_caches(vq);
> + vring_packed_event_read(vdev, &caches->driver, &e);
> +
> + old = vq->signalled_used;
> + new = vq->signalled_used = vq->used_idx;
> + v = vq->signalled_used_valid;
> + vq->signalled_used_valid = true;
> +
> + if (e.flags == RING_EVENT_FLAGS_DISABLE) {
> + return false;
> + } else if (e.flags == RING_EVENT_FLAGS_ENABLE) {
> + return true;
> + }
> +
> + return !v || vring_packed_need_event(vq, e.off_wrap, new, old);
> +}
> +
> +/* Called within rcu_read_lock(). */
> +static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> +{
> + if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
> + return virtio_packed_should_notify(vdev, vq);
> + } else {
> + return virtio_split_should_notify(vdev, vq);
> + }
> +}
> +
> void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq)
> {
> bool should_notify;
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 08/12] virtio: event suppression support for packed ring
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 08/12] virtio: event suppression support " wexu
2018-10-15 6:55 ` Jason Wang
@ 2018-10-15 6:59 ` Jason Wang
2018-10-15 8:20 ` Wei Xu
1 sibling, 1 reply; 36+ messages in thread
From: Jason Wang @ 2018-10-15 6:59 UTC (permalink / raw)
To: wexu, qemu-devel; +Cc: tiwei.bie, jfreimann, maxime.coquelin
On 2018年10月11日 22:08, wexu@redhat.com wrote:
> From: Wei Xu <wexu@redhat.com>
>
> Signed-off-by: Wei Xu <wexu@redhat.com>
> ---
> hw/virtio/virtio.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 123 insertions(+), 3 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index d12a7e3..1d25776 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -241,6 +241,30 @@ static void vring_desc_read(VirtIODevice *vdev, VRingDesc *desc,
> virtio_tswap16s(vdev, &desc->next);
> }
>
> +static void vring_packed_event_read(VirtIODevice *vdev,
> + MemoryRegionCache *cache, VRingPackedDescEvent *e)
> +{
> + address_space_read_cached(cache, 0, e, sizeof(*e));
> + virtio_tswap16s(vdev, &e->off_wrap);
> + virtio_tswap16s(vdev, &e->flags);
> +}
> +
> +static void vring_packed_off_wrap_write(VirtIODevice *vdev,
> + MemoryRegionCache *cache, uint16_t off_wrap)
> +{
> + virtio_tswap16s(vdev, &off_wrap);
> + address_space_write_cached(cache, 0, &off_wrap, sizeof(off_wrap));
> + address_space_cache_invalidate(cache, 0, sizeof(off_wrap));
> +}
> +
> +static void vring_packed_flags_write(VirtIODevice *vdev,
> + MemoryRegionCache *cache, uint16_t flags)
> +{
> + virtio_tswap16s(vdev, &flags);
> + address_space_write_cached(cache, sizeof(uint16_t), &flags, sizeof(flags));
> + address_space_cache_invalidate(cache, sizeof(uint16_t), sizeof(flags));
> +}
> +
> static VRingMemoryRegionCaches *vring_get_region_caches(struct VirtQueue *vq)
> {
> VRingMemoryRegionCaches *caches = atomic_rcu_read(&vq->vring.caches);
> @@ -347,7 +371,7 @@ static inline void vring_set_avail_event(VirtQueue *vq, uint16_t val)
> address_space_cache_invalidate(&caches->used, pa, sizeof(val));
> }
>
> -void virtio_queue_set_notification(VirtQueue *vq, int enable)
> +static void virtio_queue_set_notification_split(VirtQueue *vq, int enable)
> {
> vq->notification = enable;
>
> @@ -370,6 +394,51 @@ void virtio_queue_set_notification(VirtQueue *vq, int enable)
> rcu_read_unlock();
> }
>
> +static void virtio_queue_set_notification_packed(VirtQueue *vq, int enable)
> +{
> + VRingPackedDescEvent e;
> + VRingMemoryRegionCaches *caches;
> +
> + rcu_read_lock();
> + caches = vring_get_region_caches(vq);
> + vring_packed_event_read(vq->vdev, &caches->device, &e);
> +
> + if (!enable) {
> + e.flags = RING_EVENT_FLAGS_DISABLE;
> + goto out;
> + }
> +
> + e.flags = RING_EVENT_FLAGS_ENABLE;
> + if (virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
> + uint16_t off_wrap = vq->event_idx | vq->event_wrap_counter << 15;
Btw, why not just use shadow_avail_idx here?
Thanks
> +
> + vring_packed_off_wrap_write(vq->vdev, &caches->device, off_wrap);
> + /* Make sure off_wrap is wrote before flags */
> + smp_wmb();
> +
> + e.flags = RING_EVENT_FLAGS_DESC;
> + }
> +
> +out:
> + vring_packed_flags_write(vq->vdev, &caches->device, e.flags);
> + rcu_read_unlock();
> +}
> +
> +void virtio_queue_set_notification(VirtQueue *vq, int enable)
> +{
> + vq->notification = enable;
> +
> + if (!vq->vring.desc) {
> + return;
> + }
> +
> + if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> + virtio_queue_set_notification_packed(vq, enable);
> + } else {
> + virtio_queue_set_notification_split(vq, enable);
> + }
> +}
> +
> int virtio_queue_ready(VirtQueue *vq)
> {
> return vq->vring.avail != 0;
> @@ -2103,8 +2172,7 @@ static void virtio_set_isr(VirtIODevice *vdev, int value)
> }
> }
>
> -/* Called within rcu_read_lock(). */
> -static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> +static bool virtio_split_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> {
> uint16_t old, new;
> bool v;
> @@ -2127,6 +2195,58 @@ static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> return !v || vring_need_event(vring_get_used_event(vq), new, old);
> }
>
> +static bool vring_packed_need_event(VirtQueue *vq, uint16_t off_wrap,
> + uint16_t new, uint16_t old)
> +{
> + bool wrap = vq->event_wrap_counter;
> + int off = off_wrap & ~(1 << 15);
> +
> + if (new < old) {
> + new += vq->vring.num;
> + wrap ^= 1;
> + }
> +
> + if (wrap != off_wrap >> 15) {
> + off += vq->vring.num;
> + }
> +
> + return vring_need_event(off, new, old);
> +}
> +
> +static bool virtio_packed_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> +{
> + VRingPackedDescEvent e;
> + uint16_t old, new;
> + bool v;
> + VRingMemoryRegionCaches *caches;
> +
> + caches = vring_get_region_caches(vq);
> + vring_packed_event_read(vdev, &caches->driver, &e);
> +
> + old = vq->signalled_used;
> + new = vq->signalled_used = vq->used_idx;
> + v = vq->signalled_used_valid;
> + vq->signalled_used_valid = true;
> +
> + if (e.flags == RING_EVENT_FLAGS_DISABLE) {
> + return false;
> + } else if (e.flags == RING_EVENT_FLAGS_ENABLE) {
> + return true;
> + }
> +
> + return !v || vring_packed_need_event(vq, e.off_wrap, new, old);
> +}
> +
> +/* Called within rcu_read_lock(). */
> +static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> +{
> + if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
> + return virtio_packed_should_notify(vdev, vq);
> + } else {
> + return virtio_split_should_notify(vdev, vq);
> + }
> +}
> +
> void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq)
> {
> bool should_notify;
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 08/12] virtio: event suppression support for packed ring
2018-10-15 6:59 ` Jason Wang
@ 2018-10-15 8:20 ` Wei Xu
2018-10-15 9:11 ` Jason Wang
0 siblings, 1 reply; 36+ messages in thread
From: Wei Xu @ 2018-10-15 8:20 UTC (permalink / raw)
To: Jason Wang; +Cc: qemu-devel, tiwei.bie, jfreimann, maxime.coquelin
On Mon, Oct 15, 2018 at 02:59:48PM +0800, Jason Wang wrote:
>
>
> On 2018年10月11日 22:08, wexu@redhat.com wrote:
> >From: Wei Xu <wexu@redhat.com>
> >
> >Signed-off-by: Wei Xu <wexu@redhat.com>
> >---
> > hw/virtio/virtio.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++++++--
> > 1 file changed, 123 insertions(+), 3 deletions(-)
> >
> >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >index d12a7e3..1d25776 100644
> >--- a/hw/virtio/virtio.c
> >+++ b/hw/virtio/virtio.c
> >@@ -241,6 +241,30 @@ static void vring_desc_read(VirtIODevice *vdev, VRingDesc *desc,
> > virtio_tswap16s(vdev, &desc->next);
> > }
> >+static void vring_packed_event_read(VirtIODevice *vdev,
> >+ MemoryRegionCache *cache, VRingPackedDescEvent *e)
> >+{
> >+ address_space_read_cached(cache, 0, e, sizeof(*e));
> >+ virtio_tswap16s(vdev, &e->off_wrap);
> >+ virtio_tswap16s(vdev, &e->flags);
> >+}
> >+
> >+static void vring_packed_off_wrap_write(VirtIODevice *vdev,
> >+ MemoryRegionCache *cache, uint16_t off_wrap)
> >+{
> >+ virtio_tswap16s(vdev, &off_wrap);
> >+ address_space_write_cached(cache, 0, &off_wrap, sizeof(off_wrap));
> >+ address_space_cache_invalidate(cache, 0, sizeof(off_wrap));
> >+}
> >+
> >+static void vring_packed_flags_write(VirtIODevice *vdev,
> >+ MemoryRegionCache *cache, uint16_t flags)
> >+{
> >+ virtio_tswap16s(vdev, &flags);
> >+ address_space_write_cached(cache, sizeof(uint16_t), &flags, sizeof(flags));
> >+ address_space_cache_invalidate(cache, sizeof(uint16_t), sizeof(flags));
> >+}
> >+
> > static VRingMemoryRegionCaches *vring_get_region_caches(struct VirtQueue *vq)
> > {
> > VRingMemoryRegionCaches *caches = atomic_rcu_read(&vq->vring.caches);
> >@@ -347,7 +371,7 @@ static inline void vring_set_avail_event(VirtQueue *vq, uint16_t val)
> > address_space_cache_invalidate(&caches->used, pa, sizeof(val));
> > }
> >-void virtio_queue_set_notification(VirtQueue *vq, int enable)
> >+static void virtio_queue_set_notification_split(VirtQueue *vq, int enable)
> > {
> > vq->notification = enable;
> >@@ -370,6 +394,51 @@ void virtio_queue_set_notification(VirtQueue *vq, int enable)
> > rcu_read_unlock();
> > }
> >+static void virtio_queue_set_notification_packed(VirtQueue *vq, int enable)
> >+{
> >+ VRingPackedDescEvent e;
> >+ VRingMemoryRegionCaches *caches;
> >+
> >+ rcu_read_lock();
> >+ caches = vring_get_region_caches(vq);
> >+ vring_packed_event_read(vq->vdev, &caches->device, &e);
> >+
> >+ if (!enable) {
> >+ e.flags = RING_EVENT_FLAGS_DISABLE;
> >+ goto out;
> >+ }
> >+
> >+ e.flags = RING_EVENT_FLAGS_ENABLE;
> >+ if (virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
> >+ uint16_t off_wrap = vq->event_idx | vq->event_wrap_counter << 15;
>
> Btw, why not just use shadow_avail_idx here?
It is nice to do that but an issue here is that it is 'shadow_avail_idx' for
Rx but 'used_idx' for Tx when setting up for a kick, haven't figured out a
clear fix because it helps easier migration part work, any idea?
Wei
>
> Thanks
>
> >+
> >+ vring_packed_off_wrap_write(vq->vdev, &caches->device, off_wrap);
> >+ /* Make sure off_wrap is wrote before flags */
> >+ smp_wmb();
> >+
> >+ e.flags = RING_EVENT_FLAGS_DESC;
> >+ }
> >+
> >+out:
> >+ vring_packed_flags_write(vq->vdev, &caches->device, e.flags);
> >+ rcu_read_unlock();
> >+}
> >+
> >+void virtio_queue_set_notification(VirtQueue *vq, int enable)
> >+{
> >+ vq->notification = enable;
> >+
> >+ if (!vq->vring.desc) {
> >+ return;
> >+ }
> >+
> >+ if (virtio_vdev_has_feature(vq->vdev, VIRTIO_F_RING_PACKED)) {
> >+ virtio_queue_set_notification_packed(vq, enable);
> >+ } else {
> >+ virtio_queue_set_notification_split(vq, enable);
> >+ }
> >+}
> >+
> > int virtio_queue_ready(VirtQueue *vq)
> > {
> > return vq->vring.avail != 0;
> >@@ -2103,8 +2172,7 @@ static void virtio_set_isr(VirtIODevice *vdev, int value)
> > }
> > }
> >-/* Called within rcu_read_lock(). */
> >-static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> >+static bool virtio_split_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> > {
> > uint16_t old, new;
> > bool v;
> >@@ -2127,6 +2195,58 @@ static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> > return !v || vring_need_event(vring_get_used_event(vq), new, old);
> > }
> >+static bool vring_packed_need_event(VirtQueue *vq, uint16_t off_wrap,
> >+ uint16_t new, uint16_t old)
> >+{
> >+ bool wrap = vq->event_wrap_counter;
> >+ int off = off_wrap & ~(1 << 15);
> >+
> >+ if (new < old) {
> >+ new += vq->vring.num;
> >+ wrap ^= 1;
> >+ }
> >+
> >+ if (wrap != off_wrap >> 15) {
> >+ off += vq->vring.num;
> >+ }
> >+
> >+ return vring_need_event(off, new, old);
> >+}
> >+
> >+static bool virtio_packed_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> >+{
> >+ VRingPackedDescEvent e;
> >+ uint16_t old, new;
> >+ bool v;
> >+ VRingMemoryRegionCaches *caches;
> >+
> >+ caches = vring_get_region_caches(vq);
> >+ vring_packed_event_read(vdev, &caches->driver, &e);
> >+
> >+ old = vq->signalled_used;
> >+ new = vq->signalled_used = vq->used_idx;
> >+ v = vq->signalled_used_valid;
> >+ vq->signalled_used_valid = true;
> >+
> >+ if (e.flags == RING_EVENT_FLAGS_DISABLE) {
> >+ return false;
> >+ } else if (e.flags == RING_EVENT_FLAGS_ENABLE) {
> >+ return true;
> >+ }
> >+
> >+ return !v || vring_packed_need_event(vq, e.off_wrap, new, old);
> >+}
> >+
> >+/* Called within rcu_read_lock(). */
> >+static bool virtio_should_notify(VirtIODevice *vdev, VirtQueue *vq)
> >+{
> >+ if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
> >+ return virtio_packed_should_notify(vdev, vq);
> >+ } else {
> >+ return virtio_split_should_notify(vdev, vq);
> >+ }
> >+}
> >+
> > void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue *vq)
> > {
> > bool should_notify;
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 08/12] virtio: event suppression support for packed ring
2018-10-15 8:20 ` Wei Xu
@ 2018-10-15 9:11 ` Jason Wang
0 siblings, 0 replies; 36+ messages in thread
From: Jason Wang @ 2018-10-15 9:11 UTC (permalink / raw)
To: Wei Xu; +Cc: maxime.coquelin, jfreimann, qemu-devel, tiwei.bie
On 2018年10月15日 16:20, Wei Xu wrote:
>>> +static void virtio_queue_set_notification_packed(VirtQueue *vq, int enable)
>>> +{
>>> + VRingPackedDescEvent e;
>>> + VRingMemoryRegionCaches *caches;
>>> +
>>> + rcu_read_lock();
>>> + caches = vring_get_region_caches(vq);
>>> + vring_packed_event_read(vq->vdev, &caches->device, &e);
>>> +
>>> + if (!enable) {
>>> + e.flags = RING_EVENT_FLAGS_DISABLE;
>>> + goto out;
>>> + }
>>> +
>>> + e.flags = RING_EVENT_FLAGS_ENABLE;
>>> + if (virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX)) {
>>> + uint16_t off_wrap = vq->event_idx | vq->event_wrap_counter << 15;
>> Btw, why not just use shadow_avail_idx here?
> It is nice to do that but an issue here is that it is 'shadow_avail_idx' for
> Rx but 'used_idx' for Tx when setting up for a kick, haven't figured out a
> clear fix because it helps easier migration part work, any idea?
>
> Wei
>
I'm not sure I get the point here. Why need to care about used_idx when
enabling guest kick?
Thanks
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [[RFC v3 09/12] virtio-net: fill head desc after done all in a chain
2018-10-11 14:08 [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support wexu
` (7 preceding siblings ...)
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 08/12] virtio: event suppression support " wexu
@ 2018-10-11 14:08 ` wexu
2018-10-15 7:45 ` Jason Wang
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 10/12] virtio: packed ring feature bit for userspace backend wexu
` (3 subsequent siblings)
12 siblings, 1 reply; 36+ messages in thread
From: wexu @ 2018-10-11 14:08 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, tiwei.bie, wexu, jfreimann, maxime.coquelin
From: Wei Xu <wexu@redhat.com>
With the support of marking a descriptor used/unused in 'flags'
field for 1.1, the current way of filling a chained descriptors
does not work since driver side may get the wrong 'num_buffer'
information in case of the head descriptor has been filled in
while the subsequent ones are still in processing in device side.
This patch fills the head one after done all the others one.
Signed-off-by: Wei Xu <wexu@redhat.com>
---
hw/net/virtio-net.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 4bdd5b8..186c86cd2 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1198,6 +1198,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
struct virtio_net_hdr_mrg_rxbuf mhdr;
unsigned mhdr_cnt = 0;
size_t offset, i, guest_offset;
+ VirtQueueElement head;
+ int head_len = 0;
if (!virtio_net_can_receive(nc)) {
return -1;
@@ -1275,7 +1277,13 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
}
/* signal other side */
- virtqueue_fill(q->rx_vq, elem, total, i++);
+ if (i == 0) {
+ head_len = total;
+ head = *elem;
+ } else {
+ virtqueue_fill(q->rx_vq, elem, len, i);
+ }
+ i++;
g_free(elem);
}
@@ -1286,6 +1294,7 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
&mhdr.num_buffers, sizeof mhdr.num_buffers);
}
+ virtqueue_fill(q->rx_vq, &head, head_len, 0);
virtqueue_flush(q->rx_vq, i);
virtio_notify(vdev, q->rx_vq);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 09/12] virtio-net: fill head desc after done all in a chain
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 09/12] virtio-net: fill head desc after done all in a chain wexu
@ 2018-10-15 7:45 ` Jason Wang
2018-10-15 8:03 ` Wei Xu
0 siblings, 1 reply; 36+ messages in thread
From: Jason Wang @ 2018-10-15 7:45 UTC (permalink / raw)
To: wexu, qemu-devel; +Cc: tiwei.bie, jfreimann, maxime.coquelin
On 2018年10月11日 22:08, wexu@redhat.com wrote:
> From: Wei Xu <wexu@redhat.com>
>
> With the support of marking a descriptor used/unused in 'flags'
> field for 1.1, the current way of filling a chained descriptors
> does not work since driver side may get the wrong 'num_buffer'
> information in case of the head descriptor has been filled in
> while the subsequent ones are still in processing in device side.
>
> This patch fills the head one after done all the others one.
>
> Signed-off-by: Wei Xu <wexu@redhat.com>
> ---
> hw/net/virtio-net.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index 4bdd5b8..186c86cd2 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1198,6 +1198,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> struct virtio_net_hdr_mrg_rxbuf mhdr;
> unsigned mhdr_cnt = 0;
> size_t offset, i, guest_offset;
> + VirtQueueElement head;
> + int head_len = 0;
>
> if (!virtio_net_can_receive(nc)) {
> return -1;
> @@ -1275,7 +1277,13 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> }
>
> /* signal other side */
> - virtqueue_fill(q->rx_vq, elem, total, i++);
> + if (i == 0) {
> + head_len = total;
> + head = *elem;
> + } else {
> + virtqueue_fill(q->rx_vq, elem, len, i);
> + }
> + i++;
> g_free(elem);
> }
>
> @@ -1286,6 +1294,7 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> &mhdr.num_buffers, sizeof mhdr.num_buffers);
> }
>
> + virtqueue_fill(q->rx_vq, &head, head_len, 0);
It's not a good idea to fix API in device implementation. Let's
introduce new API and fix it there.
E.g virtqueue_fill_n() and update the flag of first elem at the last step.
Thanks
> virtqueue_flush(q->rx_vq, i);
> virtio_notify(vdev, q->rx_vq);
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 09/12] virtio-net: fill head desc after done all in a chain
2018-10-15 7:45 ` Jason Wang
@ 2018-10-15 8:03 ` Wei Xu
2018-10-15 8:05 ` Jason Wang
0 siblings, 1 reply; 36+ messages in thread
From: Wei Xu @ 2018-10-15 8:03 UTC (permalink / raw)
To: Jason Wang; +Cc: qemu-devel, maxime.coquelin, jfreimann, tiwei.bie
On Mon, Oct 15, 2018 at 03:45:46PM +0800, Jason Wang wrote:
>
>
> On 2018年10月11日 22:08, wexu@redhat.com wrote:
> >From: Wei Xu <wexu@redhat.com>
> >
> >With the support of marking a descriptor used/unused in 'flags'
> >field for 1.1, the current way of filling a chained descriptors
> >does not work since driver side may get the wrong 'num_buffer'
> >information in case of the head descriptor has been filled in
> >while the subsequent ones are still in processing in device side.
> >
> >This patch fills the head one after done all the others one.
> >
> >Signed-off-by: Wei Xu <wexu@redhat.com>
> >---
> > hw/net/virtio-net.c | 11 ++++++++++-
> > 1 file changed, 10 insertions(+), 1 deletion(-)
> >
> >diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> >index 4bdd5b8..186c86cd2 100644
> >--- a/hw/net/virtio-net.c
> >+++ b/hw/net/virtio-net.c
> >@@ -1198,6 +1198,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> > struct virtio_net_hdr_mrg_rxbuf mhdr;
> > unsigned mhdr_cnt = 0;
> > size_t offset, i, guest_offset;
> >+ VirtQueueElement head;
> >+ int head_len = 0;
> > if (!virtio_net_can_receive(nc)) {
> > return -1;
> >@@ -1275,7 +1277,13 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> > }
> > /* signal other side */
> >- virtqueue_fill(q->rx_vq, elem, total, i++);
> >+ if (i == 0) {
> >+ head_len = total;
> >+ head = *elem;
> >+ } else {
> >+ virtqueue_fill(q->rx_vq, elem, len, i);
> >+ }
> >+ i++;
> > g_free(elem);
> > }
> >@@ -1286,6 +1294,7 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
> > &mhdr.num_buffers, sizeof mhdr.num_buffers);
> > }
> >+ virtqueue_fill(q->rx_vq, &head, head_len, 0);
>
> It's not a good idea to fix API in device implementation. Let's introduce
> new API and fix it there.
>
> E.g virtqueue_fill_n() and update the flag of first elem at the last step.
OK, I haven't considered about the other devices so far.
Wei
>
> Thanks
>
> > virtqueue_flush(q->rx_vq, i);
> > virtio_notify(vdev, q->rx_vq);
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 09/12] virtio-net: fill head desc after done all in a chain
2018-10-15 8:03 ` Wei Xu
@ 2018-10-15 8:05 ` Jason Wang
0 siblings, 0 replies; 36+ messages in thread
From: Jason Wang @ 2018-10-15 8:05 UTC (permalink / raw)
To: Wei Xu; +Cc: maxime.coquelin, jfreimann, qemu-devel, tiwei.bie
On 2018年10月15日 16:03, Wei Xu wrote:
> On Mon, Oct 15, 2018 at 03:45:46PM +0800, Jason Wang wrote:
>>
>> On 2018年10月11日 22:08, wexu@redhat.com wrote:
>>> From: Wei Xu <wexu@redhat.com>
>>>
>>> With the support of marking a descriptor used/unused in 'flags'
>>> field for 1.1, the current way of filling a chained descriptors
>>> does not work since driver side may get the wrong 'num_buffer'
>>> information in case of the head descriptor has been filled in
>>> while the subsequent ones are still in processing in device side.
>>>
>>> This patch fills the head one after done all the others one.
>>>
>>> Signed-off-by: Wei Xu <wexu@redhat.com>
>>> ---
>>> hw/net/virtio-net.c | 11 ++++++++++-
>>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
>>> index 4bdd5b8..186c86cd2 100644
>>> --- a/hw/net/virtio-net.c
>>> +++ b/hw/net/virtio-net.c
>>> @@ -1198,6 +1198,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
>>> struct virtio_net_hdr_mrg_rxbuf mhdr;
>>> unsigned mhdr_cnt = 0;
>>> size_t offset, i, guest_offset;
>>> + VirtQueueElement head;
>>> + int head_len = 0;
>>> if (!virtio_net_can_receive(nc)) {
>>> return -1;
>>> @@ -1275,7 +1277,13 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
>>> }
>>> /* signal other side */
>>> - virtqueue_fill(q->rx_vq, elem, total, i++);
>>> + if (i == 0) {
>>> + head_len = total;
>>> + head = *elem;
>>> + } else {
>>> + virtqueue_fill(q->rx_vq, elem, len, i);
>>> + }
>>> + i++;
>>> g_free(elem);
>>> }
>>> @@ -1286,6 +1294,7 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
>>> &mhdr.num_buffers, sizeof mhdr.num_buffers);
>>> }
>>> + virtqueue_fill(q->rx_vq, &head, head_len, 0);
>> It's not a good idea to fix API in device implementation. Let's introduce
>> new API and fix it there.
>>
>> E.g virtqueue_fill_n() and update the flag of first elem at the last step.
> OK, I haven't considered about the other devices so far.
It will be an issue if they use virtqueue_fill() directly.
Thanks
>
> Wei
>
>> Thanks
>>
>>> virtqueue_flush(q->rx_vq, i);
>>> virtio_notify(vdev, q->rx_vq);
>>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [[RFC v3 10/12] virtio: packed ring feature bit for userspace backend
2018-10-11 14:08 [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support wexu
` (8 preceding siblings ...)
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 09/12] virtio-net: fill head desc after done all in a chain wexu
@ 2018-10-11 14:08 ` wexu
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 11/12] virtio: enable packed ring via a new command line wexu
` (2 subsequent siblings)
12 siblings, 0 replies; 36+ messages in thread
From: wexu @ 2018-10-11 14:08 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, tiwei.bie, wexu, jfreimann, maxime.coquelin
From: Wei Xu <wexu@redhat.com>
Signed-off-by: Wei Xu <wexu@redhat.com>
---
hw/net/vhost_net.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index e037db6..fb4b18f 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -78,6 +78,7 @@ static const int user_feature_bits[] = {
VIRTIO_NET_F_MRG_RXBUF,
VIRTIO_NET_F_MTU,
VIRTIO_F_IOMMU_PLATFORM,
+ VIRTIO_F_RING_PACKED,
/* This bit implies RARP isn't sent by QEMU out of band */
VIRTIO_NET_F_GUEST_ANNOUNCE,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [[RFC v3 11/12] virtio: enable packed ring via a new command line
2018-10-11 14:08 [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support wexu
` (9 preceding siblings ...)
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 10/12] virtio: packed ring feature bit for userspace backend wexu
@ 2018-10-11 14:08 ` wexu
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 12/12] virtio: feature vhost-net support for packed ring wexu
2018-11-21 14:39 ` [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support Tiwei Bie
12 siblings, 0 replies; 36+ messages in thread
From: wexu @ 2018-10-11 14:08 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, tiwei.bie, wexu, jfreimann, maxime.coquelin
From: Wei Xu <wexu@redhat.com>
only userspace virtio net backend has been supported by
the CLI so far.
(cherry picked from commit 0b3ec96f4a9402cca467c40353066e57608ac6b6)
Signed-off-by: Wei Xu <wexu@redhat.com>
(cherry picked from commit a1a3b85f00299ccc6f4bc819abe470da88059fb7)
Signed-off-by: Wei Xu <wexu@redhat.com>
---
include/hw/virtio/virtio.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index e323e76..9af8839 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -264,7 +264,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
DEFINE_PROP_BIT64("any_layout", _state, _field, \
VIRTIO_F_ANY_LAYOUT, true), \
DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
- VIRTIO_F_IOMMU_PLATFORM, false)
+ VIRTIO_F_IOMMU_PLATFORM, false), \
+ DEFINE_PROP_BIT64("ring_packed", _state, _field, \
+ VIRTIO_F_RING_PACKED, false)
hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
hwaddr virtio_queue_get_avail_addr(VirtIODevice *vdev, int n);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [[RFC v3 12/12] virtio: feature vhost-net support for packed ring
2018-10-11 14:08 [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support wexu
` (10 preceding siblings ...)
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 11/12] virtio: enable packed ring via a new command line wexu
@ 2018-10-11 14:08 ` wexu
2018-10-15 7:50 ` Jason Wang
2018-11-21 13:03 ` Maxime Coquelin
2018-11-21 14:39 ` [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support Tiwei Bie
12 siblings, 2 replies; 36+ messages in thread
From: wexu @ 2018-10-11 14:08 UTC (permalink / raw)
To: qemu-devel; +Cc: jasowang, tiwei.bie, wexu, jfreimann, maxime.coquelin
From: Wei Xu <wexu@redhat.com>
(cherry picked from commit 305a2c4640c15c5717245067ab937fd10f478ee6)
Signed-off-by: Wei Xu <wexu@redhat.com>
(cherry picked from commit 46476dae6f44c6fef8802a4a0ac7d0d79fe399e3)
Signed-off-by: Wei Xu <wexu@redhat.com>
---
hw/virtio/vhost.c | 3 +++
hw/virtio/virtio.c | 4 ++++
include/hw/virtio/virtio.h | 1 +
3 files changed, 8 insertions(+)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 9df2da3..de06d55 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -974,6 +974,9 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
}
state.num = virtio_queue_get_last_avail_idx(vdev, idx);
+ if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
+ state.num |= ((int)virtio_queue_packed_get_wc(vdev, idx)) << 31;
+ }
r = dev->vhost_ops->vhost_set_vring_base(dev, &state);
if (r) {
VHOST_OPS_DEBUG("vhost_set_vring_base failed");
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 1d25776..2a90163 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2894,6 +2894,10 @@ void virtio_init(VirtIODevice *vdev, const char *name,
vdev->use_guest_notifier_mask = true;
}
+bool virtio_queue_packed_get_wc(VirtIODevice *vdev, int n)
+{
+ return vdev->vq[n].avail_wrap_counter;
+}
hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n)
{
return vdev->vq[n].vring.desc;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 9af8839..0bb3be5 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -295,6 +295,7 @@ void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
VirtIOHandleAIOOutput handle_output);
VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector);
VirtQueue *virtio_vector_next_queue(VirtQueue *vq);
+bool virtio_queue_packed_get_wc(VirtIODevice *vdev, int n);
static inline void virtio_add_feature(uint64_t *features, unsigned int fbit)
{
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 12/12] virtio: feature vhost-net support for packed ring
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 12/12] virtio: feature vhost-net support for packed ring wexu
@ 2018-10-15 7:50 ` Jason Wang
2018-10-15 8:11 ` Wei Xu
2018-11-21 13:03 ` Maxime Coquelin
1 sibling, 1 reply; 36+ messages in thread
From: Jason Wang @ 2018-10-15 7:50 UTC (permalink / raw)
To: wexu, qemu-devel; +Cc: tiwei.bie, jfreimann, maxime.coquelin
On 2018年10月11日 22:08, wexu@redhat.com wrote:
> From: Wei Xu <wexu@redhat.com>
>
> (cherry picked from commit 305a2c4640c15c5717245067ab937fd10f478ee6)
> Signed-off-by: Wei Xu <wexu@redhat.com>
> (cherry picked from commit 46476dae6f44c6fef8802a4a0ac7d0d79fe399e3)
> Signed-off-by: Wei Xu <wexu@redhat.com>
The cherry-pick tag looks odd.
> ---
> hw/virtio/vhost.c | 3 +++
> hw/virtio/virtio.c | 4 ++++
> include/hw/virtio/virtio.h | 1 +
> 3 files changed, 8 insertions(+)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 9df2da3..de06d55 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -974,6 +974,9 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
> }
>
> state.num = virtio_queue_get_last_avail_idx(vdev, idx);
> + if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
> + state.num |= ((int)virtio_queue_packed_get_wc(vdev, idx)) << 31;
> + }
We decide to use bit 15 instead.
And please refer the recent discussion for the agreement.
Thanks
> r = dev->vhost_ops->vhost_set_vring_base(dev, &state);
> if (r) {
> VHOST_OPS_DEBUG("vhost_set_vring_base failed");
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 1d25776..2a90163 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2894,6 +2894,10 @@ void virtio_init(VirtIODevice *vdev, const char *name,
> vdev->use_guest_notifier_mask = true;
> }
>
> +bool virtio_queue_packed_get_wc(VirtIODevice *vdev, int n)
> +{
> + return vdev->vq[n].avail_wrap_counter;
> +}
> hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n)
> {
> return vdev->vq[n].vring.desc;
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 9af8839..0bb3be5 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -295,6 +295,7 @@ void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
> VirtIOHandleAIOOutput handle_output);
> VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector);
> VirtQueue *virtio_vector_next_queue(VirtQueue *vq);
> +bool virtio_queue_packed_get_wc(VirtIODevice *vdev, int n);
>
> static inline void virtio_add_feature(uint64_t *features, unsigned int fbit)
> {
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 12/12] virtio: feature vhost-net support for packed ring
2018-10-15 7:50 ` Jason Wang
@ 2018-10-15 8:11 ` Wei Xu
0 siblings, 0 replies; 36+ messages in thread
From: Wei Xu @ 2018-10-15 8:11 UTC (permalink / raw)
To: Jason Wang; +Cc: qemu-devel, maxime.coquelin, jfreimann, tiwei.bie
On Mon, Oct 15, 2018 at 03:50:21PM +0800, Jason Wang wrote:
>
>
> On 2018年10月11日 22:08, wexu@redhat.com wrote:
> >From: Wei Xu <wexu@redhat.com>
> >
> >(cherry picked from commit 305a2c4640c15c5717245067ab937fd10f478ee6)
> >Signed-off-by: Wei Xu <wexu@redhat.com>
> >(cherry picked from commit 46476dae6f44c6fef8802a4a0ac7d0d79fe399e3)
> >Signed-off-by: Wei Xu <wexu@redhat.com>
>
> The cherry-pick tag looks odd.
My bad.
>
> >---
> > hw/virtio/vhost.c | 3 +++
> > hw/virtio/virtio.c | 4 ++++
> > include/hw/virtio/virtio.h | 1 +
> > 3 files changed, 8 insertions(+)
> >
> >diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> >index 9df2da3..de06d55 100644
> >--- a/hw/virtio/vhost.c
> >+++ b/hw/virtio/vhost.c
> >@@ -974,6 +974,9 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
> > }
> > state.num = virtio_queue_get_last_avail_idx(vdev, idx);
> >+ if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
> >+ state.num |= ((int)virtio_queue_packed_get_wc(vdev, idx)) << 31;
> >+ }
>
> We decide to use bit 15 instead.
>
> And please refer the recent discussion for the agreement.
OK.
>
> Thanks
>
> > r = dev->vhost_ops->vhost_set_vring_base(dev, &state);
> > if (r) {
> > VHOST_OPS_DEBUG("vhost_set_vring_base failed");
> >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >index 1d25776..2a90163 100644
> >--- a/hw/virtio/virtio.c
> >+++ b/hw/virtio/virtio.c
> >@@ -2894,6 +2894,10 @@ void virtio_init(VirtIODevice *vdev, const char *name,
> > vdev->use_guest_notifier_mask = true;
> > }
> >+bool virtio_queue_packed_get_wc(VirtIODevice *vdev, int n)
> >+{
> >+ return vdev->vq[n].avail_wrap_counter;
> >+}
> > hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n)
> > {
> > return vdev->vq[n].vring.desc;
> >diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> >index 9af8839..0bb3be5 100644
> >--- a/include/hw/virtio/virtio.h
> >+++ b/include/hw/virtio/virtio.h
> >@@ -295,6 +295,7 @@ void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
> > VirtIOHandleAIOOutput handle_output);
> > VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector);
> > VirtQueue *virtio_vector_next_queue(VirtQueue *vq);
> >+bool virtio_queue_packed_get_wc(VirtIODevice *vdev, int n);
> > static inline void virtio_add_feature(uint64_t *features, unsigned int fbit)
> > {
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [[RFC v3 12/12] virtio: feature vhost-net support for packed ring
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 12/12] virtio: feature vhost-net support for packed ring wexu
2018-10-15 7:50 ` Jason Wang
@ 2018-11-21 13:03 ` Maxime Coquelin
2018-11-22 3:46 ` Wei Xu
1 sibling, 1 reply; 36+ messages in thread
From: Maxime Coquelin @ 2018-11-21 13:03 UTC (permalink / raw)
To: wexu, qemu-devel; +Cc: jasowang, tiwei.bie, jfreimann
Hi Wei,
On 10/11/18 4:08 PM, wexu@redhat.com wrote:
> From: Wei Xu <wexu@redhat.com>
>
> (cherry picked from commit 305a2c4640c15c5717245067ab937fd10f478ee6)
> Signed-off-by: Wei Xu <wexu@redhat.com>
> (cherry picked from commit 46476dae6f44c6fef8802a4a0ac7d0d79fe399e3)
> Signed-off-by: Wei Xu <wexu@redhat.com>
> ---
> hw/virtio/vhost.c | 3 +++
> hw/virtio/virtio.c | 4 ++++
> include/hw/virtio/virtio.h | 1 +
> 3 files changed, 8 insertions(+)
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 9df2da3..de06d55 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -974,6 +974,9 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
> }
>
> state.num = virtio_queue_get_last_avail_idx(vdev, idx);
> + if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
> + state.num |= ((int)virtio_queue_packed_get_wc(vdev, idx)) << 31;
For next version, please note that we agreed to move the wrap counter
value at bit 15. DPDK vhost lib implemented it that way.
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [[RFC v3 12/12] virtio: feature vhost-net support for packed ring
2018-11-21 13:03 ` Maxime Coquelin
@ 2018-11-22 3:46 ` Wei Xu
0 siblings, 0 replies; 36+ messages in thread
From: Wei Xu @ 2018-11-22 3:46 UTC (permalink / raw)
To: Maxime Coquelin; +Cc: qemu-devel, jasowang, jfreimann, tiwei.bie
On Wed, Nov 21, 2018 at 02:03:59PM +0100, Maxime Coquelin wrote:
> Hi Wei,
>
> On 10/11/18 4:08 PM, wexu@redhat.com wrote:
> >From: Wei Xu <wexu@redhat.com>
> >
> >(cherry picked from commit 305a2c4640c15c5717245067ab937fd10f478ee6)
> >Signed-off-by: Wei Xu <wexu@redhat.com>
> >(cherry picked from commit 46476dae6f44c6fef8802a4a0ac7d0d79fe399e3)
> >Signed-off-by: Wei Xu <wexu@redhat.com>
> >---
> > hw/virtio/vhost.c | 3 +++
> > hw/virtio/virtio.c | 4 ++++
> > include/hw/virtio/virtio.h | 1 +
> > 3 files changed, 8 insertions(+)
> >
> >diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> >index 9df2da3..de06d55 100644
> >--- a/hw/virtio/vhost.c
> >+++ b/hw/virtio/vhost.c
> >@@ -974,6 +974,9 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
> > }
> > state.num = virtio_queue_get_last_avail_idx(vdev, idx);
> >+ if (virtio_vdev_has_feature(vdev, VIRTIO_F_RING_PACKED)) {
> >+ state.num |= ((int)virtio_queue_packed_get_wc(vdev, idx)) << 31;
>
> For next version, please note that we agreed to move the wrap counter
> value at bit 15. DPDK vhost lib implemented it that way.
Yes, I have revised it in my next version, thanks for remindering.
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support
2018-10-11 14:08 [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support wexu
` (11 preceding siblings ...)
2018-10-11 14:08 ` [Qemu-devel] [[RFC v3 12/12] virtio: feature vhost-net support for packed ring wexu
@ 2018-11-21 14:39 ` Tiwei Bie
2018-11-22 3:43 ` Wei Xu
12 siblings, 1 reply; 36+ messages in thread
From: Tiwei Bie @ 2018-11-21 14:39 UTC (permalink / raw)
To: wexu; +Cc: qemu-devel, jasowang, jfreimann, maxime.coquelin
Hi Wei,
FYI, the latest packed ring series for guest driver doesn't set
the _F_NEXT bit for indirect descriptors any more. So below hack
in guest driver is needed to make it work with this series:
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cd7e755484e3..42faea7d8cf8 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -980,6 +980,7 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
unsigned int i, n, err_idx;
u16 head, id;
dma_addr_t addr;
+ int c = 0;
head = vq->packed.next_avail_idx;
desc = alloc_indirect_packed(total_sg, gfp);
@@ -1001,8 +1002,9 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
if (vring_mapping_error(vq, addr))
goto unmap_release;
- desc[i].flags = cpu_to_le16(n < out_sgs ?
- 0 : VRING_DESC_F_WRITE);
+ desc[i].flags = cpu_to_le16((n < out_sgs ?
+ 0 : VRING_DESC_F_WRITE) |
+ (++c == total_sg ? 0 : VRING_DESC_F_NEXT));
desc[i].addr = cpu_to_le64(addr);
desc[i].len = cpu_to_le32(sg->length);
i++;
--
2.14.1
On Thu, Oct 11, 2018 at 10:08:23AM -0400, wexu@redhat.com wrote:
> From: Wei Xu <wexu@redhat.com>
>
> code base:
> https://github.com/Whishay/qemu.git
>
> Todo:
> - migration has not been support yet
>
> v2->v3
> - addressed performance issue
> - fixed feedback from v2
>
> v1->v2
> - sync to tiwei's v5
> - reuse memory cache function with 1.0
> - dropped detach patch and notification helper(04 & 05 in v1)
> - guest virtio-net driver unload/reload support
> - event suppression support(not tested)
> - addressed feedback from v1
>
> Wei Xu (12):
> virtio: introduce packed ring definitions
> virtio: redefine structure & memory cache for packed ring
> virtio: init memory cache for packed ring
> virtio: init wrap counter for packed ring
> virtio: init and desc empty check for packed ring
> virtio: get avail bytes check for packed ring
> virtio: fill/flush/pop for packed ring
> virtio: event suppression support for packed ring
> virtio-net: fill head desc after done all in a chain
> virtio: packed ring feature bit for userspace backend
> virtio: enable packed ring via a new command line
> virtio: feature vhost-net support for packed ring
>
> hw/net/vhost_net.c | 1 +
> hw/net/virtio-net.c | 11 +-
> hw/virtio/vhost.c | 19 +-
> hw/virtio/virtio.c | 685 +++++++++++++++++++++++--
> include/hw/virtio/virtio.h | 9 +-
> include/standard-headers/linux/virtio_config.h | 15 +
> include/standard-headers/linux/virtio_ring.h | 43 ++
> 7 files changed, 736 insertions(+), 47 deletions(-)
>
> --
> 1.8.3.1
>
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support
2018-11-21 14:39 ` [Qemu-devel] [RFC v3 00/12] packed ring virtio-net userspace backend support Tiwei Bie
@ 2018-11-22 3:43 ` Wei Xu
0 siblings, 0 replies; 36+ messages in thread
From: Wei Xu @ 2018-11-22 3:43 UTC (permalink / raw)
To: Tiwei Bie; +Cc: qemu-devel, jasowang, jfreimann, maxime.coquelin
On Wed, Nov 21, 2018 at 10:39:20PM +0800, Tiwei Bie wrote:
> Hi Wei,
>
> FYI, the latest packed ring series for guest driver doesn't set
> the _F_NEXT bit for indirect descriptors any more. So below hack
> in guest driver is needed to make it work with this series:
OK, will do a test, thanks.
Wei
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index cd7e755484e3..42faea7d8cf8 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -980,6 +980,7 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
> unsigned int i, n, err_idx;
> u16 head, id;
> dma_addr_t addr;
> + int c = 0;
>
> head = vq->packed.next_avail_idx;
> desc = alloc_indirect_packed(total_sg, gfp);
> @@ -1001,8 +1002,9 @@ static int virtqueue_add_indirect_packed(struct vring_virtqueue *vq,
> if (vring_mapping_error(vq, addr))
> goto unmap_release;
>
> - desc[i].flags = cpu_to_le16(n < out_sgs ?
> - 0 : VRING_DESC_F_WRITE);
> + desc[i].flags = cpu_to_le16((n < out_sgs ?
> + 0 : VRING_DESC_F_WRITE) |
> + (++c == total_sg ? 0 : VRING_DESC_F_NEXT));
> desc[i].addr = cpu_to_le64(addr);
> desc[i].len = cpu_to_le32(sg->length);
> i++;
> --
> 2.14.1
>
> On Thu, Oct 11, 2018 at 10:08:23AM -0400, wexu@redhat.com wrote:
> > From: Wei Xu <wexu@redhat.com>
> >
> > code base:
> > https://github.com/Whishay/qemu.git
> >
> > Todo:
> > - migration has not been support yet
> >
> > v2->v3
> > - addressed performance issue
> > - fixed feedback from v2
> >
> > v1->v2
> > - sync to tiwei's v5
> > - reuse memory cache function with 1.0
> > - dropped detach patch and notification helper(04 & 05 in v1)
> > - guest virtio-net driver unload/reload support
> > - event suppression support(not tested)
> > - addressed feedback from v1
> >
> > Wei Xu (12):
> > virtio: introduce packed ring definitions
> > virtio: redefine structure & memory cache for packed ring
> > virtio: init memory cache for packed ring
> > virtio: init wrap counter for packed ring
> > virtio: init and desc empty check for packed ring
> > virtio: get avail bytes check for packed ring
> > virtio: fill/flush/pop for packed ring
> > virtio: event suppression support for packed ring
> > virtio-net: fill head desc after done all in a chain
> > virtio: packed ring feature bit for userspace backend
> > virtio: enable packed ring via a new command line
> > virtio: feature vhost-net support for packed ring
> >
> > hw/net/vhost_net.c | 1 +
> > hw/net/virtio-net.c | 11 +-
> > hw/virtio/vhost.c | 19 +-
> > hw/virtio/virtio.c | 685 +++++++++++++++++++++++--
> > include/hw/virtio/virtio.h | 9 +-
> > include/standard-headers/linux/virtio_config.h | 15 +
> > include/standard-headers/linux/virtio_ring.h | 43 ++
> > 7 files changed, 736 insertions(+), 47 deletions(-)
> >
> > --
> > 1.8.3.1
> >
^ permalink raw reply [flat|nested] 36+ messages in thread