Re: [PATCH 2/2] powerpc/fadump: fix additional param memory reservation for HASH MMU

[Hari Bathini <hbathini@linux.ibm.com>]

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