[NDCTL PATCH 0/3] ndctl: Add support of QoS Throttling Group (QTG) id for CXL CLI

[NDCTL PATCH 0/3] ndctl: Add support of QoS Throttling Group (QTG) id for CXL CLI

[Dave Jiang]

The series adds support for the kernel enabling [1] of QoS Throttling Group
(QTG) id. The kernel exports a QTG id for the root decoders (CFMWS) and as
well as for the CXL memory devices. The QTG id exported for a device is
calculated by the driver during device probe. Currently a QTG id is exported
for the volatile partition and another for the persistent partition. In the
future QTG id(s) will be exported for DCD regions. Display of QTG id is
through the CXL CLI list command.

A QTG id check as also been added for region creation. A warning is emitted
when the QTG id of a memory range of a CXL memory device being included in
the CXL region assembly does not match the QTG id of the root decoder. Options
are available to suppress the warning or to fail the region creation. This
enabling provides a guidance on flagging memory ranges being used is not
optimal for performance for the CXL region to be formed.

[1]: https://lore.kernel.org/linux-cxl/168088732996.1441063.10107817505475386072.stgit@djiang5-mobl3/T/#t

---

Dave Jiang (3):
      ndctl: Add QTG ID support for the root decoder
      ndctl: Add QTG ID support for the memory device
      ndctl: add QTG ID check for region creation


 Documentation/cxl/cxl-create-region.txt |  9 ++++
 cxl/json.c                              | 22 +++++++++-
 cxl/lib/libcxl.c                        | 31 ++++++++++++++
 cxl/lib/libcxl.sym                      |  3 ++
 cxl/lib/private.h                       |  3 ++
 cxl/libcxl.h                            |  5 +++
 cxl/region.c                            | 57 ++++++++++++++++++++++++-
 7 files changed, 128 insertions(+), 2 deletions(-)

--

[NDCTL PATCH 1/3] ndctl: Add QTG ID support for the root decoder

[Dave Jiang]

Add libcxl API to retrieve the QoS Throttling Group (QTG) ID for the root
decoder. Also add support to display the QTG ID for the root decoder
through the 'cxl list' command.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 cxl/json.c         |   10 ++++++++++
 cxl/lib/libcxl.c   |   11 +++++++++++
 cxl/lib/libcxl.sym |    1 +
 cxl/lib/private.h  |    1 +
 cxl/libcxl.h       |    3 +++
 5 files changed, 26 insertions(+)

diff --git a/cxl/json.c b/cxl/json.c
index e87bdd49a776..8dd65f942c6a 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -760,6 +760,16 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
 					       jobj);
 	}
 
+	if (cxl_port_is_root(port)) {
+		int qtg_id = cxl_decoder_get_qtg_id(decoder);
+
+		if (qtg_id != CXL_QTG_ID_NONE) {
+			jobj = json_object_new_int(qtg_id);
+			if (jobj)
+				json_object_object_add(jdecoder, "qtg_id", jobj);
+		}
+	}
+
 	json_object_set_userdata(jdecoder, decoder, NULL);
 	return jdecoder;
 }
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 59e5bdbcc750..26985c9344b4 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -1879,6 +1879,12 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
 	else
 		decoder->interleave_ways = strtoul(buf, NULL, 0);
 
+	sprintf(path, "%s/qtg_id", cxldecoder_base);
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		decoder->qtg_id = CXL_QTG_ID_NONE;
+	else
+		decoder->qtg_id = atoi(buf);
+
 	switch (port->type) {
 	case CXL_PORT_ENDPOINT:
 		sprintf(path, "%s/dpa_resource", cxldecoder_base);
@@ -2073,6 +2079,11 @@ CXL_EXPORT unsigned long long cxl_decoder_get_size(struct cxl_decoder *decoder)
 	return decoder->size;
 }
 
