All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
@ 2024-12-06  6:55 Avnish Chouhan
  2024-12-07  1:58 ` Michael Ellerman
  2025-01-22 12:43 ` Madhavan Srinivasan
  0 siblings, 2 replies; 14+ messages in thread
From: Avnish Chouhan @ 2024-12-06  6:55 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: mpe, brking, meghanaprakash, sourabhjain, maddy, Avnish Chouhan

Change RMA size from 512 MB to 768 MB which will result 
in more RMA at boot time for PowerPC. When PowerPC LPAR use/uses vTPM,
Secure Boot or FADump, the 512 MB RMA memory is not sufficient for 
booting. With this 512 MB RMA, GRUB2 run out of memory and unable to 
load the necessary. Sometimes even usage of CDROM which requires more
memory for installation along with the options mentioned above troubles
the boot memory and result in boot failures. Increasing the RMA size 
will resolves multiple out of memory issues observed in PowerPC. 

Failure details:

1. GRUB2

kern/ieee1275/init.c:550: mm requested region of size 8513000, flags 1
kern/ieee1275/init.c:563: Cannot satisfy allocation and retain minimum runtime
space
kern/ieee1275/init.c:550: mm requested region of size 8513000, flags 0
kern/ieee1275/init.c:563: Cannot satisfy allocation and retain minimum runtime
space
kern/file.c:215: Closing `/ppc/ppc64/initrd.img' ...
kern/disk.c:297: Closing
`ieee1275//vdevice/v-scsi
@30000067/disk@8300000000000000'...
kern/disk.c:311: Closing
`ieee1275//vdevice/v-scsi
@30000067/disk@8300000000000000' succeeded.
kern/file.c:225: Closing `/ppc/ppc64/initrd.img' failed with 3.
kern/file.c:148: Opening `/ppc/ppc64/initrd.img' succeeded.
error: ../../grub-core/kern/mm.c:552:out of memory.

2. Kernel

[    0.777633] List of all partitions:
[    0.777639] No filesystem could mount root, tried:
[    0.777640]
[    0.777649] Kernel panic - not syncing: VFS: Unable to mount root fs on "" or unknown-block(0,0)
[    0.777658] CPU: 17 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.11.0-0.rc4.20.el10.ppc64le #1
[    0.777669] Hardware name: IBM,9009-22A POWER9 (architected) 0x4e0202 0xf000005 of:IBM,FW950.B0 (VL950_149) hv:phyp pSeries
[    0.777678] Call Trace:
[    0.777682] [c000000003db7b60] [c000000001119714] dump_stack_lvl+0x88/0xc4 (unreliable)
[    0.777700] [c000000003db7b90] [c00000000016c274] panic+0x174/0x460
[    0.777711] [c000000003db7c30] [c00000000200631c] mount_root_generic+0x320/0x354
[    0.777724] [c000000003db7d00] [c0000000020066f8] prepare_namespace+0x27c/0x2f4
[    0.777735] [c000000003db7d90] [c000000002005824] kernel_init_freeable+0x254/0x294
[    0.777747] [c000000003db7df0] [c00000000001131c] kernel_init+0x30/0x1c4
[    0.777757] [c000000003db7e50] [c00000000000debc] ret_from_kernel_user_thread+0x14/0x1c
[    0.777768] --- interrupt: 0 at 0x0
[    0.784238] pstore: backend (nvram) writing error (-1)
[    0.790447] Rebooting in 10 seconds..

Signed-off-by: Avnish Chouhan <avnish@linux.ibm.com>
---
Change logs:

v2:
 - Added GRUB2 debug logs and Kernel traces.

---
 arch/powerpc/kernel/prom_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index fbb68fc28ed3..c42fd5a826c0 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(512),		/* 512MB min RMA */
+		.min_rma = cpu_to_be32(768),		/* 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.43.5



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
  2024-12-06  6:55 [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation Avnish Chouhan
@ 2024-12-07  1:58 ` Michael Ellerman
  2024-12-11 12:05   ` Avnish Chouhan
  2025-01-09  9:32   ` Sourabh Jain
  2025-01-22 12:43 ` Madhavan Srinivasan
  1 sibling, 2 replies; 14+ messages in thread
From: Michael Ellerman @ 2024-12-07  1:58 UTC (permalink / raw)
  To: Avnish Chouhan, linuxppc-dev
  Cc: brking, meghanaprakash, sourabhjain, maddy, Avnish Chouhan

Avnish Chouhan <avnish@linux.ibm.com> writes:
> Change RMA size from 512 MB to 768 MB which will result 
> in more RMA at boot time for PowerPC.

Did you consider just increasing it to 1GB?

It's possible there's some folks running LPARs with less than 1GB, but
they are unlikely to continue doing so by the time this change trickles
into distros. To be supported modern RHEL requires 2GB minimum RAM
anyway.

Can you also describe the behaviour users will see when they configure
an LPAR with less than 768MB of RAM.

cheers


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
  2024-12-07  1:58 ` Michael Ellerman
@ 2024-12-11 12:05   ` Avnish Chouhan
  2025-01-09  9:32   ` Sourabh Jain
  1 sibling, 0 replies; 14+ messages in thread
From: Avnish Chouhan @ 2024-12-11 12:05 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, brking, meghanaprakash, sourabhjain, maddy

Hi Michael,
Hope you're doing wonderful!

Thank you so much for your response. I have checked on your queries. 
Please find the findings below:

1. Did you consider just increasing it to 1GB?

We have observed in our recent Out Of Memory issues, a shortage of 
around 50-60 MBs space in RMA in current issues. So we decided to 
increase the RMA by 256 MBs. Please give me couple of days, I'm 
analyzing this 1 GB change and update you on it soon.

2. an LPAR with less than 768MB of RAM

I have analyzed the multiple RAM scenarios. The behavior seems similar 
regardless of RMA size 512 or 768 MBs, as the RMA region is used by PFW 
and GRUB2 for booting. Even if GRUB is able to load the kernel for 
booting, the machines isn't booting and behaving well in low amount of 
RAM. We observe kernel panics, mostly "Out of memory: Killed 
process...." when RAM is less than 3 GBs. The different RAM configs (via 
HMC LPAR properties) and behaviors are given below:

i. RAM (3 GBs)
    System boot fine when RAM is minimum 3 GBs (It does depend on system 
config as well).
ii. RMA (512 MBs)
     With RAM as 512 MB, the system fails to boot with firmware error (eg 
B2006006).
iii. RAM (768 MB and 1 GB)
      With RAM as 768 MB and 1 GB, System boot with kernel panic as 
"Kernel panic - not syncing: System is    deadlocked on memory".
iv. RAM (2 GBs)
     System does boot fine, but abnormal behavior after the boot. I 
observed system panic in one scenario while doing a reboot. "Out of 
memory: Killed process 167....."


Regards,
Avnish Chouhan


On 2024-12-07 07:28, Michael Ellerman wrote:
> Avnish Chouhan <avnish@linux.ibm.com> writes:
>> Change RMA size from 512 MB to 768 MB which will result
>> in more RMA at boot time for PowerPC.
> 
> Did you consider just increasing it to 1GB?
> 
> It's possible there's some folks running LPARs with less than 1GB, but
> they are unlikely to continue doing so by the time this change trickles
> into distros. To be supported modern RHEL requires 2GB minimum RAM
> anyway.
> 
> Can you also describe the behaviour users will see when they configure
> an LPAR with less than 768MB of RAM.
> 
> cheers


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
  2024-12-07  1:58 ` Michael Ellerman
  2024-12-11 12:05   ` Avnish Chouhan
