[ndctl PATCH 0/7] cxl/region: create-region command and unit

[ndctl PATCH 0/7] cxl/region: create-region command and unit

[Alison Schofield]

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.

Patches 1-2 consolidate and clean up the existing create-region tests.

Patches 3-6 fix target-list parsing and validation, release DPA after a
failed create, and move granularity validation ahead of region creation.
Each fix adds a test case for the problem it addresses.

Patch 7 corrects the documentation to reflect that --decoder is currently
required. Automatic root decoder selection will be considered separately
with the upcoming create-region usability work.

Developing tests for these fixes exposed two bugs and a topology gap in
the cxl_test mock module. A companion cxl_test series is here [1].


[1] https://lore.kernel.org/linux-cxl/cover.1790641236.git.alison.schofield@intel.com/

Alison Schofield (7):
  test/cxl-create-region.sh: fold in XOR coverage
  test/cxl-create-region.sh: deduplicate topology lookups
  cxl/region: parse comma-separated target lists
  cxl/region: reject duplicate or unknown target memdevs
  cxl/region: release reserved DPA when region creation fails
  cxl/region: validate granularity before region creation
  cxl/region: document --decoder as required

 Documentation/cxl/cxl-create-region.txt |   7 +-
 cxl/region.c                            | 139 ++++++++--
 test/cxl-create-region.sh               | 338 ++++++++++++++++++++++--
 test/cxl-xor-region.sh                  | 129 ---------
 test/meson.build                        |   2 -
 5 files changed, 442 insertions(+), 173 deletions(-)
 delete mode 100644 test/cxl-xor-region.sh

-- 
2.37.3

[ndctl PATCH 1/7] test/cxl-create-region.sh: fold in XOR coverage

[Alison Schofield]

Region creation coverage is split between cxl-create-region.sh and
cxl-xor-region.sh.

Fold the XOR cases into cxl-create-region.sh so cxl create-region
coverage can share one test harness. Remove cxl-xor-region.sh.

Assisted-by: LLM
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 test/cxl-create-region.sh | 108 +++++++++++++++++++++++++++++++
 test/cxl-xor-region.sh    | 129 --------------------------------------
 test/meson.build          |   2 -
 3 files changed, 108 insertions(+), 131 deletions(-)
 delete mode 100644 test/cxl-xor-region.sh

diff --git a/test/cxl-create-region.sh b/test/cxl-create-region.sh
index 658b9b8ff58a..fcfcea0594bb 100644
--- a/test/cxl-create-region.sh
+++ b/test/cxl-create-region.sh
@@ -133,6 +133,98 @@ create_single()
 	destroy_regions "$region"
 }
 
