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

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

[Guixin Liu]

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.

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] cxl: backwards naming of cxl_need_node_perf_attrs_update()
--

commit b11d76869ff914a767abdc38146d89e156a8de78
Author: Guixin Liu <kanie@linux.alibaba.com>

cxl/region: Create node access attributes for CFMWS-only NUMA nodes

This patch handles a case where a NUMA node only backs a CXL fixed memory
window without being enumerated by SRAT. When such a node is not backed by
a real SRAT PXM, it creates the node access attributes using
node_set_perf_attrs() rather than just trying to update them.

> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index df9261ba84e5a..1ac7ce3a6127e 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);
> +}

[Severity: Low]
Does this function name accurately describe what it checks for? It seems to
return true when the node is NOT backed by a real SRAT PXM, which means it
needs creation rather than an update.

[ ... ]

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 4f6069451fed6..11902e7f6a1c6 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c

[ ... ]

> @@ -2631,7 +2631,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);

[Severity: Low]
Is the logic backwards here because of the helper name? 

If cxl_need_node_perf_attrs_update() returns true, this calls
node_set_perf_attrs() to create the attributes. If it returns false, it
calls node_update_perf_attrs() to update them.

This effectively reads as "if we need to update, we set; otherwise we
update". Would renaming the helper to something like
cxl_need_node_perf_attrs_creation() make this logic clearer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916120338.369436-1-kanie@linux.alibaba.com?part=1

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

[Dave Jiang]

On 9/16/26 5:03 AM, 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.
> 
> 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))

Given that it's only used once and it's straight forward enough, just open code it instead of creating a helper.

DJ

> +				node_set_perf_attrs(nid, &cxlr->coord[i], i);
> +			else
> +				node_update_perf_attrs(nid, &cxlr->coord[i], i);
>  			cset++;
>  		}
>  	}

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

[Gregory Price]

On Wed, Sep 16, 2026 at 08:03:38PM +0800, Guixin Liu wrote:
> Tested on a QEMU CXL topology with a CFMWS window whose memory is not
> described by SRAT or HMAT

Sigh.  This keeps coming up.  (Not your issue Guixin Liu)

It would be nice to actually fix this in QEMU rather than make QEMU
specific fixes in linux.

On any other platform we would call this a firmware bug.

~Gregory

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

[Jonathan Cameron]

On Wed, 16 Sep 2026 20:03:38 +0800
Guixin Liu <kanie@linux.alibaba.com> 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.
> 
> Fixes: 2e454fb8056d ("cxl, acpi/hmat: Update CXL access coordinates directly instead of through HMAT")
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>

Ah.  I should have caught this one.  Indeed looks correct.

My only concern is it is relying heavily on the fact we only do this
once which is not apparent at this layer in the code.  You need to go
looking in cxl_region_perf_attrs_callback() for that.

It is also a one way gate so even you tear down all regions in the NUMA
node (so the CFWMS) we don't do this update again.

Anyhow, I think all we need here is a note that this function will only
be called once per CFWMS and thus it is fine to register the sysfs attrs
here.

With that added, this looks good to me
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.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++;
>  		}
>  	}

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

[Alison Schofield]

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
> 

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

[Guixin Liu]

在 2026/9/17 01:49, Dave Jiang 写道:
>
> On 9/16/26 5:03 AM, 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.
>>
>> 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))
> Given that it's only used once and it's straight forward enough, just open code it instead of creating a helper.
>
> DJ
OK,  removed in v2, thanks.

Best Regards,
Guixin Liu
>
>> +				node_set_perf_attrs(nid, &cxlr->coord[i], i);
>> +			else
>> +				node_update_perf_attrs(nid, &cxlr->coord[i], i);
>>   			cset++;
>>   		}
>>   	}

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

[Guixin Liu]

