[PATCH v1 0/2] Support zero-sized decoders

[PATCH v1 0/2] Support zero-sized decoders

[Vishal Aslot]

This patchset adds support for committed decoders
of size 0. It also extends cxl_test with zero-sized
decoders.

Vishal Aslot (2):
  [PATCH v1 1/2] cxl_test: enable zero sized decoders under hb0
  [PATCH v1 2/2] cxl: Allow zero sized HDM decoders

 drivers/cxl/core/hdm.c       |  9 ++--
 tools/testing/cxl/test/cxl.c | 96 +++++++++++++++++++++++++++++++++++-
 2 files changed, 100 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH v1 1/2] cxl_test: enable zero sized decoders under hb0

[Vishal Aslot]

The cxl core in linux updated to supported committed
decoders of zero size, because this is allowed by
the CXL spec.

This patch updates cxl_test to enable decoders 1 and 2
in the host-bridge 0 port, in a switch uport under hb0,
and the endpoints ports with size zero simulating
committed zero sized decoders.

Signed-off-by: Vishal Aslot <vaslot@nvidia.com>
---
 tools/testing/cxl/test/cxl.c | 96 +++++++++++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+), 2 deletions(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 2d135ca533d0..cb18ee41a7cf 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -719,6 +719,45 @@ static void default_mock_decoder(struct cxl_decoder *cxld)
 	cxld->reset = mock_decoder_reset;
 }
 
+static void size_zero_mock_decoder_ep(struct cxl_decoder *cxld, u64 base)
+{
+	struct cxl_endpoint_decoder *cxled;
+
+	cxled = to_cxl_endpoint_decoder(&cxld->dev);
+	cxld->hpa_range = (struct range){
+		.start = base,
+		.end = base - 1,  /* Size 0 */
+	};
+
+	cxld->interleave_ways = 2;
+	cxld->interleave_granularity = 4096;
+	cxld->target_type = CXL_DECODER_HOSTONLYMEM;
+	cxld->flags = CXL_DECODER_F_ENABLE;
+	cxled->state = CXL_DECODER_STATE_AUTO;
+	cxld->commit = mock_decoder_commit;
+	cxld->reset = mock_decoder_reset;
+}
+
+static void size_zero_mock_decoder_sw(struct device *dev, u64 base, int i)
+{
+	struct cxl_switch_decoder *cxlsd;
+	struct cxl_decoder *cxld;
+
+	cxlsd = to_cxl_switch_decoder(dev);
+	cxld = &cxlsd->cxld;
+	cxld->flags = CXL_DECODER_F_ENABLE;
+	cxld->target_type = CXL_DECODER_HOSTONLYMEM;
+	if (i == 0)
+		cxld->interleave_ways = 2;
+	else
+		cxld->interleave_ways = 1;
+	cxld->interleave_granularity = 4096;
+	cxld->hpa_range = (struct range) {
+		.start = base,
+		.end = base - 1, /* Size 0 */
+	};
+}
+
 static int first_decoder(struct device *dev, const void *data)
 {
 	struct cxl_decoder *cxld;
@@ -731,6 +770,30 @@ static int first_decoder(struct device *dev, const void *data)
 	return 0;
 }
 
