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