+CXL_EXPORT int cxl_decoder_get_qtg_id(struct cxl_decoder *decoder)
+{
+	return decoder->qtg_id;
+}
+
 CXL_EXPORT unsigned long long
 cxl_decoder_get_dpa_resource(struct cxl_decoder *decoder)
 {
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 1c6177c7dcae..d1c61f9252fe 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -248,4 +248,5 @@ global:
 	cxl_region_get_mode;
 	cxl_decoder_create_ram_region;
 	cxl_region_get_daxctl_region;
+	cxl_decoder_get_qtg_id;
 } LIBCXL_4;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index d648992b808d..ac6f111b5956 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -126,6 +126,7 @@ struct cxl_decoder {
 	struct list_head targets;
 	struct list_head regions;
 	struct list_head stale_regions;
+	int qtg_id;
 };
 
 enum cxl_decode_state {
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 54d9f10537dd..66ce4a021c62 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -135,6 +135,8 @@ struct cxl_dport *cxl_port_get_dport_by_memdev(struct cxl_port *port,
 	for (dport = cxl_dport_get_first(port); dport != NULL;                 \
 	     dport = cxl_dport_get_next(dport))
 
+#define CXL_QTG_ID_NONE		-1
+
 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);
@@ -146,6 +148,7 @@ unsigned long long cxl_decoder_get_dpa_resource(struct cxl_decoder *decoder);
 unsigned long long cxl_decoder_get_dpa_size(struct cxl_decoder *decoder);
 unsigned long long
 cxl_decoder_get_max_available_extent(struct cxl_decoder *decoder);
+int cxl_decoder_get_qtg_id(struct cxl_decoder *decoder);
 
 enum cxl_decoder_mode {
 	CXL_DECODER_MODE_NONE,

Re: [NDCTL PATCH 1/3] ndctl: Add QTG ID support for the root decoder

[Alison Schofield]

On Fri, Apr 14, 2023 at 10:43:01AM -0700, Dave Jiang wrote:
> Add libcxl API to retrieve the QoS Throttling Group (QTG) ID for the root
> decoder. Also add support to display the QTG ID for the root decoder
> through the 'cxl list' command.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  cxl/json.c         |   10 ++++++++++
>  cxl/lib/libcxl.c   |   11 +++++++++++
>  cxl/lib/libcxl.sym |    1 +
>  cxl/lib/private.h  |    1 +
>  cxl/libcxl.h       |    3 +++
>  5 files changed, 26 insertions(+)
> 
> diff --git a/cxl/json.c b/cxl/json.c
> index e87bdd49a776..8dd65f942c6a 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -760,6 +760,16 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
>  					       jobj);
>  	}
>  
> +	if (cxl_port_is_root(port)) {
> +		int qtg_id = cxl_decoder_get_qtg_id(decoder);
> +
> +		if (qtg_id != CXL_QTG_ID_NONE) {
> +			jobj = json_object_new_int(qtg_id);
> +			if (jobj)
> +				json_object_object_add(jdecoder, "qtg_id", jobj);
> +		}
> +	}

IIUC,
- root decoders don't always have a valid qtg_id, and the field is
omitted from the listing in that case.
- memdev's (next patch) always have a valid (not -1) qtg_id.

I guess it's customary, in cxl list, to not display empty fields.
Well, that was my question and I think I answered it ;)

Alison