+static int second_decoder(struct device *dev, const void *data)
+{
+	struct cxl_decoder *cxld;
+
+	if (!is_switch_decoder(dev))
+		return 0;
+	cxld = to_cxl_decoder(dev);
+	if (cxld->id == 1)
+		return 1;
+	return 0;
+}
+
+static int third_decoder(struct device *dev, const void *data)
+{
+	struct cxl_decoder *cxld;
+
+	if (!is_switch_decoder(dev))
+		return 0;
+	cxld = to_cxl_decoder(dev);
+	if (cxld->id == 2)
+		return 1;
+	return 0;
+}
+
 static void mock_init_hdm_decoder(struct cxl_decoder *cxld)
 {
 	struct acpi_cedt_cfmws *window = mock_cfmws[0];
@@ -743,7 +806,7 @@ static void mock_init_hdm_decoder(struct cxl_decoder *cxld)
 	struct cxl_dport *dport;
 	struct device *dev;
 	bool hb0 = false;
-	u64 base;
+	u64 base = window->base_hpa;
 	int i;
 
 	if (is_endpoint_decoder(&cxld->dev)) {
@@ -767,6 +830,20 @@ static void mock_init_hdm_decoder(struct cxl_decoder *cxld)
 		port = cxled_to_port(cxled);
 	}
 
+	/*
+	 * Decoders 1 and 2 of the endpoint under host bridge 0 should be enabled as zero-sized.
+	 * It would be even better to make sure that the parent switch uport decoder was
+	 * also enabled before enabling the size zero decoders but there is no harm in doing it
+	 * anyway.
+	 */
+	if (hb0 && (cxld->id == 1 || cxld->id == 2)) {
+		port = to_cxl_port(cxld->dev.parent);
+		size_zero_mock_decoder_ep(cxld, base);
+		/* Commit the zero-sized decoder */
+		port->commit_end = cxld->id;
+		return;
+	}
+
 	/*
 	 * The first decoder on the first 2 devices on the first switch
 	 * attached to host-bridge0 mock a fake / static RAM region. All
@@ -780,7 +857,6 @@ static void mock_init_hdm_decoder(struct cxl_decoder *cxld)
 		return;
 	}
 
-	base = window->base_hpa;
 	cxld->hpa_range = (struct range) {
 		.start = base,
 		.end = base + size - 1,
@@ -844,6 +920,22 @@ static void mock_init_hdm_decoder(struct cxl_decoder *cxld)
 			.end = base + size - 1,
 		};
 		put_device(dev);
+
+		/* Enable the next two decoders also and make them zero sized */
+		dev = device_find_child(&iter->dev, NULL, second_decoder);
+		WARN_ON(!dev);
+		if (dev) {
+			size_zero_mock_decoder_sw(dev, base, i);
+			iter->commit_end = 1;
+			put_device(dev);
+		}
+		dev = device_find_child(&iter->dev, NULL, third_decoder);
+		WARN_ON(!dev);
+		if (dev) {
+			size_zero_mock_decoder_sw(dev, base, i);
+			iter->commit_end = 2;
+			put_device(dev);
+		}
 	}
 }
 
-- 
2.43.0

[PATCH v1 2/2] cxl: Allow zero sized HDM decoders

[Vishal Aslot]

CXL spec permits committing zero sized decoders.
Linux currently considers them as an error.

Zero-sized decoders are helpful when the BIOS
is committing them. Often BIOS will also lock
them to prevent them being changed due to the
TSP requirement. For example, if the type 3
device is part of a TCB.

The host bridge, switch, and end-point decoders
can all be committed with zero-size. If they are
locked along the VH, it is often to prevent
hotplugging of a new device that could not be
attested post boot and cannot be included in
TCB.

The caller leaves the decoder allocated but does
not add it. It simply continues to the next decoder.

Signed-off-by: Vishal Aslot <vaslot@nvidia.com>
---
 drivers/cxl/core/hdm.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index d3a094ca01ad..1c036a485723 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -1036,13 +1036,14 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
 			return -ENXIO;
 		}
 
