Re: [PATCH] x86/numa: Map NUMA node to CPUs as per DeviceTree

[Saurabh Singh Sengar]

On Mon, Mar 25, 2024 at 03:42:32PM +0100, Thomas Gleixner wrote:
> On Sun, Mar 24 2024 at 09:28, Saurabh Singh Sengar wrote:
> > I recognize that due to recent changes, each dtb platform will now need to set
> > a pointer for x86_init.mpparse.early_parse_smp_cfg to get the dtb_cpu_setup
> > executed.
> 
> No. DT does not need the early parse call. The early parse call _cannot_
> enumerate APICs.
> 
> > This was not the requirement before because earlier x86_dtb_init was
> > anyway getting called.
> 
> For the wrong reasons.
> 
> > Do you think we should improve this as well by setting
> > x86_init.mpparse.early_parse_smp_cfg to x86_dtb_parse_smp_config for all the
> > dtb platforms by default.
> 
> No.
> 
> > I see the ce4100 platform is setting the parse_smp_cfg, shouldn't the
> > early_parse_smp_cfg be more accurate there ?
> 
> Again. No. Early is not the point where APICs can be enumerated.
> 
> What we can do is the below.
> 
> Thanks,
> 
>         tglx
> ---
> --- a/arch/x86/include/asm/prom.h
> +++ b/arch/x86/include/asm/prom.h
> @@ -24,18 +24,15 @@ extern u64 initial_dtb;
>  extern void add_dtb(u64 data);
>  void x86_of_pci_init(void);
>  void x86_dtb_parse_smp_config(void);
> +void x86_flattree_get_config(void);
>  #else
>  static inline void add_dtb(u64 data) { }
>  static inline void x86_of_pci_init(void) { }
>  static inline void x86_dtb_parse_smp_config(void) { }
> +static inline void x86_flattree_get_config(void) { }
>  #define of_ioapic 0
>  #endif
>  
> -#ifdef CONFIG_OF_EARLY_FLATTREE
> -void x86_flattree_get_config(void);
> -#else
> -static inline void x86_flattree_get_config(void) { }
> -#endif
>  extern char cmd_line[COMMAND_LINE_SIZE];
>  
>  #endif /* __ASSEMBLY__ */
> --- a/arch/x86/kernel/devicetree.c
> +++ b/arch/x86/kernel/devicetree.c
> @@ -277,9 +277,9 @@ static void __init dtb_apic_setup(void)
>  	dtb_ioapic_setup();
>  }
>  
> -#ifdef CONFIG_OF_EARLY_FLATTREE
>  void __init x86_flattree_get_config(void)
>  {
> +#ifdef CONFIG_OF_EARLY_FLATTREE
>  	u32 size, map_len;
>  	void *dt;
>  
> @@ -301,8 +301,10 @@ void __init x86_flattree_get_config(void
>  
>  	if (initial_dtb)
>  		early_memunmap(dt, map_len);
> -}
>  #endif
> +	if (of_have_populated_dt())
> +		x86_init.mpparse.parse_smp_cfg = x86_dtb_parse_smp_config;
> +}
>  
>  void __init x86_dtb_parse_smp_config(void)
>  {

Thanks for suggestion, this will fix for all the x86 dtb platforms at once.
I wonder if there will be any user of x86_dtb_parse_smp_config outside
of this file after this change. Possibly we can make it static ?

I can send the v2, however if you prefer pushing all these changes yourself
please let me know.

- Saurabh