All of lore.kernel.org
 help / color / mirror / Atom feed
* [ndctl PATCH v2 00/12] cxl: Region provisioning foundation
@ 2022-07-14 17:01 Dan Williams
  2022-07-14 17:01 ` [ndctl PATCH v2 01/12] cxl/list: Reformat option list Dan Williams
                   ` (11 more replies)
  0 siblings, 12 replies; 18+ messages in thread
From: Dan Williams @ 2022-07-14 17:01 UTC (permalink / raw)
  To: vishal.l.verma
  Cc: Ira Weiny, Alison Schofield, Davidlohr Bueso, nvdimm, linux-cxl

Changes since v1 [1]:
- Update ccan/list to the latest (Ira, Davidlohr)
- Collect some reviewed-bys (Davidlohr)
- Add man pages for cxl {reserve,free}-dpa (Vishal)
- Reflow some clang-format escapes (Vishal)
- Rename test/cxl-create-region.sh to test/cxl-region-sysfs.h (Vishal)
- Use $CXL in cxl-region-sysfs.sh (Vishal)
[1]: https://lore.kernel.org/r/165765284365.435671.13173937566404931163.stgit@dwillia2-xfh/

---

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-region-sysfs.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-sysfs.sh as an example of
how the ABI is used.

---

Dan Williams (12):
      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
      ccan/list: Import latest list helpers
      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-free-dpa.txt      |   53 +++++
 Documentation/cxl/cxl-reserve-dpa.txt   |   67 +++++++
 Documentation/cxl/cxl-set-partition.txt |    2 
 Documentation/cxl/lib/libcxl.txt        |   13 +
 Documentation/cxl/meson.build           |    2 
 ccan/list/list.h                        |  258 ++++++++++++++++++++++----
 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                            |  308 ++++++++++++++++++++++++++++++-
 ndctl/lib/inject.c                      |    1 
 test/cxl-region-sysfs.sh                |  122 ++++++++++++
 test/cxl-topology.sh                    |   32 ++-
 test/meson.build                        |    2 
 util/list.h                             |   63 +++---
 23 files changed, 1102 insertions(+), 102 deletions(-)
 create mode 100644 Documentation/cxl/cxl-free-dpa.txt
 create mode 100644 Documentation/cxl/cxl-reserve-dpa.txt
 create mode 100644 test/cxl-region-sysfs.sh

base-commit: bbb2cb56f08d95ecf2c7c047a33cc3dd64eb7fde

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [ndctl PATCH v2 01/12] cxl/list: Reformat option list
  2022-07-14 17:01 [ndctl PATCH v2 00/12] cxl: Region provisioning foundation Dan Williams
@ 2022-07-14 17:01 ` Dan Williams
  2022-07-14 17:01 ` [ndctl PATCH v2 02/12] cxl/list: Emit endpoint decoders filtered by memdev Dan Williams
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Dan Williams @ 2022-07-14 17:01 UTC (permalink / raw)
  To: vishal.l.verma; +Cc: Davidlohr Bueso, alison.schofield, nvdimm, linux-cxl

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


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [ndctl PATCH v2 02/12] cxl/list: Emit endpoint decoders filtered by memdev
  2022-07-14 17:01 [ndctl PATCH v2 00/12] cxl: Region provisioning foundation Dan Williams
  2022-07-14 17:01 ` [ndctl PATCH v2 01/12] cxl/list: Reformat option list Dan Williams
@ 2022-07-14 17:01 ` Dan Williams
  2022-07-14 17:02 ` [ndctl PATCH v2 03/12] cxl/list: Hide 0s in disabled decoder listings Dan Williams
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Dan Williams @ 2022-07-14 17:01 UTC (permalink / raw)
  To: vishal.l.verma; +Cc: alison.schofield, nvdimm, linux-cxl

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;


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [ndctl PATCH v2 03/12] cxl/list: Hide 0s in disabled decoder listings
  2022-07-14 17:01 [ndctl PATCH v2 00/12] cxl: Region provisioning foundation Dan Williams
  2022-07-14 17:01 ` [ndctl PATCH v2 01/12] cxl/list: Reformat option list Dan Williams
  2022-07-14 17:01 ` [ndctl PATCH v2 02/12] cxl/list: Emit endpoint decoders filtered by memdev Dan Williams
@ 2022-07-14 17:02 ` Dan Williams
  2022-07-14 17:02 ` [ndctl PATCH v2 04/12] cxl/list: Add DPA span to endpoint " Dan Williams
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Dan Williams @ 2022-07-14 17:02 UTC (permalink / raw)
  To: vishal.l.verma; +Cc: Davidlohr Bueso, alison.schofield, nvdimm, linux-cxl

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);


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [ndctl PATCH v2 04/12] cxl/list: Add DPA span to endpoint decoder listings
  2022-07-14 17:01 [ndctl PATCH v2 00/12] cxl: Region provisioning foundation Dan Williams
                   ` (2 preceding siblings ...)
  2022-07-14 17:02 ` [ndctl PATCH v2 03/12] cxl/list: Hide 0s in disabled decoder listings Dan Williams
@ 2022-07-14 17:02 ` Dan Williams
  2022-07-14 17:02 ` [ndctl PATCH v2 05/12] ccan/list: Import latest list helpers Dan Williams
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Dan Williams @ 2022-07-14 17:02 UTC (permalink / raw)
  To: vishal.l.verma; +Cc: alison.schofield, nvdimm, linux-cxl

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);


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [ndctl PATCH v2 05/12] ccan/list: Import latest list helpers
  2022-07-14 17:01 [ndctl PATCH v2 00/12] cxl: Region provisioning foundation Dan Williams
                   ` (3 preceding siblings ...)
  2022-07-14 17:02 ` [ndctl PATCH v2 04/12] cxl/list: Add DPA span to endpoint " Dan Williams
@ 2022-07-14 17:02 ` Dan Williams
  2022-07-14 17:20   ` Ira Weiny
  2022-07-14 17:02 ` [ndctl PATCH v2 06/12] cxl/lib: Maintain decoders in id order Dan Williams
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 18+ messages in thread
From: Dan Williams @ 2022-07-14 17:02 UTC (permalink / raw)
  To: vishal.l.verma; +Cc: Ira Weiny, alison.schofield, nvdimm, linux-cxl

Pick up the definition of list_add_{before,after} and other updates from
ccan at commit 52b86922f846 ("ccan/base64: fix GCC warning.").

Reported-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 ccan/list/list.h   |  258 +++++++++++++++++++++++++++++++++++++++++++++-------
 ndctl/lib/inject.c |    1 
 util/list.h        |   40 --------
 3 files changed, 222 insertions(+), 77 deletions(-)
 delete mode 100644 util/list.h

