| Message ID | 20250120173501.1147236-3-sourabhjain@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/fadump: fix additional parameter for HASH MMU | expand |
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
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,
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,
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
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
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
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
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
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
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,
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(-)