@ 2025-01-09  9:32   ` Sourabh Jain
  1 sibling, 0 replies; 14+ messages in thread
From: Sourabh Jain @ 2025-01-09  9:32 UTC (permalink / raw)
  To: Michael Ellerman, Avnish Chouhan, linuxppc-dev
  Cc: brking, meghanaprakash, maddy

Hello Michael,

On 07/12/24 07:28, Michael Ellerman wrote:
> Avnish Chouhan <avnish@linux.ibm.com> writes:
>> Change RMA size from 512 MB to 768 MB which will result
>> in more RMA at boot time for PowerPC.
> Did you consider just increasing it to 1GB?

I see an impact of setting RMA to 1GB on fadump. Here’s how:

The minimum reservation recommended by us for fadump is 768MB.
In this case, the boot_mem_top in fadump.c will be 768MB, and the
memory reservation for fadump will start from a 768MB offset.

Now, let’s say the production kernel crashes, and the firmware copies
the 0–768MB region to the fadump reserved area and boots the fadump
kernel to capture the dump.

If RMA is set to 1GB, it will allow GRUB to use memory up to 1GB. In such
a case, there is a possibility that GRUB could corrupt the fadump 
reserved area.

Thanks,
Sourabh Jain


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
  2024-12-06  6:55 [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation Avnish Chouhan
  2024-12-07  1:58 ` Michael Ellerman
@ 2025-01-22 12:43 ` Madhavan Srinivasan
  2025-01-23  3:06   ` Sourabh Jain
  2025-01-24  3:58   ` Sourabh Jain
  1 sibling, 2 replies; 14+ messages in thread
From: Madhavan Srinivasan @ 2025-01-22 12:43 UTC (permalink / raw)
  To: linuxppc-dev, Avnish Chouhan; +Cc: mpe, brking, meghanaprakash, sourabhjain

On Fri, 06 Dec 2024 12:25:45 +0530, Avnish Chouhan wrote:
> Change RMA size from 512 MB to 768 MB which will result
> in more RMA at boot time for PowerPC. When PowerPC LPAR use/uses vTPM,
> Secure Boot or FADump, the 512 MB RMA memory is not sufficient for
> booting. With this 512 MB RMA, GRUB2 run out of memory and unable to
> load the necessary. Sometimes even usage of CDROM which requires more
> memory for installation along with the options mentioned above troubles
> the boot memory and result in boot failures. Increasing the RMA size
> will resolves multiple out of memory issues observed in PowerPC.
> 
> [...]

Applied to powerpc/next.

[1/1] powerpc: increase MIN RMA size for CAS negotiation
      https://git.kernel.org/powerpc/c/ae908b87b6bb32c170e9baf5858f2a7553cacc06

Thanks


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
  2025-01-22 12:43 ` Madhavan Srinivasan
@ 2025-01-23  3:06   ` Sourabh Jain
  2025-01-24  3:58   ` Sourabh Jain
  1 sibling, 0 replies; 14+ messages in thread
From: Sourabh Jain @ 2025-01-23  3:06 UTC (permalink / raw)
  To: Madhavan Srinivasan, linuxppc-dev, Avnish Chouhan
  Cc: mpe, brking, meghanaprakash

Hello Maddy,

On 22/01/25 18:13, Madhavan Srinivasan wrote:
> On Fri, 06 Dec 2024 12:25:45 +0530, Avnish Chouhan wrote:
>> Change RMA size from 512 MB to 768 MB which will result
>> in more RMA at boot time for PowerPC. When PowerPC LPAR use/uses vTPM,
>> Secure Boot or FADump, the 512 MB RMA memory is not sufficient for
>> booting. With this 512 MB RMA, GRUB2 run out of memory and unable to
>> load the necessary. Sometimes even usage of CDROM which requires more
>> memory for installation along with the options mentioned above troubles
>> the boot memory and result in boot failures. Increasing the RMA size
>> will resolves multiple out of memory issues observed in PowerPC.
>>
>> [...]
> Applied to powerpc/next.
>
> [1/1] powerpc: increase MIN RMA size for CAS negotiation
>        https://git.kernel.org/powerpc/c/ae908b87b6bb32c170e9baf5858f2a7553cacc06

The above changes break the fadump additional parameter for the HASH MMU.
Here is the proposed fix:
https://lore.kernel.org/all/20250120173501.1147236-1-sourabhjain@linux.ibm.com/

The proposed fix ensures that the memory reserved for the additional 
parameter for
fadump on the HASH MMU does not overlap with GRUB.

Please share your opinion on the fixes.

Thanks,
Sourabh Jain


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
  2025-01-22 12:43 ` Madhavan Srinivasan
  2025-01-23  3:06   ` Sourabh Jain
@ 2025-01-24  3:58   ` Sourabh Jain
  1 sibling, 0 replies; 14+ messages in thread
From: Sourabh Jain @ 2025-01-24  3:58 UTC (permalink / raw)
  To: Madhavan Srinivasan, linuxppc-dev, Avnish Chouhan
  Cc: mpe, brking, meghanaprakash

Hello Maddy,


On 22/01/25 18:13, Madhavan Srinivasan wrote:
> On Fri, 06 Dec 2024 12:25:45 +0530, Avnish Chouhan wrote:
>> Change RMA size from 512 MB to 768 MB which will result
>> in more RMA at boot time for PowerPC. When PowerPC LPAR use/uses vTPM,
>> Secure Boot or FADump, the 512 MB RMA memory is not sufficient for
>> booting. With this 512 MB RMA, GRUB2 run out of memory and unable to
>> load the necessary. Sometimes even usage of CDROM which requires more
>> memory for installation along with the options mentioned above troubles
>> the boot memory and result in boot failures. Increasing the RMA size
>> will resolves multiple out of memory issues observed in PowerPC.
>>
>> [...]
> Applied to powerpc/next.
>
> [1/1] powerpc: increase MIN RMA size for CAS negotiation
>        https://git.kernel.org/powerpc/c/ae908b87b6bb32c170e9baf5858f2a7553cacc06
I posted a patch series that includes this patch along with patches to 
avoid the impact on fadump due to the `MIN_RMA` change introduced in 
this patch. 
https://lore.kernel.org/all/20250123114254.200527-1-sourabhjain@linux.ibm.com/ 
The patches in the above patch series are arranged to ensure they do not 
break `git bisect`. Previously, we tested this patch and the fadump 
changes together but sent them separately, which I now realize was not 
the best approach. If possible, please consider accepting the entire 
patch series as a whole. 
https://lore.kernel.org/all/20250123114254.200527-1-sourabhjain@linux.ibm.com/ 
We have included test cases for the above patch series and a changelog 
explaining additional details in the cover letter. Please let me know if 
you have queries regarding this. Thanks, Sourabh Jain


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
@ 2025-03-03 16:49 Avnish Chouhan
  2025-03-04 15:09 ` Daniel Kiper
  0 siblings, 1 reply; 14+ messages in thread
From: Avnish Chouhan @ 2025-03-03 16:49 UTC (permalink / raw)
  To: grub-devel; +Cc: daniel.kiper, brking, meghanaprakash, mamatha4, Avnish Chouhan