diff --git a/ccan/list/list.h b/ccan/list/list.h
index 3ebd1b23dc0f..15f5fb7b34eb 100644
--- a/ccan/list/list.h
+++ b/ccan/list/list.h
@@ -95,8 +95,8 @@ struct list_node *list_check_node(const struct list_node *n,
 #define list_debug(h, loc) list_check((h), loc)
 #define list_debug_node(n, loc) list_check_node((n), loc)
 #else
-#define list_debug(h, loc) (h)
-#define list_debug_node(n, loc) (n)
+#define list_debug(h, loc) ((void)loc, h)
+#define list_debug_node(n, loc) ((void)loc, n)
 #endif
 
 /**
@@ -111,7 +111,7 @@ struct list_node *list_check_node(const struct list_node *n,
  * Example:
  *	static struct list_head my_list = LIST_HEAD_INIT(my_list);
  */
-#define LIST_HEAD_INIT(name) { { &name.n, &name.n } }
+#define LIST_HEAD_INIT(name) { { &(name).n, &(name).n } }
 
 /**
  * LIST_HEAD - define and initialize an empty list_head
@@ -145,6 +145,48 @@ static inline void list_head_init(struct list_head *h)
 	h->n.next = h->n.prev = &h->n;
 }
 
+/**
+ * list_node_init - initialize a list_node
+ * @n: the list_node to link to itself.
+ *
+ * You don't need to use this normally!  But it lets you list_del(@n)
+ * safely.
+ */
+static inline void list_node_init(struct list_node *n)
+{
+	n->next = n->prev = n;
+}
+
+/**
+ * list_add_after - add an entry after an existing node in a linked list
+ * @h: the list_head to add the node to (for debugging)
+ * @p: the existing list_node to add the node after
+ * @n: the new list_node to add to the list.
+ *
+ * The existing list_node must already be a member of the list.
+ * The new list_node does not need to be initialized; it will be overwritten.
+ *
+ * Example:
+ *	struct child c1, c2, c3;
+ *	LIST_HEAD(h);
+ *
+ *	list_add_tail(&h, &c1.list);
+ *	list_add_tail(&h, &c3.list);
+ *	list_add_after(&h, &c1.list, &c2.list);
+ */
+#define list_add_after(h, p, n) list_add_after_(h, p, n, LIST_LOC)
+static inline void list_add_after_(struct list_head *h,
+				   struct list_node *p,
+				   struct list_node *n,
+				   const char *abortstr)
+{
+	n->next = p->next;
+	n->prev = p;
+	p->next->prev = n;
+	p->next = n;
+	(void)list_debug(h, abortstr);
+}
+
 /**
  * list_add - add an entry at the start of a linked list.
  * @h: the list_head to add the node to
@@ -163,10 +205,34 @@ static inline void list_add_(struct list_head *h,
 			     struct list_node *n,
 			     const char *abortstr)
 {
-	n->next = h->n.next;
-	n->prev = &h->n;
-	h->n.next->prev = n;
-	h->n.next = n;
+	list_add_after_(h, &h->n, n, abortstr);
+}
+
+/**
+ * list_add_before - add an entry before an existing node in a linked list
+ * @h: the list_head to add the node to (for debugging)
+ * @p: the existing list_node to add the node before
+ * @n: the new list_node to add to the list.
+ *
+ * The existing list_node must already be a member of the list.
+ * The new list_node does not need to be initialized; it will be overwritten.
+ *
+ * Example:
+ *	list_head_init(&h);
+ *	list_add_tail(&h, &c1.list);
+ *	list_add_tail(&h, &c3.list);
+ *	list_add_before(&h, &c3.list, &c2.list);
+ */
+#define list_add_before(h, p, n) list_add_before_(h, p, n, LIST_LOC)
+static inline void list_add_before_(struct list_head *h,
+				    struct list_node *p,
+				    struct list_node *n,
+				    const char *abortstr)
+{
+	n->next = p;
+	n->prev = p->prev;
+	p->prev->next = n;
+	p->prev = n;
 	(void)list_debug(h, abortstr);
 }
 
@@ -185,11 +251,7 @@ static inline void list_add_tail_(struct list_head *h,
 				  struct list_node *n,
 				  const char *abortstr)
 {
-	n->next = &h->n;
-	n->prev = h->n.prev;
-	h->n.prev->next = n;
-	h->n.prev = n;
-	(void)list_debug(h, abortstr);
+	list_add_before_(h, &h->n, n, abortstr);
 }
 
 /**
@@ -229,6 +291,21 @@ static inline bool list_empty_nodebug(const struct list_head *h)
 }
 #endif
 
+/**
+ * list_empty_nocheck - is a list empty?
+ * @h: the list_head
+ *
+ * If the list is empty, returns true. This doesn't perform any
+ * debug check for list consistency, so it can be called without
+ * locks, racing with the list being modified. This is ok for
+ * checks where an incorrect result is not an issue (optimized
+ * bail out path for example).
+ */
+static inline bool list_empty_nocheck(const struct list_head *h)
+{
+	return h->n.next == &h->n;
+}
+
 /**
  * list_del - delete an entry from an (unknown) linked list.
  * @n: the list_node to delete from the list.
@@ -237,7 +314,7 @@ static inline bool list_empty_nodebug(const struct list_head *h)
  * another list, but not deleted again.
  *
  * See also:
- *	list_del_from()
+ *	list_del_from(), list_del_init()
  *
  * Example:
  *	list_del(&child->list);
@@ -255,6 +332,27 @@ static inline void list_del_(struct list_node *n, const char* abortstr)
 #endif
 }
 
+/**
+ * list_del_init - delete a node, and reset it so it can be deleted again.
+ * @n: the list_node to be deleted.
+ *
+ * list_del(@n) or list_del_init() again after this will be safe,
+ * which can be useful in some cases.
+ *
+ * See also:
+ *	list_del_from(), list_del()
+ *
+ * Example:
+ *	list_del_init(&child->list);
+ *	parent->num_children--;
+ */
+#define list_del_init(n) list_del_init_(n, LIST_LOC)
+static inline void list_del_init_(struct list_node *n, const char *abortstr)
+{
+	list_del_(n, abortstr);
+	list_node_init(n);
+}
+
 /**
  * list_del_from - delete an entry from a known linked list.
  * @h: the list_head the node is in.
@@ -285,6 +383,39 @@ static inline void list_del_from(struct list_head *h, struct list_node *n)
 	list_del(n);
 }
 
+/**
+ * list_swap - swap out an entry from an (unknown) linked list for a new one.
+ * @o: the list_node to replace from the list.
+ * @n: the list_node to insert in place of the old one.
+ *
+ * Note that this leaves @o in an undefined state; it can be added to
+ * another list, but not deleted/swapped again.
+ *
+ * See also:
+ *	list_del()
+ *
+ * Example:
+ *	struct child x1, x2;
+ *	LIST_HEAD(xh);
+ *
+ *	list_add(&xh, &x1.list);
+ *	list_swap(&x1.list, &x2.list);
+ */
+#define list_swap(o, n) list_swap_(o, n, LIST_LOC)
+static inline void list_swap_(struct list_node *o,
+			      struct list_node *n,
+			      const char* abortstr)
+{
+	(void)list_debug_node(o, abortstr);
+	*n = *o;
+	n->next->prev = n;
+	n->prev->next = n;
+#ifdef CCAN_LIST_DEBUG
+	/* Catch use-after-del. */
+	o->next = o->prev = NULL;
+#endif
+}
+
 /**
  * list_entry - convert a list_node back into the structure containing it.
  * @n: the list_node
@@ -406,9 +537,29 @@ static inline const void *list_tail_(const struct list_head *h, size_t off)
  *		printf("Name: %s\n", child->name);
  */
 #define list_for_each_rev(h, i, member)					\
-	for (i = container_of_var(list_debug(h,	LIST_LOC)->n.prev, i, member); \
-	     &i->member != &(h)->n;					\
-	     i = container_of_var(i->member.prev, i, member))
+	list_for_each_rev_off(h, i, list_off_var_(i, member))
+
+/**
+ * list_for_each_rev_safe - iterate through a list backwards,
+ * maybe during deletion
+ * @h: the list_head
+ * @i: the structure containing the list_node
+ * @nxt: the structure containing the list_node
+ * @member: the list_node member of the structure
+ *
+ * This is a convenient wrapper to iterate @i over the entire list backwards.
+ * It's a for loop, so you can break and continue as normal.  The extra
+ * variable * @nxt is used to hold the next element, so you can delete @i
+ * from the list.
+ *
+ * Example:
+ *	struct child *next;
+ *	list_for_each_rev_safe(&parent->children, child, next, list) {
+ *		printf("Name: %s\n", child->name);
+ *	}
+ */
+#define list_for_each_rev_safe(h, i, nxt, member)			\
+	list_for_each_rev_safe_off(h, i, nxt, list_off_var_(i, member))
 
 /**
  * list_for_each_safe - iterate through a list, maybe during deletion
@@ -422,7 +573,6 @@ static inline const void *list_tail_(const struct list_head *h, size_t off)
  * @nxt is used to hold the next element, so you can delete @i from the list.
  *
  * Example:
- *	struct child *next;
  *	list_for_each_safe(&parent->children, child, next, list) {
  *		list_del(&child->list);
  *		parent->num_children--;
@@ -537,10 +687,28 @@ static inline void list_prepend_list_(struct list_head *to,
 	list_head_init(from);
 }
 
+/* internal macros, do not use directly */
+#define list_for_each_off_dir_(h, i, off, dir)				\
+	for (i = list_node_to_off_(list_debug(h, LIST_LOC)->n.dir,	\
+				   (off));				\
+	list_node_from_off_((void *)i, (off)) != &(h)->n;		\
+	i = list_node_to_off_(list_node_from_off_((void *)i, (off))->dir, \
+			      (off)))
+
+#define list_for_each_safe_off_dir_(h, i, nxt, off, dir)		\
+	for (i = list_node_to_off_(list_debug(h, LIST_LOC)->n.dir,	\
+				   (off)),				\
+	nxt = list_node_to_off_(list_node_from_off_(i, (off))->dir,	\
+				(off));					\
+	list_node_from_off_(i, (off)) != &(h)->n;			\
+	i = nxt,							\
+	nxt = list_node_to_off_(list_node_from_off_(i, (off))->dir,	\
+				(off)))
+
 /**
  * list_for_each_off - iterate through a list of memory regions.
  * @h: the list_head
- * @i: the pointer to a memory region wich contains list node data.
+ * @i: the pointer to a memory region which contains list node data.
  * @off: offset(relative to @i) at which list node data resides.
  *
  * This is a low-level wrapper to iterate @i over the entire list, used to
@@ -548,12 +716,12 @@ static inline void list_prepend_list_(struct list_head *to,
  * so you can break and continue as normal.
  *
  * WARNING! Being the low-level macro that it is, this wrapper doesn't know
- * nor care about the type of @i. The only assumtion made is that @i points
+ * nor care about the type of @i. The only assumption made is that @i points
  * to a chunk of memory that at some @offset, relative to @i, contains a
- * properly filled `struct node_list' which in turn contains pointers to
- * memory chunks and it's turtles all the way down. Whith all that in mind
+ * properly filled `struct list_node' which in turn contains pointers to
+ * memory chunks and it's turtles all the way down. With all that in mind
  * remember that given the wrong pointer/offset couple this macro will
- * happilly churn all you memory untill SEGFAULT stops it, in other words
+ * happily churn all you memory until SEGFAULT stops it, in other words
  * caveat emptor.
  *
  * It is worth mentioning that one of legitimate use-cases for that wrapper
@@ -567,17 +735,24 @@ static inline void list_prepend_list_(struct list_head *to,
  *		printf("Name: %s\n", child->name);
  */
 #define list_for_each_off(h, i, off)                                    \
-	for (i = list_node_to_off_(list_debug(h, LIST_LOC)->n.next,	\
-				   (off));				\
-       list_node_from_off_((void *)i, (off)) != &(h)->n;                \
-       i = list_node_to_off_(list_node_from_off_((void *)i, (off))->next, \
-                             (off)))
+	list_for_each_off_dir_((h),(i),(off),next)
+
+/**
+ * list_for_each_rev_off - iterate through a list of memory regions backwards
+ * @h: the list_head
+ * @i: the pointer to a memory region which contains list node data.
+ * @off: offset(relative to @i) at which list node data resides.
+ *
+ * See list_for_each_off for details
+ */
+#define list_for_each_rev_off(h, i, off)                                    \
+	list_for_each_off_dir_((h),(i),(off),prev)
 
 /**
  * list_for_each_safe_off - iterate through a list of memory regions, maybe
  * during deletion
  * @h: the list_head
- * @i: the pointer to a memory region wich contains list node data.
+ * @i: the pointer to a memory region which contains list node data.
  * @nxt: the structure containing the list_node
  * @off: offset(relative to @i) at which list node data resides.
  *
@@ -590,15 +765,26 @@ static inline void list_prepend_list_(struct list_head *to,
  *		printf("Name: %s\n", child->name);
  */
 #define list_for_each_safe_off(h, i, nxt, off)                          \
-	for (i = list_node_to_off_(list_debug(h, LIST_LOC)->n.next,	\
-				   (off)),				\
-         nxt = list_node_to_off_(list_node_from_off_(i, (off))->next,   \
-                                 (off));                                \
-       list_node_from_off_(i, (off)) != &(h)->n;                        \
-       i = nxt,                                                         \
-         nxt = list_node_to_off_(list_node_from_off_(i, (off))->next,   \
-                                 (off)))
+	list_for_each_safe_off_dir_((h),(i),(nxt),(off),next)
 
+/**
+ * list_for_each_rev_safe_off - iterate backwards through a list of
+ * memory regions, maybe during deletion
+ * @h: the list_head
+ * @i: the pointer to a memory region which contains list node data.
+ * @nxt: the structure containing the list_node
+ * @off: offset(relative to @i) at which list node data resides.
+ *
+ * For details see `list_for_each_rev_off' and `list_for_each_rev_safe'
+ * descriptions.
+ *
+ * Example:
+ *	list_for_each_rev_safe_off(&parent->children, child,
+ *		next, offsetof(struct child, list))
+ *		printf("Name: %s\n", child->name);
+ */
+#define list_for_each_rev_safe_off(h, i, nxt, off)                      \
+	list_for_each_safe_off_dir_((h),(i),(nxt),(off),prev)
 
 /* Other -off variants. */
 #define list_entry_off(n, type, off)		\
diff --git a/ndctl/lib/inject.c b/ndctl/lib/inject.c
index d61c02c176e2..3486ffefc40a 100644
--- a/ndctl/lib/inject.c
+++ b/ndctl/lib/inject.c
@@ -2,7 +2,6 @@
 // Copyright (C) 2014-2020, Intel Corporation. All rights reserved.
 #include <stdlib.h>
 #include <limits.h>
-#include <util/list.h>
 #include <util/size.h>
 #include <ndctl/libndctl.h>
 #include <ccan/list/list.h>
diff --git a/util/list.h b/util/list.h
deleted file mode 100644
index 1ea9c59b9f0c..000000000000
--- a/util/list.h
+++ /dev/null
@@ -1,40 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/* Copyright (C) 2015-2020 Intel Corporation. All rights reserved. */
-#ifndef _NDCTL_LIST_H_
-#define _NDCTL_LIST_H_
-
-#include <ccan/list/list.h>
-
-/**
- * list_add_after - add an entry after the given node in the linked list.
- * @h: the list_head to add the node to
- * @l: the list_node after 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_after(&parent->children, &child1->list, &child->list);
- *	parent->num_children++;
- */
-#define list_add_after(h, l, n) list_add_after_(h, l, n, LIST_LOC)
-static inline void list_add_after_(struct list_head *h,
-				   struct list_node *l,
-				   struct list_node *n,
-				   const char *abortstr)
-{
-	if (l->next == &h->n) {
-		/* l is the last element, this becomes a list_add_tail */
-		list_add_tail(h, n);
-		return;
-	}
-	n->next = l->next;
-	n->prev = l;
-	l->next->prev = n;
-	l->next = n;
-	(void)list_debug(h, abortstr);
-}
-
-#endif /* _NDCTL_LIST_H_ */


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [ndctl PATCH v2 06/12] cxl/lib: Maintain decoders in id order
  2022-07-14 17:01 [ndctl PATCH v2 00/12] cxl: Region provisioning foundation Dan Williams
                   ` (4 preceding siblings ...)
  2022-07-14 17:02 ` [ndctl PATCH v2 05/12] ccan/list: Import latest list helpers Dan Williams
@ 2022-07-14 17:02 ` Dan Williams
  2022-07-14 17:02 ` [ndctl PATCH v2 07/12] cxl/memdev: Fix json for multi-device partitioning Dan Williams
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Dan Williams @ 2022-07-14 17:02 UTC (permalink / raw)
  To: vishal.l.verma; +Cc: Davidlohr Bueso, alison.schofield, nvdimm, linux-cxl

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.

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      |   39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 util/list.h

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
new file mode 100644
index 000000000000..cb7727123ea8
--- /dev/null
+++ b/util/list.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (C) 2022 Intel Corporation. All rights reserved. */
+#ifndef _NDCTL_LIST_H_
+#define _NDCTL_LIST_H_
+
+#include <ccan/list/list.h>
+
+#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_ */


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [ndctl PATCH v2 07/12] cxl/memdev: Fix json for multi-device partitioning
  2022-07-14 17:01 [ndctl PATCH v2 00/12] cxl: Region provisioning foundation Dan Williams
                   ` (5 preceding siblings ...)
  2022-07-14 17:02 ` [ndctl PATCH v2 06/12] cxl/lib: Maintain decoders in id order Dan Williams
@ 2022-07-14 17:02 ` Dan Williams
  2022-07-14 17:02 ` [ndctl PATCH v2 08/12] cxl/list: Emit 'mode' for endpoint decoder objects Dan Williams
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Dan Williams @ 2022-07-14 17:02 UTC (permalink / raw)
  To: vishal.l.verma; +Cc: Alison Schofield, nvdimm, linux-cxl

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);


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [ndctl PATCH v2 08/12] cxl/list: Emit 'mode' for endpoint decoder objects
  2022-07-14 17:01 [ndctl PATCH v2 00/12] cxl: Region provisioning foundation Dan Williams
                   ` (6 preceding siblings ...)
  2022-07-14 17:02 ` [ndctl PATCH v2 07/12] cxl/memdev: Fix json for multi-device partitioning Dan Williams
