* [PATCH v4 0/2] Fix dma mapping when the cache is coherent @ 2015-09-29 16:50 Gregory CLEMENT 2015-09-29 16:50 ` Gregory CLEMENT 2015-09-29 16:50 ` [PATCH v4 2/2] ARM: dma-mapping: Fix the coherent case when iommu is used Gregory CLEMENT 0 siblings, 2 replies; 14+ messages in thread From: Gregory CLEMENT @ 2015-09-29 16:50 UTC (permalink / raw) To: linux-arm-kernel Hi, These two patches fixes the dma mapping functions when the system is cache coherent. The first one allows to fix an issue we have on Armada 375/38x with the PL310 that's why it is tagged for stable too. I was about ti submit it to Russell King's patch system but when I rebased it on v4.3-rc1 I got a merge conflict due too the commit 21caf3a765b0 "ARM: 8398/1: arm DMA: Fix allocation from CMA for coherent DMA". The resolution would be OK so unless I missed something I will submit it to the patch system in a few days. Thanks, Gregory Changelog v3 -> v4: - Rebased on v4.3-rc1 - Fix conflict with commit "21caf3a765b0 ARM: 8398/1: arm DMA: Fix allocation from CMA for coherent DMA" v2 -> v3: - Fix comments in patch 1 as suggested by Catalin. - Fix build issues in patch 2 (by using the multi_v7_defconfig + CONFIG_ROCKCHIP_IOMMU). - Add the arm_coherent_iommu_mmap_attrs function. Gregory CLEMENT (2): ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent ARM: dma-mapping: Fix the coherent case when iommu is used arch/arm/mm/dma-mapping.c | 140 +++++++++++++++++++++++++++++++++------------- 1 file changed, 101 insertions(+), 39 deletions(-) -- 2.1.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent 2015-09-29 16:50 [PATCH v4 0/2] Fix dma mapping when the cache is coherent Gregory CLEMENT @ 2015-09-29 16:50 ` Gregory CLEMENT 2015-09-29 16:50 ` [PATCH v4 2/2] ARM: dma-mapping: Fix the coherent case when iommu is used Gregory CLEMENT 1 sibling, 0 replies; 14+ messages in thread From: Gregory CLEMENT @ 2015-09-29 16:50 UTC (permalink / raw) To: linux-arm-kernel When a L2 cache controller is used in a system that provides hardware coherency, the entire outer cache operations are useless, and can be skipped. Moreover, on some systems, it is harmful as it causes deadlocks between the Marvell coherency mechanism, the Marvell PCIe controller and the Cortex-A9. In the current kernel implementation, the outer cache flush range operation is triggered by the dma_alloc function. This operation can be take place during runtime and in some circumstances may lead to the PCIe/PL310 deadlock on Armada 375/38x SoCs. This patch extends the __dma_clear_buffer() function to receive a boolean argument related to the coherency of the system. The same things is done for the calling functions. Reported-by: Nadav Haklai <nadavh@marvell.com> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Cc: <stable@vger.kernel.org> # v3.16+ Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> --- arch/arm/mm/dma-mapping.c | 56 +++++++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index e62604384945..c8ed0c16c5a8 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -225,7 +225,7 @@ static u64 get_coherent_dma_mask(struct device *dev) return mask; } -static void __dma_clear_buffer(struct page *page, size_t size) +static void __dma_clear_buffer(struct page *page, size_t size, bool is_coherent) { /* * Ensure that the allocated pages are zeroed, and that any data @@ -237,17 +237,21 @@ static void __dma_clear_buffer(struct page *page, size_t size) while (size > 0) { void *ptr = kmap_atomic(page); memset(ptr, 0, PAGE_SIZE); - dmac_flush_range(ptr, ptr + PAGE_SIZE); + if (!is_coherent) + dmac_flush_range(ptr, ptr + PAGE_SIZE); kunmap_atomic(ptr); page++; size -= PAGE_SIZE; } - outer_flush_range(base, end); + if (!is_coherent) + outer_flush_range(base, end); } else { void *ptr = page_address(page); memset(ptr, 0, size); - dmac_flush_range(ptr, ptr + size); - outer_flush_range(__pa(ptr), __pa(ptr) + size); + if (!is_coherent) { + dmac_flush_range(ptr, ptr + size); + outer_flush_range(__pa(ptr), __pa(ptr) + size); + } } } @@ -255,7 +259,8 @@ static void __dma_clear_buffer(struct page *page, size_t size) * Allocate a DMA buffer for 'dev' of size 'size' using the * specified gfp mask. Note that 'size' must be page aligned. */ -static struct page *__dma_alloc_buffer(struct device *dev, size_t size, gfp_t gfp) +static struct page *__dma_alloc_buffer(struct device *dev, size_t size, + gfp_t gfp, bool is_coherent) { unsigned long order = get_order(size); struct page *page, *p, *e; @@ -271,7 +276,7 @@ static struct page *__dma_alloc_buffer(struct device *dev, size_t size, gfp_t gf for (p = page + (size >> PAGE_SHIFT), e = page + (1 << order); p < e; p++) __free_page(p); - __dma_clear_buffer(page, size); + __dma_clear_buffer(page, size, is_coherent); return page; } @@ -293,7 +298,8 @@ static void __dma_free_buffer(struct page *page, size_t size) static void *__alloc_from_contiguous(struct device *dev, size_t size, pgprot_t prot, struct page **ret_page, - const void *caller, bool want_vaddr); + const void *caller, bool want_vaddr, + bool is_coherent); static void *__alloc_remap_buffer(struct device *dev, size_t size, gfp_t gfp, pgprot_t prot, struct page **ret_page, @@ -359,9 +365,13 @@ static int __init atomic_pool_init(void) if (!atomic_pool) goto out; + /* + * The atomic pool is only used for non-coherent allocations + * so we must pass false for is_coherent. + */ if (dev_get_cma_area(NULL)) ptr = __alloc_from_contiguous(NULL, atomic_pool_size, prot, - &page, atomic_pool_init, true); + &page, atomic_pool_init, true, false); else ptr = __alloc_remap_buffer(NULL, atomic_pool_size, gfp, prot, &page, atomic_pool_init, true); @@ -475,7 +485,11 @@ static void *__alloc_remap_buffer(struct device *dev, size_t size, gfp_t gfp, { struct page *page; void *ptr = NULL; - page = __dma_alloc_buffer(dev, size, gfp); + /* + * __alloc_remap_buffer is only called when the device is + * non-coherent + */ + page = __dma_alloc_buffer(dev, size, gfp, false); if (!page) return NULL; if (!want_vaddr) @@ -530,7 +544,8 @@ static int __free_from_pool(void *start, size_t size) static void *__alloc_from_contiguous(struct device *dev, size_t size, pgprot_t prot, struct page **ret_page, - const void *caller, bool want_vaddr) + const void *caller, bool want_vaddr, + bool is_coherent) { unsigned long order = get_order(size); size_t count = size >> PAGE_SHIFT; @@ -541,7 +556,7 @@ static void *__alloc_from_contiguous(struct device *dev, size_t size, if (!page) return NULL; - __dma_clear_buffer(page, size); + __dma_clear_buffer(page, size, is_coherent); if (!want_vaddr) goto out; @@ -591,7 +606,7 @@ static inline pgprot_t __get_dma_pgprot(struct dma_attrs *attrs, pgprot_t prot) #define __get_dma_pgprot(attrs, prot) __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 +#define __alloc_from_contiguous(dev, size, prot, ret, c, wv, is_coherent) NULL #define __free_from_pool(cpu_addr, size) 0 #define __free_from_contiguous(dev, page, cpu_addr, size, wv) do { } while (0) #define __dma_free_remap(cpu_addr, size) do { } while (0) @@ -602,7 +617,8 @@ static void *__alloc_simple_buffer(struct device *dev, size_t size, gfp_t gfp, struct page **ret_page) { struct page *page; - page = __dma_alloc_buffer(dev, size, gfp); + /* __alloc_simple_buffer is only called when the device is coherent */ + page = __dma_alloc_buffer(dev, size, gfp, true); if (!page) return NULL; @@ -653,7 +669,7 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle, addr = __alloc_simple_buffer(dev, size, gfp, &page); else if (dev_get_cma_area(dev) && (gfp & __GFP_WAIT)) addr = __alloc_from_contiguous(dev, size, prot, &page, - caller, want_vaddr); + caller, want_vaddr, is_coherent); else if (is_coherent) addr = __alloc_simple_buffer(dev, size, gfp, &page); else if (!(gfp & __GFP_WAIT)) @@ -1123,7 +1139,8 @@ static inline void __free_iova(struct dma_iommu_mapping *mapping, } static struct page **__iommu_alloc_buffer(struct device *dev, size_t size, - gfp_t gfp, struct dma_attrs *attrs) + gfp_t gfp, struct dma_attrs *attrs, + bool is_coherent) { struct page **pages; int count = size >> PAGE_SHIFT; @@ -1146,7 +1163,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size, if (!page) goto error; - __dma_clear_buffer(page, size); + __dma_clear_buffer(page, size, is_coherent); for (i = 0; i < count; i++) pages[i] = page + i; @@ -1190,7 +1207,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size, pages[i + j] = pages[i] + j; } - __dma_clear_buffer(pages[i], PAGE_SIZE << order); + __dma_clear_buffer(pages[i], PAGE_SIZE << order, is_coherent); i += 1 << order; count -= 1 << order; } @@ -1373,7 +1390,8 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size, */ gfp &= ~(__GFP_COMP); - pages = __iommu_alloc_buffer(dev, size, gfp, attrs); + /* For now always consider we are in a non-coherent case */ + pages = __iommu_alloc_buffer(dev, size, gfp, attrs, false); if (!pages) return NULL; -- 2.1.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent @ 2015-09-29 16:50 ` Gregory CLEMENT 0 siblings, 0 replies; 14+ messages in thread From: Gregory CLEMENT @ 2015-09-29 16:50 UTC (permalink / raw) To: Russell King, Catalin Marinas Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory CLEMENT, Thomas Petazzoni, Ezequiel Garcia, linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai, stable When a L2 cache controller is used in a system that provides hardware coherency, the entire outer cache operations are useless, and can be skipped. Moreover, on some systems, it is harmful as it causes deadlocks between the Marvell coherency mechanism, the Marvell PCIe controller and the Cortex-A9. In the current kernel implementation, the outer cache flush range operation is triggered by the dma_alloc function. This operation can be take place during runtime and in some circumstances may lead to the PCIe/PL310 deadlock on Armada 375/38x SoCs. This patch extends the __dma_clear_buffer() function to receive a boolean argument related to the coherency of the system. The same things is done for the calling functions. Reported-by: Nadav Haklai <nadavh@marvell.com> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> Cc: <stable@vger.kernel.org> # v3.16+ Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> --- arch/arm/mm/dma-mapping.c | 56 +++++++++++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 19 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index e62604384945..c8ed0c16c5a8 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -225,7 +225,7 @@ static u64 get_coherent_dma_mask(struct device *dev) return mask; } -static void __dma_clear_buffer(struct page *page, size_t size) +static void __dma_clear_buffer(struct page *page, size_t size, bool is_coherent) { /* * Ensure that the allocated pages are zeroed, and that any data @@ -237,17 +237,21 @@ static void __dma_clear_buffer(struct page *page, size_t size) while (size > 0) { void *ptr = kmap_atomic(page); memset(ptr, 0, PAGE_SIZE); - dmac_flush_range(ptr, ptr + PAGE_SIZE); + if (!is_coherent) + dmac_flush_range(ptr, ptr + PAGE_SIZE); kunmap_atomic(ptr); page++; size -= PAGE_SIZE; } - outer_flush_range(base, end); + if (!is_coherent) + outer_flush_range(base, end); } else { void *ptr = page_address(page); memset(ptr, 0, size); - dmac_flush_range(ptr, ptr + size); - outer_flush_range(__pa(ptr), __pa(ptr) + size); + if (!is_coherent) { + dmac_flush_range(ptr, ptr + size); + outer_flush_range(__pa(ptr), __pa(ptr) + size); + } } } @@ -255,7 +259,8 @@ static void __dma_clear_buffer(struct page *page, size_t size) * Allocate a DMA buffer for 'dev' of size 'size' using the * specified gfp mask. Note that 'size' must be page aligned. */ -static struct page *__dma_alloc_buffer(struct device *dev, size_t size, gfp_t gfp) +static struct page *__dma_alloc_buffer(struct device *dev, size_t size, + gfp_t gfp, bool is_coherent) { unsigned long order = get_order(size); struct page *page, *p, *e; @@ -271,7 +276,7 @@ static struct page *__dma_alloc_buffer(struct device *dev, size_t size, gfp_t gf for (p = page + (size >> PAGE_SHIFT), e = page + (1 << order); p < e; p++) __free_page(p); - __dma_clear_buffer(page, size); + __dma_clear_buffer(page, size, is_coherent); return page; } @@ -293,7 +298,8 @@ static void __dma_free_buffer(struct page *page, size_t size) static void *__alloc_from_contiguous(struct device *dev, size_t size, pgprot_t prot, struct page **ret_page, - const void *caller, bool want_vaddr); + const void *caller, bool want_vaddr, + bool is_coherent); static void *__alloc_remap_buffer(struct device *dev, size_t size, gfp_t gfp, pgprot_t prot, struct page **ret_page, @@ -359,9 +365,13 @@ static int __init atomic_pool_init(void) if (!atomic_pool) goto out; + /* + * The atomic pool is only used for non-coherent allocations + * so we must pass false for is_coherent. + */ if (dev_get_cma_area(NULL)) ptr = __alloc_from_contiguous(NULL, atomic_pool_size, prot, - &page, atomic_pool_init, true); + &page, atomic_pool_init, true, false); else ptr = __alloc_remap_buffer(NULL, atomic_pool_size, gfp, prot, &page, atomic_pool_init, true); @@ -475,7 +485,11 @@ static void *__alloc_remap_buffer(struct device *dev, size_t size, gfp_t gfp, { struct page *page; void *ptr = NULL; - page = __dma_alloc_buffer(dev, size, gfp); + /* + * __alloc_remap_buffer is only called when the device is + * non-coherent + */ + page = __dma_alloc_buffer(dev, size, gfp, false); if (!page) return NULL; if (!want_vaddr) @@ -530,7 +544,8 @@ static int __free_from_pool(void *start, size_t size) static void *__alloc_from_contiguous(struct device *dev, size_t size, pgprot_t prot, struct page **ret_page, - const void *caller, bool want_vaddr) + const void *caller, bool want_vaddr, + bool is_coherent) { unsigned long order = get_order(size); size_t count = size >> PAGE_SHIFT; @@ -541,7 +556,7 @@ static void *__alloc_from_contiguous(struct device *dev, size_t size, if (!page) return NULL; - __dma_clear_buffer(page, size); + __dma_clear_buffer(page, size, is_coherent); if (!want_vaddr) goto out; @@ -591,7 +606,7 @@ static inline pgprot_t __get_dma_pgprot(struct dma_attrs *attrs, pgprot_t prot) #define __get_dma_pgprot(attrs, prot) __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 +#define __alloc_from_contiguous(dev, size, prot, ret, c, wv, is_coherent) NULL #define __free_from_pool(cpu_addr, size) 0 #define __free_from_contiguous(dev, page, cpu_addr, size, wv) do { } while (0) #define __dma_free_remap(cpu_addr, size) do { } while (0) @@ -602,7 +617,8 @@ static void *__alloc_simple_buffer(struct device *dev, size_t size, gfp_t gfp, struct page **ret_page) { struct page *page; - page = __dma_alloc_buffer(dev, size, gfp); + /* __alloc_simple_buffer is only called when the device is coherent */ + page = __dma_alloc_buffer(dev, size, gfp, true); if (!page) return NULL; @@ -653,7 +669,7 @@ static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t *handle, addr = __alloc_simple_buffer(dev, size, gfp, &page); else if (dev_get_cma_area(dev) && (gfp & __GFP_WAIT)) addr = __alloc_from_contiguous(dev, size, prot, &page, - caller, want_vaddr); + caller, want_vaddr, is_coherent); else if (is_coherent) addr = __alloc_simple_buffer(dev, size, gfp, &page); else if (!(gfp & __GFP_WAIT)) @@ -1123,7 +1139,8 @@ static inline void __free_iova(struct dma_iommu_mapping *mapping, } static struct page **__iommu_alloc_buffer(struct device *dev, size_t size, - gfp_t gfp, struct dma_attrs *attrs) + gfp_t gfp, struct dma_attrs *attrs, + bool is_coherent) { struct page **pages; int count = size >> PAGE_SHIFT; @@ -1146,7 +1163,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size, if (!page) goto error; - __dma_clear_buffer(page, size); + __dma_clear_buffer(page, size, is_coherent); for (i = 0; i < count; i++) pages[i] = page + i; @@ -1190,7 +1207,7 @@ static struct page **__iommu_alloc_buffer(struct device *dev, size_t size, pages[i + j] = pages[i] + j; } - __dma_clear_buffer(pages[i], PAGE_SIZE << order); + __dma_clear_buffer(pages[i], PAGE_SIZE << order, is_coherent); i += 1 << order; count -= 1 << order; } @@ -1373,7 +1390,8 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size, */ gfp &= ~(__GFP_COMP); - pages = __iommu_alloc_buffer(dev, size, gfp, attrs); + /* For now always consider we are in a non-coherent case */ + pages = __iommu_alloc_buffer(dev, size, gfp, attrs, false); if (!pages) return NULL; -- 2.1.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v4 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent 2015-09-29 16:50 ` Gregory CLEMENT @ 2015-09-29 17:10 ` Russell King - ARM Linux -1 siblings, 0 replies; 14+ messages in thread From: Russell King - ARM Linux @ 2015-09-29 17:10 UTC (permalink / raw) To: linux-arm-kernel On Tue, Sep 29, 2015 at 06:50:57PM +0200, Gregory CLEMENT wrote: > When a L2 cache controller is used in a system that provides hardware You're talking about L2 cache here, but you're also masking out the L1 cache maintanence (dmac_*) too. It's my understanding that we don't yet support coherency to L1 yet. > In the current kernel implementation, the outer cache flush range > operation is triggered by the dma_alloc function. > This operation can be take place during runtime and in some > circumstances may lead to the PCIe/PL310 deadlock on Armada 375/38x > SoCs. I wonder if that's what's causing the sporadic lockups I'm seeing on 38x with a SATA PCIe card - it happens at a very specific point during boot while initialising the SATA card, right down to the kernel message character that it stops at. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent @ 2015-09-29 17:10 ` Russell King - ARM Linux 0 siblings, 0 replies; 14+ messages in thread From: Russell King - ARM Linux @ 2015-09-29 17:10 UTC (permalink / raw) To: Gregory CLEMENT Cc: Catalin Marinas, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia, linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai, stable On Tue, Sep 29, 2015 at 06:50:57PM +0200, Gregory CLEMENT wrote: > When a L2 cache controller is used in a system that provides hardware You're talking about L2 cache here, but you're also masking out the L1 cache maintanence (dmac_*) too. It's my understanding that we don't yet support coherency to L1 yet. > In the current kernel implementation, the outer cache flush range > operation is triggered by the dma_alloc function. > This operation can be take place during runtime and in some > circumstances may lead to the PCIe/PL310 deadlock on Armada 375/38x > SoCs. I wonder if that's what's causing the sporadic lockups I'm seeing on 38x with a SATA PCIe card - it happens at a very specific point during boot while initialising the SATA card, right down to the kernel message character that it stops at. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent 2015-09-29 17:10 ` Russell King - ARM Linux @ 2015-09-29 17:30 ` Thomas Petazzoni -1 siblings, 0 replies; 14+ messages in thread From: Thomas Petazzoni @ 2015-09-29 17:30 UTC (permalink / raw) To: linux-arm-kernel Hello Russell, On Tue, 29 Sep 2015 18:10:54 +0100, Russell King - ARM Linux wrote: > > In the current kernel implementation, the outer cache flush range > > operation is triggered by the dma_alloc function. > > This operation can be take place during runtime and in some > > circumstances may lead to the PCIe/PL310 deadlock on Armada 375/38x > > SoCs. > > I wonder if that's what's causing the sporadic lockups I'm seeing on > 38x with a SATA PCIe card - it happens at a very specific point during > boot while initialising the SATA card, right down to the kernel message > character that it stops at. It might very well be the case. If there's enough PCIe traffic and a PL310 cache maintenance operation happening at the same time, the system will lockup. I'm a bit surprised that just the initialization of the PCIe card generates enough traffic to trigger the deadlock, but maybe I'm underestimating the problem. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent @ 2015-09-29 17:30 ` Thomas Petazzoni 0 siblings, 0 replies; 14+ messages in thread From: Thomas Petazzoni @ 2015-09-29 17:30 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Gregory CLEMENT, Catalin Marinas, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai, stable Hello Russell, On Tue, 29 Sep 2015 18:10:54 +0100, Russell King - ARM Linux wrote: > > In the current kernel implementation, the outer cache flush range > > operation is triggered by the dma_alloc function. > > This operation can be take place during runtime and in some > > circumstances may lead to the PCIe/PL310 deadlock on Armada 375/38x > > SoCs. > > I wonder if that's what's causing the sporadic lockups I'm seeing on > 38x with a SATA PCIe card - it happens at a very specific point during > boot while initialising the SATA card, right down to the kernel message > character that it stops at. It might very well be the case. If there's enough PCIe traffic and a PL310 cache maintenance operation happening at the same time, the system will lockup. I'm a bit surprised that just the initialization of the PCIe card generates enough traffic to trigger the deadlock, but maybe I'm underestimating the problem. Best regards, Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent 2015-09-29 17:30 ` Thomas Petazzoni @ 2015-09-29 17:48 ` Russell King - ARM Linux -1 siblings, 0 replies; 14+ messages in thread From: Russell King - ARM Linux @ 2015-09-29 17:48 UTC (permalink / raw) To: linux-arm-kernel On Tue, Sep 29, 2015 at 07:30:31PM +0200, Thomas Petazzoni wrote: > Hello Russell, > > On Tue, 29 Sep 2015 18:10:54 +0100, Russell King - ARM Linux wrote: > > > > In the current kernel implementation, the outer cache flush range > > > operation is triggered by the dma_alloc function. > > > This operation can be take place during runtime and in some > > > circumstances may lead to the PCIe/PL310 deadlock on Armada 375/38x > > > SoCs. > > > > I wonder if that's what's causing the sporadic lockups I'm seeing on > > 38x with a SATA PCIe card - it happens at a very specific point during > > boot while initialising the SATA card, right down to the kernel message > > character that it stops at. > > It might very well be the case. If there's enough PCIe traffic and a > PL310 cache maintenance operation happening at the same time, the > system will lockup. I'm a bit surprised that just the initialization of > the PCIe card generates enough traffic to trigger the deadlock, but > maybe I'm underestimating the problem. It isn't every boot - the board has booted around 240 kernels so far and maybe 5% of them have needed the reset button pressed because of this. It only happens when I have the SATA card connected. I don't have any drives on the SATA card at the moment though. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent @ 2015-09-29 17:48 ` Russell King - ARM Linux 0 siblings, 0 replies; 14+ messages in thread From: Russell King - ARM Linux @ 2015-09-29 17:48 UTC (permalink / raw) To: Thomas Petazzoni Cc: Gregory CLEMENT, Catalin Marinas, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai, stable On Tue, Sep 29, 2015 at 07:30:31PM +0200, Thomas Petazzoni wrote: > Hello Russell, > > On Tue, 29 Sep 2015 18:10:54 +0100, Russell King - ARM Linux wrote: > > > > In the current kernel implementation, the outer cache flush range > > > operation is triggered by the dma_alloc function. > > > This operation can be take place during runtime and in some > > > circumstances may lead to the PCIe/PL310 deadlock on Armada 375/38x > > > SoCs. > > > > I wonder if that's what's causing the sporadic lockups I'm seeing on > > 38x with a SATA PCIe card - it happens at a very specific point during > > boot while initialising the SATA card, right down to the kernel message > > character that it stops at. > > It might very well be the case. If there's enough PCIe traffic and a > PL310 cache maintenance operation happening at the same time, the > system will lockup. I'm a bit surprised that just the initialization of > the PCIe card generates enough traffic to trigger the deadlock, but > maybe I'm underestimating the problem. It isn't every boot - the board has booted around 240 kernels so far and maybe 5% of them have needed the reset button pressed because of this. It only happens when I have the SATA card connected. I don't have any drives on the SATA card at the moment though. -- FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent 2015-09-29 17:48 ` Russell King - ARM Linux @ 2015-09-29 17:55 ` Thomas Petazzoni -1 siblings, 0 replies; 14+ messages in thread From: Thomas Petazzoni @ 2015-09-29 17:55 UTC (permalink / raw) To: linux-arm-kernel Hello, On Tue, 29 Sep 2015 18:48:35 +0100, Russell King - ARM Linux wrote: > > It might very well be the case. If there's enough PCIe traffic and a > > PL310 cache maintenance operation happening at the same time, the > > system will lockup. I'm a bit surprised that just the initialization of > > the PCIe card generates enough traffic to trigger the deadlock, but > > maybe I'm underestimating the problem. > > It isn't every boot - the board has booted around 240 kernels so far and > maybe 5% of them have needed the reset button pressed because of this. > It only happens when I have the SATA card connected. I don't have any > drives on the SATA card at the moment though. Hum, ok. Then I'm not sure it's the same problem. I believe it's unlikely that the few PCIe accesses used just to enumerate the PCIe device and initialize it are enough to cause the deadlock. But I don't have (and anyway wouldn't fully understand) all the details about this issue, so I can't say for sure. Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent @ 2015-09-29 17:55 ` Thomas Petazzoni 0 siblings, 0 replies; 14+ messages in thread From: Thomas Petazzoni @ 2015-09-29 17:55 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Gregory CLEMENT, Catalin Marinas, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Ezequiel Garcia, linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai, stable Hello, On Tue, 29 Sep 2015 18:48:35 +0100, Russell King - ARM Linux wrote: > > It might very well be the case. If there's enough PCIe traffic and a > > PL310 cache maintenance operation happening at the same time, the > > system will lockup. I'm a bit surprised that just the initialization of > > the PCIe card generates enough traffic to trigger the deadlock, but > > maybe I'm underestimating the problem. > > It isn't every boot - the board has booted around 240 kernels so far and > maybe 5% of them have needed the reset button pressed because of this. > It only happens when I have the SATA card connected. I don't have any > drives on the SATA card at the moment though. Hum, ok. Then I'm not sure it's the same problem. I believe it's unlikely that the few PCIe accesses used just to enumerate the PCIe device and initialize it are enough to cause the deadlock. But I don't have (and anyway wouldn't fully understand) all the details about this issue, so I can't say for sure. Thomas -- Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent 2015-09-29 17:10 ` Russell King - ARM Linux @ 2015-09-30 9:28 ` Gregory CLEMENT -1 siblings, 0 replies; 14+ messages in thread From: Gregory CLEMENT @ 2015-09-30 9:28 UTC (permalink / raw) To: linux-arm-kernel Hi Russell, On mar., sept. 29 2015, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Tue, Sep 29, 2015 at 06:50:57PM +0200, Gregory CLEMENT wrote: >> When a L2 cache controller is used in a system that provides hardware > > You're talking about L2 cache here, but you're also masking out the L1 > cache maintanence (dmac_*) too. It's my understanding that we don't > yet support coherency to L1 yet. Do you suggest to not masking the dmac_* operation ? Initially I tought that L1 and L2 cache maintanence operation were more or less linked but I might mix up. Thanks, Gregory -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C is coherent @ 2015-09-30 9:28 ` Gregory CLEMENT 0 siblings, 0 replies; 14+ messages in thread From: Gregory CLEMENT @ 2015-09-30 9:28 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Catalin Marinas, Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Thomas Petazzoni, Ezequiel Garcia, linux-arm-kernel, Maxime Ripard, Boris BREZILLON, Lior Amsalem, Tawfik Bayouk, Nadav Haklai, stable Hi Russell, On mar., sept. 29 2015, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Tue, Sep 29, 2015 at 06:50:57PM +0200, Gregory CLEMENT wrote: >> When a L2 cache controller is used in a system that provides hardware > > You're talking about L2 cache here, but you're also masking out the L1 > cache maintanence (dmac_*) too. It's my understanding that we don't > yet support coherency to L1 yet. Do you suggest to not masking the dmac_* operation ? Initially I tought that L1 and L2 cache maintanence operation were more or less linked but I might mix up. Thanks, Gregory -- Gregory Clement, Free Electrons Kernel, drivers, real-time and embedded Linux development, consulting, training and support. http://free-electrons.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 2/2] ARM: dma-mapping: Fix the coherent case when iommu is used 2015-09-29 16:50 [PATCH v4 0/2] Fix dma mapping when the cache is coherent Gregory CLEMENT 2015-09-29 16:50 ` Gregory CLEMENT @ 2015-09-29 16:50 ` Gregory CLEMENT 1 sibling, 0 replies; 14+ messages in thread From: Gregory CLEMENT @ 2015-09-29 16:50 UTC (permalink / raw) To: linux-arm-kernel When doing dma allocation with IOMMU the __iommu_alloc_atomic() was used even when the system was coherent. However, this function allocates from a non-cacheable pool, which is fine when the device is not cache coherent but won't work as expected in the device is cache coherent. Indeed, the CPU and device must access the memory using the same cacheability attributes. Moreover when the devices are coherent, the mmap call must not change the pg_prot flags in the vma struct. The arm_coherent_iommu_mmap_attrs has been updated in the same way that it was done for the arm_dma_mmap in commit 55af8a91640d ("ARM: 8387/1: arm/mm/dma-mapping.c: Add arm_coherent_dma_mmap"). Suggested-by: Catalin Marinas <catalin.marinas@arm.com> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com> --- arch/arm/mm/dma-mapping.c | 88 +++++++++++++++++++++++++++++++++++------------ 1 file changed, 66 insertions(+), 22 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index c8ed0c16c5a8..de1b0f070353 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -1340,13 +1340,16 @@ static struct page **__iommu_get_pages(void *cpu_addr, struct dma_attrs *attrs) return NULL; } -static void *__iommu_alloc_atomic(struct device *dev, size_t size, - dma_addr_t *handle) +static void *__iommu_alloc_simple(struct device *dev, size_t size, gfp_t gfp, + dma_addr_t *handle, bool is_coherent) { struct page *page; void *addr; - addr = __alloc_from_pool(size, &page); + if (is_coherent) + addr = __alloc_simple_buffer(dev, size, gfp, &page); + else + addr = __alloc_from_pool(size, &page); if (!addr) return NULL; @@ -1361,15 +1364,19 @@ err_mapping: return NULL; } -static void __iommu_free_atomic(struct device *dev, void *cpu_addr, - dma_addr_t handle, size_t size) +static void __iommu_free_simple(struct device *dev, void *cpu_addr, + dma_addr_t handle, size_t size, bool is_coherent) { __iommu_remove_mapping(dev, handle, size); - __free_from_pool(cpu_addr, size); + if (is_coherent) + __dma_free_buffer(virt_to_page(cpu_addr), size); + else + __free_from_pool(cpu_addr, size); } -static void *arm_iommu_alloc_attrs(struct device *dev, size_t size, - dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs) +static void *__arm_iommu_alloc_attrs(struct device *dev, size_t size, + dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs, + bool is_coherent) { pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL); struct page **pages; @@ -1378,8 +1385,8 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size, *handle = DMA_ERROR_CODE; size = PAGE_ALIGN(size); - if (!(gfp & __GFP_WAIT)) - return __iommu_alloc_atomic(dev, size, handle); + if (is_coherent || !(gfp & __GFP_WAIT)) + return __iommu_alloc_simple(dev, size, gfp, handle, is_coherent); /* * Following is a work-around (a.k.a. hack) to prevent pages @@ -1390,8 +1397,7 @@ static void *arm_iommu_alloc_attrs(struct device *dev, size_t size, */ gfp &= ~(__GFP_COMP); - /* For now always consider we are in a non-coherent case */ - pages = __iommu_alloc_buffer(dev, size, gfp, attrs, false); + pages = __iommu_alloc_buffer(dev, size, gfp, attrs, is_coherent); if (!pages) return NULL; @@ -1416,7 +1422,19 @@ err_buffer: return NULL; } -static int arm_iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma, +static void *arm_iommu_alloc_attrs(struct device *dev, size_t size, + dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs) +{ + return __arm_iommu_alloc_attrs(dev, size, handle, gfp, attrs, false); +} + +static void *arm_coherent_iommu_alloc_attrs(struct device *dev, size_t size, + dma_addr_t *handle, gfp_t gfp, struct dma_attrs *attrs) +{ + return __arm_iommu_alloc_attrs(dev, size, handle, gfp, attrs, true); +} + +static int __arm_iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma, void *cpu_addr, dma_addr_t dma_addr, size_t size, struct dma_attrs *attrs) { @@ -1424,8 +1442,6 @@ 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); - if (!pages) return -ENXIO; @@ -1442,18 +1458,34 @@ static int arm_iommu_mmap_attrs(struct device *dev, struct vm_area_struct *vma, return 0; } +static int arm_iommu_mmap_attrs(struct device *dev, + struct vm_area_struct *vma, void *cpu_addr, + dma_addr_t dma_addr, size_t size, struct dma_attrs *attrs) +{ + vma->vm_page_prot = __get_dma_pgprot(attrs, vma->vm_page_prot); + + return __arm_iommu_mmap_attrs(dev, vma, cpu_addr, dma_addr, size, attrs); +} + +static int arm_coherent_iommu_mmap_attrs(struct device *dev, + struct vm_area_struct *vma, void *cpu_addr, + dma_addr_t dma_addr, size_t size, struct dma_attrs *attrs) +{ + return __arm_iommu_mmap_attrs(dev, vma, cpu_addr, dma_addr, size, attrs); +} + /* * free a page as defined by the above mapping. * Must not be called with IRQs disabled. */ -void arm_iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr, - dma_addr_t handle, struct dma_attrs *attrs) +void __arm_iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr, + dma_addr_t handle, struct dma_attrs *attrs, bool is_coherent) { struct page **pages; size = PAGE_ALIGN(size); - if (__in_atomic_pool(cpu_addr, size)) { - __iommu_free_atomic(dev, cpu_addr, handle, size); + if (is_coherent || __in_atomic_pool(cpu_addr, size)) { + __iommu_free_simple(dev, cpu_addr, handle, size, is_coherent); return; } @@ -1472,6 +1504,18 @@ void arm_iommu_free_attrs(struct device *dev, size_t size, void *cpu_addr, __iommu_free_buffer(dev, pages, size, attrs); } +void arm_iommu_free_attrs(struct device *dev, size_t size, + void *cpu_addr, dma_addr_t handle, struct dma_attrs *attrs) +{ + __arm_iommu_free_attrs(dev, size, cpu_addr, handle, attrs, false); +} + +void arm_coherent_iommu_free_attrs(struct device *dev, size_t size, + void *cpu_addr, dma_addr_t handle, struct dma_attrs *attrs) +{ + __arm_iommu_free_attrs(dev, size, cpu_addr, handle, attrs, true); +} + static int arm_iommu_get_sgtable(struct device *dev, struct sg_table *sgt, void *cpu_addr, dma_addr_t dma_addr, size_t size, struct dma_attrs *attrs) @@ -1878,9 +1922,9 @@ struct dma_map_ops iommu_ops = { }; struct dma_map_ops iommu_coherent_ops = { - .alloc = arm_iommu_alloc_attrs, - .free = arm_iommu_free_attrs, - .mmap = arm_iommu_mmap_attrs, + .alloc = arm_coherent_iommu_alloc_attrs, + .free = arm_coherent_iommu_free_attrs, + .mmap = arm_coherent_iommu_mmap_attrs, .get_sgtable = arm_iommu_get_sgtable, .map_page = arm_coherent_iommu_map_page, -- 2.1.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2015-09-30 9:28 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-09-29 16:50 [PATCH v4 0/2] Fix dma mapping when the cache is coherent Gregory CLEMENT 2015-09-29 16:50 ` [PATCH v4 1/2] ARM: dma-mapping: Don't use outer_flush_range when the L2C " Gregory CLEMENT 2015-09-29 16:50 ` Gregory CLEMENT 2015-09-29 17:10 ` Russell King - ARM Linux 2015-09-29 17:10 ` Russell King - ARM Linux 2015-09-29 17:30 ` Thomas Petazzoni 2015-09-29 17:30 ` Thomas Petazzoni 2015-09-29 17:48 ` Russell King - ARM Linux 2015-09-29 17:48 ` Russell King - ARM Linux 2015-09-29 17:55 ` Thomas Petazzoni 2015-09-29 17:55 ` Thomas Petazzoni 2015-09-30 9:28 ` Gregory CLEMENT 2015-09-30 9:28 ` Gregory CLEMENT 2015-09-29 16:50 ` [PATCH v4 2/2] ARM: dma-mapping: Fix the coherent case when iommu is used Gregory CLEMENT
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.