Re: [Qemu-devel] [PATCH 3/6] vhost: Update rings information for IOTLB earlier

[Michael S. Tsirkin]

On Thu, May 11, 2017 at 02:32:43PM +0200, Maxime Coquelin wrote:
> Vhost-kernel backend need to receive IOTLB entries for rings
> information early, but vhost-user need the same information
> earlier, before VHOST_USER_SET_VRING_ADDR is sent.

Weird. What does VHOST_USER_SET_VRING_ADDR have to do with it?

According to
        Starting and stopping rings
in vhost user spec, vhost user does not access
anything until ring is started and enabled.


> This patch also trigger IOTLB miss for all rings informations
> for robustness, even if in practice these adresses are on the
> same page.
> 
> Signed-off-by: Maxime Coquelin <address@hidden>
> ---
>  hw/virtio/vhost.c | 19 +++++++++++--------
>  1 file changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 748e331..817f6d0 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -799,7 +799,17 @@ static int vhost_virtqueue_set_addr(struct vhost_dev 
> *dev,
>          .log_guest_addr = vq->used_phys,
>          .flags = enable_log ? (1 << VHOST_VRING_F_LOG) : 0,
>      };
> -    int r = dev->vhost_ops->vhost_set_vring_addr(dev, &addr);
> +    int r;
> +
> +    /* Update rings information for IOTLB to work correctly,
> +     * vhost-kernel & vhost-user backends require for this.*/

Any requirements must be in the spec. Pls add them there.
Pls fix comment style as you move code.


> +    if (vhost_dev_has_iommu(dev)) {
> +        vhost_device_iotlb_miss(dev, addr.desc_user_addr, true);
> +        vhost_device_iotlb_miss(dev, addr.used_user_addr, true);
> +        vhost_device_iotlb_miss(dev, addr.avail_user_addr, true);
> +    }
> +
> +    r = dev->vhost_ops->vhost_set_vring_addr(dev, &addr);
>      if (r < 0) {
>          VHOST_OPS_DEBUG("vhost_set_vring_addr failed");
>          return -errno;
> @@ -1551,13 +1561,6 @@ int vhost_dev_start(struct vhost_dev *hdev, 
> VirtIODevice *vdev)
>  
>      if (vhost_dev_has_iommu(hdev)) {
>          hdev->vhost_ops->vhost_set_iotlb_callback(hdev, true);
> -
> -        /* Update used ring information for IOTLB to work correctly,
> -         * vhost-kernel code requires for this.*/
> -        for (i = 0; i < hdev->nvqs; ++i) {
> -            struct vhost_virtqueue *vq = hdev->vqs + i;
> -            vhost_device_iotlb_miss(hdev, vq->used_phys, true);
> -        }
>      }
>      return 0;
>  fail_log:
> -- 
> 2.9.3