[NDCTL PATCH v3 0/3] ndctl: Add support of qos_class for CXL CLI

[NDCTL PATCH v3 0/3] ndctl: Add support of qos_class for CXL CLI

[Dave Jiang]

Hi Vishal,
With the QoS class series merged to the v6.8 kernel, can you please review and
apply this series to ndctl if acceptable?

v3:
- Rebase against latest ndctl/pending branch.

The series adds support for the kernel enabling of QoS class in the v6.8
kernel. The kernel exports a qos_class token for the root decoders (CFMWS) and as
well as for the CXL memory devices. The qos_class exported for a device is
calculated by the driver during device probe. Currently a qos_class is exported
for the volatile partition (ram) and another for the persistent partition (pmem).
In the future qos_class will be exported for DCD regions. Display of qos_class is
through the CXL CLI list command with -vvv for extra verbose.

A qos_class check as also been added for region creation. A warning is emitted
when the qos_class of a memory range of a CXL memory device being included in
the CXL region assembly does not match the qos_class 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.

---

Dave Jiang (3):
      ndctl: cxl: Add QoS class retrieval for the root decoder
      ndctl: cxl: Add QoS class support for the memory device
      ndctl: cxl: add QoS class check for CXL region creation


 Documentation/cxl/cxl-create-region.txt |  9 ++++
 cxl/filter.h                            |  4 ++
 cxl/json.c                              | 46 ++++++++++++++++-
 cxl/lib/libcxl.c                        | 62 +++++++++++++++++++++++
 cxl/lib/libcxl.sym                      |  3 ++
 cxl/lib/private.h                       |  3 ++
 cxl/libcxl.h                            | 10 ++++
 cxl/list.c                              |  1 +
 cxl/region.c                            | 67 ++++++++++++++++++++++++-
 util/json.h                             |  1 +
 10 files changed, 204 insertions(+), 2 deletions(-)

--

[NDCTL PATCH v3 1/3] ndctl: cxl: Add QoS class retrieval for the root decoder

[Dave Jiang]

Add libcxl API to retrieve the QoS class for the root decoder. Also add
support to display the QoS class for the root decoder through the 'cxl
list' command. The qos_class is displayed behind -vvv verbose level.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v3:
- Rebase to latest pending branch
---
 cxl/filter.h       |    4 ++++
 cxl/json.c         |   10 ++++++++++
 cxl/lib/libcxl.c   |   14 ++++++++++++++
 cxl/lib/libcxl.sym |    1 +
 cxl/lib/private.h  |    1 +
 cxl/libcxl.h       |    3 +++
 cxl/list.c         |    1 +
 util/json.h        |    1 +
 8 files changed, 35 insertions(+)

diff --git a/cxl/filter.h b/cxl/filter.h
index 1241f72ccf62..3c5f9e8a0452 100644
--- a/cxl/filter.h
+++ b/cxl/filter.h
@@ -31,6 +31,7 @@ struct cxl_filter_params {
 	bool alert_config;
 	bool dax;
 	bool poison;
+	bool qos;
 	int verbose;
 	struct log_ctx ctx;
 };
@@ -91,6 +92,9 @@ static inline unsigned long cxl_filter_to_flags(struct cxl_filter_params *param)
 		flags |= UTIL_JSON_DAX | UTIL_JSON_DAX_DEVS;
 	if (param->poison)
 		flags |= UTIL_JSON_MEDIA_ERRORS;
+	if (param->qos)
+		flags |= UTIL_JSON_QOS_CLASS;
+
 	return flags;
 }
 
diff --git a/cxl/json.c b/cxl/json.c
index 6fb17582a1cb..48a43ddf14b0 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -1062,6 +1062,16 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
 					       jobj);
 	}
 
+	if ((flags & UTIL_JSON_QOS_CLASS) && cxl_port_is_root(port)) {
+		int qos_class = cxl_root_decoder_get_qos_class(decoder);
+
+		if (qos_class != CXL_QOS_CLASS_NONE) {
+			jobj = json_object_new_int(qos_class);
+			if (jobj)
+				json_object_object_add(jdecoder, "qos_class", jobj);
+		}
+	}
+
 	json_object_set_userdata(jdecoder, decoder, NULL);
 	return jdecoder;
 }
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 1537a33d370e..9a1ac7001803 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -2229,6 +2229,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/qos_class", cxldecoder_base);
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		decoder->qos_class = CXL_QOS_CLASS_NONE;
+	else
+		decoder->qos_class = atoi(buf);
+
 	switch (port->type) {
 	case CXL_PORT_ENDPOINT:
 		sprintf(path, "%s/dpa_resource", cxldecoder_base);
@@ -2423,6 +2429,14 @@ CXL_EXPORT unsigned long long cxl_decoder_get_size(struct cxl_decoder *decoder)
 	return decoder->size;
 }
 
