[PATCH v2] arm64/dma-mapping: fix DMA_ATTR_FORCE_CONTIGUOUS mmaping code

[Andrzej Hajda]

Hi Russel,


On 31.03.2017 13:16, Russell King - ARM Linux wrote:
> On Fri, Mar 31, 2017 at 01:02:51PM +0200, Andrzej Hajda wrote:
>> In this version of the patch I have replaced temporal pages and
>> iommu_dma_mmap with remap_pfn_range or rather its simplified version
>> vm_iomap_memory.
>> Unfortunately I have not find nice helper for sgtable creation, so I
>> left sg allocation inside _iommu_mmap_attrs.
>>
>> As the fixing of DMA_ATTR_FORCE_CONTIGUOUS related crashes has higher
>> priority I have focused only on it in this patch.
> As I mentioned elsewhere, I don't believe that fudging around in this way
> is a proper fix.
>
> DMA coherent memory was never, ever, intended to be passed back into the
> streaming APIs - it was designed that the two would be mutually exclusive.
>
> The problem is that we now have DMA coherent allocations being passed
> through the dma-buf API using this dma_get_sgtable() thing, which is quite
> broken.  

This patch just fixes current implementation of the API by re-using
existing techniques, nothing more.
It prevents code from using some pointer (area->pages) not because of
lack backing struct pages, but because this pointer is just not
initialized and apparently not necessary in case of contiguous allocations.
I think, the problem of fixing DMA-API should be addressed in separate
set of patches, as it is a different issue and requires much more
experience in MM and DMA-API than I have.

Regards
Andrzej

> I regard dma_get_sgtable() as very broken, and had I realised at
> the time that this was what it was trying to do, I would have NAK'd it.
>
> Rather than bodging around this brokenness, trying to get dma_get_sgtable()
> to work, I believe we need to address the root cause - which is proper
> support for passing DMA coherent allocations through dma-buf, which does
> not involve scatterlists and calling dma_map_sg() on it.
>
> That's going to need to be addressed in any case, because of the
> dma_declare_coherent_memory() issue, where we may not have struct pages
> backing a coherent allocation.  Such a case can come up on ARM64 via
> DT's "shared-dma-pool" reserved memory stuff.
>
> Right now, I have a "fix" for ARM queued which causes dma_get_sgtable()
> to fail when used with reserved memory, but we have one user who needs
> this to work.  So, dma-buf needs to be fixed for this one way or another,
> and I don't think that bending the current broken stuff to suit it by
> trying these vmalloc_to_page() hacks is acceptable.
>
> dma_get_sgtable() is fundamentally broken IMHO.
>