[ndctl PATCH 0/6] cxl/test: Add more region ABI validation

[ndctl PATCH 0/6] cxl/test: Add more region ABI validation

[Dan Williams]

Expand cxl-region-sysfs.sh to validate that all decoders in the test
region are setup per expectations. In support of that testing, decoder
listings gain 'interleave_ways', 'interleave_granularity', and 'region'
attributes, while port listings gain a 'depth' attribute. Decoders can
now be filtered by their associated region.

---

Dan Williams (6):
      cxl/test: Validate endpoint interleave geometry
      cxl/list: Add interleave parameters to decoder listings
      cxl/list: Add region to decoder listings
      cxl/list: Filter decoders by region
      cxl/list: Add 'depth' to port listings
      cxl/test: Validate switch port settings in cxl-region-sysfs.sh


 Documentation/cxl/lib/libcxl.txt |    8 ++++++
 cxl/filter.c                     |   23 +++++++++++++++++
 cxl/json.c                       |   32 +++++++++++++++++++++++
 cxl/lib/libcxl.c                 |   39 +++++++++++++++++++++++++++++
 cxl/lib/libcxl.sym               |    2 +
 cxl/libcxl.h                     |    2 +
 test/cxl-region-sysfs.sh         |   52 +++++++++++++++++++++++++++++++++++++-
 7 files changed, 156 insertions(+), 2 deletions(-)

[ndctl PATCH 1/6] cxl/test: Validate endpoint interleave geometry

[Dan Williams]

Check that endpoint interleave geometry settings are updated once the
endpoint decoders are associated with a region.

Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/cxl-region-sysfs.sh |   20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh
index 2582edb3f306..110e03709e39 100644
--- a/test/cxl-region-sysfs.sh
+++ b/test/cxl-region-sysfs.sh
@@ -44,8 +44,8 @@ uuidgen > /sys/bus/cxl/devices/$region/uuid
 # setup interleave geometry
 nr_targets=${#endpoint[@]}
 echo $nr_targets > /sys/bus/cxl/devices/$region/interleave_ways
-g=$(cat /sys/bus/cxl/devices/$decoder/interleave_granularity)
-echo $g > /sys/bus/cxl/devices/$region/interleave_granularity
+r_ig=$(cat /sys/bus/cxl/devices/$decoder/interleave_granularity)
+echo $r_ig > /sys/bus/cxl/devices/$region/interleave_granularity
 echo $((nr_targets * (256<<20))) > /sys/bus/cxl/devices/$region/size
 
 # grab the list of memdevs grouped by host-bridge interleave position
@@ -96,6 +96,22 @@ do
 done
 echo "$region added ${#endpoint[@]} targets: ${endpoint[@]}"
 
+# validate all endpoint decoders have the correct setting
+region_size=$(cat /sys/bus/cxl/devices/$region/size)
+region_base=$(cat /sys/bus/cxl/devices/$region/resource)
+for i in ${endpoint[@]}
+do
+	iw=$(cat /sys/bus/cxl/devices/$i/interleave_ways)
+	ig=$(cat /sys/bus/cxl/devices/$i/interleave_granularity)
+	[ $iw -ne $nr_targets ] && err "$LINENO: decoder: $i iw: $iw targets: $nr_targets"
+	[ $ig -ne $r_ig] && err "$LINENO: decoder: $i ig: $ig root ig: $r_ig"
+
+	sz=$(cat /sys/bus/cxl/devices/$i/size)
+	res=$(cat /sys/bus/cxl/devices/$i/start)
+	[ $sz -ne $region_size ] && err "$LINENO: decoder: $i sz: $sz region_size: $region_size"
+	[ $res -ne $region_base ] && err "$LINENO: decoder: $i base: $res region_base: $region_base"
+done
+
 # walk up the topology and commit all decoders
 echo 1 > /sys/bus/cxl/devices/$region/commit
 

[ndctl PATCH 2/6] cxl/list: Add interleave parameters to decoder listings

[Dan Williams]

Emit interleave_ways and interleave_granularity in decoder output.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 cxl/json.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/cxl/json.c b/cxl/json.c
index ad93413b5f05..36b76d34b4e5 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -466,6 +466,26 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
 			json_object_object_add(jdecoder, "size", jobj);
 	}
 
