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

[Yi Liu <yi.l.liu@intel.com>]

On 2024/11/5 04:59, Alex Williamson wrote:
> 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,

you are absolutely right. :)

-- 
Regards,
Yi Liu