Re: [PATCH 5/5] iommu: Retire group->domain

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Feb 11, 2025 at 10:05:40PM -0800, Yi Liu wrote:
> @@ -2133,11 +2164,16 @@ struct iommu_domain *iommu_get_domain_for_dev(struct device *dev)
>  {
>  	/* Caller must be a probed driver on dev */
>  	struct iommu_group *group = dev->iommu_group;
> +	struct iommu_domain *gdomain;
>  
>  	if (!group)
>  		return NULL;
>  
> -	return group->domain;
> +	mutex_lock(&group->mutex);
> +	gdomain = iommu_group_domain(group);
> +	mutex_unlock(&group->mutex);

That will deadlock

Eg:

static int mtk_iommu_identity_attach(struct iommu_domain *identity_domain,
				     struct device *dev)
{
	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);

And attach is already called under the group mutex.

IMHO I would have two operations, iommu_group_domain() which lockdep
asserts that group->mutex is held by the caller. We should try to call
this function when possible to get clear locking rules

And then iommu_get_domain_for_dev() which explains it is an older
function that depends on the caller somehow ensuring that the attached
domain cannot change. Ideally we would someday try to remove calls to
iommu_get_domain_for_dev()..

Since the xarray cannot change for non-lockdepable reasons, it can
just call xa_load(). Probably re-organize iommu_group_domain() to have
a __iommu_group_domain() which has the body but no lockdep assertion.

Jason