[PATCH] powerpc/dma: Fix invalid DMA mmap behavior

[Oliver O'Halloran]

On Thu, Jul 18, 2019 at 1:16 PM Shawn Anastasio <shawn at anastas.io> wrote:
>
> On 7/17/19 9:59 PM, Alexey Kardashevskiy wrote:
> >
> > On 18/07/2019 09:54, Shawn Anastasio wrote:
> >> The refactor of powerpc DMA functions in commit 6666cc17d780
> >> ("powerpc/dma: remove dma_nommu_mmap_coherent") incorrectly
> >> changes the way DMA mappings are handled on powerpc.
> >> Since this change, all mapped pages are marked as cache-inhibited
> >> through the default implementation of arch_dma_mmap_pgprot.
> >> This differs from the previous behavior of only marking pages
> >> in noncoherent mappings as cache-inhibited and has resulted in
> >> sporadic system crashes in certain hardware configurations and
> >> workloads (see Bugzilla).
> >>
> >> This commit restores the previous correct behavior by providing
> >> an implementation of arch_dma_mmap_pgprot that only marks
> >> pages in noncoherent mappings as cache-inhibited. As this behavior
> >> should be universal for all powerpc platforms a new file,
> >> dma-generic.c, was created to store it.
> >>
> >> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204145
> >> Fixes: 6666cc17d780 ("powerpc/dma: remove dma_nommu_mmap_coherent")
> >> Signed-off-by: Shawn Anastasio <shawn at anastas.io>
> >
> > Is this the default one?
> >
> > include/linux/dma-noncoherent.h
> > # define arch_dma_mmap_pgprot(dev, prot, attrs) pgprot_noncached(prot)
>
> Yep, that's the one.
>
> > Out of curiosity - do not we want to fix this one for everyone?
>
> Other than m68k, mips, and arm64, everybody else that doesn't have
> ARCH_NO_COHERENT_DMA_MMAP set uses this default implementation, so
> I assume this behavior is acceptable on those architectures.

It might be acceptable, but there's no reason to use pgport_noncached
if the platform supports cache-coherent DMA.

Christoph (+cc) made the change so maybe he saw something we're missing.

> >> ---
> >>   arch/powerpc/Kconfig             |  1 +
> >>   arch/powerpc/kernel/Makefile     |  3 ++-
> >>   arch/powerpc/kernel/dma-common.c | 17 +++++++++++++++++
> >>   3 files changed, 20 insertions(+), 1 deletion(-)
> >>   create mode 100644 arch/powerpc/kernel/dma-common.c
> >>
> >> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> >> index d8dcd8820369..77f6ebf97113 100644
> >> --- a/arch/powerpc/Kconfig
> >> +++ b/arch/powerpc/Kconfig
> >> @@ -121,6 +121,7 @@ config PPC
> >>       select ARCH_32BIT_OFF_T if PPC32
> >>       select ARCH_HAS_DEBUG_VIRTUAL
> >>       select ARCH_HAS_DEVMEM_IS_ALLOWED
> >> +    select ARCH_HAS_DMA_MMAP_PGPROT
> >>       select ARCH_HAS_ELF_RANDOMIZE
> >>       select ARCH_HAS_FORTIFY_SOURCE
> >>       select ARCH_HAS_GCOV_PROFILE_ALL
> >> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> >> index 56dfa7a2a6f2..ea0c69236789 100644
> >> --- a/arch/powerpc/kernel/Makefile
> >> +++ b/arch/powerpc/kernel/Makefile
> >> @@ -49,7 +49,8 @@ obj-y                := cputable.o ptrace.o
> >> syscalls.o \
> >>                      signal.o sysfs.o cacheinfo.o time.o \
> >>                      prom.o traps.o setup-common.o \
> >>                      udbg.o misc.o io.o misc_$(BITS).o \
> >> -                   of_platform.o prom_parse.o
> >> +                   of_platform.o prom_parse.o \
> >> +                   dma-common.o
> >>   obj-$(CONFIG_PPC64)        += setup_64.o sys_ppc32.o \
> >>                      signal_64.o ptrace32.o \
> >>                      paca.o nvram_64.o firmware.o
> >> diff --git a/arch/powerpc/kernel/dma-common.c
> >> b/arch/powerpc/kernel/dma-common.c
> >> new file mode 100644
> >> index 000000000000..5a15f99f4199
> >> --- /dev/null
> >> +++ b/arch/powerpc/kernel/dma-common.c
> >> @@ -0,0 +1,17 @@
> >> +// SPDX-License-Identifier: GPL-2.0-or-later
> >> +/*
> >> + * Contains common dma routines for all powerpc platforms.
> >> + *
> >> + * Copyright (C) 2019 Shawn Anastasio (shawn at anastas.io)
> >> + */
> >> +
> >> +#include <linux/mm.h>
> >> +#include <linux/dma-noncoherent.h>
> >> +
> >> +pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
> >> +        unsigned long attrs)
> >> +{
> >> +    if (!dev_is_dma_coherent(dev))
> >> +        return pgprot_noncached(prot);
> >> +    return prot;
> >> +}
> >>
> >