Re: [PATCH v2 15/16] powernv/fadump: consider f/w load area

From: Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com>
To: Hari Bathini <hbathini@linux.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@linux.ibm.com>,
	Mahesh J Salgaonkar <mahesh@linux.ibm.com>,
	Vasant Hegde <hegdevasant@linux.ibm.com>,
	linuxppc-dev <linuxppc-dev@ozlabs.org>,
	Nicholas Piggin <npiggin@gmail.com>,
	Stewart Smith <stewart@linux.ibm.com>,
	Daniel Axtens <dja@axtens.net>
Subject: Re: [PATCH v2 15/16] powernv/fadump: consider f/w load area
Date: Tue, 7 May 2019 22:43:31 +0530	[thread overview]
Message-ID: <20190507171331.p5wwzc3asvpkltxb@in.ibm.com> (raw)
In-Reply-To: <155541097094.812.18328895014763068053.stgit@hbathini.in.ibm.com>

On 2019-04-16 16:06:13 Tue, Hari Bathini wrote:
> OPAL loads kernel & initrd at 512MB offset (256MB size), also exported
> as ibm,opal/dump/fw-load-area. So, if boot memory size of FADump is
> less than 768MB, kernel memory to be exported as '/proc/vmcore' would
> be overwritten by f/w while loading kernel & initrd. To avoid such a
> scenario, enforce a minimum boot memory size of 768MB on OPAL platform.
> 
> Also, skip using FADump if a newer F/W version loads kernel & initrd
> above 768MB.
> 
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>  arch/powerpc/kernel/fadump-common.h          |   15 +++++++++++++--
>  arch/powerpc/kernel/fadump.c                 |    8 ++++++++
>  arch/powerpc/platforms/powernv/opal-fadump.c |   23 +++++++++++++++++++++++
>  3 files changed, 44 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/fadump-common.h b/arch/powerpc/kernel/fadump-common.h
> index 1bd3aeb..f59fdc7 100644
> --- a/arch/powerpc/kernel/fadump-common.h
> +++ b/arch/powerpc/kernel/fadump-common.h
> @@ -24,14 +24,25 @@
>  #define RMA_END		(ppc64_rma_size)
>  
>  /*
> + * With kernel & initrd loaded at 512MB (with 256MB size), enforce a minimum
> + * boot memory size of 768MB to ensure f/w loading kernel and initrd doesn't
> + * mess with crash'ed kernel's memory during MPIPL.
> + */
> +#define OPAL_MIN_BOOT_MEM	(0x30000000UL)
> +
> +/*
>   * On some Power systems where RMO is 128MB, it still requires minimum of
>   * 256MB for kernel to boot successfully. When kdump infrastructure is
>   * configured to save vmcore over network, we run into OOM issue while
>   * loading modules related to network setup. Hence we need additional 64M
>   * of memory to avoid OOM issue.
>   */
> -#define MIN_BOOT_MEM	(((RMA_END < (0x1UL << 28)) ? (0x1UL << 28) : RMA_END) \
> -			+ (0x1UL << 26))
> +#define PSERIES_MIN_BOOT_MEM	(((RMA_END < (0x1UL << 28)) ? (0x1UL << 28) : \
> +				 RMA_END) + (0x1UL << 26))
> +
> +#define MIN_BOOT_MEM	((fw_dump.fadump_platform ==			\
> +			 FADUMP_PLATFORM_POWERNV) ? OPAL_MIN_BOOT_MEM :	\
> +			 PSERIES_MIN_BOOT_MEM)

Can we hide this behind fadump_ops.get_bootmem_min() instead of common code
doing platform check ?

>  
>  /* The upper limit percentage for user specified boot memory size (25%) */
>  #define MAX_BOOT_MEM_RATIO			4
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index ba26169..3c3adc2 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -582,6 +582,14 @@ int __init fadump_reserve_mem(void)
>  				ALIGN(fw_dump.boot_memory_size,
>  							FADUMP_CMA_ALIGNMENT);
>  #endif
> +
> +		if ((fw_dump.fadump_platform == FADUMP_PLATFORM_POWERNV) &&
> +		    (fw_dump.boot_memory_size < OPAL_MIN_BOOT_MEM)) {

and here too.. fadump_ops.validate_bootmem_size() ? push platform specific
stuff behind fadump_ops.

Rest looks good.

Thanks,
-Mahesh.