On Wed, Jun 29, 2022 at 04:25:41PM +0800, Xie Yongji
wrote:> Let's update api version to 1 since we introduced
> some new ioctls to support registering userspace
> memory for IOTLB.
>
> Signed-off-by: Xie Yongji <xieyongji at bytedance.com>
Adding new ioctls does not justify things like this.
Besides, adding UAPI then changing it is not nice
since it makes git bisect behave incorrectly.
> ---
> drivers/vdpa/vdpa_user/vduse_dev.c | 12 ++++++++++++
> include/uapi/linux/vduse.h | 8 +++++++-
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 7b2ea7612da9..2795785ca6a2 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1206,6 +1206,10 @@ static long vduse_dev_ioctl(struct file *file,
unsigned int cmd,
> case VDUSE_IOTLB_GET_INFO: {
> struct vduse_iotlb_info iotlb;
>
> + ret = -EPERM;
Almost for sure a wrong error code.
> + if (dev->api_version < 1)
> + break;
> +
> iotlb.bounce_iova = 0;
> iotlb.bounce_size = dev->domain->bounce_size;
>
Wait a second. so you are intentionally breaking any userspace
that called VDUSE_SET_API_VERSION with version 0?
Please don't.
> @@ -1219,6 +1223,10 @@ static long vduse_dev_ioctl(struct file *file,
unsigned int cmd,
> case VDUSE_IOTLB_REG_UMEM: {
> struct vduse_iotlb_umem umem;
>
> + ret = -EPERM;
> + if (dev->api_version < 1)
> + break;
> +
> ret = -EFAULT;
> if (copy_from_user(&umem, argp, sizeof(umem)))
> break;
> @@ -1230,6 +1238,10 @@ static long vduse_dev_ioctl(struct file *file,
unsigned int cmd,
> case VDUSE_IOTLB_DEREG_UMEM: {
> struct vduse_iotlb_umem umem;
>
> + ret = -EPERM;
> + if (dev->api_version < 1)
> + break;
> +
> ret = -EFAULT;
> if (copy_from_user(&umem, argp, sizeof(umem)))
> break;
> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> index 1b17391e228f..902ea19cd9e0 100644
> --- a/include/uapi/linux/vduse.h
> +++ b/include/uapi/linux/vduse.h
> @@ -8,7 +8,13 @@
>
> /* The ioctls for control device (/dev/vduse/control) */
>
> -#define VDUSE_API_VERSION 0
> +/*
> + * v0 -> v1:
> + * - Introduce VDUSE_IOTLB_GET_INFO ioctl
> + * - Introduce VDUSE_VDUSE_IOTLB_REG_UMEM ioctl
> + * - Introduce VDUSE_IOTLB_DEREG_UMEM ioctl
> + */
> +#define VDUSE_API_VERSION 1
>
> /*
> * Get the version of VDUSE API that kernel supported (VDUSE_API_VERSION).
> --
> 2.20.1