@ 2022-07-14 17:02 ` Dan Williams
  2022-07-14 17:02 ` [ndctl PATCH v2 09/12] cxl/set-partition: Accept 'ram' as an alias for 'volatile' Dan Williams
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Dan Williams @ 2022-07-14 17:02 UTC (permalink / raw)
  To: vishal.l.verma; +Cc: alison.schofield, nvdimm, linux-cxl

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);


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [ndctl PATCH v2 09/12] cxl/set-partition: Accept 'ram' as an alias for 'volatile'
  2022-07-14 17:01 [ndctl PATCH v2 00/12] cxl: Region provisioning foundation Dan Williams
                   ` (7 preceding siblings ...)
  2022-07-14 17:02 ` [ndctl PATCH v2 08/12] cxl/list: Emit 'mode' for endpoint decoder objects Dan Williams
@ 2022-07-14 17:02 ` Dan Williams
  2022-07-14 17:02 ` [ndctl PATCH v2 10/12] cxl/memdev: Add {reserve,free}-dpa commands Dan Williams
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 18+ messages in thread
From: Dan Williams @ 2022-07-14 17:02 UTC (permalink / raw)
  To: vishal.l.verma; +Cc: Alison Schofield, Davidlohr Bueso, nvdimm, linux-cxl

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

