[PATCH 0/2] cxl: Allow passthrough decoders with >16K granularity

[PATCH 0/2] cxl: Allow passthrough decoders with >16K granularity

[Alison Schofield]

Sashiko reported [1] that region setup rejects valid topologies
containing a passthrough decoder beneath a wide parent interleave. In
these configurations, a passthrough decoder can compute a granularity
above the 16K maximum encodable in the interleave granularity (IG)
field even though non-interleaving decoders do not consume that field.

The first patch avoids validating the interleave granularity of
non-interleaving decoders, encodes the IG field as a don't-care when
committing those decoders, and updates the sysfs ABI documentation
accordingly.

The second patch extends cxl-test with a topology that allows a CXL
unit test to cover this case.

A unit test update for this case is posted for review [2].

[1] https://lore.kernel.org/all/20260530002613.682E71F00893@smtp.kernel.org/
[2] https://lore.kernel.org/all/cover.1781136221.git.alison.schofield@intel.com/


Alison Schofield (2):
  cxl/region: Allow passthrough decoders with >16K granularity
  cxl/test: Add a 2-way 16K root decoder for passthrough testing

 Documentation/ABI/testing/sysfs-bus-cxl |  5 ++-
 drivers/cxl/core/hdm.c                  | 13 +++++--
 drivers/cxl/core/region.c               | 47 ++++++++++++++----------
 tools/testing/cxl/test/cxl.c            | 48 +++++++++++++++++++++++--
 4 files changed, 88 insertions(+), 25 deletions(-)


base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48
-- 
2.37.3

[PATCH 1/2] cxl/region: Allow passthrough decoders with >16K granularity

[Alison Schofield]

Region configuration rejects valid topologies that contain a
passthrough decoder beneath a wide parent interleave.

For example, a passthrough switch below an 8-way root decoder with
4K granularity computes a 32K granularity. That exceeds the maximum
encodable value of the HDM Decoder Control IG field, causing region
setup to fail even though a non-interleaving decoder does not consume
the IG field.

Only require the granularity to be encodable for interleaving decoders,
both where it is inherited from the parent and where it is computed for
the current decoder. Keep the computed value for passthrough decoders so
it can seed descendant decoder setup, and program a don't-care IG
encoding when committing a non-interleaving decoder.

As a consequence, the interleave_granularity attribute of a decoder
whose interleave_ways is 1 may report a value above 16K. Document that
the reported granularity for non-interleaving decoders is a don't-care
value that may exceed the maximum encodable in hardware.

Fixes: 18f35dc9314d ("cxl/region: Refactor granularity select in cxl_port_setup_targets()")
Suggested-by: Sashiko AI Review <sashiko-bot@kernel.org>
Assisted-by: Claude:Opus-4-8
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-cxl |  5 ++-
 drivers/cxl/core/hdm.c                  | 13 +++++--
 drivers/cxl/core/region.c               | 47 +++++++++++++++----------
 3 files changed, 42 insertions(+), 23 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
index 16a9b3d2e2c0..3f96e5bf1fd3 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -407,7 +407,10 @@ Description:
 		space this decoder claims at address N before the decode rotates
 		to the next target in the interleave at address N +
 		interleave_granularity (assuming N is aligned to
-		interleave_granularity).
+		interleave_granularity). When 'interleave_ways' is 1, the
+		decoder does not interleave and the reported granularity is
+		a don't-care value that may exceed the maximum encodable in
+		hardware.
 
 
 What:		/sys/bus/cxl/devices/decoderX.Y/create_{pmem,ram}_region
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 0c80b76a5f9b..d164662668c1 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -691,9 +691,16 @@ static void cxld_set_interleave(struct cxl_decoder *cxld, u32 *ctrl)
 	if (WARN_ONCE(ways_to_eiw(cxld->interleave_ways, &eiw),
 		      "invalid interleave_ways: %d\n", cxld->interleave_ways))
 		return;