Change RMA size from 512 MB to 768 MB which will result
in more memory at boot time for PowerPC. When vTPM, Secure Boot or
FADump are enabled on PowerPC, the 512 MB RMA memory is not sufficient for
booting. With this 512 MB RMA, GRUB2 runs out of memory and fails to
boot the machine. Sometimes even usage of CDROM which requires more
memory for installation along with the options mentioned above troubles
the boot memory and result in boot failures. Increasing the RMA size
will resolves multiple out of memory issues observed in PowerPC.

Failure details (GRUB2 debugs):

  kern/ieee1275/init.c:550: mm requested region of size 8513000, flags 1
  kern/ieee1275/init.c:563: Cannot satisfy allocation and retain minimum runtime space
  kern/ieee1275/init.c:550: mm requested region of size 8513000, flags 0
  kern/ieee1275/init.c:563: Cannot satisfy allocation and retain minimum runtime space
  kern/file.c:215: Closing `/ppc/ppc64/initrd.img' ...
  kern/disk.c:297: Closing `ieee1275//vdevice/v-scsi@30000067/disk@8300000000000000'...
  kern/disk.c:311: Closing `ieee1275//vdevice/v-scsi@30000067/disk@8300000000000000' succeeded.
  kern/file.c:225: Closing `/ppc/ppc64/initrd.img' failed with 3.
  kern/file.c:148: Opening `/ppc/ppc64/initrd.img' succeeded.
  error: ../../grub-core/kern/mm.c:552:out of memory.

Signed-off-by: Avnish Chouhan <avnish@linux.ibm.com>
---
 grub-core/kern/ieee1275/init.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
index dfbd0b8..34e9e13 100644
--- a/grub-core/kern/ieee1275/init.c
+++ b/grub-core/kern/ieee1275/init.c
@@ -852,7 +852,7 @@ grub_ieee1275_ibm_cas (void)
     .vec1 = 0x80, /* ignore */
     .vec2_size = 1 + sizeof (struct option_vector2) - 2,
     .vec2 = {
-      0, 0, -1, -1, -1, -1, -1, 512, -1, 0, 48
+      0, 0, -1, -1, -1, -1, -1, 768, -1, 0, 48
     },
     .vec3_size = 2 - 1,
     .vec3 = 0x00e0, /* ask for FP + VMX + DFP but don't halt if unsatisfied */
@@ -889,6 +889,10 @@ grub_claim_heap (void)
 {
   grub_err_t err;
   grub_uint32_t total = HEAP_MAX_SIZE;
+#if defined(__powerpc__)
+  grub_uint32_t ibm_ca_support_reboot = 0;
+  grub_ssize_t actual;
+#endif
 
   err = grub_ieee1275_total_mem (&rmo_top);
 
@@ -901,11 +905,30 @@ grub_claim_heap (void)
     grub_mm_add_region_fn = grub_ieee1275_mm_add_region;
 
 #if defined(__powerpc__)
+  /* Check if it's a CAS reboot with below property. If so, we will skip CAS call */
+  if (grub_ieee1275_get_integer_property (grub_ieee1275_chosen,
+                                          "ibm,client-architecture-support-reboot",
+                                          &ibm_ca_support_reboot,
+                                          sizeof (ibm_ca_support_reboot),
+                                          &actual) >= 0)
+    grub_dprintf ("ieee1275", "ibm,client-architecture-support-reboot: %u\n",
+                  ibm_ca_support_reboot);
+
   if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY))
     {
-      /* if we have an error, don't call CAS, just hope for the best */
-      if (err == GRUB_ERR_NONE && rmo_top < (512 * 1024 * 1024))
-	grub_ieee1275_ibm_cas ();
+      /*
+       * If we have an error or the reboot is detected as CAS reboot,
+       * don't call CAS, just hope for the best.
+       * Along with the above, if the rmo_top is 512 MB or above. We
+       * will skip the CAS call. Though if we call CAS, the rmo_top will
+       * be set to 768 MB via CAS Vector2. This condition is required to avoid the
+       * issue where the older Linux kernels are still using rmo_top as 512 MB.
+       * If we call CAS where rmo_top is less then 768 MB, this will result in an issue
+       * due to IBM CAS reboot feature and we won't be able to boot the newer kernel.
+       * The machine will boot with the last booted kernel which has rmo_top as 512 MB.
+       */
+      if (!ibm_ca_support_reboot && err == GRUB_ERR_NONE && rmo_top < (512 * 1024 * 1024))
+        grub_ieee1275_ibm_cas ();
     }
 #endif
 
-- 
2.39.3


_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
  2025-03-03 16:49 Avnish Chouhan
@ 2025-03-04 15:09 ` Daniel Kiper
  2025-03-07  9:01   ` Avnish Chouhan
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Kiper @ 2025-03-04 15:09 UTC (permalink / raw)
  To: Avnish Chouhan; +Cc: grub-devel, daniel.kiper, brking, meghanaprakash, mamatha4

On Mon, Mar 03, 2025 at 10:19:22PM +0530, Avnish Chouhan wrote:
> Change RMA size from 512 MB to 768 MB which will result
> in more memory at boot time for PowerPC. When vTPM, Secure Boot or
> FADump are enabled on PowerPC, the 512 MB RMA memory is not sufficient for
> booting. With this 512 MB RMA, GRUB2 runs out of memory and fails to
> boot the machine. Sometimes even usage of CDROM which requires more

s/which//

> memory for installation along with the options mentioned above troubles

s/installation along/installation and along/
s/troubles/exhausts/

> the boot memory and result in boot failures. Increasing the RMA size

s/and result/which results/

> will resolves multiple out of memory issues observed in PowerPC.
>
> Failure details (GRUB2 debugs):
>
>   kern/ieee1275/init.c:550: mm requested region of size 8513000, flags 1
>   kern/ieee1275/init.c:563: Cannot satisfy allocation and retain minimum runtime space
>   kern/ieee1275/init.c:550: mm requested region of size 8513000, flags 0
>   kern/ieee1275/init.c:563: Cannot satisfy allocation and retain minimum runtime space
>   kern/file.c:215: Closing `/ppc/ppc64/initrd.img' ...
>   kern/disk.c:297: Closing `ieee1275//vdevice/v-scsi@30000067/disk@8300000000000000'...
>   kern/disk.c:311: Closing `ieee1275//vdevice/v-scsi@30000067/disk@8300000000000000' succeeded.
>   kern/file.c:225: Closing `/ppc/ppc64/initrd.img' failed with 3.
>   kern/file.c:148: Opening `/ppc/ppc64/initrd.img' succeeded.
>   error: ../../grub-core/kern/mm.c:552:out of memory.
>
> Signed-off-by: Avnish Chouhan <avnish@linux.ibm.com>
> ---
>  grub-core/kern/ieee1275/init.c | 31 +++++++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/grub-core/kern/ieee1275/init.c b/grub-core/kern/ieee1275/init.c
> index dfbd0b8..34e9e13 100644
> --- a/grub-core/kern/ieee1275/init.c
> +++ b/grub-core/kern/ieee1275/init.c
> @@ -852,7 +852,7 @@ grub_ieee1275_ibm_cas (void)
>      .vec1 = 0x80, /* ignore */
>      .vec2_size = 1 + sizeof (struct option_vector2) - 2,
>      .vec2 = {
> -      0, 0, -1, -1, -1, -1, -1, 512, -1, 0, 48
> +      0, 0, -1, -1, -1, -1, -1, 768, -1, 0, 48
>      },
>      .vec3_size = 2 - 1,
>      .vec3 = 0x00e0, /* ask for FP + VMX + DFP but don't halt if unsatisfied */
> @@ -889,6 +889,10 @@ grub_claim_heap (void)
>  {
>    grub_err_t err;
>    grub_uint32_t total = HEAP_MAX_SIZE;
> +#if defined(__powerpc__)
> +  grub_uint32_t ibm_ca_support_reboot = 0;
> +  grub_ssize_t actual;
> +#endif
>
>    err = grub_ieee1275_total_mem (&rmo_top);
>
> @@ -901,11 +905,30 @@ grub_claim_heap (void)
>      grub_mm_add_region_fn = grub_ieee1275_mm_add_region;
>
>  #if defined(__powerpc__)
> +  /* Check if it's a CAS reboot with below property. If so, we will skip CAS call */
> +  if (grub_ieee1275_get_integer_property (grub_ieee1275_chosen,
> +                                          "ibm,client-architecture-support-reboot",
> +                                          &ibm_ca_support_reboot,
> +                                          sizeof (ibm_ca_support_reboot),
> +                                          &actual) >= 0)
> +    grub_dprintf ("ieee1275", "ibm,client-architecture-support-reboot: %u\n",
> +                  ibm_ca_support_reboot);
> +
>    if (grub_ieee1275_test_flag (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY))
>      {
> -      /* if we have an error, don't call CAS, just hope for the best */
> -      if (err == GRUB_ERR_NONE && rmo_top < (512 * 1024 * 1024))
> -	grub_ieee1275_ibm_cas ();
> +      /*
> +       * If we have an error or the reboot is detected as CAS reboot,
> +       * don't call CAS, just hope for the best.
> +       * Along with the above, if the rmo_top is 512 MB or above. We
> +       * will skip the CAS call. Though if we call CAS, the rmo_top will
> +       * be set to 768 MB via CAS Vector2. This condition is required to avoid the
> +       * issue where the older Linux kernels are still using rmo_top as 512 MB.
> +       * If we call CAS where rmo_top is less then 768 MB, this will result in an issue
> +       * due to IBM CAS reboot feature and we won't be able to boot the newer kernel.

Could you be more specific? What is "an issue due to IBM CAS reboot feature"?

And I think it would be nice if you put here a reference to documentation,
including chapters names, etc., which discuss RMA and issues fixed here.

> +       * The machine will boot with the last booted kernel which has rmo_top as 512 MB.
> +       */
> +      if (!ibm_ca_support_reboot && err == GRUB_ERR_NONE && rmo_top < (512 * 1024 * 1024))
> +        grub_ieee1275_ibm_cas ();
>      }
>  #endif

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
  2025-03-04 15:09 ` Daniel Kiper
@ 2025-03-07  9:01   ` Avnish Chouhan
  2025-03-10 13:12     ` Daniel Kiper
  0 siblings, 1 reply; 14+ messages in thread
From: Avnish Chouhan @ 2025-03-07  9:01 UTC (permalink / raw)
  To: Daniel Kiper; +Cc: grub-devel, daniel.kiper, brking, meghanaprakash, mamatha4

Hi Daniel,
Thank you so much for your patch reviews.

I'll replace the words as suggested by you.

****
>> +       * If we have an error or the reboot is detected as CAS reboot,
>> +       * don't call CAS, just hope for the best.
>> +       * Along with the above, if the rmo_top is 512 MB or above. We
>> +       * will skip the CAS call. Though if we call CAS, the rmo_top 
>> will
>> +       * be set to 768 MB via CAS Vector2. This condition is required 
>> to avoid the
>> +       * issue where the older Linux kernels are still using rmo_top 
>> as 512 MB.
>> +       * If we call CAS where rmo_top is less then 768 MB, this will 
>> result in an issue
>> +       * due to IBM CAS reboot feature and we won't be able to boot 
>> the newer kernel.
> 
> Could you be more specific? What is "an issue due to IBM CAS reboot 
> feature"?
> 
> And I think it would be nice if you put here a reference to 
> documentation,
> including chapters names, etc., which discuss RMA and issues fixed 
> here.
> 
>> +       * The machine will boot with the last booted kernel which has 
>> rmo_top as 512 MB.
>> +       */
****

On this. This patch only change the size of RMA from 512 MB to 768 MB. 
The change is done via CAS call. Condition for calling a CAS has no 
change other than adding a check on "whether the reboot is a CAS 
reboot". This is required to avoid unwanted and repetitive CAS calls.

With this CAS reboot check condition, in any scenario, where we are 
using older kernel and CAS is still using 512 MB RMA but with the 
updated Grub. The repeated CAS calls will be avoided.

In IBM CAS reboot feature, whenever CAS call occurred. We skip providing 
the kernel options to boot to and we directly boot to the lasted booted 
kernel.

With this feature in place. If we upgrade the machine's kernel and grub, 
and as the machine is lasted booted with old kernel. Any CAS call from 
Grub will restrict user to keep booting to last booted kernel!

So these conditions are made to avoid any of these possible issues.

on adding the documentation, we can add the PAPR document link which has 
been shared with you earlier by IBM folks.

Please let me know if you have more questions on this.
Thank you!

Regards,
Avnish Chouhan


On 2025-03-04 20:39, Daniel Kiper wrote:
> On Mon, Mar 03, 2025 at 10:19:22PM +0530, Avnish Chouhan wrote:
>> Change RMA size from 512 MB to 768 MB which will result
>> in more memory at boot time for PowerPC. When vTPM, Secure Boot or
>> FADump are enabled on PowerPC, the 512 MB RMA memory is not sufficient 
>> for
>> booting. With this 512 MB RMA, GRUB2 runs out of memory and fails to
>> boot the machine. Sometimes even usage of CDROM which requires more
> 
> s/which//
> 
>> memory for installation along with the options mentioned above 
>> troubles
> 
> s/installation along/installation and along/
> s/troubles/exhausts/
> 
>> the boot memory and result in boot failures. Increasing the RMA size
> 
> s/and result/which results/
> 
>> will resolves multiple out of memory issues observed in PowerPC.
>> 
>> Failure details (GRUB2 debugs):
>> 
>>   kern/ieee1275/init.c:550: mm requested region of size 8513000, flags 
>> 1
>>   kern/ieee1275/init.c:563: Cannot satisfy allocation and retain 
>> minimum runtime space
>>   kern/ieee1275/init.c:550: mm requested region of size 8513000, flags 
>> 0
>>   kern/ieee1275/init.c:563: Cannot satisfy allocation and retain 
>> minimum runtime space
>>   kern/file.c:215: Closing `/ppc/ppc64/initrd.img' ...
>>   kern/disk.c:297: Closing 
>> `ieee1275//vdevice/v-scsi@30000067/disk@8300000000000000'...
>>   kern/disk.c:311: Closing 
>> `ieee1275//vdevice/v-scsi@30000067/disk@8300000000000000' succeeded.
>>   kern/file.c:225: Closing `/ppc/ppc64/initrd.img' failed with 3.
>>   kern/file.c:148: Opening `/ppc/ppc64/initrd.img' succeeded.
>>   error: ../../grub-core/kern/mm.c:552:out of memory.
>> 
>> Signed-off-by: Avnish Chouhan <avnish@linux.ibm.com>
>> ---
>>  grub-core/kern/ieee1275/init.c | 31 +++++++++++++++++++++++++++----
>>  1 file changed, 27 insertions(+), 4 deletions(-)
>> 
>> diff --git a/grub-core/kern/ieee1275/init.c 
>> b/grub-core/kern/ieee1275/init.c
>> index dfbd0b8..34e9e13 100644
>> --- a/grub-core/kern/ieee1275/init.c
>> +++ b/grub-core/kern/ieee1275/init.c
>> @@ -852,7 +852,7 @@ grub_ieee1275_ibm_cas (void)
>>      .vec1 = 0x80, /* ignore */
>>      .vec2_size = 1 + sizeof (struct option_vector2) - 2,
>>      .vec2 = {
>> -      0, 0, -1, -1, -1, -1, -1, 512, -1, 0, 48
>> +      0, 0, -1, -1, -1, -1, -1, 768, -1, 0, 48
>>      },
>>      .vec3_size = 2 - 1,
>>      .vec3 = 0x00e0, /* ask for FP + VMX + DFP but don't halt if 
>> unsatisfied */
>> @@ -889,6 +889,10 @@ grub_claim_heap (void)
>>  {
>>    grub_err_t err;
>>    grub_uint32_t total = HEAP_MAX_SIZE;
>> +#if defined(__powerpc__)
>> +  grub_uint32_t ibm_ca_support_reboot = 0;
>> +  grub_ssize_t actual;
>> +#endif
>> 
>>    err = grub_ieee1275_total_mem (&rmo_top);
>> 
>> @@ -901,11 +905,30 @@ grub_claim_heap (void)
>>      grub_mm_add_region_fn = grub_ieee1275_mm_add_region;
>> 
>>  #if defined(__powerpc__)
>> +  /* Check if it's a CAS reboot with below property. If so, we will 
>> skip CAS call */
>> +  if (grub_ieee1275_get_integer_property (grub_ieee1275_chosen,
>> +                                          
>> "ibm,client-architecture-support-reboot",
>> +                                          &ibm_ca_support_reboot,
>> +                                          sizeof 
>> (ibm_ca_support_reboot),
>> +                                          &actual) >= 0)
>> +    grub_dprintf ("ieee1275", 
>> "ibm,client-architecture-support-reboot: %u\n",
>> +                  ibm_ca_support_reboot);
>> +
>>    if (grub_ieee1275_test_flag 
>> (GRUB_IEEE1275_FLAG_CAN_TRY_CAS_FOR_MORE_MEMORY))
>>      {
>> -      /* if we have an error, don't call CAS, just hope for the best 
>> */
>> -      if (err == GRUB_ERR_NONE && rmo_top < (512 * 1024 * 1024))
>> -	grub_ieee1275_ibm_cas ();
>> +      /*
>> +       * If we have an error or the reboot is detected as CAS reboot,
>> +       * don't call CAS, just hope for the best.
>> +       * Along with the above, if the rmo_top is 512 MB or above. We
>> +       * will skip the CAS call. Though if we call CAS, the rmo_top 
>> will
>> +       * be set to 768 MB via CAS Vector2. This condition is required 
>> to avoid the
>> +       * issue where the older Linux kernels are still using rmo_top 
>> as 512 MB.
>> +       * If we call CAS where rmo_top is less then 768 MB, this will 
>> result in an issue
>> +       * due to IBM CAS reboot feature and we won't be able to boot 
>> the newer kernel.
> 
> Could you be more specific? What is "an issue due to IBM CAS reboot 
> feature"?
> 
> And I think it would be nice if you put here a reference to 
> documentation,
> including chapters names, etc., which discuss RMA and issues fixed 
> here.
> 
>> +       * The machine will boot with the last booted kernel which has 
>> rmo_top as 512 MB.
>> +       */
>> +      if (!ibm_ca_support_reboot && err == GRUB_ERR_NONE && rmo_top < 
>> (512 * 1024 * 1024))
>> +        grub_ieee1275_ibm_cas ();
>>      }
>>  #endif
> 
> Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
  2025-03-07  9:01   ` Avnish Chouhan
@ 2025-03-10 13:12     ` Daniel Kiper
  2025-03-11  9:29       ` Avnish Chouhan
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Kiper @ 2025-03-10 13:12 UTC (permalink / raw)
  To: Avnish Chouhan; +Cc: grub-devel, daniel.kiper, brking, meghanaprakash, mamatha4

On Fri, Mar 07, 2025 at 02:31:18PM +0530, Avnish Chouhan wrote:
> Hi Daniel,
> Thank you so much for your patch reviews.
>
> I'll replace the words as suggested by you.

Thank you!

> ****
> > > +       * If we have an error or the reboot is detected as CAS reboot,
> > > +       * don't call CAS, just hope for the best.
> > > +       * Along with the above, if the rmo_top is 512 MB or above. We
> > > +       * will skip the CAS call. Though if we call CAS, the rmo_top
> > > will
> > > +       * be set to 768 MB via CAS Vector2. This condition is
> > > required to avoid the
> > > +       * issue where the older Linux kernels are still using
> > > rmo_top as 512 MB.
> > > +       * If we call CAS where rmo_top is less then 768 MB, this
> > > will result in an issue
> > > +       * due to IBM CAS reboot feature and we won't be able to boot
> > > the newer kernel.
> >
> > Could you be more specific? What is "an issue due to IBM CAS reboot
> > feature"?
> >
> > And I think it would be nice if you put here a reference to
> > documentation,
> > including chapters names, etc., which discuss RMA and issues fixed here.
> >
> > > +       * The machine will boot with the last booted kernel which
> > > has rmo_top as 512 MB.
> > > +       */
> ****
>
> On this. This patch only change the size of RMA from 512 MB to 768 MB. The
> change is done via CAS call. Condition for calling a CAS has no change other
> than adding a check on "whether the reboot is a CAS reboot". This is
> required to avoid unwanted and repetitive CAS calls.

OK...

> With this CAS reboot check condition, in any scenario, where we are using
> older kernel and CAS is still using 512 MB RMA but with the updated Grub.

I am not sure I understand this sentence...

> The repeated CAS calls will be avoided.
>
> In IBM CAS reboot feature, whenever CAS call occurred. We skip providing the

Ditto...

> kernel options to boot to and we directly boot to the lasted booted kernel.

s/lasted/last/?

> With this feature in place. If we upgrade the machine's kernel and grub, and

The project name is GRUB not grub nor Grub... Please be consistent...

> as the machine is lasted booted with old kernel. Any CAS call from Grub will

s/lasted/last/?

> restrict user to keep booting to last booted kernel!

Again, I am not sure I understand this paragraph. Please rephrase it.

> So these conditions are made to avoid any of these possible issues.
>
> on adding the documentation, we can add the PAPR document link which has
> been shared with you earlier by IBM folks.

That would be perfect!

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
  2025-03-10 13:12     ` Daniel Kiper
@ 2025-03-11  9:29       ` Avnish Chouhan
  2025-03-11 13:44         ` Daniel Kiper
  0 siblings, 1 reply; 14+ messages in thread
From: Avnish Chouhan @ 2025-03-11  9:29 UTC (permalink / raw)
  To: Daniel Kiper; +Cc: grub-devel, brking, meghanaprakash, mamatha4

Hi Daniel,
Thank you for your response!

--------------
Condition before the patch:

if (err == GRUB_ERR_NONE && rmo_top < (512 * 1024 * 1024))
   grub_ieee1275_ibm_cas ();

Condition after the patch:

if (!ibm_ca_support_reboot && err == GRUB_ERR_NONE && rmo_top < (512 * 
1024 * 1024))
   grub_ieee1275_ibm_cas ();

--------------

We have added just one extra check in the code "!ibm_ca_support_reboot" 
to check whether the reboot is a CAS reboot or not!

And these are below comments in the patch which are in question:

+      /*
+       * If we have an error or the reboot is detected as CAS reboot,
+       * don't call CAS, just hope for the best.
+       * Along with the above, if the rmo_top is 512 MB or above. We
+       * will skip the CAS call. Though if we call CAS, the rmo_top 
will
+       * be set to 768 MB via CAS Vector2. This condition is required 
to avoid the
+       * issue where the older Linux kernels are still using rmo_top as 
512 MB.
+       * If we call CAS where rmo_top is less then 768 MB, this will 
result in an issue
+       * due to IBM CAS reboot feature and we won't be able to boot the 
newer kernel.
+       * The machine will boot with the last booted kernel which has 
rmo_top as 512 MB.
+       */

I'm tried to explain in the comment on when the CAS will be called. And 
why we need to use this old condition "rmo_top < 512 MB" and not 
"rmo_top < 768 MB".

+      if (!ibm_ca_support_reboot && err == GRUB_ERR_NONE && rmo_top < 
(512 * 1024 * 1024))
+        grub_ieee1275_ibm_cas ();
      }


Condition 1: (!ibm_ca_support_reboot)

This condition checks whether the last reboot is caused by CAS. If the 
reboot is detected as a CAS reboot, the GRUB will skip the CAS call. As 
the CAS has already been called earlier and it's not required to call 
even if the other conditions are met!

Condition 2: (rmo_top < (512 * 1024 * 1024))

If the machine detects rmo_top as less than 512 MB, the CAS will be 
called.

Why we need this condition:

Logically as we are changing MIN_RMA as 768 MB in GRUB Options_vector2. 
We should check "rmo_top < (768 * 1024 * 1024)" and not "rmo_top < (512 
* 1024 * 1024)".

