* [PATCH 1/1] dma: coherent: check no-map property for arm64 @ 2021-06-11 13:10 Dong Aisheng 2021-06-14 8:36 ` Will Deacon 0 siblings, 1 reply; 7+ messages in thread From: Dong Aisheng @ 2021-06-11 13:10 UTC (permalink / raw) To: iommu Cc: linux-kernel, linux-mm, dongas86, linux-arm-kernel, Dong Aisheng, Christoph Hellwig, Marek Szyprowski, Robin Murphy Coherent dma on ARM64 also can't work with mapped system ram, that means 'no-map' property must be specified in dts. Add the missing check for ARM64 platforms as well. Besides 'no-map' checking, 'linux,dma-default' feature is also enabled for ARM64 along with this patch. Cc: Christoph Hellwig <hch@lst.de> Cc: Marek Szyprowski <m.szyprowski@samsung.com> Cc: Robin Murphy <robin.murphy@arm.com> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> --- kernel/dma/coherent.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c index 5b5b6c7ec7f2..d1831da7afba 100644 --- a/kernel/dma/coherent.c +++ b/kernel/dma/coherent.c @@ -356,7 +356,7 @@ static int __init rmem_dma_setup(struct reserved_mem *rmem) if (of_get_flat_dt_prop(node, "reusable", NULL)) return -EINVAL; -#ifdef CONFIG_ARM +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) if (!of_get_flat_dt_prop(node, "no-map", NULL)) { pr_err("Reserved memory: regions without no-map are not yet supported\n"); return -EINVAL; -- 2.25.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] dma: coherent: check no-map property for arm64 2021-06-11 13:10 [PATCH 1/1] dma: coherent: check no-map property for arm64 Dong Aisheng @ 2021-06-14 8:36 ` Will Deacon 2021-06-14 10:07 ` Dong Aisheng 0 siblings, 1 reply; 7+ messages in thread From: Will Deacon @ 2021-06-14 8:36 UTC (permalink / raw) To: Dong Aisheng Cc: iommu, linux-kernel, linux-mm, dongas86, linux-arm-kernel, Christoph Hellwig, Marek Szyprowski, Robin Murphy, catalin.marinas [+Catalin] On Fri, Jun 11, 2021 at 09:10:56PM +0800, Dong Aisheng wrote: > Coherent dma on ARM64 also can't work with mapped system ram, > that means 'no-map' property must be specified in dts. > Add the missing check for ARM64 platforms as well. > Besides 'no-map' checking, 'linux,dma-default' feature is also > enabled for ARM64 along with this patch. Please can you explain _why_ it can't work? We don't need to tear down aliases from the linear map for the streaming DMA API, so why is this case different? Also, coherent devices wouldn't need this either way, would they? What problem are you solving here? Thanks, Will > > Cc: Christoph Hellwig <hch@lst.de> > Cc: Marek Szyprowski <m.szyprowski@samsung.com> > Cc: Robin Murphy <robin.murphy@arm.com> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > --- > kernel/dma/coherent.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c > index 5b5b6c7ec7f2..d1831da7afba 100644 > --- a/kernel/dma/coherent.c > +++ b/kernel/dma/coherent.c > @@ -356,7 +356,7 @@ static int __init rmem_dma_setup(struct reserved_mem *rmem) > if (of_get_flat_dt_prop(node, "reusable", NULL)) > return -EINVAL; > > -#ifdef CONFIG_ARM > +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) > if (!of_get_flat_dt_prop(node, "no-map", NULL)) { > pr_err("Reserved memory: regions without no-map are not yet supported\n"); > return -EINVAL; > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] dma: coherent: check no-map property for arm64 2021-06-14 8:36 ` Will Deacon @ 2021-06-14 10:07 ` Dong Aisheng 2021-06-14 14:51 ` Catalin Marinas 0 siblings, 1 reply; 7+ messages in thread From: Dong Aisheng @ 2021-06-14 10:07 UTC (permalink / raw) To: Will Deacon Cc: Dong Aisheng, iommu, open list, linux-mm, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, Christoph Hellwig, Marek Szyprowski, Robin Murphy, Catalin Marinas Hi Will, On Mon, Jun 14, 2021 at 4:36 PM Will Deacon <will@kernel.org> wrote: > > [+Catalin] > > On Fri, Jun 11, 2021 at 09:10:56PM +0800, Dong Aisheng wrote: > > Coherent dma on ARM64 also can't work with mapped system ram, > > that means 'no-map' property must be specified in dts. > > Add the missing check for ARM64 platforms as well. > > Besides 'no-map' checking, 'linux,dma-default' feature is also > > enabled for ARM64 along with this patch. > > Please can you explain _why_ it can't work? We don't need to tear down > aliases from the linear map for the streaming DMA API, so why is this > case different? Also, coherent devices wouldn't need this either way, > would they? What problem are you solving here? > Not sure if i get your point correctly. Here is my understanding. (fix me if wrong) In current implementation, the coherent dma memory will be remapped as writecombine and uncached type which can't reuse the linear mapping. The prerequisite to do this is the memory must not be mapped System RAM. e.g. reserved memory with no-map property and invisible to the buddy system. This seems a little different from CMA which the memory is still underlying managed by the buddy system in order to support migration. The patch here does not resolve a real issue but just open the sanity check for ARM64 case as well as ARM which reports the issue a little bit earlier at rmem_dma_setup() time. Regards Aisheng > Thanks, > > Will > > > > > Cc: Christoph Hellwig <hch@lst.de> > > Cc: Marek Szyprowski <m.szyprowski@samsung.com> > > Cc: Robin Murphy <robin.murphy@arm.com> > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > > --- > > kernel/dma/coherent.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c > > index 5b5b6c7ec7f2..d1831da7afba 100644 > > --- a/kernel/dma/coherent.c > > +++ b/kernel/dma/coherent.c > > @@ -356,7 +356,7 @@ static int __init rmem_dma_setup(struct reserved_mem *rmem) > > if (of_get_flat_dt_prop(node, "reusable", NULL)) > > return -EINVAL; > > > > -#ifdef CONFIG_ARM > > +#if defined(CONFIG_ARM) || defined(CONFIG_ARM64) > > if (!of_get_flat_dt_prop(node, "no-map", NULL)) { > > pr_err("Reserved memory: regions without no-map are not yet supported\n"); > > return -EINVAL; > > -- > > 2.25.1 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] dma: coherent: check no-map property for arm64 2021-06-14 10:07 ` Dong Aisheng @ 2021-06-14 14:51 ` Catalin Marinas 2021-06-14 15:34 ` Robin Murphy 0 siblings, 1 reply; 7+ messages in thread From: Catalin Marinas @ 2021-06-14 14:51 UTC (permalink / raw) To: Dong Aisheng Cc: Will Deacon, Dong Aisheng, iommu, open list, linux-mm, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, Christoph Hellwig, Marek Szyprowski, Robin Murphy On Mon, Jun 14, 2021 at 06:07:04PM +0800, Dong Aisheng wrote: > On Mon, Jun 14, 2021 at 4:36 PM Will Deacon <will@kernel.org> wrote: > > On Fri, Jun 11, 2021 at 09:10:56PM +0800, Dong Aisheng wrote: > > > Coherent dma on ARM64 also can't work with mapped system ram, > > > that means 'no-map' property must be specified in dts. > > > Add the missing check for ARM64 platforms as well. > > > Besides 'no-map' checking, 'linux,dma-default' feature is also > > > enabled for ARM64 along with this patch. > > > > Please can you explain _why_ it can't work? We don't need to tear down > > aliases from the linear map for the streaming DMA API, so why is this > > case different? Also, coherent devices wouldn't need this either way, > > would they? What problem are you solving here? > > > > Not sure if i get your point correctly. Here is my understanding. (fix > me if wrong) > In current implementation, the coherent dma memory will be remapped as > writecombine and uncached type which can't reuse the linear mapping. > The prerequisite to do this is the memory must not be mapped System RAM. > e.g. reserved memory with no-map property and invisible to the buddy system. The architecture allows the system RAM to be mapped in the linear map while there's another writecombine alias, as long as there are no dirty cache lines that could be evicted randomly. This works fine with the DMA API (and we have some cache maintenance when the non-cacheable mapping is first created). Looking at the rmem_dma_device_init() -> dma_init_coherent_memory(), it ends up calling memremap(MEMREMAP_WC) which would warn if it intersects with system RAM regardless of the architecture. If the memory region is nomap, it doesn't end up as IORESOURCE_SYSTEM_RAM, so memremap() won't warn. But why is this specific only to arm (or arm64)? Is the "shared-dma-pool" property only meant for Normal Non-cacheable memory (hence the MEMREMAP_WC flag)? If a system is fully cache coherent, does this check still make sense or the DT is not supposed to have such nodes? > This seems a little different from CMA which the memory is still > underlying managed by the buddy system in order to support migration. > > The patch here does not resolve a real issue but just open the sanity check for > ARM64 case as well as ARM which reports the issue a little bit earlier at > rmem_dma_setup() time. I think we first need to figure out what the real issue is and then try to solve it. -- Catalin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] dma: coherent: check no-map property for arm64 2021-06-14 14:51 ` Catalin Marinas @ 2021-06-14 15:34 ` Robin Murphy 2021-06-14 15:36 ` Christoph Hellwig 0 siblings, 1 reply; 7+ messages in thread From: Robin Murphy @ 2021-06-14 15:34 UTC (permalink / raw) To: Catalin Marinas, Dong Aisheng Cc: Will Deacon, Dong Aisheng, iommu, open list, linux-mm, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, Christoph Hellwig, Marek Szyprowski On 2021-06-14 15:51, Catalin Marinas wrote: > On Mon, Jun 14, 2021 at 06:07:04PM +0800, Dong Aisheng wrote: >> On Mon, Jun 14, 2021 at 4:36 PM Will Deacon <will@kernel.org> wrote: >>> On Fri, Jun 11, 2021 at 09:10:56PM +0800, Dong Aisheng wrote: >>>> Coherent dma on ARM64 also can't work with mapped system ram, >>>> that means 'no-map' property must be specified in dts. >>>> Add the missing check for ARM64 platforms as well. >>>> Besides 'no-map' checking, 'linux,dma-default' feature is also >>>> enabled for ARM64 along with this patch. Note that the "linux,dma-default" stuff is really only for NOMMU (with caches), so hardly relevant to arm64. >>> Please can you explain _why_ it can't work? We don't need to tear down >>> aliases from the linear map for the streaming DMA API, so why is this >>> case different? Also, coherent devices wouldn't need this either way, >>> would they? What problem are you solving here? >>> >> >> Not sure if i get your point correctly. Here is my understanding. (fix >> me if wrong) >> In current implementation, the coherent dma memory will be remapped as >> writecombine and uncached type which can't reuse the linear mapping. >> The prerequisite to do this is the memory must not be mapped System RAM. >> e.g. reserved memory with no-map property and invisible to the buddy system. > > The architecture allows the system RAM to be mapped in the linear map > while there's another writecombine alias, as long as there are no dirty > cache lines that could be evicted randomly. This works fine with the DMA > API (and we have some cache maintenance when the non-cacheable mapping > is first created). > > Looking at the rmem_dma_device_init() -> dma_init_coherent_memory(), it > ends up calling memremap(MEMREMAP_WC) which would warn if it intersects > with system RAM regardless of the architecture. If the memory region is > nomap, it doesn't end up as IORESOURCE_SYSTEM_RAM, so memremap() won't > warn. But why is this specific only to arm (or arm64)? Didn't some ARMv7 implementations permit unexpected cache hits for the non-cacheable address if the same PA has been speculatively fetched via the cacheable alias? > Is the "shared-dma-pool" property only meant for Normal Non-cacheable > memory (hence the MEMREMAP_WC flag)? If a system is fully cache > coherent, does this check still make sense or the DT is not supposed to > have such nodes? I don't think "shared-dma-pool" carries any particular expectation itself of how things are mapped, especially since "reusable" effectively implies a cacheable mapping for 'normal' kernel usage. Absent "reusable" to take things down the CMA path instead, "no-map" would currently be needed for coherent devices, since even when the CPU is guaranteed to bypass the cacheable alias the device can still inadvertently snoop it and see stale data. However if the device *is* coherent then it would seem more sensible to skip the remap entirely and just use the linear map address of the pool, unless of course it needs to be shared by multiple devices some of which are non-coherent... :/ >> This seems a little different from CMA which the memory is still >> underlying managed by the buddy system in order to support migration. >> >> The patch here does not resolve a real issue but just open the sanity check for >> ARM64 case as well as ARM which reports the issue a little bit earlier at >> rmem_dma_setup() time. > > I think we first need to figure out what the real issue is and then try > to solve it. Agreed. Robin. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] dma: coherent: check no-map property for arm64 2021-06-14 15:34 ` Robin Murphy @ 2021-06-14 15:36 ` Christoph Hellwig 2021-06-15 7:19 ` Marek Szyprowski 0 siblings, 1 reply; 7+ messages in thread From: Christoph Hellwig @ 2021-06-14 15:36 UTC (permalink / raw) To: Robin Murphy Cc: Catalin Marinas, Dong Aisheng, Will Deacon, Dong Aisheng, iommu, open list, linux-mm, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE, Christoph Hellwig, Marek Szyprowski On Mon, Jun 14, 2021 at 04:34:05PM +0100, Robin Murphy wrote: >> Looking at the rmem_dma_device_init() -> dma_init_coherent_memory(), it >> ends up calling memremap(MEMREMAP_WC) which would warn if it intersects >> with system RAM regardless of the architecture. If the memory region is >> nomap, it doesn't end up as IORESOURCE_SYSTEM_RAM, so memremap() won't >> warn. But why is this specific only to arm (or arm64)? > > Didn't some ARMv7 implementations permit unexpected cache hits for the > non-cacheable address if the same PA has been speculatively fetched via the > cacheable alias? If we care about that we need to change these platforms to change the cache attributes of the kernel direct mapping instead of using vmap. We already have code to do that for openrisc, someone just needs to write the glue code for other platforms. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/1] dma: coherent: check no-map property for arm64 2021-06-14 15:36 ` Christoph Hellwig @ 2021-06-15 7:19 ` Marek Szyprowski 0 siblings, 0 replies; 7+ messages in thread From: Marek Szyprowski @ 2021-06-15 7:19 UTC (permalink / raw) To: Christoph Hellwig, Robin Murphy Cc: Catalin Marinas, Dong Aisheng, Will Deacon, Dong Aisheng, iommu, open list, linux-mm, moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE Hi Christoph, On 14.06.2021 17:36, Christoph Hellwig wrote: > On Mon, Jun 14, 2021 at 04:34:05PM +0100, Robin Murphy wrote: >>> Looking at the rmem_dma_device_init() -> dma_init_coherent_memory(), it >>> ends up calling memremap(MEMREMAP_WC) which would warn if it intersects >>> with system RAM regardless of the architecture. If the memory region is >>> nomap, it doesn't end up as IORESOURCE_SYSTEM_RAM, so memremap() won't >>> warn. But why is this specific only to arm (or arm64)? >> Didn't some ARMv7 implementations permit unexpected cache hits for the >> non-cacheable address if the same PA has been speculatively fetched via the >> cacheable alias? > If we care about that we need to change these platforms to change the > cache attributes of the kernel direct mapping instead of using vmap. > We already have code to do that for openrisc, someone just needs to > write the glue code for other platforms. ARAIR there is a problem with changing cache attributes of the direct mappings on ARM 32bit. The whole lowmem is mapped with 2M 'section' mappings. Changing cache attributes causes 2 issues. First - one would need to split 2M entry into 4K entries. Second - 2M section mappings for the whole lowmem area are located in the per-process page tables. Changing the cache attributes would require locking all processes and iterating over their page table entries, which is a huge task. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-06-15 7:20 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-11 13:10 [PATCH 1/1] dma: coherent: check no-map property for arm64 Dong Aisheng 2021-06-14 8:36 ` Will Deacon 2021-06-14 10:07 ` Dong Aisheng 2021-06-14 14:51 ` Catalin Marinas 2021-06-14 15:34 ` Robin Murphy 2021-06-14 15:36 ` Christoph Hellwig 2021-06-15 7:19 ` Marek Szyprowski
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 NNTP newsgroup(s).