> +
>  	json_object_set_userdata(jdecoder, decoder, NULL);
>  	return jdecoder;
>  }
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 59e5bdbcc750..26985c9344b4 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -1879,6 +1879,12 @@ static void *add_cxl_decoder(void *parent, int id, const char *cxldecoder_base)
>  	else
>  		decoder->interleave_ways = strtoul(buf, NULL, 0);
>  
> +	sprintf(path, "%s/qtg_id", cxldecoder_base);
> +	if (sysfs_read_attr(ctx, path, buf) < 0)
> +		decoder->qtg_id = CXL_QTG_ID_NONE;
> +	else
> +		decoder->qtg_id = atoi(buf);
> +
>  	switch (port->type) {
>  	case CXL_PORT_ENDPOINT:
>  		sprintf(path, "%s/dpa_resource", cxldecoder_base);
> @@ -2073,6 +2079,11 @@ CXL_EXPORT unsigned long long cxl_decoder_get_size(struct cxl_decoder *decoder)
>  	return decoder->size;
>  }
>  
> +CXL_EXPORT int cxl_decoder_get_qtg_id(struct cxl_decoder *decoder)
> +{
> +	return decoder->qtg_id;
> +}
> +
>  CXL_EXPORT unsigned long long
>  cxl_decoder_get_dpa_resource(struct cxl_decoder *decoder)
>  {
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 1c6177c7dcae..d1c61f9252fe 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -248,4 +248,5 @@ global:
>  	cxl_region_get_mode;
>  	cxl_decoder_create_ram_region;
>  	cxl_region_get_daxctl_region;
> +	cxl_decoder_get_qtg_id;
>  } LIBCXL_4;
> diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> index d648992b808d..ac6f111b5956 100644
> --- a/cxl/lib/private.h
> +++ b/cxl/lib/private.h
> @@ -126,6 +126,7 @@ struct cxl_decoder {
>  	struct list_head targets;
>  	struct list_head regions;
>  	struct list_head stale_regions;
> +	int qtg_id;
>  };
>  
>  enum cxl_decode_state {
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index 54d9f10537dd..66ce4a021c62 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -135,6 +135,8 @@ struct cxl_dport *cxl_port_get_dport_by_memdev(struct cxl_port *port,
>  	for (dport = cxl_dport_get_first(port); dport != NULL;                 \
>  	     dport = cxl_dport_get_next(dport))
>  
> +#define CXL_QTG_ID_NONE		-1
> +
>  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);
> @@ -146,6 +148,7 @@ unsigned long long cxl_decoder_get_dpa_resource(struct cxl_decoder *decoder);
>  unsigned long long cxl_decoder_get_dpa_size(struct cxl_decoder *decoder);
>  unsigned long long
>  cxl_decoder_get_max_available_extent(struct cxl_decoder *decoder);
> +int cxl_decoder_get_qtg_id(struct cxl_decoder *decoder);
>  
>  enum cxl_decoder_mode {
>  	CXL_DECODER_MODE_NONE,
> 
> 

[NDCTL PATCH 2/3] ndctl: Add QTG ID support for the memory device

[Dave Jiang]

Add libcxl API to retrieve the QoS Throttling Group (QTG) ID for the
memory devices. Two API calls are added. One for 'ram' or 'volatile'
mode and another for 'pmem' or 'persistent' mode. Support also added
for displaying the QTG ID through the 'cxl list' command.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 cxl/json.c         |   12 +++++++++++-
 cxl/lib/libcxl.c   |   20 ++++++++++++++++++++
 cxl/lib/libcxl.sym |    2 ++
 cxl/lib/private.h  |    2 ++
 cxl/libcxl.h       |    2 ++
 5 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/cxl/json.c b/cxl/json.c
index 8dd65f942c6a..9a508cf7950e 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -486,7 +486,7 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
 	const char *devname = cxl_memdev_get_devname(memdev);
 	struct json_object *jdev, *jobj;
 	unsigned long long serial, size;
-	int numa_node;
+	int numa_node, qtg_id;
 
 	jdev = json_object_new_object();
 	if (!jdev)
@@ -501,6 +501,11 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
 		jobj = util_json_object_size(size, flags);
 		if (jobj)
 			json_object_object_add(jdev, "pmem_size", jobj);
+
+		qtg_id = cxl_memdev_get_pmem_qtg_id(memdev);
+		jobj = json_object_new_int(qtg_id);
+		if (jobj)
+			json_object_object_add(jdev, "qtg_id", jobj);
 	}
 
 	size = cxl_memdev_get_ram_size(memdev);
@@ -508,6 +513,11 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
 		jobj = util_json_object_size(size, flags);
 		if (jobj)
 			json_object_object_add(jdev, "ram_size", jobj);
+
+		qtg_id = cxl_memdev_get_ram_qtg_id(memdev);
+		jobj = json_object_new_int(qtg_id);
+		if (jobj)
+			json_object_object_add(jdev, "qtg_id", jobj);
 	}
 
 	if (flags & UTIL_JSON_HEALTH) {
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 26985c9344b4..dca1683523a1 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -1210,6 +1210,16 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
 		goto err_read;
 	memdev->ram_size = strtoull(buf, NULL, 0);
 
+	sprintf(path, "%s/pmem/qtg_id", cxlmem_base);
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		goto err_read;
+	memdev->pmem_qtg_id = atoi(buf);
+
+	sprintf(path, "%s/ram/qtg_id", cxlmem_base);
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		goto err_read;
+	memdev->ram_qtg_id = atoi(buf);
+
 	sprintf(path, "%s/payload_max", cxlmem_base);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
 		goto err_read;
@@ -1368,6 +1378,16 @@ CXL_EXPORT unsigned long long cxl_memdev_get_ram_size(struct cxl_memdev *memdev)
 	return memdev->ram_size;
 }
 
