Hi Poonam, James On Sat, Jan 13, 2018 at 8:37 AM, Poonam Aggrwal <poonam.aggrwal@xxxxxxx> wrote: > Hi James > > Regards > Poonam > >> -----Original Message----- >> From: James Morse [mailto:james.morse@xxxxxxx] >> Sent: Friday, January 12, 2018 11:29 PM >> To: Poonam Aggrwal <poonam.aggrwal@xxxxxxx> >> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; takahiro.akashi@xxxxxxxxxx; >> will.deacon@xxxxxxx; G.h. Gao <guanhua.gao@xxxxxxx>; Abhimanyu Saini >> <abhimanyu.saini@xxxxxxx>; kexec@xxxxxxxxxxxxxxxxxxx >> Subject: Re: [PATCH][v2] arm64: Allocate elfcorehdr & crashkernel mem before >> any reservation >> >> Hi Poonam, >> >> On 08/01/18 04:31, Poonam Aggrwal wrote: >> > James Morse wrote: >> >> On 04/01/18 15:34, Poonam Aggrwal wrote: >> >>> Address for both crashkernel memory and elfcorehdr can be assigned >> >>> statically. For crashkernel memory it is via >> >>> crashkernel=SIZE@ADDRESS and elfcorehdr_addr via by kexec-utils in dump >> kernel device tree. >> >> >> >> There are some crashkernel=SIZE@ADDRESS values that the user can >> >> supply that we must reject. The obvious one is if it overlaps with >> >> the kernel text. (this patch won't spot this). We need to read the >> >> hardware's reserved regions from the DT before we allocate the >> >> crashkernel region, for example if the bootloader reserved a chunk of >> >> memory for a frame-buffer, I shouldn't be able to use that as crashkernel >> memory. >> >> >> >> (you shouldn't need to specify an address, doing so prevents the >> >> kernel from picking memory it can use) >> >> >> >> >> >>> So memory should be reserved for both the above areas before any >> >>> other memory reservations are done. Otherwise overlaps may happen >> >>> and memory allocations will fail leading to failure in booting the >> >>> dump capture kernel. >> >> >>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index >> >>> 00e7b90..24ce15d 100644 >> >>> --- a/arch/arm64/mm/init.c >> >>> +++ b/arch/arm64/mm/init.c >> >>> @@ -453,6 +453,14 @@ void __init arm64_memblock_init(void) >> >> >>> + reserve_elfcorehdr(); >> >> >> >> (Moving reserve_elfcorehdr() makes sense, but..) >> >> >> >> >> >>> + reserve_crashkernel(); >> >> >> >> reserve_crashkernel() does the allocation for the crashkernels reserved >> memory. >> >> I expect this to always fail in the kdump kernel because there isn't >> >> enough memory. (fdt_enforce_memory_region() at the top of this >> >> function calls memblock_cap_memory_range()). >> >> >> >> Moving this allocation above the early_init_fdt_scan_reserved_mem() >> >> below means we may allocate memory for the crashdump that is in use >> >> by firmware/hardware and described as reserved in the DT. >> >> > Yeah, this is a good point. So ideally the address of the crash kernel >> > should be diligently provided by the user based on the system. >> >> Even better: the region to store the crash kernel in should be chosen by the >> kernel. >> When using kdump I boot with 'crashkernel=1G', the kernel chooses where to >> place the reserved region. > Right. This is a commonly used. > Even if I specified a reasonable physical address, the >> efistub may relocate the kernel over the top during boot as part of its KASLR >> work. > Agree >> >> (Why does anyone ever need to specify an offset here?) > offset is normally an optional argument. Request Takahiro to provide his inputs. Does this imply any updates in kexec design/implementation/documentation? offset is a optional argument. For relocatable kernels (and kernels which support KASLR), specifying offset is normally not needed. Please refer to the 'Extended crashkernel syntax' documentation (<https://github.com/torvalds/linux/blob/master/Documentation/kdump/kdump.txt#L259>): Extended crashkernel syntax =========================== While the "crashkernel=size[@offset]" syntax is sufficient for most configurations, sometimes it's handy to have the reserved memory dependent on the value of System RAM -- that's mostly for distributors that pre-setup the kernel command line to avoid a unbootable system after some memory has been removed from the machine. The syntax is: crashkernel=<range1>:<size1>[,<range2>:<size2>,...][@offset] range=start-[end] For example: crashkernel=512M-2G:64M,2G-:128M This would mean: 1) if the RAM is smaller than 512M, then don't reserve anything (this is the "rescue" case) 2) if the RAM size is between 512M and 2G (exclusive), then reserve 64M 3) if the RAM size is larger than 2G, then reserve 128M As James mentioned for arm64, in case of relocatable/kaslr kernels, the efistub may relocate the kernel over the top during boot as part of its KASLR. So, the offset field may make more sense for non-relocatable/static kernels, but for newer kernels, its better to use the 'Extended crashkernel syntax' syntax which is also supported by newer distribution versions. For e.g. see ubuntu trusty kdump-config man page - <http://manpages.ubuntu.com/manpages/trusty/man8/kdump-config.8.html>: kdump kernel relocation address does not match crashkernel= parameter: For non-relocatable architectures, the kdump kernel must be built with a predetermined start address. This message indicates that the start address of the kdump kernel and the start address in the crashkernel= parameter do not match. Regards, Bhupesh _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec
