[ndctl PATCH 00/11] cxl: Region provisioning foundation

[ndctl PATCH 00/11] cxl: Region provisioning foundation

[Dan Williams]

On the way towards a "cxl create-region" command add support for a unit
test of the raw sysfs interfaces used for region provisioning. This
includes support for listing endpoint decoders filtered by their
associated memdev, listing decoders in id order by default, and managing
DPA allocations. Those capabilities plus some other miscellaneous
cleanups are validated in a new 'cxl-create-region.sh' test.

All of the sysfs ABI leveraged in these updates is provided by this
pending series of updates:

https://lore.kernel.org/linux-cxl/165603869943.551046.3498980330327696732.stgit@dwillia2-xfh/

To date that review has not identified any ABI changes so there is a
reasonable chance that this cxl-cli series will not need to be respun to
address a kernel-side change. That said, the kernel changes need to
complete review and enter linux-next before these proposed patches can
be committed to the ndctl project. In the meantime that kernel review
can be helped along by having test/cxl-region-create.sh as an example of
how the ABI is used.

---

Dan Williams (11):
      cxl/list: Reformat option list
      cxl/list: Emit endpoint decoders filtered by memdev
      cxl/list: Hide 0s in disabled decoder listings
      cxl/list: Add DPA span to endpoint decoder listings
      cxl/lib: Maintain decoders in id order
      cxl/memdev: Fix json for multi-device partitioning
      cxl/list: Emit 'mode' for endpoint decoder objects
      cxl/set-partition: Accept 'ram' as an alias for 'volatile'
      cxl/memdev: Add {reserve,free}-dpa commands
      cxl/test: Update CXL memory parameters
      cxl/test: Checkout region setup/teardown


 .clang-format                           |    1 
 Documentation/cxl/cxl-set-partition.txt |    2 
 Documentation/cxl/lib/libcxl.txt        |   13 +
 cxl/builtin.h                           |    2 
 cxl/cxl.c                               |    2 
 cxl/filter.c                            |   12 +
 cxl/filter.h                            |    2 
 cxl/json.c                              |   38 +++-
 cxl/lib/libcxl.c                        |  167 +++++++++++++++++
 cxl/lib/libcxl.sym                      |   11 +
 cxl/lib/private.h                       |    3 
 cxl/libcxl.h                            |   34 +++
 cxl/list.c                              |    9 -
 cxl/memdev.c                            |  304 ++++++++++++++++++++++++++++++-
 test/cxl-region-create.sh               |  122 ++++++++++++
 test/cxl-topology.sh                    |   32 ++-
 test/meson.build                        |    2 
 util/list.h                             |   61 ++++++
 18 files changed, 784 insertions(+), 33 deletions(-)
 create mode 100644 test/cxl-region-create.sh

base-commit: bbb2cb56f08d95ecf2c7c047a33cc3dd64eb7fde

[ndctl PATCH 01/11] cxl/list: Reformat option list

[Dan Williams]

Cleanup some spurious spaces and let clang-format re-layout the options.

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

diff --git a/cxl/list.c b/cxl/list.c
index 940782d33a10..1b5f58328047 100644
--- a/cxl/list.c
+++ b/cxl/list.c
@@ -36,8 +36,7 @@ static const struct option options[] = {
 		   "filter by CXL endpoint device name(s)"),
 	OPT_BOOLEAN('E', "endpoints", &param.endpoints,
 		    "include CXL endpoint info"),
-	OPT_STRING('d', "decoder", &param.decoder_filter,
-		   "decoder device name",
+	OPT_STRING('d', "decoder", &param.decoder_filter, "decoder device name",
 		   "filter by CXL decoder device name(s) / class"),
 	OPT_BOOLEAN('D', "decoders", &param.decoders,
 		    "include CXL decoder info"),
@@ -45,11 +44,11 @@ static const struct option options[] = {
 		    "include CXL target data with decoders or ports"),
 	OPT_BOOLEAN('i', "idle", &param.idle, "include disabled devices"),
 	OPT_BOOLEAN('u', "human", &param.human,
-		    "use human friendly number formats "),
+		    "use human friendly number formats"),
 	OPT_BOOLEAN('H', "health", &param.health,
-		    "include memory device health information "),
+		    "include memory device health information"),
 	OPT_BOOLEAN('I', "partition", &param.partition,
-		    "include memory device partition information "),
+		    "include memory device partition information"),
 #ifdef ENABLE_DEBUG
 	OPT_BOOLEAN(0, "debug", &debug, "debug list walk"),
 #endif

[ndctl PATCH 02/11] cxl/list: Emit endpoint decoders filtered by memdev

[Dan Williams]

For example, dump all the endpoint decoders from memdev 'mem8'.

    cxl list -Di -m 8 -d endpoint

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