+CXL_EXPORT int cxl_memdev_get_pmem_qtg_id(struct cxl_memdev *memdev)
+{
+	return memdev->pmem_qtg_id;
+}
+
+CXL_EXPORT int cxl_memdev_get_ram_qtg_id(struct cxl_memdev *memdev)
+{
+	return memdev->ram_qtg_id;
+}
+
 CXL_EXPORT const char *cxl_memdev_get_firmware_verison(struct cxl_memdev *memdev)
 {
 	return memdev->firmware_version;
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index d1c61f9252fe..60ad16e33f30 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -249,4 +249,6 @@ global:
 	cxl_decoder_create_ram_region;
 	cxl_region_get_daxctl_region;
 	cxl_decoder_get_qtg_id;
+	cxl_memdev_get_pmem_qtg_id;
+	cxl_memdev_get_ram_qtg_id;
 } LIBCXL_4;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index ac6f111b5956..dfd77ff2a781 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -32,6 +32,8 @@ struct cxl_memdev {
 	struct list_node list;
 	unsigned long long pmem_size;
 	unsigned long long ram_size;
+	int pmem_qtg_id;
+	int ram_qtg_id;
 	int payload_max;
 	size_t lsa_size;
 	struct kmod_module *module;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 66ce4a021c62..c89806cbbd57 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -47,6 +47,8 @@ int cxl_memdev_get_minor(struct cxl_memdev *memdev);
 struct cxl_ctx *cxl_memdev_get_ctx(struct cxl_memdev *memdev);
 unsigned long long cxl_memdev_get_pmem_size(struct cxl_memdev *memdev);
 unsigned long long cxl_memdev_get_ram_size(struct cxl_memdev *memdev);
+int cxl_memdev_get_pmem_qtg_id(struct cxl_memdev *memdev);
+int cxl_memdev_get_ram_qtg_id(struct cxl_memdev *memdev);
 const char *cxl_memdev_get_firmware_verison(struct cxl_memdev *memdev);
 
 /* ABI spelling mistakes are forever */

[NDCTL PATCH 3/3] ndctl: add QTG ID check for region creation

[Dave Jiang]

The CFMWS provides a QTG ID. The kernel driver creates a root decoder that
represents the CFMWS. A qtg_id attribute is exported via sysfs for the root
decoder.

A QTG id is retrieved via QTG ID _DSM from the ACPI0017 device for a CXL
memory device. The input for the _DSM is the read and write latency and
bandwidth for the path between the device and the CPU. The numbers are
constructed by the kernel driver for the _DSM input. When a device is
probed, the QTG ID is retrieved. This is useful for a hot-plugged CXL
memory device that does not have regions created.

Add a check for config check during region creation. Emit a warning if the
QTG ID from the root decoder is different than the mem device QTG ID. User
parameter options are provided to fail instead of just warning.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/cxl/cxl-create-region.txt |    9 +++++
 cxl/region.c                            |   57 ++++++++++++++++++++++++++++++-
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
index f11a412bddfe..9ab2e0fee152 100644
--- a/Documentation/cxl/cxl-create-region.txt
+++ b/Documentation/cxl/cxl-create-region.txt
@@ -105,6 +105,15 @@ include::bus-option.txt[]
 	supplied, the first cross-host bridge (if available), decoder that
 	supports the largest interleave will be chosen.
 
+-e::
+--strict::
+	Enforce strict execution where any potential error will force failure.
+	For example, if QTG ID mismatches will cause failure.
+
+-q::
+--no-enforce-qtg::
+	Parameter to bypass QTG ID mismatch failure. Will only emit warning.
+
 include::human-option.txt[]
 
 include::debug-option.txt[]
diff --git a/cxl/region.c b/cxl/region.c
index 07ce4a319fd0..6f611799f39f 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -31,6 +31,8 @@ static struct region_params {
 	bool force;
 	bool human;
 	bool debug;
+	bool strict;
+	bool no_qtg;
 } param = {
 	.ways = INT_MAX,
 	.granularity = INT_MAX,
@@ -48,6 +50,8 @@ struct parsed_params {
 	const char **argv;
 	struct cxl_decoder *root_decoder;
 	enum cxl_decoder_mode mode;
+	bool strict;
+	bool no_qtg;
 };
 
 enum region_actions {
@@ -80,7 +84,9 @@ OPT_STRING('U', "uuid", &param.uuid, \
 	   "region uuid", "uuid for the new region (default: autogenerate)"), \
 OPT_BOOLEAN('m', "memdevs", &param.memdevs, \
 	    "non-option arguments are memdevs"), \
-OPT_BOOLEAN('u', "human", &param.human, "use human friendly number formats")
+OPT_BOOLEAN('u', "human", &param.human, "use human friendly number formats"), \
+OPT_BOOLEAN('e', "strict", &param.strict, "strict execution enforcement"), \
+OPT_BOOLEAN('q', "no-enforce-qtg", &param.no_qtg, "no enforce of QTG ID")
 
 static const struct option create_options[] = {
 	BASE_OPTIONS(),
@@ -357,6 +363,9 @@ static int parse_create_options(struct cxl_ctx *ctx, int count,
 		}
 	}
 
+	p->strict = param.strict;
+	p->no_qtg = param.no_qtg;
+
 	return 0;
 }
 
@@ -460,6 +469,50 @@ static void set_type_from_decoder(struct cxl_ctx *ctx, struct parsed_params *p)
 		p->mode = CXL_DECODER_MODE_PMEM;
 }
 
+static int create_region_validate_qtg_id(struct cxl_ctx *ctx,
+					 struct parsed_params *p)
+{
+	int root_qtg_id, dev_qtg_id, i;
+
+	root_qtg_id = cxl_decoder_get_qtg_id(p->root_decoder);
+	if (root_qtg_id == -1)
+		return 0;
+
+	for (i = 0; i < p->ways; i++) {
+		struct json_object *jobj =
+			json_object_array_get_idx(p->memdevs, i);
+		struct cxl_memdev *memdev = json_object_get_userdata(jobj);
+
+		if (p->mode == CXL_DECODER_MODE_RAM)
+			dev_qtg_id = cxl_memdev_get_ram_qtg_id(memdev);
+		else
+			dev_qtg_id = cxl_memdev_get_pmem_qtg_id(memdev);
+
+		if (dev_qtg_id == -1)
+			return 0;
+
+		if (root_qtg_id != dev_qtg_id) {
+			if (p->strict && !p->no_qtg) {
+				log_err(&rl, "%s QTG ID %d mismatch %s QTG ID %d\n",
+					cxl_decoder_get_devname(p->root_decoder),
+					root_qtg_id,
+					cxl_memdev_get_devname(memdev),
+					dev_qtg_id);
+
+				return -ENXIO;
+			} else {
+				log_notice(&rl, "%s QTG ID %d mismatch %s QTG ID %d\n",
+					   cxl_decoder_get_devname(p->root_decoder),
+					   root_qtg_id,
+					   cxl_memdev_get_devname(memdev),
+					   dev_qtg_id);
+			}
+		}
+	}
+
+	return 0;
+}
+
 static int create_region_validate_config(struct cxl_ctx *ctx,
 					 struct parsed_params *p)
 {
@@ -500,6 +553,8 @@ found:
 		return rc;
 
 	collect_minsize(ctx, p);
+	create_region_validate_qtg_id(ctx, p);
+
 	return 0;
 }
 

Re: [NDCTL PATCH 0/3] ndctl: Add support of QoS Throttling Group (QTG) id for CXL CLI

[Alison Schofield]

On Fri, Apr 14, 2023 at 10:42:55AM -0700, Dave Jiang wrote:
> The series adds support for the kernel enabling [1] of QoS Throttling Group
> (QTG) id. The kernel exports a QTG id for the root decoders (CFMWS) and as
> well as for the CXL memory devices. The QTG id exported for a device is
> calculated by the driver during device probe. Currently a QTG id is exported
> for the volatile partition and another for the persistent partition. In the
> future QTG id(s) will be exported for DCD regions. Display of QTG id is
> through the CXL CLI list command.
> 
> A QTG id check as also been added for region creation. A warning is emitted
> when the QTG id of a memory range of a CXL memory device being included in
> the CXL region assembly does not match the QTG id of the root decoder. Options
> are available to suppress the warning or to fail the region creation. This
> enabling provides a guidance on flagging memory ranges being used is not
> optimal for performance for the CXL region to be formed.

Can you cut/paste me some cxl list sample output?  I'm not going to be
trying this out to review.

Thanks!


> 
> [1]: https://lore.kernel.org/linux-cxl/168088732996.1441063.10107817505475386072.stgit@djiang5-mobl3/T/#t
> 
> ---
> 
> Dave Jiang (3):
>       ndctl: Add QTG ID support for the root decoder
>       ndctl: Add QTG ID support for the memory device
>       ndctl: add QTG ID check for region creation
> 
> 
>  Documentation/cxl/cxl-create-region.txt |  9 ++++
>  cxl/json.c                              | 22 +++++++++-
>  cxl/lib/libcxl.c                        | 31 ++++++++++++++
>  cxl/lib/libcxl.sym                      |  3 ++
>  cxl/lib/private.h                       |  3 ++
>  cxl/libcxl.h                            |  5 +++
>  cxl/region.c                            | 57 ++++++++++++++++++++++++-
>  7 files changed, 128 insertions(+), 2 deletions(-)
> 
> --
> 

Re: [NDCTL PATCH 0/3] ndctl: Add support of QoS Throttling Group (QTG) id for CXL CLI

[Dave Jiang]

On 4/14/23 2:49 PM, Alison Schofield wrote:
> On Fri, Apr 14, 2023 at 10:42:55AM -0700, Dave Jiang wrote:
>> The series adds support for the kernel enabling [1] of QoS Throttling Group
>> (QTG) id. The kernel exports a QTG id for the root decoders (CFMWS) and as
>> well as for the CXL memory devices. The QTG id exported for a device is
>> calculated by the driver during device probe. Currently a QTG id is exported
>> for the volatile partition and another for the persistent partition. In the
>> future QTG id(s) will be exported for DCD regions. Display of QTG id is
>> through the CXL CLI list command.
>>
>> A QTG id check as also been added for region creation. A warning is emitted
>> when the QTG id of a memory range of a CXL memory device being included in
>> the CXL region assembly does not match the QTG id of the root decoder. Options
>> are available to suppress the warning or to fail the region creation. This
>> enabling provides a guidance on flagging memory ranges being used is not
>> optimal for performance for the CXL region to be formed.
> 
> Can you cut/paste me some cxl list sample output?  I'm not going to be
> trying this out to review.
# cxl list -D
[
   {
     "decoder":"decoder0.0",
     "resource":49660559360,
     "size":4294967296,
     "interleave_ways":1,
     "max_available_extent":4294967296,
     "pmem_capable":true,
     "volatile_capable":true,
     "accelmem_capable":true,
     "qtg_id":0,
     "nr_targets":1
   },
   {
     "decoder":"decoder0.1",
     "resource":53955526656,
     "size":4294967296,
     "interleave_ways":2,
     "interleave_granularity":8192,
     "max_available_extent":4294967296,
     "pmem_capable":true,
     "volatile_capable":true,
     "accelmem_capable":true,
     "qtg_id":0,
     "nr_targets":2
   }
]

# cxl list
[
   {
     "memdev":"mem3",
     "ram_size":268435456,
     "qtg_id":0,
     "serial":0,
     "host":"0000:c5:00.0"
   },
   {
     "memdev":"mem5",
     "pmem_size":268435456,
     "qtg_id":0,
     "serial":0,
     "host":"0000:c2:00.0"
   },
   {
     "memdev":"mem2",
     "ram_size":268435456,
     "qtg_id":0,
     "serial":0,
     "host":"0000:c4:00.0"
   },
   {
     "memdev":"mem7",
     "pmem_size":268435456,
     "qtg_id":0,
     "serial":0,
     "host":"0000:c3:00.0"
   },
   {
     "memdev":"mem6",
     "ram_size":268435456,
     "qtg_id":0,
     "serial":0,
     "host":"0000:38:00.0"
   },
   {
     "memdev":"mem1",
     "pmem_size":268435456,
     "qtg_id":0,
     "serial":0,
     "host":"0000:37:00.0"
   },
   {
     "memdev":"mem4",
     "ram_size":268435456,
     "qtg_id":0,
     "serial":0,
     "host":"0000:39:00.0"
   },
   {
     "memdev":"mem0",
     "pmem_size":268435456,
     "qtg_id":0,
     "serial":0,
     "host":"0000:36:00.0"
   }
]



> 
> Thanks!
> 
> 
>>
>> [1]: https://lore.kernel.org/linux-cxl/168088732996.1441063.10107817505475386072.stgit@djiang5-mobl3/T/#t
>>
>> ---
>>
>> Dave Jiang (3):
>>        ndctl: Add QTG ID support for the root decoder
>>        ndctl: Add QTG ID support for the memory device
>>        ndctl: add QTG ID check for region creation
>>
>>
>>   Documentation/cxl/cxl-create-region.txt |  9 ++++
>>   cxl/json.c                              | 22 +++++++++-
>>   cxl/lib/libcxl.c                        | 31 ++++++++++++++
>>   cxl/lib/libcxl.sym                      |  3 ++
>>   cxl/lib/private.h                       |  3 ++
>>   cxl/libcxl.h                            |  5 +++
>>   cxl/region.c                            | 57 ++++++++++++++++++++++++-
>>   7 files changed, 128 insertions(+), 2 deletions(-)
>>
>> --
>>

Re: [NDCTL PATCH 0/3] ndctl: Add support of QoS Throttling Group (QTG) id for CXL CLI

[Alison Schofield]

On Fri, Apr 14, 2023 at 04:27:47PM -0700, Dave Jiang wrote:
> 
> 
> On 4/14/23 2:49 PM, Alison Schofield wrote:
> > On Fri, Apr 14, 2023 at 10:42:55AM -0700, Dave Jiang wrote:
> > > The series adds support for the kernel enabling [1] of QoS Throttling Group
> > > (QTG) id. The kernel exports a QTG id for the root decoders (CFMWS) and as
> > > well as for the CXL memory devices. The QTG id exported for a device is
> > > calculated by the driver during device probe. Currently a QTG id is exported
> > > for the volatile partition and another for the persistent partition. In the
> > > future QTG id(s) will be exported for DCD regions. Display of QTG id is
> > > through the CXL CLI list command.
> > > 
> > > A QTG id check as also been added for region creation. A warning is emitted
> > > when the QTG id of a memory range of a CXL memory device being included in
> > > the CXL region assembly does not match the QTG id of the root decoder. Options
> > > are available to suppress the warning or to fail the region creation. This
> > > enabling provides a guidance on flagging memory ranges being used is not
> > > optimal for performance for the CXL region to be formed.
> > 
> > Can you cut/paste me some cxl list sample output?  I'm not going to be
> > trying this out to review.


Thanks for the samples.
The cxl list man page sample output may need updating.
Alison


> # cxl list -D
> [
>   {
>     "decoder":"decoder0.0",
>     "resource":49660559360,
>     "size":4294967296,
>     "interleave_ways":1,
>     "max_available_extent":4294967296,
>     "pmem_capable":true,
>     "volatile_capable":true,
>     "accelmem_capable":true,
>     "qtg_id":0,
>     "nr_targets":1
>   },
>   {
>     "decoder":"decoder0.1",
>     "resource":53955526656,
>     "size":4294967296,
>     "interleave_ways":2,
>     "interleave_granularity":8192,
>     "max_available_extent":4294967296,
>     "pmem_capable":true,
>     "volatile_capable":true,
>     "accelmem_capable":true,
>     "qtg_id":0,
>     "nr_targets":2
>   }
> ]
> 
> # cxl list
> [
>   {
>     "memdev":"mem3",
>     "ram_size":268435456,
>     "qtg_id":0,
>     "serial":0,
>     "host":"0000:c5:00.0"
>   },
>   {
>     "memdev":"mem5",
>     "pmem_size":268435456,
>     "qtg_id":0,
>     "serial":0,
>     "host":"0000:c2:00.0"
>   },
>   {
>     "memdev":"mem2",
>     "ram_size":268435456,
>     "qtg_id":0,
>     "serial":0,
>     "host":"0000:c4:00.0"
>   },
>   {
>     "memdev":"mem7",
>     "pmem_size":268435456,
>     "qtg_id":0,
>     "serial":0,
>     "host":"0000:c3:00.0"
>   },
>   {
>     "memdev":"mem6",
>     "ram_size":268435456,
>     "qtg_id":0,
>     "serial":0,
>     "host":"0000:38:00.0"
>   },
>   {
>     "memdev":"mem1",
>     "pmem_size":268435456,
>     "qtg_id":0,
>     "serial":0,
>     "host":"0000:37:00.0"
>   },
>   {
>     "memdev":"mem4",
>     "ram_size":268435456,
>     "qtg_id":0,
>     "serial":0,
>     "host":"0000:39:00.0"
>   },
>   {
>     "memdev":"mem0",
>     "pmem_size":268435456,
>     "qtg_id":0,
>     "serial":0,
>     "host":"0000:36:00.0"
>   }
> ]
> 
> 
> 
> > 
> > Thanks!
> > 
> > 
> > > 
> > > [1]: https://lore.kernel.org/linux-cxl/168088732996.1441063.10107817505475386072.stgit@djiang5-mobl3/T/#t
> > > 
> > > ---
> > > 
> > > Dave Jiang (3):
> > >        ndctl: Add QTG ID support for the root decoder
> > >        ndctl: Add QTG ID support for the memory device
> > >        ndctl: add QTG ID check for region creation
> > > 
> > > 
> > >   Documentation/cxl/cxl-create-region.txt |  9 ++++
> > >   cxl/json.c                              | 22 +++++++++-
> > >   cxl/lib/libcxl.c                        | 31 ++++++++++++++
> > >   cxl/lib/libcxl.sym                      |  3 ++
> > >   cxl/lib/private.h                       |  3 ++
> > >   cxl/libcxl.h                            |  5 +++
> > >   cxl/region.c                            | 57 ++++++++++++++++++++++++-
> > >   7 files changed, 128 insertions(+), 2 deletions(-)
> > > 
> > > --
> > >