| Message ID | 1455632065-14620-1-git-send-email-m.szyprowski@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Marek, On 16/02/16 14:14, Marek Szyprowski wrote: > 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. 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. > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > arch/arm64/mm/dma-mapping.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c > index 331c4ca..573b1e6 100644 > --- a/arch/arm64/mm/dma-mapping.c > +++ b/arch/arm64/mm/dma-mapping.c > @@ -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; >
Hello, On 2016-02-16 16:02, Robin Murphy wrote: > Hi Marek, > > On 16/02/16 14:14, Marek Szyprowski wrote: >> 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. > > 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. 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. > > 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? > 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. > Robin. > >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> >> --- >> arch/arm64/mm/dma-mapping.c | 5 ----- >> 1 file changed, 5 deletions(-) >> >> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c >> index 331c4ca..573b1e6 100644 >> --- a/arch/arm64/mm/dma-mapping.c >> +++ b/arch/arm64/mm/dma-mapping.c >> @@ -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; > Best regards
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c index 331c4ca..573b1e6 100644 --- a/arch/arm64/mm/dma-mapping.c +++ b/arch/arm64/mm/dma-mapping.c @@ -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;
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. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- arch/arm64/mm/dma-mapping.c | 5 ----- 1 file changed, 5 deletions(-)