Re: [PATCH] MIPS: Use common way to parse elfcorehdr

[Jinyang He <hejinyang@xxxxxxxxxxx>]

On 02/04/2021 11:28 AM, Randy Dunlap wrote:

> On 2/3/21 7:07 PM, Jinyang He wrote:
> > "elfcorehdr" can be parsed at kernel/crash_dump.c
> >
> > Signed-off-by: Jinyang He <hejinyang@xxxxxxxxxxx>
> Hm, looks like that would require CONFIG_CRASH_DUMP?
Hi,

I don't think so. We expect to generate a /proc/vmcore when we reserved
memory for elfcorehdr. So put it under CONFIG_PROC_VMCORE. If only rely
on CONFIG_CRASH_DUMP and CONFIG_PROC_VMCORE is not enabled, we will
reserve these memory while cannot use these later. And the reason why
kernel/crash_dump.c being under CONFIG_CRASH_DUMP is that it provide
is_kdump_kernel() for others.

Thanks,
Jinyang

> > ---
> >   arch/mips/kernel/setup.c | 49 +++++++++++++++++++++---------------------------
> >   1 file changed, 21 insertions(+), 28 deletions(-)
> >
> > diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> > index 7e1f8e2..4d2f915 100644
> > --- a/arch/mips/kernel/setup.c
> > +++ b/arch/mips/kernel/setup.c
> > @@ -29,6 +29,7 @@
> >   #include <linux/of_fdt.h>
> >   #include <linux/of_reserved_mem.h>
> >   #include <linux/dmi.h>
> > +#include <linux/crash_dump.h>
> >   
> >   #include <asm/addrspace.h>
> >   #include <asm/bootinfo.h>
> > @@ -404,34 +405,32 @@ static int __init early_parse_memmap(char *p)
> >   }
> >   early_param("memmap", early_parse_memmap);
> >   
> > -#ifdef CONFIG_PROC_VMCORE
> > -static unsigned long setup_elfcorehdr, setup_elfcorehdr_size;
> > -static int __init early_parse_elfcorehdr(char *p)
> > +static void __init mips_reserve_vmcore(void)
> >   {
> > +#ifdef CONFIG_PROC_VMCORE
> >   	phys_addr_t start, end;
> >   	u64 i;
> >   
> > -	setup_elfcorehdr = memparse(p, &p);
> > -
> > -	for_each_mem_range(i, &start, &end) {
> > -		if (setup_elfcorehdr >= start && setup_elfcorehdr < end) {
> > -			/*
> > -			 * Reserve from the elf core header to the end of
> > -			 * the memory segment, that should all be kdump
> > -			 * reserved memory.
> > -			 */
> > -			setup_elfcorehdr_size = end - setup_elfcorehdr;
> > -			break;
> > +	if (!elfcorehdr_size) {
> > +		for_each_mem_range(i, &start, &end) {
> > +			if (elfcorehdr_addr >= start && elfcorehdr_addr < end) {
> > +				/*
> > +				 * Reserve from the elf core header to the end of
> > +				 * the memory segment, that should all be kdump
> > +				 * reserved memory.
> > +				 */
> > +				elfcorehdr_size = end - elfcorehdr_addr;
> > +				break;
> > +			}
> >   		}
> >   	}
> > -	/*
> > -	 * If we don't find it in the memory map, then we shouldn't
> > -	 * have to worry about it, as the new kernel won't use it.
> > -	 */
> > -	return 0;
> > -}
> > -early_param("elfcorehdr", early_parse_elfcorehdr);
> > +
> > +	pr_info("Reserving %ldKB of memory at %ldKB for kdump\n",
> > +		(unsigned long)elfcorehdr_size >> 10, (unsigned long)elfcorehdr_addr >> 10);
> > +
> > +	memblock_reserve(elfcorehdr_addr, elfcorehdr_size);
> >   #endif
> > +}
> >   
> >   #ifdef CONFIG_KEXEC
> >   
> > @@ -653,13 +652,7 @@ static void __init arch_mem_init(char **cmdline_p)
> >   	 */
> >   	memblock_set_current_limit(PFN_PHYS(max_low_pfn));
> >   
> > -#ifdef CONFIG_PROC_VMCORE
> > -	if (setup_elfcorehdr && setup_elfcorehdr_size) {
> > -		printk(KERN_INFO "kdump reserved memory at %lx-%lx\n",
> > -		       setup_elfcorehdr, setup_elfcorehdr_size);
> > -		memblock_reserve(setup_elfcorehdr, setup_elfcorehdr_size);
> > -	}
> > -#endif
> > +	mips_reserve_vmcore();
> >   
> >   	mips_parse_crashkernel();
> >   #ifdef CONFIG_KEXEC