Re: [PATCH 1/1] dma: coherent: check no-map property for arm64

From: Dong Aisheng <dongas86@gmail.com>
To: Will Deacon <will@kernel.org>
Cc: Dong Aisheng <aisheng.dong@nxp.com>,
	iommu@lists.linux-foundation.org,
	open list <linux-kernel@vger.kernel.org>,
	linux-mm@kvack.org,
	"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
	<linux-arm-kernel@lists.infradead.org>,
	Christoph Hellwig <hch@lst.de>,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Robin Murphy <robin.murphy@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>
Subject: Re: [PATCH 1/1] dma: coherent: check no-map property for arm64
Date: Mon, 14 Jun 2021 18:07:04 +0800	[thread overview]
Message-ID: <CAA+hA=S8x4S0sgAiJbqOC-wgtOshV_jhAq6eVqX5-EAeg3Dczg@mail.gmail.com> (raw)
In-Reply-To: <20210614083609.GA18701@willie-the-truck>

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
> >