Re: [PATCH v2 2/2] fadump: reserve param area if below boot_mem_top

["Ritesh Harjani (IBM)" <ritesh.list@gmail.com>]

Sourabh Jain <sourabhjain@linux.ibm.com> writes:

> The param area is a memory region where the kernel places additional
> command-line arguments for fadump kernel. Currently, the param memory
> area is reserved in fadump kernel if it is above boot_mem_top. However,
> it should be reserved if it is below boot_mem_top because the fadump
> kernel already reserves memory from boot_mem_top to the end of DRAM.

did you mean s/reserves/preserves ?

>
> Currently, there is no impact from not reserving param memory if it is
> below boot_mem_top, as it is not used after the early boot phase of the
> fadump kernel. However, if this changes in the future, it could lead to
> issues in the fadump kernel.

This will only affect Hash and not radix correct? Because for radix your
param_area is somewhere within [memblock_end_of_DRAM() / 2, memblock_end_of_DRAM()]
which is anyway above boot_mem_top so it is anyway preserved as is... 

... On second thoughts since param_area during normal kernel boot anyway
comes from memblock now. And irrespective of where it falls (above or below
boot_mem_top), we anyway append the bootargs to that. So we don't really
preserve the original contents :) right? So why not just always call for
memblock_reserve() on param_area during capture kernel run?

Thoughts?

>
> Fixes: 3416c9daa6b1 ("powerpc/fadump: pass additional parameters when fadump is active")
> Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Acked-by: Hari Bathini <hbathini@linux.ibm.com>
> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
> ---
>
> Changelog:
>
> Since v1: https://lore.kernel.org/all/20241104083528.99520-1-sourabhjain@linux.ibm.com/
>   - Include Fixes and Acked-by tag in the commit message
>   - No functional changes
>
> ---
>  arch/powerpc/kernel/fadump.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c
> index 3a2863307863..3f3674060164 100644
> --- a/arch/powerpc/kernel/fadump.c
> +++ b/arch/powerpc/kernel/fadump.c
> @@ -143,7 +143,7 @@ void __init fadump_append_bootargs(void)
>  	if (!fw_dump.dump_active || !fw_dump.param_area_supported || !fw_dump.param_area)
>  		return;
>  
> -	if (fw_dump.param_area >= fw_dump.boot_mem_top) {
> +	if (fw_dump.param_area < fw_dump.boot_mem_top) {
>  		if (memblock_reserve(fw_dump.param_area, COMMAND_LINE_SIZE)) {
>  			pr_warn("WARNING: Can't use additional parameters area!\n");
>  			fw_dump.param_area = 0;
> -- 
> 2.46.2