在 2026/9/17 02:00, Jonathan Cameron 写道:
> On Wed, 16 Sep 2026 20:03:38 +0800
> Guixin Liu <kanie@linux.alibaba.com> 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.
>>
>> Fixes: 2e454fb8056d ("cxl, acpi/hmat: Update CXL access coordinates directly instead of through HMAT")
>> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> Ah.  I should have caught this one.  Indeed looks correct.
>
> My only concern is it is relying heavily on the fact we only do this
> once which is not apparent at this layer in the code.  You need to go
> looking in cxl_region_perf_attrs_callback() for that.
>
> It is also a one way gate so even you tear down all regions in the NUMA
> node (so the CFWMS) we don't do this update again.
>
> Anyhow, I think all we need here is a note that this function will only
> be called once per CFWMS and thus it is fine to register the sysfs attrs
> here.
OK, added in v2, thanks.

Best Regards,
Guixin Liu
>
> With that added, this looks good to me
> Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.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++;
>>   		}
>>   	}

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

[Guixin Liu]

在 2026/9/17 04:56, Alison Schofield 写道:
> 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.
Yes, it's better, will be changed in v2, thanks.

Best Regards,
Guixin Liu
>
> 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
>>

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

[Jonathan Cameron]

On Wed, 16 Sep 2026 13:59:48 -0400
Gregory Price <gourry@gourry.net> wrote:

> On Wed, Sep 16, 2026 at 08:03:38PM +0800, Guixin Liu wrote:
> > Tested on a QEMU CXL topology with a CFMWS window whose memory is not
> > described by SRAT or HMAT  
> 
> Sigh.  This keeps coming up.  (Not your issue Guixin Liu)

I won't reply in depth again.  If a platform supports non trivial hotplug
(i.e. more than one device spec) then the bios has to be clairvoyant
to create those tables in a fashion that is useful. Generic Ports were
added to put the missing piece in to solve the problem so there
is no longer a need to pre bake. 

> 
> It would be nice to actually fix this in QEMU rather than make QEMU
> specific fixes in linux.
> 
> On any other platform we would call this a firmware bug.

You are going to see more 'buggy' platforms as the ones that don't
have to comply with the ancient history aren't going to add these fake
SRAT entrees.

Personally I'd take the more radical view that if a platform gives
us Generic Ports, right answer is throw away any fictional / approximate
SRAT entrees, do the discovery ourselves and build representative
NUMA nodes for what is actually there.

This is going to be more fun when we have more complex devices showing
up and need to create a NUMA node per device (i.e. those nice compression
devices of yours). At that point you aren't using SRAT memory proximity
nodes anyway - unless they are all there at boot and you are spinning a PXM
for each one.  How do you do if a mix of different sized (or at least
sizes you didn't know at boot) devices get hotplugged later?

Jonathan

> 
> ~Gregory

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

[Gregory Price]

On Thu, Sep 17, 2026 at 10:04:32PM +0100, Jonathan Cameron wrote:
> On Wed, 16 Sep 2026 13:59:48 -0400
> Gregory Price <gourry@gourry.net> wrote:
> 
> > On Wed, Sep 16, 2026 at 08:03:38PM +0800, Guixin Liu wrote:
> > > Tested on a QEMU CXL topology with a CFMWS window whose memory is not
> > > described by SRAT or HMAT  
> > 
> > Sigh.  This keeps coming up.  (Not your issue Guixin Liu)
> 
> I won't reply in depth again.  If a platform supports non trivial hotplug
> (i.e. more than one device spec) then the bios has to be clairvoyant
> to create those tables in a fashion that is useful. Generic Ports were
> added to put the missing piece in to solve the problem so there
> is no longer a need to pre bake. 
>

Well sure, but [SRAT, Generic Ports] - pick one.

But giving nothing is just a wart :|

> This is going to be more fun when we have more complex devices showing
> up and need to create a NUMA node per device (i.e. those nice compression
> devices of yours). At that point you aren't using SRAT memory proximity
> nodes anyway - unless they are all there at boot and you are spinning a PXM
> for each one.  How do you do if a mix of different sized (or at least
> sizes you didn't know at boot) devices get hotplugged later?

cough

https://lore.kernel.org/linux-mm/20260610014517.253609-1-gourry@gourry.net/