On 10/07/26 08:14, Ritesh Harjani (IBM) wrote:
Minor nits

Sourabh Jain <[email protected]> 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 <[email protected]>
---
  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) <[email protected]>




Reply via email to