On Fri, Jul 10, 2015 at 08:19:34PM +0100, Robin Murphy wrote: > diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c > index d16a1ce..ccadfd4 100644 > --- a/arch/arm64/mm/dma-mapping.c > +++ b/arch/arm64/mm/dma-mapping.c > @@ -526,3 +526,426 @@ static int __init dma_debug_do_init(void) > return 0; > } > fs_initcall(dma_debug_do_init); > + > + > +#ifdef CONFIG_IOMMU_DMA > +#include <linux/dma-iommu.h> > +#include <linux/platform_device.h> > +#include <linux/amba/bus.h> > + > +/* Thankfully, all cache ops are by VA so we can ignore phys here */ > +static void flush_page(const void *virt, phys_addr_t phys) > +{ > + __dma_flush_range(virt, virt + PAGE_SIZE); > +} > + > +static void *__iommu_alloc_attrs(struct device *dev, size_t size, > + dma_addr_t *handle, gfp_t gfp, > + struct dma_attrs *attrs) > +{ > + bool coherent = is_device_dma_coherent(dev); > + int ioprot = dma_direction_to_prot(DMA_BIDIRECTIONAL, coherent); > + void *addr; > + > + if (WARN(!dev, "cannot create IOMMU mapping for unknown device\n")) > + return NULL; > + > + if (gfp & __GFP_WAIT) { > + struct page **pages; > + pgprot_t pgprot = coherent ? __pgprot(PROT_NORMAL) : > + __pgprot(PROT_NORMAL_NC); > + > + pgprot = __get_dma_pgprot(attrs, pgprot, coherent); > + pages = iommu_dma_alloc(dev, size, gfp, ioprot, coherent, > + handle, coherent ? NULL : flush_page);
As I replied already on the other patch, the "coherent" argument here should always be true. BTW, why do we need to call flush_page via iommu_dma_alloc() and not flush the buffer directly in the arch __iommu_alloc_attrs()? We already have the pointer and size after remapping in the CPU address space), it would keep the iommu_dma_alloc() simpler. > + if (!pages) > + return NULL; > + > + addr = dma_common_pages_remap(pages, size, VM_USERMAP, pgprot, > + __builtin_return_address(0)); > + if (!addr) > + iommu_dma_free(dev, pages, size, handle); > + } else { > + struct page *page; > + /* > + * In atomic context we can't remap anything, so we'll only > + * get the virtually contiguous buffer we need by way of a > + * physically contiguous allocation. > + */ > + if (coherent) { > + page = alloc_pages(gfp, get_order(size)); > + addr = page ? page_address(page) : NULL; We could even use __get_free_pages(gfp & ~__GFP_HIGHMEM) since we don't have/need highmem on arm64. > + } else { > + addr = __alloc_from_pool(size, &page, gfp); > + } > + if (addr) { > + *handle = iommu_dma_map_page(dev, page, 0, size, > + ioprot, false); Why coherent == false? > + if (iommu_dma_mapping_error(dev, *handle)) { > + if (coherent) > + __free_pages(page, get_order(size)); > + else > + __free_from_pool(addr, size); > + addr = NULL; > + } > + } > + } > + return addr; > +} In the second case here (!__GFP_WAIT), do we do any cache maintenance? I can't see it and it's needed for the !coherent case. > +static void __iommu_free_attrs(struct device *dev, size_t size, void > *cpu_addr, > + dma_addr_t handle, struct dma_attrs *attrs) > +{ > + /* > + * @cpu_addr will be one of 3 things depending on how it was allocated: > + * - A remapped array of pages from iommu_dma_alloc(), for all > + * non-atomic allocations. > + * - A non-cacheable alias from the atomic pool, for atomic > + * allocations by non-coherent devices. > + * - A normal lowmem address, for atomic allocations by > + * coherent devices. > + * Hence how dodgy the below logic looks... > + */ > + if (__free_from_pool(cpu_addr, size)) { > + iommu_dma_unmap_page(dev, handle, size, 0, NULL); > + } else if (is_vmalloc_addr(cpu_addr)){ > + struct vm_struct *area = find_vm_area(cpu_addr); > + > + if (WARN_ON(!area || !area->pages)) > + return; > + iommu_dma_free(dev, area->pages, size, &handle); > + dma_common_free_remap(cpu_addr, size, VM_USERMAP); > + } else { > + __free_pages(virt_to_page(cpu_addr), get_order(size)); > + iommu_dma_unmap_page(dev, handle, size, 0, NULL); Just slightly paranoid but it's better to unmap the page from the iommu space before freeing (in case there is some rogue device still accessing it). -- Catalin _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu