Re: [PATCH v4 2/4] vfio-iommufd: Support pasid [at|de]tach for physical VFIO devices

[Alex Williamson <alex.williamson@redhat.com>]

On Mon,  4 Nov 2024 05:27:30 -0800
Yi Liu <yi.l.liu@intel.com> wrote:

> This adds pasid_at|de]tach_ioas ops for attaching hwpt to pasid of a
> device and the helpers for it. For now, only vfio-pci supports pasid
> attach/detach.
> 
> Signed-off-by: Kevin Tian <kevin.tian@intel.com>
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
>  drivers/vfio/iommufd.c      | 50 +++++++++++++++++++++++++++++++++++++
>  drivers/vfio/pci/vfio_pci.c |  2 ++
>  include/linux/vfio.h        | 11 ++++++++
>  3 files changed, 63 insertions(+)
> 
> diff --git a/drivers/vfio/iommufd.c b/drivers/vfio/iommufd.c
> index 82eba6966fa5..2f5cb4f616ce 100644
> --- a/drivers/vfio/iommufd.c
> +++ b/drivers/vfio/iommufd.c
> @@ -119,14 +119,22 @@ int vfio_iommufd_physical_bind(struct vfio_device *vdev,
>  	if (IS_ERR(idev))
>  		return PTR_ERR(idev);
>  	vdev->iommufd_device = idev;
> +	ida_init(&vdev->pasids);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(vfio_iommufd_physical_bind);
>  
>  void vfio_iommufd_physical_unbind(struct vfio_device *vdev)
>  {
> +	int pasid;
> +
>  	lockdep_assert_held(&vdev->dev_set->lock);
>  
> +	while ((pasid = ida_find_first(&vdev->pasids)) >= 0) {
> +		iommufd_device_pasid_detach(vdev->iommufd_device, pasid);
> +		ida_free(&vdev->pasids, pasid);
> +	}
> +
>  	if (vdev->iommufd_attached) {
>  		iommufd_device_detach(vdev->iommufd_device);
>  		vdev->iommufd_attached = false;
> @@ -168,6 +176,48 @@ void vfio_iommufd_physical_detach_ioas(struct vfio_device *vdev)
>  }
>  EXPORT_SYMBOL_GPL(vfio_iommufd_physical_detach_ioas);
>  
> +int vfio_iommufd_physical_pasid_attach_ioas(struct vfio_device *vdev,
> +					    u32 pasid, u32 *pt_id)
> +{
> +	int rc;
> +
> +	lockdep_assert_held(&vdev->dev_set->lock);
> +
> +	if (WARN_ON(!vdev->iommufd_device))
> +		return -EINVAL;
> +
> +	if (ida_exists(&vdev->pasids, pasid))
> +		return iommufd_device_pasid_replace(vdev->iommufd_device,
> +						    pasid, pt_id);
> +
> +	rc = ida_alloc_range(&vdev->pasids, pasid, pasid, GFP_KERNEL);
> +	if (rc < 0)
> +		return rc;
> +
> +	rc = iommufd_device_pasid_attach(vdev->iommufd_device, pasid, pt_id);
> +	if (rc)
> +		ida_free(&vdev->pasids, pasid);
> +
> +	return 0;

I think you meant to return rc here.  Thanks,

Alex

> +}
> +EXPORT_SYMBOL_GPL(vfio_iommufd_physical_pasid_attach_ioas);
> +
> +void vfio_iommufd_physical_pasid_detach_ioas(struct vfio_device *vdev,
> +					     u32 pasid)
> +{
> +	lockdep_assert_held(&vdev->dev_set->lock);
> +
> +	if (WARN_ON(!vdev->iommufd_device))
> +		return;
> +
> +	if (!ida_exists(&vdev->pasids, pasid))
> +		return;
> +
> +	iommufd_device_pasid_detach(vdev->iommufd_device, pasid);
> +	ida_free(&vdev->pasids, pasid);
> +}
> +EXPORT_SYMBOL_GPL(vfio_iommufd_physical_pasid_detach_ioas);
> +
>  /*
>   * The emulated standard ops mean that vfio_device is going to use the
>   * "mdev path" and will call vfio_pin_pages()/vfio_dma_rw(). Drivers using this
> diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
> index e727941f589d..6f7ae7e5b7b0 100644
> --- a/drivers/vfio/pci/vfio_pci.c
> +++ b/drivers/vfio/pci/vfio_pci.c
> @@ -144,6 +144,8 @@ static const struct vfio_device_ops vfio_pci_ops = {
>  	.unbind_iommufd	= vfio_iommufd_physical_unbind,
>  	.attach_ioas	= vfio_iommufd_physical_attach_ioas,
>  	.detach_ioas	= vfio_iommufd_physical_detach_ioas,
> +	.pasid_attach_ioas	= vfio_iommufd_physical_pasid_attach_ioas,
> +	.pasid_detach_ioas	= vfio_iommufd_physical_pasid_detach_ioas,
>  };
>  
>  static int vfio_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 000a6cab2d31..11b3b453752e 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -67,6 +67,7 @@ struct vfio_device {
>  	struct inode *inode;
>  #if IS_ENABLED(CONFIG_IOMMUFD)
>  	struct iommufd_device *iommufd_device;
> +	struct ida pasids;
>  	u8 iommufd_attached:1;
>  #endif
>  	u8 cdev_opened:1;
> @@ -91,6 +92,8 @@ struct vfio_device {
>   *		 bound iommufd. Undo in unbind_iommufd if @detach_ioas is not
>   *		 called.
>   * @detach_ioas: Opposite of attach_ioas
> + * @pasid_attach_ioas: The pasid variation of attach_ioas
> + * @pasid_detach_ioas: Opposite of pasid_attach_ioas
>   * @open_device: Called when the first file descriptor is opened for this device
>   * @close_device: Opposite of open_device
>   * @read: Perform read(2) on device file descriptor
> @@ -115,6 +118,8 @@ struct vfio_device_ops {
>  	void	(*unbind_iommufd)(struct vfio_device *vdev);
>  	int	(*attach_ioas)(struct vfio_device *vdev, u32 *pt_id);
>  	void	(*detach_ioas)(struct vfio_device *vdev);
> +	int	(*pasid_attach_ioas)(struct vfio_device *vdev, u32 pasid, u32 *pt_id);
> +	void	(*pasid_detach_ioas)(struct vfio_device *vdev, u32 pasid);
>  	int	(*open_device)(struct vfio_device *vdev);
>  	void	(*close_device)(struct vfio_device *vdev);
>  	ssize_t	(*read)(struct vfio_device *vdev, char __user *buf,
> @@ -139,6 +144,8 @@ int vfio_iommufd_physical_bind(struct vfio_device *vdev,
>  void vfio_iommufd_physical_unbind(struct vfio_device *vdev);
>  int vfio_iommufd_physical_attach_ioas(struct vfio_device *vdev, u32 *pt_id);
>  void vfio_iommufd_physical_detach_ioas(struct vfio_device *vdev);
> +int vfio_iommufd_physical_pasid_attach_ioas(struct vfio_device *vdev, u32 pasid, u32 *pt_id);
> +void vfio_iommufd_physical_pasid_detach_ioas(struct vfio_device *vdev, u32 pasid);
>  int vfio_iommufd_emulated_bind(struct vfio_device *vdev,
>  			       struct iommufd_ctx *ictx, u32 *out_device_id);
>  void vfio_iommufd_emulated_unbind(struct vfio_device *vdev);
> @@ -166,6 +173,10 @@ vfio_iommufd_get_dev_id(struct vfio_device *vdev, struct iommufd_ctx *ictx)
>  	((int (*)(struct vfio_device *vdev, u32 *pt_id)) NULL)
>  #define vfio_iommufd_physical_detach_ioas \
>  	((void (*)(struct vfio_device *vdev)) NULL)
> +#define vfio_iommufd_physical_pasid_attach_ioas \
> +	((int (*)(struct vfio_device *vdev, u32 pasid, u32 *pt_id)) NULL)
> +#define vfio_iommufd_physical_pasid_detach_ioas \
> +	((void (*)(struct vfio_device *vdev, u32 pasid)) NULL)
>  #define vfio_iommufd_emulated_bind                                      \
>  	((int (*)(struct vfio_device *vdev, struct iommufd_ctx *ictx,   \
>  		  u32 *out_device_id)) NULL)