On Fri, 8 Nov 2024 04:17:40 -0800
Yi Liu <yi.l.liu@xxxxxxxxx> wrote:
> This extends the VFIO_DEVICE_[AT|DE]TACH_IOMMUFD_PT ioctls to attach/detach
> a given pasid of a vfio device to/from an IOAS/HWPT.
>
> Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
> ---
> drivers/vfio/device_cdev.c | 69 +++++++++++++++++++++++++++++++++-----
> include/uapi/linux/vfio.h | 29 ++++++++++------
> 2 files changed, 80 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c
> index bb1817bd4ff3..4519f482e212 100644
> --- a/drivers/vfio/device_cdev.c
> +++ b/drivers/vfio/device_cdev.c
> @@ -162,9 +162,9 @@ void vfio_df_unbind_iommufd(struct vfio_device_file *df)
> int vfio_df_ioctl_attach_pt(struct vfio_device_file *df,
> struct vfio_device_attach_iommufd_pt __user *arg)
> {
> - struct vfio_device *device = df->device;
> struct vfio_device_attach_iommufd_pt attach;
> - unsigned long minsz;
> + struct vfio_device *device = df->device;
> + unsigned long minsz, xend = 0;
> int ret;
>
> minsz = offsetofend(struct vfio_device_attach_iommufd_pt, pt_id);
> @@ -172,11 +172,38 @@ int vfio_df_ioctl_attach_pt(struct vfio_device_file *df,
> if (copy_from_user(&attach, arg, minsz))
> return -EFAULT;
>
> - if (attach.argsz < minsz || attach.flags)
> + if (attach.argsz < minsz)
> return -EINVAL;
>
> + if (attach.flags & (~VFIO_DEVICE_ATTACH_PASID))
> + return -EINVAL;
> +
> + if (attach.flags & VFIO_DEVICE_ATTACH_PASID)
> + xend = offsetofend(struct vfio_device_attach_iommufd_pt, pasid);
> +
> + /*
> + * xend may be equal to minsz if a flag is defined for reusing a
> + * reserved field or a special usage of an existing field.
> + */
> + if (xend > minsz) {
> + if (attach.argsz < xend)
> + return -EINVAL;
> +
> + if (copy_from_user((void *)&attach + minsz,
> + (void __user *)arg + minsz, xend - minsz))
> + return -EFAULT;
> + }
> +
> + if ((attach.flags & VFIO_DEVICE_ATTACH_PASID) &&
> + !device->ops->pasid_attach_ioas)
> + return -EOPNOTSUPP;
> +
> mutex_lock(&device->dev_set->lock);
> - ret = device->ops->attach_ioas(device, &attach.pt_id);
> + if (attach.flags & VFIO_DEVICE_ATTACH_PASID)
I'd just do the ops test here:
{
if (!device->ops->pasid_attach_ios)
ret = -EOPNOTSUPP;
else...
> + ret = device->ops->pasid_attach_ioas(device, attach.pasid,
> + &attach.pt_id);
} else {
(Obviously if we weren't about to generalize the prior chunk of code,
we'd test ops before the 2nd copy_from_user) Thanks,
Alex
> + else
> + ret = device->ops->attach_ioas(device, &attach.pt_id);
> if (ret)
> goto out_unlock;
>
> @@ -198,20 +225,46 @@ int vfio_df_ioctl_attach_pt(struct vfio_device_file *df,
> int vfio_df_ioctl_detach_pt(struct vfio_device_file *df,
> struct vfio_device_detach_iommufd_pt __user *arg)
> {
> - struct vfio_device *device = df->device;
> struct vfio_device_detach_iommufd_pt detach;
> - unsigned long minsz;
> + struct vfio_device *device = df->device;
> + unsigned long minsz, xend = 0;
>
> minsz = offsetofend(struct vfio_device_detach_iommufd_pt, flags);
>
> if (copy_from_user(&detach, arg, minsz))
> return -EFAULT;
>
> - if (detach.argsz < minsz || detach.flags)
> + if (detach.argsz < minsz)
> + return -EINVAL;
> +
> + if (detach.flags & (~VFIO_DEVICE_DETACH_PASID))
> return -EINVAL;
>
> + if (detach.flags & VFIO_DEVICE_DETACH_PASID)
> + xend = offsetofend(struct vfio_device_detach_iommufd_pt, pasid);
> +
> + /*
> + * xend may be equal to minsz if a flag is defined for reusing a
> + * reserved field or a special usage of an existing field.
> + */
> + if (xend > minsz) {
> + if (detach.argsz < xend)
> + return -EINVAL;
> +
> + if (copy_from_user((void *)&detach + minsz,
> + (void __user *)arg + minsz, xend - minsz))
> + return -EFAULT;
> + }
> +
> + if ((detach.flags & VFIO_DEVICE_DETACH_PASID) &&
> + !device->ops->pasid_detach_ioas)
> + return -EOPNOTSUPP;
> +
> mutex_lock(&device->dev_set->lock);
> - device->ops->detach_ioas(device);
> + if (detach.flags & VFIO_DEVICE_DETACH_PASID)
> + device->ops->pasid_detach_ioas(device, detach.pasid);
> + else
> + device->ops->detach_ioas(device);
> mutex_unlock(&device->dev_set->lock);
>
> return 0;
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index c8dbf8219c4f..6899da70b929 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -931,29 +931,34 @@ struct vfio_device_bind_iommufd {
> * VFIO_DEVICE_ATTACH_IOMMUFD_PT - _IOW(VFIO_TYPE, VFIO_BASE + 19,
> * struct vfio_device_attach_iommufd_pt)
> * @argsz: User filled size of this data.
> - * @flags: Must be 0.
> + * @flags: Flags for attach.
> * @pt_id: Input the target id which can represent an ioas or a hwpt
> * allocated via iommufd subsystem.
> * Output the input ioas id or the attached hwpt id which could
> * be the specified hwpt itself or a hwpt automatically created
> * for the specified ioas by kernel during the attachment.
> + * @pasid: The pasid to be attached, only meaningful when
> + * VFIO_DEVICE_ATTACH_PASID is set in @flags
> *
> * Associate the device with an address space within the bound iommufd.
> * Undo by VFIO_DEVICE_DETACH_IOMMUFD_PT or device fd close. This is only
> * allowed on cdev fds.
> *
> - * If a vfio device is currently attached to a valid hw_pagetable, without doing
> - * a VFIO_DEVICE_DETACH_IOMMUFD_PT, a second VFIO_DEVICE_ATTACH_IOMMUFD_PT ioctl
> - * passing in another hw_pagetable (hwpt) id is allowed. This action, also known
> - * as a hw_pagetable replacement, will replace the device's currently attached
> - * hw_pagetable with a new hw_pagetable corresponding to the given pt_id.
> + * If a vfio device or a pasid of this device is currently attached to a valid
> + * hw_pagetable (hwpt), without doing a VFIO_DEVICE_DETACH_IOMMUFD_PT, a second
> + * VFIO_DEVICE_ATTACH_IOMMUFD_PT ioctl passing in another hwpt id is allowed.
> + * This action, also known as a hw_pagetable replacement, will replace the
> + * currently attached hwpt of the device or the pasid of this device with a new
> + * hwpt corresponding to the given pt_id.
> *
> * Return: 0 on success, -errno on failure.
> */
> struct vfio_device_attach_iommufd_pt {
> __u32 argsz;
> __u32 flags;
> +#define VFIO_DEVICE_ATTACH_PASID (1 << 0)
> __u32 pt_id;
> + __u32 pasid;
> };
>
> #define VFIO_DEVICE_ATTACH_IOMMUFD_PT _IO(VFIO_TYPE, VFIO_BASE + 19)
> @@ -962,17 +967,21 @@ struct vfio_device_attach_iommufd_pt {
> * VFIO_DEVICE_DETACH_IOMMUFD_PT - _IOW(VFIO_TYPE, VFIO_BASE + 20,
> * struct vfio_device_detach_iommufd_pt)
> * @argsz: User filled size of this data.
> - * @flags: Must be 0.
> + * @flags: Flags for detach.
> + * @pasid: The pasid to be detached, only meaningful when
> + * VFIO_DEVICE_DETACH_PASID is set in @flags
> *
> - * Remove the association of the device and its current associated address
> - * space. After it, the device should be in a blocking DMA state. This is only
> - * allowed on cdev fds.
> + * Remove the association of the device or a pasid of the device and its current
> + * associated address space. After it, the device or the pasid should be in a
> + * blocking DMA state. This is only allowed on cdev fds.
> *
> * Return: 0 on success, -errno on failure.
> */
> struct vfio_device_detach_iommufd_pt {
> __u32 argsz;
> __u32 flags;
> +#define VFIO_DEVICE_DETACH_PASID (1 << 0)
> + __u32 pasid;
> };
>
> #define VFIO_DEVICE_DETACH_IOMMUFD_PT _IO(VFIO_TYPE, VFIO_BASE + 20)
