Re: [PATCH 05/20] iommu: Allow an IDENTITY domain as the default_domain in ARM32

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, May 03, 2023 at 02:50:02PM +0100, Robin Murphy wrote:

> > If the driver does not want to support dma_api with translation then it
> > always sets default_domain to the identity domain and even if IOMMU_DMA is
> > turned on it will not allow it to be used.
> 
> Could we not retain the use of .def_domain_type for this? I think if we can
> avoid the IOMMU_DOMAIN_PLATFORM thing altogether than that's the more
> appealing option.

We can, but given default_domain is already there this is the simpler
implementation.

If we can avoid platform then I would switch this too..

> > +	/*
> > +	 * ARM32 drivers supporting CONFIG_ARM_DMA_USE_IOMMU can declare an
> > +	 * identity_domain and it becomes their default domain. Later on
> > +	 * ARM_DMA_USE_IOMMU will install its UNMANAGED domain.
> > +	 */
> > +	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU) &&
> 
> For the sake of reasoning, I think just use CONFIG_ARM for this. Otherwise
> we may still end up with potential corner cases of default_domain == NULL
> that we'd rather not have to accommodate.

The last patches prevent that directly.

I picked this option because I want to eventually get to the point of
having the special ARM_IOMMU code consolidated and marked using this
ifdef so we can clearly find it all.

> > +	    bus->iommu_ops->identity_domain)
> > +		return __iommu_group_alloc_default_domain(
> > +			bus, group, IOMMU_DOMAIN_IDENTITY);
> 
> Why not simply return the identity_domain that we now know exists?

Yeah at this patch it is true, but the later patches it has to call
the function. I organized it like this to avoid some churn

> It would seem even more logical, however, to put this ARM workaround in
> iommu_get_default_domain_type() and keep the actual allocation path
> clean.

I thought about that too.. It felt it might end up more convoluted but
I can try to darft it and see.

It is still logical here because this is the "use the DMA API policy
default" code path, and the default for ARM32 right now is this.

> Do we strictly need to check for identity_domain up front? 

You mean when registering the driver? It does make sense

> (Note that as it
> stands this narrowly misses out on un-breaking arm-smmu for 32-bit)

Can you elaborate? I don't know about this..

Thanks,
Jason