+CXL_EXPORT int cxl_root_decoder_get_qos_class(struct cxl_decoder *decoder)
+{
+	if (!cxl_port_is_root(decoder->port))
+		return -EINVAL;
+
+	return decoder->qos_class;
+}
+
 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 2149f84d764e..384fea2c25e3 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -282,4 +282,5 @@ global:
 LIBCXL_8 {
 global:
 	cxl_memdev_wait_sanitize;
+	cxl_root_decoder_get_qos_class;
 } LIBCXL_7;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index b26a8629e047..4847ff448f71 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -144,6 +144,7 @@ struct cxl_decoder {
 	struct list_head targets;
 	struct list_head regions;
 	struct list_head stale_regions;
+	int qos_class;
 };
 
 enum cxl_decode_state {
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 352b3a866f63..e5c08da77f77 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -173,6 +173,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_QOS_CLASS_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);
@@ -184,6 +186,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_root_decoder_get_qos_class(struct cxl_decoder *decoder);
 
 enum cxl_decoder_mode {
 	CXL_DECODER_MODE_NONE,
diff --git a/cxl/list.c b/cxl/list.c
index 13fef8569340..f6446f98c2bd 100644
--- a/cxl/list.c
+++ b/cxl/list.c
@@ -123,6 +123,7 @@ int cmd_list(int argc, const char **argv, struct cxl_ctx *ctx)
 		param.fw = true;
 		param.alert_config = true;
 		param.dax = true;
+		param.qos = true;
 		/* fallthrough */
 	case 2:
 		param.idle = true;
diff --git a/util/json.h b/util/json.h
index ea370df4d1b7..b07055005084 100644
--- a/util/json.h
+++ b/util/json.h
@@ -21,6 +21,7 @@ enum util_json_flags {
 	UTIL_JSON_TARGETS	= (1 << 11),
 	UTIL_JSON_PARTITION	= (1 << 12),
 	UTIL_JSON_ALERT_CONFIG	= (1 << 13),
+	UTIL_JSON_QOS_CLASS	= (1 << 14),
 };
 
 void util_display_json_array(FILE *f_out, struct json_object *jarray,

[NDCTL PATCH v3 2/3] ndctl: cxl: Add QoS class support for the memory device

[Dave Jiang]

Add libcxl API to retrieve the QoS class tokens 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 QoS class tokens through the 'cxl list' command.
There can be 1 or more QoS class tokens for the memory device if
they are valid. The qos_class tokens are displayed behind -vvv
verbose level.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v3:
- Rebase to pending branch
- Skip from failing if no qos_class sysfs attrib found
---
 cxl/json.c         |   36 +++++++++++++++++++++++++++++++++++-
 cxl/lib/libcxl.c   |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
 cxl/lib/libcxl.sym |    2 ++
 cxl/lib/private.h  |    2 ++
 cxl/libcxl.h       |    7 +++++++
 5 files changed, 94 insertions(+), 1 deletion(-)

diff --git a/cxl/json.c b/cxl/json.c
index 48a43ddf14b0..dcbac8c14f03 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -770,12 +770,32 @@ err_free:
 	return jpoison;
 }
 
+static struct json_object *get_qos_json_object(struct json_object *jdev,
+					       struct qos_class *qos_class)
+{
+	struct json_object *jqos_array = json_object_new_array();
+	struct json_object *jobj;
+	int i;
+
+	if (!jqos_array)
+		return NULL;
+
+	for (i = 0; i < qos_class->nr; i++) {
+		jobj = json_object_new_int(qos_class->qos[i]);
+		if (jobj)
+			json_object_array_add(jqos_array, jobj);
+	}
+
+	return jqos_array;
+}
+
 struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
 		unsigned long flags)
 {
 	const char *devname = cxl_memdev_get_devname(memdev);
-	struct json_object *jdev, *jobj;
+	struct json_object *jdev, *jobj, *jqos;
 	unsigned long long serial, size;
+	struct qos_class *qos_class;
 	int numa_node;
 
 	jdev = json_object_new_object();
@@ -791,6 +811,13 @@ 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);
+
+		if (flags & UTIL_JSON_QOS_CLASS) {
+			qos_class = cxl_memdev_get_pmem_qos_class(memdev);
+			jqos = get_qos_json_object(jdev, qos_class);
+			if (jqos)
+				json_object_object_add(jdev, "pmem_qos_class", jqos);
+		}
 	}
 
 	size = cxl_memdev_get_ram_size(memdev);
@@ -798,6 +825,13 @@ 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);
+
+		if (flags & UTIL_JSON_QOS_CLASS) {
+			qos_class = cxl_memdev_get_ram_qos_class(memdev);
+			jqos = get_qos_json_object(jdev, qos_class);
+			if (jqos)
+				json_object_object_add(jdev, "ram_qos_class", jqos);
+		}
 	}
 
 	if (flags & UTIL_JSON_HEALTH) {
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 9a1ac7001803..c69a18c4d237 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -88,6 +88,10 @@ static void free_memdev(struct cxl_memdev *memdev, struct list_head *head)
 	free(memdev->dev_buf);
 	free(memdev->dev_path);
 	free(memdev->host_path);
+	if (memdev->ram_qos_class.nr)
+		free(memdev->ram_qos_class.qos);
+	if (memdev->pmem_qos_class.nr)
+		free(memdev->pmem_qos_class.qos);
 	free(memdev);
 }
 
@@ -1224,6 +1228,27 @@ static int add_cxl_memdev_fwl(struct cxl_memdev *memdev,
 	return -ENOMEM;
 }
 
+static int *get_qos_class(struct cxl_ctx *ctx, char *buf, int *entries)
+{
+	int *varray = NULL;
+	int i = 0;
+	char *p;
+
+	p = strtok(buf, ",");
+	while (p != NULL) {
+		int val = atoi(p);
+
+		varray = reallocarray(varray, i + 1, sizeof(int));
+		varray[i] = val;
+		p = strtok(NULL, ",");
+		i++;
+	}
+
+	*entries = i;
+
+	return varray;
+}
+
 static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
 {
 	const char *devname = devpath_to_devname(cxlmem_base);
@@ -1233,6 +1258,7 @@ static void *add_cxl_memdev(void *parent, int id, const char *cxlmem_base)
 	char buf[SYSFS_ATTR_SIZE];
 	struct stat st;
 	char *host;
+	int qnr;
 
 	if (!path)
 		return NULL;
@@ -1260,6 +1286,18 @@ 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/qos_class", cxlmem_base);
+	if (sysfs_read_attr(ctx, path, buf) == 0) {
+		memdev->pmem_qos_class.qos = get_qos_class(ctx, buf, &qnr);
+		memdev->pmem_qos_class.nr = qnr;
+	}
+
+	sprintf(path, "%s/ram/qos_class", cxlmem_base);
+	if (sysfs_read_attr(ctx, path, buf) == 0) {
+		memdev->ram_qos_class.qos = get_qos_class(ctx, buf, &qnr);
+		memdev->ram_qos_class.nr = qnr;
+	}
+
 	sprintf(path, "%s/payload_max", cxlmem_base);
 	if (sysfs_read_attr(ctx, path, buf) < 0)
 		goto err_read;
@@ -1483,6 +1521,16 @@ CXL_EXPORT unsigned long long cxl_memdev_get_ram_size(struct cxl_memdev *memdev)
 	return memdev->ram_size;
 }
 
+CXL_EXPORT struct qos_class *cxl_memdev_get_pmem_qos_class(struct cxl_memdev *memdev)
+{
+	return &memdev->pmem_qos_class;
+}
+
+CXL_EXPORT struct qos_class *cxl_memdev_get_ram_qos_class(struct cxl_memdev *memdev)
+{
+	return &memdev->ram_qos_class;
+}
+
 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 384fea2c25e3..465c78dc6c70 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -283,4 +283,6 @@ LIBCXL_8 {
 global:
 	cxl_memdev_wait_sanitize;
 	cxl_root_decoder_get_qos_class;
+	cxl_memdev_get_pmem_qos_class;
+	cxl_memdev_get_ram_qos_class;
 } LIBCXL_7;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index 4847ff448f71..1fe3654bc7ff 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -47,6 +47,8 @@ struct cxl_memdev {
 	struct list_node list;
 	unsigned long long pmem_size;
 	unsigned long long ram_size;
+	struct qos_class ram_qos_class;
+	struct qos_class pmem_qos_class;
 	int payload_max;
 	size_t lsa_size;
 	struct kmod_module *module;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index e5c08da77f77..84d1683f234c 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -58,6 +58,11 @@ static inline enum cxl_fwl_status cxl_fwl_status_from_ident(char *status)
 	return CXL_FWL_STATUS_UNKNOWN;
 }
 
+struct qos_class {
+	int nr;
+	int *qos;
+};
+
 struct cxl_memdev;
 struct cxl_memdev *cxl_memdev_get_first(struct cxl_ctx *ctx);
 struct cxl_memdev *cxl_memdev_get_next(struct cxl_memdev *memdev);
@@ -72,6 +77,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);
+struct qos_class *cxl_memdev_get_pmem_qos_class(struct cxl_memdev *memdev);
+struct qos_class *cxl_memdev_get_ram_qos_class(struct cxl_memdev *memdev);
 const char *cxl_memdev_get_firmware_verison(struct cxl_memdev *memdev);
 bool cxl_memdev_fw_update_in_progress(struct cxl_memdev *memdev);
 size_t cxl_memdev_fw_update_get_remaining(struct cxl_memdev *memdev);

[NDCTL PATCH v3 3/3] ndctl: cxl: add QoS class check for CXL region creation

[Dave Jiang]

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

One or more QoS class tokens are 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, QoS class tokens  are 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
QoS class token from the root decoder is different than the mem device QoS
class token. User parameter options are provided to fail instead of just
warning.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v3:
- Rebase to pending branch
---
 Documentation/cxl/cxl-create-region.txt |    9 ++++
 cxl/region.c                            |   67 +++++++++++++++++++++++++++++++
 2 files changed, 75 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 3a762db4800e..00dd63e160df 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -32,6 +32,8 @@ static struct region_params {
 	bool force;
 	bool human;
 	bool debug;
+	bool strict;
+	bool no_qtg;
 } param = {
 	.ways = INT_MAX,
 	.granularity = INT_MAX,
@@ -49,6 +51,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 {
@@ -81,7 +85,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(),
@@ -360,6 +366,9 @@ static int parse_create_options(struct cxl_ctx *ctx, int count,
 		}
 	}
 
+	p->strict = param.strict;
+	p->no_qtg = param.no_qtg;
+
 	return 0;
 
 err:
@@ -467,6 +476,60 @@ static void set_type_from_decoder(struct cxl_ctx *ctx, struct parsed_params *p)
 		p->mode = CXL_DECODER_MODE_PMEM;
 }
 
