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

Couple of minor nits below..

On 08/07/26 8:03 pm, Sourabh Jain wrote:
> 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().
> 
> 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);
>   void hash__early_init_devtree(void);
>   void radix__early_init_devtree(void);
>   #ifdef CONFIG_PPC_PKEY
> diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
> index 5f9c5d436e17..c40dc6349e55 100644
> --- a/arch/powerpc/include/asm/mmu.h
> +++ b/arch/powerpc/include/asm/mmu.h
> @@ -384,6 +384,7 @@ extern void early_init_mmu_secondary(void);
>   extern void setup_initial_memory_limit(phys_addr_t first_memblock_base,
>   				       phys_addr_t first_memblock_size);
>   static inline void mmu_early_init_devtree(void) { }
> +static inline void mmu_early_type_finalize(void) { }
>   
>   static inline void pkey_early_init_devtree(void) {}
>   
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index 9ed9dde7d231..db1615f26075 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -853,6 +853,21 @@ void __init early_init_devtree(void *params)
>   	if (PHYSICAL_START > MEMORY_START)
>   		memblock_reserve(MEMORY_START, int_vector_size);
>   	reserve_kdump_trampoline();
> +
> +	DBG("Scanning CPUs ...\n");
> +
> +	dt_cpu_ftrs_scan();
> +
> +	/* Retrieve CPU related informations from the flat tree
> +	 * (altivec support, boot CPU ID, ...)
> +	 */
> +	of_scan_flat_dt(early_init_dt_scan_cpus, NULL);
> +	if (boot_cpuid < 0) {
> +		printk("Failed to identify boot CPU !\n");
> +		BUG();
> +	}
> +

> +	mmu_early_type_finalize();
>   #if defined(CONFIG_FA_DUMP) || defined(CONFIG_PRESERVE_FA_DUMP)

Please add a blank line here

>   	/*
>   	 * If we fail to reserve memory for firmware-assisted dump then
> @@ -884,19 +899,6 @@ void __init early_init_devtree(void *params)
>   	 * FIXME .. and the initrd too? */
>   	move_device_tree();
>   
> -	DBG("Scanning CPUs ...\n");
> -
> -	dt_cpu_ftrs_scan();
> -
> -	/* Retrieve CPU related informations from the flat tree
> -	 * (altivec support, boot CPU ID, ...)
> -	 */
> -	of_scan_flat_dt(early_init_dt_scan_cpus, NULL);
> -	if (boot_cpuid < 0) {
> -		printk("Failed to identify boot CPU !\n");
> -		BUG();
> -	}
> -
>   	save_fscr_to_task();
>   
>   #if defined(CONFIG_SMP) && defined(CONFIG_PPC64)
> diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
> index 64f0df5bb5cd..2fd533bc17cc 100644
> --- a/arch/powerpc/mm/init_64.c
> +++ b/arch/powerpc/mm/init_64.c
> @@ -614,8 +614,10 @@ static void __init early_init_memory_block_size(void)
>   	of_scan_flat_dt(probe_memory_block_size, &memory_block_size);
>   }
>   
> -void __init mmu_early_init_devtree(void)
> +

> +void __init mmu_early_type_finalize(void)
>   {
> +

Please remove this extra blank line

>   	bool hvmode = !!(mfmsr() & MSR_HV);
>   
>   	/* Disable radix mode based on kernel command line. */
> @@ -626,6 +628,20 @@ void __init mmu_early_init_devtree(void)
>   			pr_warn("WARNING: Ignoring cmdline option disable_radix\n");
>   	}
>   
> +	/*
> +	 * Check /chosen/ibm,architecture-vec-5 if running as a guest.
> +	 * When running bare-metal, we can use radix if we like
> +	 * even though the ibm,architecture-vec-5 property created by
> +	 * skiboot doesn't have the necessary bits set.
> +	 */
> +	if (!hvmode)
> +		early_check_vec5();
> +}
> +
> +void __init mmu_early_init_devtree(void)
> +{
> +	bool hvmode = !!(mfmsr() & MSR_HV);
> +
>   	of_scan_flat_dt(dt_scan_mmu_pid_width, NULL);
>   	if (hvmode && !mmu_lpid_bits) {
>   		if (early_cpu_has_feature(CPU_FTR_ARCH_207S))
> @@ -638,15 +654,6 @@ void __init mmu_early_init_devtree(void)
>   			mmu_pid_bits = 20; /* POWER9-10 */
>   	}
>   
> -	/*
> -	 * Check /chosen/ibm,architecture-vec-5 if running as a guest.
> -	 * When running bare-metal, we can use radix if we like
> -	 * even though the ibm,architecture-vec-5 property created by
> -	 * skiboot doesn't have the necessary bits set.
> -	 */
> -	if (!hvmode)
> -		early_check_vec5();
> -
>   	early_init_memory_block_size();
>   
>   	if (early_radix_enabled()) {

- Hari