From: takahiro.akashi@linaro.org (AKASHI Takahiro) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH v22 2/8] arm64: limit memory regions based on DT property, usable-memory-range Date: Wed, 20 Jul 2016 13:22:05 +0900 [thread overview] Message-ID: <20160720042204.GI20774@linaro.org> (raw) In-Reply-To: <20160720033909.GA30575@arm.com> On Wed, Jul 20, 2016 at 11:39:11AM +0800, Dennis Chen wrote: > 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 I didn't say memblock_enforce_memory_limit(), but *your* memblock_memblock_remove_limit(). > 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, but I have already prototyped the function. If you don't agree to my opinion, I will have to submit a patch for a quite similar but different function. I think that nobody will accept this. -Takahiro AKASHI > Thanks, > Dennis > > > > > Thanks, > > -Takahiro AKASHI > > > > > > > > > > > > > > Thanks, > > > > -Takahiro AKASHI > > > > >
WARNING: multiple messages have this Message-ID (diff)
From: AKASHI Takahiro <takahiro.akashi@linaro.org> To: Dennis Chen <dennis.chen@arm.com> Cc: mark.rutland@arm.com, geoff@infradead.org, catalin.marinas@arm.com, will.deacon@arm.com, robh+dt@kernel.org, James Morse <james.morse@arm.com>, bauerman@linux.vnet.ibm.com, nd@arm.com, dyoung@redhat.com, kexec@lists.infradead.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v22 2/8] arm64: limit memory regions based on DT property, usable-memory-range Date: Wed, 20 Jul 2016 13:22:05 +0900 [thread overview] Message-ID: <20160720042204.GI20774@linaro.org> (raw) In-Reply-To: <20160720033909.GA30575@arm.com> On Wed, Jul 20, 2016 at 11:39:11AM +0800, Dennis Chen wrote: > 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 I didn't say memblock_enforce_memory_limit(), but *your* memblock_memblock_remove_limit(). > 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, but I have already prototyped the function. If you don't agree to my opinion, I will have to submit a patch for a quite similar but different function. I think that nobody will accept this. -Takahiro AKASHI > Thanks, > Dennis > > > > > Thanks, > > -Takahiro AKASHI > > > > > > > > > > > > > > Thanks, > > > > -Takahiro AKASHI > > > > > _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
next prev parent reply other threads:[~2016-07-20 4:22 UTC|newest] Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-07-12 5:05 [PATCH v22 0/8] arm64: add kdump support AKASHI Takahiro 2016-07-12 5:05 ` AKASHI Takahiro 2016-07-12 5:05 ` [PATCH v22 1/8] arm64: kdump: reserve memory for crash dump kernel AKASHI Takahiro 2016-07-12 5:05 ` AKASHI Takahiro 2016-07-13 9:12 ` Suzuki K Poulose 2016-07-13 9:12 ` Suzuki K Poulose 2016-07-13 15:42 ` AKASHI Takahiro 2016-07-13 15:42 ` AKASHI Takahiro 2016-07-19 9:39 ` Dennis Chen 2016-07-19 9:39 ` Dennis Chen 2016-07-19 10:28 ` AKASHI Takahiro 2016-07-19 10:28 ` AKASHI Takahiro 2016-07-19 10:41 ` Dennis Chen 2016-07-19 10:41 ` Dennis Chen 2016-07-19 12:48 ` Mark Salter 2016-07-19 12:48 ` Mark Salter 2016-07-19 13:27 ` Mark Rutland 2016-07-19 13:27 ` Mark Rutland 2016-07-20 2:17 ` AKASHI Takahiro 2016-07-20 2:17 ` AKASHI Takahiro 2016-07-20 3:48 ` Dennis Chen 2016-07-20 3:48 ` Dennis Chen 2016-07-19 23:34 ` AKASHI Takahiro 2016-07-19 23:34 ` AKASHI Takahiro 2016-07-12 5:05 ` [PATCH v22 2/8] arm64: limit memory regions based on DT property, usable-memory-range AKASHI Takahiro 2016-07-12 5:05 ` AKASHI Takahiro 2016-07-18 18:04 ` James Morse 2016-07-18 18:04 ` James Morse 2016-07-19 8:35 ` AKASHI Takahiro 2016-07-19 8:35 ` AKASHI Takahiro 2016-07-19 10:06 ` Dennis Chen 2016-07-19 10:06 ` Dennis Chen 2016-07-19 11:01 ` AKASHI Takahiro 2016-07-19 11:01 ` AKASHI Takahiro 2016-07-20 3:39 ` Dennis Chen 2016-07-20 3:39 ` Dennis Chen 2016-07-20 4:22 ` AKASHI Takahiro [this message] 2016-07-20 4:22 ` AKASHI Takahiro 2016-07-20 4:36 ` Dennis Chen 2016-07-20 4:36 ` Dennis Chen 2016-07-21 0:57 ` AKASHI Takahiro 2016-07-21 0:57 ` AKASHI Takahiro 2016-07-22 13:55 ` James Morse 2016-07-22 13:55 ` James Morse 2016-07-25 5:27 ` AKASHI Takahiro 2016-07-25 5:27 ` AKASHI Takahiro 2016-08-04 6:21 ` AKASHI Takahiro 2016-08-04 6:21 ` AKASHI Takahiro 2016-08-09 16:22 ` James Morse 2016-08-09 16:22 ` James Morse 2016-07-12 5:05 ` [PATCH v22 3/8] arm64: kdump: implement machine_crash_shutdown() AKASHI Takahiro 2016-07-12 5:05 ` AKASHI Takahiro 2016-07-13 9:32 ` Suzuki K Poulose 2016-07-13 9:32 ` Suzuki K Poulose 2016-07-13 16:00 ` AKASHI Takahiro 2016-07-13 16:00 ` AKASHI Takahiro 2016-07-12 5:05 ` [PATCH v22 4/8] arm64: kdump: add kdump support AKASHI Takahiro 2016-07-12 5:05 ` AKASHI Takahiro 2016-07-12 5:05 ` [PATCH v22 5/8] arm64: kdump: add VMCOREINFO's for user-space coredump tools AKASHI Takahiro 2016-07-12 5:05 ` AKASHI Takahiro 2016-07-12 5:05 ` [PATCH v22 6/8] arm64: kdump: enable kdump in the arm64 defconfig AKASHI Takahiro 2016-07-12 5:05 ` AKASHI Takahiro 2016-07-12 5:05 ` [PATCH v22 7/8] arm64: kdump: update a kernel doc AKASHI Takahiro 2016-07-12 5:05 ` AKASHI Takahiro 2016-07-12 5:05 ` [PATCH v22 8/8] Documentation: dt: chosen properties for arm64 kdump AKASHI Takahiro 2016-07-12 5:05 ` AKASHI Takahiro 2016-07-12 10:07 ` Mark Rutland 2016-07-12 10:07 ` Mark Rutland 2016-07-13 15:14 ` AKASHI Takahiro 2016-07-13 15:14 ` AKASHI Takahiro
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20160720042204.GI20774@linaro.org \ --to=takahiro.akashi@linaro.org \ --cc=linux-arm-kernel@lists.infradead.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.