Re: [PATCH 2/2] arm-nommu: Add use_reserved_mem() to check if device support reserved memory

[dillon min]

Hi Vladimir,

Thanks for reviewing.

Hi Christoph Hellwig,

I just want to know if kernel dma mapping/direct is focused on
platforms with MMU.
leave arch code to handle dma coherent memory management themself for
no-MMU platform.

so, you just return error code in kernel/dma/mapping.c,direct.c
without CONFIG_MMU defined ?
which means dma-direct mapping doesn't support !CONFIG_MMU is not a
bug, but design as it's.
or, just return error code currently, will add dma direct mapping
support for !CONFIG_MMU in the
future?

As Vladimir Murzin's suggestion has changes in kernel code, I need
your input to get
the design goal about dma-direct mapping, thanks.

On Tue, Jun 9, 2020 at 10:07 PM Vladimir Murzin <vladimir.murzin@xxxxxxx> wrote:
>
> On 6/8/20 9:30 AM, dillon.minfei@xxxxxxxxx wrote:
> > From: dillon min <dillon.minfei@xxxxxxxxx>
> >
> > Currently, we use dma direct to request coherent memory for driver on armv7m
> > platform if 'cacheid' is zero, but dma_direct_can_mmap() is return false,
> > dma_direct_mmap() return -ENXIO for CONFIG_MMU undefined platform.
> >
> > so we have to back to use 'arm_nommu_dma_ops', add use_reserved_mem() to check
> > if device support global or device corherent memory. if yes, then call
> > set_dma_ops()
> >
> > Signed-off-by: dillon min <dillon.minfei@xxxxxxxxx>
> > ---
> >  arch/arm/mm/dma-mapping-nommu.c | 28 +++++++++++++++++++++++++++-
> >  1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
> > index 287ef898a55e..e1c213fec152 100644
> > --- a/arch/arm/mm/dma-mapping-nommu.c
> > +++ b/arch/arm/mm/dma-mapping-nommu.c
> > @@ -14,6 +14,7 @@
> >  #include <asm/cacheflush.h>
> >  #include <asm/outercache.h>
> >  #include <asm/cp15.h>
> > +#include <linux/of.h>
> >
> >  #include "dma.h"
> >
> > @@ -188,6 +189,31 @@ const struct dma_map_ops arm_nommu_dma_ops = {
> >  };
> >  EXPORT_SYMBOL(arm_nommu_dma_ops);
> >
> > +static bool use_reserved_mem(struct device *dev)
> > +{
> > +     struct device_node *np;
> > +
> > +     np = of_find_node_by_path("/reserved-memory/linux,dma");
> > +
> > +     if (np &&
> > +             of_device_is_compatible(np, "shared-dma-pool") &&
> > +             of_property_read_bool(np, "no-map") &&
> > +             of_property_read_bool(np, "linux,dma-default")) {
> > +             /* has global corherent mem support */
> > +             of_node_put(np);
> > +             return true;
> > +     }
> > +
> > +     np = of_parse_phandle(dev->of_node, "memory-region", 0);
> > +     if (np) {
> > +             /* has dev corherent mem support */
> > +             of_node_put(np);
> > +             return true;
> > +     }
> > +
> > +     return false;
> > +}
> > +
> >  void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> >                       const struct iommu_ops *iommu, bool coherent)
> >  {
> > @@ -206,6 +232,6 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> >               dev->archdata.dma_coherent = (get_cr() & CR_M) ? coherent : true;
> >       }
> >
> > -     if (!dev->archdata.dma_coherent)
> > +     if (!dev->archdata.dma_coherent || use_reserved_mem(dev))
> >               set_dma_ops(dev, &arm_nommu_dma_ops);
> >  }
> >
>
> Sorry I have to NAK this hack :(
>
> Digging git history reveled 79964a1c2972 ("ARM: 8633/1: nommu: allow mmap when !CONFIG_MMU")
> which make me wonder if diff below does the trick for you
>
> diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
> index 8f4bbda..8623b9e 100644
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -456,14 +456,14 @@ int dma_direct_mmap(struct device *dev, struct vm_area_struct *vma,
>  #else /* CONFIG_MMU */
>  bool dma_direct_can_mmap(struct device *dev)
>  {
> -       return false;
> +       return true;
>  }
>
>  int dma_direct_mmap(struct device *dev, struct vm_area_struct *vma,
>                 void *cpu_addr, dma_addr_t dma_addr, size_t size,
>                 unsigned long attrs)
>  {
> -       return -ENXIO;
> +       return vm_iomap_memory(vma, vma->vm_start, (vma->vm_end - vma->vm_start));;
>  }
>  #endif /* CONFIG_MMU */
Yes, this is a quite nice way to support !CONFIG_MMU without cache on
platforms. I will try your suggestion.
thanks

>
> Cheers
> Vladimir