In the patch, whenever we are calling CAS. We set MIN_RMA as 768 MB. But 
we decide when to call CAS is based on old condition rmo_top < 512 MB. 
Logically it should be 768 MB. But we can't do this right now due to the 
below scenarios. We will change this condition to "rmo_top < (768 * 1024 
* 1024)" in the future.

*****
Scenario 1:
In kernel prom_init.c file. The Options_vector2 is using 512 MB as 
MIN_RMA. And GRUB is using "rmo_top < (768 * 1024 * 1024)" to call CAS.

1. Machine boots, GRUB detects rmo_top as less than 512 MB.
    GRUB calls CAS and sets MIN_RMA as 768MB.
    The machine reboots after the CAS call. (Every CAS call will result 
in a reboot)
2. Machine boots, GRUB detects rmo_top is not as less than 512 MB.
    GRUB skips CAS call.
3. After this kernel boots and detects MIN_RMA as other than its 512 MB 
required value.
    It calls CAS and makes the MIN_RMA again to 512 MB.
    As the CAS is called, the machine will go for a reboot again.

4. Now GRUB will again detects rmo_top as less than 512 MB (changed by 
kernel).
    And then we will again go back to step 1.

And machine will keep doing the CAS calls and change MIN_RMA from 512 to 
768 to 512 to 768 and so on. With this, the machine will stuck in this 
stage forever!
*****

