Messages in this thread | | | Date | Wed, 29 Jun 2022 04:33:35 -0400 | From | "Michael S. Tsirkin" <> | Subject | Re: [PATCH 6/6] vduse: Update api version to 1 |
| |
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@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
| |