[PATCH] ACPI: NUMA: debug invalid unused PXM value for CFMWs

Yuquan Wang posted 1 patch 11 months ago
There is a newer version of this series
drivers/acpi/numa/srat.c | 3 +++
1 file changed, 3 insertions(+)
[PATCH] ACPI: NUMA: debug invalid unused PXM value for CFMWs
Posted by Yuquan Wang 11 months ago
The absence of SRAT would cause the fake_pxm to be -1 and increment
to 0, then send to acpi_parse_cfmws(). If there exists CXL memory
ranges that are defined in the CFMWS and not already defined in the
SRAT, the new node (node0) for the CXL memory would be invalid, as
node0 is already in "used".

Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>
---
 drivers/acpi/numa/srat.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
index 00ac0d7bb8c9..eb8628e217fa 100644
--- a/drivers/acpi/numa/srat.c
+++ b/drivers/acpi/numa/srat.c
@@ -646,6 +646,9 @@ int __init acpi_numa_init(void)
 		if (node_to_pxm_map[i] > fake_pxm)
 			fake_pxm = node_to_pxm_map[i];
 	}
+	if (fake_pxm == PXM_INVAL)
+		pr_warn("Failed to find the next unused PXM value for CFMWs\n");
+
 	last_real_pxm = fake_pxm;
 	fake_pxm++;
 	acpi_table_parse_cedt(ACPI_CEDT_TYPE_CFMWS, acpi_parse_cfmws,
-- 
2.34.1
Re: [PATCH] ACPI: NUMA: debug invalid unused PXM value for CFMWs
Posted by Alison Schofield 11 months ago
On Mon, Mar 10, 2025 at 05:39:10PM +0800, Yuquan Wang wrote:
> The absence of SRAT would cause the fake_pxm to be -1 and increment
> to 0, then send to acpi_parse_cfmws(). If there exists CXL memory
> ranges that are defined in the CFMWS and not already defined in the
> SRAT, the new node (node0) for the CXL memory would be invalid, as
> node0 is already in "used".


If no SRAT or bad SRAT, then all memory is at node:0, and first fake
node for CFMWs should start at 1. Right?

If so, might it be safest to always start the the CFMWS fake nodes at
at a minimum of node[1]. Maybe srat_disabled() can be used to decide.

> 
> Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>
> ---
>  drivers/acpi/numa/srat.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
> index 00ac0d7bb8c9..eb8628e217fa 100644
> --- a/drivers/acpi/numa/srat.c
> +++ b/drivers/acpi/numa/srat.c
> @@ -646,6 +646,9 @@ int __init acpi_numa_init(void)
>  		if (node_to_pxm_map[i] > fake_pxm)
>  			fake_pxm = node_to_pxm_map[i];
>  	}
> +	if (fake_pxm == PXM_INVAL)
> +		pr_warn("Failed to find the next unused PXM value for CFMWs\n");
> +

How come it is sufficient to just warn?
As per my comment above, can we adjust?



>  	last_real_pxm = fake_pxm;
>  	fake_pxm++;
>  	acpi_table_parse_cedt(ACPI_CEDT_TYPE_CFMWS, acpi_parse_cfmws,
> -- 
> 2.34.1
>
Re: [PATCH] ACPI: NUMA: debug invalid unused PXM value for CFMWs
Posted by Yuquan Wang 11 months ago
On Mon, Mar 10, 2025 at 11:13:32AM -0700, Alison Schofield wrote:
> On Mon, Mar 10, 2025 at 05:39:10PM +0800, Yuquan Wang wrote:
> > The absence of SRAT would cause the fake_pxm to be -1 and increment
> > to 0, then send to acpi_parse_cfmws(). If there exists CXL memory
> > ranges that are defined in the CFMWS and not already defined in the
> > SRAT, the new node (node0) for the CXL memory would be invalid, as
> > node0 is already in "used".
> 
> 
> If no SRAT or bad SRAT, then all memory is at node:0, and first fake
> node for CFMWs should start at 1. Right?
Yes.
> 
> If so, might it be safest to always start the the CFMWS fake nodes at
> at a minimum of node[1]. Maybe srat_disabled() can be used to decide.
> 
> > 
> > Signed-off-by: Yuquan Wang <wangyuquan1236@phytium.com.cn>
> > ---
> >  drivers/acpi/numa/srat.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
> > index 00ac0d7bb8c9..eb8628e217fa 100644
> > --- a/drivers/acpi/numa/srat.c
> > +++ b/drivers/acpi/numa/srat.c
> > @@ -646,6 +646,9 @@ int __init acpi_numa_init(void)
> >  		if (node_to_pxm_map[i] > fake_pxm)
> >  			fake_pxm = node_to_pxm_map[i];
> >  	}
> > +	if (fake_pxm == PXM_INVAL)
> > +		pr_warn("Failed to find the next unused PXM value for CFMWs\n");
> > +
> 
> How come it is sufficient to just warn?
> As per my comment above, can we adjust?
>
Sure. Thanks for your suggestion.
> 
> 
> >  	last_real_pxm = fake_pxm;
> >  	fake_pxm++;
> >  	acpi_table_parse_cedt(ACPI_CEDT_TYPE_CFMWS, acpi_parse_cfmws,
> > -- 
> > 2.34.1
> >