In the above scenario 1, with (!ibm_ca_support_reboot) condition in 
place. We will avoid this CAS reboot loop. But if we use "rmo_top < (768 
* 1024 * 1024)". The machine will never get stuck in reboot loop, but as 
the CAS is called from GRUB (currently all the powerpc machines has 
rmo_top is 512 MB). The IBM CAS reboot feature will not allow us to boot 
with the newer kernel!

IBM CAS reboot feature:

Whenever a reboot is detected as the CAS reboot by GRUB. It will boot 
the machine with the last booted kernel by reading the variable 
"boot-last-label" that has the info related to the last boot. This is 
specific to IBM powerpc and no other architecture  has this.

*****
Scenario 2:
In kernel prom_init.c file. The Options_vector2 is using 768 MB as 
MIN_RMA. And GRUB is using "rmo_top < (768 * 1024 * 1024)" to call CAS.

1. Machine boots, GRUB detects rmo_top as less than 512 MB.
    GRUB calls CAS and sets MIN_RMA as 768MB.
    The machine reboots after the CAS call. (Every CAS call will result 
in a reboot)
2. Machine boots, GRUB detects rmo_top is not as less than 512 MB.
    GRUB skips CAS call.
3. But as the last boot was a CAS reboot, the machine will boot with the 
last booted kernel having MIN_RMA as 512 MB. We will not see an option 
to choose which kernel a user like to boot to.
*****

