Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation

[Avnish Chouhan <avnish@linux.ibm.com>]

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