Received-SPF: pass (google.com: domain of linux-kernel+bounces-117464-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Mon, 25 Mar 2024 09:24:21 -0700
From: Saurabh Singh Sengar <ssengar@linux.microsoft.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: dave.hansen@linux.intel.com, luto@kernel.org, peterz@infradead.org,
	mingo@redhat.com, bp@alien8.de, x86@kernel.org, hpa@zytor.com,
	linux-kernel@vger.kernel.org, ssengar@microsoft.com,
	sgeorgejohn@microsoft.com
Subject: Re: [PATCH] x86/numa: Map NUMA node to CPUs as per DeviceTree
Message-ID: <20240325162421.GA2326@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
References: <1710265404-23146-1-git-send-email-ssengar@linux.microsoft.com>
 <87v85bfzg0.ffs@tglx>
 <20240324162833.GA18417@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>
 <87jzlqfk4n.ffs@tglx>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87jzlqfk4n.ffs@tglx>
User-Agent: Mutt/1.5.21 (2010-09-15)

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