+static bool region_qos_match_decoder(struct qos_class *qos_class, int decoder_qc)
+{
+	int i;
+
+	for (i = 0; i < qos_class->nr; i++) {
+		if (qos_class->qos[i] == decoder_qc)
+			return true;
+	}
+
+	return false;
+}
+
+static int create_region_validate_qtg_id(struct cxl_ctx *ctx,
+					 struct parsed_params *p)
+{
+	struct qos_class *qos_class;
+	int root_qos_class, i;
+
+	root_qos_class = cxl_root_decoder_get_qos_class(p->root_decoder);
+	if (root_qos_class == CXL_QOS_CLASS_NONE)
+		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)
+			qos_class = cxl_memdev_get_ram_qos_class(memdev);
+		else
+			qos_class = cxl_memdev_get_pmem_qos_class(memdev);
+
+		/* No qos_class entries. Possibly no kernel support */
+		if (qos_class->nr == 0)
+			break;
+
+		if (!region_qos_match_decoder(qos_class, root_qos_class)) {
+			if (p->strict && !p->no_qtg) {
+				log_err(&rl, "%s QoS Class mismatches %s\n",
+					cxl_decoder_get_devname(p->root_decoder),
+					cxl_memdev_get_devname(memdev));
+
+				return -ENXIO;
+			}
+
+			log_notice(&rl, "%s QoS Class mismatches %s\n",
+				   cxl_decoder_get_devname(p->root_decoder),
+				   cxl_memdev_get_devname(memdev));
+		}
+	}
+
+	return 0;
+}
+
 static int create_region_validate_config(struct cxl_ctx *ctx,
 					 struct parsed_params *p)
 {
@@ -507,6 +570,8 @@ found:
 		return rc;
 
 	collect_minsize(ctx, p);
+	create_region_validate_qtg_id(ctx, p);
+
 	return 0;
 }
 

RE: [NDCTL PATCH v3 0/3] ndctl: Add support of qos_class for CXL CLI

[Dan Williams]

Dave Jiang wrote:
> Hi Vishal,
> With the QoS class series merged to the v6.8 kernel, can you please review and
> apply this series to ndctl if acceptable?
> 
> v3:
> - Rebase against latest ndctl/pending branch.
> 
> The series adds support for the kernel enabling of QoS class in the v6.8
> kernel. The kernel exports a qos_class token for the root decoders (CFMWS) and as
> well as for the CXL memory devices. The qos_class exported for a device is
> calculated by the driver during device probe. Currently a qos_class is exported
> for the volatile partition (ram) and another for the persistent partition (pmem).
> In the future qos_class will be exported for DCD regions. Display of qos_class is
> through the CXL CLI list command with -vvv for extra verbose.
> 
> A qos_class check as also been added for region creation. A warning is emitted
> when the qos_class of a memory range of a CXL memory device being included in
> the CXL region assembly does not match the qos_class 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.
> 
> ---
> 
> Dave Jiang (3):
>       ndctl: cxl: Add QoS class retrieval for the root decoder
>       ndctl: cxl: Add QoS class support for the memory device
>       ndctl: cxl: add QoS class check for CXL region creation
> 
> 
>  Documentation/cxl/cxl-create-region.txt |  9 ++++
>  cxl/filter.h                            |  4 ++
>  cxl/json.c                              | 46 ++++++++++++++++-
>  cxl/lib/libcxl.c                        | 62 +++++++++++++++++++++++
>  cxl/lib/libcxl.sym                      |  3 ++
>  cxl/lib/private.h                       |  3 ++
>  cxl/libcxl.h                            | 10 ++++
>  cxl/list.c                              |  1 +
>  cxl/region.c                            | 67 ++++++++++++++++++++++++-
>  util/json.h                             |  1 +
>  10 files changed, 204 insertions(+), 2 deletions(-)

This needs changes to test/cxl-topology.sh to validate that the
qos_class file pops in the right place per and has prepopulated values
per cxl_test expectation.

Re: [NDCTL PATCH v3 0/3] ndctl: Add support of qos_class for CXL CLI

[Dave Jiang]

