Skip to contentSkip to conversation

cxl/test: mock fixups and new topology support

PATCH Superseded Alison Schofield 2 participants 7 messages Sep 29 – Oct 9 CXL

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Alison Schofield

author maintainer
#0 lore raw

cover cxl/test: mock fixups and new topology support

While working to enhance the cxl-cli cxl-create-region command to handle out-of-order target lists or no user-supplied target list, I ran into some basic issues in the existing command that need to be addressed first. Developing tests for those fixes exposed two bugs and a topology gap in the cxl_test mock module.

This cxl_test series addresses those issues:

1/3 fixes a DPA skip leak across mock decoder reset.
2/3 skips auto-region setup when no volatile window is active. 3/3 adds two x1 CFMWS windows needed to exercise root decoder selection.

A companion ndctl series will fix the existing cxl-create-region issues in cxl-cli, clearing the way for out-of-order or automatically generated target lists.

These changes preserve the existing cxl_test unit test behavior while enabling new create-region test cases.

Alison Schofield (3):
tools/testing/cxl: Preserve DPA skip across mock decoder reset tools/testing/cxl: Skip the auto region when no volatile window is active tools/testing/cxl: Add CFMWS windows for region selection tests

 tools/testing/cxl/test/cxl.c | 100 +++++++++++++++++++++++++++--------
 1 file changed, 77 insertions(+), 23 deletions(-)

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935

Signature · 1 line
2.37.3
author maintainer
#1 lore raw

PATCH 1/3 tools/testing/cxl: Preserve DPA skip across mock decoder reset

Mock decoder reset clears the DPA skip before releasing the decoder's resources. This leaks skipped capacity and can cause a later DPA allocation to fail with ENOSPC.

Preserve the skip until the DPA resource is released.

Assisted-by LLM
---
 tools/testing/cxl/test/cxl.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
