Re: [PATCH] cxl/region: Create node access attributes for CFMWS-only NUMA nodes

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

On Wed, Sep 16, 2026 at 08:03:38PM +0800, Guixin Liu wrote:
> A NUMA node that only backs a CXL fixed memory window and is not
> enumerated by SRAT has no memory_target in the HMAT code. When the
> first memory of such a node comes online, hmat_callback() exits early
> at find_mem_target() without creating anything, and
> node_update_perf_attrs() only updates attributes that already exist,
> so the node never gets its accessN sysfs attributes at all:
> /sys/devices/system/node/nodeX/accessN/initiators/* stays missing even
> though the region's own sysfs reports the coordinates.
> 
> Commit debdce20c4f2 ("cxl/region: Deal with numa nodes not enumerated
> by SRAT") handled this by having the CXL region notifier call
> node_set_perf_attrs() directly for such nodes, but
> 
>   commit 2e454fb8056d ("cxl, acpi/hmat: Update CXL access coordinates
>   directly instead of through HMAT")
> 
> replaced that call with node_update_perf_attrs() on the assumption
> that the HMAT callback has already created the attributes. That
> assumption does not hold for CFMWS-only nodes.
> 
> Restore the distinction: when the node is not backed by a real SRAT
> pxm, create the attributes with node_set_perf_attrs() instead of
> only trying to update them.
> 
> Tested on a QEMU CXL topology with a CFMWS window whose memory is not
> described by SRAT or HMAT: after the region's first memory block comes
> online, node1 has no accessN directory before this patch; with it,
> node1/access0/initiators/{read,write}_{bandwidth,latency} appear with
> the calculated coordinates.


The detailed analysis establishes that this is a regression in the existing
CFMWS-only NUMA node support, but I think that gets a little lost in the
forensics.

Could you make that more explicit up front? In particular, the user-visible
impact is that a NUMA node which the kernel already supports creating from
CFMWS no longer gets its accessN attributes.

Gregory's point about whether QEMU should also provide SRAT is a
separate issue, like he notes. The kernel already supports the
CFMWS-only path, so I think we should continue to handle it correctly.

-- Alison

> 
> Fixes: 2e454fb8056d ("cxl, acpi/hmat: Update CXL access coordinates directly instead of through HMAT")
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> ---
> checkpatch reports one "Prefer a maximum 75 chars per line" warning:
> the 2e454fb8056d reference line above is 84 columns because checkpatch
> requires the quoted title to match the git subject exactly, and that
> subject is 62 characters long. Wrapping it instead produces a
> GIT_COMMIT_ID error, so the long line is the lesser evil.
>  drivers/cxl/core/cdat.c   | 5 +++++
>  drivers/cxl/core/core.h   | 1 +
>  drivers/cxl/core/region.c | 5 ++++-
>  3 files changed, 10 insertions(+), 1 deletion(-)
> ---
>  drivers/cxl/core/cdat.c   | 5 +++++
>  drivers/cxl/core/core.h   | 1 +
>  drivers/cxl/core/region.c | 5 ++++-
>  3 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index 5c9f07262513..e28c40159d94 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -1072,3 +1072,8 @@ void cxl_region_perf_data_calculate(struct cxl_region *cxlr,
>  		cxlr->coord[i].write_bandwidth += perf->coord[i].write_bandwidth;
>  	}
>  }
> +
> +bool cxl_need_node_perf_attrs_update(int nid)
> +{
> +	return !acpi_node_backed_by_real_pxm(nid);
> +}
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 35eaf636adc9..19a5b8edbfda 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -229,4 +229,5 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
>  
>  resource_size_t cxl_rcd_component_reg_phys(struct device *dev,
>  					   struct cxl_dport *dport);
> +bool cxl_need_node_perf_attrs_update(int nid);
>  #endif /* __CXL_CORE_H__ */
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 27e63e6dab7c..636435733754 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2630,7 +2630,10 @@ static bool cxl_region_update_coordinates(struct cxl_region *cxlr, int nid)
>  
>  	for (int i = 0; i < ACCESS_COORDINATE_MAX; i++) {
>  		if (cxlr->coord[i].read_bandwidth) {
> -			node_update_perf_attrs(nid, &cxlr->coord[i], i);
> +			if (cxl_need_node_perf_attrs_update(nid))
> +				node_set_perf_attrs(nid, &cxlr->coord[i], i);
> +			else
> +				node_update_perf_attrs(nid, &cxlr->coord[i], i);
>  			cset++;
>  		}
>  	}
> -- 
> 2.43.7
>