Re: [Qemu-devel] [PATCH] virtio: right size for virtio_queue_get_avail_size

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Wed,  2 Sep 2015 17:23:49 +0200
Pierre Morel <pmorel@linux.vnet.ibm.com> wrote:

> Being working on dataplane I notice something strange:
> 
> virtio_queue_get_avail_size() used a 64bit size index
> for the calculation of the available ring size.
> 
> It is quite strange but it did work with the old calculation
> of the avail ring, at most with performance penalty,
> and I wonder where I missed something.
> 
> This patch let use a 16bit size as defined in virtio_ring.h
> 
> Signed-off-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
> ---
>  hw/virtio/virtio.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 788b556..5c856eb 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1460,7 +1460,7 @@ hwaddr virtio_queue_get_desc_size(VirtIODevice *vdev, int n)
>  hwaddr virtio_queue_get_avail_size(VirtIODevice *vdev, int n)
>  {
>      return offsetof(VRingAvail, ring) +
> -        sizeof(uint64_t) * vdev->vq[n].vring.num;
> +        sizeof(uint16_t) * vdev->vq[n].vring.num;
>  }
> 
>  hwaddr virtio_queue_get_used_size(VirtIODevice *vdev, int n)

I'm wondering about the semantics of the _size() functions. Naively I
would expect (size of buffer) * (number of buffers). I think at least
vhost expects the {used,avail} indices in there as well? The
s390-virtio code seems not to expect the indices to be contained in the
size, though...