tools/testing/cxl/test/cxl.c +3 -6
index 62bd92b3be45..aa140307e0c3 100644
@@ -913,15 +913,13 @@ static void __cxld_registry_save(struct cxl_test_decoder *td,
 
 		cxld_copy(&td->cxled.cxld, cxld);
 		td->cxled.state = cxled->state;
-		td->cxled.skip = cxled->skip;
 
-		if (!(cxld->flags & CXL_DECODER_F_ENABLE)) {
-			td->dpa_range.start = 0;
-			td->dpa_range.end = -1;
-		} else if (cxled->dpa_res) {
+		if ((cxld->flags & CXL_DECODER_F_ENABLE) && cxled->dpa_res) {
+			td->cxled.skip = cxled->skip;
 			td->dpa_range.start = cxled->dpa_res->start;
 			td->dpa_range.end = cxled->dpa_res->end;
 		} else {
+			td->cxled.skip = 0;
 			td->dpa_range.start = 0;
 			td->dpa_range.end = -1;
 		}
@@ -1001,7 +999,6 @@ static void mock_decoder_reset(struct cxl_decoder *cxld)
 			to_cxl_endpoint_decoder(&cxld->dev);
 
 		cxled->state = CXL_DECODER_STATE_MANUAL;
-		cxled->skip = 0;
 	}
 	if (decoder_reset_preserve_registry)
 		dev_dbg(port->uport_dev, "decoder%d: skip registry update\n",
Signature · 1 line
2.37.3
Reviewed-by on v1 1/3
#2 lore raw

Re: [PATCH 1/3] tools/testing/cxl: Preserve DPA skip across mock decoder reset

Quoted 46 lines · Alison Schofield

Mock decoder reset clears the DPA skip before releasing the decoder's resources. This leaks skipped capacity and can cause a later DPA allocation to fail with ENOSPC.

Preserve the skip until the DPA resource is released.

Assisted-by LLM
---
 tools/testing/cxl/test/cxl.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
tools/testing/cxl/test/cxl.c +3 -6
index 62bd92b3be45..aa140307e0c3 100644
@@ -913,15 +913,13 @@ static void __cxld_registry_save(struct cxl_test_decoder *td,
 
 		cxld_copy(&td->cxled.cxld, cxld);
 		td->cxled.state = cxled->state;
-		td->cxled.skip = cxled->skip;
 
-		if (!(cxld->flags & CXL_DECODER_F_ENABLE)) {
-			td->dpa_range.start = 0;
-			td->dpa_range.end = -1;
-		} else if (cxled->dpa_res) {
+		if ((cxld->flags & CXL_DECODER_F_ENABLE) && cxled->dpa_res) {
+			td->cxled.skip = cxled->skip;
 			td->dpa_range.start = cxled->dpa_res->start;
 			td->dpa_range.end = cxled->dpa_res->end;
 		} else {
+			td->cxled.skip = 0;
 			td->dpa_range.start = 0;
 			td->dpa_range.end = -1;
 		}
@@ -1001,7 +999,6 @@ static void mock_decoder_reset(struct cxl_decoder *cxld)
 			to_cxl_endpoint_decoder(&cxld->dev);
 
 		cxled->state = CXL_DECODER_STATE_MANUAL;
-		cxled->skip = 0;
 	}
 	if (decoder_reset_preserve_registry)
 		dev_dbg(port->uport_dev, "decoder%d: skip registry update\n",
Signature · 1 line
2.37.3

LGTM.

Best regards,
Richard Cheng.

author maintainer
#3 lore raw

PATCH 2/3 tools/testing/cxl: Skip the auto region when no volatile window is active

In reply to Alison Schofield

The mock auto region always uses CFMWS0. With XOR interleave arithmetic that window is inactive, so its HPA base is never assigned and the endpoint decoders are programmed at HPA 0.

No root decoder matches that range, leaving DPA allocated to endpoint decoders that are not part of a region.

Set up the auto region only when a volatile window is active.

Assisted-by LLM
---
 tools/testing/cxl/test/cxl.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)
tools/testing/cxl/test/cxl.c +18 -2
index aa140307e0c3..9d844932f0c6 100644
@@ -423,6 +423,18 @@ struct acpi_cedt_cxims *mock_cxims[1] = {
 	[0] = &mock_cedt.cxims0.cxims,
 };
 
+static struct acpi_cedt_cfmws *mock_auto_region_cfmws(void)
+{
+	for (int i = cfmws_start; i <= cfmws_end; i++) {
+		struct acpi_cedt_cfmws *window = mock_cfmws[i];
+
+		if (window->restrictions & ACPI_CEDT_CFMWS_RESTRICT_VOLATILE)
+			return window;
+	}
+
+	return NULL;
+}
+
 struct cxl_mock_res {
 	struct list_head list;
 	struct range range;
@@ -1114,6 +1126,10 @@ static enum cxld_init_type get_decoder_init_type(struct cxl_decoder *cxld,
 	    pdev->id > 4 || cxld->id > 0)
 		return MOCK_DECODER_INIT_DEFAULT;
 
+	/* XOR windows are all pmem, no window to host the RAM region */
+	if (!mock_auto_region_cfmws())
+		return MOCK_DECODER_INIT_DEFAULT;
+
 	return type2_test ? MOCK_DECODER_INIT_TYPE2_AUTO :
 			    MOCK_DECODER_INIT_TYPE3_AUTO;
 }
@@ -1137,7 +1153,7 @@ static bool mock_decoder_handle_saved(struct cxl_decoder *cxld, struct cxl_test_
 static void mock_init_hdm_type2_cxled(struct cxl_endpoint_decoder *cxled,
 				      struct cxl_port *port)
 {
-	struct acpi_cedt_cfmws *window = mock_cfmws[0];
+	struct acpi_cedt_cfmws *window = mock_auto_region_cfmws();
 	struct cxl_decoder *cxld = &cxled->cxld;
 	struct cxl_switch_decoder *cxlsd;
 	struct cxl_dport *dport;
@@ -1211,7 +1227,7 @@ static void mock_init_hdm_type3_cxled(struct cxl_endpoint_decoder *cxled,
 				      struct platform_device *pdev,
 				      bool hb0)
 {
-	struct acpi_cedt_cfmws *window = mock_cfmws[0];
+	struct acpi_cedt_cfmws *window = mock_auto_region_cfmws();
 	struct cxl_decoder *cxld = &cxled->cxld;
 	struct cxl_switch_decoder *cxlsd;
 	struct cxl_dport *dport;
Signature · 1 line
2.37.3
#4 lore raw

Re: [PATCH 2/3] tools/testing/cxl: Skip the auto region when no volatile window is active

Quoted 70 lines · Alison Schofield

The mock auto region always uses CFMWS0. With XOR interleave arithmetic that window is inactive, so its HPA base is never assigned and the endpoint decoders are programmed at HPA 0.

No root decoder matches that range, leaving DPA allocated to endpoint decoders that are not part of a region.

Set up the auto region only when a volatile window is active.

Assisted-by LLM
---
 tools/testing/cxl/test/cxl.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)
tools/testing/cxl/test/cxl.c +18 -2
index aa140307e0c3..9d844932f0c6 100644
@@ -423,6 +423,18 @@ struct acpi_cedt_cxims *mock_cxims[1] = {
 	[0] = &mock_cedt.cxims0.cxims,
 };
 
+static struct acpi_cedt_cfmws *mock_auto_region_cfmws(void)
+{
+	for (int i = cfmws_start; i <= cfmws_end; i++) {
+		struct acpi_cedt_cfmws *window = mock_cfmws[i];
+
+		if (window->restrictions & ACPI_CEDT_CFMWS_RESTRICT_VOLATILE)
+			return window;
+	}
+
+	return NULL;
+}
+
 struct cxl_mock_res {
 	struct list_head list;
 	struct range range;
@@ -1114,6 +1126,10 @@ static enum cxld_init_type get_decoder_init_type(struct cxl_decoder *cxld,
 	    pdev->id > 4 || cxld->id > 0)
 		return MOCK_DECODER_INIT_DEFAULT;
 
+	/* XOR windows are all pmem, no window to host the RAM region */
+	if (!mock_auto_region_cfmws())
+		return MOCK_DECODER_INIT_DEFAULT;
+
 	return type2_test ? MOCK_DECODER_INIT_TYPE2_AUTO :
 			    MOCK_DECODER_INIT_TYPE3_AUTO;
 }
@@ -1137,7 +1153,7 @@ static bool mock_decoder_handle_saved(struct cxl_decoder *cxld, struct cxl_test_
 static void mock_init_hdm_type2_cxled(struct cxl_endpoint_decoder *cxled,
 				      struct cxl_port *port)
 {
-	struct acpi_cedt_cfmws *window = mock_cfmws[0];
+	struct acpi_cedt_cfmws *window = mock_auto_region_cfmws();
 	struct cxl_decoder *cxld = &cxled->cxld;
 	struct cxl_switch_decoder *cxlsd;
 	struct cxl_dport *dport;
@@ -1211,7 +1227,7 @@ static void mock_init_hdm_type3_cxled(struct cxl_endpoint_decoder *cxled,
 				      struct platform_device *pdev,
 				      bool hb0)
 {
-	struct acpi_cedt_cfmws *window = mock_cfmws[0];
+	struct acpi_cedt_cfmws *window = mock_auto_region_cfmws();
 	struct cxl_decoder *cxld = &cxled->cxld;
 	struct cxl_switch_decoder *cxlsd;
 	struct cxl_dport *dport;
Signature · 1 line
2.37.3

Hi Alison,

The caller of get_decoder_init_type() and mock_init_hdm_type2/3_cxled() is mock_init_hdm_decoder(), would it be possible to just call mock_auto_region_cfmws() once and pass the window into each function ?

That way we can save some redundant traversal of entries. Do you think this idea makes sense ?

Best regards,
Richard Cheng.

author maintainer
#5 lore raw

Re: [PATCH 2/3] tools/testing/cxl: Skip the auto region when no volatile window is active

Quoted 15 lines · Richard Cheng
Quoted 14 lines · Alison Schofield

The mock auto region always uses CFMWS0. With XOR interleave arithmetic that window is inactive, so its HPA base is never assigned and the endpoint decoders are programmed at HPA 0.

No root decoder matches that range, leaving DPA allocated to endpoint decoders that are not part of a region.

Set up the auto region only when a volatile window is active.

Assisted-by LLM
---
 tools/testing/cxl/test/cxl.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

snip

Quoted 9 lines

Hi Alison,

The caller of get_decoder_init_type() and mock_init_hdm_type2/3_cxled() is mock_init_hdm_decoder(), would it be possible to just call mock_auto_region_cfmws() once and pass the window into each function ?

That way we can save some redundant traversal of entries. Do you think this idea makes sense ?

Yes, tried it out and updated v2 like so -
get_decoder_init_type() now passes back the window it already looked up, and the type2/type3 helpers take it as a parameter. The lookup stays after the auto-region filter, so it runs only for the two auto-region decoders. That also removes the unchecked second lookup in the helpers.

Please take a look at v2.

Thanks!

Quoted 3 lines

Best regards,
Richard Cheng.

author maintainer
#6 lore raw

PATCH 3/3 tools/testing/cxl: Add CFMWS windows for region selection tests

In reply to Alison Schofield

Region selection needs topologies where multiple root decoders can satisfy a request and where a decoder supports both RAM and PMEM. The mock topology provides neither.

Add two x1 windows, one PMEM only and one supporting both RAM and PMEM, while preserving the existing modulo decoder identities.

Assisted-by LLM
---
 tools/testing/cxl/test/cxl.c | 71 ++++++++++++++++++++++++++++--------
 1 file changed, 56 insertions(+), 15 deletions(-)
tools/testing/cxl/test/cxl.c +56 -15
index 9d844932f0c6..3e76d9bb2d5f 100644
@@ -184,12 +184,20 @@ static struct {
 	} cfmws6;
 	struct {
 		struct acpi_cedt_cfmws cfmws;
-		u32 target[2];
+		u32 target[1];
 	} cfmws7;
 	struct {
 		struct acpi_cedt_cfmws cfmws;
-		u32 target[3];
+		u32 target[1];
 	} cfmws8;
+	struct {
+		struct acpi_cedt_cfmws cfmws;
+		u32 target[2];
+	} cfmws9;
+	struct {
+		struct acpi_cedt_cfmws cfmws;
+		u32 target[3];
+	} cfmws10;
 	struct {
 		struct acpi_cedt_cxims cxims;
 		u64 xormap_list[2];
@@ -324,22 +332,20 @@ static struct {
 		},
 		.target = { 3 },
 	},
-	/* .cfmws6,7,8 use ACPI_CEDT_CFMWS_ARITHMETIC_XOR */
 	.cfmws6 = {
 		.cfmws = {
 			.header = {
 				.type = ACPI_CEDT_TYPE_CFMWS,
 				.length = sizeof(mock_cedt.cfmws6),
 			},
-			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
 			.interleave_ways = 0,
 			.granularity = 4,
 			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
 					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
 			.qtg_id = FAKE_QTG_ID,
-			.window_size = SZ_256M * 8UL,
+			.window_size = SZ_256M * 4UL,
 		},
-		.target = { 0, },
+		.target = { 2 },
 	},
 	.cfmws7 = {
 		.cfmws = {
@@ -347,16 +353,17 @@ static struct {
 				.type = ACPI_CEDT_TYPE_CFMWS,
 				.length = sizeof(mock_cedt.cfmws7),
 			},
-			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
-			.interleave_ways = 1,
-			.granularity = 0,
+			.interleave_ways = 0,
+			.granularity = 4,
 			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
+					ACPI_CEDT_CFMWS_RESTRICT_VOLATILE |
 					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
 			.qtg_id = FAKE_QTG_ID,
-			.window_size = SZ_256M * 8UL,
+			.window_size = SZ_256M * 4UL,
 		},
-		.target = { 0, 1, },
+		.target = { 1 },
 	},
+	/* .cfmws8,9,10 use ACPI_CEDT_CFMWS_ARITHMETIC_XOR */
 	.cfmws8 = {
 		.cfmws = {
 			.header = {
@@ -364,6 +371,38 @@ static struct {
 				.length = sizeof(mock_cedt.cfmws8),
 			},
 			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
+			.interleave_ways = 0,
+			.granularity = 4,
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
+					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
+			.qtg_id = FAKE_QTG_ID,
+			.window_size = SZ_256M * 8UL,
+		},
+		.target = { 0, },
+	},
+	.cfmws9 = {
+		.cfmws = {
+			.header = {
+				.type = ACPI_CEDT_TYPE_CFMWS,
+				.length = sizeof(mock_cedt.cfmws9),
+			},
+			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
+			.interleave_ways = 1,
+			.granularity = 0,
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
+					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
+			.qtg_id = FAKE_QTG_ID,
+			.window_size = SZ_256M * 8UL,
+		},
+		.target = { 0, 1, },
+	},
+	.cfmws10 = {
+		.cfmws = {
+			.header = {
+				.type = ACPI_CEDT_TYPE_CFMWS,
+				.length = sizeof(mock_cedt.cfmws10),
+			},
+			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
 			.interleave_ways = 8,
 			.granularity = 1,
 			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
@@ -406,18 +445,20 @@ struct acpi_cedt_cfmws *mock_cfmws[] = {
 	[3] = &mock_cedt.cfmws3.cfmws,
 	[4] = &mock_cedt.cfmws4.cfmws,
 	[5] = &mock_cedt.cfmws5.cfmws,
-	/* Modulo Math above, XOR Math below */
 	[6] = &mock_cedt.cfmws6.cfmws,
 	[7] = &mock_cedt.cfmws7.cfmws,
+	/* Modulo Math above, XOR Math below */
 	[8] = &mock_cedt.cfmws8.cfmws,
+	[9] = &mock_cedt.cfmws9.cfmws,
+	[10] = &mock_cedt.cfmws10.cfmws,
 };
 
 static int cfmws_start;
 static int cfmws_end;
 #define CFMWS_MOD_ARRAY_START 0
-#define CFMWS_MOD_ARRAY_END   5
-#define CFMWS_XOR_ARRAY_START 6
-#define CFMWS_XOR_ARRAY_END   8
+#define CFMWS_MOD_ARRAY_END   7
+#define CFMWS_XOR_ARRAY_START 8
+#define CFMWS_XOR_ARRAY_END   10
 
 struct acpi_cedt_cxims *mock_cxims[1] = {
 	[0] = &mock_cedt.cxims0.cxims,
Signature · 1 line
2.37.3