+	val = cxl_decoder_get_interleave_ways(decoder);
+	if (val < UINT_MAX) {
+		jobj = json_object_new_int(val);
+		if (jobj)
+			json_object_object_add(jdecoder, "interleave_ways",
+					       jobj);
+
+		/* granularity is a don't care if not interleaving */
+		if (val > 1) {
+			val = cxl_decoder_get_interleave_granularity(decoder);
+			if (val < UINT_MAX) {
+				jobj = json_object_new_int(val);
+				if (jobj)
+					json_object_object_add(
+						jdecoder,
+						"interleave_granularity", jobj);
+			}
+		}
+	}
+
 	if (size == 0) {
 		jobj = json_object_new_string("disabled");
 		if (jobj)

[ndctl PATCH 3/6] cxl/list: Add region to decoder listings

[Dan Williams]

While decoders can be matched with regions by physical address, or filtered
by region, it is also useful to get a plain listing of the association.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/cxl/lib/libcxl.txt |    7 +++++++
 cxl/json.c                       |    8 ++++++++
 cxl/lib/libcxl.c                 |   34 ++++++++++++++++++++++++++++++++++
 cxl/lib/libcxl.sym               |    1 +
 cxl/libcxl.h                     |    1 +
 5 files changed, 51 insertions(+)

diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
index 7a38ce4a54e2..72641699633b 100644
--- a/Documentation/cxl/lib/libcxl.txt
+++ b/Documentation/cxl/lib/libcxl.txt
@@ -398,6 +398,7 @@ int cxl_decoder_set_dpa_size(struct cxl_decoder *decoder, unsigned long long siz
 const char *cxl_decoder_get_devname(struct cxl_decoder *decoder);
 int cxl_decoder_get_id(struct cxl_decoder *decoder);
 int cxl_decoder_get_nr_targets(struct cxl_decoder *decoder);
+struct cxl_region *cxl_decoder_get_region(struct cxl_decoder *decoder);
 
 enum cxl_decoder_target_type {
        CXL_DECODER_TTYPE_UNKNOWN,
@@ -446,6 +447,12 @@ Platform firmware may setup the CXL decode hierarchy before the OS
 boots, and may additionally require that the OS not change the decode
 settings. This property is indicated by the cxl_decoder_is_locked() API.
 
+When a decoder is associated with a region cxl_decoder_get_region()
+returns that region object. Note that it is only applicable to switch
+and endpoint decoders as root decoders have a 1:N relationship with
+regions.  Use cxl_region_foreach() for the similar functionality for
+root decoders.
+
 ==== TARGETS
 A root or switch level decoder takes an SPA (system-physical-address) as
 input and routes it to a downstream port. Which downstream port depends
diff --git a/cxl/json.c b/cxl/json.c
index 36b76d34b4e5..82e3c552cdb1 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -442,6 +442,7 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
 	const char *devname = cxl_decoder_get_devname(decoder);
 	struct cxl_port *port = cxl_decoder_get_port(decoder);
 	struct json_object *jdecoder, *jobj;
+	struct cxl_region *region;
 	u64 val, size;
 
 	jdecoder = json_object_new_object();
@@ -486,6 +487,13 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
 		}
 	}
 
+	region = cxl_decoder_get_region(decoder);
+	if (region) {
+		jobj = json_object_new_string(cxl_region_get_devname(region));
+		if (jobj)
+			json_object_object_add(jdecoder, "region", jobj);
+	}
+
 	if (size == 0) {
 		jobj = json_object_new_string("disabled");
 		if (jobj)
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 5001c5685d74..aec3671b1625 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -2019,6 +2019,40 @@ cxl_decoder_get_interleave_ways(struct cxl_decoder *decoder)
 	return decoder->interleave_ways;
 }
 
+CXL_EXPORT struct cxl_region *
+cxl_decoder_get_region(struct cxl_decoder *decoder)
+{
+	struct cxl_port *port = cxl_decoder_get_port(decoder);
+	struct cxl_ctx *ctx = cxl_decoder_get_ctx(decoder);
+	char *path = decoder->dev_buf;
+	char buf[SYSFS_ATTR_SIZE];
+	struct cxl_region *region;
+	struct cxl_decoder *iter;
+	int rc;
+
+	if (cxl_port_is_root(port))
+		return NULL;
+
+	sprintf(path, "%s/region", decoder->dev_path);
+	rc = sysfs_read_attr(ctx, path, buf);
+	if (rc < 0) {
+		err(ctx, "failed to read region name: %s\n", strerror(-rc));
+		return NULL;
+	}
+
+	if (strcmp(buf, "") == 0)
+		return NULL;
+
+	while (!cxl_port_is_root(port))
+		port = cxl_port_get_parent(port);
+
+	cxl_decoder_foreach(port, iter)
+		cxl_region_foreach(iter, region)
+			if (strcmp(cxl_region_get_devname(region), buf) == 0)
+				return region;
+	return NULL;
+}
+
 CXL_EXPORT struct cxl_region *
 cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
 {
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 6bf3e91bdecc..573fcdf532d6 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -213,4 +213,5 @@ global:
 	cxl_ep_decoder_get_memdev;
 	cxl_decoder_get_interleave_granularity;
 	cxl_decoder_get_interleave_ways;
+	cxl_decoder_get_region;
 } LIBCXL_2;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 0b84977c2a2c..4b5490986a2a 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -185,6 +185,7 @@ bool cxl_decoder_is_locked(struct cxl_decoder *decoder);
 unsigned int
 cxl_decoder_get_interleave_granularity(struct cxl_decoder *decoder);
 unsigned int cxl_decoder_get_interleave_ways(struct cxl_decoder *decoder);
+struct cxl_region *cxl_decoder_get_region(struct cxl_decoder *decoder);
 struct cxl_region *cxl_decoder_create_pmem_region(struct cxl_decoder *decoder);
 struct cxl_decoder *cxl_decoder_get_by_name(struct cxl_ctx *ctx,
 					    const char *ident);

[ndctl PATCH 4/6] cxl/list: Filter decoders by region

[Dan Williams]

With a region name in hand, it is useful to be able to filter all the
decoders in the topology that are mapping that region.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 cxl/filter.c |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/cxl/filter.c b/cxl/filter.c
index 38ece5528794..9a3de8c75387 100644
--- a/cxl/filter.c
+++ b/cxl/filter.c
@@ -652,6 +652,26 @@ struct cxl_region *util_cxl_region_filter(struct cxl_region *region,
 
 }
 
+static struct cxl_decoder *
+util_cxl_decoder_filter_by_region(struct cxl_decoder *decoder,
+				  const char *__ident)
+{
+	struct cxl_region *region;
+
+	if (!__ident)
+		return decoder;
+
+	region = cxl_decoder_get_region(decoder);
+	if (!region)
+		return NULL;
+
+	region = util_cxl_region_filter(region, __ident);
+	if (!region)
+		return NULL;
+
+	return decoder;
+}
+
 static unsigned long params_to_flags(struct cxl_filter_params *param)
 {
 	unsigned long flags = 0;
@@ -790,6 +810,9 @@ static void walk_decoders(struct cxl_port *port, struct cxl_filter_params *p,
 		if (!util_cxl_decoder_filter_by_memdev(
 			    decoder, p->memdev_filter, p->serial_filter))
 			goto walk_children;
+		if (!util_cxl_decoder_filter_by_region(decoder,
+						       p->region_filter))
+			goto walk_children;
 		if (!p->idle && cxl_decoder_get_size(decoder) == 0)
 			continue;
 		jdecoder = util_cxl_decoder_to_json(decoder, flags);

[ndctl PATCH 5/6] cxl/list: Add 'depth' to port listings

[Dan Williams]

Simplify the task of determining how deep a port is in the hierarchy by
just emitting what libcxl already counted. This is useful for validating
interleave math.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/cxl/lib/libcxl.txt |    1 +
 cxl/json.c                       |    4 ++++
 cxl/lib/libcxl.c                 |    5 +++++
 cxl/lib/libcxl.sym               |    1 +
 cxl/libcxl.h                     |    1 +
 5 files changed, 12 insertions(+)

diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
index 72641699633b..5efa60124111 100644
--- a/Documentation/cxl/lib/libcxl.txt
+++ b/Documentation/cxl/lib/libcxl.txt
@@ -290,6 +290,7 @@ int cxl_port_is_enabled(struct cxl_port *port);
 bool cxl_port_is_root(struct cxl_port *port);
 bool cxl_port_is_switch(struct cxl_port *port);
 bool cxl_port_is_endpoint(struct cxl_port *port);
+int cxl_port_get_depth(struct cxl_port *port);
 bool cxl_port_hosts_memdev(struct cxl_port *port, struct cxl_memdev *memdev);
 int cxl_port_get_nr_dports(struct cxl_port *port);
 ----
diff --git a/cxl/json.c b/cxl/json.c
index 82e3c552cdb1..7aefcadb0795 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -761,6 +761,10 @@ static struct json_object *__util_cxl_port_to_json(struct cxl_port *port,
 	if (jobj)
 		json_object_object_add(jport, "host", jobj);
 
+	jobj = json_object_new_int(cxl_port_get_depth(port));
+	if (jobj)
+		json_object_object_add(jport, "depth", jobj);
+
 	if (!cxl_port_is_enabled(port)) {
 		jobj = json_object_new_string("disabled");
 		if (jobj)
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index aec3671b1625..4b78ecc1d115 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -2309,6 +2309,11 @@ CXL_EXPORT bool cxl_port_is_endpoint(struct cxl_port *port)
 	return port->type == CXL_PORT_ENDPOINT;
 }
 
+CXL_EXPORT int cxl_port_get_depth(struct cxl_port *port)
+{
+	return port->depth;
+}
+
 CXL_EXPORT struct cxl_bus *cxl_port_get_bus(struct cxl_port *port)
 {
 	struct cxl_bus *bus;
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 573fcdf532d6..7dc3eee8a63c 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -96,6 +96,7 @@ global:
 	cxl_port_get_parent;
 	cxl_port_is_root;
 	cxl_port_is_switch;
+	cxl_port_get_depth;
 	cxl_port_to_bus;
 	cxl_port_is_endpoint;
 	cxl_port_to_endpoint;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 4b5490986a2a..aa0a89d91b30 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -89,6 +89,7 @@ int cxl_port_is_enabled(struct cxl_port *port);
 struct cxl_port *cxl_port_get_parent(struct cxl_port *port);
 bool cxl_port_is_root(struct cxl_port *port);
 bool cxl_port_is_switch(struct cxl_port *port);
+int cxl_port_get_depth(struct cxl_port *port);
 struct cxl_bus *cxl_port_to_bus(struct cxl_port *port);
 bool cxl_port_is_endpoint(struct cxl_port *port);
 struct cxl_endpoint *cxl_port_to_endpoint(struct cxl_port *port);

[ndctl PATCH 6/6] cxl/test: Validate switch port settings in cxl-region-sysfs.sh

[Dan Williams]

A recent kernel fix to add the missing update of endpoint decoder HPA range
settings regressed switch decoder HPA range settings. Add validation for
switch port settings to avoid regressions like that going forward.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/cxl-region-sysfs.sh |   32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh
index 110e03709e39..ae0f55653814 100644
--- a/test/cxl-region-sysfs.sh
+++ b/test/cxl-region-sysfs.sh
@@ -112,6 +112,38 @@ do
 	[ $res -ne $region_base ] && err "$LINENO: decoder: $i base: $res region_base: $region_base"
 done
 
+# validate all switch decoders have the correct settings
+nr_switches=$((nr_targets/2))
+nr_host_bridges=$((nr_switches/2))
+nr_switch_decoders=$((nr_switches + nr_host_bridges))
+
+json=$($CXL list -D -r $region -d switch)
+readarray -t switch_decoders < <(echo $json | jq -r ".[].decoder")
+
+[ ${#switch_decoders[@]} -ne $nr_switch_decoders ] && err \
+"$LINENO: expected $nr_switch_decoders got ${#switch_decoders[@]} switch decoders"
+
+for i in ${switch_decoders[@]}
+do
+	decoder=$(echo $json | jq -r ".[] | select(.decoder == \"$i\")")
+	id=${i#decoder}
+	port_id=${id%.*}
+	depth=$($CXL list -p $port_id -S | jq -r ".[].depth")
+	iw=$(echo $decoder | jq -r ".interleave_ways")
+	ig=$(echo $decoder | jq -r ".interleave_granularity")
+
+	[ $iw -ne 2 ] && err "$LINENO: decoder: $i iw: $iw targets: 2"
+	[ $ig -ne $((r_ig << depth)) ] && err \
+	"$LINENO: decoder: $i ig: $ig switch_ig: $((r_ig << depth))"
+
+	res=$(echo $decoder | jq -r ".resource")
+	sz=$(echo $decoder | jq -r ".size")
+	[ $sz -ne $region_size ] && err \
+	"$LINENO: decoder: $i sz: $sz region_size: $region_size"
+	[ $res -ne $region_base ] && err \
+	"$LINENO: decoder: $i base: $res region_base: $region_base"
+done
+
 # walk up the topology and commit all decoders
 echo 1 > /sys/bus/cxl/devices/$region/commit
 

Re: [ndctl PATCH 0/6] cxl/test: Add more region ABI validation

[Dave Jiang]

On 8/5/2022 1:37 PM, Dan Williams wrote:
> Expand cxl-region-sysfs.sh to validate that all decoders in the test
> region are setup per expectations. In support of that testing, decoder
> listings gain 'interleave_ways', 'interleave_granularity', and 'region'
> attributes, while port listings gain a 'depth' attribute. Decoders can
> now be filtered by their associated region.
>
> ---
>
> Dan Williams (6):
>        cxl/test: Validate endpoint interleave geometry
>        cxl/list: Add interleave parameters to decoder listings
>        cxl/list: Add region to decoder listings
>        cxl/list: Filter decoders by region
>        cxl/list: Add 'depth' to port listings
>        cxl/test: Validate switch port settings in cxl-region-sysfs.sh
Reviewed-by: Dave Jiang <dave.jiang@intel.com> for the series
>
>
>   Documentation/cxl/lib/libcxl.txt |    8 ++++++
>   cxl/filter.c                     |   23 +++++++++++++++++
>   cxl/json.c                       |   32 +++++++++++++++++++++++
>   cxl/lib/libcxl.c                 |   39 +++++++++++++++++++++++++++++
>   cxl/lib/libcxl.sym               |    2 +
>   cxl/libcxl.h                     |    2 +
>   test/cxl-region-sysfs.sh         |   52 +++++++++++++++++++++++++++++++++++++-
>   7 files changed, 156 insertions(+), 2 deletions(-)