Cc: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
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;


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [ndctl PATCH v2 10/12] cxl/memdev: Add {reserve,free}-dpa commands
  2022-07-14 17:01 [ndctl PATCH v2 00/12] cxl: Region provisioning foundation Dan Williams
                   ` (8 preceding siblings ...)
  2022-07-14 17:02 ` [ndctl PATCH v2 09/12] cxl/set-partition: Accept 'ram' as an alias for 'volatile' Dan Williams
@ 2022-07-14 17:02 ` Dan Williams
  2022-07-20 13:16   ` Jonathan Cameron
  2022-07-14 17:02 ` [ndctl PATCH v2 11/12] cxl/test: Update CXL memory parameters Dan Williams
  2022-07-14 17:02 ` [ndctl PATCH v2 12/12] cxl/test: Checkout region setup/teardown Dan Williams
  11 siblings, 1 reply; 18+ messages in thread
From: Dan Williams @ 2022-07-14 17:02 UTC (permalink / raw)
  To: vishal.l.verma; +Cc: alison.schofield, nvdimm, linux-cxl

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/cxl-free-dpa.txt    |   53 ++++++
 Documentation/cxl/cxl-reserve-dpa.txt |   67 ++++++++
 Documentation/cxl/lib/libcxl.txt      |    2 
 Documentation/cxl/meson.build         |    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                          |  280 +++++++++++++++++++++++++++++++++
 13 files changed, 511 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/cxl/cxl-free-dpa.txt
 create mode 100644 Documentation/cxl/cxl-reserve-dpa.txt

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/cxl-free-dpa.txt b/Documentation/cxl/cxl-free-dpa.txt
new file mode 100644
index 000000000000..73fb048fdee3
--- /dev/null
+++ b/Documentation/cxl/cxl-free-dpa.txt
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0
+
+cxl-free-dpa(1)
+===============
+
+NAME
+----
+cxl-free-dpa - release device-physical address space
+
+SYNOPSIS
+--------
+[verse]
+'cxl free-dpa' <mem0> [<mem1>..<memN>] [<options>]
+
+The CXL region provisioning process proceeds in multiple steps. One of
+the steps is identifying and reserving the DPA span that each member of
+the interleave-set (region) contributes in advance of attaching that
+allocation to a region. For development, test, and debug purposes this
+command is a helper to find the last allocated decoder on a device and
+zero-out / free its DPA allocation.
+
+OPTIONS
+-------
+<memory device(s)>::
+include::memdev-option.txt[]
+
+-d::
+--decoder::
+	Specify the decoder to free. The CXL specification
+	mandates that DPA must be released in the reverse order it was
+	allocated. See linkcxl:cxl-reserve-dpa[1]
+
+-t::
+--type::
+	Constrain the search for "last allocated decoder" to decoders targeting
+	the given partition.
+
+-f::
+--force::
+	The kernel enforces CXL DPA ordering constraints on deallocation events,
+	and the tool anticipates those and fails operations that are expected to
+	fail without sending them to the kernel. For test purposes, continue to
+	attempt "expected to fail" operations to exercise the driver.
+
+-v::
+	Turn on verbose debug messages in the library (if libcxl was built with
+	logging and debug enabled).
+
+include::../copyright.txt[]
+
+SEE ALSO
+--------
+linkcxl:cxl-reserve-dpa[1]
diff --git a/Documentation/cxl/cxl-reserve-dpa.txt b/Documentation/cxl/cxl-reserve-dpa.txt
new file mode 100644
index 000000000000..560daf0cb277
--- /dev/null
+++ b/Documentation/cxl/cxl-reserve-dpa.txt
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: GPL-2.0
+
+cxl-reserve-dpa(1)
+==================
+
+NAME
+----
+cxl-reserve-dpa - allocate device-physical address space
+
+SYNOPSIS
+--------
+[verse]
+'cxl reserve-dpa' <mem0> [<mem1>..<memN>] [<options>]
+
+The CXL region provisioning process proceeds in multiple steps. One of
+the steps is identifying and reserving the DPA span that each member of
+the interleave-set (region) contributes in advance of attaching that
+allocation to a region. For development, test, and debug purposes this
+command is a helper to find the next available decoder on endpoint
+(memdev) and mark a span of DPA as busy.
+
+OPTIONS
+-------
+<memory device(s)>::
+include::memdev-option.txt[]
+
+-d::
+--decoder::
+	Specify the decoder to attempt the allocation. The CXL specification
+	mandates that allocations must be ordered by DPA and decoder instance.
+	I.e. the lowest DPA allocation on the device is covered by deocder0, and
+	the last / highest DPA allocation is covered by the last decoder. This
+	ordering is enforced by the kernel. By default the tool picks the 'next
+	available' decoder.
+
+-t::
+--type::
+	Select the partition for the allocation. CXL devices implement a
+	partition that divdes 'ram' and 'pmem' capacity, where 'pmem' capacity
+	consumes the higher DPA capacity above the partition boundary. The type
+	defaults to 'pmem'. Note that given CXL DPA allocation constraints, once
+	any 'pmem' allocation is established then all remaining 'ram' capacity
+	becomes reserved (skipped).
+
+-f::
+--force::
+	The kernel enforces CXL DPA allocation ordering constraints, and
+	the tool anticipates those and fails operations that are expected to
+	fail without sending them to the kernel. For test purposes, continue to
+	attempt "expected to fail" operations to exercise the driver.
+
+-s::
+--size::
+	Specify the size of the allocation. This option supports the suffixes
+	"k" or "K" for KiB, "m" or "M" for MiB, "g" or "G" for GiB and "t" or
+	"T" for TiB. This defaults to "all available capacity of the specified
+	type".
+
+-v::
+	Turn on verbose debug messages in the library (if libcxl was built with
+	logging and debug enabled).
+
+include::../copyright.txt[]
+
+SEE ALSO
+--------
+linkcxl:cxl-free-dpa[1]
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/Documentation/cxl/meson.build b/Documentation/cxl/meson.build
index 974a5a41d169..d019dfc23659 100644
--- a/Documentation/cxl/meson.build
+++ b/Documentation/cxl/meson.build
@@ -36,6 +36,8 @@ cxl_manpages = [
   'cxl-disable-port.txt',
   'cxl-disable-bus.txt',
   'cxl-set-partition.txt',
+  'cxl-reserve-dpa.txt',
+  'cxl-free-dpa.txt',
 ]
 
 foreach man : cxl_manpages
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..e42f554326ec 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,242 @@ 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 +702,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 +746,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 +760,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 +773,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 +878,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;
+}


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [ndctl PATCH v2 11/12] cxl/test: Update CXL memory parameters
  2022-07-14 17:01 [ndctl PATCH v2 00/12] cxl: Region provisioning foundation Dan Williams
                   ` (9 preceding siblings ...)
  2022-07-14 17:02 ` [ndctl PATCH v2 10/12] cxl/memdev: Add {reserve,free}-dpa commands Dan Williams
