Re: [Qemu-devel] [PULL v3 26/28] vhost: drop legacy vring layout bits

[Greg Kurz]

On Tue, 15 Nov 2016 21:22:31 +0200
"Michael S. Tsirkin" <address@hidden> wrote:

> From: Greg Kurz <address@hidden>
> 
> The legacy vring layout is not used anymore as we use the separate
> mappings even for legacy devices.
> This patch simply removes it.
> 
> This also fixes a bug with virtio 1 devices when the vring descriptor table
> is mapped at a higher address than the used vring because the following
> function may return an insanely great value:
> 
> hwaddr virtio_queue_get_ring_size(VirtIODevice *vdev, int n)
> {
>     return vdev->vq[n].vring.used - vdev->vq[n].vring.desc +
>            virtio_queue_get_used_size(vdev, n);
> }
> 
> and the mapping fails.
> 

Michael,

I see the "vhost: adapt vhost_verify_ring_mappings() to virtio  1 ring layout"
patch got Cc'ed to stable... any reason for not doing the same with this patch
since it fixes the bug which motivated the whole series ?

Cheers.

--
Greg

> Signed-off-by: Greg Kurz <address@hidden>
> Reviewed-by: Cornelia Huck <address@hidden>
> Reviewed-by: Michael S. Tsirkin <address@hidden>
> Signed-off-by: Michael S. Tsirkin <address@hidden>
> ---
>  include/hw/virtio/vhost.h |  3 ---
>  hw/virtio/vhost.c         | 13 -------------
>  2 files changed, 16 deletions(-)
> 
> diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> index 56b567f..1fe5aad 100644
> --- a/include/hw/virtio/vhost.h
> +++ b/include/hw/virtio/vhost.h
> @@ -20,9 +20,6 @@ struct vhost_virtqueue {
>      unsigned avail_size;
>      unsigned long long used_phys;
>      unsigned used_size;
> -    void *ring;
> -    unsigned long long ring_phys;
> -    unsigned ring_size;
>      EventNotifier masked_notifier;
>  };
>  
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index d88d34a..30aee88 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -923,14 +923,6 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
>          goto fail_alloc_used;
>      }
>  
> -    vq->ring_size = s = l = virtio_queue_get_ring_size(vdev, idx);
> -    vq->ring_phys = a = virtio_queue_get_ring_addr(vdev, idx);
> -    vq->ring = cpu_physical_memory_map(a, &l, 1);
> -    if (!vq->ring || l != s) {
> -        r = -ENOMEM;
> -        goto fail_alloc_ring;
> -    }
> -
>      r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
>      if (r < 0) {
>          r = -errno;
> @@ -971,9 +963,6 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
>  fail_vector:
>  fail_kick:
>  fail_alloc:
> -    cpu_physical_memory_unmap(vq->ring, virtio_queue_get_ring_size(vdev, 
> idx),
> -                              0, 0);
> -fail_alloc_ring:
>      cpu_physical_memory_unmap(vq->used, virtio_queue_get_used_size(vdev, 
> idx),
>                                0, 0);
>  fail_alloc_used:
> @@ -1014,8 +1003,6 @@ static void vhost_virtqueue_stop(struct vhost_dev *dev,
>                                                  vhost_vq_index);
>      }
>  
> -    cpu_physical_memory_unmap(vq->ring, virtio_queue_get_ring_size(vdev, 
> idx),
> -                              0, virtio_queue_get_ring_size(vdev, idx));
>      cpu_physical_memory_unmap(vq->used, virtio_queue_get_used_size(vdev, 
> idx),
>                                1, virtio_queue_get_used_size(vdev, idx));
>      cpu_physical_memory_unmap(vq->avail, virtio_queue_get_avail_size(vdev, 
> idx),