+create_and_destroy_region()
+{
+	region=$($CXL create-region -d "$decoder" -m "$memdevs" |
+		jq -r ".region")
+
+	if [[ ! $region ]]; then
+		echo "create-region failed for $decoder"
+		err "$LINENO"
+	fi
+
+	destroy_regions "$region"
+}
+
+setup_x1()
+{
+	# Find an x1 decoder
+	decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] |
+		select(.pmem_capable == true) |
+		select(.nr_targets == 1) |
+		.decoder")
+
+	# Find a memdev for this host-bridge
+	port_dev0=$($CXL list -T -d "$decoder" | jq -r ".[] |
+		.targets | .[] | select(.position == 0) | .target")
+	mem0=$($CXL list -M -p "$port_dev0" | jq -r ".[0].memdev")
+	memdevs="$mem0"
+}
+
+setup_x2()
+{
+	# Find an x2 decoder
+	decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] |
+		select(.pmem_capable == true) |
+		select(.nr_targets == 2) |
+		.decoder")
+
+	# Find a memdev for each host-bridge interleave position
+	port_dev0=$($CXL list -T -d "$decoder" | jq -r ".[] |
+		.targets | .[] | select(.position == 0) | .target")
+	port_dev1=$($CXL list -T -d "$decoder" | jq -r ".[] |
+		.targets | .[] | select(.position == 1) | .target")
+	mem0=$($CXL list -M -p "$port_dev0" | jq -r ".[0].memdev")
+	mem1=$($CXL list -M -p "$port_dev1" | jq -r ".[0].memdev")
+	memdevs="$mem0 $mem1"
+}
+
+setup_x4()
+{
+	# find an x2 decoder
+	decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] |
+		select(.pmem_capable == true) |
+		select(.nr_targets == 2) |
+		.decoder")
+
+	# Find a memdev for each host-bridge interleave position
+	port_dev0=$($CXL list -T -d "$decoder" | jq -r ".[] |
+		.targets | .[] | select(.position == 0) | .target")
+	port_dev1=$($CXL list -T -d "$decoder" | jq -r ".[] |
+		.targets | .[] | select(.position == 1) | .target")
+	mem0=$($CXL list -M -p "$port_dev0" | jq -r ".[0].memdev")
+	mem1=$($CXL list -M -p "$port_dev1" | jq -r ".[0].memdev")
+	mem2=$($CXL list -M -p "$port_dev0" | jq -r ".[1].memdev")
+	mem3=$($CXL list -M -p "$port_dev1" | jq -r ".[1].memdev")
+	memdevs="$mem0 $mem1 $mem2 $mem3"
+}
+
+setup_x3()
+{
+	# find an x3 decoder
+	decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] |
+		select(.pmem_capable == true) |
+		select(.nr_targets == 3) |
+		.decoder")
+
+	if [[ ! $decoder ]]; then
+		echo "no x3 decoder found, skipping xor-x3 test"
+		return
+	fi
+
+	# Find a memdev for each host-bridge interleave position
+	port_dev0=$($CXL list -T -d "$decoder" | jq -r ".[] |
+		.targets | .[] | select(.position == 0) | .target")
+	port_dev1=$($CXL list -T -d "$decoder" | jq -r ".[] |
+		.targets | .[] | select(.position == 1) | .target")
+	port_dev2=$($CXL list -T -d "$decoder" | jq -r ".[] |
+		.targets | .[] | select(.position == 2) | .target")
+	mem0=$($CXL list -M -p "$port_dev0" | jq -r ".[0].memdev")
+	mem1=$($CXL list -M -p "$port_dev1" | jq -r ".[0].memdev")
+	mem2=$($CXL list -M -p "$port_dev2" | jq -r ".[0].memdev")
+	memdevs="$mem0 $mem1 $mem2"
+}
+
 # test region creation on devices behind a single-port host-bridge
 create_single
 
@@ -149,6 +241,22 @@ for mem in ${mems[@]}; do
 	create_subregions "$mem"
 done
 
+# Repeat x1/x2/x3/x4 region creation with XOR interleave arithmetic.
+modprobe -r cxl_test
+modprobe cxl_test interleave_arithmetic=1
+
+setup_x1
+create_and_destroy_region
+setup_x2
+create_and_destroy_region
+setup_x4
+create_and_destroy_region
+# x3 decoder may not be available in cxl/test topo yet
+setup_x3
+if [[ $decoder ]]; then
+	create_and_destroy_region
+fi
+
 check_dmesg "$LINENO"
 
 modprobe -r cxl_test
diff --git a/test/cxl-xor-region.sh b/test/cxl-xor-region.sh
deleted file mode 100644
index fb4f9a0a1515..000000000000
--- a/test/cxl-xor-region.sh
+++ /dev/null
@@ -1,129 +0,0 @@
-#!/bin/bash
-# SPDX-License-Identifier: GPL-2.0
-# Copyright (C) 2022 Intel Corporation. All rights reserved.
-
-. $(dirname $0)/common
-
-rc=77
-
-set -ex
-
-trap 'err $LINENO' ERR
-
-check_prereq "jq"
-
-modprobe -r cxl_test
-modprobe cxl_test interleave_arithmetic=1
-rc=1
-
-# THEORY OF OPERATION: Create x1,2,3,4 regions to exercise the XOR math
-# option of the CXL driver. As with other cxl_test tests, changes to the
-# CXL topology in tools/testing/cxl/test/cxl.c may require an update here.
-
-create_and_destroy_region()
-{
-	region=$($CXL create-region -d "$decoder" -m "$memdevs" |
-		jq -r ".region")
-
-	if [[ ! $region ]]; then
-		echo "create-region failed for $decoder"
-		err "$LINENO"
-	fi
-
-	$CXL destroy-region -f -b cxl_test "$region"
-}
-
-setup_x1()
-{
-	# Find an x1 decoder
-	decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] |
-		select(.pmem_capable == true) |
-		select(.nr_targets == 1) |
-		.decoder")
-
-	# Find a memdev for this host-bridge
-	port_dev0=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 0) | .target")
-	mem0=$($CXL list -M -p "$port_dev0" | jq -r ".[0].memdev")
-	memdevs="$mem0"
-}
-
-setup_x2()
-{
-	# Find an x2 decoder
-	decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] |
-		select(.pmem_capable == true) |
-		select(.nr_targets == 2) |
-		.decoder")
-
-	# Find a memdev for each host-bridge interleave position
-	port_dev0=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 0) | .target")
-	port_dev1=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 1) | .target")
-	mem0=$($CXL list -M -p "$port_dev0" | jq -r ".[0].memdev")
-	mem1=$($CXL list -M -p "$port_dev1" | jq -r ".[0].memdev")
-	memdevs="$mem0 $mem1"
-}
-
-setup_x4()
-{
-	# find an x2 decoder
-	decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] |
-		select(.pmem_capable == true) |
-		select(.nr_targets == 2) |
-		.decoder")
-
-	# Find a memdev for each host-bridge interleave position
-	port_dev0=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 0) | .target")
-	port_dev1=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 1) | .target")
-	mem0=$($CXL list -M -p "$port_dev0" | jq -r ".[0].memdev")
-	mem1=$($CXL list -M -p "$port_dev1" | jq -r ".[0].memdev")
-	mem2=$($CXL list -M -p "$port_dev0" | jq -r ".[1].memdev")
-	mem3=$($CXL list -M -p "$port_dev1" | jq -r ".[1].memdev")
-	memdevs="$mem0 $mem1 $mem2 $mem3"
-}
-
-setup_x3()
-{
-	# find an x3 decoder
-	decoder=$($CXL list -b cxl_test -D -d root | jq -r ".[] |
-		select(.pmem_capable == true) |
-		select(.nr_targets == 3) |
-		.decoder")
-
-	if [[ ! $decoder ]]; then
-		echo "no x3 decoder found, skipping xor-x3 test"
-		return
-	fi
-
-	# Find a memdev for each host-bridge interleave position
-	port_dev0=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 0) | .target")
-	port_dev1=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 1) | .target")
-	port_dev2=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 2) | .target")
-	mem0=$($CXL list -M -p "$port_dev0" | jq -r ".[0].memdev")
-	mem1=$($CXL list -M -p "$port_dev1" | jq -r ".[0].memdev")
-	mem2=$($CXL list -M -p "$port_dev2" | jq -r ".[0].memdev")
-	memdevs="$mem0 $mem1 $mem2"
-}
-
-setup_x1
-create_and_destroy_region
-setup_x2
-create_and_destroy_region
-setup_x4
-create_and_destroy_region
-# x3 decoder may not be available in cxl/test topo yet
-setup_x3
-if [[ $decoder ]]; then
-	create_and_destroy_region
-fi
-
-check_dmesg "$LINENO"
-
-modprobe -r cxl_test
diff --git a/test/meson.build b/test/meson.build
index 07c8618cbc6e..d6cd2a3d4b10 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -162,7 +162,6 @@ cxl_topo = find_program('cxl-topology.sh')
 cxl_sysfs = find_program('cxl-region-sysfs.sh')
 cxl_labels = find_program('cxl-labels.sh')
 cxl_create_region = find_program('cxl-create-region.sh')