@ 2022-07-14 17:02 ` Dan Williams
  2022-07-14 17:02 ` [ndctl PATCH v2 12/12] cxl/test: Checkout region setup/teardown Dan Williams
  11 siblings, 0 replies; 18+ messages in thread
From: Dan Williams @ 2022-07-14 17:02 UTC (permalink / raw)
  To: vishal.l.verma; +Cc: alison.schofield, nvdimm, linux-cxl

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[@]}


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [ndctl PATCH v2 12/12] cxl/test: Checkout region setup/teardown
  2022-07-14 17:01 [ndctl PATCH v2 00/12] cxl: Region provisioning foundation Dan Williams
                   ` (10 preceding siblings ...)
  2022-07-14 17:02 ` [ndctl PATCH v2 11/12] cxl/test: Update CXL memory parameters Dan Williams
@ 2022-07-14 17:02 ` Dan Williams
  2022-07-14 20:55   ` Verma, Vishal L
  11 siblings, 1 reply; 18+ messages in thread
From: Dan Williams @ 2022-07-14 17:02 UTC (permalink / raw)
  To: vishal.l.verma; +Cc: alison.schofield, nvdimm, linux-cxl

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-sysfs.sh |  122 ++++++++++++++++++++++++++++++++++++++++++++++
 test/meson.build         |    2 +
 2 files changed, 124 insertions(+)
 create mode 100644 test/cxl-region-sysfs.sh

diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh
new file mode 100644
index 000000000000..2582edb3f306
--- /dev/null
+++ b/test/cxl-region-sysfs.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..3203d9cea243 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_sysfs = find_program('cxl-region-sysfs.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-sysfs.sh',    cxl_sysfs,	  'cxl'   ],
 ]
 
 if get_option('destructive').enabled()


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [ndctl PATCH v2 05/12] ccan/list: Import latest list helpers
  2022-07-14 17:02 ` [ndctl PATCH v2 05/12] ccan/list: Import latest list helpers Dan Williams
@ 2022-07-14 17:20   ` Ira Weiny
  0 siblings, 0 replies; 18+ messages in thread
From: Ira Weiny @ 2022-07-14 17:20 UTC (permalink / raw)
  To: Dan Williams; +Cc: vishal.l.verma, alison.schofield, nvdimm, linux-cxl

