ARM64: dma-mapping: fix IOMMU-based dma_mmap_atts() implementation
PATCH 1/1 ARM64: dma-mapping: fix IOMMU-based dma_mmap_atts() implementation
IOMMU-based DMA-mapping implementation doesn't support dma_declare_coherent() based regions, so there is no point to call dma_mmap_from_coherent() from __iommu_mmap_atts(). This patch removes useless code.
--- arch/arm64/mm/dma-mapping.c | 5 ----- 1 file changed, 5 deletions(-)
arch/arm64/mm/dma-mapping.c +0 -5
@@ -636,14 +636,9 @@ static int __iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma, struct dma_attrs *attrs) {struct vm_struct *area; -int ret; vma->vm_page_prot =__get_dma_pgprot (attrs, vma->vm_page_prot,is_device_dma_coherent (dev)); - -if (dma_mmap_from_coherent (dev, vma, cpu_addr, size, &ret)) -return ret; - area =find_vm_area (cpu_addr);if (WARN_ON (!area || !area->pages))return -ENXIO ;
Signature · 1 line
1.9.2
PATCH 1/1 ARM64: dma-mapping: fix IOMMU-based dma_mmap_atts() implementation
Hi Marek,
Quoted 4 lines · Marek Szyprowski
IOMMU-based DMA-mapping implementation doesn't support
dma_declare_coherent()based regions, so there is no point to calldma_mmap_from_coherent()from__iommu_mmap_atts(). This patch removes useless code.
I'm not sure that's the case - AFAICS a device could still have its own local memory despite being behind an IOMMU relative to the rest of the system. If it uses dma_declare_coherent() for that, the .alloc and .free callbacks here might be bypassed by dma_{alloc,free}_attrs() short-circuiting into dma_{alloc,release}_from_coherent(), but in the case of dma_mmap_attrs() it hits the .mmap callback first and only falls back to the from_coherent check in dma_common_mmap() if that isn't implemented.
Maybe the right thing to do would be to move this check (with its associated guards) out from dma_common_mmap() up to dma_mmap_attrs() to be consistent with alloc/free; so that it doesn't need to be reimplemented all over arch/{arm,arm64,mips} at all?
Robin.
Quoted 25 lines
Signed-off-by Marek Szyprowski <[email protected]>--- arch/arm64/mm/dma-mapping.c | 5 ----- 1 file changed, 5 deletions(-)arch/arm64/mm/dma-mapping.c +0
-5@@ -636,14 +636,9 @@ static int __iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma, struct dma_attrs *attrs) {struct vm_struct *area; -int ret; vma->vm_page_prot =__get_dma_pgprot (attrs, vma->vm_page_prot,is_device_dma_coherent (dev)); - -if (dma_mmap_from_coherent (dev, vma, cpu_addr, size, &ret)) -return ret; - area =find_vm_area (cpu_addr);if (WARN_ON (!area || !area->pages))return -ENXIO ;
PATCH 1/1 ARM64: dma-mapping: fix IOMMU-based dma_mmap_atts() implementation
Hello,
Quoted 8 lines · Robin Murphy
Hi Marek,
Quoted 4 lines · Marek Szyprowski
IOMMU-based DMA-mapping implementation doesn't support
dma_declare_coherent()based regions, so there is no point to calldma_mmap_from_coherent()from__iommu_mmap_atts(). This patch removes useless code.
Quoted 8 lines
I'm not sure that's the case - AFAICS a device could still have its own local memory despite being behind an IOMMU relative to the rest of the system. If it uses
dma_declare_coherent()for that, the .alloc and .free callbacks here might be bypassed by dma_{alloc,free}_attrs() short-circuiting into dma_{alloc,release}_from_coherent(), but in the case ofdma_mmap_attrs()it hits the .mmap callback first and only falls back to thefrom_coherentcheck indma_common_mmap()if that isn't implemented.
Okay, I've missed that alloc/free_from_coherent is handled in generic include/linux/dma-mapping.h. It is still a bit strange case with no real user, but if this code is already there, then let it be.
Quoted 6 lines
Maybe the right thing to do would be to move this check (with its associated guards) out from
dma_common_mmap()up todma_mmap_attrs()to be consistent with alloc/free; so that it doesn't need to be reimplemented all overarch/{arm,arm64,mips}at all?
That might be a good idea, although I'm not sure how portable is pgprot_dmacoherent(), which will be needed by such common code.
Please ignore my patch then.
Quoted 18 lines
Robin.
Quoted 16 lines
Signed-off-by Marek Szyprowski <[email protected]>--- arch/arm64/mm/dma-mapping.c | 5 ----- 1 file changed, 5 deletions(-)arch/arm64/mm/dma-mapping.c +0
-1@@ -636,14 +636,9 @@ static int __iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma, struct dma_attrs *attrs) {struct vm_struct *area; -int ret;
Quoted 10 lines
Quoted 9 lines
arch/arm64/mm/dma-mapping.c +0
-4vma->vm_page_prot =__get_dma_pgprot (attrs, vma->vm_page_prot,is_device_dma_coherent (dev)); - -if (dma_mmap_from_coherent (dev, vma, cpu_addr, size, &ret)) -return ret; - area =find_vm_area (cpu_addr);if (WARN_ON (!area || !area->pages))return -ENXIO;
Best regards
Signature · 2 lines
Marek Szyprowski, PhD Samsung R&D Institute Poland