Re: [PATCH v2] powerpc/fadump: set an upper limit for boot memory size

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

Hi Michael,


On Friday 24 February 2017 05:54 PM, Hari Bathini wrote:
> By default, 5% of system RAM is reserved for preserving boot memory.
> Alternatively, a user can specify the amount of memory to reserve.
> See Documentation/powerpc/firmware-assisted-dump.txt for details. In
> addition to the memory reserved for preserving boot memory, some more
> memory is reserved, to save HPTE region, CPU state data and ELF core
> headers.
>
> Memory Reservation during first kernel looks like below:
>
>    Low memory                                        Top of memory
>    0      boot memory size                                       |
>    |           |                       |<--Reserved dump area -->|
>    V           V                       |   Permanent Reservation V
>    +-----------+----------/ /----------+---+----+-----------+----+
>    |           |                       |CPU|HPTE|  DUMP     |ELF |
>    +-----------+----------/ /----------+---+----+-----------+----+
>          |                                           ^
>          |                                           |
>          \                                           /
>           -------------------------------------------
>            Boot memory content gets transferred to
>            reserved area by firmware at the time of
>            crash
>
> This implicitly means that the sum of the sizes of boot memory, CPU
> state data, HPTE region, DUMP preserving area and ELF core headers
> can't be greater than the total memory size. But currently, a user is
> allowed to specify any value as boot memory size. So, the above rule
> is violated when a boot memory size around 50% of the total available
> memory is specified. As the kernel is not handling this currently, it
> may lead to undefined behavior. Fix it by setting an upper limit for
> boot memory size to 25% of the total available memory. Also, instead
> of using memblock_end_of_DRAM(), which doesn't take the holes, if any,
> in the memory layout into account, use memblock_phys_mem_size() to
> calculate the percentage of total available memory.
>
> Signed-off-by: Hari Bathini <hbathini@linux.vnet.ibm.com>
> ---
>
> This patch is based on top of the patchset to reuse-crashkernel-parameter-
> for-fadump (http://patchwork.ozlabs.org/patch/711522).

The above mentioned patch-set is upstream.
Can you please review this patch.

Thanks
Hari

> Changes from v1:
> * Using memblock_phys_mem_size() instead of memblock_end_of_DRAM() to
>    get system RAM size.
>
>
>   arch/powerpc/include/asm/fadump.h |    3 +++
>   arch/powerpc/kernel/fadump.c      |   16 +++++++++++++++-
>   2 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/fadump.h b/arch/powerpc/include/asm/fadump.h
> index 60b9108..a3de219 100644
> --- a/arch/powerpc/include/asm/fadump.h
> +++ b/arch/powerpc/include/asm/fadump.h
> @@ -43,6 +43,9 @@
>   #define MIN_BOOT_MEM	(((RMA_END < (0x1UL << 28)) ? (0x1UL << 28) : RMA_END) \
>   			+ (0x1UL << 26))
>
> +/* The upper limit percentage for user specified boot memory size (25%) */
> +#define MAX_BOOT_MEM_RATIO			4
> +
>   #define memblock_num_regions(memblock_type)	(memblock.memblock_type.cnt)
>
>   /* Firmware provided dump sections */
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index e013f8f..21d5404 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -221,12 +221,26 @@ static inline unsigned long fadump_calculate_reserve_size(void)
>   	ret = parse_crashkernel(boot_command_line, memblock_phys_mem_size(),
>   				&size, &base);
>   	if (ret == 0 && size > 0) {
> +		unsigned long max_size;
> +
>   		fw_dump.reserve_bootvar = (unsigned long)size;
> +
> +		/*
> +		 * Adjust if the boot memory size specified is above
> +		 * the upper limit.
> +		 */
> +		max_size = memblock_phys_mem_size() / MAX_BOOT_MEM_RATIO;
> +		if (fw_dump.reserve_bootvar > max_size) {
> +			fw_dump.reserve_bootvar = max_size;
> +			pr_info("Adjusted boot memory size to %luMB\n",
> +				(fw_dump.reserve_bootvar >> 20));
> +		}
> +
>   		return fw_dump.reserve_bootvar;
>   	}
>
>   	/* divide by 20 to get 5% of value */
> -	size = memblock_end_of_DRAM() / 20;
> +	size = memblock_phys_mem_size() / 20;
>
>   	/* round it down in multiples of 256 */
>   	size = size & ~0x0FFFFFFFUL;
>