-	if (WARN_ONCE(granularity_to_eig(cxld->interleave_granularity, &eig),
-		      "invalid interleave_granularity: %d\n",
-		      cxld->interleave_granularity))
+
+	/*
+	 * A non-interleaving decoder ignores the IG field. Encode a
+	 * don't-care value instead of validating the stored granularity.
+	 */
+	if (cxld->interleave_ways == 1)
+		eig = 0;
+	else if (WARN_ONCE(granularity_to_eig(cxld->interleave_granularity, &eig),
+			   "invalid interleave_granularity: %d\n",
+			   cxld->interleave_granularity))
 		return;
 
 	u32p_replace_bits(ctrl, eig, CXL_HDM_DECODER0_CTRL_IG_MASK);
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index e50dc716d4e8..c3ad6db7c278 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1510,12 +1510,20 @@ static int cxl_port_setup_targets(struct cxl_port *port,
 		parent_iw = parent_cxld->interleave_ways;
 	}
 
-	rc = granularity_to_eig(parent_ig, &peig);
-	if (rc) {
-		dev_dbg(&cxlr->dev, "%s:%s: invalid parent granularity: %d\n",
-			dev_name(parent_port->uport_dev),
-			dev_name(&parent_port->dev), parent_ig);
-		return rc;
+	/*
+	 * A non-interleaving parent does not encode its granularity, so its
+	 * stored value may exceed the maximum encodable and need not be
+	 * validated here.
+	 */
+	if (parent_iw > 1) {
+		rc = granularity_to_eig(parent_ig, &peig);
+		if (rc) {
+			dev_dbg(&cxlr->dev,
+				"%s:%s: invalid parent granularity: %d\n",
+				dev_name(parent_port->uport_dev),
+				dev_name(&parent_port->dev), parent_ig);
+			return rc;
+		}
 	}
 
 	rc = ways_to_eiw(parent_iw, &peiw);
@@ -1538,20 +1546,21 @@ static int cxl_port_setup_targets(struct cxl_port *port,
 	 * Interleave granularity is a multiple of @parent_port granularity.
 	 * Multiplier is the parent port interleave ways.
 	 */
-	rc = granularity_to_eig(parent_ig * parent_iw, &eig);
-	if (rc) {
-		dev_dbg(&cxlr->dev,
-			"%s: invalid granularity calculation (%d * %d)\n",
-			dev_name(&parent_port->dev), parent_ig, parent_iw);
-		return rc;
-	}
+	ig = parent_ig * parent_iw;
 
-	rc = eig_to_granularity(eig, &ig);
-	if (rc) {
-		dev_dbg(&cxlr->dev, "%s:%s: invalid interleave: %d\n",
-			dev_name(port->uport_dev), dev_name(&port->dev),
-			256 << eig);
-		return rc;
+	/*
+	 * Keep the computed granularity for descendant setup. Only
+	 * interleaving decoders require an encodable granularity.
+	 */
+	if (iw > 1) {
+		rc = granularity_to_eig(ig, &eig);
+		if (rc) {
+			dev_dbg(&cxlr->dev,
+				"%s: invalid granularity calculation (%d * %d)\n",
+				dev_name(&parent_port->dev), parent_ig,
+				parent_iw);
+			return rc;
+		}
 	}
 
 	if (iw > 8 || iw > cxlsd->nr_targets) {
-- 
2.37.3

Re: [PATCH 1/2] cxl/region: Allow passthrough decoders with >16K granularity

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Passthrough decoders in CXL_REGION_F_AUTO mode do not update their `interleave_granularity` to the logically computed `ig`, breaking auto-discovery for downstream interleaving decoders.
--

commit e8de72986281000607b505c71ee58ce7b4899cb6
Author: Alison Schofield <alison.schofield@intel.com>
Subject: cxl/region: Allow passthrough decoders with >16K granularity

This commit relaxes granularity validation for non-interleaving passthrough
decoders. It ensures the computed granularity is kept to seed descendant
decoder setups while programming a don't-care encoding when committing
non-interleaving decoders.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c

[ ... ]

> @@ -1538,20 +1546,21 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>  	 * Interleave granularity is a multiple of @parent_port granularity.
>  	 * Multiplier is the parent port interleave ways.
>  	 */
> -	rc = granularity_to_eig(parent_ig * parent_iw, &eig);
> -	if (rc) {
> -		dev_dbg(&cxlr->dev,
> -			"%s: invalid granularity calculation (%d * %d)\n",
> -			dev_name(&parent_port->dev), parent_ig, parent_iw);
> -		return rc;
> -	}
> +	ig = parent_ig * parent_iw;
>  
> -	rc = eig_to_granularity(eig, &ig);
> -	if (rc) {
> -		dev_dbg(&cxlr->dev, "%s:%s: invalid interleave: %d\n",
> -			dev_name(port->uport_dev), dev_name(&port->dev),
> -			256 << eig);
> -		return rc;
> +	/*
> +	 * Keep the computed granularity for descendant setup. Only
> +	 * interleaving decoders require an encodable granularity.
> +	 */
> +	if (iw > 1) {
> +		rc = granularity_to_eig(ig, &eig);
> +		if (rc) {
> +			dev_dbg(&cxlr->dev,
> +				"%s: invalid granularity calculation (%d * %d)\n",
> +				dev_name(&parent_port->dev), parent_ig,
> +				parent_iw);
> +			return rc;
> +		}
>  	}
>  
>  	if (iw > 8 || iw > cxlsd->nr_targets) {

[Severity: High]
This is a pre-existing issue, but since the commit message mentions keeping
the computed value for passthrough decoders to seed descendant setup, does
the auto-discovery path also need to be updated to ensure this?

During region auto-discovery (CXL_REGION_F_AUTO), cxl_port_setup_targets()
checks the configuration but does not appear to update the decoder's
interleave_granularity for passthrough decoders:

	if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) {
		if (cxld->interleave_ways != iw ||
		    (iw > 1 && cxld->interleave_granularity != ig) ||
		    !spa_maps_hpa(p, &cxld->hpa_range) ||
		    ((cxld->flags & CXL_DECODER_F_ENABLE) == 0)) {
			// ... validation fails ...
			return -ENXIO;
		}
		// cxld->interleave_granularity is not updated to ig here
	} else {
		// ... manual config path updates it:
		cxld->interleave_granularity = ig;
	}

Since cxld->interleave_granularity is never updated to the computed ig in
the auto-discovery block, it retains the hardware don't-care value (such as
256 bytes or 0) read from the register.

When a downstream switch decoder is subsequently evaluated, it loads this
unencodable hardware value as parent_ig. If the downstream decoder is
interleaving (iw > 1), it will calculate a logically incorrect ig from the
garbage parent_ig. This would cause it to fail its own validation and abort
region assembly with -ENXIO.

Should cxld->interleave_granularity be explicitly updated to ig in the
CXL_REGION_F_AUTO path to prevent downstream decoders from inheriting
unencodable don't-care values?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1781136281.git.alison.schofield@intel.com?part=1

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

[Alison Schofield]

The cxl-test topologies do not provide a configuration where pass-
through decoders compute a granularity above the 16K max encodable
in hardware. The widest power-of-2 root decoder currently available
is 2-way at 4K granularity, so a passthrough decoder below it
computes at most 16K.

Add a CFMWS entry for a 2-way root decoder at 16K granularity. This
topology supports a region configuration spanning the two multi-port
host bridges and placing one endpoint under each bridge. The
intermediate switch decoders are then passthrough decoders with a
computed granularity of 32K.

This allows a CXL unit test to be added for this case.

Place the new window in the XOR topology set so the default modulo
topology used by most cxl-test coverage is unchanged.

Assisted-by: Claude:Opus-4-8
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 tools/testing/cxl/test/cxl.c | 48 ++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 296516eecfd6..ec16cd9cdbe2 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -188,10 +188,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 = {
@@ -371,6 +379,28 @@ static struct {
 		},
 		.target = { 0, 1, 2, },
 	},
+	/*
+	 * A 2-way root at 16K granularity. A passthrough decoder below this
+	 * root computes a 32K granularity (16K * 2), which exceeds the maximum
+	 * encodable in hardware. It exercises the passthrough granularity path
+	 * in cxl_port_setup_targets().
+	 */
+	.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 = {
@@ -382,6 +412,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, },
+	},
 };
 
 struct acpi_cedt_cfmws *mock_cfmws[] = {
@@ -395,6 +437,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;
@@ -402,10 +445,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 {
-- 
2.37.3