[PATCH] arch/arm64: elfcorehdr should be the first allocation

[Poonam Aggrwal]

Thanks Takahiro and Will,

Please find responses below.

Regards
Poonam
> -----Original Message-----
> From: AKASHI Takahiro [mailto:takahiro.akashi at linaro.org]
> Sent: Wednesday, December 13, 2017 4:17 PM
> To: Abhimanyu Saini <abhimanyu.saini at nxp.com>
> Cc: Will Deacon <will.deacon at arm.com>; Prabhakar Kushwaha
> <prabhakar.kushwaha at nxp.com>; linux-arm-kernel at lists.infradead.org;
> Poonam Aggrwal <poonam.aggrwal at nxp.com>; G.h. Gao
> <guanhua.gao at nxp.com>; james.morse at arm.com
> Subject: Re: [PATCH] arch/arm64: elfcorehdr should be the first allocation
> 
> On Mon, Dec 11, 2017 at 02:07:14PM +0000, Will Deacon wrote:
> > On Mon, Dec 11, 2017 at 11:03:32AM +0530, Prabhakar Kushwaha wrote:
> > > From: Abhimanyu Saini <abhimanyu.saini at nxp.com>
> > >
> > > elfcorehdr_addr is assigned by kexec-utils and device tree of dump
> > > kernel is fixed in chosen node with parameter "linux,elfcorehdr".
> > > So, memory should be first reserved for elfcorehdr, otherwise
> > > overlaps may happen with other memory allocations which were done
> > > before the allocation of elcorehdr in the crash kernel
> >
> > What happens in that case? Do you have a crash log we can include in
> > the commit message?
> 
> In private discussions with Poonam, he said:
> |   The overlap here I observed was for the reserved-mem areas in the dtb.
> |   And they were specific to NXP device.
Thanks Takahiro for providing this information.
Yes the memory overlap happens because when the dump-capture kernel tries to reserve elfcoreheader at the specific address, it finds that the address has already been reserved for some other use by early_init_fdt_scan_reserved_mem().
Based on the "reserved-memory" node entries in the dts file , the overlap is seen to happen with one of the below alocations:
[    0.000000] Reserved memory: created DMA memory pool at 0x00000000fb800000, size 4 MiB
[    0.000000] Reserved memory: initialized node qman-fqd, compatible id shared-dma-pool
[    0.000000] Reserved memory: created DMA memory pool at 0x00000000f8000000, size 32 MiB
[    0.000000] Reserved memory: initialized node qman-pfdr, compatible id shared-dma-pool
> 
> Since I have not got any details since then, I'm not sure whether your patch is
> the way to go.
> (I suspect that we might better fix the issue on kexec-tools side.)
> 
I may not have full understanding of kexec-tools, but I am not sure how will kexec tools be able to know what addresses will get  assigned for the reserved-memory regions in the device tree. 
> Thanks,
> -Takahiro AKASHI
> 
> > > Signed-off-by: Guanhua <guanhua.gao at nxp.com>
> > > Signed-off-by: Poonam Aggrwal <poonam.aggrwal at nxp.com>
> > > Signed-off-by: Abhimanyu Saini <abhimanyu.saini at nxp.com>
> > > ---
> >
> > Really? How on Earth did you get three people co-developing this patch?
 : )Contribution was from all in terms of reproducing, testing on various platforms including the debug experiments. Just wanted to acknowledge the effort.
> >
> > >  arch/arm64/mm/init.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index
> > > 5960bef0170d..551048cfcfff 100644
> > > --- a/arch/arm64/mm/init.c
> > > +++ b/arch/arm64/mm/init.c
> > > @@ -453,6 +453,10 @@ void __init arm64_memblock_init(void)
> > >  	 * Register the kernel text, kernel data, initrd, and initial
> > >  	 * pagetables with memblock.
> > >  	 */
> > > +
> > > +	/* make this the first reservation so that there are no chances of
> > > +	 * overlap */
> > > +	reserve_elfcorehdr();
> > >  	memblock_reserve(__pa_symbol(_text), _end - _text);  #ifdef
> > > CONFIG_BLK_DEV_INITRD
> > >  	if (initrd_start) {
> > > @@ -474,8 +478,6 @@ void __init arm64_memblock_init(void)
> > >
> > >  	reserve_crashkernel();
> > >
> > > -	reserve_elfcorehdr();
> >
> > Why isn't this also a problem for reserve_crashkernel() or any other
> > static reservations?
Thanks, for raising this. Yes looking at the code flow this may also cause a problem, definitely when the start address of the crash kernel is specified in the bootargs. We mostly tested with just the size, so it was an allocation without a specific address.

> >
> > Will