[RFC,FIX,v1,1/2] powerpc: Discover radix availability before scanning the memory nodes

Message ID 1515150321-24894-2-git-send-email-bharata@linux.vnet.ibm.com
State New
Headers show
Series
  • powerpc: Fix memory unplug failure for radix guests
Related show

Commit Message

Bharata B Rao Jan. 5, 2018, 11:05 a.m.
Currently device tree nodes for memory are scanned before the
radix feature is discovered in mmu_early_init_devtree(). Move this
routine ahead of scanning memory nodes so that we know if the
guest is radix or not when scanning ibm,dynamic-reconfiguration-memory.

Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/prom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Aneesh Kumar K.V Jan. 5, 2018, 11:16 a.m. | #1
On 01/05/2018 04:35 PM, Bharata B Rao wrote:
> Currently device tree nodes for memory are scanned before the
> radix feature is discovered in mmu_early_init_devtree(). Move this
> routine ahead of scanning memory nodes so that we know if the
> guest is radix or not when scanning ibm,dynamic-reconfiguration-memory.
> 
> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com>
> ---
>   arch/powerpc/kernel/prom.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index b15bae2..079d893 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -722,6 +722,8 @@ void __init early_init_devtree(void *params)
>   	 */
>   	of_scan_flat_dt(early_init_dt_scan_chosen_ppc, boot_command_line);
> 
> +	mmu_early_init_devtree();
> +
>   	/* Scan memory nodes and rebuild MEMBLOCKs */
>   	of_scan_flat_dt(early_init_dt_scan_root, NULL);
>   	of_scan_flat_dt(early_init_dt_scan_memory_ppc, NULL);
> @@ -783,8 +785,6 @@ void __init early_init_devtree(void *params)
>   	spinning_secondaries = boot_cpu_count - 1;
>   #endif
> 
> -	mmu_early_init_devtree();
> -
>   #ifdef CONFIG_PPC_POWERNV
>   	/* Scan and build the list of machine check recoverable ranges */
>   	of_scan_flat_dt(early_init_dt_scan_recoverable_ranges, NULL);
> 

I guess that will cause issues with pa-feature scanning. I don't think 
we finalize cpu features/mmu features that early.

-aneesh
Michael Ellerman Jan. 5, 2018, 11:28 p.m. | #2
Bharata B Rao <bharata@linux.vnet.ibm.com> writes:

> Currently device tree nodes for memory are scanned before the
> radix feature is discovered in mmu_early_init_devtree(). Move this
> routine ahead of scanning memory nodes so that we know if the
> guest is radix or not when scanning ibm,dynamic-reconfiguration-memory.

Sorry this doesn't work.

> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index b15bae2..079d893 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -722,6 +722,8 @@ void __init early_init_devtree(void *params)
>  	 */
>  	of_scan_flat_dt(early_init_dt_scan_chosen_ppc, boot_command_line);
>  
> +	mmu_early_init_devtree();
> +

You've moved this above parse_early_param(), but
mmu_early_init_devtree() uses disable_radix, which is an early param. So
this will break disable_radix handling.

It will probably break other things too because the ordering of this
init code is very fragile - bootstrapping is hard :)

>  	/* Scan memory nodes and rebuild MEMBLOCKs */
>  	of_scan_flat_dt(early_init_dt_scan_root, NULL);
>  	of_scan_flat_dt(early_init_dt_scan_memory_ppc, NULL);
> @@ -783,8 +785,6 @@ void __init early_init_devtree(void *params)
>  	spinning_secondaries = boot_cpu_count - 1;
>  #endif
>  
> -	mmu_early_init_devtree();
> -
>  #ifdef CONFIG_PPC_POWERNV
>  	/* Scan and build the list of machine check recoverable ranges */
>  	of_scan_flat_dt(early_init_dt_scan_recoverable_ranges, NULL);

cheers
Bharata B Rao Jan. 8, 2018, 9:16 a.m. | #3
On Sat, Jan 06, 2018 at 10:28:00AM +1100, Michael Ellerman wrote:
> Bharata B Rao <bharata@linux.vnet.ibm.com> writes:
> 
> > Currently device tree nodes for memory are scanned before the
> > radix feature is discovered in mmu_early_init_devtree(). Move this
> > routine ahead of scanning memory nodes so that we know if the
> > guest is radix or not when scanning ibm,dynamic-reconfiguration-memory.
> 
> Sorry this doesn't work.
> 
> > diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> > index b15bae2..079d893 100644
> > --- a/arch/powerpc/kernel/prom.c
> > +++ b/arch/powerpc/kernel/prom.c
> > @@ -722,6 +722,8 @@ void __init early_init_devtree(void *params)
> >  	 */
> >  	of_scan_flat_dt(early_init_dt_scan_chosen_ppc, boot_command_line);
> >  
> > +	mmu_early_init_devtree();
> > +
> 
> You've moved this above parse_early_param(), but
> mmu_early_init_devtree() uses disable_radix, which is an early param. So
> this will break disable_radix handling.
> 
> It will probably break other things too because the ordering of this
> init code is very fragile - bootstrapping is hard :)

So from your and Aneesh's reply I realize that my current approach
involving upfront mapping of hotplugged memory with smaller mappings
without afffecting PowerVM will not be feasible. Guess breaking the
mapping appropriately during unplug would be the only real solution then.

Regards,
Bharata.

Patch

diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
index b15bae2..079d893 100644
--- a/arch/powerpc/kernel/prom.c
+++ b/arch/powerpc/kernel/prom.c
@@ -722,6 +722,8 @@  void __init early_init_devtree(void *params)
 	 */
 	of_scan_flat_dt(early_init_dt_scan_chosen_ppc, boot_command_line);
 
+	mmu_early_init_devtree();
+
 	/* Scan memory nodes and rebuild MEMBLOCKs */
 	of_scan_flat_dt(early_init_dt_scan_root, NULL);
 	of_scan_flat_dt(early_init_dt_scan_memory_ppc, NULL);
@@ -783,8 +785,6 @@  void __init early_init_devtree(void *params)
 	spinning_secondaries = boot_cpu_count - 1;
 #endif
 
-	mmu_early_init_devtree();
-
 #ifdef CONFIG_PPC_POWERNV
 	/* Scan and build the list of machine check recoverable ranges */
 	of_scan_flat_dt(early_init_dt_scan_recoverable_ranges, NULL);