Skip to contentSkip to conversation

cxl/region: Carry the effective interleave state through target setup

PATCH New Alison Schofield 1 participant 4 messages Nov 7 CXL

lore
Share & tools
lore

Alison Schofield

author maintainer
#0 lore raw

cover cxl/region: Carry the effective interleave state through target setup

This replaces "cxl: Allow passthrough decoders with >16K granularity"[1]

Review of that series showed the >16K failure was actually a symptom of a pre-existing problem. The switch decoder setup used the parent decoder object to carry the interleave state down the hierarchy, and a passthrough decoder's granularity is a hardware don't-care.

Patch 1 carries the effective interleave ways and granularity in each port's region reference. The approach was discussed here [2].

Patches 2 and 3 add support for cxl_test coverage.

Based on cxl/for-7.5/cxl-port-refactor

[1] https://lore.kernel.org/linux-cxl/[email protected]/
[2] https://lore.kernel.org/linux-cxl/[email protected]/

Alison Schofield (3):
cxl/region: Carry the effective interleave state through target setup cxl/test: Add a 2-way 16K root decoder for passthrough testing cxl/test: Program a don't-care granularity into the passthrough host
bridge

 drivers/cxl/core/region.c    | 28 +++++++++++++----
 drivers/cxl/cxl.h            |  5 +++
 tools/testing/cxl/test/cxl.c | 61 +++++++++++++++++++++++++++++++-----
 3 files changed, 81 insertions(+), 13 deletions(-)

base-commit: 0e3e8b653ca3ab145aced729861cf0d1a06da69e

Signature · 1 line
2.37.3
author maintainer
#1 lore raw

PATCH 1/3 cxl/region: Carry the effective interleave state through target setup

Switch decoder setup derives interleave settings from the parent decoder. A passthrough decoder's granularity is a hardware don't-care, so using it can cause auto region assembly to reject a downstream decoder whose programmed granularity differs from the calculated value.

User-created regions instead store the calculated granularity in the passthrough decoder. Below a 2-way 16K root, that calculation produces 32K, exceeding the hardware limit even though the decoder does not interleave.

Carry the effective interleave state in each port's region reference. Interleaving decoders establish that state and passthrough decoders preserve it for descendants. Store the state because a later endpoint may be the first to reach a descendant of an already configured port.

In both paths, passthrough decoders report the effective granularity inherited from above. Their interleave_granularity ABI permits this because a single-target decoder never rotates.

Fixes 18f35dc9314d ("cxl/region: Refactor granularity select in cxl_port_setup_targets()")
Fixes ce32b0c9c522 ("cxl: core/region - ignore interleave granularity when ways=1")

---

The Suggested-by's come from this discussion (also linked in cover letter)
https://lore.kernel.org/linux-cxl/[email protected]/

 drivers/cxl/core/region.c | 28 ++++++++++++++++++++++------
 drivers/cxl/cxl.h         |  5 +++++
 2 files changed, 27 insertions(+), 6 deletions(-)