_________________

Please let me know if you feel I need to change or add any content in my 
"comment" in the patch. I have tried my best to explain and cover these 
above scenarios in simple and short message.
And let me know if you have any queries on this!

Thank you,
Avnish Chouhan


On 2025-03-10 18:42, Daniel Kiper wrote:
> On Fri, Mar 07, 2025 at 02:31:18PM +0530, Avnish Chouhan wrote:
>> Hi Daniel,
>> Thank you so much for your patch reviews.
>> 
>> I'll replace the words as suggested by you.
> 
> Thank you!
> 
>> ****
>> > > +       * If we have an error or the reboot is detected as CAS reboot,
>> > > +       * don't call CAS, just hope for the best.
>> > > +       * Along with the above, if the rmo_top is 512 MB or above. We
>> > > +       * will skip the CAS call. Though if we call CAS, the rmo_top
>> > > will
>> > > +       * be set to 768 MB via CAS Vector2. This condition is
>> > > required to avoid the
>> > > +       * issue where the older Linux kernels are still using
>> > > rmo_top as 512 MB.
>> > > +       * If we call CAS where rmo_top is less then 768 MB, this
>> > > will result in an issue
>> > > +       * due to IBM CAS reboot feature and we won't be able to boot
>> > > the newer kernel.
>> >
>> > Could you be more specific? What is "an issue due to IBM CAS reboot
>> > feature"?
>> >
>> > And I think it would be nice if you put here a reference to
>> > documentation,
>> > including chapters names, etc., which discuss RMA and issues fixed here.
>> >
>> > > +       * The machine will boot with the last booted kernel which
>> > > has rmo_top as 512 MB.
>> > > +       */
>> ****
>> 
>> On this. This patch only change the size of RMA from 512 MB to 768 MB. 
>> The
>> change is done via CAS call. Condition for calling a CAS has no change 
>> other
>> than adding a check on "whether the reboot is a CAS reboot". This is
>> required to avoid unwanted and repetitive CAS calls.
> 
> OK...
> 
>> With this CAS reboot check condition, in any scenario, where we are 
>> using
>> older kernel and CAS is still using 512 MB RMA but with the updated 
>> Grub.
> 
> I am not sure I understand this sentence...
> 
>> The repeated CAS calls will be avoided.
>> 
>> In IBM CAS reboot feature, whenever CAS call occurred. We skip 
>> providing the
> 
> Ditto...
> 
>> kernel options to boot to and we directly boot to the lasted booted 
>> kernel.
> 
> s/lasted/last/?
> 
>> With this feature in place. If we upgrade the machine's kernel and 
>> grub, and
> 
> The project name is GRUB not grub nor Grub... Please be consistent...
> 
>> as the machine is lasted booted with old kernel. Any CAS call from 
>> Grub will
> 
> s/lasted/last/?
> 
>> restrict user to keep booting to last booted kernel!
> 
> Again, I am not sure I understand this paragraph. Please rephrase it.
> 
>> So these conditions are made to avoid any of these possible issues.
>> 
>> on adding the documentation, we can add the PAPR document link which 
>> has
>> been shared with you earlier by IBM folks.
> 
> That would be perfect!
> 
> Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
  2025-03-11  9:29       ` Avnish Chouhan
@ 2025-03-11 13:44         ` Daniel Kiper
  2025-03-12  9:46           ` Avnish Chouhan
  0 siblings, 1 reply; 14+ messages in thread
From: Daniel Kiper @ 2025-03-11 13:44 UTC (permalink / raw)
  To: Avnish Chouhan; +Cc: grub-devel, brking, meghanaprakash, mamatha4

On Tue, Mar 11, 2025 at 02:59:35PM +0530, Avnish Chouhan wrote:
> Hi Daniel,
> Thank you for your response!
>
> --------------
> Condition before the patch:
>
> if (err == GRUB_ERR_NONE && rmo_top < (512 * 1024 * 1024))
>   grub_ieee1275_ibm_cas ();
>
> Condition after the patch:
>
> if (!ibm_ca_support_reboot && err == GRUB_ERR_NONE && rmo_top < (512 * 1024 * 1024))
>   grub_ieee1275_ibm_cas ();

Avnish, I understand the code. The problem is the commit message and
comment are partially unreadable/incomprehensible. So, that is why
I am asking you to rephrase them. Now they are better but still...

> --------------
>
> We have added just one extra check in the code "!ibm_ca_support_reboot" to
> check whether the reboot is a CAS reboot or not!
>
> And these are below comments in the patch which are in question:
>
> +      /*
> +       * If we have an error or the reboot is detected as CAS reboot,
> +       * don't call CAS, just hope for the best.
> +       * Along with the above, if the rmo_top is 512 MB or above. We
> +       * will skip the CAS call. Though if we call CAS, the rmo_top will
> +       * be set to 768 MB via CAS Vector2. This condition is required to avoid the
> +       * issue where the older Linux kernels are still using rmo_top as 512 MB.
> +       * If we call CAS where rmo_top is less then 768 MB, this will result in an issue
> +       * due to IBM CAS reboot feature and we won't be able to boot the newer kernel.
> +       * The machine will boot with the last booted kernel which has rmo_top as 512 MB.
> +       */
>
> I'm tried to explain in the comment on when the CAS will be called. And why
> we need to use this old condition "rmo_top < 512 MB" and not "rmo_top < 768
> MB".
>
> +      if (!ibm_ca_support_reboot && err == GRUB_ERR_NONE && rmo_top < (512
> * 1024 * 1024))
> +        grub_ieee1275_ibm_cas ();
>      }
>
>
> Condition 1: (!ibm_ca_support_reboot)
>
> This condition checks whether the last reboot is caused by CAS. If the
> reboot is detected as a CAS reboot, the GRUB will skip the CAS call. As the
> CAS has already been called earlier and it's not required to call even if
> the other conditions are met!
>
> Condition 2: (rmo_top < (512 * 1024 * 1024))
>
> If the machine detects rmo_top as less than 512 MB, the CAS will be called.
>
> Why we need this condition:
>
> Logically as we are changing MIN_RMA as 768 MB in GRUB Options_vector2. We
> should check "rmo_top < (768 * 1024 * 1024)" and not "rmo_top < (512 * 1024
> * 1024)".
>
> In the patch, whenever we are calling CAS. We set MIN_RMA as 768 MB. But we
> decide when to call CAS is based on old condition rmo_top < 512 MB.
> Logically it should be 768 MB. But we can't do this right now due to the
> below scenarios. We will change this condition to "rmo_top < (768 * 1024 *
> 1024)" in the future.
>
> *****
> Scenario 1:
> In kernel prom_init.c file. The Options_vector2 is using 512 MB as MIN_RMA.
> And GRUB is using "rmo_top < (768 * 1024 * 1024)" to call CAS.
>
> 1. Machine boots, GRUB detects rmo_top as less than 512 MB.
>    GRUB calls CAS and sets MIN_RMA as 768MB.
>    The machine reboots after the CAS call. (Every CAS call will result in a reboot)
> 2. Machine boots, GRUB detects rmo_top is not as less than 512 MB.
>    GRUB skips CAS call.
> 3. After this kernel boots and detects MIN_RMA as other than its 512 MB required value.
>    It calls CAS and makes the MIN_RMA again to 512 MB.
>    As the CAS is called, the machine will go for a reboot again.
>
> 4. Now GRUB will again detects rmo_top as less than 512 MB (changed by kernel).
>    And then we will again go back to step 1.
>
> And machine will keep doing the CAS calls and change MIN_RMA from 512 to 768
> to 512 to 768 and so on. With this, the machine will stuck in this stage
> forever!
> *****
>
> In the above scenario 1, with (!ibm_ca_support_reboot) condition in place.
> We will avoid this CAS reboot loop. But if we use "rmo_top < (768 * 1024 *
> 1024)". The machine will never get stuck in reboot loop, but as the CAS is
> called from GRUB (currently all the powerpc machines has rmo_top is 512 MB).
> The IBM CAS reboot feature will not allow us to boot with the newer kernel!
>
> IBM CAS reboot feature:
>
> Whenever a reboot is detected as the CAS reboot by GRUB. It will boot the
> machine with the last booted kernel by reading the variable
> "boot-last-label" that has the info related to the last boot. This is
> specific to IBM powerpc and no other architecture  has this.
>
> *****
> Scenario 2:
> In kernel prom_init.c file. The Options_vector2 is using 768 MB as MIN_RMA.
> And GRUB is using "rmo_top < (768 * 1024 * 1024)" to call CAS.
>
> 1. Machine boots, GRUB detects rmo_top as less than 512 MB.
>    GRUB calls CAS and sets MIN_RMA as 768MB.
>    The machine reboots after the CAS call. (Every CAS call will result in a reboot)
> 2. Machine boots, GRUB detects rmo_top is not as less than 512 MB.
>    GRUB skips CAS call.
> 3. But as the last boot was a CAS reboot, the machine will boot with the
> last booted kernel having MIN_RMA as 512 MB. We will not see an option to
> choose which kernel a user like to boot to.
> *****
>
> _________________
>
> Please let me know if you feel I need to change or add any content in my
> "comment" in the patch. I have tried my best to explain and cover these
> above scenarios in simple and short message.
> And let me know if you have any queries on this!

I think something like that, maybe a bit shortened by dropping some
obvious things, should land in the comment...

Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
  2025-03-11 13:44         ` Daniel Kiper