+		port->commit_end = cxld->id;
+
 		if (size == 0) {
-			dev_warn(&port->dev,
+			dev_dbg(&port->dev,
 				 "decoder%d.%d: Committed with zero size\n",
 				 port->id, cxld->id);
-			return -ENXIO;
+			return -ENOSPC;
 		}
-		port->commit_end = cxld->id;
 	} else {
 		if (cxled) {
 			struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
@@ -1198,6 +1199,8 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
 
 		rc = init_hdm_decoder(port, cxld, hdm, i, &dpa_base, info);
 		if (rc) {
+			if (rc == -ENOSPC)
+				continue;
 			dev_warn(&port->dev,
 				 "Failed to initialize decoder%d.%d\n",
 				 port->id, i);
-- 
2.43.0

Re: [PATCH v1 1/2] cxl_test: enable zero sized decoders under hb0

[Dave Jiang]

On 10/14/25 7:40 PM, Vishal Aslot wrote:
> The cxl core in linux updated to supported committed
> decoders of zero size, because this is allowed by
> the CXL spec.
> 
> This patch updates cxl_test to enable decoders 1 and 2
> in the host-bridge 0 port, in a switch uport under hb0,
> and the endpoints ports with size zero simulating
> committed zero sized decoders.

Hi Vishal, first of all, really appreciate you doing this. If there's another rev of the series, let's reorder and the test patch should go after the implementation patch.

Can you add a little more in the commit log on how this is tested? i.e. this code is exercised in cxl-topology.sh unit test or something else etc etc.

> 
> Signed-off-by: Vishal Aslot <vaslot@nvidia.com>
> ---
>  tools/testing/cxl/test/cxl.c | 96 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 94 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
> index 2d135ca533d0..cb18ee41a7cf 100644
> --- a/tools/testing/cxl/test/cxl.c
> +++ b/tools/testing/cxl/test/cxl.c
> @@ -719,6 +719,45 @@ static void default_mock_decoder(struct cxl_decoder *cxld)
>  	cxld->reset = mock_decoder_reset;
>  }
>  
> +static void size_zero_mock_decoder_ep(struct cxl_decoder *cxld, u64 base)
> +{
> +	struct cxl_endpoint_decoder *cxled;
> +
> +	cxled = to_cxl_endpoint_decoder(&cxld->dev);
> +	cxld->hpa_range = (struct range){
> +		.start = base,
> +		.end = base - 1,  /* Size 0 */
> +	};
> +
> +	cxld->interleave_ways = 2;
> +	cxld->interleave_granularity = 4096;
> +	cxld->target_type = CXL_DECODER_HOSTONLYMEM;
> +	cxld->flags = CXL_DECODER_F_ENABLE;
> +	cxled->state = CXL_DECODER_STATE_AUTO;
> +	cxld->commit = mock_decoder_commit;
> +	cxld->reset = mock_decoder_reset;
> +}
> +
> +static void size_zero_mock_decoder_sw(struct device *dev, u64 base, int i)
> +{
> +	struct cxl_switch_decoder *cxlsd;
> +	struct cxl_decoder *cxld;
> +
> +	cxlsd = to_cxl_switch_decoder(dev);
> +	cxld = &cxlsd->cxld;
> +	cxld->flags = CXL_DECODER_F_ENABLE;
> +	cxld->target_type = CXL_DECODER_HOSTONLYMEM;
> +	if (i == 0)
> +		cxld->interleave_ways = 2;
> +	else
> +		cxld->interleave_ways = 1;
> +	cxld->interleave_granularity = 4096;
> +	cxld->hpa_range = (struct range) {
> +		.start = base,
> +		.end = base - 1, /* Size 0 */
> +	};
> +}
> +
>  static int first_decoder(struct device *dev, const void *data)
>  {
>  	struct cxl_decoder *cxld;
> @@ -731,6 +770,30 @@ static int first_decoder(struct device *dev, const void *data)
>  	return 0;
>  }
>  
> +static int second_decoder(struct device *dev, const void *data)
> +{
> +	struct cxl_decoder *cxld;
> +
> +	if (!is_switch_decoder(dev))
> +		return 0;
> +	cxld = to_cxl_decoder(dev);
> +	if (cxld->id == 1)
> +		return 1;
> +	return 0;
> +}
> +
> +static int third_decoder(struct device *dev, const void *data)
> +{
> +	struct cxl_decoder *cxld;
> +
> +	if (!is_switch_decoder(dev))
> +		return 0;
> +	cxld = to_cxl_decoder(dev);
> +	if (cxld->id == 2)
> +		return 1;
> +	return 0;
> +}
> +
>  static void mock_init_hdm_decoder(struct cxl_decoder *cxld)
>  {
>  	struct acpi_cedt_cfmws *window = mock_cfmws[0];
> @@ -743,7 +806,7 @@ static void mock_init_hdm_decoder(struct cxl_decoder *cxld)
>  	struct cxl_dport *dport;
>  	struct device *dev;
>  	bool hb0 = false;
> -	u64 base;
> +	u64 base = window->base_hpa;
>  	int i;
>  
>  	if (is_endpoint_decoder(&cxld->dev)) {
> @@ -767,6 +830,20 @@ static void mock_init_hdm_decoder(struct cxl_decoder *cxld)
>  		port = cxled_to_port(cxled);
>  	}
>  
> +	/*
> +	 * Decoders 1 and 2 of the endpoint under host bridge 0 should be enabled as zero-sized.
> +	 * It would be even better to make sure that the parent switch uport decoder was
> +	 * also enabled before enabling the size zero decoders but there is no harm in doing it
> +	 * anyway.
> +	 */
> +	if (hb0 && (cxld->id == 1 || cxld->id == 2)) {
> +		port = to_cxl_port(cxld->dev.parent);
> +		size_zero_mock_decoder_ep(cxld, base);
> +		/* Commit the zero-sized decoder */
> +		port->commit_end = cxld->id;
> +		return;
> +	}
> +
>  	/*
>  	 * The first decoder on the first 2 devices on the first switch
>  	 * attached to host-bridge0 mock a fake / static RAM region. All
> @@ -780,7 +857,6 @@ static void mock_init_hdm_decoder(struct cxl_decoder *cxld)
>  		return;
>  	}
>  
> -	base = window->base_hpa;
>  	cxld->hpa_range = (struct range) {
>  		.start = base,
>  		.end = base + size - 1,
> @@ -844,6 +920,22 @@ static void mock_init_hdm_decoder(struct cxl_decoder *cxld)
>  			.end = base + size - 1,
>  		};
>  		put_device(dev);
> +
> +		/* Enable the next two decoders also and make them zero sized */

s/decoders/switch decoders/

> +		dev = device_find_child(&iter->dev, NULL, second_decoder);

Maybe just pass in the index as the data parameter and then you can just use match_decoder_by_index instead of static code second and third.

DJ

> +		WARN_ON(!dev);
> +		if (dev) {
> +			size_zero_mock_decoder_sw(dev, base, i);
> +			iter->commit_end = 1;
> +			put_device(dev);
> +		}
> +		dev = device_find_child(&iter->dev, NULL, third_decoder);
> +		WARN_ON(!dev);
> +		if (dev) {
> +			size_zero_mock_decoder_sw(dev, base, i);
> +			iter->commit_end = 2;
> +			put_device(dev);
> +		}
>  	}
>  }
>  

Re: [PATCH v1 2/2] cxl: Allow zero sized HDM decoders

[Dave Jiang]

On 10/14/25 7:40 PM, Vishal Aslot wrote:
> CXL spec permits committing zero sized decoders.
> Linux currently considers them as an error.
> 
> Zero-sized decoders are helpful when the BIOS
> is committing them. Often BIOS will also lock
> them to prevent them being changed due to the
> TSP requirement. For example, if the type 3
> device is part of a TCB.
> 
> The host bridge, switch, and end-point decoders
> can all be committed with zero-size. If they are
> locked along the VH, it is often to prevent
> hotplugging of a new device that could not be
> attested post boot and cannot be included in
> TCB.
> 
> The caller leaves the decoder allocated but does
> not add it. It simply continues to the next decoder.

I think it should add it as well, just as a locked decoder that's size 0. When we do cxl list of decoders, we should see those decoders as how they are.

DJ

> 
> Signed-off-by: Vishal Aslot <vaslot@nvidia.com>
> ---
>  drivers/cxl/core/hdm.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index d3a094ca01ad..1c036a485723 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -1036,13 +1036,14 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>  			return -ENXIO;
>  		}
>  
> +		port->commit_end = cxld->id;
> +
>  		if (size == 0) {
> -			dev_warn(&port->dev,
> +			dev_dbg(&port->dev,
>  				 "decoder%d.%d: Committed with zero size\n",
>  				 port->id, cxld->id);
> -			return -ENXIO;
> +			return -ENOSPC;
>  		}
> -		port->commit_end = cxld->id;
>  	} else {
>  		if (cxled) {
>  			struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> @@ -1198,6 +1199,8 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
>  
>  		rc = init_hdm_decoder(port, cxld, hdm, i, &dpa_base, info);
>  		if (rc) {
> +			if (rc == -ENOSPC)
> +				continue;
>  			dev_warn(&port->dev,
>  				 "Failed to initialize decoder%d.%d\n",
>  				 port->id, i);

Re: [PATCH v1 2/2] cxl: Allow zero sized HDM decoders

[Alison Schofield]

On Tue, Oct 14, 2025 at 07:40:06PM -0700, Vishal Aslot wrote:
> CXL spec permits committing zero sized decoders.
> Linux currently considers them as an error.
> 
> Zero-sized decoders are helpful when the BIOS
> is committing them. Often BIOS will also lock
> them to prevent them being changed due to the
> TSP requirement. For example, if the type 3
> device is part of a TCB.
> 
> The host bridge, switch, and end-point decoders
> can all be committed with zero-size. If they are
> locked along the VH, it is often to prevent
> hotplugging of a new device that could not be
> attested post boot and cannot be included in
> TCB.
> 
> The caller leaves the decoder allocated but does
> not add it. It simply continues to the next decoder.

Please write out acronyms TSP and TCB ?
Commit log wraps at 70.

I tried out the cxl-test patch of this. And came up with the same
question that DaveJ responded here.  Why aren't we adding these
decoders to the topology? Are these guaranteed to always be lesser
id's than than the decoders for any BIOS defined regions, or is
the driver expected to handle non contiguous decoder sets?

It's an accounting task, whether they are in the topology or not,
but guessing in the topology may make that accounting easier.


> 
> Signed-off-by: Vishal Aslot <vaslot@nvidia.com>
> ---
>  drivers/cxl/core/hdm.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index d3a094ca01ad..1c036a485723 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -1036,13 +1036,14 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>  			return -ENXIO;
>  		}
>  
> +		port->commit_end = cxld->id;
> +
>  		if (size == 0) {
> -			dev_warn(&port->dev,
> +			dev_dbg(&port->dev,
>  				 "decoder%d.%d: Committed with zero size\n",
>  				 port->id, cxld->id);
> -			return -ENXIO;
> +			return -ENOSPC;
>  		}
> -		port->commit_end = cxld->id;
>  	} else {
>  		if (cxled) {
>  			struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> @@ -1198,6 +1199,8 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
>  
>  		rc = init_hdm_decoder(port, cxld, hdm, i, &dpa_base, info);
>  		if (rc) {
> +			if (rc == -ENOSPC)
> +				continue;

This needs to be in the mock_devm_cxl_enumerate_decoders() ....

>  			dev_warn(&port->dev,
>  				 "Failed to initialize decoder%d.%d\n",
>  				 port->id, i);
> -- 
> 2.43.0
> 

Re: [PATCH v1 1/2] cxl_test: enable zero sized decoders under hb0

[Alison Schofield]

On Tue, Oct 14, 2025 at 07:40:05PM -0700, Vishal Aslot wrote:
> The cxl core in linux updated to supported committed
> decoders of zero size, because this is allowed by
> the CXL spec.

Tell more about what the spec allows?

> 
> This patch updates cxl_test to enable decoders 1 and 2
> in the host-bridge 0 port, in a switch uport under hb0,
> and the endpoints ports with size zero simulating
> committed zero sized decoders.

Decoders 1 & 2 - those are after decoder 0, the autoregion.
That's a problem ATM, when we try to teardown the autoregion we
get out of order resets. Like I asked in the other patch, if there
are rules about where these zero size decoders may appear, that
may make the solution here simpler.

Add the skip to mock_devm_enumerate_decoders() to clean up -
a bunch like this:
[   94.222456] cxl decoder0.0: unsupported mode 223426864
that were followed by construct region failures...
[   94.222456] cxl decoder0.0: unsupported mode 223426864
[   94.223790] probe of endpoint13 returned 0 after 136794 usecs
[   94.224595] cxl_mock_mem cxl_mem.6: mem3:decoder15.1: construct_region failed assign region: -22

FWIW - cxl-test mocks the init, enumerate, commit, and reset of the
decoders, so all those need to be coordinated w the 'real' driver.
That may sound like a pain and it could lead to bugs that are only
in cxl-test - but it lets us then test region creation paths.

-- Alison

Re: [PATCH v1 1/2] cxl_test: enable zero sized decoders under hb0

[Gregory Price]

On Mon, Oct 20, 2025 at 12:09:34AM -0700, Alison Schofield wrote:
> > This patch updates cxl_test to enable decoders 1 and 2
> > in the host-bridge 0 port, in a switch uport under hb0,
> > and the endpoints ports with size zero simulating
> > committed zero sized decoders.
> 
> Decoders 1 & 2 - those are after decoder 0, the autoregion.
> That's a problem ATM, when we try to teardown the autoregion we
> get out of order resets. Like I asked in the other patch, if there
> are rules about where these zero size decoders may appear, that
> may make the solution here simpler.
> 

I think this is going to require a quirk-doc like other deviations.

A committed decoder must have a base address, and with 0-size subsequent
or previous decoders would also have an address that covers that address
as well.  This is on top of the ordering issue if the 0-side decoders
come after a programmable decoder.

I'm not convinced this even makes sense as a security thing if you can
reset the bus and re-activate everything (after a graceful teardown).

Seems easier to just report the decoders as unavailable and then not
probe them.

~Gregory

Re: [PATCH v1 1/2] cxl_test: enable zero sized decoders under hb0

[Alison Schofield]

On Mon, Oct 20, 2025 at 10:19:09AM -0400, Gregory Price wrote:
> On Mon, Oct 20, 2025 at 12:09:34AM -0700, Alison Schofield wrote:
> > > This patch updates cxl_test to enable decoders 1 and 2
> > > in the host-bridge 0 port, in a switch uport under hb0,
> > > and the endpoints ports with size zero simulating
> > > committed zero sized decoders.
> > 
> > Decoders 1 & 2 - those are after decoder 0, the autoregion.
> > That's a problem ATM, when we try to teardown the autoregion we
> > get out of order resets. Like I asked in the other patch, if there
> > are rules about where these zero size decoders may appear, that
> > may make the solution here simpler.
> > 
> 
> I think this is going to require a quirk-doc like other deviations.

Really need to hear more about spec here. You mention quirk, but is it
really a quirk or spec defined behavior?

> 
> A committed decoder must have a base address, and with 0-size subsequent
> or previous decoders would also have an address that covers that address
> as well.  This is on top of the ordering issue if the 0-side decoders
> come after a programmable decoder.
> 
> I'm not convinced this even makes sense as a security thing if you can
> reset the bus and re-activate everything (after a graceful teardown).
> 
> Seems easier to just report the decoders as unavailable and then not
> probe them.

Users see a memdev in the topology and want to use it but find no
available endpoint decoder. We'll probably want a mechanism to show why
that is so, hence the suggestion to add to topology and show as locked.

> 
> ~Gregory

Re: [PATCH v1 1/2] cxl_test: enable zero sized decoders under hb0

[Dave Jiang]

On 10/20/25 12:30 PM, Alison Schofield wrote:
> On Mon, Oct 20, 2025 at 10:19:09AM -0400, Gregory Price wrote:
>> On Mon, Oct 20, 2025 at 12:09:34AM -0700, Alison Schofield wrote:
>>>> This patch updates cxl_test to enable decoders 1 and 2
>>>> in the host-bridge 0 port, in a switch uport under hb0,
>>>> and the endpoints ports with size zero simulating
>>>> committed zero sized decoders.
>>>
>>> Decoders 1 & 2 - those are after decoder 0, the autoregion.
>>> That's a problem ATM, when we try to teardown the autoregion we
>>> get out of order resets. Like I asked in the other patch, if there
>>> are rules about where these zero size decoders may appear, that
>>> may make the solution here simpler.
>>>
>>
>> I think this is going to require a quirk-doc like other deviations.
> 
> Really need to hear more about spec here. You mention quirk, but is it
> really a quirk or spec defined behavior?
> 
>>
>> A committed decoder must have a base address, and with 0-size subsequent
>> or previous decoders would also have an address that covers that address
>> as well.  This is on top of the ordering issue if the 0-side decoders
>> come after a programmable decoder.
>>
>> I'm not convinced this even makes sense as a security thing if you can
>> reset the bus and re-activate everything (after a graceful teardown).
>>
>> Seems easier to just report the decoders as unavailable and then not
>> probe them.
> 
> Users see a memdev in the topology and want to use it but find no
> available endpoint decoder. We'll probably want a mechanism to show why
> that is so, hence the suggestion to add to topology and show as locked.

I think the kernel driver should be fully aware of what is and isn't fully present and handle them appropriately. And on the user side, 'cxl list' should show a decoder in a zero size state so the admin knows why things are the way they are.  > 
>>
>> ~Gregory

Re: [PATCH v1 1/2] cxl_test: enable zero sized decoders under hb0

[Gregory Price]

On Mon, Oct 20, 2025 at 12:30:21PM -0700, Alison Schofield wrote:
> On Mon, Oct 20, 2025 at 10:19:09AM -0400, Gregory Price wrote:
> > On Mon, Oct 20, 2025 at 12:09:34AM -0700, Alison Schofield wrote:
> > > > This patch updates cxl_test to enable decoders 1 and 2
> > > > in the host-bridge 0 port, in a switch uport under hb0,
> > > > and the endpoints ports with size zero simulating
> > > > committed zero sized decoders.
> > > 
> > > Decoders 1 & 2 - those are after decoder 0, the autoregion.
> > > That's a problem ATM, when we try to teardown the autoregion we
> > > get out of order resets. Like I asked in the other patch, if there
> > > are rules about where these zero size decoders may appear, that
> > > may make the solution here simpler.
> > > 
> > 
> > I think this is going to require a quirk-doc like other deviations.
> 
> Really need to hear more about spec here. You mention quirk, but is it
> really a quirk or spec defined behavior?
>

Quoted below.  Small bit of ambiguity around base=[X] when size=0.

There's no requirement on base for a size=0 decoder, so it sort of implies
the base is ignored - except that the normal commit requires (base+size)
checks on decoder[m]/[m+1] without discussing size=0 decoders.

So while the spec doesn't say the base in a 0-size decoder base address
must be set, if you implement the logic here trivially you will always
fail if the zero-size decoder.base=0.

Note that 14.13.9 also doesn't say firmware must enforce commit-order,
so deviant software could go off and commit decoders out of order.

All the spec says is "If software intends to set Lock On Commit,
Software must configure the decoders in order".

~Gregory


------------------------------------------

Re commit order - Vishal said they do post-lock order
https://lore.kernel.org/linux-cxl/aOP3Kr3jHLWOydRp@gourry-fedora-PF4VCD3F/

Post-lock order
[programmable] [zero-lock] [zero-lock] ... [zero-lock]

Pre-lock order:
[zero-lock] [zero-lock] ... [zero-lock] [programmable]

My reading of the spec suggests that Post-lock ordering is *not legal*,
and would suggest the software has deviated from the spec - which does
not allow for size-zero decoders to ignore commit order.

------------------------------------------
8.2.4.20.12: Committing Decoder Programming

If Software intends to set Lock On Commit, Software must configure the
decoders in order. In other words, decoder m must be configured and
committed before decoder m+1 for all values of m.

Decoder m must cover an HPA range that is below decoder m+1.

...

It is legal for software to program Decoder Size to 0 and commit it. Such a decoder will
not participate in HDM decode.

------------------------------------------
14.13.10: CXL HDM Decoder Zero Size Commit

Test Steps:
  1. Program 0 in the Decoder[m].Size register.
  2. Set the Commit bit in the Decoder[m].Control register.

Pass Criteria:
  • Committed bit in the Decoder[m].Control register is set
  • Error Not Committed bit in the Decoder[].Control register is not set

Fail Conditions:
  • Committed bit in the Decoder[m].Control register is not set within 10 ms
  • Error Not Committed bit in the Decoder[m].Control register is set

------------------------------------------
14.13.9 CXL HDM Decoder Commit

Test Steps:
  1. Program an address range in the Decoder[m+1].Base and Decoder[m+1].Size
     registers such that:

     — Decoder[m+1].Base >= (Decoder[m].Base+Decoder[m].Size), and
     — Decoder[m+1].Base <= Decoder[m+1].Base+Decoder[m+1].Size

Re: [PATCH v1 1/2] cxl_test: enable zero sized decoders under hb0

[Gregory Price]

On Tue, Oct 21, 2025 at 10:03:22AM -0400, Gregory Price wrote:
> 
> Re commit order - Vishal said they do post-lock order
> https://lore.kernel.org/linux-cxl/aOP3Kr3jHLWOydRp@gourry-fedora-PF4VCD3F/
> 
> Post-lock order
> [programmable] [zero-lock] [zero-lock] ... [zero-lock]
> 
> Pre-lock order:
> [zero-lock] [zero-lock] ... [zero-lock] [programmable]
> 
> My reading of the spec suggests that Post-lock ordering is *not legal*,
> and would suggest the software has deviated from the spec - which does
> not allow for size-zero decoders to ignore commit order.
> 

Revisiting this - it might be reasonable to allow post-lock ordering if
*all* decoders in beyond the programmable one are zero-locked.

Seems ok? Thoughts?

~Gregory

Re: [PATCH v1 1/2] cxl_test: enable zero sized decoders under hb0

[Gregory Price]

On Wed, Feb 11, 2026 at 10:04:41AM -0500, Gregory Price wrote:
> Revisiting this - it might be reasonable to allow post-lock ordering if
> *all* decoders in beyond the programmable one are zero-locked.
> 
> Seems ok? Thoughts?
> 

Sorry, got myself a bit mixed up, lets me a bit more precise, there are
annoying corner conditions here that make reasoning about this
difficult, and the mild ambiguity in the spec doesn't imbue confidence.

[open] = programmable
[programmed] = usually auto-decoder, but some cases may allow runtime
               programming, i haven't logic'd that out.


                    Post-lock   Legal
decoder        0             1             2     ...     N
------------------------------------------------------------------
            [open]       [zero-lock]  [zero-lock]   [zero-lock]
	    [open]         [open]     [zero-lock]   [zero-lock]
	  [programmed]     [open]     [zero-lock]   [zero-lock]
	  [programmed]  [programmed]  [zero-lock]   [zero-lock]


In these cases you basically act as-if the zero-locked decoders
essentially don't exist.



                   Pre-lock    Legal
decoder        0             1             2     ...     N
------------------------------------------------------------------
	  [zero-lock]    [zero-lock]  [zero-lock]      [open]
	  [zero-lock]    [zero-lock]  [zero-lock]    [programmed]
	  [zero-lock]    [zero-lock]  [programmed]     [open]
	  [zero-lock]    [zero-lock]  [programmed]   [programmed]

Again, in these causes you just act like the decoders don't exist.


                           Illegal
decoder        0              1             2      ...     N
------------------------------------------------------------------
            [open]       [zero-lock]      [open]      [zero-lock]
          [programmed]   [zero-lock]      [open]      [zero-lock]
            [open]       [zero-lock]   [programmed]   [zero-lock]
          [zero-lock]       [open]      [zero-lock]   [zero-lock]
          [zero-lock]       [open]      [zero-lock]     [open]
          [zero-lock]       [open]      [zero-lock]     [open]
          [zero-lock]       [open]      [zero-lock]   [programmed]

These cases are all illegal because there is always some form of
decoder committed out-of-order in the presence of at least one locked
decoder (the zero-locks).


                           Questionable
decoder        0              1             2      ...     N
------------------------------------------------------------------
	  [zero-lock]    [programmed]   [zero-lock]    [zero-lock]
          [programmed]   [zero-lock]    [programmed]   [zero-lock]
	  [zero-lock]    [programmed]   [zero-lock]    [programmed]
******    [zero-lock]    [programmed]   [zero-lock]  [open]...[open]


In these cases, it's only really legal IFF the programmed decoder(s) are
auto-decoders, because it means an out-of-order condition is impossible.

I think you can validate this on probe, but it's annoying

if zero-lock decoder is present
	a) all decoders prior to the zero-lock are locked (zero | auto)
	b) all decoders after the zero-lock are locked*

* if an open decoder appears after a zero-lock, all decoders after
  the open decoder must also be open. (supports corner case above)


I... think that works?  It's annoying though lol.

~Gregory