Re: [PATCH 1/1] dma: coherent: check no-map property for arm64
Hi Will,
On Mon, Jun 14, 2021 at 4:36 PM Will Deacon 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
> > Cc: Marek Szyprowski
> > Cc: Robin Murphy
> > Signed-off-by: Dong Aisheng
> > ---
> > 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
> >
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/1] dma: coherent: check no-map property for arm64
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
Cc: Marek Szyprowski
Cc: Robin Murphy
Signed-off-by: Dong Aisheng
---
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
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] dma-contiguous: return early for dt case in dma_contiguous_reserve
On Tue, May 18, 2021 at 7:29 PM Dong Aisheng wrote:
>
> dma_contiguous_reserve() aims to support cmdline case for CMA memory
> reserve. But if users define reserved memory in DT,
> 'dma_contiguous_default_area' will not be 0, then it's meaningless
> to continue to run dma_contiguous_reserve(). So we return early
> if detect 'dma_contiguous_default_area' is unzero.
>
> Cc: Christoph Hellwig
> Cc: Marek Szyprowski
> Cc: Robin Murphy
> Signed-off-by: Dong Aisheng
Gently ping
Regards
Aisheng
> ---
> kernel/dma/contiguous.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> index 3d63d91cba5c..ebade9f43eff 100644
> --- a/kernel/dma/contiguous.c
> +++ b/kernel/dma/contiguous.c
> @@ -171,6 +171,9 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
> phys_addr_t selected_limit = limit;
> bool fixed = false;
>
> + if (dma_contiguous_default_area)
> + return;
> +
> pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
>
> if (size_cmdline != -1) {
> @@ -191,7 +194,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
> #endif
> }
>
> - if (selected_size && !dma_contiguous_default_area) {
> + if (selected_size) {
> pr_debug("%s: reserving %ld MiB for global area\n", __func__,
> (unsigned long)selected_size / SZ_1M);
>
> --
> 2.25.1
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] dma-contiguous: return early for dt case in dma_contiguous_reserve
On Wed, May 19, 2021 at 2:51 AM Robin Murphy wrote:
>
> On 2021-05-18 12:28, Dong Aisheng wrote:
> > dma_contiguous_reserve() aims to support cmdline case for CMA memory
> > reserve. But if users define reserved memory in DT,
> > 'dma_contiguous_default_area' will not be 0, then it's meaningless
> > to continue to run dma_contiguous_reserve(). So we return early
> > if detect 'dma_contiguous_default_area' is unzero.
>
> But dma_contiguous_default_area *shouldn't* be set if the command-line
> argument is present - see the "if (size_cmdline != -1 && default_cma)"
> part of rmem_cma_setup(). Are you seeing something different in practice?
>
yes, you're right.
> > Cc: Christoph Hellwig
> > Cc: Marek Szyprowski
> > Cc: Robin Murphy
> > Signed-off-by: Dong Aisheng
> > ---
> > kernel/dma/contiguous.c | 5 -
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> > index 3d63d91cba5c..ebade9f43eff 100644
> > --- a/kernel/dma/contiguous.c
> > +++ b/kernel/dma/contiguous.c
> > @@ -171,6 +171,9 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
> > phys_addr_t selected_limit = limit;
> > bool fixed = false;
> >
> > + if (dma_contiguous_default_area)
> > + return;
> > +
> > pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
> >
> > if (size_cmdline != -1) {
> > @@ -191,7 +194,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
> > #endif
> > }
> >
> > - if (selected_size && !dma_contiguous_default_area) {
> > + if (selected_size) {
>
> Either way, does skipping a handful of trivial calculations and a
> debugging message really matter even when it is redundant? I can't
> imagine it has any measurable effect on boot times...
>
I think it's not about performance.
It aims to improve the code readability as it's meaningless to continue to
execute cmdline CMA reserve logic once DT is used successfully which is
a bit confusing when people first read this part of code.
Does it make sense to you?
Regards
Aisheng
> Robin.
>
> > pr_debug("%s: reserving %ld MiB for global area\n", __func__,
> >(unsigned long)selected_size / SZ_1M);
> >
> >
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/1] dma-contiguous: return early for dt case in dma_contiguous_reserve
dma_contiguous_reserve() aims to support cmdline case for CMA memory
reserve. But if users define reserved memory in DT,
'dma_contiguous_default_area' will not be 0, then it's meaningless
to continue to run dma_contiguous_reserve(). So we return early
if detect 'dma_contiguous_default_area' is unzero.
Cc: Christoph Hellwig
Cc: Marek Szyprowski
Cc: Robin Murphy
Signed-off-by: Dong Aisheng
---
kernel/dma/contiguous.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
index 3d63d91cba5c..ebade9f43eff 100644
--- a/kernel/dma/contiguous.c
+++ b/kernel/dma/contiguous.c
@@ -171,6 +171,9 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
phys_addr_t selected_limit = limit;
bool fixed = false;
+ if (dma_contiguous_default_area)
+ return;
+
pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);
if (size_cmdline != -1) {
@@ -191,7 +194,7 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
#endif
}
- if (selected_size && !dma_contiguous_default_area) {
+ if (selected_size) {
pr_debug("%s: reserving %ld MiB for global area\n", __func__,
(unsigned long)selected_size / SZ_1M);
--
2.25.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
