Re: [PATCH] ACPI: NUMA: Only parse CFMWS at boot when CXL_ACPI is on

[Alison Schofield <alison.schofield@intel.com>]

On Wed, Mar 04, 2026 at 04:04:57PM -0800, Huang, Kai wrote:
> On Wed, 2026-03-04 at 15:20 -0800, Alison Schofield wrote:
> > On Wed, Mar 04, 2026 at 05:33:26PM -0500, Gregory Price wrote:
> > > On Thu, Mar 05, 2026 at 10:33:42AM +1300, Kai Huang wrote:
> > > > Increasing the 'nr_node_ids' has side effects.  For instance, it is
> > > > widely used by the kernel for "highest possible NUMA node" based memory
> > > > allocations.  It also impacts userspace ABIs, e.g., some NUMA memory
> > > > related system calls such as 'get_mempolicy' which requires 'maxnode'
> > > > not being smaller than the 'nr_node_ids'.
> > > > 
> > 
> > > 
> > > Is this a Linux issue or a Firmware issue?
> 
> There are two issues actually: 
> 
> 1) on my testing platform, we can see CFMWS tables but there's no actual CXL
> memory inserted AFAICT; 
> 
> 2) The kernel unconditionally bumps the 'nr_node_ids' even CXL_ACPI driver
> is not able to detect any real CXL memory (or it is CXL port/mem driver
> cannot detect, for which I am not very familiar with).
> 
> We can argue the 1) is FW issue.  But this patch aims to improve the 2),
> which is an improvement regardless the firmware has issue or not:
> 
> If CXL_ACPI isn't even enabled in Kconfig, there's no need to detect CFMWS
> tables and assign a 'faked NUMA node' during boot.
> 
> IIUC this is even true on a perfect fine machine with actual CXL memory
> installed, since the kernel still have an option to disable CXL_ACPI in
> Kconfig.
> 
> > 
> > IIUC BIOS creates the CEDT based on the hardware it 'sees' as present.
> > 
> > This patch is describing the case (weird as it seems to me) where we
> > then boot a system with ACPI and NUMA enabled but CXL_ACPI disabled.
> 
> When I found this issue the CXL_ACPI is enabled in Kconfig, but there's no
> actual CXL memory being discovered AFAICT.

So this patch will not fix that case, because CXL_ACPI will be enabled.

I'd like to take a look at that system Kai. I'll ping you off list.

> 
>  #cxl list
> 
> tells me nothing.
> 
> > 
> > So, I don't think we can blame BIOS.
> 
> As said above, I think it's an improvement in the kernel even on a good
> machine with CXL memory inserted.
> 
> > 
> > > 
> > > Is GNR exporting more CFMWS than it should?
> > Not sure of any limits on flavors of CFMWS's a BIOS can offer.
> > If BIOS can carve out a window, it can create a CFMWS.
> > Not clear how that matters to the issue.
> > 
> > > 
> > > Is your SRAT missing entries for CFMWS that are otherwise present?
> > > 
> > > Are the CFMWS empty? (is that even valid)
> > 
> > Why this line of questioning ;)  I see the problem as a bit simpler.
> > We have other code that tells us if the CFMWS's are valid, etc, but
> > the point here is, we are not going to use these CFMWS's so stop
> > the parse as early as possible, like right here as Kai has done.
> 
> Yes. :-)
> 
> > 
> > > 
> > > > E.g., on the aforementioned GNR platform, the "Slab" in /proc/meminfo is
> > > > reduced with this change (when CXL_ACPI is off):
> > > > 
> > > > 	w/ this change		w/o
> > > > 
> > > > Slab	900488 kB		923660 kB
> > > > 
> > > 
> > > This is a good effect, but I still question the premise.
> > > 
> > > We don't usually want #ifdef's inside of .c files if we can avoid it.
> > 
> > I thought similar, but for early init and static helpers, this 
> > #if IS_ENABLE(..) wrapper is common.
> 
> Yes.  They are early init and static helpers, so no header file is involved
> here.
> 
> I also see #if defined(CONFIG_X86) || defined(CONFIG_ARM64) is used for
> acpi_parse_gi_affinity() in this file.
> 
> > 
> > 
> > Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> 
> Thanks!