[PATCH 0/2] powerpc/fadump: fix additional parameter for HASH MMU

[PATCH 0/2] powerpc/fadump: fix additional parameter for HASH MMU

[Sourabh Jain]

Patch https://lore.kernel.org/all/20241206065545.14815-1-avnish@linux.ibm.com/
proposed the MIN RMA change to 768MB, which impacts the fadump additional
parameter feature for HASH MMU. Here’s how:

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 patch ("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 additional parameter in HASH MMU.

The first patch exports the MIN_RMA so that fadump can dynamically
decide the suitable memory range for the additional parameter. This will
help us avoid similar fadump changes in the future.

The second patch includes changes that actually fix the issue described
above.

Note:
This patch series is dependent on the below patch:
https://lore.kernel.org/all/20241206065545.14815-1-avnish@linux.ibm.com/

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>
Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Cc: Sourabh Jain <sourabhjain@linux.ibm.com>

Sourabh Jain (2):
  powerpc: export MIN RMA size
  powerpc/fadump: fix additional param memory reservation for HASH MMU

 arch/powerpc/include/asm/prom.h |  2 ++
 arch/powerpc/kernel/fadump.c    | 21 +++++++++++----------
 arch/powerpc/kernel/prom_init.c |  2 +-
 3 files changed, 14 insertions(+), 11 deletions(-)

-- 
2.47.1

[PATCH 1/2] powerpc: export MIN RMA size

[Sourabh Jain]

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 patch ("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, export the MIN_RMA so that the next patch
("powerpc/fadump: fix additional param memory reservation for HASH MMU")
can identify the correct memory range for the additional parameter
feature in fadump for HASH MMU.

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>
Cc: Mahesh Salgaonkar <mahesh@linux.ibm.com>
Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
---
 arch/powerpc/include/asm/prom.h | 2 ++
 arch/powerpc/kernel/prom_init.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index c0107d8ddd8c..09d0a4b1eb84 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -17,6 +17,8 @@
 struct device_node;
 struct property;
 
+#define MIN_RMA			768		/* Minimum RMA for CAS negotiation */
+
 #define OF_DT_BEGIN_NODE	0x1		/* Start of node, full name */
 #define OF_DT_END_NODE		0x2		/* End node */
 #define OF_DT_PROP		0x3		/* Property: name off, size,
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 0ddaa4403095..2ad9e9eee513 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -1061,7 +1061,7 @@ static const struct ibm_arch_vec ibm_architecture_vec_template __initconst = {
 		.virt_base = cpu_to_be32(0xffffffff),
 		.virt_size = cpu_to_be32(0xffffffff),
 		.load_base = cpu_to_be32(0xffffffff),
-		.min_rma = cpu_to_be32(768),		/* 768MB min RMA */
+		.min_rma = cpu_to_be32(MIN_RMA),	/* 768MB min RMA */
 		.min_load = cpu_to_be32(0xffffffff),	/* full client load */
 		.min_rma_percent = 0,	/* min RMA percentage of total RAM */
 		.max_pft_size = 48,	/* max log_2(hash table size) */
-- 
2.47.1

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

[Sourabh Jain]

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 patch ("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>
Cc: 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..5831f3ec8561 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
+		 *    up to RMA size. So it should be avoided.
+		 * 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,
-- 
2.47.1

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

[Mahesh J Salgaonkar]

On 2025-01-20 23:05:00 Mon, 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 patch ("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.

I see. Please include above patch in single patch series, with this
change as first patch.

> 
> 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>
> Cc: 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..5831f3ec8561 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
> +		 *    up to RMA size. So it should be avoided.
> +		 * 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);

Please update fadump documentation which makes this restriction clear.

Rest looks good to me.

Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>

Thanks,
-Mahesh

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

[Hari Bathini]

On 20/01/25 11:05 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 patch ("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.

IIRC, even memory above MIN_RMA is used by the bootloader except for
640MB to 768MB (assuming RMA size is >768MB). So, how does this change
guarantee that the bootloader is not using memory reserved for bootargs?

Avnish, earlier, bootloader was using RUNTIME_MIN_SPACE (128MB) starting
top-down at 768MB earlier. With MIN_RMA changed to 768MB, is bootloader
still using the concept of RUNTIME_MIN_SPACE to set aside some memory
for kernel to use. If yes, where exactly is it allocating this space
now? Also, rtas instantiates top-down at 768MB. Would that not have
a conflict with grub allocations without RUNTIME_MIN_SPACE at 768MB?

- Hari

> 
> 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>
> Cc: 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..5831f3ec8561 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
> +		 *    up to RMA size. So it should be avoided.
> +		 * 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,

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

[Avnish Chouhan]

On 2025-01-23 15:26, Hari Bathini wrote:
> On 20/01/25 11:05 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 patch ("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.
> 
> IIRC, even memory above MIN_RMA is used by the bootloader except for
> 640MB to 768MB (assuming RMA size is >768MB). So, how does this change
> guarantee that the bootloader is not using memory reserved for 
> bootargs?
> 
> Avnish, earlier, bootloader was using RUNTIME_MIN_SPACE (128MB) 
> starting
> top-down at 768MB earlier. With MIN_RMA changed to 768MB, is bootloader
> still using the concept of RUNTIME_MIN_SPACE to set aside some memory
> for kernel to use. If yes, where exactly is it allocating this space
> now? Also, rtas instantiates top-down at 768MB. Would that not have
> a conflict with grub allocations without RUNTIME_MIN_SPACE at 768MB?
> 
> - Hari

Hi Hari,

The RUNTIME_MIN_SPACE is the space left aside by Grub is within the 
MIN_RMA size. Grub won't use memory beyond the MIN_RMA. With this 
change, we haven't changed the RUNTIME_MIN_SPACE behavior. Grub will 
still keep the 128 MB space in MIN_RMA for loading stock kernel and 
initrd.

For your RTAS query, as it gets initiated just below the MIN_RMA. So it 
will not have any impact with this RMA size change.
**
When MIN_RMA is 768MB, rtas will be instantiate at 0x000000002ec50000 
(approximately at 748 MB).
**

Thank you!

Regards,
Avnish Chouhan

> 
>> 
>> 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>
>> Cc: 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..5831f3ec8561 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
>> +		 *    up to RMA size. So it should be avoided.
>> +		 * 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,

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

[Sourabh Jain]

Hello Mahesh,


On 23/01/25 12:28, Mahesh J Salgaonkar wrote:
> On 2025-01-20 23:05:00 Mon, 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 patch ("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.
> I see. Please include above patch in single patch series, with this
> change as first patch.

Thanks for the suggestion. I posted v3 with both patches combined:
https://lore.kernel.org/all/20250123114254.200527-1-sourabhjain@linux.ibm.com/


>
>> 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>
>> Cc: 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..5831f3ec8561 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
>> +		 *    up to RMA size. So it should be avoided.
>> +		 * 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);
> Please update fadump documentation which makes this restriction clear.

The v3 patch series has a patch that update fadump documentation:
https://lore.kernel.org/all/20250123114254.200527-5-sourabhjain@linux.ibm.com/

>
> Rest looks good to me.
>
> Reviewed-by: Mahesh Salgaonkar <mahesh@linux.ibm.com>

Thanks for the review and suggestion.

- Sourabh Jain

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

[Hari Bathini]

On 23/01/25 7:54 pm, Avnish Chouhan wrote:
> On 2025-01-23 15:26, Hari Bathini wrote:
>> On 20/01/25 11:05 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 patch ("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.
>>
>> IIRC, even memory above MIN_RMA is used by the bootloader except for
>> 640MB to 768MB (assuming RMA size is >768MB). So, how does this change
>> guarantee that the bootloader is not using memory reserved for bootargs?
>>
>> Avnish, earlier, bootloader was using RUNTIME_MIN_SPACE (128MB) starting
>> top-down at 768MB earlier. With MIN_RMA changed to 768MB, is bootloader
>> still using the concept of RUNTIME_MIN_SPACE to set aside some memory
>> for kernel to use. If yes, where exactly is it allocating this space
>> now? Also, rtas instantiates top-down at 768MB. Would that not have
>> a conflict with grub allocations without RUNTIME_MIN_SPACE at 768MB?
>>
>> - Hari
> 
> Hi Hari,

Hi Avnish,

> The RUNTIME_MIN_SPACE is the space left aside by Grub is within the 
> MIN_RMA size. Grub won't use memory beyond the MIN_RMA. With this 
> change, we haven't changed the RUNTIME_MIN_SPACE behavior. Grub will 
> still keep the 128 MB space in MIN_RMA for loading stock kernel and initrd.

IIUC, you mean, 640MB to 768MB is not used by Grub even if MIN_RMA
is at 768MB? If that is true, this change is not needed, as fadump
could still use the memory between 640MB to 768MB, right?
Am I missing something here..

- Hari

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

[Avnish Chouhan]

On 2025-01-31 20:44, Hari Bathini wrote:
> On 23/01/25 7:54 pm, Avnish Chouhan wrote:
>> On 2025-01-23 15:26, Hari Bathini wrote:
>>> On 20/01/25 11:05 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 patch ("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.
>>> 
>>> IIRC, even memory above MIN_RMA is used by the bootloader except for
>>> 640MB to 768MB (assuming RMA size is >768MB). So, how does this 
>>> change
>>> guarantee that the bootloader is not using memory reserved for 
>>> bootargs?
>>> 
>>> Avnish, earlier, bootloader was using RUNTIME_MIN_SPACE (128MB) 
>>> starting
>>> top-down at 768MB earlier. With MIN_RMA changed to 768MB, is 
>>> bootloader
>>> still using the concept of RUNTIME_MIN_SPACE to set aside some memory
>>> for kernel to use. If yes, where exactly is it allocating this space
>>> now? Also, rtas instantiates top-down at 768MB. Would that not have
>>> a conflict with grub allocations without RUNTIME_MIN_SPACE at 768MB?
>>> 
>>> - Hari
>> 
>> Hi Hari,
> 
> Hi Avnish,
> 
>> The RUNTIME_MIN_SPACE is the space left aside by Grub is within the 
>> MIN_RMA size. Grub won't use memory beyond the MIN_RMA. With this 
>> change, we haven't changed the RUNTIME_MIN_SPACE behavior. Grub will 
>> still keep the 128 MB space in MIN_RMA for loading stock kernel and 
>> initrd.
> 
> IIUC, you mean, 640MB to 768MB is not used by Grub even if MIN_RMA
> is at 768MB? If that is true, this change is not needed, as fadump
> could still use the memory between 640MB to 768MB, right?
> Am I missing something here..

Hari,

No. As we are changing MIN_RMA to 768 MB, GRUB can use memory till 768 
MB if required.

Regards,
Avnish Chouhan

> 
> - Hari

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

[Hari Bathini]

On 04/02/25 10:58 am, Avnish Chouhan wrote:
> On 2025-01-31 20:44, Hari Bathini wrote:
>> On 23/01/25 7:54 pm, Avnish Chouhan wrote:
>>> On 2025-01-23 15:26, Hari Bathini wrote:
>>>> On 20/01/25 11:05 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 patch ("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.
>>>>
>>>> IIRC, even memory above MIN_RMA is used by the bootloader except for
>>>> 640MB to 768MB (assuming RMA size is >768MB). So, how does this change
>>>> guarantee that the bootloader is not using memory reserved for 
>>>> bootargs?
>>>>
>>>> Avnish, earlier, bootloader was using RUNTIME_MIN_SPACE (128MB) 
>>>> starting
>>>> top-down at 768MB earlier. With MIN_RMA changed to 768MB, is bootloader
>>>> still using the concept of RUNTIME_MIN_SPACE to set aside some memory
>>>> for kernel to use. If yes, where exactly is it allocating this space
>>>> now? Also, rtas instantiates top-down at 768MB. Would that not have
>>>> a conflict with grub allocations without RUNTIME_MIN_SPACE at 768MB?
>>>>
>>>> - Hari
>>>
>>> Hi Hari,
>>
>> Hi Avnish,
>>
>>> The RUNTIME_MIN_SPACE is the space left aside by Grub is within the 
>>> MIN_RMA size. Grub won't use memory beyond the MIN_RMA. With this 
>>> change, we haven't changed the RUNTIME_MIN_SPACE behavior. Grub will 
>>> still keep the 128 MB space in MIN_RMA for loading stock kernel and 
>>> initrd.
>>
>> IIUC, you mean, 640MB to 768MB is not used by Grub even if MIN_RMA
>> is at 768MB? If that is true, this change is not needed, as fadump
>> could still use the memory between 640MB to 768MB, right?
>> Am I missing something here..
> 
> Hari,
> 
> No. As we are changing MIN_RMA to 768 MB, GRUB can use memory till 768 
> MB if required.

Does that mean 'linux_rmo_save' related code in
grub-core/kern/ieee1275/init.c is going to be dead code after this
change. Also, does this imply, there isn't going to be any
RUNTIME_MIN_SPACE support for linux in grub?

- Hari

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

[Avnish Chouhan]

On 2025-02-04 11:57, Hari Bathini wrote:
> On 04/02/25 10:58 am, Avnish Chouhan wrote:
>> On 2025-01-31 20:44, Hari Bathini wrote:
>>> On 23/01/25 7:54 pm, Avnish Chouhan wrote:
>>>> On 2025-01-23 15:26, Hari Bathini wrote:
>>>>> On 20/01/25 11:05 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 patch ("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.
>>>>> 
>>>>> IIRC, even memory above MIN_RMA is used by the bootloader except 
>>>>> for
>>>>> 640MB to 768MB (assuming RMA size is >768MB). So, how does this 
>>>>> change
>>>>> guarantee that the bootloader is not using memory reserved for 
>>>>> bootargs?
>>>>> 
>>>>> Avnish, earlier, bootloader was using RUNTIME_MIN_SPACE (128MB) 
>>>>> starting
>>>>> top-down at 768MB earlier. With MIN_RMA changed to 768MB, is 
>>>>> bootloader
>>>>> still using the concept of RUNTIME_MIN_SPACE to set aside some 
>>>>> memory
>>>>> for kernel to use. If yes, where exactly is it allocating this 
>>>>> space
>>>>> now? Also, rtas instantiates top-down at 768MB. Would that not have
>>>>> a conflict with grub allocations without RUNTIME_MIN_SPACE at 
>>>>> 768MB?
>>>>> 
>>>>> - Hari
>>>> 
>>>> Hi Hari,
>>> 
>>> Hi Avnish,
>>> 
>>>> The RUNTIME_MIN_SPACE is the space left aside by Grub is within the 
>>>> MIN_RMA size. Grub won't use memory beyond the MIN_RMA. With this 
>>>> change, we haven't changed the RUNTIME_MIN_SPACE behavior. Grub will 
>>>> still keep the 128 MB space in MIN_RMA for loading stock kernel and 
>>>> initrd.
>>> 
>>> IIUC, you mean, 640MB to 768MB is not used by Grub even if MIN_RMA
>>> is at 768MB? If that is true, this change is not needed, as fadump
>>> could still use the memory between 640MB to 768MB, right?
>>> Am I missing something here..
>> 
>> Hari,
>> 
>> No. As we are changing MIN_RMA to 768 MB, GRUB can use memory till 768 
>> MB if required.
> 
> Does that mean 'linux_rmo_save' related code in
> grub-core/kern/ieee1275/init.c is going to be dead code after this
> change. Also, does this imply, there isn't going to be any
> RUNTIME_MIN_SPACE support for linux in grub?

No Hari, there's no change in RUNTIME_MIN_SPACE as mentioned earlier nor 
the change leading to any dead code in grub. If we have MIN_RMA as 512 
MB, the grub will consider RUNTIME_MIN_SPACE region within the MIN_RMA 
as (384[512-128] to 512). And if we have MIN_RMA as 768 MB, it will be 
(640[768-128] to 768).

Grub will keep the 128 MB space in MIN_RMA for loading stock kernel and 
initrd as stated earlier.

Thank you,
Avnish Chouhan
> 
> - Hari

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

[Hari Bathini]

On 04/02/25 2:07 pm, Avnish Chouhan wrote:
> On 2025-02-04 11:57, Hari Bathini wrote:
>> On 04/02/25 10:58 am, Avnish Chouhan wrote:
>>> On 2025-01-31 20:44, Hari Bathini wrote:
>>>> On 23/01/25 7:54 pm, Avnish Chouhan wrote:
>>>>> On 2025-01-23 15:26, Hari Bathini wrote:
>>>>>> On 20/01/25 11:05 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 patch ("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.
>>>>>>
>>>>>> IIRC, even memory above MIN_RMA is used by the bootloader except for
>>>>>> 640MB to 768MB (assuming RMA size is >768MB). So, how does this 
>>>>>> change
>>>>>> guarantee that the bootloader is not using memory reserved for 
>>>>>> bootargs?
>>>>>>
>>>>>> Avnish, earlier, bootloader was using RUNTIME_MIN_SPACE (128MB) 
>>>>>> starting
>>>>>> top-down at 768MB earlier. With MIN_RMA changed to 768MB, is 
>>>>>> bootloader
>>>>>> still using the concept of RUNTIME_MIN_SPACE to set aside some memory
>>>>>> for kernel to use. If yes, where exactly is it allocating this space
>>>>>> now? Also, rtas instantiates top-down at 768MB. Would that not have
>>>>>> a conflict with grub allocations without RUNTIME_MIN_SPACE at 768MB?
>>>>>>
>>>>>> - Hari
>>>>>
>>>>> Hi Hari,
>>>>
>>>> Hi Avnish,
>>>>
>>>>> The RUNTIME_MIN_SPACE is the space left aside by Grub is within the 
>>>>> MIN_RMA size. Grub won't use memory beyond the MIN_RMA. With this 
>>>>> change, we haven't changed the RUNTIME_MIN_SPACE behavior. Grub 
>>>>> will still keep the 128 MB space in MIN_RMA for loading stock 
>>>>> kernel and initrd.
>>>>
>>>> IIUC, you mean, 640MB to 768MB is not used by Grub even if MIN_RMA
>>>> is at 768MB? If that is true, this change is not needed, as fadump
>>>> could still use the memory between 640MB to 768MB, right?
>>>> Am I missing something here..
>>>
>>> Hari,
>>>
>>> No. As we are changing MIN_RMA to 768 MB, GRUB can use memory till 
>>> 768 MB if required.
>>
>> Does that mean 'linux_rmo_save' related code in
>> grub-core/kern/ieee1275/init.c is going to be dead code after this
>> change. Also, does this imply, there isn't going to be any
>> RUNTIME_MIN_SPACE support for linux in grub?
> 
> No Hari, there's no change in RUNTIME_MIN_SPACE as mentioned earlier nor 
> the change leading to any dead code in grub. If we have MIN_RMA as 512 
> MB, the grub will consider RUNTIME_MIN_SPACE region within the MIN_RMA 
> as (384[512-128] to 512). And if we have MIN_RMA as 768 MB, it will be 
> (640[768-128] to 768).
> 
> Grub will keep the 128 MB space in MIN_RMA for loading stock kernel and 
> initrd as stated earlier.

Thanks, Avnish.
That clears it.

- Hari