-cxl_xor_region = find_program('cxl-xor-region.sh')
 cxl_update_firmware = find_program('cxl-update-firmware.sh')
 cxl_events = find_program('cxl-events.sh')
 cxl_sanitize = find_program('cxl-sanitize.sh')
@@ -201,7 +200,6 @@ tests = [
   [ 'cxl-region-sysfs.sh',    cxl_sysfs,	  'cxl'   ],
   [ 'cxl-labels.sh',          cxl_labels,	  'cxl'   ],
   [ 'cxl-create-region.sh',   cxl_create_region,  'cxl'   ],
-  [ 'cxl-xor-region.sh',      cxl_xor_region,     'cxl'   ],
   [ 'cxl-events.sh',          cxl_events,         'cxl'   ],
   [ 'cxl-sanitize.sh',        cxl_sanitize,       'cxl'   ],
   [ 'cxl-destroy-region.sh',  cxl_destroy_region, 'cxl'   ],
-- 
2.37.3

[ndctl PATCH 2/7] test/cxl-create-region.sh: deduplicate topology lookups

[Alison Schofield]

Several region setup paths repeat the same decoder and memdev lookups.

Factor them into shared helpers and centralize cxl_test reload and
memdev discovery. No functional change.

Assisted-by: LLM
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 test/cxl-create-region.sh | 99 +++++++++++++++++++--------------------
 1 file changed, 48 insertions(+), 51 deletions(-)

diff --git a/test/cxl-create-region.sh b/test/cxl-create-region.sh
index fcfcea0594bb..927edf4c17d4 100644
--- a/test/cxl-create-region.sh
+++ b/test/cxl-create-region.sh
@@ -12,8 +12,14 @@ trap 'err $LINENO' ERR
 
 check_prereq "jq"
 
-modprobe -r cxl_test
-modprobe cxl_test
+load_cxl_test()
+{
+	modprobe -r cxl_test
+	modprobe cxl_test "$@"
+	readarray -t mems < <("$CXL" list -b cxl_test -M | jq -r '.[].memdev')
+}
+
+load_cxl_test
 rc=1
 
 destroy_regions()
@@ -25,17 +31,27 @@ destroy_regions()
 	fi
 }
 
+# List x1 root decoders for a memdev matching the jq predicate.
+find_x1_decoders()
+{
+	$CXL list -b cxl_test -D -d root -m "$1" |
+		  jq -r ".[] |
+		  select($2) |
+		  select(.nr_targets == 1) |
+		  .decoder"
+}
+
+# Pick the first pmem-capable x1 root decoder for a memdev.
+find_x1_decoder()
+{
+	find_x1_decoders "$1" ".pmem_capable == true" | head -n1
+}
+
 create_x1_region()
 {
 	mem="$1"
 
-	# find a pmem capable root decoder for this mem
-	decoder=$($CXL list -b cxl_test -D -d root -m "$mem" |
-		  jq -r ".[] |
-		  select(.pmem_capable == true) |
-		  select(.nr_targets == 1) |
-		  .decoder")
-
+	decoder=$(find_x1_decoder "$mem")
 	if [[ ! $decoder ]]; then
 		echo "no suitable decoder found for $mem, skipping"
 		return
@@ -69,13 +85,7 @@ create_subregions()
 	slice=$((256 << 20))
 	mem="$1"
 
-	# find a pmem capable root decoder for this mem
-	decoder=$($CXL list -b cxl_test -D -d root -m "$mem" |
-		  jq -r ".[] |
-		  select(.pmem_capable == true) |
-		  select(.nr_targets == 1) |
-		  .decoder")
-
+	decoder=$(find_x1_decoder "$mem")
 	if [[ ! $decoder ]]; then
 		echo "no suitable decoder found for $mem, skipping"
 		return
@@ -133,6 +143,17 @@ create_single()
 	destroy_regions "$region"
 }
 
+# Return memdev @index behind root target @position.
+find_memdev()
+{
+	local decoder="$1" position="$2" index="$3"
+	local port_dev
+
+	port_dev=$($CXL list -T -d "$decoder" | jq -r ".[] |
+		.targets | .[] | select(.position == $position) | .target")
+	$CXL list -M -p "$port_dev" | jq -r ".[$index].memdev"
+}
+
 create_and_destroy_region()
 {
 	region=$($CXL create-region -d "$decoder" -m "$memdevs" |
@@ -154,10 +175,7 @@ setup_x1()
 		select(.nr_targets == 1) |
 		.decoder")
 
-	# Find a memdev for this host-bridge
-	port_dev0=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 0) | .target")
-	mem0=$($CXL list -M -p "$port_dev0" | jq -r ".[0].memdev")
+	mem0=$(find_memdev "$decoder" 0 0)
 	memdevs="$mem0"
 }
 
@@ -169,13 +187,8 @@ setup_x2()
 		select(.nr_targets == 2) |
 		.decoder")
 
-	# Find a memdev for each host-bridge interleave position
-	port_dev0=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 0) | .target")
-	port_dev1=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 1) | .target")
-	mem0=$($CXL list -M -p "$port_dev0" | jq -r ".[0].memdev")
-	mem1=$($CXL list -M -p "$port_dev1" | jq -r ".[0].memdev")
+	mem0=$(find_memdev "$decoder" 0 0)
+	mem1=$(find_memdev "$decoder" 1 0)
 	memdevs="$mem0 $mem1"
 }
 