drivers/cxl/core/region.c +22 -6
index d6a5dbe65d26..b81a142108e5 100644
@@ -1489,12 +1489,15 @@ static int cxl_port_parent_interleave(struct cxl_port *port,
 			parent_iw /= 3;
 	} else {
 		struct cxl_region_ref *parent_rr;
-		struct cxl_decoder *parent_cxld;
 
+		/*
+		 * The granularity of a passthrough (iw == 1) decoder is a
+		 * hardware don't-care, so use the parent port's effective
+		 * interleave state rather than the parent decoder's settings.
+		 */
 		parent_rr = cxl_rr_load(parent_port, cxlr);
-		parent_cxld = parent_rr->decoder;
-		parent_ig = parent_cxld->interleave_granularity;
-		parent_iw = parent_cxld->interleave_ways;
+		parent_ig = parent_rr->eff_ig;
+		parent_iw = parent_rr->eff_iw;
 	}
 
 	if (!granularity_valid(parent_ig)) {
@@ -1539,9 +1542,13 @@ static int cxl_port_calc_interleave(struct cxl_port *port, struct cxl_region *cx
 
 	/*
 	 * Interleave granularity is a multiple of @parent_port granularity.
-	 * Multiplier is the parent port interleave ways.
+	 * Multiplier is the parent port interleave ways. A passthrough
+	 * decoder does not interleave, so it takes the parent granularity.
 	 */
-	ig = parent_ig * parent_iw;
+	if (iw == 1)
+		ig = parent_ig;
+	else
+		ig = parent_ig * parent_iw;
 
 	if (!granularity_valid(ig)) {
 		dev_dbg(&cxlr->dev,
@@ -1559,6 +1566,12 @@ static int cxl_port_calc_interleave(struct cxl_port *port, struct cxl_region *cx
 		return -ENXIO;
 	}
 
+	if (iw == 1)
+		cxl_rr->eff_iw = parent_iw;
+	else
+		cxl_rr->eff_iw = iw;
+	cxl_rr->eff_ig = ig;
+
 	*out_iw = iw;
 	*out_ig = ig;
 
@@ -1589,6 +1602,9 @@ static int cxl_port_setup_decoder(struct cxl_port *port, struct cxl_region *cxlr
 				cxld->hpa_range.start, cxld->hpa_range.end);
 			return -ENXIO;
 		}
+		/* Report the effective granularity, not the firmware value */
+		if (iw == 1)
+			cxld->interleave_granularity = ig;
 	} else {
 		rc = check_interleave_cap(cxld, iw, ig);
 		if (rc) {
drivers/cxl/cxl.h +5 -0
index 11688d17d09f..aeb7704869ed 100644
@@ -716,6 +716,9 @@ struct cxl_ep {
  * @nr_targets_set: track how many targets have been programmed during setup
  * @nr_eps: number of endpoints beneath @port
  * @nr_targets: number of distinct targets needed to reach @nr_eps
+ * @eff_iw: effective interleave ways at @port, set by the nearest
+ *	    interleaving decoder at or above @port
+ * @eff_ig: effective interleave granularity at @port, set as for @eff_iw
  */
 struct cxl_region_ref {
 	struct cxl_port *port;
@@ -725,6 +728,8 @@ struct cxl_region_ref {
 	int nr_targets_set;
 	int nr_eps;
 	int nr_targets;
+	int eff_iw;
+	int eff_ig;
 };
 
 /*
Signature · 1 line
2.37.3
author maintainer
#2 lore raw

PATCH 2/3 cxl/test: Add a 2-way 16K root decoder for passthrough testing

In reply to Alison Schofield

Add a 2-way root decoder at the maximum hardware granularity of 16K. Selecting one endpoint under each host bridge makes the intermediate decoders passthrough, allowing a unit test to check that they carry the 16K interleave without exceeding the hardware granularity limit.

Place the window in the XOR topology set to leave the default modulo topology unchanged.

---
 tools/testing/cxl/test/cxl.c | 49 +++++++++++++++++++++++++++++++++---
 1 file changed, 46 insertions(+), 3 deletions(-)
tools/testing/cxl/test/cxl.c +46 -3
index 3eb1051b1ee8..76154c4bc15d 100644
@@ -191,10 +191,18 @@ static struct {
 		struct acpi_cedt_cfmws cfmws;
 		u32 target[3];
 	} cfmws8;
+	struct {
+		struct acpi_cedt_cfmws cfmws;
+		u32 target[2];
+	} cfmws9;
 	struct {
 		struct acpi_cedt_cxims cxims;
 		u64 xormap_list[2];
 	} cxims0;
+	struct {
+		struct acpi_cedt_cxims cxims;
+		u64 xormap_list[1];
+	} cxims1;
 } __packed mock_cedt = {
 	.cedt = {
 		.header = {
@@ -325,7 +333,7 @@ static struct {
 		},
 		.target = { 3 },
 	},
-	/* .cfmws6,7,8 use ACPI_CEDT_CFMWS_ARITHMETIC_XOR */
+	/* .cfmws6,7,8,9 use ACPI_CEDT_CFMWS_ARITHMETIC_XOR */
 	.cfmws6 = {
 		.cfmws = {
 			.header = {
@@ -374,6 +382,27 @@ static struct {
 		},
 		.target = { 0, 1, 2, },
 	},
+	/*
+	 * A 2-way root at 16K, the maximum granularity encodable in
+	 * hardware. Passthrough decoders below it must carry the 16K
+	 * interleave through rather than multiply it.
+	 */
+	.cfmws9 = {
+		.cfmws = {
+			.header = {
+				.type = ACPI_CEDT_TYPE_CFMWS,
+				.length = sizeof(mock_cedt.cfmws9),
+			},
+			.interleave_arithmetic = ACPI_CEDT_CFMWS_ARITHMETIC_XOR,
+			.interleave_ways = 1,
+			.granularity = 6,
+			.restrictions = ACPI_CEDT_CFMWS_RESTRICT_HOSTONLYMEM |
+					ACPI_CEDT_CFMWS_RESTRICT_PMEM,
+			.qtg_id = FAKE_QTG_ID,
+			.window_size = SZ_256M * 8UL,
+		},
+		.target = { 0, 1, },
+	},
 	.cxims0 = {
 		.cxims = {
 			.header = {
@@ -385,6 +414,18 @@ static struct {
 		},
 		.xormap_list = { 0x404100, 0x808200, },
 	},
+	/* CXIMS for the 16K (hbig = 6) 2-way root, cfmws9 */
+	.cxims1 = {
+		.cxims = {
+			.header = {
+				.type = ACPI_CEDT_TYPE_CXIMS,
+				.length = sizeof(mock_cedt.cxims1),
+			},
+			.hbig = 6,
+			.nr_xormaps = 1,
+		},
+		.xormap_list = { 0x0, },
+	},
 };
 
 static struct acpi_cedt_cfmws type2_cfmws0 = {
@@ -411,6 +452,7 @@ struct acpi_cedt_cfmws *mock_cfmws[] = {
 	[6] = &mock_cedt.cfmws6.cfmws,
 	[7] = &mock_cedt.cfmws7.cfmws,
 	[8] = &mock_cedt.cfmws8.cfmws,
+	[9] = &mock_cedt.cfmws9.cfmws,
 };
 
 static int cfmws_start;
@@ -418,10 +460,11 @@ 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_XOR_ARRAY_END   9
 
-struct acpi_cedt_cxims *mock_cxims[1] = {
+struct acpi_cedt_cxims *mock_cxims[2] = {
 	[0] = &mock_cedt.cxims0.cxims,
+	[1] = &mock_cedt.cxims1.cxims,
 };
 
 struct cxl_mock_res {
Signature · 1 line
2.37.3
author maintainer
#3 lore raw

PATCH 3/3 cxl/test: Program a don't-care granularity into the passthrough host bridge

In reply to Alison Schofield

The auto region mock gives the passthrough host bridge decoder the same 4K granularity as the switch decoder below it. Hardware ignores granularity at 1-way, so matching values leave propagation of a firmware don't-care value untested.

Set the host bridge decoder's granularity to 256 bytes. Auto region assembly then fails unless target setup carries the effective interleave state through the passthrough decoder.

---
 tools/testing/cxl/test/cxl.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
tools/testing/cxl/test/cxl.c +8 -4
index 76154c4bc15d..cd4a99fface6 100644
@@ -1374,13 +1374,17 @@ static void mock_init_hdm_type3_cxled(struct cxl_endpoint_decoder *cxled,
 		iter->commit_end = 0;
 		/*
 		 * Switch targets 2 endpoints, while host bridge targets
-		 * one root port
+		 * one root port. Like some platform firmware, program the
+		 * passthrough host bridge with a granularity that differs
+		 * from the region's, since hardware ignores it at 1-way.
 		 */
-		if (i == 0)
+		if (i == 0) {
 			cxld->interleave_ways = 2;
-		else
+			cxld->interleave_granularity = 4096;
+		} else {
 			cxld->interleave_ways = 1;
-		cxld->interleave_granularity = 4096;
+			cxld->interleave_granularity = 256;
+		}
 		cxld->hpa_range = (struct range) {
 			.start = base,
 			.end = base + mock_auto_region_size - 1,
Signature · 1 line
2.37.3