| Message ID | 20230221034812.138051-17-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add vfio_device cdev for iommufd support | expand |
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Tuesday, February 21, 2023 11:48 AM > > + > +void vfio_device_cdev_close(struct vfio_device_file *df) > +{ > + struct vfio_device *device = df->device; > + > + mutex_lock(&device->dev_set->lock); > + if (!smp_load_acquire(&df->access_granted)) { there is no contention with another one changing this flag at this point so directly accessing it is fine. but actually should check device->open_count as v3 does. Otherwise the last error on copy_to_user() in ioctl_bind_iommufd() simply returns here given df->access_granted hasn't been set but .open_device() has been completed.
> From: Tian, Kevin <kevin.tian@intel.com> > Sent: Wednesday, February 22, 2023 3:40 PM > > > From: Liu, Yi L <yi.l.liu@intel.com> > > Sent: Tuesday, February 21, 2023 11:48 AM > > > > + > > +void vfio_device_cdev_close(struct vfio_device_file *df) > > +{ > > + struct vfio_device *device = df->device; > > + > > + mutex_lock(&device->dev_set->lock); > > + if (!smp_load_acquire(&df->access_granted)) { > > there is no contention with another one changing this flag at this > point so directly accessing it is fine. make sense. > but actually should check device->open_count as v3 does. Otherwise > the last error on copy_to_user() in ioctl_bind_iommufd() simply returns > here given df->access_granted hasn't been set but .open_device() > has been completed. If copy_to_user() failed, vfio_device_close() would be called in the error path. Then this close function just returns. Regards, Yi Liu
On Mon, Feb 20, 2023 at 07:48:09PM -0800, Yi Liu wrote: > This adds ioctl for userspace to bind device cdev fd to iommufd. > > VFIO_DEVICE_BIND_IOMMUFD: bind device to an iommufd, hence gain DMA > control provided by the iommufd. open_device > op is called after bind_iommufd op. > VFIO no iommu mode is indicated by passing > a negative iommufd value. > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > --- > drivers/vfio/device_cdev.c | 137 +++++++++++++++++++++++++++++++++++++ > drivers/vfio/vfio.h | 17 ++++- > drivers/vfio/vfio_main.c | 30 ++++++-- > include/linux/iommufd.h | 6 ++ > include/uapi/linux/vfio.h | 34 +++++++++ > 5 files changed, 219 insertions(+), 5 deletions(-) > > diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c > index 9e2c1ecaaf4f..be62f0a5687e 100644 > --- a/drivers/vfio/device_cdev.c > +++ b/drivers/vfio/device_cdev.c > @@ -3,6 +3,7 @@ > * Copyright (c) 2023 Intel Corporation. > */ > #include <linux/vfio.h> > +#include <linux/iommufd.h> > > #include "vfio.h" > > @@ -45,6 +46,142 @@ int vfio_device_fops_cdev_open(struct inode *inode, struct file *filep) > return ret; > } > > +static void vfio_device_get_kvm_safe(struct vfio_device_file *df) > +{ > + spin_lock(&df->kvm_ref_lock); > + if (!df->kvm) > + goto unlock; > + > + _vfio_device_get_kvm_safe(df->device, df->kvm); > + > +unlock: > + spin_unlock(&df->kvm_ref_lock); > +} > + > +void vfio_device_cdev_close(struct vfio_device_file *df) > +{ > + struct vfio_device *device = df->device; > + > + mutex_lock(&device->dev_set->lock); > + if (!smp_load_acquire(&df->access_granted)) { > + mutex_unlock(&device->dev_set->lock); > + return; > + } > + vfio_device_close(df); > + vfio_device_put_kvm(device); > + if (df->iommufd) > + iommufd_ctx_put(df->iommufd); > + mutex_unlock(&device->dev_set->lock); > + vfio_device_unblock_group(device); > +} > + > +long vfio_device_ioctl_bind_iommufd(struct vfio_device_file *df, > + unsigned long arg) > +{ > + struct vfio_device *device = df->device; > + struct vfio_device_bind_iommufd bind; > + struct iommufd_ctx *iommufd = NULL; > + struct fd f; > + unsigned long minsz; > + int ret; > + > + minsz = offsetofend(struct vfio_device_bind_iommufd, out_devid); > + > + if (copy_from_user(&bind, (void __user *)arg, minsz)) > + return -EFAULT; > + > + if (bind.argsz < minsz || bind.flags) > + return -EINVAL; > + > + if (!device->ops->bind_iommufd) > + return -ENODEV; > + > + ret = vfio_device_block_group(device); > + if (ret) > + return ret; > + > + mutex_lock(&device->dev_set->lock); > + /* > + * If already been bound to an iommufd, or already set noiommu > + * then fail it. > + */ > + if (df->iommufd || df->noiommu) { > + ret = -EINVAL; > + goto out_unlock; > + } > + > + /* iommufd < 0 means noiommu mode */ > + if (bind.iommufd < 0) { > + if (!capable(CAP_SYS_RAWIO)) { > + ret = -EPERM; > + goto out_unlock; > + } > + df->noiommu = true; > + } else { > + f = fdget(bind.iommufd); > + if (!f.file) { > + ret = -EBADF; > + goto out_unlock; > + } > + iommufd = iommufd_ctx_from_file(f.file); > + if (IS_ERR(iommufd)) { > + ret = PTR_ERR(iommufd); > + goto out_put_file; > + } > + } > + > + /* > + * Before the device open, get the KVM pointer currently > + * associated with the device file (if there is) and obtain a > + * reference. This reference is held until device closed. Save > + * the pointer in the device for use by drivers. > + */ > + vfio_device_get_kvm_safe(df); > + > + df->iommufd = iommufd; > + ret = vfio_device_open(df, &bind.out_devid, NULL); > + if (ret) > + goto out_put_kvm; > + > + ret = copy_to_user((void __user *)arg + > + offsetofend(struct vfio_device_bind_iommufd, iommufd), > + &bind.out_devid, > + sizeof(bind.out_devid)) ? -EFAULT : 0; > + if (ret) > + goto out_close_device; > + > + if (iommufd) > + fdput(f); > + else if (df->noiommu) > + dev_warn(device->dev, "vfio-noiommu device used by user " > + "(%s:%d)\n", current->comm, task_pid_nr(current)); > + > + /* > + * Paired with smp_load_acquire() in vfio_device_fops::ioctl/ > + * read/write/mmap > + */ > + smp_store_release(&df->access_granted, true); > + mutex_unlock(&device->dev_set->lock); > + > + return 0; > + > +out_close_device: > + vfio_device_close(df); > +out_put_kvm: > + df->iommufd = NULL; > + df->noiommu = false; > + vfio_device_put_kvm(device); > +out_put_file: > + if (iommufd) { > + iommufd_ctx_put(iommufd); > + fdput(f); even if iommufd is NULL, still need to fdput(f) if f.file is true, right? > + } > +out_unlock: > + mutex_unlock(&device->dev_set->lock); > + vfio_device_unblock_group(device); > + return ret; > +} > +
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Wednesday, February 22, 2023 3:44 PM > > > From: Tian, Kevin <kevin.tian@intel.com> > > Sent: Wednesday, February 22, 2023 3:40 PM > > > > > From: Liu, Yi L <yi.l.liu@intel.com> > > > Sent: Tuesday, February 21, 2023 11:48 AM > > > > > > + > > > +void vfio_device_cdev_close(struct vfio_device_file *df) > > > +{ > > > + struct vfio_device *device = df->device; > > > + > > > + mutex_lock(&device->dev_set->lock); > > > + if (!smp_load_acquire(&df->access_granted)) { > > > > there is no contention with another one changing this flag at this > > point so directly accessing it is fine. > > make sense. > > > but actually should check device->open_count as v3 does. Otherwise > > the last error on copy_to_user() in ioctl_bind_iommufd() simply returns > > here given df->access_granted hasn't been set but .open_device() > > has been completed. > > If copy_to_user() failed, vfio_device_close() would be called in the > error path. Then this close function just returns. > yeah, I misread it.
> From: Zhao, Yan Y <yan.y.zhao@intel.com> > Sent: Wednesday, February 22, 2023 3:53 PM > > On Mon, Feb 20, 2023 at 07:48:09PM -0800, Yi Liu wrote: > > This adds ioctl for userspace to bind device cdev fd to iommufd. > > > > VFIO_DEVICE_BIND_IOMMUFD: bind device to an iommufd, hence gain > DMA > > control provided by the iommufd. open_device > > op is called after bind_iommufd op. > > VFIO no iommu mode is indicated by passing > > a negative iommufd value. > > > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > > --- > > drivers/vfio/device_cdev.c | 137 > +++++++++++++++++++++++++++++++++++++ > > drivers/vfio/vfio.h | 17 ++++- > > drivers/vfio/vfio_main.c | 30 ++++++-- > > include/linux/iommufd.h | 6 ++ > > include/uapi/linux/vfio.h | 34 +++++++++ > > 5 files changed, 219 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c > > index 9e2c1ecaaf4f..be62f0a5687e 100644 > > --- a/drivers/vfio/device_cdev.c > > +++ b/drivers/vfio/device_cdev.c > > @@ -3,6 +3,7 @@ > > * Copyright (c) 2023 Intel Corporation. > > */ > > #include <linux/vfio.h> > > +#include <linux/iommufd.h> > > > > #include "vfio.h" > > > > @@ -45,6 +46,142 @@ int vfio_device_fops_cdev_open(struct inode > *inode, struct file *filep) > > return ret; > > } > > > > +static void vfio_device_get_kvm_safe(struct vfio_device_file *df) > > +{ > > + spin_lock(&df->kvm_ref_lock); > > + if (!df->kvm) > > + goto unlock; > > + > > + _vfio_device_get_kvm_safe(df->device, df->kvm); > > + > > +unlock: > > + spin_unlock(&df->kvm_ref_lock); > > +} > > + > > +void vfio_device_cdev_close(struct vfio_device_file *df) > > +{ > > + struct vfio_device *device = df->device; > > + > > + mutex_lock(&device->dev_set->lock); > > + if (!smp_load_acquire(&df->access_granted)) { > > + mutex_unlock(&device->dev_set->lock); > > + return; > > + } > > + vfio_device_close(df); > > + vfio_device_put_kvm(device); > > + if (df->iommufd) > > + iommufd_ctx_put(df->iommufd); > > + mutex_unlock(&device->dev_set->lock); > > + vfio_device_unblock_group(device); > > +} > > + > > +long vfio_device_ioctl_bind_iommufd(struct vfio_device_file *df, > > + unsigned long arg) > > +{ > > + struct vfio_device *device = df->device; > > + struct vfio_device_bind_iommufd bind; > > + struct iommufd_ctx *iommufd = NULL; > > + struct fd f; > > + unsigned long minsz; > > + int ret; > > + > > + minsz = offsetofend(struct vfio_device_bind_iommufd, out_devid); > > + > > + if (copy_from_user(&bind, (void __user *)arg, minsz)) > > + return -EFAULT; > > + > > + if (bind.argsz < minsz || bind.flags) > > + return -EINVAL; > > + > > + if (!device->ops->bind_iommufd) > > + return -ENODEV; > > + > > + ret = vfio_device_block_group(device); > > + if (ret) > > + return ret; > > + > > + mutex_lock(&device->dev_set->lock); > > + /* > > + * If already been bound to an iommufd, or already set noiommu > > + * then fail it. > > + */ > > + if (df->iommufd || df->noiommu) { > > + ret = -EINVAL; > > + goto out_unlock; > > + } > > + > > + /* iommufd < 0 means noiommu mode */ > > + if (bind.iommufd < 0) { > > + if (!capable(CAP_SYS_RAWIO)) { > > + ret = -EPERM; > > + goto out_unlock; > > + } > > + df->noiommu = true; > > + } else { > > + f = fdget(bind.iommufd); > > + if (!f.file) { > > + ret = -EBADF; > > + goto out_unlock; > > + } > > + iommufd = iommufd_ctx_from_file(f.file); > > + if (IS_ERR(iommufd)) { > > + ret = PTR_ERR(iommufd); > > + goto out_put_file; > > + } > > + } > > + > > + /* > > + * Before the device open, get the KVM pointer currently > > + * associated with the device file (if there is) and obtain a > > + * reference. This reference is held until device closed. Save > > + * the pointer in the device for use by drivers. > > + */ > > + vfio_device_get_kvm_safe(df); > > + > > + df->iommufd = iommufd; > > + ret = vfio_device_open(df, &bind.out_devid, NULL); > > + if (ret) > > + goto out_put_kvm; > > + > > + ret = copy_to_user((void __user *)arg + > > + offsetofend(struct vfio_device_bind_iommufd, > iommufd), > > + &bind.out_devid, > > + sizeof(bind.out_devid)) ? -EFAULT : 0; > > + if (ret) > > + goto out_close_device; > > + > > + if (iommufd) > > + fdput(f); Here check f.file > > + else if (df->noiommu) > > + dev_warn(device->dev, "vfio-noiommu device used by user > " > > + "(%s:%d)\n", current->comm, > task_pid_nr(current)); > > + > > + /* > > + * Paired with smp_load_acquire() in vfio_device_fops::ioctl/ > > + * read/write/mmap > > + */ > > + smp_store_release(&df->access_granted, true); > > + mutex_unlock(&device->dev_set->lock); > > + > > + return 0; > > + > > +out_close_device: > > + vfio_device_close(df); > > +out_put_kvm: > > + df->iommufd = NULL; > > + df->noiommu = false; > > + vfio_device_put_kvm(device); > > +out_put_file: > > + if (iommufd) { > > + iommufd_ctx_put(iommufd); > > + fdput(f); > even if iommufd is NULL, still need to fdput(f) if f.file is true, right? Yes. it is. And also another line in above.
On Wed, Feb 22, 2023 at 07:44:12AM +0000, Liu, Yi L wrote: > > From: Tian, Kevin <kevin.tian@intel.com> > > Sent: Wednesday, February 22, 2023 3:40 PM > > > > > From: Liu, Yi L <yi.l.liu@intel.com> > > > Sent: Tuesday, February 21, 2023 11:48 AM > > > > > > + > > > +void vfio_device_cdev_close(struct vfio_device_file *df) > > > +{ > > > + struct vfio_device *device = df->device; > > > + > > > + mutex_lock(&device->dev_set->lock); > > > + if (!smp_load_acquire(&df->access_granted)) { > > > > there is no contention with another one changing this flag at this > > point so directly accessing it is fine. > > make sense. Have to use READ_ONCE though Jason
On Wed, Feb 22, 2023 at 08:59:51AM -0400, Jason Gunthorpe wrote: > On Wed, Feb 22, 2023 at 07:44:12AM +0000, Liu, Yi L wrote: > > > From: Tian, Kevin <kevin.tian@intel.com> > > > Sent: Wednesday, February 22, 2023 3:40 PM > > > > > > > From: Liu, Yi L <yi.l.liu@intel.com> > > > > Sent: Tuesday, February 21, 2023 11:48 AM > > > > > > > > + > > > > +void vfio_device_cdev_close(struct vfio_device_file *df) > > > > +{ > > > > + struct vfio_device *device = df->device; > > > > + > > > > + mutex_lock(&device->dev_set->lock); > > > > + if (!smp_load_acquire(&df->access_granted)) { > > > > > > there is no contention with another one changing this flag at this > > > point so directly accessing it is fine. > > > > make sense. > > Have to use READ_ONCE though > Just a curious question: given df->access_granted is now written with device->dev_set->lock held and also read with this lock held in vfio_device_cdev_close(), is READ_ONCE still required? And what about df->iommufd ? Thanks Yan
On Fri, Feb 24, 2023 at 12:58:22PM +0800, Yan Zhao wrote: > On Wed, Feb 22, 2023 at 08:59:51AM -0400, Jason Gunthorpe wrote: > > On Wed, Feb 22, 2023 at 07:44:12AM +0000, Liu, Yi L wrote: > > > > From: Tian, Kevin <kevin.tian@intel.com> > > > > Sent: Wednesday, February 22, 2023 3:40 PM > > > > > > > > > From: Liu, Yi L <yi.l.liu@intel.com> > > > > > Sent: Tuesday, February 21, 2023 11:48 AM > > > > > > > > > > + > > > > > +void vfio_device_cdev_close(struct vfio_device_file *df) > > > > > +{ > > > > > + struct vfio_device *device = df->device; > > > > > + > > > > > + mutex_lock(&device->dev_set->lock); > > > > > + if (!smp_load_acquire(&df->access_granted)) { > > > > > > > > there is no contention with another one changing this flag at this > > > > point so directly accessing it is fine. > > > > > > make sense. > > > > Have to use READ_ONCE though > > > Just a curious question: > given df->access_granted is now written with device->dev_set->lock held and > also read with this lock held in vfio_device_cdev_close(), is READ_ONCE > still required? And what about df->iommufd ? No, if the writer is under a lock held by the reader then it is always OK to use naked read. Best to document it with a comment Jason
On Fri, Feb 24, 2023 at 10:31:35AM -0400, Jason Gunthorpe wrote: > On Fri, Feb 24, 2023 at 12:58:22PM +0800, Yan Zhao wrote: > > On Wed, Feb 22, 2023 at 08:59:51AM -0400, Jason Gunthorpe wrote: > > > On Wed, Feb 22, 2023 at 07:44:12AM +0000, Liu, Yi L wrote: > > > > > From: Tian, Kevin <kevin.tian@intel.com> > > > > > Sent: Wednesday, February 22, 2023 3:40 PM > > > > > > > > > > > From: Liu, Yi L <yi.l.liu@intel.com> > > > > > > Sent: Tuesday, February 21, 2023 11:48 AM > > > > > > > > > > > > + > > > > > > +void vfio_device_cdev_close(struct vfio_device_file *df) > > > > > > +{ > > > > > > + struct vfio_device *device = df->device; > > > > > > + > > > > > > + mutex_lock(&device->dev_set->lock); > > > > > > + if (!smp_load_acquire(&df->access_granted)) { > > > > > > > > > > there is no contention with another one changing this flag at this > > > > > point so directly accessing it is fine. > > > > > > > > make sense. > > > > > > Have to use READ_ONCE though > > > > > Just a curious question: > > given df->access_granted is now written with device->dev_set->lock held and > > also read with this lock held in vfio_device_cdev_close(), is READ_ONCE > > still required? And what about df->iommufd ? > > No, if the writer is under a lock held by the reader then it is always > OK to use naked read. Best to document it with a comment > Thanks for the clarification!
diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c index 9e2c1ecaaf4f..be62f0a5687e 100644 --- a/drivers/vfio/device_cdev.c +++ b/drivers/vfio/device_cdev.c @@ -3,6 +3,7 @@ * Copyright (c) 2023 Intel Corporation. */ #include <linux/vfio.h> +#include <linux/iommufd.h> #include "vfio.h" @@ -45,6 +46,142 @@ int vfio_device_fops_cdev_open(struct inode *inode, struct file *filep) return ret; } +static void vfio_device_get_kvm_safe(struct vfio_device_file *df) +{ + spin_lock(&df->kvm_ref_lock); + if (!df->kvm) + goto unlock; + + _vfio_device_get_kvm_safe(df->device, df->kvm); + +unlock: + spin_unlock(&df->kvm_ref_lock); +} + +void vfio_device_cdev_close(struct vfio_device_file *df) +{ + struct vfio_device *device = df->device; + + mutex_lock(&device->dev_set->lock); + if (!smp_load_acquire(&df->access_granted)) { + mutex_unlock(&device->dev_set->lock); + return; + } + vfio_device_close(df); + vfio_device_put_kvm(device); + if (df->iommufd) + iommufd_ctx_put(df->iommufd); + mutex_unlock(&device->dev_set->lock); + vfio_device_unblock_group(device); +} + +long vfio_device_ioctl_bind_iommufd(struct vfio_device_file *df, + unsigned long arg) +{ + struct vfio_device *device = df->device; + struct vfio_device_bind_iommufd bind; + struct iommufd_ctx *iommufd = NULL; + struct fd f; + unsigned long minsz; + int ret; + + minsz = offsetofend(struct vfio_device_bind_iommufd, out_devid); + + if (copy_from_user(&bind, (void __user *)arg, minsz)) + return -EFAULT; + + if (bind.argsz < minsz || bind.flags) + return -EINVAL; + + if (!device->ops->bind_iommufd) + return -ENODEV; + + ret = vfio_device_block_group(device); + if (ret) + return ret; + + mutex_lock(&device->dev_set->lock); + /* + * If already been bound to an iommufd, or already set noiommu + * then fail it. + */ + if (df->iommufd || df->noiommu) { + ret = -EINVAL; + goto out_unlock; + } + + /* iommufd < 0 means noiommu mode */ + if (bind.iommufd < 0) { + if (!capable(CAP_SYS_RAWIO)) { + ret = -EPERM; + goto out_unlock; + } + df->noiommu = true; + } else { + f = fdget(bind.iommufd); + if (!f.file) { + ret = -EBADF; + goto out_unlock; + } + iommufd = iommufd_ctx_from_file(f.file); + if (IS_ERR(iommufd)) { + ret = PTR_ERR(iommufd); + goto out_put_file; + } + } + + /* + * Before the device open, get the KVM pointer currently + * associated with the device file (if there is) and obtain a + * reference. This reference is held until device closed. Save + * the pointer in the device for use by drivers. + */ + vfio_device_get_kvm_safe(df); + + df->iommufd = iommufd; + ret = vfio_device_open(df, &bind.out_devid, NULL); + if (ret) + goto out_put_kvm; + + ret = copy_to_user((void __user *)arg + + offsetofend(struct vfio_device_bind_iommufd, iommufd), + &bind.out_devid, + sizeof(bind.out_devid)) ? -EFAULT : 0; + if (ret) + goto out_close_device; + + if (iommufd) + fdput(f); + else if (df->noiommu) + dev_warn(device->dev, "vfio-noiommu device used by user " + "(%s:%d)\n", current->comm, task_pid_nr(current)); + + /* + * Paired with smp_load_acquire() in vfio_device_fops::ioctl/ + * read/write/mmap + */ + smp_store_release(&df->access_granted, true); + mutex_unlock(&device->dev_set->lock); + + return 0; + +out_close_device: + vfio_device_close(df); +out_put_kvm: + df->iommufd = NULL; + df->noiommu = false; + vfio_device_put_kvm(device); +out_put_file: + if (iommufd) { + iommufd_ctx_put(iommufd); + fdput(f); + } +out_unlock: + mutex_unlock(&device->dev_set->lock); + vfio_device_unblock_group(device); + return ret; +} + static char *vfio_device_devnode(const struct device *dev, umode_t *mode) { return kasprintf(GFP_KERNEL, "vfio/devices/%s", dev_name(dev)); diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h index 15734dbba9fb..40b54b0ce993 100644 --- a/drivers/vfio/vfio.h +++ b/drivers/vfio/vfio.h @@ -23,7 +23,9 @@ struct vfio_device_file { bool access_granted; spinlock_t kvm_ref_lock; /* protect kvm field */ struct kvm *kvm; - struct iommufd_ctx *iommufd; /* protected by struct vfio_device_set::lock */ + /* protected by struct vfio_device_set::lock */ + struct iommufd_ctx *iommufd; + bool noiommu; }; void vfio_device_put_registration(struct vfio_device *device); @@ -259,6 +261,9 @@ static inline void vfio_device_del(struct vfio_device *device) void vfio_init_device_cdev(struct vfio_device *device); int vfio_device_fops_cdev_open(struct inode *inode, struct file *filep); +void vfio_device_cdev_close(struct vfio_device_file *df); +long vfio_device_ioctl_bind_iommufd(struct vfio_device_file *df, + unsigned long arg); int vfio_cdev_init(struct class *device_class); void vfio_cdev_cleanup(void); #else @@ -282,6 +287,16 @@ static inline int vfio_device_fops_cdev_open(struct inode *inode, return 0; } +static inline void vfio_device_cdev_close(struct vfio_device_file *df) +{ +} + +static inline long vfio_device_ioctl_bind_iommufd(struct vfio_device_file *df, + unsigned long arg) +{ + return -EOPNOTSUPP; +} + static inline int vfio_cdev_init(struct class *device_class) { return 0; diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c index 48616ef97dfd..5e9a4fd800be 100644 --- a/drivers/vfio/vfio_main.c +++ b/drivers/vfio/vfio_main.c @@ -37,6 +37,7 @@ #include <linux/interval_tree.h> #include <linux/iova_bitmap.h> #include <linux/iommufd.h> +#include <uapi/linux/iommufd.h> #include "vfio.h" #define DRIVER_VERSION "0.3" @@ -422,16 +423,32 @@ static int vfio_device_first_open(struct vfio_device_file *df, { struct vfio_device *device = df->device; struct iommufd_ctx *iommufd = df->iommufd; - int ret; + int ret = 0; lockdep_assert_held(&device->dev_set->lock); + if (WARN_ON(iommufd && df->noiommu)) + return -EINVAL; + if (!try_module_get(device->dev->driver->owner)) return -ENODEV; + /* + * For group/container path, iommufd pointer is NULL when comes + * into this helper. Its noiommu support is handled by + * vfio_device_group_use_iommu() + * + * For iommufd compat mode, iommufd pointer here is a valid value. + * Its noiommu support is in vfio_iommufd_bind(). + * + * For device cdev path, iommufd pointer here is a valid value for + * normal cases, but it is NULL if it's noiommu. Check df->noiommu + * to differentiate cdev noiommu from the group/container path which + * also passes NULL iommufd pointer in. If set then do nothing. + */ if (iommufd) ret = vfio_iommufd_bind(device, iommufd, dev_id, pt_id); - else + else if (!df->noiommu) ret = vfio_device_group_use_iommu(device); if (ret) goto err_module_put; @@ -446,7 +463,7 @@ static int vfio_device_first_open(struct vfio_device_file *df, err_unuse_iommu: if (iommufd) vfio_iommufd_unbind(device); - else + else if (!df->noiommu) vfio_device_group_unuse_iommu(device); err_module_put: module_put(device->dev->driver->owner); @@ -464,7 +481,7 @@ static void vfio_device_last_close(struct vfio_device_file *df) device->ops->close_device(device); if (iommufd) vfio_iommufd_unbind(device); - else + else if (!df->noiommu) vfio_device_group_unuse_iommu(device); module_put(device->dev->driver->owner); } @@ -549,6 +566,8 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep) if (!df->is_cdev_device) vfio_device_group_close(df); + else + vfio_device_cdev_close(df); vfio_device_put_registration(device); @@ -1122,6 +1141,9 @@ static long vfio_device_fops_unl_ioctl(struct file *filep, struct vfio_device *device = df->device; int ret; + if (cmd == VFIO_DEVICE_BIND_IOMMUFD) + return vfio_device_ioctl_bind_iommufd(df, arg); + /* Paired with smp_store_release() in vfio_device_open() */ if (!smp_load_acquire(&df->access_granted)) return -EINVAL; diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h index 650d45629647..9672cf839687 100644 --- a/include/linux/iommufd.h +++ b/include/linux/iommufd.h @@ -17,6 +17,12 @@ struct iommufd_ctx; struct iommufd_access; struct file; +/* + * iommufd core init xarray with flags==XA_FLAGS_ALLOC1, so valid + * ID starts from 1. + */ +#define IOMMUFD_INVALID_ID 0 + struct iommufd_device *iommufd_device_bind(struct iommufd_ctx *ictx, struct device *dev, u32 *id); void iommufd_device_unbind(struct iommufd_device *idev); diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h index 0552e8dcf0cb..1fe69173e739 100644 --- a/include/uapi/linux/vfio.h +++ b/include/uapi/linux/vfio.h @@ -194,6 +194,40 @@ struct vfio_group_status { /* --------------- IOCTLs for DEVICE file descriptors --------------- */ +/* + * VFIO_DEVICE_BIND_IOMMUFD - _IOR(VFIO_TYPE, VFIO_BASE + 19, + * struct vfio_device_bind_iommufd) + * + * Bind a vfio_device to the specified iommufd. + * + * The user should provide a device cookie when calling this ioctl. The + * cookie is carried only in event e.g. I/O fault reported to userspace + * via iommufd. The user should use devid returned by this ioctl to mark + * the target device in other ioctls (e.g. capability query via iommufd). + * + * User is not allowed to access the device before the binding operation + * is completed. + * + * Unbind is automatically conducted when device fd is closed. + * + * @argsz: user filled size of this data. + * @flags: reserved for future extension. + * @dev_cookie: a per device cookie provided by userspace. + * @iommufd: iommufd to bind. a negative value means noiommu. + * @out_devid: the device id generated by this bind. + * + * Return: 0 on success, -errno on failure. + */ +struct vfio_device_bind_iommufd { + __u32 argsz; + __u32 flags; + __aligned_u64 dev_cookie; + __s32 iommufd; + __u32 out_devid; +}; + +#define VFIO_DEVICE_BIND_IOMMUFD _IO(VFIO_TYPE, VFIO_BASE + 19) + /** * VFIO_DEVICE_GET_INFO - _IOR(VFIO_TYPE, VFIO_BASE + 7, * struct vfio_device_info)
This adds ioctl for userspace to bind device cdev fd to iommufd. VFIO_DEVICE_BIND_IOMMUFD: bind device to an iommufd, hence gain DMA control provided by the iommufd. open_device op is called after bind_iommufd op. VFIO no iommu mode is indicated by passing a negative iommufd value. Signed-off-by: Yi Liu <yi.l.liu@intel.com> --- drivers/vfio/device_cdev.c | 137 +++++++++++++++++++++++++++++++++++++ drivers/vfio/vfio.h | 17 ++++- drivers/vfio/vfio_main.c | 30 ++++++-- include/linux/iommufd.h | 6 ++ include/uapi/linux/vfio.h | 34 +++++++++ 5 files changed, 219 insertions(+), 5 deletions(-)