@@ -187,15 +200,10 @@ setup_x4()
 		select(.nr_targets == 2) |
 		.decoder")
 
-	# Find a memdev for each host-bridge interleave position
-	port_dev0=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 0) | .target")
-	port_dev1=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 1) | .target")
-	mem0=$($CXL list -M -p "$port_dev0" | jq -r ".[0].memdev")
-	mem1=$($CXL list -M -p "$port_dev1" | jq -r ".[0].memdev")
-	mem2=$($CXL list -M -p "$port_dev0" | jq -r ".[1].memdev")
-	mem3=$($CXL list -M -p "$port_dev1" | jq -r ".[1].memdev")
+	mem0=$(find_memdev "$decoder" 0 0)
+	mem1=$(find_memdev "$decoder" 1 0)
+	mem2=$(find_memdev "$decoder" 0 1)
+	mem3=$(find_memdev "$decoder" 1 1)
 	memdevs="$mem0 $mem1 $mem2 $mem3"
 }
 
@@ -212,25 +220,15 @@ setup_x3()
 		return
 	fi
 
-	# Find a memdev for each host-bridge interleave position
-	port_dev0=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 0) | .target")
-	port_dev1=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 1) | .target")
-	port_dev2=$($CXL list -T -d "$decoder" | jq -r ".[] |
-		.targets | .[] | select(.position == 2) | .target")
-	mem0=$($CXL list -M -p "$port_dev0" | jq -r ".[0].memdev")
-	mem1=$($CXL list -M -p "$port_dev1" | jq -r ".[0].memdev")
-	mem2=$($CXL list -M -p "$port_dev2" | jq -r ".[0].memdev")
+	mem0=$(find_memdev "$decoder" 0 0)
+	mem1=$(find_memdev "$decoder" 1 0)
+	mem2=$(find_memdev "$decoder" 2 0)
 	memdevs="$mem0 $mem1 $mem2"
 }
 
 # test region creation on devices behind a single-port host-bridge
 create_single
 
-# test reading labels directly through cxl-cli
-readarray -t mems < <("$CXL" list -b cxl_test -M | jq -r '.[].memdev')
-
 for mem in ${mems[@]}; do
 	create_x1_region "$mem"
 done
@@ -242,8 +240,7 @@ for mem in ${mems[@]}; do
 done
 
 # Repeat x1/x2/x3/x4 region creation with XOR interleave arithmetic.
-modprobe -r cxl_test
-modprobe cxl_test interleave_arithmetic=1
+load_cxl_test interleave_arithmetic=1
 
 setup_x1
 create_and_destroy_region
-- 
2.37.3

[ndctl PATCH 3/7] cxl/region: parse comma-separated target lists

[Alison Schofield]

A comma-separated target list can be miscounted, causing cxl create-region
to reject a valid multi-target request when --ways is omitted. The
generated filter can also acquire a trailing comma.

This happens because the separator is re-evaluated after tokenization has
already modified the input.

Choose the separator before tokenizing and use the target count when
emitting the flattened list.

Fixes: 3d6cd829ec08 ("cxl/region: Use cxl_filter_walk() to gather create-region targets")
Assisted-by: LLM
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 cxl/region.c              | 21 +++++++-----
 test/cxl-create-region.sh | 72 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 84 insertions(+), 9 deletions(-)

