Re: [PATCH 09/10] vhost: add VHOST_SET_VRING_ENABLE support

[Stefan Hajnoczi]

On Thu, Nov 12, 2020 at 05:19:09PM -0600, Mike Christie wrote:
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 2f98b81..e953031 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1736,6 +1736,28 @@ static long vhost_vring_set_num_addr(struct vhost_dev 
> *d,
>  
>       return r;
>  }
> +
> +static long vhost_vring_set_enable(struct vhost_dev *d,
> +                                struct vhost_virtqueue *vq,
> +                                void __user *argp)
> +{
> +     struct vhost_vring_state s;
> +     int ret = 0;
> +
> +     if (vq->private_data)
> +             return -EBUSY;
> +
> +     if (copy_from_user(&s, argp, sizeof s))
> +             return -EFAULT;
> +
> +     if (s.num != 1 && s.num != 0)
> +             return -EINVAL;
> +
> +     if (d->ops && d->ops->enable_vring)
> +             ret = d->ops->enable_vring(vq, s.num);
> +     return ret;
> +}

Silently ignoring this ioctl on drivers that don't implement
d->ops->enable_vring() could be a problem. Userspace expects to be able
to enable/disable vqs, we can't just return 0 because the vq won't be
enabled/disabled as requested.

> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index a293f48..1279c09 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -158,6 +158,7 @@ struct vhost_msg_node {
>  
>  struct vhost_dev_ops {
>       int (*msg_handler)(struct vhost_dev *dev, struct vhost_iotlb_msg *msg);
> +     int (*enable_vring)(struct vhost_virtqueue *vq, bool enable);

Please add doc comments explaining what this callback needs to do and
the environment in which it is executed (locks that are held, etc).

signature.asc
Description: PGP signature