Re: [PATCH] iommu/exynos: add missing set_platform_dma_ops callback

[Robin Murphy <robin.murphy@arm.com>]

On 2023-02-17 12:35, Jason Gunthorpe wrote:
> On Fri, Feb 17, 2023 at 12:08:42PM +0100, Marek Szyprowski wrote:
>> Hi,
>>
>> I'm sorry for a delay in replying, but I was busy with other stuff.
>>
>> On 23.01.2023 22:00, Jason Gunthorpe wrote:
>>> On Mon, Jan 23, 2023 at 10:31:01AM +0100, Marek Szyprowski wrote:
>>>> Add set_platform_dma_ops() required for proper driver operation on ARM
>>>> 32bit arch after recent changes in the IOMMU framework (detach ops
>>>> removal).
>>> Thanks for looking into this!
>>>
>>> Can you explain more about how this actually solves the problem in the
>>> commit message? I don't get it.
>>
>> Exynos DRM driver calls arm_iommu_detach_device(), then
>> arm_iommu_attach_device() with a difrent 'mapping', see
>> drivers/gpu/drm/exynos/exynos_drm_dma.c Lack of set_platform_dma_ops
>> leads to a warning in iommu_group_do_set_platform_dma(). The other case
>> of calling arm_iommu_detach_device() is after unsuccessful probe of the
>> platform device.
> 
> Why can't this just use the normal iommu path in all cases?
> 
> It looks like it is trying to copy the DMA API domain from a parent
> device to a sub device.
> 
> Even when using arm_iommu an iommu_domain is still present, so the
> copy code should work?

The ARM DMA domain is a regular unmanaged domain owned by the ARM DMA 
code - in order to use any *other* domain, a user has to detach from 
that first (wrapped up in arm_iommu_detach_device() which also swizzles 
the DMA ops at the same time). Without set_platform_dma, that detach is 
now impossible (because no IOMMU API default domain exists either).

> Though I'm still not really sure how this arm_iommu stuff works..
> 
> eg if a driver does iommu_device_claim_dma_owner() how does the
> iommu_domain get set back to the arm_iommu's mapping's iommu_domain?

It doesn't. Fact is that VFIO has only ever worked on Arm with arm-smmu 
and its old deprecated DT bindings which don't interact with any of the 
DMA ops stuff. And ownership has always been inherently enforced here 
since the DMA ops are just another external user from the IOMMU API's PoV.

> According to the API that is what set_platform is supposed to do (eg
> it is what s390 does), but I don't see any code like that in any of
> the ARM32 drivers..

As above, it's in the relevant users, not the drivers. For what you're 
thinking of, the "platform" state is only ever transient.

(for the purposes of this discussion, ignore the MTK and Renesas drivers 
bodging around to implement pseudo-default-domain support on *top* of 
the user-centric arm_iommu_* APIs; that's a whole other mess to unravel)

Thanks,
Robin.