On Fri, Jun 02, 2023 at 05:16:47AM -0700, 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.
>
> Tested-by: Yanting Jiang <yanting.jiang@xxxxxxxxx>
> Tested-by: Shameer Kolothum <shameerali.kolothum.thodi@xxxxxxxxxx>
> Tested-by: Terrence Xu <terrence.xu@xxxxxxxxx>
> Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
> ---
> drivers/vfio/device_cdev.c | 123 +++++++++++++++++++++++++++++++++++++
> drivers/vfio/vfio.h | 13 ++++
> drivers/vfio/vfio_main.c | 5 ++
> include/linux/vfio.h | 3 +-
> include/uapi/linux/vfio.h | 27 ++++++++
> 5 files changed, 170 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c
> index 1c640016a824..a4498ddbe774 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"
>
> @@ -44,6 +45,128 @@ 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)
> + _vfio_device_get_kvm_safe(df->device, df->kvm);
> + spin_unlock(&df->kvm_ref_lock);
> +}
I'm surprised symbol_get() can be called from a spinlock, but it sure
looks like it can..
Also moving the if kvm is null test into _vfio_device_get_kvm_safe()
will save a few lines.
Also shouldn't be called _vfio_device...
> +void vfio_df_cdev_close(struct vfio_device_file *df)
> +{
> + struct vfio_device *device = df->device;
> +
> + /*
> + * In the time of close, there is no contention with another one
> + * changing this flag. So read df->access_granted without lock
> + * and no smp_load_acquire() is ok.
> + */
> + if (!df->access_granted)
> + return;
> +
> + mutex_lock(&device->dev_set->lock);
> + vfio_df_close(df);
> + vfio_device_put_kvm(device);
> + iommufd_ctx_put(df->iommufd);
> + device->cdev_opened = false;
> + mutex_unlock(&device->dev_set->lock);
> + vfio_device_unblock_group(device);
> +}
Lets call this vfio_df_unbind_iommufd() and put it near
vfio_df_ioctl_bind_iommufd()
> static struct iommufd_ctx *vfio_get_iommufd_from_fd(int fd)
This can probably be an iommufd function:
iommufd_ctx_from_fd(int fd)
> +long vfio_df_ioctl_bind_iommufd(struct vfio_device_file *df,
> + struct vfio_device_bind_iommufd __user *arg)
> +{
> + ret = copy_to_user(&arg->out_devid, &df->devid,
> + sizeof(df->devid)) ? -EFAULT : 0;
> + if (ret)
> + goto out_close_device;
> +
> + /*
> + * Paired with smp_load_acquire() in vfio_device_fops::ioctl/
> + * read/write/mmap
> + */
> + smp_store_release(&df->access_granted, true);
> + device->cdev_opened = true;
Move the cdev_opened up above the release just for consistency.
Jason
