Re: [PATCH v2 1/4] powerpc/mmu: do MMU type discovery before crashkernel reservation

[Sourabh Jain <sourabhjain@linux.ibm.com>]

On 10/07/26 08:14, Ritesh Harjani (IBM) wrote:
> Minor nits
>
> Sourabh Jain <sourabhjain@linux.ibm.com> writes:
>
>> Crashkernel reservation on high memory depends on the MMU type, so
>> finalize the MMU type before calling arch_reserve_crashkernel().
>>
>> With the changes introduced here, early_radix_enabled() becomes usable
>> and will be used in arch_reserve_crashkernel() in the upcoming patch.
>>
>> early_radix_enabled() depends on cur_cpu_spec->mmu_features to find
>> out if the radix MMU is enabled. The radix MMU bit in mmu_features is
>> discovered from the FDT and kernel configs. To make sure the MMU type is
>> finalized before arch_reserve_crashkernel() is called, the function that
>> scans the FDT and sets mmu_features, along with some bits from
>> mmu_early_type_finalize(), has been moved above
>> arch_reserve_crashkernel().
>>
> Can you also add a short description of why can't we move
> arch_reserve_crashkernel() to a later point instead of breaking
> mmu_early_init_devtree() and moving the xx_type_finalize() part above?

The main reason is that the crashkernel requires one large contiguous memory
block to be reserved at a specific memory offset. To ensure this 
allocation succeeds,
the crashkernel memory is reserved before any other components reserve 
memory.

> If I am not wrong, it is since move_device_tree() checks whether the FDT
> overlaps the crash kernel reservation. So arch_reserve_crashkernel()
> must be called before move_device_tree().

I think the bootloader loads the device tree and may chose a location 
that may falls
within the crashkernel region. The purpose of move_device_tree() is to 
relocate
the device tree so that it no longer occupies the crashkernel memory. 
Otherwise,
that memory region would also need to be excluded from the crashkernel
reserved area (and also need special APIs to access device tree present in
reserved memory regions), similar to the RTAS region.

- Sourabh Jain

>
>
>> Signed-off-by: Sourabh Jain <sourabhjain@linux.ibm.com>
>> ---
>>   arch/powerpc/include/asm/book3s/64/mmu.h |  1 +
>>   arch/powerpc/include/asm/mmu.h           |  1 +
>>   arch/powerpc/kernel/prom.c               | 28 +++++++++++++-----------
>>   arch/powerpc/mm/init_64.c                | 27 ++++++++++++++---------
>>   4 files changed, 34 insertions(+), 23 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
>> index 48631365b48c..7a3b2ff02041 100644
>> --- a/arch/powerpc/include/asm/book3s/64/mmu.h
>> +++ b/arch/powerpc/include/asm/book3s/64/mmu.h
>> @@ -208,6 +208,7 @@ extern int mmu_vmemmap_psize;
>>   
>>   /* MMU initialization */
>>   void mmu_early_init_devtree(void);
>> +void mmu_early_type_finalize(void);
> can you rename this as mmu_early_init_type().
>
> Otherwise the change looks good to me. With the above 2 addressed, feel
> free to add:
>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>
>