On 1/25/24 14:16, Dan Williams wrote:
> Dave Jiang wrote:
>> Hi Vishal,
>> With the QoS class series merged to the v6.8 kernel, can you please review and
>> apply this series to ndctl if acceptable?
>>
>> v3:
>> - Rebase against latest ndctl/pending branch.
>>
>> The series adds support for the kernel enabling of QoS class in the v6.8
>> kernel. The kernel exports a qos_class token for the root decoders (CFMWS) and as
>> well as for the CXL memory devices. The qos_class exported for a device is
>> calculated by the driver during device probe. Currently a qos_class is exported
>> for the volatile partition (ram) and another for the persistent partition (pmem).
>> In the future qos_class will be exported for DCD regions. Display of qos_class is
>> through the CXL CLI list command with -vvv for extra verbose.
>>
>> A qos_class check as also been added for region creation. A warning is emitted
>> when the qos_class of a memory range of a CXL memory device being included in
>> the CXL region assembly does not match the qos_class 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.
>>
>> ---
>>
>> Dave Jiang (3):
>>       ndctl: cxl: Add QoS class retrieval for the root decoder
>>       ndctl: cxl: Add QoS class support for the memory device
>>       ndctl: cxl: add QoS class check for CXL region creation
>>
>>
>>  Documentation/cxl/cxl-create-region.txt |  9 ++++
>>  cxl/filter.h                            |  4 ++
>>  cxl/json.c                              | 46 ++++++++++++++++-
>>  cxl/lib/libcxl.c                        | 62 +++++++++++++++++++++++
>>  cxl/lib/libcxl.sym                      |  3 ++
>>  cxl/lib/private.h                       |  3 ++
>>  cxl/libcxl.h                            | 10 ++++
>>  cxl/list.c                              |  1 +
>>  cxl/region.c                            | 67 ++++++++++++++++++++++++-
>>  util/json.h                             |  1 +
>>  10 files changed, 204 insertions(+), 2 deletions(-)
> 
> This needs changes to test/cxl-topology.sh to validate that the
> qos_class file pops in the right place per and has prepopulated values
> per cxl_test expectation.

Do we need to plumb cxl_test to support qos_class with mock functions? Currently cxl_test does not support qos_class under memdev due it not support CDAT, HMAT/SRAT, and any of the PCIe bandwidth/latency attributes. It only has root decoder qos_class exposed. 

Re: [NDCTL PATCH v3 0/3] ndctl: Add support of qos_class for CXL CLI

[Dan Williams]

Dave Jiang wrote:
[..]
> > This needs changes to test/cxl-topology.sh to validate that the
> > qos_class file pops in the right place per and has prepopulated values
> > per cxl_test expectation.
> 
> Do we need to plumb cxl_test to support qos_class with mock functions?
> Currently cxl_test does not support qos_class under memdev due it not support
> CDAT, HMAT/SRAT, and any of the PCIe bandwidth/latency attributes. It only
> has root decoder qos_class exposed. 

Something is need to avoid basic failures like "attribute published at
wrong path", but it does not need to go through full CDAT emulation.

I would do something like the below where ops->cxl_endpoint_parse_cdat()
does not need to actually read any real HMAT or CDAT. All it needs to do
is fake a final cxl_dpa_perf result(s) into the right places to make the
sysfs attribute show up.

Then we can do driver shutdown and access testing with the attribute
code to get lockdep coverage and other basic checkout.

diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index caff3834671f..030b388800f0 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -13,6 +13,7 @@ ldflags-y += --wrap=cxl_hdm_decode_init
 ldflags-y += --wrap=cxl_dvsec_rr_decode
 ldflags-y += --wrap=devm_cxl_add_rch_dport
 ldflags-y += --wrap=cxl_rcd_component_reg_phys
+ldflags-y += --wrap=cxl_endpoint_parse_cdat
 
 DRIVERS := ../../../drivers
 CXL_SRC := $(DRIVERS)/cxl
diff --git a/tools/testing/cxl/test/mock.c b/tools/testing/cxl/test/mock.c
index 1a61e68e3095..9133cae1f5c1 100644
--- a/tools/testing/cxl/test/mock.c
+++ b/tools/testing/cxl/test/mock.c
@@ -285,6 +285,19 @@ resource_size_t __wrap_cxl_rcd_component_reg_phys(struct device *dev,
 }
 EXPORT_SYMBOL_NS_GPL(__wrap_cxl_rcd_component_reg_phys, CXL);
 
+void __wrap_cxl_endpoint_parse_cdat(struct cxl_port *port)
+{
+       int index;
+       struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+       struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
+
+       if (ops && ops->is_mock_dev(cxlmd->dev.parent))
+               ops->cxl_endpoint_parse_cdat(port);
+       else
+               cxl_endpoint_parse_cdat(port);
+       put_cxl_mock_ops(index);
+}
+
 MODULE_LICENSE("GPL v2");
 MODULE_IMPORT_NS(ACPI);
 MODULE_IMPORT_NS(CXL);

Re: [NDCTL PATCH v3 1/3] ndctl: cxl: Add QoS class retrieval for the root decoder

[Alison Schofield]

On Wed, Jan 24, 2024 at 01:54:41PM -0700, Dave Jiang wrote:
> Add libcxl API to retrieve the QoS class for the root decoder. Also add
> support to display the QoS class for the root decoder through the 'cxl
> list' command. The qos_class is displayed behind -vvv verbose level.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Alison Schofield <alison.schofield@intel.com>

