Re: [Qemu-devel] [PATCH 7/8] virtio: get avail bytes check for packed ring
On Wed, Apr 11, 2018 at 11:03:24AM +0800, Jason Wang wrote: > > > On 2018年04月04日 20:54, w...@redhat.com wrote: > >From: Wei Xu > > > >mostly as same as 1.0, copy it separately for > >prototype, need a refactoring. > > > >Signed-off-by: Wei Xu > >--- > > hw/virtio/virtio.c | 142 > > +++-- > > 1 file changed, 139 insertions(+), 3 deletions(-) > > > >diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > >index def07c6..cf726f3 100644 > >--- a/hw/virtio/virtio.c > >+++ b/hw/virtio/virtio.c > >@@ -836,9 +836,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_get_avail_bytes_split(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; > >@@ -961,6 +961,142 @@ err: > > goto done; > > } > >+static void virtqueue_get_avail_bytes_packed(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; > >+VRingDescPacked desc; > >+ > >+if (unlikely(!vq->packed.desc)) { > >+if (in_bytes) { > >+*in_bytes = 0; > >+} > >+if (out_bytes) { > >+*out_bytes = 0; > >+} > >+return; > >+} > >+ > >+rcu_read_lock(); > >+idx = vq->last_avail_idx; > >+total_bufs = in_total = out_total = 0; > >+ > >+max = vq->packed.num; > >+caches = vring_get_region_caches(vq); > >+if (caches->desc.len < max * sizeof(VRingDescPacked)) { > >+virtio_error(vdev, "Cannot map descriptor ring"); > >+goto err; > >+} > >+ > >+desc_cache = >desc; > >+vring_desc_read_packed(vdev, , desc_cache, idx); > >+while (is_desc_avail()) { > >+unsigned int num_bufs; > >+unsigned int i; > >+ > >+num_bufs = total_bufs; > >+ > >+if (desc.flags & VRING_DESC_F_INDIRECT) { > >+if (desc.len % sizeof(VRingDescPacked)) { > >+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(_desc_cache, > >+ vdev->dma_as, > >+ desc.addr, desc.len, false); > >+desc_cache = _desc_cache; > >+if (len < desc.len) { > >+virtio_error(vdev, "Cannot map indirect buffer"); > >+goto err; > >+} > >+ > >+max = desc.len / sizeof(VRingDescPacked); > >+num_bufs = i = 0; > >+vring_desc_read_packed(vdev, , desc_cache, i); > >+} > >+ > >+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 == _desc_cache) { > >+vring_desc_read_packed(vdev, , desc_cache, > >+ ++i % vq->packed.num); > >+} else { > >+vring_desc_read_packed(vdev, , desc_cache, > >+ ++idx % vq->packed.num); > >+} > > Need to make sure desc.flags was read before other fields, otherwise we may > get stale address. OK, need a barrier here, thanks. Wei > > Thanks > > >+} while (desc.flags & VRING_DESC_F_NEXT); > >+ > >+if (desc_cache == _desc_cache) { > >+address_space_cache_destroy(_desc_cache);
Re: [Qemu-devel] [PATCH 7/8] virtio: get avail bytes check for packed ring
On 2018年04月04日 20:54, w...@redhat.com wrote: From: Wei Xumostly as same as 1.0, copy it separately for prototype, need a refactoring. Signed-off-by: Wei Xu --- hw/virtio/virtio.c | 142 +++-- 1 file changed, 139 insertions(+), 3 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index def07c6..cf726f3 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -836,9 +836,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_get_avail_bytes_split(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; @@ -961,6 +961,142 @@ err: goto done; } +static void virtqueue_get_avail_bytes_packed(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; +VRingDescPacked desc; + +if (unlikely(!vq->packed.desc)) { +if (in_bytes) { +*in_bytes = 0; +} +if (out_bytes) { +*out_bytes = 0; +} +return; +} + +rcu_read_lock(); +idx = vq->last_avail_idx; +total_bufs = in_total = out_total = 0; + +max = vq->packed.num; +caches = vring_get_region_caches(vq); +if (caches->desc.len < max * sizeof(VRingDescPacked)) { +virtio_error(vdev, "Cannot map descriptor ring"); +goto err; +} + +desc_cache = >desc; +vring_desc_read_packed(vdev, , desc_cache, idx); +while (is_desc_avail()) { +unsigned int num_bufs; +unsigned int i; + +num_bufs = total_bufs; + +if (desc.flags & VRING_DESC_F_INDIRECT) { +if (desc.len % sizeof(VRingDescPacked)) { +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(_desc_cache, + vdev->dma_as, + desc.addr, desc.len, false); +desc_cache = _desc_cache; +if (len < desc.len) { +virtio_error(vdev, "Cannot map indirect buffer"); +goto err; +} + +max = desc.len / sizeof(VRingDescPacked); +num_bufs = i = 0; +vring_desc_read_packed(vdev, , desc_cache, i); +} + +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 == _desc_cache) { +vring_desc_read_packed(vdev, , desc_cache, + ++i % vq->packed.num); +} else { +vring_desc_read_packed(vdev, , desc_cache, + ++idx % vq->packed.num); +} Need to make sure desc.flags was read before other fields, otherwise we may get stale address. Thanks +} while (desc.flags & VRING_DESC_F_NEXT); + +if (desc_cache == _desc_cache) { +address_space_cache_destroy(_desc_cache); +total_bufs++; +/* We missed one step on for indirect desc */ +idx++; +} else { +total_bufs = num_bufs; +} + +desc_cache = >desc; +vring_desc_read_packed(vdev, , desc_cache, idx % vq->packed.num); +} + +done: +address_space_cache_destroy(_desc_cache); +if (in_bytes) { +*in_bytes = in_total; +} +if (out_bytes) { +*out_bytes = out_total; +}
[Qemu-devel] [PATCH 7/8] virtio: get avail bytes check for packed ring
From: Wei Xumostly as same as 1.0, copy it separately for prototype, need a refactoring. Signed-off-by: Wei Xu --- hw/virtio/virtio.c | 142 +++-- 1 file changed, 139 insertions(+), 3 deletions(-) diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index def07c6..cf726f3 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -836,9 +836,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_get_avail_bytes_split(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; @@ -961,6 +961,142 @@ err: goto done; } +static void virtqueue_get_avail_bytes_packed(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; +VRingDescPacked desc; + +if (unlikely(!vq->packed.desc)) { +if (in_bytes) { +*in_bytes = 0; +} +if (out_bytes) { +*out_bytes = 0; +} +return; +} + +rcu_read_lock(); +idx = vq->last_avail_idx; +total_bufs = in_total = out_total = 0; + +max = vq->packed.num; +caches = vring_get_region_caches(vq); +if (caches->desc.len < max * sizeof(VRingDescPacked)) { +virtio_error(vdev, "Cannot map descriptor ring"); +goto err; +} + +desc_cache = >desc; +vring_desc_read_packed(vdev, , desc_cache, idx); +while (is_desc_avail()) { +unsigned int num_bufs; +unsigned int i; + +num_bufs = total_bufs; + +if (desc.flags & VRING_DESC_F_INDIRECT) { +if (desc.len % sizeof(VRingDescPacked)) { +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(_desc_cache, + vdev->dma_as, + desc.addr, desc.len, false); +desc_cache = _desc_cache; +if (len < desc.len) { +virtio_error(vdev, "Cannot map indirect buffer"); +goto err; +} + +max = desc.len / sizeof(VRingDescPacked); +num_bufs = i = 0; +vring_desc_read_packed(vdev, , desc_cache, i); +} + +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 == _desc_cache) { +vring_desc_read_packed(vdev, , desc_cache, + ++i % vq->packed.num); +} else { +vring_desc_read_packed(vdev, , desc_cache, + ++idx % vq->packed.num); +} +} while (desc.flags & VRING_DESC_F_NEXT); + +if (desc_cache == _desc_cache) { +address_space_cache_destroy(_desc_cache); +total_bufs++; +/* We missed one step on for indirect desc */ +idx++; +} else { +total_bufs = num_bufs; +} + +desc_cache = >desc; +vring_desc_read_packed(vdev, , desc_cache, idx % vq->packed.num); +} + +done: +address_space_cache_destroy(_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, +