@ 2025-03-12  9:46           ` Avnish Chouhan
  0 siblings, 0 replies; 14+ messages in thread
From: Avnish Chouhan @ 2025-03-12  9:46 UTC (permalink / raw)
  To: Daniel Kiper; +Cc: grub-devel, brking, meghanaprakash, mamatha4

Thank you Daniel!

As suggested, I will rephrase the comment and re submit the patch!

On 2025-03-11 19:14, Daniel Kiper wrote:
> On Tue, Mar 11, 2025 at 02:59:35PM +0530, Avnish Chouhan wrote:
>> Hi Daniel,
>> Thank you for your response!
>> 
>> --------------
>> Condition before the patch:
>> 
>> if (err == GRUB_ERR_NONE && rmo_top < (512 * 1024 * 1024))
>>   grub_ieee1275_ibm_cas ();
>> 
>> Condition after the patch:
>> 
>> if (!ibm_ca_support_reboot && err == GRUB_ERR_NONE && rmo_top < (512 * 
>> 1024 * 1024))
>>   grub_ieee1275_ibm_cas ();
> 
> Avnish, I understand the code. The problem is the commit message and
> comment are partially unreadable/incomprehensible. So, that is why
> I am asking you to rephrase them. Now they are better but still...
> 
>> --------------
>> 
>> We have added just one extra check in the code 
>> "!ibm_ca_support_reboot" to
>> check whether the reboot is a CAS reboot or not!
>> 
>> And these are below comments in the patch which are in question:
>> 
>> +      /*
>> +       * If we have an error or the reboot is detected as CAS reboot,
>> +       * don't call CAS, just hope for the best.
>> +       * Along with the above, if the rmo_top is 512 MB or above. We
>> +       * will skip the CAS call. Though if we call CAS, the rmo_top 
>> will
>> +       * be set to 768 MB via CAS Vector2. This condition is required 
>> to avoid the
>> +       * issue where the older Linux kernels are still using rmo_top 
>> as 512 MB.
>> +       * If we call CAS where rmo_top is less then 768 MB, this will 
>> result in an issue
>> +       * due to IBM CAS reboot feature and we won't be able to boot 
>> the newer kernel.
>> +       * The machine will boot with the last booted kernel which has 
>> rmo_top as 512 MB.
>> +       */
>> 
>> I'm tried to explain in the comment on when the CAS will be called. 
>> And why
>> we need to use this old condition "rmo_top < 512 MB" and not "rmo_top 
>> < 768
>> MB".
>> 
>> +      if (!ibm_ca_support_reboot && err == GRUB_ERR_NONE && rmo_top < 
>> (512
>> * 1024 * 1024))
>> +        grub_ieee1275_ibm_cas ();
>>      }
>> 
>> 
>> Condition 1: (!ibm_ca_support_reboot)
>> 
>> This condition checks whether the last reboot is caused by CAS. If the
>> reboot is detected as a CAS reboot, the GRUB will skip the CAS call. 
>> As the
>> CAS has already been called earlier and it's not required to call even 
>> if
>> the other conditions are met!
>> 
>> Condition 2: (rmo_top < (512 * 1024 * 1024))
>> 
>> If the machine detects rmo_top as less than 512 MB, the CAS will be 
>> called.
>> 
>> Why we need this condition:
>> 
>> Logically as we are changing MIN_RMA as 768 MB in GRUB 
>> Options_vector2. We
>> should check "rmo_top < (768 * 1024 * 1024)" and not "rmo_top < (512 * 
>> 1024
>> * 1024)".
>> 
>> In the patch, whenever we are calling CAS. We set MIN_RMA as 768 MB. 
>> But we
>> decide when to call CAS is based on old condition rmo_top < 512 MB.
>> Logically it should be 768 MB. But we can't do this right now due to 
>> the
>> below scenarios. We will change this condition to "rmo_top < (768 * 
>> 1024 *
>> 1024)" in the future.
>> 
>> *****
>> Scenario 1:
>> In kernel prom_init.c file. The Options_vector2 is using 512 MB as 
>> MIN_RMA.
>> And GRUB is using "rmo_top < (768 * 1024 * 1024)" to call CAS.
>> 
>> 1. Machine boots, GRUB detects rmo_top as less than 512 MB.
>>    GRUB calls CAS and sets MIN_RMA as 768MB.
>>    The machine reboots after the CAS call. (Every CAS call will result 
>> in a reboot)
>> 2. Machine boots, GRUB detects rmo_top is not as less than 512 MB.
>>    GRUB skips CAS call.
>> 3. After this kernel boots and detects MIN_RMA as other than its 512 
>> MB required value.
>>    It calls CAS and makes the MIN_RMA again to 512 MB.
>>    As the CAS is called, the machine will go for a reboot again.
>> 
>> 4. Now GRUB will again detects rmo_top as less than 512 MB (changed by 
>> kernel).
>>    And then we will again go back to step 1.
>> 
>> And machine will keep doing the CAS calls and change MIN_RMA from 512 
>> to 768
>> to 512 to 768 and so on. With this, the machine will stuck in this 
>> stage
>> forever!
>> *****
>> 
>> In the above scenario 1, with (!ibm_ca_support_reboot) condition in 
>> place.
>> We will avoid this CAS reboot loop. But if we use "rmo_top < (768 * 
>> 1024 *
>> 1024)". The machine will never get stuck in reboot loop, but as the 
>> CAS is
>> called from GRUB (currently all the powerpc machines has rmo_top is 
>> 512 MB).
>> The IBM CAS reboot feature will not allow us to boot with the newer 
>> kernel!
>> 
>> IBM CAS reboot feature:
>> 
>> Whenever a reboot is detected as the CAS reboot by GRUB. It will boot 
>> the
>> machine with the last booted kernel by reading the variable
>> "boot-last-label" that has the info related to the last boot. This is
>> specific to IBM powerpc and no other architecture  has this.
>> 
>> *****
>> Scenario 2:
>> In kernel prom_init.c file. The Options_vector2 is using 768 MB as 
>> MIN_RMA.
>> And GRUB is using "rmo_top < (768 * 1024 * 1024)" to call CAS.
>> 
>> 1. Machine boots, GRUB detects rmo_top as less than 512 MB.
>>    GRUB calls CAS and sets MIN_RMA as 768MB.
>>    The machine reboots after the CAS call. (Every CAS call will result 
>> in a reboot)
>> 2. Machine boots, GRUB detects rmo_top is not as less than 512 MB.
>>    GRUB skips CAS call.
>> 3. But as the last boot was a CAS reboot, the machine will boot with 
>> the
>> last booted kernel having MIN_RMA as 512 MB. We will not see an option 
>> to
>> choose which kernel a user like to boot to.
>> *****
>> 
>> _________________
>> 
>> Please let me know if you feel I need to change or add any content in 
>> my
>> "comment" in the patch. I have tried my best to explain and cover 
>> these
>> above scenarios in simple and short message.
>> And let me know if you have any queries on this!
> 
> I think something like that, maybe a bit shortened by dropping some
> obvious things, should land in the comment...
> 
> Daniel

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2025-03-12  9:47 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-06  6:55 [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation Avnish Chouhan
2024-12-07  1:58 ` Michael Ellerman
2024-12-11 12:05   ` Avnish Chouhan
2025-01-09  9:32   ` Sourabh Jain
2025-01-22 12:43 ` Madhavan Srinivasan
2025-01-23  3:06   ` Sourabh Jain
2025-01-24  3:58   ` Sourabh Jain
  -- strict thread matches above, loose matches on Subject: below --
2025-03-03 16:49 Avnish Chouhan
2025-03-04 15:09 ` Daniel Kiper
2025-03-07  9:01   ` Avnish Chouhan
2025-03-10 13:12     ` Daniel Kiper
2025-03-11  9:29       ` Avnish Chouhan
2025-03-11 13:44         ` Daniel Kiper
2025-03-12  9:46           ` Avnish Chouhan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.