> 
> ---
> v3:
> - Rebase to latest pending branch
> ---
>  cxl/filter.h       |    4 ++++
>  cxl/json.c         |   10 ++++++++++
>  cxl/lib/libcxl.c   |   14 ++++++++++++++
>  cxl/lib/libcxl.sym |    1 +
>  cxl/lib/private.h  |    1 +
>  cxl/libcxl.h       |    3 +++
>  cxl/list.c         |    1 +
>  util/json.h        |    1 +
>  8 files changed, 35 insertions(+)
> 
> diff --git a/cxl/filter.h b/cxl/filter.h
> index 1241f72ccf62..3c5f9e8a0452 100644
> --- a/cxl/filter.h
> +++ b/cxl/filter.h
> @@ -31,6 +31,7 @@ struct cxl_filter_params {
>  	bool alert_config;
>  	bool dax;
>  	bool poison;
> +	bool qos;
>  	int verbose;
>  	struct log_ctx ctx;
>  };
> @@ -91,6 +92,9 @@ static inline unsigned long cxl_filter_to_flags(struct cxl_filter_params *param)
>  		flags |= UTIL_JSON_DAX | UTIL_JSON_DAX_DEVS;
>  	if (param->poison)
>  		flags |= UTIL_JSON_MEDIA_ERRORS;
> +	if (param->qos)
> +		flags |= UTIL_JSON_QOS_CLASS;
> +
>  	return flags;
>  }
>  
> diff --git a/cxl/json.c b/cxl/json.c
> index 6fb17582a1cb..48a43ddf14b0 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -1062,6 +1062,16 @@ struct json_object *util_cxl_decoder_to_json(struct cxl_decoder *decoder,
>  					       jobj);
>  	}
>  
> +	if ((flags & UTIL_JSON_QOS_CLASS) && cxl_port_is_root(port)) {
> +		int qos_class = cxl_root_decoder_get_qos_class(decoder);
> +
> +		if (qos_class != CXL_QOS_CLASS_NONE) {
> +			jobj = json_object_new_int(qos_class);
> +			if (jobj)
> +				json_object_object_add(jdecoder, "qos_class", jobj);
> +		}
> +	}
> +
>  	json_object_set_userdata(jdecoder, decoder, NULL);
>  	return jdecoder;
>  }
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 1537a33d370e..9a1ac7001803 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -2229,6 +2229,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/qos_class", cxldecoder_base);
> +	if (sysfs_read_attr(ctx, path, buf) < 0)
> +		decoder->qos_class = CXL_QOS_CLASS_NONE;
> +	else
> +		decoder->qos_class = atoi(buf);
> +
>  	switch (port->type) {
>  	case CXL_PORT_ENDPOINT:
>  		sprintf(path, "%s/dpa_resource", cxldecoder_base);
> @@ -2423,6 +2429,14 @@ CXL_EXPORT unsigned long long cxl_decoder_get_size(struct cxl_decoder *decoder)
>  	return decoder->size;
>  }
>  
> +CXL_EXPORT int cxl_root_decoder_get_qos_class(struct cxl_decoder *decoder)
> +{
> +	if (!cxl_port_is_root(decoder->port))
> +		return -EINVAL;
> +
> +	return decoder->qos_class;
> +}
> +
>  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 2149f84d764e..384fea2c25e3 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -282,4 +282,5 @@ global:
>  LIBCXL_8 {
>  global:
>  	cxl_memdev_wait_sanitize;
> +	cxl_root_decoder_get_qos_class;
>  } LIBCXL_7;
> diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> index b26a8629e047..4847ff448f71 100644
> --- a/cxl/lib/private.h
> +++ b/cxl/lib/private.h
> @@ -144,6 +144,7 @@ struct cxl_decoder {
>  	struct list_head targets;
>  	struct list_head regions;
>  	struct list_head stale_regions;
> +	int qos_class;
>  };
>  
>  enum cxl_decode_state {
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index 352b3a866f63..e5c08da77f77 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -173,6 +173,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_QOS_CLASS_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);
> @@ -184,6 +186,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_root_decoder_get_qos_class(struct cxl_decoder *decoder);
>  
>  enum cxl_decoder_mode {
>  	CXL_DECODER_MODE_NONE,
> diff --git a/cxl/list.c b/cxl/list.c
> index 13fef8569340..f6446f98c2bd 100644
> --- a/cxl/list.c
> +++ b/cxl/list.c
> @@ -123,6 +123,7 @@ int cmd_list(int argc, const char **argv, struct cxl_ctx *ctx)
>  		param.fw = true;
>  		param.alert_config = true;
>  		param.dax = true;
> +		param.qos = true;
>  		/* fallthrough */
>  	case 2:
>  		param.idle = true;
> diff --git a/util/json.h b/util/json.h
> index ea370df4d1b7..b07055005084 100644
> --- a/util/json.h
> +++ b/util/json.h
> @@ -21,6 +21,7 @@ enum util_json_flags {
>  	UTIL_JSON_TARGETS	= (1 << 11),
>  	UTIL_JSON_PARTITION	= (1 << 12),
>  	UTIL_JSON_ALERT_CONFIG	= (1 << 13),
> +	UTIL_JSON_QOS_CLASS	= (1 << 14),
>  };
>  
>  void util_display_json_array(FILE *f_out, struct json_object *jarray,
> 
> 
> 

Re: [NDCTL PATCH v3 2/3] ndctl: cxl: Add QoS class support for the memory device

[Alison Schofield]

On Wed, Jan 24, 2024 at 01:54:47PM -0700, Dave Jiang wrote:
> Add libcxl API to retrieve the QoS class tokens 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 QoS class tokens through the 'cxl list' command.
> There can be 1 or more QoS class tokens for the memory device if
> they are valid. The qos_class tokens are displayed behind -vvv
> verbose level.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

One tidbit below. 

Reviewed-by: Alison Schofield <alison.schofield@intel.com>


> ---
> v3:
> - Rebase to pending branch
> - Skip from failing if no qos_class sysfs attrib found
> ---
>  cxl/json.c         |   36 +++++++++++++++++++++++++++++++++++-
>  cxl/lib/libcxl.c   |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  cxl/lib/libcxl.sym |    2 ++
>  cxl/lib/private.h  |    2 ++
>  cxl/libcxl.h       |    7 +++++++
>  5 files changed, 94 insertions(+), 1 deletion(-)
> 
> diff --git a/cxl/json.c b/cxl/json.c
> index 48a43ddf14b0..dcbac8c14f03 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -770,12 +770,32 @@ err_free:
>  	return jpoison;
>  }
>  
> +static struct json_object *get_qos_json_object(struct json_object *jdev,
> +					       struct qos_class *qos_class)
> +{
> +	struct json_object *jqos_array = json_object_new_array();
> +	struct json_object *jobj;
> +	int i;
> +
> +	if (!jqos_array)
> +		return NULL;
> +
> +	for (i = 0; i < qos_class->nr; i++) {
> +		jobj = json_object_new_int(qos_class->qos[i]);
> +		if (jobj)
> +			json_object_array_add(jqos_array, jobj);
> +	}
> +
> +	return jqos_array;
> +}
> +
>  struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>  		unsigned long flags)
>  {
>  	const char *devname = cxl_memdev_get_devname(memdev);
> -	struct json_object *jdev, *jobj;
> +	struct json_object *jdev, *jobj, *jqos;

Can the generic *jobj be used below rather than adding the new *jqos?


>  	unsigned long long serial, size;
> +	struct qos_class *qos_class;
>  	int numa_node;
>  
>  	jdev = json_object_new_object();
> @@ -791,6 +811,13 @@ 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);
> +
> +		if (flags & UTIL_JSON_QOS_CLASS) {
> +			qos_class = cxl_memdev_get_pmem_qos_class(memdev);
> +			jqos = get_qos_json_object(jdev, qos_class);
> +			if (jqos)
> +				json_object_object_add(jdev, "pmem_qos_class", jqos);
> +		}
>  	}
>  
>  	size = cxl_memdev_get_ram_size(memdev);
> @@ -798,6 +825,13 @@ 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);
> +
> +		if (flags & UTIL_JSON_QOS_CLASS) {
> +			qos_class = cxl_memdev_get_ram_qos_class(memdev);
> +			jqos = get_qos_json_object(jdev, qos_class);
> +			if (jqos)
> +				json_object_object_add(jdev, "ram_qos_class", jqos);
> +		}
>  	}
>  
>  	if (flags & UTIL_JSON_HEALTH) {

snip

> 
> 

Re: [NDCTL PATCH v3 3/3] ndctl: cxl: add QoS class check for CXL region creation

[Alison Schofield]

On Wed, Jan 24, 2024 at 01:54:54PM -0700, Dave Jiang wrote:
> The CFMWS provides a QTG ID. The kernel driver creates a root decoder that
> represents the CFMWS. A qos_class attribute is exported via sysfs for the root
> decoder.
> 
> One or more QoS class tokens are 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, QoS class tokens  are 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

Maybe "Add a QoS check during region creation."

> QoS class token from the root decoder is different than the mem device QoS
> class token. User parameter options are provided to fail instead of just
> warning.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v3:
> - Rebase to pending branch
> ---
>  Documentation/cxl/cxl-create-region.txt |    9 ++++
>  cxl/region.c                            |   67 +++++++++++++++++++++++++++++++
>  2 files changed, 75 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.

The definition of this 'Enforce ...' sounds very broad like it's
going to be used for more that this QTG ID check. Is it?  Maybe I
missed some earlier reviews and that is intentional.

I was expecting it to say something like: Enforce strict QTG ID matching.
Fail to create region on any mismatch.


> +
> +-q::
> +--no-enforce-qtg::
> +	Parameter to bypass QTG ID mismatch failure. Will only emit warning.
> +
>  include::human-option.txt[]
>  
>  include::debug-option.txt[]


snip

> 

Re: [NDCTL PATCH v3 2/3] ndctl: cxl: Add QoS class support for the memory device

[Dave Jiang]

On 1/26/24 11:01, Alison Schofield wrote:
> On Wed, Jan 24, 2024 at 01:54:47PM -0700, Dave Jiang wrote:
>> Add libcxl API to retrieve the QoS class tokens 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 QoS class tokens through the 'cxl list' command.
>> There can be 1 or more QoS class tokens for the memory device if
>> they are valid. The qos_class tokens are displayed behind -vvv
>> verbose level.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> One tidbit below. 
> 
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> 
> 
>> ---
>> v3:
>> - Rebase to pending branch
>> - Skip from failing if no qos_class sysfs attrib found
>> ---
>>  cxl/json.c         |   36 +++++++++++++++++++++++++++++++++++-
>>  cxl/lib/libcxl.c   |   48 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  cxl/lib/libcxl.sym |    2 ++
>>  cxl/lib/private.h  |    2 ++
>>  cxl/libcxl.h       |    7 +++++++
>>  5 files changed, 94 insertions(+), 1 deletion(-)
>>
>> diff --git a/cxl/json.c b/cxl/json.c
>> index 48a43ddf14b0..dcbac8c14f03 100644
>> --- a/cxl/json.c
>> +++ b/cxl/json.c
>> @@ -770,12 +770,32 @@ err_free:
>>  	return jpoison;
>>  }
>>  
>> +static struct json_object *get_qos_json_object(struct json_object *jdev,
>> +					       struct qos_class *qos_class)
>> +{
>> +	struct json_object *jqos_array = json_object_new_array();
>> +	struct json_object *jobj;
>> +	int i;
>> +
>> +	if (!jqos_array)
>> +		return NULL;
>> +
>> +	for (i = 0; i < qos_class->nr; i++) {
>> +		jobj = json_object_new_int(qos_class->qos[i]);
>> +		if (jobj)
>> +			json_object_array_add(jqos_array, jobj);
>> +	}
>> +
>> +	return jqos_array;
>> +}
>> +
>>  struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
>>  		unsigned long flags)
>>  {
>>  	const char *devname = cxl_memdev_get_devname(memdev);
>> -	struct json_object *jdev, *jobj;
>> +	struct json_object *jdev, *jobj, *jqos;
> 
> Can the generic *jobj be used below rather than adding the new *jqos?

With the kernel code change, this code is simplified and jobj will be used now.

DJ

> 
> 
>>  	unsigned long long serial, size;
>> +	struct qos_class *qos_class;
>>  	int numa_node;
>>  
>>  	jdev = json_object_new_object();
>> @@ -791,6 +811,13 @@ 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);
>> +
>> +		if (flags & UTIL_JSON_QOS_CLASS) {
>> +			qos_class = cxl_memdev_get_pmem_qos_class(memdev);
>> +			jqos = get_qos_json_object(jdev, qos_class);
>> +			if (jqos)
>> +				json_object_object_add(jdev, "pmem_qos_class", jqos);
>> +		}
>>  	}
>>  
>>  	size = cxl_memdev_get_ram_size(memdev);
>> @@ -798,6 +825,13 @@ 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);
>> +
>> +		if (flags & UTIL_JSON_QOS_CLASS) {
>> +			qos_class = cxl_memdev_get_ram_qos_class(memdev);
>> +			jqos = get_qos_json_object(jdev, qos_class);
>> +			if (jqos)
>> +				json_object_object_add(jdev, "ram_qos_class", jqos);
>> +		}
>>  	}
>>  
>>  	if (flags & UTIL_JSON_HEALTH) {
> 
> snip
> 
>>
>>

Re: [NDCTL PATCH v3 3/3] ndctl: cxl: add QoS class check for CXL region creation

[Dave Jiang]

On 1/26/24 11:14, Alison Schofield wrote:
> On Wed, Jan 24, 2024 at 01:54:54PM -0700, Dave Jiang wrote:
>> The CFMWS provides a QTG ID. The kernel driver creates a root decoder that
>> represents the CFMWS. A qos_class attribute is exported via sysfs for the root
>> decoder.
>>
>> One or more QoS class tokens are 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, QoS class tokens  are 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
> 
> Maybe "Add a QoS check during region creation."
> 
>> QoS class token from the root decoder is different than the mem device QoS
>> class token. User parameter options are provided to fail instead of just
>> warning.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> v3:
>> - Rebase to pending branch
>> ---
>>  Documentation/cxl/cxl-create-region.txt |    9 ++++
>>  cxl/region.c                            |   67 +++++++++++++++++++++++++++++++
>>  2 files changed, 75 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.
> 
> The definition of this 'Enforce ...' sounds very broad like it's
> going to be used for more that this QTG ID check. Is it?  Maybe I
> missed some earlier reviews and that is intentional.

I left it intentionally broad to allow it be used in other strict checking in the future and not exclusively to qos_class. And then the -q parameter below is used to override for qos_class. Speaking of which, I should fixup the documentation to use qos_class rather than QTG ID. 

> 
> I was expecting it to say something like: Enforce strict QTG ID matching.
> Fail to create region on any mismatch.
> 
> 
>> +
>> +-q::
>> +--no-enforce-qtg::
>> +	Parameter to bypass QTG ID mismatch failure. Will only emit warning.
>> +
>>  include::human-option.txt[]
>>  
>>  include::debug-option.txt[]
> 
> 
> snip
> 
>>