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

[Hari Bathini <hbathini@linux.ibm.com>]

On 07/05/19 10:43 PM, Mahesh J Salgaonkar wrote:
> 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.

Actually, it would be fine to make this check for both platforms.
So, fadump_ops.get_bootmem_min() callback can be used at both places, I guess..

- Hari