| Message ID | 1404324218-4743-4-git-send-email-lauraa@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 2, 2014 at 11:03 AM, Laura Abbott <lauraa@codeaurora.org> wrote: > > For architectures without coherent DMA, memory for DMA may > need to be remapped with coherent attributes. Factor out > the the remapping code from arm and put it in a > common location to reduced code duplication. > > Signed-off-by: Laura Abbott <lauraa@codeaurora.org> Hm. The switch from ioremap to map_vm_area() here seems to imply that lib/ioremap can/should be reworked to use just wrap the vmalloc functions too? Unrelated to this change. I did a pass of review here. Nothing stands out as wrong but I don't claim to know this area well these days. What's the merge/ack plan here? It might reduce the complexity of merging to add the common functions in your series, then move the ARM code over separately? -Olof
On Wed, Jul 02, 2014 at 07:03:36PM +0100, Laura Abbott wrote: > +void *dma_common_pages_remap(struct page **pages, size_t size, > + unsigned long vm_flags, pgprot_t prot, > + const void *caller) > +{ > + struct vm_struct *area; > + > + area = get_vm_area_caller(size, vm_flags, caller); > + if (!area) > + return NULL; > + > + if (map_vm_area(area, prot, &pages)) { > + vunmap(area->addr); > + return NULL; > + } > + > + return area->addr; > +} Why not just replace this function with vmap()? It is nearly identical.
On Wed, Jul 09, 2014 at 11:46:56PM +0100, Olof Johansson wrote: > On Wed, Jul 2, 2014 at 11:03 AM, Laura Abbott <lauraa@codeaurora.org> wrote: > > For architectures without coherent DMA, memory for DMA may > > need to be remapped with coherent attributes. Factor out > > the the remapping code from arm and put it in a > > common location to reduced code duplication. > > > > Signed-off-by: Laura Abbott <lauraa@codeaurora.org> > > Hm. The switch from ioremap to map_vm_area() here seems to imply that > lib/ioremap can/should be reworked to use just wrap the vmalloc > functions too? ioremap_page_range() does not require the allocation of a map page array and assumes that the mapped memory is physically contiguous. This would be more efficient than the vmap() implementation which is generic enough to allow non-contiguous memory allocations. At some point, we'll have to support SMMU on arm64 and we can have 4 combinations of coherency and IOMMU. When an IOMMU is present, we don't require physically contiguous memory but we may require non-cacheable mappings, in which case vmap comes in handy.
On 7/18/2014 6:53 AM, Catalin Marinas wrote: > On Wed, Jul 02, 2014 at 07:03:36PM +0100, Laura Abbott wrote: >> +void *dma_common_pages_remap(struct page **pages, size_t size, >> + unsigned long vm_flags, pgprot_t prot, >> + const void *caller) >> +{ >> + struct vm_struct *area; >> + >> + area = get_vm_area_caller(size, vm_flags, caller); >> + if (!area) >> + return NULL; >> + >> + if (map_vm_area(area, prot, &pages)) { >> + vunmap(area->addr); >> + return NULL; >> + } >> + >> + return area->addr; >> +} > > Why not just replace this function with vmap()? It is nearly identical. > With this version, the caller stored and printed via /proc/vmallocinfo is the actual caller of the DMA API whereas if we just call vmap we don't get any useful caller information. Going to vmap would change the existing behavior on ARM so it seems unwise to switch. Another option is to move this into vmalloc.c and add vmap_caller. Thanks, Laura
On Mon, Jul 21, 2014 at 08:33:48PM +0100, Laura Abbott wrote: > On 7/18/2014 6:53 AM, Catalin Marinas wrote: > > On Wed, Jul 02, 2014 at 07:03:36PM +0100, Laura Abbott wrote: > >> +void *dma_common_pages_remap(struct page **pages, size_t size, > >> + unsigned long vm_flags, pgprot_t prot, > >> + const void *caller) > >> +{ > >> + struct vm_struct *area; > >> + > >> + area = get_vm_area_caller(size, vm_flags, caller); > >> + if (!area) > >> + return NULL; > >> + > >> + if (map_vm_area(area, prot, &pages)) { > >> + vunmap(area->addr); > >> + return NULL; > >> + } > >> + > >> + return area->addr; > >> +} > > > > Why not just replace this function with vmap()? It is nearly identical. > > With this version, the caller stored and printed via /proc/vmallocinfo > is the actual caller of the DMA API whereas if we just call vmap we > don't get any useful caller information. Going to vmap would change > the existing behavior on ARM so it seems unwise to switch. OK. > Another option is to move this into vmalloc.c and add vmap_caller. Maybe as a subsequent clean-up (once this series gets merged).
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 4c88935..f5190ac 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -297,37 +297,19 @@ static void * __dma_alloc_remap(struct page *page, size_t size, gfp_t gfp, pgprot_t prot, const void *caller) { - struct vm_struct *area; - unsigned long addr; - /* * DMA allocation can be mapped to user space, so lets * set VM_USERMAP flags too. */ - area = get_vm_area_caller(size, VM_ARM_DMA_CONSISTENT | VM_USERMAP, - caller); - if (!area) - return NULL; - addr = (unsigned long)area->addr; - area->phys_addr = __pfn_to_phys(page_to_pfn(page)); - - if (ioremap_page_range(addr, addr + size, area->phys_addr, prot)) { - vunmap((void *)addr); - return NULL; - } - return (void *)addr; + return dma_common_contiguous_remap(page, size, + VM_ARM_DMA_CONSISTENT | VM_USERMAP, + prot, caller); } static void __dma_free_remap(void *cpu_addr, size_t size) { - unsigned int flags = VM_ARM_DMA_CONSISTENT | VM_USERMAP; - struct vm_struct *area = find_vm_area(cpu_addr); - if (!area || (area->flags & flags) != flags) { - WARN(1, "trying to free invalid coherent area: %p\n", cpu_addr); - return; - } - unmap_kernel_range((unsigned long)cpu_addr, size); - vunmap(cpu_addr); + dma_common_free_remap(cpu_addr, size, + VM_ARM_DMA_CONSISTENT | VM_USERMAP); } #define DEFAULT_DMA_COHERENT_POOL_SIZE SZ_256K @@ -1261,29 +1243,8 @@ static void * __iommu_alloc_remap(struct page **pages, size_t size, gfp_t gfp, pgprot_t prot, const void *caller) { - unsigned int i, nr_pages = PAGE_ALIGN(size) >> PAGE_SHIFT; - struct vm_struct *area; - unsigned long p; - - area = get_vm_area_caller(size, VM_ARM_DMA_CONSISTENT | VM_USERMAP, - caller); - if (!area) - return NULL; - - area->pages = pages; - area->nr_pages = nr_pages; - p = (unsigned long)area->addr; - - for (i = 0; i < nr_pages; i++) { - phys_addr_t phys = __pfn_to_phys(page_to_pfn(pages[i])); - if (ioremap_page_range(p, p + PAGE_SIZE, phys, prot)) - goto err; - p += PAGE_SIZE; - } - return area->addr; -err: - unmap_kernel_range((unsigned long)area->addr, size); - vunmap(area->addr); + return dma_common_pages_remap(pages, size, + VM_ARM_DMA_CONSISTENT | VM_USERMAP, prot, caller); return NULL; } @@ -1491,8 +1452,8 @@ void arm_iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr, } if (!dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs)) { - unmap_kernel_range((unsigned long)cpu_addr, size); - vunmap(cpu_addr); + dma_common_free_remap(cpu_addr, size, + VM_ARM_DMA_CONSISTENT | VM_USERMAP); } __iommu_remove_mapping(dev, handle, size); diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c index 6cd08e1..34265b5 100644 --- a/drivers/base/dma-mapping.c +++ b/drivers/base/dma-mapping.c @@ -10,6 +10,8 @@ #include <linux/dma-mapping.h> #include <linux/export.h> #include <linux/gfp.h> +#include <linux/slab.h> +#include <linux/vmalloc.h> #include <asm-generic/dma-coherent.h> /* @@ -267,3 +269,68 @@ int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, return ret; } EXPORT_SYMBOL(dma_common_mmap); + +/* + * remaps an allocated contiguous region into another vm_area. + * Cannot be used in non-sleeping contexts + */ + +void *dma_common_contiguous_remap(struct page *page, size_t size, + unsigned long vm_flags, + pgprot_t prot, const void *caller) +{ + int i; + struct page **pages; + void *ptr; + + pages = kmalloc(sizeof(struct page *) << get_order(size), GFP_KERNEL); + if (!pages) + return NULL; + + for (i = 0; i < (size >> PAGE_SHIFT); i++) + pages[i] = page + i; + + ptr = dma_common_pages_remap(pages, size, vm_flags, prot, caller); + + kfree(pages); + + return ptr; +} + +/* + * remaps an array of PAGE_SIZE pages into another vm_area + * Cannot be used in non-sleeping contexts + */ +void *dma_common_pages_remap(struct page **pages, size_t size, + unsigned long vm_flags, pgprot_t prot, + const void *caller) +{ + struct vm_struct *area; + + area = get_vm_area_caller(size, vm_flags, caller); + if (!area) + return NULL; + + if (map_vm_area(area, prot, &pages)) { + vunmap(area->addr); + return NULL; + } + + return area->addr; +} + +/* + * unmaps a range previously mapped by dma_common_*_remap + */ +void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags) +{ + struct vm_struct *area = find_vm_area(cpu_addr); + + if (!area || (area->flags & vm_flags) != vm_flags) { + WARN(1, "trying to free invalid coherent area: %p\n", cpu_addr); + return; + } + + unmap_kernel_range((unsigned long)cpu_addr, size); + vunmap(cpu_addr); +} diff --git a/include/asm-generic/dma-mapping-common.h b/include/asm-generic/dma-mapping-common.h index de8bf89..a9fd248 100644 --- a/include/asm-generic/dma-mapping-common.h +++ b/include/asm-generic/dma-mapping-common.h @@ -179,6 +179,15 @@ dma_sync_sg_for_device(struct device *dev, struct scatterlist *sg, extern int dma_common_mmap(struct device *dev, struct vm_area_struct *vma, void *cpu_addr, dma_addr_t dma_addr, size_t size); +void *dma_common_contiguous_remap(struct page *page, size_t size, + unsigned long vm_flags, + pgprot_t prot, const void *caller); + +void *dma_common_pages_remap(struct page **pages, size_t size, + unsigned long vm_flags, pgprot_t prot, + const void *caller); +void dma_common_free_remap(void *cpu_addr, size_t size, unsigned long vm_flags); + /** * dma_mmap_attrs - map a coherent DMA allocation into user space * @dev: valid struct device pointer, or NULL for ISA and EISA-like devices
For architectures without coherent DMA, memory for DMA may need to be remapped with coherent attributes. Factor out the the remapping code from arm and put it in a common location to reduced code duplication. Signed-off-by: Laura Abbott <lauraa@codeaurora.org> --- arch/arm/mm/dma-mapping.c | 57 +++++---------------------- drivers/base/dma-mapping.c | 67 ++++++++++++++++++++++++++++++++ include/asm-generic/dma-mapping-common.h | 9 +++++ 3 files changed, 85 insertions(+), 48 deletions(-)