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

[Huang, Kai]

On Wed, 2026-03-04 at 17:29 -0800, dan.j.williams@xxxxxxxxx wrote:
> Huang, Kai wrote:
> > On Wed, 2026-03-04 at 19:29 -0500, Gregory Price wrote:
> > > On Thu, Mar 05, 2026 at 12:14:52AM +0000, Huang, Kai wrote:
> > > > On Wed, 2026-03-04 at 18:56 -0500, Gregory Price wrote:
> > > > > 
> > > > > This basically says if specifically CXL_ACPI is built out, the NUMA
> > > > > structure is forever lost - even though it's accurately described by
> > > > > BIOS.  
> > > > > 
> > > > 
> > > > The normal NUMA info described in SRAT is still there.  It only avoids
> > > > detecting CFMWS, which doesn't provide any NUMA info actually -- that's why
> > > > kernel assigns a 'faked' NUMA node for each of them.
> > > > 
> > > > So we are not losing anything AFAICT.
> > > 
> > > Well, I'm mostly confused why there are CEDT entries for hardware that
> > > presumably isn't even there - unless this platform is reserving space
> > > for future hotplug.  
> > > 
> > 
> > I think this should be the case.
> > 
> > > Just want to make sure we're not adjusting for
> > > strange firmware behavior.
> > 
> > How to check whether it is "strange"?
> > 
> 
> So these are fine. These are CXL hotplug windows and the expectation is
> that they *might* be populated in the future. SRAT can not make claims
> about future CXL hotplug (see NOTE). So the expecation for CXL hotplug
> is reserve some numa nodes that Linux can determine the affinity of
> dynamically with HMAT Generic Port and device CDAT information.
> 
> NOTE: SRAT *does* make claims about the affinity of future *ACPI*
> Hotplug, but in that case the platform statically knows something about
> the configuration of memory that can possibly be plugged in the future.

Thanks for the info!

> 
> > > You are taking something away by nature of compiling something out by
> > > default that was previously not compiled out by default.
> > 
> > Yeah, and it is due to "there's a cost" if we don't compile out by default.
> > 
> > Hope that justifies?
> 
> I think it makes sense that if you disable CXL hotplug by setting
> CONFIG_CXL_ACPI=n then no need to reserve numa ids. However, just do
> something like this rather than add ifdefs to the code:
> 
> diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
> index aa87ee1583a4..62d4a8df0b8c 100644
> --- a/drivers/acpi/numa/srat.c
> +++ b/drivers/acpi/numa/srat.c
> @@ -654,8 +654,11 @@ int __init acpi_numa_init(void)
>  	}
>  	last_real_pxm = fake_pxm;
>  	fake_pxm++;
> -	acpi_table_parse_cedt(ACPI_CEDT_TYPE_CFMWS, acpi_parse_cfmws,
> -			      &fake_pxm);
> +
> +	/* No need to expand numa nodes if CXL is disabled */
> +	if (IS_ENABLED(CONFIG_CXL_ACPI))
> +		acpi_table_parse_cedt(ACPI_CEDT_TYPE_CFMWS, acpi_parse_cfmws,
> +				      &fake_pxm);
>  
>  	if (cnt < 0)
>  		return cnt;
> 
> The call to acpi_table_parse_cedt() will get skipped and the code for
> acpi_parse_cfmws() will get automatically compiled out of the file.

Yeah agreed this is simpler.  Thanks for the suggestion.

I would like to honor Gregory's suggestion about the comment since it
provides more info if you agree?

(I also added "impacts certain NUMA userspace ABIs")

/*
 * Some platforms report CEDT CFMWS for hotplug device support. These
 * windows are unusable without CXL drivers, so don't reserve fake nodes
 * if they're compiled out - it wastes memory on per-node structures and
 * impacts certain NUMA userspace ABIs.
 */

> 
> At the same time I doubt this patch provides end users much value in
> practice as most distro kernels have CONFIG_CXL_ACPI, and the few end
> users that have CXL are not going blink at the overhead to support the
> full feature set.
> 
> Can you not just disable CXL support in the BIOS for your system and
> avoid complicating this code path for a small win?

Sure I can (I need to figure out how, though).  But I think it's still
useful for developers because some of them will tend to only enable Kconfig
options that they need to save build time.

And sometimes it's not quite easy to turn off CXL in the BIOS since the
machine would be mostly remote.

So I think this patch is still worthy to do?