Re: [PATCH v3] ARM: Parse kdump DT properties

[Geert Uytterhoeven]

Hi Rob,

On Mon, Mar 22, 2021 at 5:59 PM Rob Herring <robh+dt@xxxxxxxxxx> wrote:
> On Wed, Mar 17, 2021 at 5:31 AM Geert Uytterhoeven
> <geert+renesas@xxxxxxxxx> wrote:
> > Parse the following DT properties in the crash dump kernel, to provide a
> > modern interface between kexec and the crash dump kernel:
> >   - linux,elfcorehdr: ELF core header segment, similar to the
> >     "elfcorehdr=" kernel parameter.
> >   - linux,usable-memory-range: Usable memory reserved for the crash dump
> >     kernel.
> >     This makes the memory reservation explicit.  If present, Linux no
> >     longer needs to mask the program counter, and rely on the "mem="
> >     kernel parameter to obtain the start and size of usable memory.
> >
> > For backwards compatibility, the traditional method to derive the start
> > of memory is still used if "linux,usable-memory-range" is absent, and
> > the "elfcorehdr=" and "mem=" kernel parameters are still parsed.
> >
> > Loosely based on the ARM64 version by Akashi Takahiro.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

> > --- a/arch/arm/mm/init.c
> > +++ b/arch/arm/mm/init.c
> > @@ -4,6 +4,7 @@
> >   *
> >   *  Copyright (C) 1995-2005 Russell King
> >   */
> > +#include <linux/crash_dump.h>
> >  #include <linux/kernel.h>
> >  #include <linux/errno.h>
> >  #include <linux/swap.h>
> > @@ -210,8 +211,95 @@ void check_cpu_icache_size(int cpuid)
> >  }
> >  #endif
> >
> > +#ifdef CONFIG_OF_EARLY_FLATTREE
> > +static int __init early_init_dt_scan_usablemem(unsigned long node,
> > +               const char *uname, int depth, void *data)
> > +{
> > +       struct memblock_region *usablemem = data;
> > +       const __be32 *reg;
> > +       int len;
> > +
> > +       if (depth != 1 || strcmp(uname, "chosen") != 0)
> > +               return 0;
>
> We have libfdt now, just get the '/chosen' node rather than using
> of_scan_flat_dt().
>
>
> > +
> > +       reg = of_get_flat_dt_prop(node, "linux,usable-memory-range", &len);
> > +       if (!reg || (len < (dt_root_addr_cells + dt_root_size_cells)))
> > +               return 1;
> > +
> > +       usablemem->base = dt_mem_next_cell(dt_root_addr_cells, &reg);
> > +       usablemem->size = dt_mem_next_cell(dt_root_size_cells, &reg);
> > +       return 1;
> > +}
> > +
> > +static void __init fdt_enforce_memory_region(void)
> > +{
> > +       struct memblock_region reg = {
> > +               .size = 0,
> > +       };
> > +
> > +       of_scan_flat_dt(early_init_dt_scan_usablemem, &reg);
> > +
> > +       if (reg.size)
> > +               memblock_cap_memory_range(reg.base, reg.size);
>
> We should be able to do this in the DT core code. It doesn't matter
> that these properties are arm* only. Other arches won't find the
> properties.
>
> Also, note that there is now a drivers/of/kexec.c (in -next) though
> not sure if all this would go there or stay in fdt.c with the rest of
> the memory parsing.

It's gonna be the latter, as that file handles the FDT during early
kernel startup, for both normal and kdump kernels.

Despite the name, drivers/of/kexec.c is not for kexec, but for
kexec_file.  This is the "new" fancy syscall that prepares the DTB
for the new kernel itself, unlike the classic kexec syscall, where
userspace is responsible for preparing the DTB for the new kernel.

> > +#if defined(CONFIG_CRASH_DUMP) && defined(CONFIG_OF_EARLY_FLATTREE)
> > +static int __init early_init_dt_scan_elfcorehdr(unsigned long node,
> > +               const char *uname, int depth, void *data)
>
> Same comments as above.

This one can indeed be handled easily by drivers/of/fdt.c.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds