All of lore.kernel.org
 help / color / mirror / Atom feed
From: Avnish Chouhan <avnish@linux.ibm.com>
To: Daniel Kiper <dkiper@net-space.pl>
Cc: grub-devel@gnu.org, daniel.kiper@oracle.com,
	brking@linux.ibm.com, meghanaprakash@in.ibm.com,
	mamatha4@linux.ibm.com
Subject: Re: [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation
Date: Fri, 07 Mar 2025 14:31:18 +0530	[thread overview]
Message-ID: <33f136fcd288bd539212f9dcc0918eec@linux.ibm.com> (raw)
In-Reply-To: <20250304150949.wvevh5f6rxlrmabr@tomti.i.net-space.pl>

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

  reply	other threads:[~2025-03-07  9:01 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-03 16:49 [PATCH v2] powerpc: increase MIN RMA size for CAS negotiation Avnish Chouhan
2025-03-04 15:09 ` Daniel Kiper
2025-03-07  9:01   ` Avnish Chouhan [this message]
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
  -- strict thread matches above, loose matches on Subject: below --
2024-12-06  6:55 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=33f136fcd288bd539212f9dcc0918eec@linux.ibm.com \
    --to=avnish@linux.ibm.com \
    --cc=brking@linux.ibm.com \
    --cc=daniel.kiper@oracle.com \
    --cc=dkiper@net-space.pl \
    --cc=grub-devel@gnu.org \
    --cc=mamatha4@linux.ibm.com \
    --cc=meghanaprakash@in.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.