* [RFC PATCH v2] arm DMA: Fix allocation from CMA for coherent DMA
@ 2015-05-26 18:55 Lorenzo Nava
0 siblings, 0 replies; 8+ messages in thread
From: Lorenzo Nava @ 2015-05-26 18:55 UTC (permalink / raw
To: linux; +Cc: linux-arm-kernel, linux-kernel, catalin.marinas, arnd,
Lorenzo Nava
This patch allows the use of CMA for DMA coherent memory allocation.
At the moment if "is_coherent" is set to true the allocation can't use the CMA, which I think is not the desired behaviour.
Changes in v2:
correct __arm_dma_free() according to __dma_alloc() allocation
---
arch/arm/mm/dma-mapping.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 7e7583d..15643b9 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -645,9 +645,9 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
size = PAGE_ALIGN(size);
want_vaddr = !dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs);
- if (is_coherent || nommu())
+ if (nommu())
addr = __alloc_simple_buffer(dev, size, gfp, &page);
- else if (!(gfp & __GFP_WAIT))
+ else if (!is_coherent && !(gfp & __GFP_WAIT))
addr = __alloc_from_pool(size, &page);
else if (!dev_get_cma_area(dev))
addr = __alloc_remap_buffer(dev, size, gfp, prot, &page, caller, want_vaddr);
@@ -735,7 +735,7 @@ static void __arm_dma_free(struct device *dev, size_t size, void *cpu_addr,
size = PAGE_ALIGN(size);
- if (is_coherent || nommu()) {
+ if (nommu()) {
__dma_free_buffer(page, size);
} else if (__free_from_pool(cpu_addr, size)) {
return;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH v2] arm DMA: Fix allocation from CMA for coherent DMA
@ 2015-06-03 17:15 Lorenzo Nava
2015-06-10 11:20 ` Lorenzo Nava
2015-06-10 16:28 ` Catalin Marinas
0 siblings, 2 replies; 8+ messages in thread
From: Lorenzo Nava @ 2015-06-03 17:15 UTC (permalink / raw
To: arnd; +Cc: linux, linux-arm-kernel, linux-kernel, catalin.marinas,
Lorenzo Nava
This patch allows the use of CMA for DMA coherent memory allocation.
At the moment if the input parameter "is_coherent" is set to true
the allocation is not made using the CMA, which I think is not the
desired behaviour.
Signed-off-by: Lorenzo Nava <lorenx4@xxxxxxxx>
---
Changes in v2:
correct __arm_dma_free() according to __dma_alloc() allocation
---
arch/arm/mm/dma-mapping.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 7e7583d..15643b9 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -645,9 +645,9 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
size = PAGE_ALIGN(size);
want_vaddr = !dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs);
- if (is_coherent || nommu())
+ if (nommu())
addr = __alloc_simple_buffer(dev, size, gfp, &page);
- else if (!(gfp & __GFP_WAIT))
+ else if (!is_coherent && !(gfp & __GFP_WAIT))
addr = __alloc_from_pool(size, &page);
else if (!dev_get_cma_area(dev))
addr = __alloc_remap_buffer(dev, size, gfp, prot, &page, caller, want_vaddr);
@@ -735,7 +735,7 @@ static void __arm_dma_free(struct device *dev, size_t size, void *cpu_addr,
size = PAGE_ALIGN(size);
- if (is_coherent || nommu()) {
+ if (nommu()) {
__dma_free_buffer(page, size);
} else if (__free_from_pool(cpu_addr, size)) {
return;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v2] arm DMA: Fix allocation from CMA for coherent DMA
2015-06-03 17:15 [RFC PATCH v2] arm DMA: Fix allocation from CMA for coherent DMA Lorenzo Nava
@ 2015-06-10 11:20 ` Lorenzo Nava
2015-06-10 16:28 ` Catalin Marinas
1 sibling, 0 replies; 8+ messages in thread
From: Lorenzo Nava @ 2015-06-10 11:20 UTC (permalink / raw
To: Arnd Bergmann
Cc: Russell King - ARM Linux, linux-arm-kernel, linux-kernel,
Catalin Marinas, Lorenzo Nava
ping!
On Wed, Jun 3, 2015 at 7:15 PM, Lorenzo Nava <lorenx4@gmail.com> wrote:
> This patch allows the use of CMA for DMA coherent memory allocation.
> At the moment if the input parameter "is_coherent" is set to true
> the allocation is not made using the CMA, which I think is not the
> desired behaviour.
>
> Signed-off-by: Lorenzo Nava <lorenx4@xxxxxxxx>
> ---
> Changes in v2:
> correct __arm_dma_free() according to __dma_alloc() allocation
> ---
> arch/arm/mm/dma-mapping.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 7e7583d..15643b9 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -645,9 +645,9 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
> size = PAGE_ALIGN(size);
> want_vaddr = !dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs);
>
> - if (is_coherent || nommu())
> + if (nommu())
> addr = __alloc_simple_buffer(dev, size, gfp, &page);
> - else if (!(gfp & __GFP_WAIT))
> + else if (!is_coherent && !(gfp & __GFP_WAIT))
> addr = __alloc_from_pool(size, &page);
> else if (!dev_get_cma_area(dev))
> addr = __alloc_remap_buffer(dev, size, gfp, prot, &page, caller, want_vaddr);
> @@ -735,7 +735,7 @@ static void __arm_dma_free(struct device *dev, size_t size, void *cpu_addr,
>
> size = PAGE_ALIGN(size);
>
> - if (is_coherent || nommu()) {
> + if (nommu()) {
> __dma_free_buffer(page, size);
> } else if (__free_from_pool(cpu_addr, size)) {
> return;
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v2] arm DMA: Fix allocation from CMA for coherent DMA
2015-06-03 17:15 [RFC PATCH v2] arm DMA: Fix allocation from CMA for coherent DMA Lorenzo Nava
2015-06-10 11:20 ` Lorenzo Nava
@ 2015-06-10 16:28 ` Catalin Marinas
2015-06-10 19:34 ` Lorenzo Nava
1 sibling, 1 reply; 8+ messages in thread
From: Catalin Marinas @ 2015-06-10 16:28 UTC (permalink / raw
To: Lorenzo Nava; +Cc: arnd, linux, linux-kernel, linux-arm-kernel
On Wed, Jun 03, 2015 at 07:15:45PM +0200, Lorenzo Nava wrote:
> This patch allows the use of CMA for DMA coherent memory allocation.
> At the moment if the input parameter "is_coherent" is set to true
> the allocation is not made using the CMA, which I think is not the
> desired behaviour.
>
> Signed-off-by: Lorenzo Nava <lorenx4@xxxxxxxx>
> ---
> Changes in v2:
> correct __arm_dma_free() according to __dma_alloc() allocation
> ---
> arch/arm/mm/dma-mapping.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 7e7583d..15643b9 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -645,9 +645,9 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
> size = PAGE_ALIGN(size);
> want_vaddr = !dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs);
>
> - if (is_coherent || nommu())
> + if (nommu())
> addr = __alloc_simple_buffer(dev, size, gfp, &page);
> - else if (!(gfp & __GFP_WAIT))
> + else if (!is_coherent && !(gfp & __GFP_WAIT))
> addr = __alloc_from_pool(size, &page);
> else if (!dev_get_cma_area(dev))
> addr = __alloc_remap_buffer(dev, size, gfp, prot, &page, caller, want_vaddr);
So while you allow __alloc_from_contiguous() to be called when
is_coherent, the memory returned is still non-cacheable. The reason is
that the "prot" argument passed to __dma_alloc() in
arm_coherent_dma_alloc() is pgprot_dmacoherent(PAGE_KERNEL) which means
Normal NonCacheable memory. The mmap seems to create a cacheable mapping
as vma->vm_page_prot is not passed through __get_dma_pgprot().
I think you need something like below, completely untested:
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 1ced8a0f7a52..1ee3d8e8c313 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -573,11 +573,13 @@ static void __free_from_contiguous(struct device *dev, struct page *page,
dma_release_from_contiguous(dev, page, size >> PAGE_SHIFT);
}
-static inline pgprot_t __get_dma_pgprot(struct dma_attrs *attrs, pgprot_t prot)
+static inline pgprot_t __get_dma_pgprot(struct dma_attrs *attrs, pgprot_t prot,
+ bool coherent)
{
- prot = dma_get_attr(DMA_ATTR_WRITE_COMBINE, attrs) ?
- pgprot_writecombine(prot) :
- pgprot_dmacoherent(prot);
+ if (dma_get_attr(DMA_ATTR_WRITE_COMBINE))
+ prot = pgprot_writecombine(prot);
+ else if (!coherent)
+ prot = pgprot_dmacoherent(prot);
return prot;
}
@@ -587,7 +589,7 @@ static inline pgprot_t __get_dma_pgprot(struct dma_attrs *attrs, pgprot_t prot)
#define nommu() 1
-#define __get_dma_pgprot(attrs, prot) __pgprot(0)
+#define __get_dma_pgprot(attrs, prot, coherent) __pgprot(0)
#define __alloc_remap_buffer(dev, size, gfp, prot, ret, c, wv) NULL
#define __alloc_from_pool(size, ret_page) NULL
#define __alloc_from_contiguous(dev, size, prot, ret, c, wv) NULL
@@ -670,7 +672,7 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
void *arm_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
gfp_t gfp, struct dma_attrs *attrs)
{
- pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL);
+ pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, false);
void *memory;
if (dma_alloc_from_coherent(dev, size, handle, &memory))
@@ -683,7 +685,7 @@ void *arm_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
static void *arm_coherent_dma_alloc(struct device *dev, size_t size,
dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs)
{
- pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL);
+ pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, true);
void *memory;
if (dma_alloc_from_coherent(dev, size, handle, &memory))
@@ -733,7 +735,7 @@ int arm_dma_mmap(struct device *dev, struct vm_area_struct *vma,
struct dma_attrs *attrs)
{
#ifdef CONFIG_MMU
- vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot);
+ vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot, false);
#endif /* CONFIG_MMU */
return __arm_dma_mmap(dev, vma, cpu_addr, dma_addr, size, attrs);
}
@@ -1362,7 +1364,7 @@ static void __iommu_free_atomic(struct device *dev, void *cpu_addr,
static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs)
{
- pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL);
+ pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, is_device_dma_coherent(dev));
struct page **pages;
void *addr = NULL;
@@ -1414,7 +1416,7 @@ static int arm_iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
unsigned long usize = vma->vm_end - vma->vm_start;
struct page **pages = __iommu_get_pages(cpu_addr, attrs);
- vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot);
+ vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot, is_device_dma_coherent(dev));
if (!pages)
return -ENXIO;
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v2] arm DMA: Fix allocation from CMA for coherent DMA
2015-06-10 16:28 ` Catalin Marinas
@ 2015-06-10 19:34 ` Lorenzo Nava
2015-06-11 14:26 ` Catalin Marinas
0 siblings, 1 reply; 8+ messages in thread
From: Lorenzo Nava @ 2015-06-10 19:34 UTC (permalink / raw
To: Catalin Marinas
Cc: Arnd Bergmann, Russell King - ARM Linux, linux-kernel,
linux-arm-kernel
On Wed, Jun 10, 2015 at 6:28 PM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
>
> On Wed, Jun 03, 2015 at 07:15:45PM +0200, Lorenzo Nava wrote:
> > This patch allows the use of CMA for DMA coherent memory allocation.
> > At the moment if the input parameter "is_coherent" is set to true
> > the allocation is not made using the CMA, which I think is not the
> > desired behaviour.
> >
> > Signed-off-by: Lorenzo Nava <lorenx4@xxxxxxxx>
> > ---
> > Changes in v2:
> > correct __arm_dma_free() according to __dma_alloc() allocation
> > ---
> > arch/arm/mm/dma-mapping.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> > index 7e7583d..15643b9 100644
> > --- a/arch/arm/mm/dma-mapping.c
> > +++ b/arch/arm/mm/dma-mapping.c
> > @@ -645,9 +645,9 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
> > size = PAGE_ALIGN(size);
> > want_vaddr = !dma_get_attr(DMA_ATTR_NO_KERNEL_MAPPING, attrs);
> >
> > - if (is_coherent || nommu())
> > + if (nommu())
> > addr = __alloc_simple_buffer(dev, size, gfp, &page);
> > - else if (!(gfp & __GFP_WAIT))
> > + else if (!is_coherent && !(gfp & __GFP_WAIT))
> > addr = __alloc_from_pool(size, &page);
> > else if (!dev_get_cma_area(dev))
> > addr = __alloc_remap_buffer(dev, size, gfp, prot, &page, caller, want_vaddr);
>
> So while you allow __alloc_from_contiguous() to be called when
> is_coherent, the memory returned is still non-cacheable. The reason is
> that the "prot" argument passed to __dma_alloc() in
> arm_coherent_dma_alloc() is pgprot_dmacoherent(PAGE_KERNEL) which means
> Normal NonCacheable memory. The mmap seems to create a cacheable mapping
> as vma->vm_page_prot is not passed through __get_dma_pgprot().
>
> I think you need something like below, completely untested:
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 1ced8a0f7a52..1ee3d8e8c313 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -573,11 +573,13 @@ static void __free_from_contiguous(struct device *dev, struct page *page,
> dma_release_from_contiguous(dev, page, size >> PAGE_SHIFT);
> }
>
> -static inline pgprot_t __get_dma_pgprot(struct dma_attrs *attrs, pgprot_t prot)
> +static inline pgprot_t __get_dma_pgprot(struct dma_attrs *attrs, pgprot_t prot,
> + bool coherent)
> {
> - prot = dma_get_attr(DMA_ATTR_WRITE_COMBINE, attrs) ?
> - pgprot_writecombine(prot) :
> - pgprot_dmacoherent(prot);
> + if (dma_get_attr(DMA_ATTR_WRITE_COMBINE))
> + prot = pgprot_writecombine(prot);
> + else if (!coherent)
> + prot = pgprot_dmacoherent(prot);
> return prot;
> }
>
> @@ -587,7 +589,7 @@ static inline pgprot_t __get_dma_pgprot(struct dma_attrs *attrs, pgprot_t prot)
>
> #define nommu() 1
>
> -#define __get_dma_pgprot(attrs, prot) __pgprot(0)
> +#define __get_dma_pgprot(attrs, prot, coherent) __pgprot(0)
> #define __alloc_remap_buffer(dev, size, gfp, prot, ret, c, wv) NULL
> #define __alloc_from_pool(size, ret_page) NULL
> #define __alloc_from_contiguous(dev, size, prot, ret, c, wv) NULL
> @@ -670,7 +672,7 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
> void *arm_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
> gfp_t gfp, struct dma_attrs *attrs)
> {
> - pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL);
> + pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, false);
> void *memory;
>
> if (dma_alloc_from_coherent(dev, size, handle, &memory))
> @@ -683,7 +685,7 @@ void *arm_dma_alloc(struct device *dev, size_t size, dma_addr_t *handle,
> static void *arm_coherent_dma_alloc(struct device *dev, size_t size,
> dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs)
> {
> - pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL);
> + pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, true);
> void *memory;
>
> if (dma_alloc_from_coherent(dev, size, handle, &memory))
> @@ -733,7 +735,7 @@ int arm_dma_mmap(struct device *dev, struct vm_area_struct *vma,
> struct dma_attrs *attrs)
> {
> #ifdef CONFIG_MMU
> - vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot);
> + vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot, false);
> #endif /* CONFIG_MMU */
> return __arm_dma_mmap(dev, vma, cpu_addr, dma_addr, size, attrs);
> }
> @@ -1362,7 +1364,7 @@ static void __iommu_free_atomic(struct device *dev, void *cpu_addr,
> static void *arm_iommu_alloc_attrs(struct device *dev, size_t size,
> dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs)
> {
> - pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL);
> + pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, is_device_dma_coherent(dev));
> struct page **pages;
> void *addr = NULL;
>
> @@ -1414,7 +1416,7 @@ static int arm_iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma,
> unsigned long usize = vma->vm_end - vma->vm_start;
> struct page **pages = __iommu_get_pages(cpu_addr, attrs);
>
> - vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot);
> + vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot, is_device_dma_coherent(dev));
>
> if (!pages)
> return -ENXIO;
Thanks for the answer.
Well the final scope of this patch is just to fix what in my opinion
is an incorrect behaviour: the lack of use of CMA when the flag
"is_coherent" is set.
Of course it still exists the problem of modify the attribute to make
the memory cacheable, but it is something I would like to do in a
second step (the patch you posted is of course a good starting point).
I think that the current implementation maps memory keeping non
cacheable attributes enable, because the 'attrs' parameter passed to
arm_dma_mmap() has no WRITE_COMBINE attribute set (according to
dma_mmap_coherent() in include/asm-generic/dma-mapping-common.h).
I also notice this patch that is pending "[PATCH v3]
arm/mm/dma-mapping.c: Add arm_coherent_dma_mmap": it modifies the
mapping of memory for coherent DMA. I want to understand if the merge
of this patch requires any other modification to guarantee that
coherent memory is allocated with cacheable attributes.
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v2] arm DMA: Fix allocation from CMA for coherent DMA
2015-06-10 19:34 ` Lorenzo Nava
@ 2015-06-11 14:26 ` Catalin Marinas
2015-06-11 21:42 ` Lorenzo Nava
2015-06-12 6:34 ` Lorenzo Nava
0 siblings, 2 replies; 8+ messages in thread
From: Catalin Marinas @ 2015-06-11 14:26 UTC (permalink / raw
To: Lorenzo Nava
Cc: linux-arm-kernel, Russell King - ARM Linux, linux-kernel,
Arnd Bergmann
On Wed, Jun 10, 2015 at 09:34:43PM +0200, Lorenzo Nava wrote:
> On Wed, Jun 10, 2015 at 6:28 PM, Catalin Marinas
> <catalin.marinas@arm.com> wrote:
> > On Wed, Jun 03, 2015 at 07:15:45PM +0200, Lorenzo Nava wrote:
> > > This patch allows the use of CMA for DMA coherent memory allocation.
> > > At the moment if the input parameter "is_coherent" is set to true
> > > the allocation is not made using the CMA, which I think is not the
> > > desired behaviour.
> > >
> > > Signed-off-by: Lorenzo Nava <lorenx4@xxxxxxxx>
[...]
> > So while you allow __alloc_from_contiguous() to be called when
> > is_coherent, the memory returned is still non-cacheable. The reason is
> > that the "prot" argument passed to __dma_alloc() in
> > arm_coherent_dma_alloc() is pgprot_dmacoherent(PAGE_KERNEL) which means
> > Normal NonCacheable memory. The mmap seems to create a cacheable mapping
> > as vma->vm_page_prot is not passed through __get_dma_pgprot().
[...]
> Well the final scope of this patch is just to fix what in my opinion
> is an incorrect behaviour: the lack of use of CMA when the flag
> "is_coherent" is set.
But you still have to fix it properly: "is_coherent" means cacheable
memory which you don't get with your patch.
> Of course it still exists the problem of modify the attribute to make
> the memory cacheable, but it is something I would like to do in a
> second step (the patch you posted is of course a good starting point).
So between the first and the second step, you basically break
dma_alloc_coherent() by moving the allocation from
__alloc_simple_buffer() (returning cacheable memory) to
__alloc_from_contiguous() which changes the memory attributes to
whatever __get_dma_pgprot() returned (currently Normal Non-cacheable).
> I think that the current implementation maps memory keeping non
> cacheable attributes enable, because the 'attrs' parameter passed to
> arm_dma_mmap() has no WRITE_COMBINE attribute set (according to
> dma_mmap_coherent() in include/asm-generic/dma-mapping-common.h).
At least on ARMv7, WRITE_COMBINE and Normal Non-cacheable are the same.
> I also notice this patch that is pending "[PATCH v3]
> arm/mm/dma-mapping.c: Add arm_coherent_dma_mmap": it modifies the
> mapping of memory for coherent DMA. I want to understand if the merge
> of this patch requires any other modification to guarantee that
> coherent memory is allocated with cacheable attributes.
I think this patch will go in, it is already in linux-next.
--
Catalin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v2] arm DMA: Fix allocation from CMA for coherent DMA
2015-06-11 14:26 ` Catalin Marinas
@ 2015-06-11 21:42 ` Lorenzo Nava
2015-06-12 6:34 ` Lorenzo Nava
1 sibling, 0 replies; 8+ messages in thread
From: Lorenzo Nava @ 2015-06-11 21:42 UTC (permalink / raw
To: Catalin Marinas
Cc: linux-arm-kernel, Russell King - ARM Linux, linux-kernel,
Arnd Bergmann
On Thu, Jun 11, 2015 at 4:26 PM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> On Wed, Jun 10, 2015 at 09:34:43PM +0200, Lorenzo Nava wrote:
>> On Wed, Jun 10, 2015 at 6:28 PM, Catalin Marinas
>> <catalin.marinas@arm.com> wrote:
>> > On Wed, Jun 03, 2015 at 07:15:45PM +0200, Lorenzo Nava wrote:
>> > > This patch allows the use of CMA for DMA coherent memory allocation.
>> > > At the moment if the input parameter "is_coherent" is set to true
>> > > the allocation is not made using the CMA, which I think is not the
>> > > desired behaviour.
>> > >
>> > > Signed-off-by: Lorenzo Nava <lorenx4@xxxxxxxx>
> [...]
>> > So while you allow __alloc_from_contiguous() to be called when
>> > is_coherent, the memory returned is still non-cacheable. The reason is
>> > that the "prot" argument passed to __dma_alloc() in
>> > arm_coherent_dma_alloc() is pgprot_dmacoherent(PAGE_KERNEL) which means
>> > Normal NonCacheable memory. The mmap seems to create a cacheable mapping
>> > as vma->vm_page_prot is not passed through __get_dma_pgprot().
> [...]
>> Well the final scope of this patch is just to fix what in my opinion
>> is an incorrect behaviour: the lack of use of CMA when the flag
>> "is_coherent" is set.
>
> But you still have to fix it properly: "is_coherent" means cacheable
> memory which you don't get with your patch.
>
>> Of course it still exists the problem of modify the attribute to make
>> the memory cacheable, but it is something I would like to do in a
>> second step (the patch you posted is of course a good starting point).
>
> So between the first and the second step, you basically break
> dma_alloc_coherent() by moving the allocation from
> __alloc_simple_buffer() (returning cacheable memory) to
> __alloc_from_contiguous() which changes the memory attributes to
> whatever __get_dma_pgprot() returned (currently Normal Non-cacheable).
>
Maybe I'm losing something.
What I see is that dma_alloc_coherent() calls dma_alloc_attrs() with
attrs set to NULL.
Depending on DMA coherent settings the function
arm_coherent_dma_alloc() or arm_dma_alloc() is called. Functions has
similar behaviour and set prot according to __get_dma_pgprot() which
uses the pgprot_dmacoherent() attributes (in both cases), which
defines the memory bufferable and _non_ cacheable. So the memory has
the same attribute even if __alloc_simple_buffer() is used.
What I see is that only using the dma_alloc_writecombine() function
you can get cacheable memory attributes.
>> I think that the current implementation maps memory keeping non
>> cacheable attributes enable, because the 'attrs' parameter passed to
>> arm_dma_mmap() has no WRITE_COMBINE attribute set (according to
>> dma_mmap_coherent() in include/asm-generic/dma-mapping-common.h).
>
> At least on ARMv7, WRITE_COMBINE and Normal Non-cacheable are the same.
Yes, but the function __get_dma_pgprot() uses different flags
depending on attribute DMA_ATTR_WRITE_COMBINE: if defined the memory
is marked as cacheable.
>
>> I also notice this patch that is pending "[PATCH v3]
>> arm/mm/dma-mapping.c: Add arm_coherent_dma_mmap": it modifies the
>> mapping of memory for coherent DMA. I want to understand if the merge
>> of this patch requires any other modification to guarantee that
>> coherent memory is allocated with cacheable attributes.
>
> I think this patch will go in, it is already in linux-next.
>
Ok, thanks. Anyway I think it shouldn't affect the allocation stuffs.
Lorenzo
> --
> Catalin
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH v2] arm DMA: Fix allocation from CMA for coherent DMA
2015-06-11 14:26 ` Catalin Marinas
2015-06-11 21:42 ` Lorenzo Nava
@ 2015-06-12 6:34 ` Lorenzo Nava
1 sibling, 0 replies; 8+ messages in thread
From: Lorenzo Nava @ 2015-06-12 6:34 UTC (permalink / raw
To: Catalin Marinas
Cc: linux-arm-kernel, Russell King - ARM Linux, linux-kernel,
Arnd Bergmann
On Thu, Jun 11, 2015 at 4:26 PM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> On Wed, Jun 10, 2015 at 09:34:43PM +0200, Lorenzo Nava wrote:
>> On Wed, Jun 10, 2015 at 6:28 PM, Catalin Marinas
>> <catalin.marinas@arm.com> wrote:
>> > On Wed, Jun 03, 2015 at 07:15:45PM +0200, Lorenzo Nava wrote:
>> > > This patch allows the use of CMA for DMA coherent memory allocation.
>> > > At the moment if the input parameter "is_coherent" is set to true
>> > > the allocation is not made using the CMA, which I think is not the
>> > > desired behaviour.
>> > >
>> > > Signed-off-by: Lorenzo Nava <lorenx4@xxxxxxxx>
> [...]
>> > So while you allow __alloc_from_contiguous() to be called when
>> > is_coherent, the memory returned is still non-cacheable. The reason is
>> > that the "prot" argument passed to __dma_alloc() in
>> > arm_coherent_dma_alloc() is pgprot_dmacoherent(PAGE_KERNEL) which means
>> > Normal NonCacheable memory. The mmap seems to create a cacheable mapping
>> > as vma->vm_page_prot is not passed through __get_dma_pgprot().
> [...]
>> Well the final scope of this patch is just to fix what in my opinion
>> is an incorrect behaviour: the lack of use of CMA when the flag
>> "is_coherent" is set.
>
> But you still have to fix it properly: "is_coherent" means cacheable
> memory which you don't get with your patch.
>
>> Of course it still exists the problem of modify the attribute to make
>> the memory cacheable, but it is something I would like to do in a
>> second step (the patch you posted is of course a good starting point).
>
> So between the first and the second step, you basically break
> dma_alloc_coherent() by moving the allocation from
> __alloc_simple_buffer() (returning cacheable memory) to
> __alloc_from_contiguous() which changes the memory attributes to
> whatever __get_dma_pgprot() returned (currently Normal Non-cacheable).
>
Ok, sorry, now I've understood: __alloc_simple_buffer just doesn't
consider the attributes set in arm_coherent_dma_alloc() or
arm_dma_alloc() functions.
So, as you already correctly pointed out, I have to keep cacheability
attributes coherent even using the CMA.
I will update my patch and submit a new version.
Thank you.
Lorenzo
> --
> Catalin
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-06-12 6:35 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-03 17:15 [RFC PATCH v2] arm DMA: Fix allocation from CMA for coherent DMA Lorenzo Nava
2015-06-10 11:20 ` Lorenzo Nava
2015-06-10 16:28 ` Catalin Marinas
2015-06-10 19:34 ` Lorenzo Nava
2015-06-11 14:26 ` Catalin Marinas
2015-06-11 21:42 ` Lorenzo Nava
2015-06-12 6:34 ` Lorenzo Nava
-- strict thread matches above, loose matches on Subject: below --
2015-05-26 18:55 Lorenzo Nava
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).