Re: [PATCH 1/2] powerpc: iommu: Bring back table group release_ownership() call

[Shivaprasad G Bhat <sbhat@linux.ibm.com>]

On 1/25/24 21:20, Jason Gunthorpe wrote:
> On Thu, Jan 25, 2024 at 06:08:39AM -0600, Shivaprasad G Bhat wrote:
>> The commit 2ad56efa80db ("powerpc/iommu: Setup a default domain and
[snip]
>> +		/*
>> +		 * The domain being set to PLATFORM from earlier
>> +		 * BLOCKED. The table_group ownership has to be released.
>> +		 */
>> +		table_group->ops->release_ownership(table_group);
>> +	} else if (platform_domain->type == IOMMU_DOMAIN_BLOCKED) {
>> +		table_group = iommu_group_get_iommudata(grp);
>> +		ret = table_group->ops->take_ownership(table_group);
>> +		iommu_group_put(grp);
>> +	}
> Sure, but please split the function, don't test on the
> platform->domain_type.
Sure.
> Also, is there any chance someone can work on actually fixing this to
> be a proper iommu driver? I think that will become important for power
> to use the common dma_iommu code in the next year...
We are looking into it.
> Sort of like this:
>
> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
> index ebe259bdd46298..0d6a7fea2bd9a5 100644
> --- a/arch/powerpc/kernel/iommu.c
> +++ b/arch/powerpc/kernel/iommu.c
> @@ -1287,20 +1287,20 @@ spapr_tce_platform_iommu_attach_dev(struct iommu_domain *platform_domain,
>   	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
>   	struct iommu_group *grp = iommu_group_get(dev);
[snip]
> +static const struct iommu_domain_ops spapr_tce_blocked_domain_ops = {
> +	.attach_dev = spapr_tce_blocked_iommu_attach_dev,
> +};
> +
> +static struct iommu_domain spapr_tce_blocked_domain = {
> +	.type = IOMMU_DOMAIN_BLOCKED,
> +	.ops = &spapr_tce_blocked_domain_ops,
>   };
>   
>   static bool spapr_tce_iommu_capable(struct device *dev, enum iommu_cap cap)

I have posted the next version as suggested.

Thanks for the quick review!

Regards,

Shivaprasad