On Tue, Jul 19, 2016 at 08:01:21PM +0900, AKASHI Takahiro wrote:
> On Tue, Jul 19, 2016 at 06:06:18PM +0800, Dennis Chen wrote:
> > Hello AKASHI,
> >
> > On Tue, Jul 19, 2016 at 05:35:55PM +0900, AKASHI Takahiro wrote:
> > > James,
> > >
> > > On Mon, Jul 18, 2016 at 07:04:33PM +0100, James Morse wrote:
> > > > Hi!
> > > >
> > > > (CC: Dennis Chen)
> > > >
> > > > On 12/07/16 06:05, AKASHI Takahiro wrote:
> > > > > Crash dump kernel will be run with a limited range of memory as System
> > > > > RAM.
> > > > >
> > > > > On arm64, we will use a device-tree property under /chosen,
> > > > > linux,usable-memory-range = <BASE SIZE>
> > > > > in order for primary kernel either on uefi or non-uefi (device tree only)
> > > > > system to hand over the information about usable memory region to crash
> > > > > dump kernel. This property will supercede entries in uefi memory map table
> > > > > and "memory" nodes in a device tree.
> > > >
> > > > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > > > > index 51b1302..d8b296f 100644
> > > > > --- a/arch/arm64/mm/init.c
> > > > > +++ b/arch/arm64/mm/init.c
> > > > > @@ -300,10 +300,48 @@ static int __init early_mem(char *p)
> > > > > }
> > > > > early_param("mem", early_mem);
> > > > >
> > > > > +static int __init early_init_dt_scan_usablemem(unsigned long node,
> > > > > + const char *uname, int depth, void *data)
> > > > > +{
> > > > > + struct memblock_region *usablemem = (struct memblock_region *)data;
> > > > > + const __be32 *reg;
> > > > > + int len;
> > > > > +
> > > > > + usablemem->size = 0;
> > > > > +
> > > > > + if (depth != 1 || strcmp(uname, "chosen") != 0)
> > > > > + return 0;
> > > > > +
> > > > > + 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, ®);
> > > > > + usablemem->size = dt_mem_next_cell(dt_root_size_cells, ®);
> > > > > +
> > > > > + return 1;
> > > > > +}
> > > > > +
> > > > > +static void __init fdt_enforce_memory_region(void)
> > > > > +{
> > > > > + struct memblock_region reg;
> > > > > +
> > > > > + of_scan_flat_dt(early_init_dt_scan_usablemem, ®);
> > > > > +
> > > > > + if (reg.size) {
> > > > > + memblock_remove(0, PAGE_ALIGN(reg.base));
> > > > > + memblock_remove(round_down(reg.base + reg.size, PAGE_SIZE),
> > > > > + ULLONG_MAX);
> > > >
> >
> > According to the panic message from James, I guess the ACPI regions are out of the range
> > [reg.base, reg.base + reg.size] and removed by your above codes. On ARM64, those ACPI
> > regions have been added into memblock and marked as NOMAP, so I think it should be
> > easy to adapt my fix to retain the NOMAP regions here
>
> See below.
>
> > Thanks,
> > Dennis
> >
> > > > I think this is a new way to trip the problem Dennis Chen has been working on
> > > > [0]. If I kdump with --reuse-cmdline on a kernel booted with 'acpi=on', I get
> > > > the panic below [1]...
> > >
> > > Yeah, it can be.
> > >
> > > > It looks like Dennis's fix involves changes in mm/memblock.c, maybe they can be
> > > > extended to support a range instead of just a limit?
> > > >
> > > > (It looks like x86 explicitly adds the acpi regions to the crash-kernels memory
> > > > map in crash_setup_memmap_entries()).
> > > >
> > > >
> > > >
> > > > Is it possible for the kernel text to be outside this range? (a bug in
> > > > kexec-tools, or another user of the DT property) If we haven't already failed in
> > > > this case, it may be worth printing a warning, or refusing to
> > > > restrict-memory/expose-vmcore.
> > >
> > > In my implementation of kdump, the usable memory for crash dump
> > > kernel will be allocated within memblock.memory after ACPI-related
> > > regions have been mapped. "linux,usable-memory-range" indicates
> > > this exact memory range.
> > > On crash dump kernel, my fdt_enforce_memory_region() in arm64_memblock_init()
> > > will exclude all the other regions from memblock.memory.
> > > So the kernel (with acpi=on) won't recognize ACPI-regions as
> > > normal memory, and map them by ioremap().
> > >
> > > I thought that it was safe, but actually not due to unaligned accesses.
> > > As you suggested, we will probably be able to do the same thing of
> > > Chen's solution in fdt_enforce_memory_region().
>
> memblock_isolate_range() and memblock_remove_range() are not exported.
> So we'd better implement an unified interface like:
> memblock_cap_memory_range(phys_addr_t base, size_t size);
>
> If base == NULL, it would behave in the exact same way as your
> memblock_mem_limit_remove_map().
>
Hello AKASHI, it's not reasonable to change the prototype of an existing memblock API
which will be used by other components as we did with memblock_enforce_memory_limit.
Moreover the @size in you case is to specify a memory range of the memblock, which is
different from the @limit as an indicator of the total size of memblocks being limited.
But I can be help to post an new memblock API patch to cater for your case.
Thanks,
Dennis
>
> Thanks,
> -Takahiro AKASHI
>
> > >
> > >
> > > Thanks,
> > > -Takahiro AKASHI
> > >