diff --git a/cxl/filter.c b/cxl/filter.c
index 66fd7420144a..2f88a9d2f398 100644
--- a/cxl/filter.c
+++ b/cxl/filter.c
@@ -428,7 +428,9 @@ util_cxl_decoder_filter_by_memdev(struct cxl_decoder *decoder,
 				  const char *ident, const char *serial)
 {
 	struct cxl_ctx *ctx = cxl_decoder_get_ctx(decoder);
+	struct cxl_endpoint *endpoint;
 	struct cxl_memdev *memdev;
+	struct cxl_port *port;
 
 	if (!ident && !serial)
 		return decoder;
@@ -438,6 +440,12 @@ util_cxl_decoder_filter_by_memdev(struct cxl_decoder *decoder,
 			continue;
 		if (cxl_decoder_get_target_by_memdev(decoder, memdev))
 			return decoder;
+		port = cxl_decoder_get_port(decoder);
+		if (!port || !cxl_port_is_endpoint(port))
+			continue;
+		endpoint = cxl_port_to_endpoint(port);
+		if (cxl_endpoint_get_memdev(endpoint) == memdev)
+			return decoder;
 	}
 
 	return NULL;

[ndctl PATCH 03/11] cxl/list: Hide 0s in disabled decoder listings

[Dan Williams]

Trim some redundant information from decoder listings when they are
disabled.

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

diff --git a/cxl/json.c b/cxl/json.c
index fdc6f73a86c1..a213fdad55fd 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -442,7 +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;
-	u64 val;
+	u64 val, size;
 
 	jdecoder = json_object_new_object();
 	if (!jdecoder)
@@ -452,21 +452,21 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
 	if (jobj)
 		json_object_object_add(jdecoder, "decoder", jobj);
 
+	size = cxl_decoder_get_size(decoder);
 	val = cxl_decoder_get_resource(decoder);
-	if (val < ULLONG_MAX) {
+	if (size && val < ULLONG_MAX) {
 		jobj = util_json_object_hex(val, flags);
 		if (jobj)
 			json_object_object_add(jdecoder, "resource", jobj);
 	}
 
-	val = cxl_decoder_get_size(decoder);
-	if (val < ULLONG_MAX) {
-		jobj = util_json_object_size(val, flags);
+	if (size && size < ULLONG_MAX) {
+		jobj = util_json_object_size(size, flags);
 		if (jobj)
 			json_object_object_add(jdecoder, "size", jobj);
 	}
 
-	if (val == 0) {
+	if (size == 0) {
 		jobj = json_object_new_string("disabled");
 		if (jobj)
 			json_object_object_add(jdecoder, "state", jobj);

[ndctl PATCH 04/11] cxl/list: Add DPA span to endpoint decoder listings

[Dan Williams]

Optionally include in decoder listings the device local address space for
endpoint decoders with active / allocated capacity.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/cxl/lib/libcxl.txt |    2 ++
 cxl/json.c                       |   18 ++++++++++++++++
 cxl/lib/libcxl.c                 |   43 +++++++++++++++++++++++++++++++++++++-
 cxl/lib/libcxl.sym               |    6 +++++
 cxl/lib/private.h                |    2 ++
 cxl/libcxl.h                     |    2 ++
 6 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
index f8f0e668ab59..2aef489e8e12 100644
--- a/Documentation/cxl/lib/libcxl.txt
+++ b/Documentation/cxl/lib/libcxl.txt
@@ -392,6 +392,8 @@ more CXL decoder objects.
 ----
 unsigned long long cxl_decoder_get_resource(struct cxl_decoder *decoder);
 unsigned long long cxl_decoder_get_size(struct cxl_decoder *decoder);
+unsigned long long cxl_decoder_get_dpa_resource(struct cxl_decoder *decoder);
+unsigned long long cxl_decoder_get_dpa_size(struct cxl_decoder *decoder);
 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);
diff --git a/cxl/json.c b/cxl/json.c
index a213fdad55fd..3f52d3bbff45 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -472,6 +472,24 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
 			json_object_object_add(jdecoder, "state", jobj);
 	}
 
+	if (cxl_port_is_endpoint(port)) {
+		size = cxl_decoder_get_dpa_size(decoder);
+		val = cxl_decoder_get_dpa_resource(decoder);
+		if (size && val < ULLONG_MAX) {
+			jobj = util_json_object_hex(val, flags);
+			if (jobj)
+				json_object_object_add(jdecoder, "dpa_resource",
+						       jobj);
+		}
+
+		if (size && size < ULLONG_MAX) {
+			jobj = util_json_object_size(size, flags);
+			if (jobj)
+				json_object_object_add(jdecoder, "dpa_size",
+						       jobj);
+		}
+	}
+
 	if (cxl_port_is_root(port) && cxl_decoder_is_mem_capable(decoder)) {
 		if (cxl_decoder_is_pmem_capable(decoder)) {
 			jobj = json_object_new_boolean(true);
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index c988ce2ddea9..f36edcfc735a 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -955,8 +955,19 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
 		decoder->size = strtoull(buf, NULL, 0);
 
 	switch (port->type) {
-	case CXL_PORT_SWITCH:
 	case CXL_PORT_ENDPOINT:
+		sprintf(path, "%s/dpa_resource", cxldecoder_base);
+		if (sysfs_read_attr(ctx, path, buf) < 0)
+			decoder->dpa_resource = ULLONG_MAX;
+		else
+			decoder->dpa_resource = strtoull(buf, NULL, 0);
+		sprintf(path, "%s/dpa_size", cxldecoder_base);
+		if (sysfs_read_attr(ctx, path, buf) < 0)
+			decoder->dpa_size = ULLONG_MAX;
+		else
+			decoder->dpa_size = strtoull(buf, NULL, 0);
+
+	case CXL_PORT_SWITCH:
 		decoder->pmem_capable = true;
 		decoder->volatile_capable = true;
 		decoder->mem_capable = true;
@@ -1113,6 +1124,36 @@ CXL_EXPORT unsigned long long cxl_decoder_get_size(struct cxl_decoder *decoder)
 	return decoder->size;
 }
 
+CXL_EXPORT unsigned long long
+cxl_decoder_get_dpa_resource(struct cxl_decoder *decoder)
+{
+	struct cxl_port *port = cxl_decoder_get_port(decoder);
+	struct cxl_ctx *ctx = cxl_decoder_get_ctx(decoder);
+
+	if (!cxl_port_is_endpoint(port)) {
+		err(ctx, "%s: not an endpoint decoder\n",
+		    cxl_decoder_get_devname(decoder));
+		return ULLONG_MAX;
+	}
+
+	return decoder->dpa_resource;
+}
+
+CXL_EXPORT unsigned long long
+cxl_decoder_get_dpa_size(struct cxl_decoder *decoder)
+{
+	struct cxl_port *port = cxl_decoder_get_port(decoder);
+	struct cxl_ctx *ctx = cxl_decoder_get_ctx(decoder);
+
+	if (!cxl_port_is_endpoint(port)) {
+		err(ctx, "%s: not an endpoint decoder\n",
+		    cxl_decoder_get_devname(decoder));
+		return ULLONG_MAX;
+	}
+
+	return decoder->dpa_size;
+}
+
 CXL_EXPORT enum cxl_decoder_target_type
 cxl_decoder_get_target_type(struct cxl_decoder *decoder)
 {
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index dffcb60b8dd0..8e2fc75557f9 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -167,3 +167,9 @@ global:
 	cxl_cmd_new_set_partition;
 	cxl_cmd_partition_set_mode;
 } LIBCXL_1;
+
+LIBCXL_3 {
+global:
+	cxl_decoder_get_dpa_resource;
+	cxl_decoder_get_dpa_size;
+} LIBCXL_2;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index c6d88f7140f2..24a2ae6787be 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -101,6 +101,8 @@ struct cxl_decoder {
 	struct cxl_ctx *ctx;
 	u64 start;
 	u64 size;
+	u64 dpa_resource;
+	u64 dpa_size;
 	void *dev_buf;
 	size_t buf_len;
 	char *dev_path;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 0007f4d9bcee..76aebe3efda8 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -129,6 +129,8 @@ struct cxl_decoder *cxl_decoder_get_first(struct cxl_port *port);
 struct cxl_decoder *cxl_decoder_get_next(struct cxl_decoder *decoder);
 unsigned long long cxl_decoder_get_resource(struct cxl_decoder *decoder);
 unsigned long long cxl_decoder_get_size(struct cxl_decoder *decoder);
+unsigned long long cxl_decoder_get_dpa_resource(struct cxl_decoder *decoder);
+unsigned long long cxl_decoder_get_dpa_size(struct cxl_decoder *decoder);
 const char *cxl_decoder_get_devname(struct cxl_decoder *decoder);
 struct cxl_target *cxl_decoder_get_target_by_memdev(struct cxl_decoder *decoder,
 						    struct cxl_memdev *memdev);

[ndctl PATCH 05/11] cxl/lib: Maintain decoders in id order

[Dan Williams]

Given that decoder instance order is fundamental to the DPA translation
sequence for endpoint decoders, enforce that cxl_decoder_for_each() returns
decoders in instance order. Otherwise, they show up in readddir() order
which is not predictable.

Add a list_add_sorted() to generically handle inserting into a sorted list.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 cxl/lib/libcxl.c |    8 ++++++-
 util/list.h      |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 1 deletion(-)

diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index f36edcfc735a..e4c5d3819e88 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -19,6 +19,7 @@
 #include <ccan/short_types/short_types.h>
 
 #include <util/log.h>
+#include <util/list.h>
 #include <util/size.h>
 #include <util/sysfs.h>
 #include <util/bitmap.h>
@@ -908,6 +909,11 @@ cxl_endpoint_get_memdev(struct cxl_endpoint *endpoint)
 	return NULL;
 }
 
+static int decoder_id_cmp(struct cxl_decoder *d1, struct cxl_decoder *d2)
+{
+	return d1->id - d2->id;
+}
+
 static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
 {
 	const char *devname = devpath_to_devname(cxldecoder_base);
@@ -1049,7 +1055,7 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
 			return decoder_dup;
 		}
 
-	list_add(&port->decoders, &decoder->list);
+	list_add_sorted(&port->decoders, decoder, list, decoder_id_cmp);
 
 	free(path);
 	return decoder;
diff --git a/util/list.h b/util/list.h
index 1ea9c59b9f0c..c6584e3ec725 100644
--- a/util/list.h
+++ b/util/list.h
@@ -37,4 +37,65 @@ static inline void list_add_after_(struct list_head *h,
 	(void)list_debug(h, abortstr);
 }
 
+/**
+ * list_add_before - add an entry before the given node in the linked list.
+ * @h: the list_head to add the node to
+ * @l: the list_node before which to add to
+ * @n: the list_node to add to the list.
+ *
+ * The list_node does not need to be initialized; it will be overwritten.
+ * Example:
+ *	struct child *child = malloc(sizeof(*child));
+ *
+ *	child->name = "geoffrey";
+ *	list_add_before(&parent->children, &child1->list, &child->list);
+ *	parent->num_children++;
+ */
+#define list_add_before(h, l, n) list_add_before_(h, l, n, LIST_LOC)
+static inline void list_add_before_(struct list_head *h, struct list_node *l,
+				    struct list_node *n, const char *abortstr)
+{
+	if (l->prev == &h->n) {
+		/* l is the first element, this becomes a list_add */
+		list_add(h, n);
+		return;
+	}
+
+	n->next = l;
+	n->prev = l->prev;
+	l->prev->next = n;
+	l->prev = n;
+}
+
+#define list_add_sorted(head, n, node, cmp)                                    \
+	do {                                                                   \
+		struct list_head *__head = (head);                             \
+		typeof(n) __iter, __next;                                      \
+		typeof(n) __new = (n);                                         \
+                                                                               \
+		if (list_empty(__head)) {                                      \
+			list_add(__head, &__new->node);                        \
+			break;                                                 \
+		}                                                              \
+                                                                               \
+		list_for_each (__head, __iter, node) {                         \
+			if (cmp(__new, __iter) < 0) {                          \
+				list_add_before(__head, &__iter->node,         \
+						&__new->node);                 \
+				break;                                         \
+			}                                                      \
+			__next = list_next(__head, __iter, node);              \
+			if (!__next) {                                         \
+				list_add_after(__head, &__iter->node,          \
+					       &__new->node);                  \
+				break;                                         \
+			}                                                      \
+			if (cmp(__new, __next) < 0) {                          \
+				list_add_before(__head, &__next->node,         \
+						&__new->node);                 \
+				break;                                         \
+			}                                                      \
+		}                                                              \
+	} while (0)
+
 #endif /* _NDCTL_LIST_H_ */

[ndctl PATCH 06/11] cxl/memdev: Fix json for multi-device partitioning

[Dan Williams]

In the case when someone partitions several devices at once, collect all
the affected memdevs into a json array.

With the move to use util_display_json_array() that also requires a set of
flags to be specifiied. Apply the UTIL_JSON_HUMAN flag for all interactive
command result output to bring this command in line with other tools.

Cc: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 cxl/memdev.c |   26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/cxl/memdev.c b/cxl/memdev.c
index 91d914db5af6..9fcd8ae5724b 100644
--- a/cxl/memdev.c
+++ b/cxl/memdev.c
@@ -19,6 +19,7 @@
 struct action_context {
 	FILE *f_out;
 	FILE *f_in;
+	struct json_object *jdevs;
 };
 
 static struct parameters {
@@ -339,12 +340,13 @@ out:
 }
 
 static int action_setpartition(struct cxl_memdev *memdev,
-		struct action_context *actx)
+			       struct action_context *actx)
 {
 	const char *devname = cxl_memdev_get_devname(memdev);
 	enum cxl_setpart_type type = CXL_SETPART_PMEM;
 	unsigned long long size = ULLONG_MAX;
 	struct json_object *jmemdev;
+	unsigned long flags;
 	struct cxl_cmd *cmd;
 	int rc;
 
@@ -396,10 +398,12 @@ out_err:
 	if (rc)
 		log_err(&ml, "%s error: %s\n", devname, strerror(-rc));
 
-	jmemdev = util_cxl_memdev_to_json(memdev, UTIL_JSON_PARTITION);
-	if (jmemdev)
-		printf("%s\n", json_object_to_json_string_ext(jmemdev,
-		       JSON_C_TO_STRING_PRETTY));
+	flags = UTIL_JSON_PARTITION;
+	if (actx->f_out == stdout && isatty(1))
+		flags |= UTIL_JSON_HUMAN;
+	jmemdev = util_cxl_memdev_to_json(memdev, flags);
+	if (actx->jdevs && jmemdev)
+		json_object_array_add(actx->jdevs, jmemdev);
 
 	return rc;
 }
@@ -446,6 +450,9 @@ static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx,
 		err++;
 	}
 
+	if (action == action_setpartition)
+		actx.jdevs = json_object_new_array();
+
 	if (err == argc) {
 		usage_with_options(u, options);
 		return -EINVAL;
@@ -528,6 +535,15 @@ static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx,
 	if (actx.f_in != stdin)
 		fclose(actx.f_in);
 
+	if (actx.jdevs) {
+		unsigned long flags = 0;
+
+		if (actx.f_out == stdout && isatty(1))
+			flags |= UTIL_JSON_HUMAN;
+		util_display_json_array(actx.f_out, actx.jdevs, flags);
+	}
+
+
  out_close_fout:
 	if (actx.f_out != stdout)
 		fclose(actx.f_out);

[ndctl PATCH 07/11] cxl/list: Emit 'mode' for endpoint decoder objects

[Dan Williams]

The 'mode' property of an endpoint decoder indicates the access
properties of the DPA (device physical address) mapped into HPA (host
physical address) by the decoder. Where the modes are 'none'
(decoder-disabled), 'ram' (voltaile memory), 'pmem' (persistent memory),
and 'mixed' (an unexpected, but possible, case where the decoder
straddles a mode / partition boundary).

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/cxl/lib/libcxl.txt |    9 +++++++++
 cxl/json.c                       |    8 ++++++++
 cxl/lib/libcxl.c                 |   30 ++++++++++++++++++++++++++++++
 cxl/lib/libcxl.sym               |    1 +
 cxl/lib/private.h                |    1 +
 cxl/libcxl.h                     |   23 +++++++++++++++++++++++
 6 files changed, 72 insertions(+)

diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
index 2aef489e8e12..90fe33887821 100644
--- a/Documentation/cxl/lib/libcxl.txt
+++ b/Documentation/cxl/lib/libcxl.txt
@@ -405,6 +405,15 @@ enum cxl_decoder_target_type {
 };
 
 cxl_decoder_get_target_type(struct cxl_decoder *decoder);
+
+enum cxl_decoder_mode {
+	CXL_DECODER_MODE_NONE,
+	CXL_DECODER_MODE_MIXED,
+	CXL_DECODER_MODE_PMEM,
+	CXL_DECODER_MODE_RAM,
+};
+enum cxl_decoder_mode cxl_decoder_get_mode(struct cxl_decoder *decoder);
+
 bool cxl_decoder_is_pmem_capable(struct cxl_decoder *decoder);
 bool cxl_decoder_is_volatile_capable(struct cxl_decoder *decoder);
 bool cxl_decoder_is_mem_capable(struct cxl_decoder *decoder);
diff --git a/cxl/json.c b/cxl/json.c
index 3f52d3bbff45..ae9c8126f14f 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -473,6 +473,8 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
 	}
 
 	if (cxl_port_is_endpoint(port)) {
+		enum cxl_decoder_mode mode = cxl_decoder_get_mode(decoder);
+
 		size = cxl_decoder_get_dpa_size(decoder);
 		val = cxl_decoder_get_dpa_resource(decoder);
 		if (size && val < ULLONG_MAX) {
@@ -488,6 +490,12 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
 				json_object_object_add(jdecoder, "dpa_size",
 						       jobj);
 		}
+
+		if (mode > CXL_DECODER_MODE_NONE) {
+			jobj = json_object_new_string(cxl_decoder_mode_name(mode));
+			if (jobj)
+				json_object_object_add(jdecoder, "mode", jobj);
+		}
 	}
 
 	if (cxl_port_is_root(port) && cxl_decoder_is_mem_capable(decoder)) {
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index e4c5d3819e88..a4709175678d 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -960,6 +960,21 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
 	else
 		decoder->size = strtoull(buf, NULL, 0);
 
+	sprintf(path, "%s/mode", cxldecoder_base);
+	if (sysfs_read_attr(ctx, path, buf) == 0) {
+		if (strcmp(buf, "ram") == 0)
+			decoder->mode = CXL_DECODER_MODE_RAM;
+		else if (strcmp(buf, "pmem") == 0)
+			decoder->mode = CXL_DECODER_MODE_PMEM;
+		else if (strcmp(buf, "mixed") == 0)
+			decoder->mode = CXL_DECODER_MODE_MIXED;
+		else if (strcmp(buf, "none") == 0)
+			decoder->mode = CXL_DECODER_MODE_NONE;
+		else
+			decoder->mode = CXL_DECODER_MODE_MIXED;
+	} else
+		decoder->mode = CXL_DECODER_MODE_NONE;
+
 	switch (port->type) {
 	case CXL_PORT_ENDPOINT:
 		sprintf(path, "%s/dpa_resource", cxldecoder_base);
@@ -1160,6 +1175,21 @@ cxl_decoder_get_dpa_size(struct cxl_decoder *decoder)
 	return decoder->dpa_size;
 }
 
+CXL_EXPORT enum cxl_decoder_mode
+cxl_decoder_get_mode(struct cxl_decoder *decoder)
+{
+	struct cxl_port *port = cxl_decoder_get_port(decoder);
+	struct cxl_ctx *ctx = cxl_decoder_get_ctx(decoder);
+
+	if (!cxl_port_is_endpoint(port)) {
+		err(ctx, "%s: not an endpoint decoder\n",
+		    cxl_decoder_get_devname(decoder));
+		return CXL_DECODER_MODE_NONE;
+	}
+
+	return decoder->mode;
+}
+
 CXL_EXPORT enum cxl_decoder_target_type
 cxl_decoder_get_target_type(struct cxl_decoder *decoder)
 {
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 8e2fc75557f9..88c5a7edd33e 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -172,4 +172,5 @@ LIBCXL_3 {
 global:
 	cxl_decoder_get_dpa_resource;
 	cxl_decoder_get_dpa_size;
+	cxl_decoder_get_mode;
 } LIBCXL_2;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index 24a2ae6787be..f6d4573757fd 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -108,6 +108,7 @@ struct cxl_decoder {
 	char *dev_path;
 	int nr_targets;
 	int id;
+	enum cxl_decoder_mode mode;
 	bool pmem_capable;
 	bool volatile_capable;
 	bool mem_capable;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 76aebe3efda8..1436dc4601cc 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -127,10 +127,33 @@ struct cxl_dport *cxl_port_get_dport_by_memdev(struct cxl_port *port,
 struct cxl_decoder;
 struct cxl_decoder *cxl_decoder_get_first(struct cxl_port *port);
 struct cxl_decoder *cxl_decoder_get_next(struct cxl_decoder *decoder);
+struct cxl_decoder *cxl_decoder_get_last(struct cxl_port *port);
+struct cxl_decoder *cxl_decoder_get_prev(struct cxl_decoder *decoder);
 unsigned long long cxl_decoder_get_resource(struct cxl_decoder *decoder);
 unsigned long long cxl_decoder_get_size(struct cxl_decoder *decoder);
 unsigned long long cxl_decoder_get_dpa_resource(struct cxl_decoder *decoder);
 unsigned long long cxl_decoder_get_dpa_size(struct cxl_decoder *decoder);
+enum cxl_decoder_mode {
+	CXL_DECODER_MODE_NONE,
+	CXL_DECODER_MODE_MIXED,
+	CXL_DECODER_MODE_PMEM,
+	CXL_DECODER_MODE_RAM,
+};
+static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode)
+{
+	static const char *names[] = {
+		[CXL_DECODER_MODE_NONE] = "none",
+		[CXL_DECODER_MODE_MIXED] = "mixed",
+		[CXL_DECODER_MODE_PMEM] = "pmem",
+		[CXL_DECODER_MODE_RAM] = "ram",
+	};
+
+	if (mode < CXL_DECODER_MODE_NONE || mode > CXL_DECODER_MODE_RAM)
+		mode = CXL_DECODER_MODE_NONE;
+	return names[mode];
+}
+
+enum cxl_decoder_mode cxl_decoder_get_mode(struct cxl_decoder *decoder);
 const char *cxl_decoder_get_devname(struct cxl_decoder *decoder);
 struct cxl_target *cxl_decoder_get_target_by_memdev(struct cxl_decoder *decoder,
 						    struct cxl_memdev *memdev);

[ndctl PATCH 08/11] cxl/set-partition: Accept 'ram' as an alias for 'volatile'

[Dan Williams]

'ram' is a more convenient shorthand for volatile memory.

Cc: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/cxl/cxl-set-partition.txt |    2 +-
 cxl/memdev.c                            |    4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/cxl/cxl-set-partition.txt b/Documentation/cxl/cxl-set-partition.txt
index 1e548af77da2..f0126daf808b 100644
--- a/Documentation/cxl/cxl-set-partition.txt
+++ b/Documentation/cxl/cxl-set-partition.txt
@@ -37,7 +37,7 @@ include::memdev-option.txt[]
 
 -t::
 --type=::
-	Type of partition, 'pmem' or 'volatile', to modify.
+	Type of partition, 'pmem' or 'ram' (volatile), to modify.
 	Default: 'pmem'
 
 -s::
diff --git a/cxl/memdev.c b/cxl/memdev.c
index 9fcd8ae5724b..1cecad2dba4b 100644
--- a/cxl/memdev.c
+++ b/cxl/memdev.c
@@ -65,7 +65,7 @@ OPT_BOOLEAN('f', "force", &param.force,                                \
 
 #define SET_PARTITION_OPTIONS() \
 OPT_STRING('t', "type",  &param.type, "type",			\
-	"'pmem' or 'volatile' (Default: 'pmem')"),		\
+	"'pmem' or 'ram' (volatile) (Default: 'pmem')"),		\
 OPT_STRING('s', "size",  &param.size, "size",			\
 	"size in bytes (Default: all available capacity)"),	\
 OPT_BOOLEAN('a', "align",  &param.align,			\
@@ -355,6 +355,8 @@ static int action_setpartition(struct cxl_memdev *memdev,
 			/* default */;
 		else if (strcmp(param.type, "volatile") == 0)
 			type = CXL_SETPART_VOLATILE;
+		else if (strcmp(param.type, "ram") == 0)
+			type = CXL_SETPART_VOLATILE;
 		else {
 			log_err(&ml, "invalid type '%s'\n", param.type);
 			return -EINVAL;

[ndctl PATCH 09/11] cxl/memdev: Add {reserve,free}-dpa commands

[Dan Williams]

Add helper commands for managing allocations of DPA (device physical
address) capacity on a set of CXL memory devices.

The main convenience this command affords is automatically picking the next
decoder to allocate per-memdev.

For example, to allocate 256MiB from all endpoints that are covered by a
given root decoder, and collect those resulting endpoint-decoders into an
array:

  readarray -t mem < <(cxl list -M -d $decoder | jq -r ".[].memdev")
  readarray -t endpoint < <(cxl reserve-dpa -t pmem ${mem[*]} -s $((256<<20)) |
                            jq -r ".[] | .decoder.decoder")

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 .clang-format                    |    1 
 Documentation/cxl/lib/libcxl.txt |    2 
 cxl/builtin.h                    |    2 
 cxl/cxl.c                        |    2 
 cxl/filter.c                     |    4 -
 cxl/filter.h                     |    2 
 cxl/lib/libcxl.c                 |   86 ++++++++++++
 cxl/lib/libcxl.sym               |    4 +
 cxl/libcxl.h                     |    9 +
 cxl/memdev.c                     |  276 ++++++++++++++++++++++++++++++++++++++
 10 files changed, 385 insertions(+), 3 deletions(-)

diff --git a/.clang-format b/.clang-format
index 6aabcb68e6a9..7254a1b15738 100644
--- a/.clang-format
+++ b/.clang-format
@@ -81,6 +81,7 @@ ForEachMacros:
   - 'cxl_bus_foreach'
   - 'cxl_port_foreach'
   - 'cxl_decoder_foreach'
+  - 'cxl_decoder_foreach_reverse'
   - 'cxl_target_foreach'
   - 'cxl_dport_foreach'
   - 'cxl_endpoint_foreach'
diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
index 90fe33887821..7a38ce4a54e2 100644
--- a/Documentation/cxl/lib/libcxl.txt
+++ b/Documentation/cxl/lib/libcxl.txt
@@ -394,6 +394,7 @@ unsigned long long cxl_decoder_get_resource(struct cxl_decoder *decoder);
 unsigned long long cxl_decoder_get_size(struct cxl_decoder *decoder);
 unsigned long long cxl_decoder_get_dpa_resource(struct cxl_decoder *decoder);
 unsigned long long cxl_decoder_get_dpa_size(struct cxl_decoder *decoder);
+int cxl_decoder_set_dpa_size(struct cxl_decoder *decoder, unsigned long long size);
 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);
@@ -413,6 +414,7 @@ enum cxl_decoder_mode {
 	CXL_DECODER_MODE_RAM,
 };
 enum cxl_decoder_mode cxl_decoder_get_mode(struct cxl_decoder *decoder);
+int cxl_decoder_set_mode(struct cxl_decoder *decoder, enum cxl_decoder_mode mode);
 
 bool cxl_decoder_is_pmem_capable(struct cxl_decoder *decoder);
 bool cxl_decoder_is_volatile_capable(struct cxl_decoder *decoder);
diff --git a/cxl/builtin.h b/cxl/builtin.h
index a437bc314a30..9e6fc624e86c 100644
--- a/cxl/builtin.h
+++ b/cxl/builtin.h
@@ -12,6 +12,8 @@ int cmd_init_labels(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_check_labels(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_disable_memdev(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_enable_memdev(int argc, const char **argv, struct cxl_ctx *ctx);
+int cmd_reserve_dpa(int argc, const char **argv, struct cxl_ctx *ctx);
+int cmd_free_dpa(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_disable_port(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_enable_port(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_set_partition(int argc, const char **argv, struct cxl_ctx *ctx);
diff --git a/cxl/cxl.c b/cxl/cxl.c
index aa4ce61b7c87..ef4cda9e7c23 100644
--- a/cxl/cxl.c
+++ b/cxl/cxl.c
@@ -66,6 +66,8 @@ static struct cmd_struct commands[] = {
 	{ "write-labels", .c_fn = cmd_write_labels },
 	{ "disable-memdev", .c_fn = cmd_disable_memdev },
 	{ "enable-memdev", .c_fn = cmd_enable_memdev },
+	{ "reserve-dpa", .c_fn = cmd_reserve_dpa },
+	{ "free-dpa", .c_fn = cmd_free_dpa },
 	{ "disable-port", .c_fn = cmd_disable_port },
 	{ "enable-port", .c_fn = cmd_enable_port },
 	{ "set-partition", .c_fn = cmd_set_partition },
diff --git a/cxl/filter.c b/cxl/filter.c
index 2f88a9d2f398..e5fab19ec47f 100644
--- a/cxl/filter.c
+++ b/cxl/filter.c
@@ -380,8 +380,8 @@ struct cxl_port *util_cxl_port_filter_by_memdev(struct cxl_port *port,
 	return NULL;
 }
 
-static struct cxl_decoder *util_cxl_decoder_filter(struct cxl_decoder *decoder,
-						   const char *__ident)
+struct cxl_decoder *util_cxl_decoder_filter(struct cxl_decoder *decoder,
+					    const char *__ident)
 {
 	struct cxl_port *port = cxl_decoder_get_port(decoder);
 	int pid, did;
diff --git a/cxl/filter.h b/cxl/filter.h
index 955794366d5c..c913dafd85ba 100644
--- a/cxl/filter.h
+++ b/cxl/filter.h
@@ -50,6 +50,8 @@ struct cxl_target *util_cxl_target_filter_by_memdev(struct cxl_target *target,
 struct cxl_dport *util_cxl_dport_filter_by_memdev(struct cxl_dport *dport,
 						  const char *ident,
 						  const char *serial);
+struct cxl_decoder *util_cxl_decoder_filter(struct cxl_decoder *decoder,
+					    const char *__ident);
 int cxl_filter_walk(struct cxl_ctx *ctx, struct cxl_filter_params *param);
 bool cxl_filter_has(const char *needle, const char *__filter);
 #endif /* _CXL_UTIL_FILTER_H_ */
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index a4709175678d..be6bc2c65483 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -1120,6 +1120,20 @@ CXL_EXPORT struct cxl_decoder *cxl_decoder_get_next(struct cxl_decoder *decoder)
 	return list_next(&port->decoders, decoder, list);
 }
 
+CXL_EXPORT struct cxl_decoder *cxl_decoder_get_last(struct cxl_port *port)
+{
+	cxl_decoders_init(port);
+
+	return list_tail(&port->decoders, struct cxl_decoder, list);
+}
+
+CXL_EXPORT struct cxl_decoder *cxl_decoder_get_prev(struct cxl_decoder *decoder)
+{
+	struct cxl_port *port = decoder->port;
+
+	return list_prev(&port->decoders, decoder, list);
+}
+
 CXL_EXPORT struct cxl_ctx *cxl_decoder_get_ctx(struct cxl_decoder *decoder)
 {
 	return decoder->ctx;
@@ -1175,6 +1189,78 @@ cxl_decoder_get_dpa_size(struct cxl_decoder *decoder)
 	return decoder->dpa_size;
 }
 
+CXL_EXPORT int cxl_decoder_set_dpa_size(struct cxl_decoder *decoder,
+					unsigned long long size)
+{
+	struct cxl_port *port = cxl_decoder_get_port(decoder);
+	struct cxl_ctx *ctx = cxl_decoder_get_ctx(decoder);
+	char *path = decoder->dev_buf;
+	int len = decoder->buf_len, rc;
+	char buf[SYSFS_ATTR_SIZE];
+
+	if (!cxl_port_is_endpoint(port)) {
+		err(ctx, "%s: not an endpoint decoder\n",
+		    cxl_decoder_get_devname(decoder));
+		return -EINVAL;
+	}
+
+	if (snprintf(path, len, "%s/dpa_size", decoder->dev_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+		    cxl_decoder_get_devname(decoder));
+		return -ENOMEM;
+	}
+
+	sprintf(buf, "%#llx\n", size);
+	rc = sysfs_write_attr(ctx, path, buf);
+	if (rc < 0)
+		return rc;
+
+	decoder->dpa_size = size;
+	return 0;
+}
+
+CXL_EXPORT int cxl_decoder_set_mode(struct cxl_decoder *decoder,
+				    enum cxl_decoder_mode mode)
+{
+	struct cxl_port *port = cxl_decoder_get_port(decoder);
+	struct cxl_ctx *ctx = cxl_decoder_get_ctx(decoder);
+	char *path = decoder->dev_buf;
+	int len = decoder->buf_len, rc;
+	char buf[SYSFS_ATTR_SIZE];
+
+	if (!cxl_port_is_endpoint(port)) {
+		err(ctx, "%s: not an endpoint decoder\n",
+		    cxl_decoder_get_devname(decoder));
+		return -EINVAL;
+	}
+
+	switch (mode) {
+	case CXL_DECODER_MODE_PMEM:
+		sprintf(buf, "pmem");
+		break;
+	case CXL_DECODER_MODE_RAM:
+		sprintf(buf, "ram");
+		break;
+	default:
+		err(ctx, "%s: unsupported mode: %d\n",
+		    cxl_decoder_get_devname(decoder), mode);
+		return -EINVAL;
+	}
+
+	if (snprintf(path, len, "%s/mode", decoder->dev_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+		    cxl_decoder_get_devname(decoder));
+		return -ENOMEM;
+	}
+
+	rc = sysfs_write_attr(ctx, path, buf);
+	if (rc < 0)
+		return rc;
+
+	decoder->mode = mode;
+	return 0;
+}
+
 CXL_EXPORT enum cxl_decoder_mode
 cxl_decoder_get_mode(struct cxl_decoder *decoder)
 {
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 88c5a7edd33e..7712de0c5010 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -173,4 +173,8 @@ global:
 	cxl_decoder_get_dpa_resource;
 	cxl_decoder_get_dpa_size;
 	cxl_decoder_get_mode;
+	cxl_decoder_get_last;
+	cxl_decoder_get_prev;
+	cxl_decoder_set_dpa_size;
+	cxl_decoder_set_mode;
 } LIBCXL_2;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 1436dc4601cc..33a216ee3a4c 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -139,6 +139,7 @@ enum cxl_decoder_mode {
 	CXL_DECODER_MODE_PMEM,
 	CXL_DECODER_MODE_RAM,
 };
+
 static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode)
 {
 	static const char *names[] = {
@@ -154,6 +155,10 @@ static inline const char *cxl_decoder_mode_name(enum cxl_decoder_mode mode)
 }
 
 enum cxl_decoder_mode cxl_decoder_get_mode(struct cxl_decoder *decoder);
+int cxl_decoder_set_mode(struct cxl_decoder *decoder,
+			 enum cxl_decoder_mode mode);
+int cxl_decoder_set_dpa_size(struct cxl_decoder *decoder,
+			     unsigned long long size);
 const char *cxl_decoder_get_devname(struct cxl_decoder *decoder);
 struct cxl_target *cxl_decoder_get_target_by_memdev(struct cxl_decoder *decoder,
 						    struct cxl_memdev *memdev);
@@ -182,6 +187,10 @@ bool cxl_decoder_is_locked(struct cxl_decoder *decoder);
 	for (decoder = cxl_decoder_get_first(port); decoder != NULL;           \
 	     decoder = cxl_decoder_get_next(decoder))
 
+#define cxl_decoder_foreach_reverse(port, decoder)                             \
+	for (decoder = cxl_decoder_get_last(port); decoder != NULL;           \
+	     decoder = cxl_decoder_get_prev(decoder))
+
 struct cxl_target;
 struct cxl_target *cxl_target_get_first(struct cxl_decoder *decoder);
 struct cxl_target *cxl_target_get_next(struct cxl_target *target);
diff --git a/cxl/memdev.c b/cxl/memdev.c
index 1cecad2dba4b..dc2f9da81153 100644
--- a/cxl/memdev.c
+++ b/cxl/memdev.c
@@ -33,6 +33,7 @@ static struct parameters {
 	bool align;
 	const char *type;
 	const char *size;
+	const char *decoder_filter;
 } param;
 
 static struct log_ctx ml;
@@ -71,6 +72,19 @@ OPT_STRING('s', "size",  &param.size, "size",			\
 OPT_BOOLEAN('a', "align",  &param.align,			\
 	"auto-align --size per device's requirement")
 
+#define RESERVE_DPA_OPTIONS()                                          \
+OPT_STRING('s', "size", &param.size, "size",                           \
+	   "size in bytes (Default: all available capacity)")
+
+#define DPA_OPTIONS()                                          \
+OPT_STRING('d', "decoder", &param.decoder_filter,              \
+   "decoder instance id",                                      \
+   "override the automatic decoder selection"),                \
+OPT_STRING('t', "type", &param.type, "type",                   \
+	   "'pmem' or 'ram' (volatile) (Default: 'pmem')"),    \
+OPT_BOOLEAN('f', "force", &param.force,                        \
+	    "Attempt 'expected to fail' operations")
+
 static const struct option read_options[] = {
 	BASE_OPTIONS(),
 	LABEL_OPTIONS(),
@@ -108,6 +122,238 @@ static const struct option set_partition_options[] = {
 	OPT_END(),
 };
 
+static const struct option reserve_dpa_options[] = {
+	BASE_OPTIONS(),
+	RESERVE_DPA_OPTIONS(),
+	DPA_OPTIONS(),
+	OPT_END(),
+};
+
+static const struct option free_dpa_options[] = {
+	BASE_OPTIONS(),
+	DPA_OPTIONS(),
+	OPT_END(),
+};
+
+enum reserve_dpa_mode {
+	DPA_ALLOC,
+	DPA_FREE,
+};
+
+static int __reserve_dpa(struct cxl_memdev *memdev,
+			 enum reserve_dpa_mode alloc_mode,
+			 struct action_context *actx)
+{
+	struct cxl_decoder *decoder, *auto_target = NULL, *target = NULL;
+	struct cxl_endpoint *endpoint = cxl_memdev_get_endpoint(memdev);
+	const char *devname = cxl_memdev_get_devname(memdev);
+	unsigned long long avail_dpa, size;
+	enum cxl_decoder_mode mode;
+	struct cxl_port *port;
+	char buf[256];
+	int rc;
+
+	if (param.type) {
+		if (strcmp(param.type, "ram") == 0)
+			mode = CXL_DECODER_MODE_RAM;
+		else if (strcmp(param.type, "volatile") == 0)
+			mode = CXL_DECODER_MODE_RAM;
+		else if (strcmp(param.type, "ram") == 0)
+			mode = CXL_DECODER_MODE_RAM;
+		else if (strcmp(param.type, "pmem") == 0)
+			mode = CXL_DECODER_MODE_PMEM;
+		else {
+			log_err(&ml, "%s: unsupported type: %s\n", devname,
+				param.type);
+			return -EINVAL;
+		}
+	} else
+		mode = CXL_DECODER_MODE_RAM;
+
+	if (!endpoint) {
+		log_err(&ml, "%s: CXL operation disabled\n", devname);
+		return -ENXIO;
+	}
+
+	port = cxl_endpoint_get_port(endpoint);
+
+	if (mode == CXL_DECODER_MODE_RAM)
+		avail_dpa = cxl_memdev_get_ram_size(memdev);
+	else
+		avail_dpa = cxl_memdev_get_pmem_size(memdev);
+
+	cxl_decoder_foreach(port, decoder) {
+		size = cxl_decoder_get_dpa_size(decoder);
+		if (size == ULLONG_MAX)
+			continue;
+		if (cxl_decoder_get_mode(decoder) != mode)
+			continue;
+
+		if (size > avail_dpa) {
+			log_err(&ml, "%s: capacity accounting error\n", devname);
+			return -ENXIO;
+		}
+		avail_dpa -= size;
+	}
+
+	if (!param.size)
+		if (alloc_mode == DPA_ALLOC) {
+			size = avail_dpa;
+			if (!avail_dpa) {
+				log_err(&ml, "%s: no available capacity\n", devname);
+				return -ENOSPC;
+			}
+		} else
+			size = 0;
+	else {
+		size = parse_size64(param.size);
+		if (size == ULLONG_MAX) {
+			log_err(&ml, "%s: failed to parse size option '%s'\n",
+				devname, param.size);
+			return -EINVAL;
+		}
+		if (size > avail_dpa) {
+			log_err(&ml, "%s: '%s' exceeds available capacity\n",
+				devname, param.size);
+			if (!param.force)
+				return -ENOSPC;
+		}
+	}
+
+	/*
+	 * Find next free decoder, assumes cxl_decoder_foreach() is in
+	 * hardware instance-id order
+	 */
+	if (alloc_mode == DPA_ALLOC)
+		cxl_decoder_foreach(port, decoder) {
+			/* first 0-dpa_size is our target */
+			if (cxl_decoder_get_dpa_size(decoder) == 0) {
+				auto_target = decoder;
+				break;
+			}
+		}
+	else
+		cxl_decoder_foreach_reverse(port, decoder) {
+			/* nothing to free? */
+			if (!cxl_decoder_get_dpa_size(decoder))
+				continue;
+			/*
+			 * Active decoders can't be freed, and by definition all
+			 * previous decoders must also be active
+			 */
+			if (cxl_decoder_get_size(decoder))
+				break;
+			/* first dpa_size > 0 + disabled decoder is our target */
+			if (cxl_decoder_get_dpa_size(decoder) < ULLONG_MAX) {
+				auto_target = decoder;
+				break;
+			}
+		}
+
+	if (param.decoder_filter) {
+		unsigned long id;
+		char *end;
+
+		id = strtoul(param.decoder_filter, &end, 0);
+		/* allow for standalone ordinal decoder ids */
+		if (*end == '\0')
+			rc = snprintf(buf, sizeof(buf), "decoder%d.%ld",
+				      cxl_port_get_id(port), id);
+		else
+			rc = snprintf(buf, sizeof(buf), "%s",
+				      param.decoder_filter);
+
+		if (rc >= (int) sizeof(buf)) {
+			log_err(&ml, "%s: decoder filter '%s' too long\n",
+				devname, param.decoder_filter);
+			return -EINVAL;
+		}
+
+		if (alloc_mode == DPA_ALLOC)
+			cxl_decoder_foreach(port, decoder) {
+				target = util_cxl_decoder_filter(decoder, buf);
+				if (target)
+					break;
+			}
+		else
+			cxl_decoder_foreach_reverse(port, decoder) {
+				target = util_cxl_decoder_filter(decoder, buf);
+				if (target)
+					break;
+			}
+
+		if (!target) {
+			log_err(&ml, "%s: no match for decoder: '%s'\n",
+				devname, param.decoder_filter);
+			return -ENXIO;
+		}
+
+		if (target != auto_target) {
+			log_err(&ml, "%s: %s is out of sequence\n", devname,
+				cxl_decoder_get_devname(target));
+			if (!param.force)
+				return -EINVAL;
+		}
+	}
+
+	if (!target)
+		target = auto_target;
+
+	if (!target) {
+		log_err(&ml, "%s: no suitable decoder found\n", devname);
+		return -ENXIO;
+	}
+
+	if (cxl_decoder_get_mode(target) != mode) {
+		rc = cxl_decoder_set_dpa_size(target, 0);
+		if (rc) {
+			log_err(&ml,
+				"%s: %s: failed to clear allocation to set mode\n",
+				devname, cxl_decoder_get_devname(target));
+			return rc;
+		}
+		rc = cxl_decoder_set_mode(target, mode);
+		if (rc) {
+			log_err(&ml, "%s: %s: failed to set %s mode\n", devname,
+				cxl_decoder_get_devname(target),
+				mode == CXL_DECODER_MODE_PMEM ? "pmem" : "ram");
+			return rc;
+		}
+	}
+
+	rc = cxl_decoder_set_dpa_size(target, size);
+	if (rc)
+		log_err(&ml, "%s: %s: failed to set dpa allocation\n", devname,
+			cxl_decoder_get_devname(target));
+	else {
+		struct json_object *jdev, *jdecoder;
+		unsigned long flags = 0;
+
+		if (actx->f_out == stdout && isatty(1))
+			flags |= UTIL_JSON_HUMAN;
+		jdev = util_cxl_memdev_to_json(memdev, flags);
+		jdecoder = util_cxl_decoder_to_json(target, flags);
+		if (!jdev || !jdecoder) {
+			json_object_put(jdev);
+			json_object_put(jdecoder);
+		} else {
+			json_object_object_add(jdev, "decoder", jdecoder);
+			json_object_array_add(actx->jdevs, jdev);
+		}
+	}
+	return rc;
+}
+
+static int action_reserve_dpa(struct cxl_memdev *memdev, struct action_context *actx)
+{
+	return __reserve_dpa(memdev, DPA_ALLOC, actx);
+}
+
+static int action_free_dpa(struct cxl_memdev *memdev, struct action_context *actx)
+{
+	return __reserve_dpa(memdev, DPA_FREE, actx);
+}
+
 static int action_disable(struct cxl_memdev *memdev, struct action_context *actx)
 {
 	if (!cxl_memdev_is_enabled(memdev))
@@ -452,7 +698,8 @@ static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx,
 		err++;
 	}
 
-	if (action == action_setpartition)
+	if (action == action_setpartition || action == action_reserve_dpa ||
+	    action == action_free_dpa)
 		actx.jdevs = json_object_new_array();
 
 	if (err == argc) {
@@ -495,6 +742,8 @@ static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx,
 	count = 0;
 
 	for (i = 0; i < argc; i++) {
+		bool found = false;
+
 		cxl_memdev_foreach(ctx, memdev) {
 			const char *memdev_filter = NULL;
 			const char *serial_filter = NULL;
@@ -507,6 +756,7 @@ static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx,
 			if (!util_cxl_memdev_filter(memdev, memdev_filter,
 						    serial_filter))
 				continue;
+			found = true;
 
 			if (action == action_write) {
 				single = memdev;
@@ -519,6 +769,8 @@ static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx,
 			else if (rc && !err)
 				err = rc;
 		}
+		if (!found)
+			log_info(&ml, "no memdev matches %s\n", argv[i]);
 	}
 	rc = err;
 
@@ -622,3 +874,25 @@ int cmd_set_partition(int argc, const char **argv, struct cxl_ctx *ctx)
 
 	return count >= 0 ? 0 : EXIT_FAILURE;
 }
+
+int cmd_reserve_dpa(int argc, const char **argv, struct cxl_ctx *ctx)
+{
+	int count = memdev_action(
+		argc, argv, ctx, action_reserve_dpa, reserve_dpa_options,
+		"cxl reserve-dpa <mem0> [<mem1>..<memn>] [<options>]");
+	log_info(&ml, "reservation completed on %d mem device%s\n",
+		 count >= 0 ? count : 0, count > 1 ? "s" : "");
+
+	return count >= 0 ? 0 : EXIT_FAILURE;
+}
+
+int cmd_free_dpa(int argc, const char **argv, struct cxl_ctx *ctx)
+{
+	int count = memdev_action(
+		argc, argv, ctx, action_free_dpa, free_dpa_options,
+		"cxl free-dpa <mem0> [<mem1>..<memn>] [<options>]");
+	log_info(&ml, "reservation release completed on %d mem device%s\n",
+		 count >= 0 ? count : 0, count > 1 ? "s" : "");
+
+	return count >= 0 ? 0 : EXIT_FAILURE;
+}

[ndctl PATCH 10/11] cxl/test: Update CXL memory parameters

[Dan Williams]

In support of testing CXL region configurations cxl_test changed the size
of its root decoders and endpoints. Use the size of the first root decoder
to determine if this is an updated kernel.

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

diff --git a/test/cxl-topology.sh b/test/cxl-topology.sh
index ff11614f4f14..2583005fef26 100644
--- a/test/cxl-topology.sh
+++ b/test/cxl-topology.sh
@@ -64,14 +64,9 @@ switch[2]=$(jq -r ".[] | .[\"ports:${bridge[1]}\"] | $port_sort | .[0].host" <<<
 switch[3]=$(jq -r ".[] | .[\"ports:${bridge[1]}\"] | $port_sort | .[1].host" <<< $json)
 
 
-# check that all 8 cxl_test memdevs are enabled by default and have a
-# pmem size of 256M
-json=$($CXL list -b cxl_test -M)
-count=$(jq "map(select(.pmem_size == $((256 << 20)))) | length" <<< $json)
-((count == 8)) || err "$LINENO"
-
-
 # validate the expected properties of the 4 root decoders
+# use the size of the first decoder to determine the cxl_test version /
+# properties
 json=$($CXL list -b cxl_test -D -d root)
 port_id=${root:4}
 port_id_len=${#port_id}
@@ -80,26 +75,41 @@ count=$(jq "[ $decoder_sort | .[0] |
 	select(.volatile_capable == true) |
 	select(.size == $((256 << 20))) |
 	select(.nr_targets == 1) ] | length" <<< $json)
-((count == 1)) || err "$LINENO"
+
+if [ $count -eq 1 ]; then
+	decoder_base_size=$((256 << 20))
+	pmem_size=$((256 << 20))
+else
+	decoder_base_size=$((1 << 30))
+	pmem_size=$((1 << 30))
+fi
 
 count=$(jq "[ $decoder_sort | .[1] |
 	select(.volatile_capable == true) |
-	select(.size == $((512 << 20))) |
+	select(.size == $((decoder_base_size * 2))) |
 	select(.nr_targets == 2) ] | length" <<< $json)
 ((count == 1)) || err "$LINENO"
 
 count=$(jq "[ $decoder_sort | .[2] |
 	select(.pmem_capable == true) |
-	select(.size == $((256 << 20))) |
+	select(.size == $decoder_base_size) |
 	select(.nr_targets == 1) ] | length" <<< $json)
 ((count == 1)) || err "$LINENO"
 
 count=$(jq "[ $decoder_sort | .[3] |
 	select(.pmem_capable == true) |
-	select(.size == $((512 << 20))) |
+	select(.size == $((decoder_base_size * 2))) |
 	select(.nr_targets == 2) ] | length" <<< $json)
 ((count == 1)) || err "$LINENO"
 
+
+# check that all 8 cxl_test memdevs are enabled by default and have a
+# pmem size of 256M, or 1G
+json=$($CXL list -b cxl_test -M)
+count=$(jq "map(select(.pmem_size == $pmem_size)) | length" <<< $json)
+((count == 8)) || err "$LINENO"
+
+
 # check that switch ports disappear after all of their memdevs have been
 # disabled, and return when the memdevs are enabled.
 for s in ${switch[@]}

[ndctl PATCH 11/11] cxl/test: Checkout region setup/teardown

[Dan Williams]

Exercise the fundamental region provisioning sysfs mechanisms of discovering
available DPA capacity, allocating DPA to a region, and programming HDM
decoders.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/cxl-region-create.sh |  122 +++++++++++++++++++++++++++++++++++++++++++++
 test/meson.build          |    2 +
 2 files changed, 124 insertions(+)
 create mode 100644 test/cxl-region-create.sh

diff --git a/test/cxl-region-create.sh b/test/cxl-region-create.sh
new file mode 100644
index 000000000000..389988759b08
--- /dev/null
+++ b/test/cxl-region-create.sh
@@ -0,0 +1,122 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2022 Intel Corporation. All rights reserved.
+
+. $(dirname $0)/common
+
+rc=1
+
+set -ex
+
+trap 'err $LINENO' ERR
+
+check_prereq "jq"
+
+modprobe -r cxl_test
+modprobe cxl_test
+udevadm settle
+
+# THEORY OF OPERATION: Create a x8 interleave across the pmem capacity
+# of the 8 endpoints defined by cxl_test, commit the decoders (which
+# just stubs out the actual hardware programming aspect, but updates the
+# driver state), and then tear it all down again. As with other cxl_test
+# tests if the CXL topology in tools/testing/cxl/test/cxl.c ever changes
+# then the paired update must be made to this test.
+
+# find the root decoder that spans both test host-bridges and support pmem
+decoder=$(cxl list -b cxl_test -D -d root | jq -r ".[] |
+	  select(.pmem_capable == true) |
+	  select(.nr_targets == 2) |
+	  .decoder")
+
+# find the memdevs mapped by that decoder
+readarray -t mem < <(cxl list -M -d $decoder | jq -r ".[].memdev")
+
+# ask cxl reserve-dpa to allocate pmem capacity from each of those memdevs
+readarray -t endpoint < <(cxl reserve-dpa -t pmem ${mem[*]} -s $((256<<20)) |
+			  jq -r ".[] | .decoder.decoder")
+
+# instantiate an empty region
+region=$(cat /sys/bus/cxl/devices/$decoder/create_pmem_region)
+echo $region > /sys/bus/cxl/devices/$decoder/create_pmem_region
+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
+echo $((nr_targets * (256<<20))) > /sys/bus/cxl/devices/$region/size
+
+# grab the list of memdevs grouped by 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")
+readarray -t mem_sort0 < <(cxl list -M -p $port_dev0 | jq -r ".[] | .memdev")
+readarray -t mem_sort1 < <(cxl list -M -p $port_dev1 | jq -r ".[] | .memdev")
+
+# TODO: add a cxl list option to list memdevs in valid region provisioning
+# order, hardcode for now.
+mem_sort=()
+mem_sort[0]=${mem_sort0[0]}
+mem_sort[1]=${mem_sort1[0]}
+mem_sort[2]=${mem_sort0[2]}
+mem_sort[3]=${mem_sort1[2]}
+mem_sort[4]=${mem_sort0[1]}
+mem_sort[5]=${mem_sort1[1]}
+mem_sort[6]=${mem_sort0[3]}
+mem_sort[7]=${mem_sort1[3]}
+
+# TODO: use this alternative memdev ordering to validate a negative test for
+# specifying invalid positions of memdevs
+#mem_sort[2]=${mem_sort0[0]}
+#mem_sort[1]=${mem_sort1[0]}
+#mem_sort[0]=${mem_sort0[2]}
+#mem_sort[3]=${mem_sort1[2]}
+#mem_sort[4]=${mem_sort0[1]}
+#mem_sort[5]=${mem_sort1[1]}
+#mem_sort[6]=${mem_sort0[3]}
+#mem_sort[7]=${mem_sort1[3]}
+
+# re-generate the list of endpoint decoders in region position programming order
+endpoint=()
+for i in ${mem_sort[@]}
+do
+	readarray -O ${#endpoint[@]} -t endpoint < <(cxl list -Di -d endpoint -m $i | jq -r ".[] |
+						     select(.mode == \"pmem\") | .decoder")
+done
+
+# attach all endpoint decoders to the region
+pos=0
+for i in ${endpoint[@]}
+do
+	echo $i > /sys/bus/cxl/devices/$region/target$pos
+	pos=$((pos+1))
+done
+echo "$region added ${#endpoint[@]} targets: ${endpoint[@]}"
+
+# walk up the topology and commit all decoders
+echo 1 > /sys/bus/cxl/devices/$region/commit
+
+# walk down the topology and de-commit all decoders
+echo 0 > /sys/bus/cxl/devices/$region/commit
+
+# remove endpoints from the region
+pos=0
+for i in ${endpoint[@]}
+do
+	echo "" > /sys/bus/cxl/devices/$region/target$pos
+	pos=$((pos+1))
+done
+
+# release DPA capacity
+readarray -t endpoint < <(cxl free-dpa -t pmem ${mem[*]} |
+			  jq -r ".[] | .decoder.decoder")
+echo "$region released ${#endpoint[@]} targets: ${endpoint[@]}"
+
+# validate no WARN or lockdep report during the run
+log=$(journalctl -r -k --since "-$((SECONDS+1))s")
+grep -q "Call Trace" <<< $log && err "$LINENO"
+
+modprobe -r cxl_test
diff --git a/test/meson.build b/test/meson.build
index 210dcb0b5ff1..fbcfc08d03ee 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -151,6 +151,7 @@ max_extent = find_program('max_available_extent_ns.sh')
 pfn_meta_errors = find_program('pfn-meta-errors.sh')
 track_uuid = find_program('track-uuid.sh')
 cxl_topo = find_program('cxl-topology.sh')
+cxl_region = find_program('cxl-region-create.sh')
 
 tests = [
   [ 'libndctl',               libndctl,		  'ndctl' ],
@@ -176,6 +177,7 @@ tests = [
   [ 'pfn-meta-errors.sh',     pfn_meta_errors,	  'ndctl' ],
   [ 'track-uuid.sh',          track_uuid,	  'ndctl' ],
   [ 'cxl-topology.sh',	      cxl_topo,		  'cxl'   ],
+  [ 'cxl-region-create.sh',   cxl_region,	  'cxl'   ],
 ]
 
 if get_option('destructive').enabled()

Re: [ndctl PATCH 11/11] cxl/test: Checkout region setup/teardown

[Verma, Vishal L]

On Tue, 2022-07-12 at 12:08 -0700, Dan Williams wrote:
> Exercise the fundamental region provisioning sysfs mechanisms of discovering
> available DPA capacity, allocating DPA to a region, and programming HDM
> decoders.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  test/cxl-region-create.sh |  122 +++++++++++++++++++++++++++++++++++++++++++++
>  test/meson.build          |    2 +
>  2 files changed, 124 insertions(+)
>  create mode 100644 test/cxl-region-create.sh

Since this isn't actually creating a region, should this be named
cxl-reserve-dpa.sh ?

Alternatively - I guess this test could just be extended to do actual
region creation once that is available in cxl-cli?

> 
> diff --git a/test/cxl-region-create.sh b/test/cxl-region-create.sh
> new file mode 100644
> index 000000000000..389988759b08
> --- /dev/null
> +++ b/test/cxl-region-create.sh
> @@ -0,0 +1,122 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2022 Intel Corporation. All rights reserved.
> +
> +. $(dirname $0)/common
> +
> +rc=1
> +
> +set -ex
> +
> +trap 'err $LINENO' ERR
> +
> +check_prereq "jq"
> +
> +modprobe -r cxl_test
> +modprobe cxl_test
> +udevadm settle
> +
> +# THEORY OF OPERATION: Create a x8 interleave across the pmem capacity
> +# of the 8 endpoints defined by cxl_test, commit the decoders (which
> +# just stubs out the actual hardware programming aspect, but updates the
> +# driver state), and then tear it all down again. As with other cxl_test
> +# tests if the CXL topology in tools/testing/cxl/test/cxl.c ever changes
> +# then the paired update must be made to this test.
> +
> +# find the root decoder that spans both test host-bridges and support pmem
> +decoder=$(cxl list -b cxl_test -D -d root | jq -r ".[] |

This and all following 'cxl' invocations should be "$CXL" which we get
from test/common

> +         select(.pmem_capable == true) |
> +         select(.nr_targets == 2) |
> +         .decoder")
> +
> +# find the memdevs mapped by that decoder
> +readarray -t mem < <(cxl list -M -d $decoder | jq -r ".[].memdev")
> +
> +# ask cxl reserve-dpa to allocate pmem capacity from each of those memdevs
> +readarray -t endpoint < <(cxl reserve-dpa -t pmem ${mem[*]} -s $((256<<20)) |
> +                         jq -r ".[] | .decoder.decoder")
> +
> +# instantiate an empty region
> +region=$(cat /sys/bus/cxl/devices/$decoder/create_pmem_region)
> +echo $region > /sys/bus/cxl/devices/$decoder/create_pmem_region
> +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
> +echo $((nr_targets * (256<<20))) > /sys/bus/cxl/devices/$region/size
> +
> +# grab the list of memdevs grouped by 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")
> +readarray -t mem_sort0 < <(cxl list -M -p $port_dev0 | jq -r ".[] | .memdev")
> +readarray -t mem_sort1 < <(cxl list -M -p $port_dev1 | jq -r ".[] | .memdev")
> +
> +# TODO: add a cxl list option to list memdevs in valid region provisioning
> +# order, hardcode for now.
> +mem_sort=()
> +mem_sort[0]=${mem_sort0[0]}
> +mem_sort[1]=${mem_sort1[0]}
> +mem_sort[2]=${mem_sort0[2]}
> +mem_sort[3]=${mem_sort1[2]}
> +mem_sort[4]=${mem_sort0[1]}
> +mem_sort[5]=${mem_sort1[1]}
> +mem_sort[6]=${mem_sort0[3]}
> +mem_sort[7]=${mem_sort1[3]}
> +
> +# TODO: use this alternative memdev ordering to validate a negative test for
> +# specifying invalid positions of memdevs
> +#mem_sort[2]=${mem_sort0[0]}
> +#mem_sort[1]=${mem_sort1[0]}
> +#mem_sort[0]=${mem_sort0[2]}
> +#mem_sort[3]=${mem_sort1[2]}
> +#mem_sort[4]=${mem_sort0[1]}
> +#mem_sort[5]=${mem_sort1[1]}
> +#mem_sort[6]=${mem_sort0[3]}
> +#mem_sort[7]=${mem_sort1[3]}
> +
> +# re-generate the list of endpoint decoders in region position programming order
> +endpoint=()
> +for i in ${mem_sort[@]}
> +do
> +       readarray -O ${#endpoint[@]} -t endpoint < <(cxl list -Di -d endpoint -m $i | jq -r ".[] |
> +                                                    select(.mode == \"pmem\") | .decoder")
> +done
> +
> +# attach all endpoint decoders to the region
> +pos=0
> +for i in ${endpoint[@]}
> +do
> +       echo $i > /sys/bus/cxl/devices/$region/target$pos
> +       pos=$((pos+1))
> +done
> +echo "$region added ${#endpoint[@]} targets: ${endpoint[@]}"
> +
> +# walk up the topology and commit all decoders
> +echo 1 > /sys/bus/cxl/devices/$region/commit
> +
> +# walk down the topology and de-commit all decoders
> +echo 0 > /sys/bus/cxl/devices/$region/commit
> +
> +# remove endpoints from the region
> +pos=0
> +for i in ${endpoint[@]}
> +do
> +       echo "" > /sys/bus/cxl/devices/$region/target$pos
> +       pos=$((pos+1))
> +done
> +
> +# release DPA capacity
> +readarray -t endpoint < <(cxl free-dpa -t pmem ${mem[*]} |
> +                         jq -r ".[] | .decoder.decoder")
> +echo "$region released ${#endpoint[@]} targets: ${endpoint[@]}"
> +
> +# validate no WARN or lockdep report during the run
> +log=$(journalctl -r -k --since "-$((SECONDS+1))s")
> +grep -q "Call Trace" <<< $log && err "$LINENO"
> +
> +modprobe -r cxl_test
> diff --git a/test/meson.build b/test/meson.build
> index 210dcb0b5ff1..fbcfc08d03ee 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -151,6 +151,7 @@ max_extent = find_program('max_available_extent_ns.sh')
>  pfn_meta_errors = find_program('pfn-meta-errors.sh')
>  track_uuid = find_program('track-uuid.sh')
>  cxl_topo = find_program('cxl-topology.sh')
> +cxl_region = find_program('cxl-region-create.sh')
>  
>  tests = [
>    [ 'libndctl',               libndctl,                  'ndctl' ],
> @@ -176,6 +177,7 @@ tests = [
>    [ 'pfn-meta-errors.sh',     pfn_meta_errors,   'ndctl' ],
>    [ 'track-uuid.sh',          track_uuid,        'ndctl' ],
>    [ 'cxl-topology.sh',       cxl_topo,           'cxl'   ],
> +  [ 'cxl-region-create.sh',   cxl_region,        'cxl'   ],
>  ]
>  
>  if get_option('destructive').enabled()
> 

Re: [ndctl PATCH 09/11] cxl/memdev: Add {reserve,free}-dpa commands

[Verma, Vishal L]

On Tue, 2022-07-12 at 12:08 -0700, Dan Williams wrote:
> Add helper commands for managing allocations of DPA (device physical
> address) capacity on a set of CXL memory devices.
> 
> The main convenience this command affords is automatically picking the next
> decoder to allocate per-memdev.
> 
> For example, to allocate 256MiB from all endpoints that are covered by a
> given root decoder, and collect those resulting endpoint-decoders into an
> array:
> 
>   readarray -t mem < <(cxl list -M -d $decoder | jq -r ".[].memdev")
>   readarray -t endpoint < <(cxl reserve-dpa -t pmem ${mem[*]} -s $((256<<20)) |
>                             jq -r ".[] | .decoder.decoder")
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  .clang-format                    |    1 
>  Documentation/cxl/lib/libcxl.txt |    2 

I guess the new commands are mostly for debug only - but should we add
man pages for them?

>  cxl/builtin.h                    |    2 
>  cxl/cxl.c                        |    2 
>  cxl/filter.c                     |    4 -
>  cxl/filter.h                     |    2 
>  cxl/lib/libcxl.c                 |   86 ++++++++++++
>  cxl/lib/libcxl.sym               |    4 +
>  cxl/libcxl.h                     |    9 +
>  cxl/memdev.c                     |  276 ++++++++++++++++++++++++++++++++++++++
>  10 files changed, 385 insertions(+), 3 deletions(-)
> 

<snip>

> 
> +
> +       if (cxl_decoder_get_mode(target) != mode) {
> +               rc = cxl_decoder_set_dpa_size(target, 0);
> +               if (rc) {
> +                       log_err(&ml,
> +                               "%s: %s: failed to clear allocation to set mode\n",
> +                               devname, cxl_decoder_get_devname(target));
> +                       return rc;
> +               }
> +               rc = cxl_decoder_set_mode(target, mode);
> +               if (rc) {
> +                       log_err(&ml, "%s: %s: failed to set %s mode\n", devname,
> +                               cxl_decoder_get_devname(target),
> +                               mode == CXL_DECODER_MODE_PMEM ? "pmem" : "ram");
> +                       return rc;
> +               }
> +       }
> +
> +       rc = cxl_decoder_set_dpa_size(target, size);
> +       if (rc)
> +               log_err(&ml, "%s: %s: failed to set dpa allocation\n", devname,

This patch adds some >80 col lines, which is fine by me - maybe we
should update .clang-format to 100 to make it official?

Re: [ndctl PATCH 05/11] cxl/lib: Maintain decoders in id order

[Ira Weiny]

On Tue, Jul 12, 2022 at 12:07:52PM -0700, Dan Williams wrote:
> Given that decoder instance order is fundamental to the DPA translation
> sequence for endpoint decoders, enforce that cxl_decoder_for_each() returns
> decoders in instance order. Otherwise, they show up in readddir() order
> which is not predictable.
> 
> Add a list_add_sorted() to generically handle inserting into a sorted list.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  cxl/lib/libcxl.c |    8 ++++++-
>  util/list.h      |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 68 insertions(+), 1 deletion(-)
> 
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index f36edcfc735a..e4c5d3819e88 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -19,6 +19,7 @@
>  #include <ccan/short_types/short_types.h>
>  
>  #include <util/log.h>
> +#include <util/list.h>
>  #include <util/size.h>
>  #include <util/sysfs.h>
>  #include <util/bitmap.h>
> @@ -908,6 +909,11 @@ cxl_endpoint_get_memdev(struct cxl_endpoint *endpoint)
>  	return NULL;
>  }
>  
> +static int decoder_id_cmp(struct cxl_decoder *d1, struct cxl_decoder *d2)
> +{
> +	return d1->id - d2->id;
> +}
> +
>  static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
>  {
>  	const char *devname = devpath_to_devname(cxldecoder_base);
> @@ -1049,7 +1055,7 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
>  			return decoder_dup;
>  		}
>  
> -	list_add(&port->decoders, &decoder->list);
> +	list_add_sorted(&port->decoders, decoder, list, decoder_id_cmp);
>  
>  	free(path);
>  	return decoder;
> diff --git a/util/list.h b/util/list.h
> index 1ea9c59b9f0c..c6584e3ec725 100644
> --- a/util/list.h
> +++ b/util/list.h
> @@ -37,4 +37,65 @@ static inline void list_add_after_(struct list_head *h,
>  	(void)list_debug(h, abortstr);
>  }
>  
> +/**
> + * list_add_before - add an entry before the given node in the linked list.
> + * @h: the list_head to add the node to
> + * @l: the list_node before which to add to
> + * @n: the list_node to add to the list.
> + *
> + * The list_node does not need to be initialized; it will be overwritten.
> + * Example:
> + *	struct child *child = malloc(sizeof(*child));
> + *
> + *	child->name = "geoffrey";
> + *	list_add_before(&parent->children, &child1->list, &child->list);
> + *	parent->num_children++;
> + */
> +#define list_add_before(h, l, n) list_add_before_(h, l, n, LIST_LOC)
> +static inline void list_add_before_(struct list_head *h, struct list_node *l,
> +				    struct list_node *n, const char *abortstr)

I see a list_add_before() in the latest ccan list code.[1]  Should we just use
that?  The implementation is a bit different.

[1] https://ccodearchive.net/info/list.html 

> +{
> +	if (l->prev == &h->n) {
> +		/* l is the first element, this becomes a list_add */
> +		list_add(h, n);
> +		return;
> +	}
> +
> +	n->next = l;
> +	n->prev = l->prev;
> +	l->prev->next = n;
> +	l->prev = n;

Did you mean to add list_debug() here?

Ira

> +}
> +
> +#define list_add_sorted(head, n, node, cmp)                                    \
> +	do {                                                                   \
> +		struct list_head *__head = (head);                             \
> +		typeof(n) __iter, __next;                                      \
> +		typeof(n) __new = (n);                                         \
> +                                                                               \
> +		if (list_empty(__head)) {                                      \
> +			list_add(__head, &__new->node);                        \
> +			break;                                                 \
> +		}                                                              \
> +                                                                               \
> +		list_for_each (__head, __iter, node) {                         \
> +			if (cmp(__new, __iter) < 0) {                          \
> +				list_add_before(__head, &__iter->node,         \
> +						&__new->node);                 \
> +				break;                                         \
> +			}                                                      \
> +			__next = list_next(__head, __iter, node);              \
> +			if (!__next) {                                         \
> +				list_add_after(__head, &__iter->node,          \
> +					       &__new->node);                  \
> +				break;                                         \
> +			}                                                      \
> +			if (cmp(__new, __next) < 0) {                          \
> +				list_add_before(__head, &__next->node,         \
> +						&__new->node);                 \
> +				break;                                         \
> +			}                                                      \
> +		}                                                              \
> +	} while (0)
> +
>  #endif /* _NDCTL_LIST_H_ */
> 
> 

Re: [ndctl PATCH 11/11] cxl/test: Checkout region setup/teardown

[Dan Williams]

Verma, Vishal L wrote:
> On Tue, 2022-07-12 at 12:08 -0700, Dan Williams wrote:
> > Exercise the fundamental region provisioning sysfs mechanisms of discovering
> > available DPA capacity, allocating DPA to a region, and programming HDM
> > decoders.
> > 
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  test/cxl-region-create.sh |  122 +++++++++++++++++++++++++++++++++++++++++++++
> >  test/meson.build          |    2 +
> >  2 files changed, 124 insertions(+)
> >  create mode 100644 test/cxl-region-create.sh
> 
> Since this isn't actually creating a region, should this be named
> cxl-reserve-dpa.sh ?

The test goes all the way to the point of registering a new region with
libnvdimm, so it is region creation.

> Alternatively - I guess this test could just be extended to do actual
> region creation once that is available in cxl-cli?

I was thinking that's a separate test that moves from just one hardcoded
pmem region via sysfs toggling to permuting all the possible cxl_test
region configurations across both modes via 'cxl create-region'. One is
a sysfs smoke test the other is a create-region unit test.

> 
> > 
> > diff --git a/test/cxl-region-create.sh b/test/cxl-region-create.sh
> > new file mode 100644
> > index 000000000000..389988759b08
> > --- /dev/null
> > +++ b/test/cxl-region-create.sh
> > @@ -0,0 +1,122 @@
> > +#!/bin/bash
> > +# SPDX-License-Identifier: GPL-2.0
> > +# Copyright (C) 2022 Intel Corporation. All rights reserved.
> > +
> > +. $(dirname $0)/common
> > +
> > +rc=1
> > +
> > +set -ex
> > +
> > +trap 'err $LINENO' ERR
> > +
> > +check_prereq "jq"
> > +
> > +modprobe -r cxl_test
> > +modprobe cxl_test
> > +udevadm settle
> > +
> > +# THEORY OF OPERATION: Create a x8 interleave across the pmem capacity
> > +# of the 8 endpoints defined by cxl_test, commit the decoders (which
> > +# just stubs out the actual hardware programming aspect, but updates the
> > +# driver state), and then tear it all down again. As with other cxl_test
> > +# tests if the CXL topology in tools/testing/cxl/test/cxl.c ever changes
> > +# then the paired update must be made to this test.
> > +
> > +# find the root decoder that spans both test host-bridges and support pmem
> > +decoder=$(cxl list -b cxl_test -D -d root | jq -r ".[] |
> 
> This and all following 'cxl' invocations should be "$CXL" which we get
> from test/common

Ah, yeah, missed that.

Re: [ndctl PATCH 11/11] cxl/test: Checkout region setup/teardown

[Verma, Vishal L]

On Wed, 2022-07-13 at 07:47 -0700, Dan Williams wrote:
> Verma, Vishal L wrote:
> > On Tue, 2022-07-12 at 12:08 -0700, Dan Williams wrote:
> > > Exercise the fundamental region provisioning sysfs mechanisms of discovering
> > > available DPA capacity, allocating DPA to a region, and programming HDM
> > > decoders.
> > > 
> > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > ---
> > >  test/cxl-region-create.sh |  122 +++++++++++++++++++++++++++++++++++++++++++++
> > >  test/meson.build          |    2 +
> > >  2 files changed, 124 insertions(+)
> > >  create mode 100644 test/cxl-region-create.sh
> > 
> > Since this isn't actually creating a region, should this be named
> > cxl-reserve-dpa.sh ?
> 
> The test goes all the way to the point of registering a new region with
> libnvdimm, so it is region creation.
> 
> > Alternatively - I guess this test could just be extended to do actual
> > region creation once that is available in cxl-cli?
> 
> I was thinking that's a separate test that moves from just one hardcoded
> pmem region via sysfs toggling to permuting all the possible cxl_test
> region configurations across both modes via 'cxl create-region'. One is
> a sysfs smoke test the other is a create-region unit test.

Ah okay makes sense - maybe this should be called cxl-region-sysfs to
reflect that?
> 

Re: [ndctl PATCH 09/11] cxl/memdev: Add {reserve,free}-dpa commands

[Dan Williams]

Verma, Vishal L wrote:
> On Tue, 2022-07-12 at 12:08 -0700, Dan Williams wrote:
> > Add helper commands for managing allocations of DPA (device physical
> > address) capacity on a set of CXL memory devices.
> > 
> > The main convenience this command affords is automatically picking the next
> > decoder to allocate per-memdev.
> > 
> > For example, to allocate 256MiB from all endpoints that are covered by a
> > given root decoder, and collect those resulting endpoint-decoders into an
> > array:
> > 
> >   readarray -t mem < <(cxl list -M -d $decoder | jq -r ".[].memdev")
> >   readarray -t endpoint < <(cxl reserve-dpa -t pmem ${mem[*]} -s $((256<<20)) |
> >                             jq -r ".[] | .decoder.decoder")
> > 
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  .clang-format                    |    1 
> >  Documentation/cxl/lib/libcxl.txt |    2 
> 
> I guess the new commands are mostly for debug only - but should we add
> man pages for them?

Oh whoops, yes, I missed that, will add.

> 
> >  cxl/builtin.h                    |    2 
> >  cxl/cxl.c                        |    2 
> >  cxl/filter.c                     |    4 -
> >  cxl/filter.h                     |    2 
> >  cxl/lib/libcxl.c                 |   86 ++++++++++++
> >  cxl/lib/libcxl.sym               |    4 +
> >  cxl/libcxl.h                     |    9 +
> >  cxl/memdev.c                     |  276 ++++++++++++++++++++++++++++++++++++++
> >  10 files changed, 385 insertions(+), 3 deletions(-)
> > 
> 
> <snip>
> 
> > 
> > +
> > +       if (cxl_decoder_get_mode(target) != mode) {
> > +               rc = cxl_decoder_set_dpa_size(target, 0);
> > +               if (rc) {
> > +                       log_err(&ml,
> > +                               "%s: %s: failed to clear allocation to set mode\n",
> > +                               devname, cxl_decoder_get_devname(target));
> > +                       return rc;
> > +               }
> > +               rc = cxl_decoder_set_mode(target, mode);
> > +               if (rc) {
> > +                       log_err(&ml, "%s: %s: failed to set %s mode\n", devname,
> > +                               cxl_decoder_get_devname(target),
> > +                               mode == CXL_DECODER_MODE_PMEM ? "pmem" : "ram");
> > +                       return rc;
> > +               }
> > +       }
> > +
> > +       rc = cxl_decoder_set_dpa_size(target, size);
> > +       if (rc)
> > +               log_err(&ml, "%s: %s: failed to set dpa allocation\n", devname,
> 
> This patch adds some >80 col lines, which is fine by me - maybe we
> should update .clang-format to 100 to make it official?

.clang_format does not break up print format strings that span 80
columns. Same as the kernel. So those are properly formatted as the
non-format string portions of those print statements stay <= 80.

Re: [ndctl PATCH 09/11] cxl/memdev: Add {reserve,free}-dpa commands

[Verma, Vishal L]

On Wed, 2022-07-13 at 08:22 -0700, Dan Williams wrote:
> Verma, Vishal L wrote:
> > On Tue, 2022-07-12 at 12:08 -0700, Dan Williams wrote:
> > 
> > > 
> > > +
> > > +       if (cxl_decoder_get_mode(target) != mode) {
> > > +               rc = cxl_decoder_set_dpa_size(target, 0);
> > > +               if (rc) {
> > > +                       log_err(&ml,
> > > +                               "%s: %s: failed to clear allocation to set mode\n",
> > > +                               devname, cxl_decoder_get_devname(target));
> > > +                       return rc;
> > > +               }
> > > +               rc = cxl_decoder_set_mode(target, mode);
> > > +               if (rc) {
> > > +                       log_err(&ml, "%s: %s: failed to set %s mode\n", devname,
> > > +                               cxl_decoder_get_devname(target),
> > > +                               mode == CXL_DECODER_MODE_PMEM ? "pmem" : "ram");
> > > +                       return rc;
> > > +               }
> > > +       }
> > > +
> > > +       rc = cxl_decoder_set_dpa_size(target, size);
> > > +       if (rc)
> > > +               log_err(&ml, "%s: %s: failed to set dpa allocation\n", devname,
> > 
> > This patch adds some >80 col lines, which is fine by me - maybe we
> > should update .clang-format to 100 to make it official?
> 
> .clang_format does not break up print format strings that span 80
> columns. Same as the kernel. So those are properly formatted as the
> non-format string portions of those print statements stay <= 80.

Oh sure - though I thought it would at least drop the " devname" to the
next line. (Just checked, looked like it does).

There were some other non-string long lines as well, e.g.:

+static int action_reserve_dpa(struct cxl_memdev *memdev, struct action_context *actx)

Re: [ndctl PATCH 05/11] cxl/lib: Maintain decoders in id order

[Dan Williams]

Ira Weiny wrote:
> On Tue, Jul 12, 2022 at 12:07:52PM -0700, Dan Williams wrote:
> > Given that decoder instance order is fundamental to the DPA translation
> > sequence for endpoint decoders, enforce that cxl_decoder_for_each() returns
> > decoders in instance order. Otherwise, they show up in readddir() order
> > which is not predictable.
> > 
> > Add a list_add_sorted() to generically handle inserting into a sorted list.
> > 
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > ---
> >  cxl/lib/libcxl.c |    8 ++++++-
> >  util/list.h      |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 68 insertions(+), 1 deletion(-)
> > 
> > diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> > index f36edcfc735a..e4c5d3819e88 100644
> > --- a/cxl/lib/libcxl.c
> > +++ b/cxl/lib/libcxl.c
> > @@ -19,6 +19,7 @@
> >  #include <ccan/short_types/short_types.h>
> >  
> >  #include <util/log.h>
> > +#include <util/list.h>
> >  #include <util/size.h>
> >  #include <util/sysfs.h>
> >  #include <util/bitmap.h>
> > @@ -908,6 +909,11 @@ cxl_endpoint_get_memdev(struct cxl_endpoint *endpoint)
> >  	return NULL;
> >  }
> >  
> > +static int decoder_id_cmp(struct cxl_decoder *d1, struct cxl_decoder *d2)
> > +{
> > +	return d1->id - d2->id;
> > +}
> > +
> >  static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
> >  {
> >  	const char *devname = devpath_to_devname(cxldecoder_base);
> > @@ -1049,7 +1055,7 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
> >  			return decoder_dup;
> >  		}
> >  
> > -	list_add(&port->decoders, &decoder->list);
> > +	list_add_sorted(&port->decoders, decoder, list, decoder_id_cmp);
> >  
> >  	free(path);
> >  	return decoder;
> > diff --git a/util/list.h b/util/list.h
> > index 1ea9c59b9f0c..c6584e3ec725 100644
> > --- a/util/list.h
> > +++ b/util/list.h
> > @@ -37,4 +37,65 @@ static inline void list_add_after_(struct list_head *h,
> >  	(void)list_debug(h, abortstr);
> >  }
> >  
> > +/**
> > + * list_add_before - add an entry before the given node in the linked list.
> > + * @h: the list_head to add the node to
> > + * @l: the list_node before which to add to
> > + * @n: the list_node to add to the list.
> > + *
> > + * The list_node does not need to be initialized; it will be overwritten.
> > + * Example:
> > + *	struct child *child = malloc(sizeof(*child));
> > + *
> > + *	child->name = "geoffrey";
> > + *	list_add_before(&parent->children, &child1->list, &child->list);
> > + *	parent->num_children++;
> > + */
> > +#define list_add_before(h, l, n) list_add_before_(h, l, n, LIST_LOC)
> > +static inline void list_add_before_(struct list_head *h, struct list_node *l,
> > +				    struct list_node *n, const char *abortstr)
> 
> I see a list_add_before() in the latest ccan list code.[1]  Should we just use
> that?  The implementation is a bit different.
> 
> [1] https://ccodearchive.net/info/list.html 

Ah, thanks for checking. Yeah, probably time to refresh our fork to the
latest upstream baseline.

> 
> > +{
> > +	if (l->prev == &h->n) {
> > +		/* l is the first element, this becomes a list_add */
> > +		list_add(h, n);
> > +		return;
> > +	}
> > +
> > +	n->next = l;
> > +	n->prev = l->prev;
> > +	l->prev->next = n;
> > +	l->prev = n;
> 
> Did you mean to add list_debug() here?

Looks like we get that for free with the upstream rebase.

Re: [ndctl PATCH 11/11] cxl/test: Checkout region setup/teardown

[Dan Williams]

Verma, Vishal L wrote:
> On Wed, 2022-07-13 at 07:47 -0700, Dan Williams wrote:
> > Verma, Vishal L wrote:
> > > On Tue, 2022-07-12 at 12:08 -0700, Dan Williams wrote:
> > > > Exercise the fundamental region provisioning sysfs mechanisms of discovering
> > > > available DPA capacity, allocating DPA to a region, and programming HDM
> > > > decoders.
> > > > 
> > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> > > > ---
> > > >  test/cxl-region-create.sh |  122 +++++++++++++++++++++++++++++++++++++++++++++
> > > >  test/meson.build          |    2 +
> > > >  2 files changed, 124 insertions(+)
> > > >  create mode 100644 test/cxl-region-create.sh
> > > 
> > > Since this isn't actually creating a region, should this be named
> > > cxl-reserve-dpa.sh ?
> > 
> > The test goes all the way to the point of registering a new region with
> > libnvdimm, so it is region creation.
> > 
> > > Alternatively - I guess this test could just be extended to do actual
> > > region creation once that is available in cxl-cli?
> > 
> > I was thinking that's a separate test that moves from just one hardcoded
> > pmem region via sysfs toggling to permuting all the possible cxl_test
> > region configurations across both modes via 'cxl create-region'. One is
> > a sysfs smoke test the other is a create-region unit test.
> 
> Ah okay makes sense - maybe this should be called cxl-region-sysfs to
> reflect that?

Sure, sounds good.

Re: [ndctl PATCH 09/11] cxl/memdev: Add {reserve,free}-dpa commands

[Dan Williams]

Verma, Vishal L wrote:
> On Wed, 2022-07-13 at 08:22 -0700, Dan Williams wrote:
> > Verma, Vishal L wrote:
> > > On Tue, 2022-07-12 at 12:08 -0700, Dan Williams wrote:
> > > 
> > > > 
> > > > +
> > > > +       if (cxl_decoder_get_mode(target) != mode) {
> > > > +               rc = cxl_decoder_set_dpa_size(target, 0);
> > > > +               if (rc) {
> > > > +                       log_err(&ml,
> > > > +                               "%s: %s: failed to clear allocation to set mode\n",
> > > > +                               devname, cxl_decoder_get_devname(target));
> > > > +                       return rc;
> > > > +               }
> > > > +               rc = cxl_decoder_set_mode(target, mode);
> > > > +               if (rc) {
> > > > +                       log_err(&ml, "%s: %s: failed to set %s mode\n", devname,
> > > > +                               cxl_decoder_get_devname(target),
> > > > +                               mode == CXL_DECODER_MODE_PMEM ? "pmem" : "ram");
> > > > +                       return rc;
> > > > +               }
> > > > +       }
> > > > +
> > > > +       rc = cxl_decoder_set_dpa_size(target, size);
> > > > +       if (rc)
> > > > +               log_err(&ml, "%s: %s: failed to set dpa allocation\n", devname,
> > > 
> > > This patch adds some >80 col lines, which is fine by me - maybe we
> > > should update .clang-format to 100 to make it official?
> > 
> > .clang_format does not break up print format strings that span 80
> > columns. Same as the kernel. So those are properly formatted as the
> > non-format string portions of those print statements stay <= 80.
> 
> Oh sure - though I thought it would at least drop the " devname" to the
> next line. (Just checked, looked like it does).

Of course I only checked the 2 previous log_err() in the quote and not
the one you directly commented on, will reflow.

> 
> There were some other non-string long lines as well, e.g.:
> 
> +static int action_reserve_dpa(struct cxl_memdev *memdev, struct action_context *actx)

Ok, will reflow that as well.

Re: [PATCH 1/11] cxl/list: Reformat option list

[Davidlohr Bueso]

On Tue, 12 Jul 2022, Dan Williams wrote:

>Cleanup some spurious spaces and let clang-format re-layout the options.

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

>
>Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>---
> cxl/list.c |    9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
>diff --git a/cxl/list.c b/cxl/list.c
>index 940782d33a10..1b5f58328047 100644
>--- a/cxl/list.c
>+++ b/cxl/list.c
>@@ -36,8 +36,7 @@ static const struct option options[] = {
>		   "filter by CXL endpoint device name(s)"),
>	OPT_BOOLEAN('E', "endpoints", &param.endpoints,
>		    "include CXL endpoint info"),
>-	OPT_STRING('d', "decoder", &param.decoder_filter,
>-		   "decoder device name",
>+	OPT_STRING('d', "decoder", &param.decoder_filter, "decoder device name",
>		   "filter by CXL decoder device name(s) / class"),
>	OPT_BOOLEAN('D', "decoders", &param.decoders,
>		    "include CXL decoder info"),
>@@ -45,11 +44,11 @@ static const struct option options[] = {
>		    "include CXL target data with decoders or ports"),
>	OPT_BOOLEAN('i', "idle", &param.idle, "include disabled devices"),
>	OPT_BOOLEAN('u', "human", &param.human,
>-		    "use human friendly number formats "),
>+		    "use human friendly number formats"),
>	OPT_BOOLEAN('H', "health", &param.health,
>-		    "include memory device health information "),
>+		    "include memory device health information"),
>	OPT_BOOLEAN('I', "partition", &param.partition,
>-		    "include memory device partition information "),
>+		    "include memory device partition information"),
> #ifdef ENABLE_DEBUG
>	OPT_BOOLEAN(0, "debug", &debug, "debug list walk"),
> #endif
>

Re: [PATCH 8/11] cxl/set-partition: Accept 'ram' as an alias for 'volatile'

[Davidlohr Bueso]

On Tue, 12 Jul 2022, Dan Williams wrote:

>'ram' is a more convenient shorthand for volatile memory.

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

>
>Cc: Alison Schofield <alison.schofield@intel.com>
>Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>---
> Documentation/cxl/cxl-set-partition.txt |    2 +-
> cxl/memdev.c                            |    4 +++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/Documentation/cxl/cxl-set-partition.txt b/Documentation/cxl/cxl-set-partition.txt
>index 1e548af77da2..f0126daf808b 100644
>--- a/Documentation/cxl/cxl-set-partition.txt
>+++ b/Documentation/cxl/cxl-set-partition.txt
>@@ -37,7 +37,7 @@ include::memdev-option.txt[]
>
> -t::
> --type=::
>-	Type of partition, 'pmem' or 'volatile', to modify.
>+	Type of partition, 'pmem' or 'ram' (volatile), to modify.
>	Default: 'pmem'
>
> -s::
>diff --git a/cxl/memdev.c b/cxl/memdev.c
>index 9fcd8ae5724b..1cecad2dba4b 100644
>--- a/cxl/memdev.c
>+++ b/cxl/memdev.c
>@@ -65,7 +65,7 @@ OPT_BOOLEAN('f', "force", &param.force,                                \
>
> #define SET_PARTITION_OPTIONS() \
> OPT_STRING('t', "type",  &param.type, "type",			\
>-	"'pmem' or 'volatile' (Default: 'pmem')"),		\
>+	"'pmem' or 'ram' (volatile) (Default: 'pmem')"),		\
> OPT_STRING('s', "size",  &param.size, "size",			\
>	"size in bytes (Default: all available capacity)"),	\
> OPT_BOOLEAN('a', "align",  &param.align,			\
>@@ -355,6 +355,8 @@ static int action_setpartition(struct cxl_memdev *memdev,
>			/* default */;
>		else if (strcmp(param.type, "volatile") == 0)
>			type = CXL_SETPART_VOLATILE;
>+		else if (strcmp(param.type, "ram") == 0)
>+			type = CXL_SETPART_VOLATILE;
>		else {
>			log_err(&ml, "invalid type '%s'\n", param.type);
>			return -EINVAL;
>

Re: [PATCH 3/11] cxl/list: Hide 0s in disabled decoder listings

[Davidlohr Bueso]

On Tue, 12 Jul 2022, Dan Williams wrote:

>Trim some redundant information from decoder listings when they are
>disabled.
>

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

>Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>---
> cxl/json.c |   12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
>diff --git a/cxl/json.c b/cxl/json.c
>index fdc6f73a86c1..a213fdad55fd 100644
>--- a/cxl/json.c
>+++ b/cxl/json.c
>@@ -442,7 +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;
>-	u64 val;
>+	u64 val, size;
>
>	jdecoder = json_object_new_object();
>	if (!jdecoder)
>@@ -452,21 +452,21 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
>	if (jobj)
>		json_object_object_add(jdecoder, "decoder", jobj);
>
>+	size = cxl_decoder_get_size(decoder);
>	val = cxl_decoder_get_resource(decoder);
>-	if (val < ULLONG_MAX) {
>+	if (size && val < ULLONG_MAX) {
>		jobj = util_json_object_hex(val, flags);
>		if (jobj)
>			json_object_object_add(jdecoder, "resource", jobj);
>	}
>
>-	val = cxl_decoder_get_size(decoder);
>-	if (val < ULLONG_MAX) {
>-		jobj = util_json_object_size(val, flags);
>+	if (size && size < ULLONG_MAX) {
>+		jobj = util_json_object_size(size, flags);
>		if (jobj)
>			json_object_object_add(jdecoder, "size", jobj);
>	}
>
>-	if (val == 0) {
>+	if (size == 0) {
>		jobj = json_object_new_string("disabled");
>		if (jobj)
>			json_object_object_add(jdecoder, "state", jobj);
>

Re: [PATCH 5/11] cxl/lib: Maintain decoders in id order

[Davidlohr Bueso]

On Tue, 12 Jul 2022, Dan Williams wrote:

>Given that decoder instance order is fundamental to the DPA translation
>sequence for endpoint decoders, enforce that cxl_decoder_for_each() returns
>decoders in instance order. Otherwise, they show up in readddir() order
>which is not predictable.
>
>Add a list_add_sorted() to generically handle inserting into a sorted list.

With the already available list_add_before ccan code nit already pointed out
by Ira.

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

>
>Signed-off-by: Dan Williams <dan.j.williams@intel.com>
>---
> cxl/lib/libcxl.c |    8 ++++++-
> util/list.h      |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 68 insertions(+), 1 deletion(-)
>
>diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
>index f36edcfc735a..e4c5d3819e88 100644
>--- a/cxl/lib/libcxl.c
>+++ b/cxl/lib/libcxl.c
>@@ -19,6 +19,7 @@
> #include <ccan/short_types/short_types.h>
>
> #include <util/log.h>
>+#include <util/list.h>
> #include <util/size.h>
> #include <util/sysfs.h>
> #include <util/bitmap.h>
>@@ -908,6 +909,11 @@ cxl_endpoint_get_memdev(struct cxl_endpoint *endpoint)
> 	return NULL;
> }
>
>+static int decoder_id_cmp(struct cxl_decoder *d1, struct cxl_decoder *d2)
>+{
>+	return d1->id - d2->id;
>+}
>+
> static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
> {
> 	const char *devname = devpath_to_devname(cxldecoder_base);
>@@ -1049,7 +1055,7 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
> 			return decoder_dup;
> 		}
>
>-	list_add(&port->decoders, &decoder->list);
>+	list_add_sorted(&port->decoders, decoder, list, decoder_id_cmp);
>
> 	free(path);
> 	return decoder;
>diff --git a/util/list.h b/util/list.h
>index 1ea9c59b9f0c..c6584e3ec725 100644
>--- a/util/list.h
>+++ b/util/list.h
>@@ -37,4 +37,65 @@ static inline void list_add_after_(struct list_head *h,
> 	(void)list_debug(h, abortstr);
> }
>
>+/**
>+ * list_add_before - add an entry before the given node in the linked list.
>+ * @h: the list_head to add the node to
>+ * @l: the list_node before which to add to
>+ * @n: the list_node to add to the list.
>+ *
>+ * The list_node does not need to be initialized; it will be overwritten.
>+ * Example:
>+ *	struct child *child = malloc(sizeof(*child));
>+ *
>+ *	child->name = "geoffrey";
>+ *	list_add_before(&parent->children, &child1->list, &child->list);
>+ *	parent->num_children++;
>+ */
>+#define list_add_before(h, l, n) list_add_before_(h, l, n, LIST_LOC)
>+static inline void list_add_before_(struct list_head *h, struct list_node *l,
>+				    struct list_node *n, const char *abortstr)
>+{
>+	if (l->prev == &h->n) {
>+		/* l is the first element, this becomes a list_add */
>+		list_add(h, n);
>+		return;
>+	}
>+
>+	n->next = l;
>+	n->prev = l->prev;
>+	l->prev->next = n;
>+	l->prev = n;
>+}
>+
>+#define list_add_sorted(head, n, node, cmp)                                    \
>+	do {                                                                   \
>+		struct list_head *__head = (head);                             \
>+		typeof(n) __iter, __next;                                      \
>+		typeof(n) __new = (n);                                         \
>+                                                                               \
>+		if (list_empty(__head)) {                                      \
>+			list_add(__head, &__new->node);                        \
>+			break;                                                 \
>+		}                                                              \
>+                                                                               \
>+		list_for_each (__head, __iter, node) {                         \
>+			if (cmp(__new, __iter) < 0) {                          \
>+				list_add_before(__head, &__iter->node,         \
>+						&__new->node);                 \
>+				break;                                         \
>+			}                                                      \
>+			__next = list_next(__head, __iter, node);              \
>+			if (!__next) {                                         \
>+				list_add_after(__head, &__iter->node,          \
>+					       &__new->node);                  \
>+				break;                                         \
>+			}                                                      \
>+			if (cmp(__new, __next) < 0) {                          \
>+				list_add_before(__head, &__next->node,         \
>+						&__new->node);                 \
>+				break;                                         \
>+			}                                                      \
>+		}                                                              \
>+	} while (0)
>+
> #endif /* _NDCTL_LIST_H_ */
>

Re: [PATCH 5/11] cxl/lib: Maintain decoders in id order

[Dan Williams]

Davidlohr Bueso wrote:
> On Tue, 12 Jul 2022, Dan Williams wrote:
> 
> >Given that decoder instance order is fundamental to the DPA translation
> >sequence for endpoint decoders, enforce that cxl_decoder_for_each() returns
> >decoders in instance order. Otherwise, they show up in readddir() order
> >which is not predictable.
> >
> >Add a list_add_sorted() to generically handle inserting into a sorted list.
> 
> With the already available list_add_before ccan code nit already pointed out
> by Ira.

Done.

> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

Thanks!