Re: [PATCH v12 05/17] iommu: Add attach/detach_dev_pasid iommu interface

Baolu Lu
2022-08-28T12:52:25Z

On 2022/8/26 22:37, Jason Gunthorpe wrote:
> On Fri, Aug 26, 2022 at 08:11:29PM +0800, Lu Baolu wrote:
>
>> + * iommu_get_domain_for_dev_pasid() - Retrieve domain for @pasid of @dev
>> + * @dev: the queried device
>> + * @pasid: the pasid of the device
>> + * @type: matched domain type, 0 for any match
>> + *
>> + * This is a variant of iommu_get_domain_for_dev(). It returns the existing
>> + * domain attached to pasid of a device. It's only for internal use of the
>> + * IOMMU subsystem.
>
> If it is only for external use then why is it exported?
>
> I would add something like:
>
> Callers must hold a lock around this function, and both
> iommu_attach/detach_dev_pasid() whenever a domain of type is being
> manipulated. This API does not internally resolve races with
> attach/detach.

Yes. Updated. This is what this API expected.

>
>> + * detaching from the device PASID.
>> + *
>> + * Return: attached domain on success, NULL otherwise.
>> + */
>> +struct iommu_domain *iommu_get_domain_for_dev_pasid(struct device *dev,
>> + ioasid_t pasid,
>> + unsigned int type)
>> +{
>> + struct iommu_domain *domain;
>> + struct iommu_group *group;
>> +
>> + group = iommu_group_get(dev);
>> + if (!group)
>> + return NULL;
>> + /*
>> + * The xarray protects its internal state with RCU. Hence the domain
>> + * obtained is either NULL or fully formed.
>> + */
>
> This has nothing to do with RCU
>
> xa_lock() is used to ensure that the domain pointer remains valid
> while we check the type since it blocks concurrent xa_erase().

With xa_lock() added, this comment is not needed anymore.

Best regards,
baolu