diff --git a/cxl/region.c b/cxl/region.c
index 85d4d9bb54f2..43c228f2f999 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -124,24 +124,25 @@ static const struct option destroy_options[] = {
 static const char *to_csv(int *count, const char **strings)
 {
 	ssize_t len = 0, cursor = 0;
+	int i, new_count = 0, emitted = 0;
 	char *csv, *list, *save;
-	int i, new_count = 0;
-	const char *arg;
+	const char *arg, *sep;
 
 	if (!*count)
 		return NULL;
 
 	for (i = 0; i < *count; i++) {
 		/*
-		 * An entry in strings may itself by a space or comma
-		 * separated list, so decompose that for the final csv
+		 * An entry may itself be space- or comma-separated. strtok_r()
+		 * modifies @list, so choose the separator before tokenizing.
 		 */
 		list = strdup(strings[i]);
 		if (!list)
 			return NULL;
 
-		for (arg = strtok_r(list, which_sep(list), &save); arg;
-		     arg = strtok_r(NULL, which_sep(list), &save)) {
+		sep = which_sep(list);
+		for (arg = strtok_r(list, sep, &save); arg;
+		     arg = strtok_r(NULL, sep, &save)) {
 			len += strlen(arg);
 			new_count++;
 		}
@@ -161,10 +162,12 @@ static const char *to_csv(int *count, const char **strings)
 			return NULL;
 		}
 
-		for (arg = strtok_r(list, which_sep(list), &save); arg;
-		     arg = strtok_r(NULL, which_sep(list), &save)) {
+		sep = which_sep(list);
+		for (arg = strtok_r(list, sep, &save); arg;
+		     arg = strtok_r(NULL, sep, &save)) {
 			cursor += snprintf(csv + cursor, len - cursor, "%s%s",
-					   arg, i + 1 < new_count ? "," : "");
+					   arg,
+					   ++emitted < new_count ? "," : "");
 			if (cursor >= len) {
 				csv[len - 1] = 0;
 				break;
diff --git a/test/cxl-create-region.sh b/test/cxl-create-region.sh
index 927edf4c17d4..6fc845c7d382 100644
--- a/test/cxl-create-region.sh
+++ b/test/cxl-create-region.sh
@@ -226,6 +226,76 @@ setup_x3()
 	memdevs="$mem0 $mem1 $mem2"
 }
 
+# Build a 4-way target list over an x2 root in interleave position order.
+setup_interleave_shape()
+{
+	x2_decoder=$($CXL list -b cxl_test -D -d root |
+		     jq -r ".[] |
+		     select(.pmem_capable == true) |
+		     select(.nr_targets == 2) |
+		     .decoder")
+
+	if [[ ! $x2_decoder ]]; then
+		echo "no pmem capable x2 root decoder found"
+		return 1
+	fi
+
+	pos0=$(find_memdev "$x2_decoder" 0 0)
+	pos1=$(find_memdev "$x2_decoder" 1 0)
+	pos2=$(find_memdev "$x2_decoder" 0 1)
+	pos3=$(find_memdev "$x2_decoder" 1 1)
+
+	if [[ ! $pos0 || ! $pos1 || ! $pos2 || ! $pos3 ]]; then
+		echo "$x2_decoder: no memdev pair for each interleave position"
+		return 1
+	fi
+}
+
+# A comma-separated target list must preserve its count and order.
+create_csv_targets()
+{
+	if ! setup_interleave_shape; then
+		echo "skipping comma separated target list"
+		return
+	fi
+
+	if ! json=$($CXL create-region -b cxl_test -d "$x2_decoder" \
+		    -m "$pos0,$pos1,$pos2,$pos3"); then
+		echo "$x2_decoder: refused $pos0,$pos1,$pos2,$pos3"
+		err "$LINENO"
+	fi
+
+	region=$(jq -r ".region" <<< "$json")
+	ways=$(jq -r ".interleave_ways" <<< "$json")
+	if [[ $ways -ne 4 ]]; then
+		echo "$region: interleaves $ways ways, expected 4"
+		err "$LINENO"
+	fi
+
+	targets=$(jq -r "[.mappings | sort_by(.position) | .[].memdev] |
+		  join(\" \")" <<< "$json")
+	if [[ "$targets" != "$pos0 $pos1 $pos2 $pos3" ]]; then
+		echo "$region: targets '$targets', expected '$pos0 $pos1 $pos2 $pos3'"
+		err "$LINENO"
+	fi
+
+	destroy_regions "$region"
+
+	# Also verify a rejected filter is echoed without a trailing comma.
+	if out=$($CXL create-region -b cxl_test -d "$x2_decoder" \
+		 -m nomemdev0,nomemdev1,nomemdev2 2>&1); then
+		echo "create-region accepted names that are not memdevs"
+		err "$LINENO"
+	fi
+
+	if ! grep -Fq -- "filter: nomemdev0,nomemdev1,nomemdev2" <<< "$out" ||
+	   grep -Fq -- "nomemdev2," <<< "$out"; then
+		echo "target list echoed back as:"
+		echo "$out"
+		err "$LINENO"
+	fi
+}
+
 # test region creation on devices behind a single-port host-bridge
 create_single
 
@@ -239,6 +309,8 @@ for mem in ${mems[@]}; do
 	create_subregions "$mem"
 done
 
+create_csv_targets
+
 # Repeat x1/x2/x3/x4 region creation with XOR interleave arithmetic.
 load_cxl_test interleave_arithmetic=1
 
-- 
2.37.3

[ndctl PATCH 4/7] cxl/region: reject duplicate or unknown target memdevs

[Alison Schofield]

Target filtering silently drops unknown names and collapses overlapping
matches. cxl create-region can therefore build a smaller region than the
target list requested, or report only a ways mismatch.

Validate the target list against the resolved memdevs. Reject unmatched
targets and memdevs claimed by more than one target filter, while allowing
one filter to select multiple memdevs.

Fixes: 3d6cd829ec08 ("cxl/region: Use cxl_filter_walk() to gather create-region targets")
Assisted-by: LLM
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 cxl/region.c              | 65 +++++++++++++++++++++++++++++++++++++++
 test/cxl-create-region.sh | 45 +++++++++++++++++++++++++++
 2 files changed, 110 insertions(+)

diff --git a/cxl/region.c b/cxl/region.c
index 43c228f2f999..2bb3b90940b0 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -222,6 +222,66 @@ static int memdev_sort(const void *a, const void *b)
 	return a_pos - b_pos;
 }
 
+/*
+ * Reject unmatched target names and overlapping target filters.
+ * A single filter may match multiple memdevs.
+ */
+static bool validate_target_list(struct json_object *jmemdevs, const char *csv)
+{
+	int i, nr_memdevs = json_object_array_length(jmemdevs);
+	const char *arg, *sep = which_sep(csv);
+	struct json_object *jobj;
+	struct cxl_memdev *memdev;
+	bool valid = true;
+	char *list, *save;
+	int *claims;
+
+	claims = calloc(nr_memdevs, sizeof(*claims));
+	if (!claims)
+		return false;
+
+	list = strdup(csv);
+	if (!list) {
+		free(claims);
+		return false;
+	}
+
+	for (arg = strtok_r(list, sep, &save); arg;
+	     arg = strtok_r(NULL, sep, &save)) {
+		int matched = 0;
+
+		for (i = 0; i < nr_memdevs; i++) {
+			jobj = json_object_array_get_idx(jmemdevs, i);
+			memdev = json_object_get_userdata(jobj);
+			if (!util_cxl_memdev_filter(memdev, arg, NULL))
+				continue;
+			claims[i]++;
+			matched++;
+		}
+
+		if (!matched) {
+			log_err(&rl, "no active memdev matches target '%s'\n",
+				arg);
+			valid = false;
+		}
+	}
+
+	for (i = 0; i < nr_memdevs; i++) {
+		if (claims[i] < 2)
+			continue;
+		jobj = json_object_array_get_idx(jmemdevs, i);
+		memdev = json_object_get_userdata(jobj);
+		log_err(&rl, "%s is named %d times in the target list\n",
+			cxl_memdev_get_devname(memdev), claims[i]);
+		valid = false;
+	}
+
+	free(list);
+	free(claims);
+
+	return valid;
+}
+
 static struct json_object *collect_memdevs(struct cxl_ctx *ctx,
 					   const char *decoder, int *count,
 					   const char **mems)
@@ -251,6 +311,11 @@ static struct json_object *collect_memdevs(struct cxl_ctx *ctx,
 	}
 
 	if (csv) {
+		if (!validate_target_list(jmemdevs, csv)) {
+			json_object_put(jmemdevs);
+			jmemdevs = NULL;
+			goto out;
+		}
 		sort_context.csv = csv,
 		json_object_array_sort(jmemdevs, memdev_sort);
 	}
diff --git a/test/cxl-create-region.sh b/test/cxl-create-region.sh
index 6fc845c7d382..6ffe36b852a2 100644
--- a/test/cxl-create-region.sh
+++ b/test/cxl-create-region.sh
@@ -296,6 +296,50 @@ create_csv_targets()
 	fi
 }
 
+list_regions()
+{
+	$CXL list -b cxl_test -R | jq -r ".[]? | .region" | sort | tr "\n" " "
+}
+
+# Verify failure occurs without creating a region and reports @expect.
+create_expect_fail()
+{
+	expect="$1"
+	shift
+
+	regions_before="$(list_regions)"
+
+	if out=$($CXL create-region -b cxl_test "$@" 2>&1); then
+		echo "create-region $*: expected failure, created a region"
+		err "$LINENO"
+	fi
+
+	if ! grep -Fq -- "$expect" <<< "$out"; then
+		echo "create-region $*: expected '$expect', got:"
+		echo "$out"
+		err "$LINENO"
+	fi
+
+	if [[ "$(list_regions)" != "$regions_before" ]]; then
+		echo "create-region $*: refused, but left a region behind"
+		err "$LINENO"
+	fi
+}
+
+refuse_bad_target_list()
+{
+	if ! setup_interleave_shape; then
+		echo "skipping bad target list refusals"
+		return
+	fi
+
+	create_expect_fail "$pos0 is named 2 times in the target list" \
+		-d "$x2_decoder" -m "$pos0,$pos0"
+
+	create_expect_fail "no active memdev matches target 'nomemdev'" \
+		-d "$x2_decoder" -m "$pos0,nomemdev"
+}
+
 # test region creation on devices behind a single-port host-bridge
 create_single
 
@@ -310,6 +354,7 @@ for mem in ${mems[@]}; do
 done
 
 create_csv_targets
+refuse_bad_target_list
 
 # Repeat x1/x2/x3/x4 region creation with XOR interleave arithmetic.
 load_cxl_test interleave_arithmetic=1
-- 
2.37.3

[ndctl PATCH 5/7] cxl/region: release reserved DPA when region creation fails

[Alison Schofield]

A cxl create-region failure after reserving endpoint DPA deletes the
region but leaves those reservations behind. Later creates can fail with
ENOSPC on capacity stranded by the earlier attempt.

Track reservations made by the current create and release them after
deleting the failed region.

Fixes: 21b089025178 ("cxl: add a 'create-region' command")
Assisted-by: LLM
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 cxl/region.c              | 35 ++++++++++++++++++++++++--
 test/cxl-create-region.sh | 52 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/cxl/region.c b/cxl/region.c
index 2bb3b90940b0..e4fc8bb2942c 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -712,14 +712,30 @@ static int cxl_region_determine_granularity(struct cxl_region *region,
 	return -EINVAL;
 }
 
+/* Delete the region before releasing its endpoint DPA. */
+static void unreserve_dpa(struct cxl_decoder **ep_decoders, int nr)
+{
+	int i, rc;
+
+	for (i = 0; i < nr; i++) {
+		rc = cxl_decoder_set_dpa_size(ep_decoders[i], 0);
+		if (rc)
+			log_err(&rl, "%s: failed to release dpa: %s\n",
+				cxl_decoder_get_devname(ep_decoders[i]),
+				strerror(abs(rc)));
+	}
+}
+
 static int create_region(struct cxl_ctx *ctx, int *count,
 			 struct parsed_params *p)
 {
 	unsigned long flags = UTIL_JSON_TARGETS;
+	struct cxl_decoder **ep_decoders;
 	struct json_object *jregion;
 	struct cxl_region *region;
 	bool default_size = true;
 	int i, rc, granularity;
+	int nr_reserved = 0;
 	u64 size, max_extent;
 	const char *devname;
 
@@ -775,6 +791,12 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 
 	devname = cxl_region_get_devname(region);
 
+	ep_decoders = calloc(p->ways, sizeof(*ep_decoders));
+	if (!ep_decoders) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
 	rc = cxl_region_determine_granularity(region, p);
 	if (rc < 0)
 		goto out;
@@ -810,6 +832,7 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 			try(cxl_decoder, set_mode, ep_decoder, p->mode);
 		}
 		try(cxl_decoder, set_dpa_size, ep_decoder, size/p->ways);
+		ep_decoders[nr_reserved++] = ep_decoder;
 		rc = cxl_region_set_target(region, i, ep_decoder);
 		if (rc) {
 			log_err(&rl, "%s: failed to set target%d to %s\n",
@@ -842,8 +865,16 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 
 out:
 	json_object_put(p->memdevs);
-	if (rc)
-		cxl_region_delete(region);
+	if (rc) {
+		int delete_rc = cxl_region_delete(region);
+
+		if (delete_rc)
+			log_err(&rl, "%s: failed to delete: %s\n", devname,
+				strerror(abs(delete_rc)));
+		else
+			unreserve_dpa(ep_decoders, nr_reserved);
+	}
+	free(ep_decoders);
 	return rc;
 }
 
diff --git a/test/cxl-create-region.sh b/test/cxl-create-region.sh
index 6ffe36b852a2..02358af7837a 100644
--- a/test/cxl-create-region.sh
+++ b/test/cxl-create-region.sh
@@ -340,6 +340,57 @@ refuse_bad_target_list()
 		-d "$x2_decoder" -m "$pos0,nomemdev"
 }
 
+# Snapshot endpoint DPA reservations.
+reserved_dpa()
+{
+	$CXL list -b cxl_test -D -d endpoint -i |
+	jq -r ".[] |
+	select((.dpa_size // 0) > 0) |
+	\"\(.decoder) \(.dpa_size)\"" | sort
+}
+
+# Fail after earlier targets reserve DPA and verify it is released.
+release_dpa_on_failure()
+{
+	if ! setup_interleave_shape; then
+		echo "skipping dpa release on failure"
+		return
+	fi
+
+	# an x1 window over position 2's host bridge, to fill it through
+	decoder=$(find_x1_decoder "$pos2")
+
+	if [[ ! $decoder ]]; then
+		echo "no x1 pmem decoder over $pos2, skipping dpa release"
+		return
+	fi
+
+	filler=$($CXL create-region -b cxl_test -d "$decoder" -m "$pos2" |
+		 jq -r ".region")
+	if [[ ! $filler ]]; then
+		echo "$decoder: unable to fill $pos2"
+		err "$LINENO"
+	fi
+
+	dpa_before="$(reserved_dpa)"
+
+	# Force the failure into endpoint DPA setup; default sizing would
+	# reject the request before reserving anything.
+	size=$((256 << 20))
+	create_expect_fail "set_dpa_size failed" \
+		-d "$x2_decoder" -m "$pos0 $pos1 $pos2 $pos3" \
+		-s $((size * 4))
+
+	dpa_after="$(reserved_dpa)"
+	if [[ "$dpa_after" != "$dpa_before" ]]; then
+		echo "dpa left reserved by the failed create:"
+		diff <(echo "$dpa_before") <(echo "$dpa_after") || true
+		err "$LINENO"
+	fi
+
+	destroy_regions "$filler"
+}
+
 # test region creation on devices behind a single-port host-bridge
 create_single
 
@@ -355,6 +406,7 @@ done
 
 create_csv_targets
 refuse_bad_target_list
+release_dpa_on_failure
 
 # Repeat x1/x2/x3/x4 region creation with XOR interleave arithmetic.
 load_cxl_test interleave_arithmetic=1
-- 
2.37.3

[ndctl PATCH 6/7] cxl/region: validate granularity before region creation

[Alison Schofield]

The root decoder and requested granularity are known before region
creation, so cxl create-region can determine whether the request is valid
before sending it to the kernel.

Currently that check happens only after the region device is created,
requiring the failed request to be cleaned up afterward.

Validate encodable values during option parsing and resolve the effective
granularity before creating the region.

Fixes: 21b089025178 ("cxl: add a 'create-region' command")
Assisted-by: LLM
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 cxl/region.c              | 22 ++++++++++++----------
 test/cxl-create-region.sh | 26 ++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/cxl/region.c b/cxl/region.c
index e4fc8bb2942c..2e62e3faace2 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -407,8 +407,11 @@ static int parse_create_options(struct cxl_ctx *ctx, int count,
 		p->ways = p->num_memdevs;
 
 	if (param.granularity < INT_MAX) {
-		if (param.granularity <= 0) {
-			log_err(&rl, "Invalid interleave granularity: %d\n",
+		/* HDM granularity is a power of two from 256B through 16KiB. */
+		if (param.granularity < 256 || param.granularity > 16384 ||
+		    !is_power_of_2(param.granularity)) {
+			log_err(&rl,
+				"Invalid interleave granularity: %d, expected a power of 2 from 256 to 16384\n",
 				param.granularity);
 			goto err;
 		}
@@ -670,10 +673,9 @@ do { \
 	} \
 } while (0)
 
-static int cxl_region_determine_granularity(struct cxl_region *region,
-					    struct parsed_params *p)
+static int cxl_region_determine_granularity(struct parsed_params *p)
 {
-	const char *devname = cxl_region_get_devname(region);
+	const char *devname = cxl_decoder_get_devname(p->root_decoder);
 	int granularity, ways;
 
 	/* Default granularity will be the root decoder's granularity */
@@ -769,6 +771,11 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 	if (size > max_extent)
 		size = ALIGN_DOWN(max_extent, SZ_256M * p->ways);
 
+	rc = cxl_region_determine_granularity(p);
+	if (rc < 0)
+		return rc;
+	granularity = rc;
+
 	if (p->mode == CXL_DECODER_MODE_PMEM) {
 		region = cxl_decoder_create_pmem_region(p->root_decoder);
 		if (!region) {
@@ -797,11 +804,6 @@ static int create_region(struct cxl_ctx *ctx, int *count,
 		goto out;
 	}
 
-	rc = cxl_region_determine_granularity(region, p);
-	if (rc < 0)
-		goto out;
-	granularity = rc;
-
 	try(cxl_region, set_interleave_granularity, region, granularity);
 	try(cxl_region, set_interleave_ways, region, p->ways);
 	if (p->mode == CXL_DECODER_MODE_PMEM) {
diff --git a/test/cxl-create-region.sh b/test/cxl-create-region.sh
index 02358af7837a..0d98b61a7e13 100644
--- a/test/cxl-create-region.sh
+++ b/test/cxl-create-region.sh
@@ -240,6 +240,16 @@ setup_interleave_shape()
 		return 1
 	fi
 
+	x2_granularity=$($CXL list -b cxl_test -D -d "$x2_decoder" |
+			 jq -r ".[0].interleave_granularity")
+
+	# Pick a valid granularity different from the x2 root's.
+	if [[ $x2_granularity -eq 256 ]]; then
+		other_granularity=512
+	else
+		other_granularity=256
+	fi
+
 	pos0=$(find_memdev "$x2_decoder" 0 0)
 	pos1=$(find_memdev "$x2_decoder" 1 0)
 	pos2=$(find_memdev "$x2_decoder" 0 1)
@@ -391,6 +401,21 @@ release_dpa_on_failure()
 	destroy_regions "$filler"
 }
 
+# Reject invalid encodings and granularities incompatible with the root.
+refuse_bad_granularity()
+{
+	if ! setup_interleave_shape; then
+		echo "skipping granularity refusals"
+		return
+	fi
+
+	create_expect_fail "Invalid interleave granularity: 1000" \
+		-d "$x2_decoder" -m "$pos0 $pos1" -g 1000
+
+	create_expect_fail "only root decoder granularity ($x2_granularity)" \
+		-d "$x2_decoder" -m "$pos0 $pos1" -g "$other_granularity"
+}
+
 # test region creation on devices behind a single-port host-bridge
 create_single
 
@@ -407,6 +432,7 @@ done
 create_csv_targets
 refuse_bad_target_list
 release_dpa_on_failure
+refuse_bad_granularity
 
 # Repeat x1/x2/x3/x4 region creation with XOR interleave arithmetic.
 load_cxl_test interleave_arithmetic=1
-- 
2.37.3

[ndctl PATCH 7/7] cxl/region: document --decoder as required

[Alison Schofield]

The cxl create-region documentation describes automatic root decoder
selection when --decoder is omitted, but that behavior has never been
implemented. Users see:

	# cxl create-region -m mem0 mem1
	cxl region: parse_create_options: no root decoder specified
	cxl region: cmd_create_region: created 0 regions

Document --decoder as required.

Assisted-by: LLM
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---

Automatic root decoder selection may be revisited as part of future
usability enhancements to the cxl create-region command.
---
 Documentation/cxl/cxl-create-region.txt | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
index b244af60b8a6..47c23ffb2565 100644
--- a/Documentation/cxl/cxl-create-region.txt
+++ b/Documentation/cxl/cxl-create-region.txt
@@ -94,16 +94,15 @@ include::bus-option.txt[]
 -g::
 --granularity=::
 	The interleave granularity for the new region. Must match the selected
-	root decoder's (if provided) granularity. If the root decoder is
+	root decoder's granularity. If the root decoder is
 	interleaved across more than one host-bridge then this value must match
 	that granularity. Otherwise, for non-interleaved decode windows, any
 	granularity can be specified as long as all devices support that setting.
 
 -d::
 --decoder=::
-	The root decoder that the region should be created under. If not
-	supplied, the first cross-host bridge (if available), decoder that
-	supports the largest interleave will be chosen.
+	The root decoder under which to create the region. This option is
+	required.
 
 -Q::
 --enforce-qos::
-- 
2.37.3

Re: [ndctl PATCH 0/7] cxl/region: create-region command and unit

[Alison Schofield]

That Subject: got truncated, is:
	cxl/region: create-region command and unit test fixups

On Mon, Sep 28, 2026 at 10:36:52PM -0700, Alison Schofield wrote:
> 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.
> 
> Patches 1-2 consolidate and clean up the existing create-region tests.
> 
> Patches 3-6 fix target-list parsing and validation, release DPA after a
> failed create, and move granularity validation ahead of region creation.
> Each fix adds a test case for the problem it addresses.
> 
> Patch 7 corrects the documentation to reflect that --decoder is currently
> required. Automatic root decoder selection will be considered separately
> with the upcoming create-region usability work.
> 
> Developing tests for these fixes exposed two bugs and a topology gap in
> the cxl_test mock module. A companion cxl_test series is here [1].
> 
> 
> [1] https://lore.kernel.org/linux-cxl/cover.1790641236.git.alison.schofield@intel.com/
> 
> Alison Schofield (7):
>   test/cxl-create-region.sh: fold in XOR coverage
>   test/cxl-create-region.sh: deduplicate topology lookups
>   cxl/region: parse comma-separated target lists
>   cxl/region: reject duplicate or unknown target memdevs
>   cxl/region: release reserved DPA when region creation fails
>   cxl/region: validate granularity before region creation
>   cxl/region: document --decoder as required
> 
>  Documentation/cxl/cxl-create-region.txt |   7 +-
>  cxl/region.c                            | 139 ++++++++--
>  test/cxl-create-region.sh               | 338 ++++++++++++++++++++++--
>  test/cxl-xor-region.sh                  | 129 ---------
>  test/meson.build                        |   2 -
>  5 files changed, 442 insertions(+), 173 deletions(-)
>  delete mode 100644 test/cxl-xor-region.sh
> 
> -- 
> 2.37.3
>