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

[Geert Uytterhoeven]

Hi Catalin,

On Fri, Apr 21, 2017 at 7:32 PM, Catalin Marinas
<catalin.marinas at arm.com> wrote:
> On Fri, Mar 31, 2017 at 01:02:51PM +0200, Andrzej Hajda wrote:
>> In case of DMA_ATTR_FORCE_CONTIGUOUS allocations vm_area->pages
>> is invalid. __iommu_mmap_attrs and __iommu_get_sgtable should not
>> use it and take advantage of contiguous nature of the allocation.
>
> As I replied to your original patch, I think
> dma_common_contiguous_remap() should be fixed not to leave a dangling
> area->pages pointer.
>
>>  arch/arm64/mm/dma-mapping.c | 17 ++++++++++++++++-
>>  1 file changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
>> index f7b5401..41c7c36 100644
>> --- a/arch/arm64/mm/dma-mapping.c
>> +++ b/arch/arm64/mm/dma-mapping.c
>> @@ -703,6 +703,10 @@ static int __iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
>>       if (dma_mmap_from_coherent(dev, vma, cpu_addr, size, &ret))
>>               return ret;
>>
>> +     if (attrs & DMA_ATTR_FORCE_CONTIGUOUS)
>> +             return vm_iomap_memory(vma,
>> +                             page_to_phys(vmalloc_to_page(cpu_addr)), size);
>
> I replied to Geert's patch on whether we actually need to call
> dma_common_contiguous_remap() in __iommu_alloc_attrs() if the device is
> coherent. We don't do this for the swiotlb allocation. If we are going
> to change this, cpu_addr may or may not be in the vmalloc range and
> vmalloc_to_page() would fail (I guess we could add an is_vmalloc_addr()
> check).
>
> Alternatively, just open-code dma_common_contiguous_remap() in
> __iommu_alloc_attrs() to keep a valid area->pages around (I think Geert
> already suggested this). AFAICT, arch/arm does this already in its own
> __iommu_alloc_buffer().
>
> Yet another option would be for iommu_dma_alloc() to understand
> DMA_ATTR_FORCE_CONTIGUOUS and remove the arm64-specific handling.

That was actually the approach I took in my v1.
V2 changed that to provide standalone iommu_dma_{alloc,free}_contiguous()
functions.
V3 changed that to call everything directly from the arm64 code.
...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds