Re: [PATCH v3 2/4] powerpc/fadump: fix additional param memory reservation for HASH MMU

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

Hi Sourabh,

On 23/01/25 5:12 pm, Sourabh Jain wrote:
> Commit 683eab94da75bc ("powerpc/fadump: setup additional parameters for
> dump capture kernel") introduced the additional parameter feature in
> fadump for HASH MMU with the understanding that GRUB does not use the
> memory area between 640MB and 768MB for its operation.
> 
> However, the third patch in this series ("powerpc: increase MIN RMA
> size for CAS negotiation") changes the MIN RMA size to 768MB, allowing
> GRUB to use memory up to 768MB. This makes the fadump reservation for
> the additional parameter feature for HASH MMU unreliable.
> 
> To address this, adjust the memory range for the additional parameter in
> fadump for HASH MMU. This will ensure that GRUB does not overwrite the
> memory reserved for fadump's additional parameter in HASH MMU.
> 
> The new policy for the memory range for the additional parameter in HASH
> MMU is that the first memory block must be larger than the MIN_RMA size,
> as the bootloader can use memory up to the MIN_RMA size. The range
> should be between MIN_RMA and the RMA size (ppc64_rma_size), and it must
> not overlap with the fadump reserved area.
> 
> Cc: Avnish Chouhan <avnish@linux.ibm.com>
> Cc: Brian King <brking@linux.ibm.com>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Madhavan Srinivasan <maddy@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>   arch/powerpc/kernel/fadump.c | 21 +++++++++++----------
>   1 file changed, 11 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 4b371c738213..26e3d151e048 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -33,6 +33,7 @@
>   #include <asm/fadump-internal.h>
>   #include <asm/setup.h>
>   #include <asm/interrupt.h>
> +#include <asm/prom.h>
>   
>   /*
>    * The CPU who acquired the lock to trigger the fadump crash should
> @@ -1764,19 +1765,19 @@ void __init fadump_setup_param_area(void)
>   		range_end = memblock_end_of_DRAM();
>   	} else {
>   		/*
> -		 * Passing additional parameters is supported for hash MMU only
> -		 * if the first memory block size is 768MB or higher.
> +		 * Memory range for passing additional parameters for HASH MMU
> +		 * must meet the following conditions:
> +		 * 1. The first memory block size must be higher than the
> +		 *    minimum RMA (MIN_RMA) size. Bootloader can use memory

> +		 *    upto RMA size. So it should be avoided.

I believe you mean "upto MIN_RMA size." here?

Rest looks good.

Reviewed-by: Hari Bathini <hbathini@linux.ibm.com>


> +		 * 2. The range should be between MIN_RMA and RMA size (ppc64_rma_size)
> +		 * 3. It must not overlap with the fadump reserved area.
>   		 */
> -		if (ppc64_rma_size < 0x30000000)
> +		if (ppc64_rma_size < MIN_RMA*1024*1024)
>   			return;
>   
> -		/*
> -		 * 640 MB to 768 MB is not used by PFW/bootloader. So, try reserving
> -		 * memory for passing additional parameters in this range to avoid
> -		 * being stomped on by PFW/bootloader.
> -		 */
> -		range_start = 0x2A000000;
> -		range_end = range_start + 0x4000000;
> +		range_start = MIN_RMA * 1024 * 1024;
> +		range_end = min(ppc64_rma_size, fw_dump.boot_mem_top);
>   	}
>   
>   	fw_dump.param_area = memblock_phys_alloc_range(COMMAND_LINE_SIZE,