On Thu, Jul 14, 2022 at 10:02:15AM -0700, Dan Williams wrote:
> Pick up the definition of list_add_{before,after} and other updates from
> ccan at commit 52b86922f846 ("ccan/base64: fix GCC warning.").
> 
> Reported-by: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  ccan/list/list.h   |  258 +++++++++++++++++++++++++++++++++++++++++++++-------
>  ndctl/lib/inject.c |    1 
>  util/list.h        |   40 --------
>  3 files changed, 222 insertions(+), 77 deletions(-)
>  delete mode 100644 util/list.h
> 
> diff --git a/ccan/list/list.h b/ccan/list/list.h
> index 3ebd1b23dc0f..15f5fb7b34eb 100644
> --- a/ccan/list/list.h
> +++ b/ccan/list/list.h
> @@ -95,8 +95,8 @@ struct list_node *list_check_node(const struct list_node *n,
>  #define list_debug(h, loc) list_check((h), loc)
>  #define list_debug_node(n, loc) list_check_node((n), loc)
>  #else
> -#define list_debug(h, loc) (h)
> -#define list_debug_node(n, loc) (n)
> +#define list_debug(h, loc) ((void)loc, h)
> +#define list_debug_node(n, loc) ((void)loc, n)
>  #endif
>  
>  /**
> @@ -111,7 +111,7 @@ struct list_node *list_check_node(const struct list_node *n,
>   * Example:
>   *	static struct list_head my_list = LIST_HEAD_INIT(my_list);
>   */
> -#define LIST_HEAD_INIT(name) { { &name.n, &name.n } }
> +#define LIST_HEAD_INIT(name) { { &(name).n, &(name).n } }
>  
>  /**
>   * LIST_HEAD - define and initialize an empty list_head
> @@ -145,6 +145,48 @@ static inline void list_head_init(struct list_head *h)
>  	h->n.next = h->n.prev = &h->n;
>  }
>  
> +/**
> + * list_node_init - initialize a list_node
> + * @n: the list_node to link to itself.
> + *
> + * You don't need to use this normally!  But it lets you list_del(@n)
> + * safely.
> + */
> +static inline void list_node_init(struct list_node *n)
> +{
> +	n->next = n->prev = n;
> +}
> +
> +/**
> + * list_add_after - add an entry after an existing node in a linked list
> + * @h: the list_head to add the node to (for debugging)
> + * @p: the existing list_node to add the node after
> + * @n: the new list_node to add to the list.
> + *
> + * The existing list_node must already be a member of the list.
> + * The new list_node does not need to be initialized; it will be overwritten.
> + *
> + * Example:
> + *	struct child c1, c2, c3;
> + *	LIST_HEAD(h);
> + *
> + *	list_add_tail(&h, &c1.list);
> + *	list_add_tail(&h, &c3.list);
> + *	list_add_after(&h, &c1.list, &c2.list);
> + */
> +#define list_add_after(h, p, n) list_add_after_(h, p, n, LIST_LOC)
> +static inline void list_add_after_(struct list_head *h,
> +				   struct list_node *p,
> +				   struct list_node *n,
> +				   const char *abortstr)
> +{
> +	n->next = p->next;
> +	n->prev = p;
> +	p->next->prev = n;
> +	p->next = n;
> +	(void)list_debug(h, abortstr);
> +}
> +
>  /**
>   * list_add - add an entry at the start of a linked list.
>   * @h: the list_head to add the node to
> @@ -163,10 +205,34 @@ static inline void list_add_(struct list_head *h,
>  			     struct list_node *n,
>  			     const char *abortstr)
>  {
> -	n->next = h->n.next;
> -	n->prev = &h->n;
> -	h->n.next->prev = n;
> -	h->n.next = n;
> +	list_add_after_(h, &h->n, n, abortstr);
> +}
> +
> +/**
> + * list_add_before - add an entry before an existing node in a linked list
> + * @h: the list_head to add the node to (for debugging)
> + * @p: the existing list_node to add the node before
> + * @n: the new list_node to add to the list.
> + *
> + * The existing list_node must already be a member of the list.
> + * The new list_node does not need to be initialized; it will be overwritten.
> + *
> + * Example:
> + *	list_head_init(&h);
> + *	list_add_tail(&h, &c1.list);
> + *	list_add_tail(&h, &c3.list);
> + *	list_add_before(&h, &c3.list, &c2.list);
> + */
> +#define list_add_before(h, p, n) list_add_before_(h, p, n, LIST_LOC)
> +static inline void list_add_before_(struct list_head *h,
> +				    struct list_node *p,
> +				    struct list_node *n,
> +				    const char *abortstr)
> +{
> +	n->next = p;
> +	n->prev = p->prev;
> +	p->prev->next = n;
> +	p->prev = n;
>  	(void)list_debug(h, abortstr);
>  }
>  
> @@ -185,11 +251,7 @@ static inline void list_add_tail_(struct list_head *h,
>  				  struct list_node *n,
>  				  const char *abortstr)
>  {
> -	n->next = &h->n;
> -	n->prev = h->n.prev;
> -	h->n.prev->next = n;
> -	h->n.prev = n;
> -	(void)list_debug(h, abortstr);
> +	list_add_before_(h, &h->n, n, abortstr);
>  }
>  
>  /**
> @@ -229,6 +291,21 @@ static inline bool list_empty_nodebug(const struct list_head *h)
>  }
>  #endif
>  
> +/**
> + * list_empty_nocheck - is a list empty?
> + * @h: the list_head
> + *
> + * If the list is empty, returns true. This doesn't perform any
> + * debug check for list consistency, so it can be called without
> + * locks, racing with the list being modified. This is ok for
> + * checks where an incorrect result is not an issue (optimized
> + * bail out path for example).
> + */
> +static inline bool list_empty_nocheck(const struct list_head *h)
> +{
> +	return h->n.next == &h->n;
> +}
> +
>  /**
>   * list_del - delete an entry from an (unknown) linked list.
>   * @n: the list_node to delete from the list.
> @@ -237,7 +314,7 @@ static inline bool list_empty_nodebug(const struct list_head *h)
>   * another list, but not deleted again.
>   *
>   * See also:
> - *	list_del_from()
> + *	list_del_from(), list_del_init()
>   *
>   * Example:
>   *	list_del(&child->list);
> @@ -255,6 +332,27 @@ static inline void list_del_(struct list_node *n, const char* abortstr)
>  #endif
>  }
>  
> +/**
> + * list_del_init - delete a node, and reset it so it can be deleted again.
> + * @n: the list_node to be deleted.
> + *
> + * list_del(@n) or list_del_init() again after this will be safe,
> + * which can be useful in some cases.
> + *
> + * See also:
> + *	list_del_from(), list_del()
> + *
> + * Example:
> + *	list_del_init(&child->list);
> + *	parent->num_children--;
> + */
> +#define list_del_init(n) list_del_init_(n, LIST_LOC)
> +static inline void list_del_init_(struct list_node *n, const char *abortstr)
> +{
> +	list_del_(n, abortstr);
> +	list_node_init(n);
> +}
> +
>  /**
>   * list_del_from - delete an entry from a known linked list.
>   * @h: the list_head the node is in.
> @@ -285,6 +383,39 @@ static inline void list_del_from(struct list_head *h, struct list_node *n)
>  	list_del(n);
>  }
>  
> +/**
> + * list_swap - swap out an entry from an (unknown) linked list for a new one.
> + * @o: the list_node to replace from the list.
> + * @n: the list_node to insert in place of the old one.
> + *
> + * Note that this leaves @o in an undefined state; it can be added to
> + * another list, but not deleted/swapped again.
> + *
> + * See also:
> + *	list_del()
> + *
> + * Example:
> + *	struct child x1, x2;
> + *	LIST_HEAD(xh);
> + *
> + *	list_add(&xh, &x1.list);
> + *	list_swap(&x1.list, &x2.list);
> + */
> +#define list_swap(o, n) list_swap_(o, n, LIST_LOC)
> +static inline void list_swap_(struct list_node *o,
> +			      struct list_node *n,
> +			      const char* abortstr)
> +{
> +	(void)list_debug_node(o, abortstr);
> +	*n = *o;
> +	n->next->prev = n;
> +	n->prev->next = n;
> +#ifdef CCAN_LIST_DEBUG
> +	/* Catch use-after-del. */
> +	o->next = o->prev = NULL;
> +#endif
> +}
> +
>  /**
>   * list_entry - convert a list_node back into the structure containing it.
>   * @n: the list_node
> @@ -406,9 +537,29 @@ static inline const void *list_tail_(const struct list_head *h, size_t off)
>   *		printf("Name: %s\n", child->name);
>   */
>  #define list_for_each_rev(h, i, member)					\
> -	for (i = container_of_var(list_debug(h,	LIST_LOC)->n.prev, i, member); \
> -	     &i->member != &(h)->n;					\
> -	     i = container_of_var(i->member.prev, i, member))
> +	list_for_each_rev_off(h, i, list_off_var_(i, member))
> +
> +/**
> + * list_for_each_rev_safe - iterate through a list backwards,
> + * maybe during deletion
> + * @h: the list_head
> + * @i: the structure containing the list_node
> + * @nxt: the structure containing the list_node
> + * @member: the list_node member of the structure
> + *
> + * This is a convenient wrapper to iterate @i over the entire list backwards.
> + * It's a for loop, so you can break and continue as normal.  The extra
> + * variable * @nxt is used to hold the next element, so you can delete @i
> + * from the list.
> + *
> + * Example:
> + *	struct child *next;
> + *	list_for_each_rev_safe(&parent->children, child, next, list) {
> + *		printf("Name: %s\n", child->name);
> + *	}
> + */
> +#define list_for_each_rev_safe(h, i, nxt, member)			\
> +	list_for_each_rev_safe_off(h, i, nxt, list_off_var_(i, member))
>  
>  /**
>   * list_for_each_safe - iterate through a list, maybe during deletion
> @@ -422,7 +573,6 @@ static inline const void *list_tail_(const struct list_head *h, size_t off)
>   * @nxt is used to hold the next element, so you can delete @i from the list.
>   *
>   * Example:
> - *	struct child *next;
>   *	list_for_each_safe(&parent->children, child, next, list) {
>   *		list_del(&child->list);
>   *		parent->num_children--;
> @@ -537,10 +687,28 @@ static inline void list_prepend_list_(struct list_head *to,
>  	list_head_init(from);
>  }
>  
> +/* internal macros, do not use directly */
> +#define list_for_each_off_dir_(h, i, off, dir)				\
> +	for (i = list_node_to_off_(list_debug(h, LIST_LOC)->n.dir,	\
> +				   (off));				\
> +	list_node_from_off_((void *)i, (off)) != &(h)->n;		\
> +	i = list_node_to_off_(list_node_from_off_((void *)i, (off))->dir, \
> +			      (off)))
> +
> +#define list_for_each_safe_off_dir_(h, i, nxt, off, dir)		\
> +	for (i = list_node_to_off_(list_debug(h, LIST_LOC)->n.dir,	\
> +				   (off)),				\
> +	nxt = list_node_to_off_(list_node_from_off_(i, (off))->dir,	\
> +				(off));					\
> +	list_node_from_off_(i, (off)) != &(h)->n;			\
> +	i = nxt,							\
> +	nxt = list_node_to_off_(list_node_from_off_(i, (off))->dir,	\
> +				(off)))
> +
>  /**
>   * list_for_each_off - iterate through a list of memory regions.
>   * @h: the list_head
> - * @i: the pointer to a memory region wich contains list node data.
> + * @i: the pointer to a memory region which contains list node data.
>   * @off: offset(relative to @i) at which list node data resides.
>   *
>   * This is a low-level wrapper to iterate @i over the entire list, used to
> @@ -548,12 +716,12 @@ static inline void list_prepend_list_(struct list_head *to,
>   * so you can break and continue as normal.
>   *
>   * WARNING! Being the low-level macro that it is, this wrapper doesn't know
> - * nor care about the type of @i. The only assumtion made is that @i points
> + * nor care about the type of @i. The only assumption made is that @i points
>   * to a chunk of memory that at some @offset, relative to @i, contains a
> - * properly filled `struct node_list' which in turn contains pointers to
> - * memory chunks and it's turtles all the way down. Whith all that in mind
> + * properly filled `struct list_node' which in turn contains pointers to
> + * memory chunks and it's turtles all the way down. With all that in mind
>   * remember that given the wrong pointer/offset couple this macro will
> - * happilly churn all you memory untill SEGFAULT stops it, in other words
> + * happily churn all you memory until SEGFAULT stops it, in other words
>   * caveat emptor.
>   *
>   * It is worth mentioning that one of legitimate use-cases for that wrapper
> @@ -567,17 +735,24 @@ static inline void list_prepend_list_(struct list_head *to,
>   *		printf("Name: %s\n", child->name);
>   */
>  #define list_for_each_off(h, i, off)                                    \
> -	for (i = list_node_to_off_(list_debug(h, LIST_LOC)->n.next,	\
> -				   (off));				\
> -       list_node_from_off_((void *)i, (off)) != &(h)->n;                \
> -       i = list_node_to_off_(list_node_from_off_((void *)i, (off))->next, \
> -                             (off)))
> +	list_for_each_off_dir_((h),(i),(off),next)
> +
> +/**
> + * list_for_each_rev_off - iterate through a list of memory regions backwards
> + * @h: the list_head
> + * @i: the pointer to a memory region which contains list node data.
> + * @off: offset(relative to @i) at which list node data resides.
> + *
> + * See list_for_each_off for details
> + */
> +#define list_for_each_rev_off(h, i, off)                                    \
> +	list_for_each_off_dir_((h),(i),(off),prev)
>  
>  /**
>   * list_for_each_safe_off - iterate through a list of memory regions, maybe
>   * during deletion
>   * @h: the list_head
> - * @i: the pointer to a memory region wich contains list node data.
> + * @i: the pointer to a memory region which contains list node data.
>   * @nxt: the structure containing the list_node
>   * @off: offset(relative to @i) at which list node data resides.
>   *
> @@ -590,15 +765,26 @@ static inline void list_prepend_list_(struct list_head *to,
>   *		printf("Name: %s\n", child->name);
>   */
>  #define list_for_each_safe_off(h, i, nxt, off)                          \
> -	for (i = list_node_to_off_(list_debug(h, LIST_LOC)->n.next,	\
> -				   (off)),				\
> -         nxt = list_node_to_off_(list_node_from_off_(i, (off))->next,   \
> -                                 (off));                                \
> -       list_node_from_off_(i, (off)) != &(h)->n;                        \
> -       i = nxt,                                                         \
> -         nxt = list_node_to_off_(list_node_from_off_(i, (off))->next,   \
> -                                 (off)))
> +	list_for_each_safe_off_dir_((h),(i),(nxt),(off),next)
>  
> +/**
> + * list_for_each_rev_safe_off - iterate backwards through a list of
> + * memory regions, maybe during deletion
> + * @h: the list_head
> + * @i: the pointer to a memory region which contains list node data.
> + * @nxt: the structure containing the list_node
> + * @off: offset(relative to @i) at which list node data resides.
> + *
> + * For details see `list_for_each_rev_off' and `list_for_each_rev_safe'
> + * descriptions.
> + *
> + * Example:
> + *	list_for_each_rev_safe_off(&parent->children, child,
> + *		next, offsetof(struct child, list))
> + *		printf("Name: %s\n", child->name);
> + */
> +#define list_for_each_rev_safe_off(h, i, nxt, off)                      \
> +	list_for_each_safe_off_dir_((h),(i),(nxt),(off),prev)
>  
>  /* Other -off variants. */
>  #define list_entry_off(n, type, off)		\
> diff --git a/ndctl/lib/inject.c b/ndctl/lib/inject.c
> index d61c02c176e2..3486ffefc40a 100644
> --- a/ndctl/lib/inject.c
> +++ b/ndctl/lib/inject.c
> @@ -2,7 +2,6 @@
>  // Copyright (C) 2014-2020, Intel Corporation. All rights reserved.
>  #include <stdlib.h>
>  #include <limits.h>
> -#include <util/list.h>
>  #include <util/size.h>
>  #include <ndctl/libndctl.h>
>  #include <ccan/list/list.h>
> diff --git a/util/list.h b/util/list.h
> deleted file mode 100644
> index 1ea9c59b9f0c..000000000000
> --- a/util/list.h
> +++ /dev/null
> @@ -1,40 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -/* Copyright (C) 2015-2020 Intel Corporation. All rights reserved. */
> -#ifndef _NDCTL_LIST_H_
> -#define _NDCTL_LIST_H_
> -
> -#include <ccan/list/list.h>
> -
> -/**
> - * list_add_after - add an entry after the given node in the linked list.
> - * @h: the list_head to add the node to
> - * @l: the list_node after 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_after(&parent->children, &child1->list, &child->list);
> - *	parent->num_children++;
> - */
> -#define list_add_after(h, l, n) list_add_after_(h, l, n, LIST_LOC)
> -static inline void list_add_after_(struct list_head *h,
> -				   struct list_node *l,
> -				   struct list_node *n,
> -				   const char *abortstr)
> -{
> -	if (l->next == &h->n) {
> -		/* l is the last element, this becomes a list_add_tail */
> -		list_add_tail(h, n);
> -		return;
> -	}
> -	n->next = l->next;
> -	n->prev = l;
> -	l->next->prev = n;
> -	l->next = n;
> -	(void)list_debug(h, abortstr);
> -}
> -
> -#endif /* _NDCTL_LIST_H_ */
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [ndctl PATCH v2 12/12] cxl/test: Checkout region setup/teardown
  2022-07-14 17:02 ` [ndctl PATCH v2 12/12] cxl/test: Checkout region setup/teardown Dan Williams
@ 2022-07-14 20:55   ` Verma, Vishal L
  2022-07-14 21:13     ` Dan Williams
  0 siblings, 1 reply; 18+ messages in thread
From: Verma, Vishal L @ 2022-07-14 20:55 UTC (permalink / raw)
  To: Williams, Dan J; +Cc: Schofield, Alison, linux-cxl, nvdimm

On Thu, 2022-07-14 at 10:02 -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-sysfs.sh |  122 ++++++++++++++++++++++++++++++++++++++++++++++
>  test/meson.build         |    2 +
>  2 files changed, 124 insertions(+)
>  create mode 100644 test/cxl-region-sysfs.sh

Hi Dan,

This is mostly looking good - just one note below found in testing:

> 
> diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh
> new file mode 100644
> index 000000000000..2582edb3f306
> --- /dev/null
> +++ b/test/cxl-region-sysfs.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")

With my pending update to make memdevs and regions the default listing
if no other top level object specified, the above listing breaks as it
can't deal with the extra memdevs now listed.

I think it may make sense to fine tune the defaults a bit - i.e. if
a -d is supplied, assume -D, but no other default top-level objects.

However I think this would be more resilient regardless, if it
explicitly specified a -D:

---

diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh
index 2582edb..eb28184 100644
--- a/test/cxl-region-sysfs.sh
+++ b/test/cxl-region-sysfs.sh
@@ -49,9 +49,9 @@ 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 ".[] |
+port_dev0=$($CXL list -DT -d $decoder | jq -r ".[] |
            .targets | .[] | select(.position == 0) | .target")
-port_dev1=$($CXL list -T -d $decoder | jq -r ".[] |
+port_dev1=$($CXL list -DT -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")


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [ndctl PATCH v2 12/12] cxl/test: Checkout region setup/teardown
  2022-07-14 20:55   ` Verma, Vishal L
@ 2022-07-14 21:13     ` Dan Williams
  2022-07-14 21:26       ` Verma, Vishal L
  0 siblings, 1 reply; 18+ messages in thread
From: Dan Williams @ 2022-07-14 21:13 UTC (permalink / raw)
  To: Verma, Vishal L, Williams, Dan J; +Cc: Schofield, Alison, linux-cxl, nvdimm

Verma, Vishal L wrote:
> On Thu, 2022-07-14 at 10:02 -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-sysfs.sh |  122 ++++++++++++++++++++++++++++++++++++++++++++++
> >  test/meson.build         |    2 +
> >  2 files changed, 124 insertions(+)
> >  create mode 100644 test/cxl-region-sysfs.sh
> 
> Hi Dan,
> 
> This is mostly looking good - just one note below found in testing:
> 
> > 
> > diff --git a/test/cxl-region-sysfs.sh b/test/cxl-region-sysfs.sh
> > new file mode 100644
> > index 000000000000..2582edb3f306
> > --- /dev/null
> > +++ b/test/cxl-region-sysfs.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")
> 
> With my pending update to make memdevs and regions the default listing
> if no other top level object specified, the above listing breaks as it
> can't deal with the extra memdevs now listed.
> 
> I think it may make sense to fine tune the defaults a bit - i.e. if
> a -d is supplied, assume -D, but no other default top-level objects.

Yes, this is what I would expect.

> However I think this would be more resilient regardless, if it
> explicitly specified a -D:

True, but it's a bit redundant.

Why does the -RM default break:

   cxl list [-T] -d $decoder

...? Doesn't the final "num_list_flags() == 0" check come after:

   if (param.decoder_filter)
           param.decoders = true;

...has prevented the default?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [ndctl PATCH v2 12/12] cxl/test: Checkout region setup/teardown
  2022-07-14 21:13     ` Dan Williams
@ 2022-07-14 21:26       ` Verma, Vishal L
  0 siblings, 0 replies; 18+ messages in thread
From: Verma, Vishal L @ 2022-07-14 21:26 UTC (permalink / raw)
  To: Williams, Dan J; +Cc: Schofield, Alison, linux-cxl, nvdimm

On Thu, 2022-07-14 at 14:13 -0700, Dan Williams wrote:
> Verma, Vishal L wrote:
> > > 
> > > +# 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")
> > 
> > With my pending update to make memdevs and regions the default listing
> > if no other top level object specified, the above listing breaks as it
> > can't deal with the extra memdevs now listed.
> > 
> > I think it may make sense to fine tune the defaults a bit - i.e. if
> > a -d is supplied, assume -D, but no other default top-level objects.
> 
> Yes, this is what I would expect.
> 
> > However I think this would be more resilient regardless, if it
> > explicitly specified a -D:
> 
> True, but it's a bit redundant.
> 
> Why does the -RM default break:
> 
>    cxl list [-T] -d $decoder
> 
> ...? Doesn't the final "num_list_flags() == 0" check come after:
> 
>    if (param.decoder_filter)
>            param.decoders = true;
> 
> ...has prevented the default?

Ah yes I see the problem - I added the new default unconditionally in
the first pass of num_list_flags() == 0, which was also checking the
above condition.

I basically need to let it run through the list of 'if -d then -D' type
stuff, then check num_list_flags() again, and if 0, only then enable
-R and -M.

Will fix this in my patch, no need for the -D change above.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [ndctl PATCH v2 10/12] cxl/memdev: Add {reserve,free}-dpa commands
  2022-07-14 17:02 ` [ndctl PATCH v2 10/12] cxl/memdev: Add {reserve,free}-dpa commands Dan Williams
@ 2022-07-20 13:16   ` Jonathan Cameron
  0 siblings, 0 replies; 18+ messages in thread
From: Jonathan Cameron @ 2022-07-20 13:16 UTC (permalink / raw)
  To: Dan Williams; +Cc: vishal.l.verma, alison.schofield, nvdimm, linux-cxl

On Thu, 14 Jul 2022 10:02:44 -0700
Dan Williams <dan.j.williams@intel.com> 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>
Not a proper review as don't have time to dive into this today. But whilst glancing at it, noticed a typo below.

> diff --git a/Documentation/cxl/cxl-reserve-dpa.txt b/Documentation/cxl/cxl-reserve-dpa.txt
> new file mode 100644
> index 000000000000..560daf0cb277
> --- /dev/null
> +++ b/Documentation/cxl/cxl-reserve-dpa.txt
> @@ -0,0 +1,67 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +cxl-reserve-dpa(1)
> +==================
> +
> +NAME
> +----
> +cxl-reserve-dpa - allocate device-physical address space
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'cxl reserve-dpa' <mem0> [<mem1>..<memN>] [<options>]
> +
> +The CXL region provisioning process proceeds in multiple steps. One of
> +the steps is identifying and reserving the DPA span that each member of
> +the interleave-set (region) contributes in advance of attaching that
> +allocation to a region. For development, test, and debug purposes this
> +command is a helper to find the next available decoder on endpoint
> +(memdev) and mark a span of DPA as busy.
> +
> +OPTIONS
> +-------
> +<memory device(s)>::
> +include::memdev-option.txt[]
> +
> +-d::
> +--decoder::
> +	Specify the decoder to attempt the allocation. The CXL specification
> +	mandates that allocations must be ordered by DPA and decoder instance.
> +	I.e. the lowest DPA allocation on the device is covered by deocder0, and

decoder0

> +	the last / highest DPA allocation is covered by the last decoder. This
> +	ordering is enforced by the kernel. By default the tool picks the 'next
> +	available' decoder.


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-07-20 13:16 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-14 17:01 [ndctl PATCH v2 00/12] cxl: Region provisioning foundation Dan Williams
2022-07-14 17:01 ` [ndctl PATCH v2 01/12] cxl/list: Reformat option list Dan Williams
2022-07-14 17:01 ` [ndctl PATCH v2 02/12] cxl/list: Emit endpoint decoders filtered by memdev Dan Williams
2022-07-14 17:02 ` [ndctl PATCH v2 03/12] cxl/list: Hide 0s in disabled decoder listings Dan Williams
2022-07-14 17:02 ` [ndctl PATCH v2 04/12] cxl/list: Add DPA span to endpoint " Dan Williams
2022-07-14 17:02 ` [ndctl PATCH v2 05/12] ccan/list: Import latest list helpers Dan Williams
2022-07-14 17:20   ` Ira Weiny
2022-07-14 17:02 ` [ndctl PATCH v2 06/12] cxl/lib: Maintain decoders in id order Dan Williams
2022-07-14 17:02 ` [ndctl PATCH v2 07/12] cxl/memdev: Fix json for multi-device partitioning Dan Williams
2022-07-14 17:02 ` [ndctl PATCH v2 08/12] cxl/list: Emit 'mode' for endpoint decoder objects Dan Williams
2022-07-14 17:02 ` [ndctl PATCH v2 09/12] cxl/set-partition: Accept 'ram' as an alias for 'volatile' Dan Williams
2022-07-14 17:02 ` [ndctl PATCH v2 10/12] cxl/memdev: Add {reserve,free}-dpa commands Dan Williams
2022-07-20 13:16   ` Jonathan Cameron
2022-07-14 17:02 ` [ndctl PATCH v2 11/12] cxl/test: Update CXL memory parameters Dan Williams
2022-07-14 17:02 ` [ndctl PATCH v2 12/12] cxl/test: Checkout region setup/teardown Dan Williams
2022-07-14 20:55   ` Verma, Vishal L
2022-07-14 21:13     ` Dan Williams
2022-07-14 21:26       ` Verma, Vishal L

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.