| Message ID | 20170201124630.6016-2-takahiro.akashi@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Wed, Feb 01, 2017 at 09:46:22PM +0900, AKASHI Takahiro wrote: > +#ifdef CONFIG_KEXEC_CORE > +/* > + * reserve_crashkernel() - reserves memory for crash kernel > + * > + * This function reserves memory area given in "crashkernel=" kernel command > + * line parameter. The memory reserved is used by dump capture kernel when > + * primary kernel is crashing. > + */ > +static void __init reserve_crashkernel(void) > +{ > + unsigned long long crash_base, crash_size; > + int ret; > + > + ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(), > + &crash_size, &crash_base); > + /* no crashkernel= or invalid value specified */ > + if (ret || !crash_size) > + return; > + > + crash_size = PAGE_ALIGN(crash_size); > + > + if (crash_base == 0) { > + /* Current arm64 boot protocol requires 2MB alignment */ > + crash_base = memblock_find_in_range(0, ARCH_LOW_ADDRESS_LIMIT, > + crash_size, SZ_2M); > + if (crash_base == 0) { > + pr_warn("Unable to allocate crashkernel (size:%llx)\n", > + crash_size); Nit: can we please make that "size: 0x%llx", so that it's always clearly a hex number? e.g. pr_warn("cannot allocate 0x%llx bytes for crashkernel\n", crash_size); > + return; > + } > + } else { > + /* User specifies base address explicitly. */ > + if (!memblock_is_region_memory(crash_base, crash_size) || > + memblock_is_region_reserved(crash_base, crash_size)) { > + pr_warn("crashkernel has wrong address or size\n"); > + return; > + } To better report the error, can we please split these up: if (!memblock_is_region_memory(crash_base, crash_size)) { pr_warn("cannot reserve crashkernel: region is not memory\n"); return; } if (!memblock_is_region_memory(crash_base, crash_size)) { pr_warn("cannot reserve crashkernel: region overlaps reserved memory\n"); return; } > + if (!IS_ALIGNED(crash_base, SZ_2M)) { > + pr_warn("crashkernel base address is not 2MB aligned\n"); > + return; > + } > + } > + memblock_reserve(crash_base, crash_size); > + > + pr_info("Reserving %lldMB of memory at %lldMB for crashkernel\n", > + crash_size >> 20, crash_base >> 20); We only page-align the size, so the MB will be a little off, but that's probably OK. However, it would also be nicer to log the base as an address. Could we dump this as we do for the kernel memory layout? e.g. pr_info("crashkernel reserved: 0x%016lx - 0x%016lx (%lld MB)\n", crash_base, crash_base + crash_size, crash_size >> 20); With those: Acked-by: Mark Rutland <mark.rutland@arm.com> Thanks, Mark.
On Wed, Feb 01, 2017 at 03:26:09PM +0000, Mark Rutland wrote: > Hi, > > On Wed, Feb 01, 2017 at 09:46:22PM +0900, AKASHI Takahiro wrote: > > +#ifdef CONFIG_KEXEC_CORE > > +/* > > + * reserve_crashkernel() - reserves memory for crash kernel > > + * > > + * This function reserves memory area given in "crashkernel=" kernel command > > + * line parameter. The memory reserved is used by dump capture kernel when > > + * primary kernel is crashing. > > + */ > > +static void __init reserve_crashkernel(void) > > +{ > > + unsigned long long crash_base, crash_size; > > + int ret; > > + > > + ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(), > > + &crash_size, &crash_base); > > + /* no crashkernel= or invalid value specified */ > > + if (ret || !crash_size) > > + return; > > + > > + crash_size = PAGE_ALIGN(crash_size); > > + > > + if (crash_base == 0) { > > + /* Current arm64 boot protocol requires 2MB alignment */ > > + crash_base = memblock_find_in_range(0, ARCH_LOW_ADDRESS_LIMIT, > > + crash_size, SZ_2M); > > + if (crash_base == 0) { > > + pr_warn("Unable to allocate crashkernel (size:%llx)\n", > > + crash_size); > > Nit: can we please make that "size: 0x%llx", so that it's always clearly > a hex number? e.g. > > pr_warn("cannot allocate 0x%llx bytes for crashkernel\n", > crash_size); OK > > + return; > > + } > > + } else { > > + /* User specifies base address explicitly. */ > > + if (!memblock_is_region_memory(crash_base, crash_size) || > > + memblock_is_region_reserved(crash_base, crash_size)) { > > + pr_warn("crashkernel has wrong address or size\n"); > > + return; > > + } > > To better report the error, can we please split these up: > > if (!memblock_is_region_memory(crash_base, crash_size)) { > pr_warn("cannot reserve crashkernel: region is not memory\n"); > return; > } > > if (!memblock_is_region_memory(crash_base, crash_size)) { > pr_warn("cannot reserve crashkernel: region overlaps reserved memory\n"); > return; > } OK, and > > + if (!IS_ALIGNED(crash_base, SZ_2M)) { > > + pr_warn("crashkernel base address is not 2MB aligned\n"); pr_warn("cannot reserve crashkernel: base address is not 2MB aligned\n"); > > + return; > > + } > > + } > > + memblock_reserve(crash_base, crash_size); > > + > > + pr_info("Reserving %lldMB of memory at %lldMB for crashkernel\n", > > + crash_size >> 20, crash_base >> 20); > > We only page-align the size, so the MB will be a little off, but that's > probably OK. However, it would also be nicer to log the base as an > address. You might notice that the exact same message is used by all the other architectures, but > Could we dump this as we do for the kernel memory layout? e.g. > > pr_info("crashkernel reserved: 0x%016lx - 0x%016lx (%lld MB)\n", > crash_base, crash_base + crash_size, crash_size >> 20); We can go either way. > With those: > > Acked-by: Mark Rutland <mark.rutland@arm.com> Thanks, -Takahiro AKASHI > Thanks, > Mark.
On Thu, Feb 02, 2017 at 01:52:36PM +0900, AKASHI Takahiro wrote: > On Wed, Feb 01, 2017 at 03:26:09PM +0000, Mark Rutland wrote: > > On Wed, Feb 01, 2017 at 09:46:22PM +0900, AKASHI Takahiro wrote: > > > + pr_info("Reserving %lldMB of memory at %lldMB for crashkernel\n", > > > + crash_size >> 20, crash_base >> 20); > > > > We only page-align the size, so the MB will be a little off, but that's > > probably OK. However, it would also be nicer to log the base as an > > address. > > You might notice that the exact same message is used by all the other > architectures, but Almost all; I see arch/sh prints the address with %08x. ;) > > Could we dump this as we do for the kernel memory layout? e.g. > > > > pr_info("crashkernel reserved: 0x%016lx - 0x%016lx (%lld MB)\n", > > crash_base, crash_base + crash_size, crash_size >> 20); > > We can go either way. Even if it's different from other archtiectures, I'd prefer to log as above, with the range in hex. Thanks, Mark.
On Thu, Feb 02, 2017 at 11:26:20AM +0000, Mark Rutland wrote: > On Thu, Feb 02, 2017 at 01:52:36PM +0900, AKASHI Takahiro wrote: > > On Wed, Feb 01, 2017 at 03:26:09PM +0000, Mark Rutland wrote: > > > On Wed, Feb 01, 2017 at 09:46:22PM +0900, AKASHI Takahiro wrote: > > > > + pr_info("Reserving %lldMB of memory at %lldMB for crashkernel\n", > > > > + crash_size >> 20, crash_base >> 20); > > > > > > We only page-align the size, so the MB will be a little off, but that's > > > probably OK. However, it would also be nicer to log the base as an > > > address. > > > > You might notice that the exact same message is used by all the other > > architectures, but > > Almost all; I see arch/sh prints the address with %08x. ;) > > > > Could we dump this as we do for the kernel memory layout? e.g. > > > > > > pr_info("crashkernel reserved: 0x%016lx - 0x%016lx (%lld MB)\n", > > > crash_base, crash_base + crash_size, crash_size >> 20); > > > > We can go either way. > > Even if it's different from other archtiectures, I'd prefer to log as > above, with the range in hex. OK. -Takhiro AKASHI > Thanks, > Mark.
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c index b051367e2149..515e9c6696df 100644 --- a/arch/arm64/kernel/setup.c +++ b/arch/arm64/kernel/setup.c @@ -31,7 +31,6 @@ #include <linux/screen_info.h> #include <linux/init.h> #include <linux/kexec.h> -#include <linux/crash_dump.h> #include <linux/root_dev.h> #include <linux/cpu.h> #include <linux/interrupt.h> @@ -224,6 +223,12 @@ static void __init request_standard_resources(void) if (kernel_data.start >= res->start && kernel_data.end <= res->end) request_resource(res, &kernel_data); +#ifdef CONFIG_KEXEC_CORE + /* Userspace will find "Crash kernel" region in /proc/iomem. */ + if (crashk_res.end && crashk_res.start >= res->start && + crashk_res.end <= res->end) + request_resource(res, &crashk_res); +#endif } } diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index 6cddb566eb21..2aba75dc7720 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -30,12 +30,14 @@ #include <linux/gfp.h> #include <linux/memblock.h> #include <linux/sort.h> +#include <linux/of.h> #include <linux/of_fdt.h> #include <linux/dma-mapping.h> #include <linux/dma-contiguous.h> #include <linux/efi.h> #include <linux/swiotlb.h> #include <linux/vmalloc.h> +#include <linux/kexec.h> #include <asm/boot.h> #include <asm/fixmap.h> @@ -76,6 +78,63 @@ static int __init early_initrd(char *p) early_param("initrd", early_initrd); #endif +#ifdef CONFIG_KEXEC_CORE +/* + * reserve_crashkernel() - reserves memory for crash kernel + * + * This function reserves memory area given in "crashkernel=" kernel command + * line parameter. The memory reserved is used by dump capture kernel when + * primary kernel is crashing. + */ +static void __init reserve_crashkernel(void) +{ + unsigned long long crash_base, crash_size; + int ret; + + ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(), + &crash_size, &crash_base); + /* no crashkernel= or invalid value specified */ + if (ret || !crash_size) + return; + + crash_size = PAGE_ALIGN(crash_size); + + if (crash_base == 0) { + /* Current arm64 boot protocol requires 2MB alignment */ + crash_base = memblock_find_in_range(0, ARCH_LOW_ADDRESS_LIMIT, + crash_size, SZ_2M); + if (crash_base == 0) { + pr_warn("Unable to allocate crashkernel (size:%llx)\n", + crash_size); + return; + } + } else { + /* User specifies base address explicitly. */ + if (!memblock_is_region_memory(crash_base, crash_size) || + memblock_is_region_reserved(crash_base, crash_size)) { + pr_warn("crashkernel has wrong address or size\n"); + return; + } + + if (!IS_ALIGNED(crash_base, SZ_2M)) { + pr_warn("crashkernel base address is not 2MB aligned\n"); + return; + } + } + memblock_reserve(crash_base, crash_size); + + pr_info("Reserving %lldMB of memory at %lldMB for crashkernel\n", + crash_size >> 20, crash_base >> 20); + + crashk_res.start = crash_base; + crashk_res.end = crash_base + crash_size - 1; +} +#else +static void __init reserve_crashkernel(void) +{ +} +#endif /* CONFIG_KEXEC_CORE */ + /* * Return the maximum physical address for ZONE_DMA (DMA_BIT_MASK(32)). It * currently assumes that for memory starting above 4G, 32-bit devices will @@ -331,6 +390,9 @@ void __init arm64_memblock_init(void) arm64_dma_phys_limit = max_zone_dma_phys(); else arm64_dma_phys_limit = PHYS_MASK + 1; + + reserve_crashkernel(); + dma_contiguous_reserve(arm64_dma_phys_limit); memblock_allow_resize();