Subject: [RFC PATCH v2 2/3] arm64: add IOMMU dma_ops

From: [email-masked] (Laura Abbott)
Date: Wed, 04 Mar 2015 16:19:45 -0800

On 3/3/2015 4:15 AM, Robin Murphy wrote:
> On 03/03/15 03:38, Yong Wu wrote:
>> On Mon, 2015-02-16 at 20:04 +0000, Robin Murphy wrote:
>>
>>>>        2)int (*domain_init)(struct iommu_domain *domain);
>>>>          About this function, it will alloc pagetable for the iommu
>>>> domain. And We expect the pagetable memory is uncacheable, so try to
>>>> call "dma_alloc_coherent", unfortunately the "struct device *" can't be
>>>> passed into this function. so is it possible if adding a parameter in
>>>> this function.
>>>
>>> ...one of the ideas of the new of_iommu_configure framework is that
>>> iommu_ops structures can represent individual IOMMU devices if
>>> necessary. The ops->priv pointer was included for that purpose, but
>>> isn't very clean so Will has plans to remove it again - it's easy enough
>>> to achieve the same effect by having the driver embed the ops in its
>>> private instance data instead. I've done that with the ARM SMMU driver
>>> which has a similar issue of needing hardware details at domain_init()
>>> time (no patches ready yet but I have an iommu/dev branch on top of the
>>> iommu/dma branch with some current work-in-progress bits)
>>>
>>> Thanks,
>>> Robin.
>>>
>> Dear Robin,
>>          Thanks very much for your suggestion.
>>
>>          I have a more question: how to do cache maintenance in arm64.
>>
>>          a) "__dma_flush_range" can not be expected to used directly.
>>          b) Following Documentation\DMA-API.txt, section: Streaming DMA
>> mappings. dma_map_single should be used, But I am not sure how to pass
>> the first "struct device *".
>>           We have a device which call arch_setup_dma_ops to create the
>> iommu domain, if we use this device, it will enter __iommu_map_page;
>>          If we input NULL for dmap_map_single, it will assert at
>> dma_capable in swiotlb_map_page...
>
> Consider that the IOMMU's page table walker is a DMA master in its own
right, and that is the device you're mapping the page tables for.
Therefore your IOMMU driver needs to have access to the struct device
of the IOMMU itself to use for the page-table-related mappings. Also,
be sure to set the IOMMU's DMA mask correctly to prevent SWIOTLB bounce
buffers being created in the process (which as I've found generally ends
in disaster).
>
>>          And normally, we always need do cache maintenance only for some
>> bytes in the pagetable but not whole a page. Then is there a easy way to
>> do the cache maintenance?
>
> For a noncoherent device, dma_map_single() will end up calling
__dma_map_area() with the page offset and size of the original request, so
the updated part gets flushed by VA, and the rest of the page isn't touched
if it doesn't need to be. On the other hand if the page tables were
allocated with dma_alloc_coherent() in the first place, then just calling
dma_sync_single_for_device() for the updated region should suffice.

Where exactly would you call the dma_unmap? It seems a bit strange to
be repeatedly calling dma_map and never calling dma_unmap. I don't see it
explicitly forbidden in the docs anywhere to do this but it seems like
it would be violating the implicit handoff of dma_map/dma_unmap.

> Robin.

Thanks,
Laura

-- 
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project