Re: [PATCH v10 09/18] iommufd/device: Add pasid_attach array to track per-PASID attach

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

On 2025/3/21 03:19, Nicolin Chen wrote:
> On Thu, Mar 20, 2025 at 06:47:35AM -0700, Yi Liu wrote:
>> @@ -497,10 +501,13 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev,
>>   
>>   /* The device attach/detach/replace helpers for attach_handle */
>>   
>> -/* Check if idev is attached to igroup->hwpt */
>> -static bool iommufd_device_is_attached(struct iommufd_device *idev)
>> +static bool iommufd_device_is_attached(struct iommufd_device *idev,
>> +				       ioasid_t pasid)
>>   {
>> -	return xa_load(&idev->igroup->attach->device_array, idev->obj.id);
>> +	struct iommufd_attach *attach;
>> +
>> +	attach = xa_load(&idev->igroup->pasid_attach, pasid);
>> +	return xa_load(&attach->device_array, idev->obj.id);
> 
> This helper is called in iommufd_device_do_replace() after it does
> xa_cmpxchg() on to the same igroup->pasid_attach?
> 
>>   static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,
>> @@ -627,19 +634,25 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
>>   
>>   	mutex_lock(&igroup->lock);
>>   
>> -	attach = igroup->attach;
>> +	attach = xa_cmpxchg(&igroup->pasid_attach, pasid, NULL,
>> +			    XA_ZERO_ENTRY, GFP_KERNEL);
>> +	if (xa_is_err(attach)) {
>> +		rc = xa_err(attach);
>> +		goto err_unlock;
>> +	}
>> +
>>   	if (!attach) {
>>   		attach = kzalloc(sizeof(*attach), GFP_KERNEL);
> 
> Since this is attach() and we do xa_cmpxchg() with an "old=NULL",
> should !attach always be true?
> 
>> -	rc = xa_insert(&igroup->attach->device_array, idev->obj.id, XA_ZERO_ENTRY,
>> +	rc = xa_insert(&attach->device_array, idev->obj.id, XA_ZERO_ENTRY,
> 
> Nit: looks like this should have been done in the PATCH-8
> "iommufd/device: Replace device_list with device_array"

aha, yes. we can save one line here in this patch.

-- 
Regards,
Yi Liu