* [NDCTL PATCH v7 1/4] ndctl: cxl: Add QoS class retrieval for the root decoder
2024-02-08 20:11 [NDCTL PATCH v7 4/4] ndctl: Add support of qos_class for CXL CLI Dave Jiang
@ 2024-02-08 20:11 ` Dave Jiang
2024-02-08 20:11 ` [NDCTL PATCH v7 2/4] ndctl: cxl: Add QoS class support for the memory device Dave Jiang
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Dave Jiang @ 2024-02-08 20:11 UTC (permalink / raw)
To: linux-cxl, nvdimm; +Cc: vishal.l.verma, Alison Schofield
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 the QTG ID of the CFMWS window that
represents the root decoder.
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v7:
- Remove QOS flag entirely. (Alison)
---
cxl/json.c | 10 ++++++++++
cxl/lib/libcxl.c | 14 ++++++++++++++
cxl/lib/libcxl.sym | 1 +
cxl/lib/private.h | 1 +
cxl/libcxl.h | 3 +++
5 files changed, 29 insertions(+)
diff --git a/cxl/json.c b/cxl/json.c
index 6fb17582a1cb..1d380e23d6ff 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 (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,
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [NDCTL PATCH v7 2/4] ndctl: cxl: Add QoS class support for the memory device
2024-02-08 20:11 [NDCTL PATCH v7 4/4] ndctl: Add support of qos_class for CXL CLI Dave Jiang
2024-02-08 20:11 ` [NDCTL PATCH v7 1/4] ndctl: cxl: Add QoS class retrieval for the root decoder Dave Jiang
@ 2024-02-08 20:11 ` Dave Jiang
2024-02-08 20:11 ` [NDCTL PATCH v7 3/4] ndctl: cxl: add QoS class check for CXL region creation Dave Jiang
2024-02-08 20:11 ` [NDCTL PATCH v7 4/4] ndctl: add test for qos_class in CXL test suite Dave Jiang
3 siblings, 0 replies; 11+ messages in thread
From: Dave Jiang @ 2024-02-08 20:11 UTC (permalink / raw)
To: linux-cxl, nvdimm; +Cc: vishal.l.verma, Alison Schofield
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.
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v7: Add check for invalid qos_class for json
---
cxl/json.c | 15 +++++++++++++++
cxl/lib/libcxl.c | 22 ++++++++++++++++++++++
cxl/lib/libcxl.sym | 2 ++
cxl/lib/private.h | 2 ++
cxl/libcxl.h | 2 ++
5 files changed, 43 insertions(+)
diff --git a/cxl/json.c b/cxl/json.c
index 1d380e23d6ff..c8bd8c27447a 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -777,6 +777,7 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
struct json_object *jdev, *jobj;
unsigned long long serial, size;
int numa_node;
+ int qos_class;
jdev = json_object_new_object();
if (!jdev)
@@ -791,6 +792,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);
+
+ qos_class = cxl_memdev_get_pmem_qos_class(memdev);
+ if (qos_class != CXL_QOS_CLASS_NONE) {
+ jobj = json_object_new_int(qos_class);
+ if (jobj)
+ json_object_object_add(jdev, "pmem_qos_class", jobj);
+ }
}
size = cxl_memdev_get_ram_size(memdev);
@@ -798,6 +806,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);
+
+ qos_class = cxl_memdev_get_ram_qos_class(memdev);
+ if (qos_class != CXL_QOS_CLASS_NONE) {
+ jobj = json_object_new_int(qos_class);
+ if (jobj)
+ json_object_object_add(jdev, "ram_qos_class", jobj);
+ }
}
if (flags & UTIL_JSON_HEALTH) {
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 9a1ac7001803..6c293f1dfc91 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -1260,6 +1260,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 = CXL_QOS_CLASS_NONE;
+ else
+ memdev->pmem_qos_class = atoi(buf);
+
+ sprintf(path, "%s/ram/qos_class", cxlmem_base);
+ if (sysfs_read_attr(ctx, path, buf) < 0)
+ memdev->ram_qos_class = CXL_QOS_CLASS_NONE;
+ else
+ memdev->ram_qos_class = atoi(buf);
+
sprintf(path, "%s/payload_max", cxlmem_base);
if (sysfs_read_attr(ctx, path, buf) < 0)
goto err_read;
@@ -1483,6 +1495,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_qos_class(struct cxl_memdev *memdev)
+{
+ return memdev->pmem_qos_class;
+}
+
+CXL_EXPORT int 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..07dc8c784f1d 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;
+ int ram_qos_class;
+ int 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..a180f01cb05e 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -72,6 +72,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_qos_class(struct cxl_memdev *memdev);
+int 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);
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [NDCTL PATCH v7 3/4] ndctl: cxl: add QoS class check for CXL region creation
2024-02-08 20:11 [NDCTL PATCH v7 4/4] ndctl: Add support of qos_class for CXL CLI Dave Jiang
2024-02-08 20:11 ` [NDCTL PATCH v7 1/4] ndctl: cxl: Add QoS class retrieval for the root decoder Dave Jiang
2024-02-08 20:11 ` [NDCTL PATCH v7 2/4] ndctl: cxl: Add QoS class support for the memory device Dave Jiang
@ 2024-02-08 20:11 ` Dave Jiang
2024-02-23 22:48 ` Verma, Vishal L
2024-02-08 20:11 ` [NDCTL PATCH v7 4/4] ndctl: add test for qos_class in CXL test suite Dave Jiang
3 siblings, 1 reply; 11+ messages in thread
From: Dave Jiang @ 2024-02-08 20:11 UTC (permalink / raw)
To: linux-cxl, nvdimm; +Cc: vishal.l.verma, Alison Schofield
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 QoS check during region creation. If --enforce-qos/-Q is set and
the qos_class mismatches, the region creation will fail.
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v7:
- Add qos_class_mismatched to region for cxl list (Vishal)
- Add create_region -Q check (Vishal)
---
Documentation/cxl/cxl-create-region.txt | 6 +++
cxl/json.c | 6 +++
cxl/lib/libcxl.c | 11 +++++
cxl/lib/libcxl.sym | 2 +
cxl/lib/private.h | 1 +
cxl/libcxl.h | 2 +
cxl/region.c | 56 ++++++++++++++++++++++++-
7 files changed, 83 insertions(+), 1 deletion(-)
diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
index f11a412bddfe..b244af60b8a6 100644
--- a/Documentation/cxl/cxl-create-region.txt
+++ b/Documentation/cxl/cxl-create-region.txt
@@ -105,6 +105,12 @@ include::bus-option.txt[]
supplied, the first cross-host bridge (if available), decoder that
supports the largest interleave will be chosen.
+-Q::
+--enforce-qos::
+ Parameter to enforce qos_class mismatch failure. Region create operation
+ will fail of the qos_class of the root decoder and one of the memdev that
+ backs the region mismatches.
+
include::human-option.txt[]
include::debug-option.txt[]
diff --git a/cxl/json.c b/cxl/json.c
index c8bd8c27447a..27cbacc84f3a 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -1238,6 +1238,12 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
}
}
+ if (cxl_region_qos_class_mismatched(region)) {
+ jobj = json_object_new_boolean(true);
+ if (jobj)
+ json_object_object_add(jregion, "qos_class_mismatched", jobj);
+ }
+
json_object_set_userdata(jregion, region, NULL);
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 6c293f1dfc91..3461c4de2097 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -414,6 +414,17 @@ CXL_EXPORT int cxl_region_is_enabled(struct cxl_region *region)
return is_enabled(path);
}
+CXL_EXPORT void cxl_region_qos_class_mismatched_set(struct cxl_region *region,
+ bool mismatched)
+{
+ region->qos_mismatched = mismatched;
+}
+
+CXL_EXPORT bool cxl_region_qos_class_mismatched(struct cxl_region *region)
+{
+ return region->qos_mismatched;
+}
+
CXL_EXPORT int cxl_region_disable(struct cxl_region *region)
{
const char *devname = cxl_region_get_devname(region);
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 465c78dc6c70..47a9c3cafc71 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -285,4 +285,6 @@ global:
cxl_root_decoder_get_qos_class;
cxl_memdev_get_pmem_qos_class;
cxl_memdev_get_ram_qos_class;
+ cxl_region_qos_class_mismatched_set;
+ cxl_region_qos_class_mismatched;
} LIBCXL_7;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index 07dc8c784f1d..88448d82d53f 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -174,6 +174,7 @@ struct cxl_region {
struct daxctl_region *dax_region;
struct kmod_module *module;
struct list_head mappings;
+ bool qos_mismatched;
};
struct cxl_memdev_mapping {
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index a180f01cb05e..7795496cdbbd 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -335,6 +335,8 @@ int cxl_region_clear_target(struct cxl_region *region, int position);
int cxl_region_clear_all_targets(struct cxl_region *region);
int cxl_region_decode_commit(struct cxl_region *region);
int cxl_region_decode_reset(struct cxl_region *region);
+void cxl_region_qos_class_mismatched_set(struct cxl_region *region, bool mismatched);
+bool cxl_region_qos_class_mismatched(struct cxl_region *region);
#define cxl_region_foreach(decoder, region) \
for (region = cxl_region_get_first(decoder); region != NULL; \
diff --git a/cxl/region.c b/cxl/region.c
index 3a762db4800e..76df177ef246 100644
--- a/cxl/region.c
+++ b/cxl/region.c
@@ -32,6 +32,7 @@ static struct region_params {
bool force;
bool human;
bool debug;
+ bool qos_enforce;
} param = {
.ways = INT_MAX,
.granularity = INT_MAX,
@@ -49,6 +50,8 @@ struct parsed_params {
const char **argv;
struct cxl_decoder *root_decoder;
enum cxl_decoder_mode mode;
+ bool qos_enforce;
+ bool qos_mismatched;
};
enum region_actions {
@@ -81,7 +84,8 @@ OPT_STRING('U', "uuid", ¶m.uuid, \
"region uuid", "uuid for the new region (default: autogenerate)"), \
OPT_BOOLEAN('m', "memdevs", ¶m.memdevs, \
"non-option arguments are memdevs"), \
-OPT_BOOLEAN('u', "human", ¶m.human, "use human friendly number formats")
+OPT_BOOLEAN('u', "human", ¶m.human, "use human friendly number formats"), \
+OPT_BOOLEAN('Q', "enforce-qos", ¶m.qos_enforce, "enforce of qos_class matching")
static const struct option create_options[] = {
BASE_OPTIONS(),
@@ -360,6 +364,8 @@ static int parse_create_options(struct cxl_ctx *ctx, int count,
}
}
+ p->qos_enforce = param.qos_enforce;
+
return 0;
err:
@@ -467,6 +473,49 @@ 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_qos_class(struct cxl_ctx *ctx,
+ struct parsed_params *p)
+{
+ int root_qos_class;
+ int qos_class;
+ int i;
+
+ if (!p->qos_enforce)
+ return 0;
+
+ 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 == CXL_QOS_CLASS_NONE)
+ break;
+
+ if (qos_class != root_qos_class) {
+ p->qos_mismatched = true;
+ if (p->qos_enforce) {
+ log_err(&rl, "%s QoS Class mismatches %s\n",
+ cxl_decoder_get_devname(p->root_decoder),
+ cxl_memdev_get_devname(memdev));
+
+ return -ENXIO;
+ }
+ }
+ }
+
+ return 0;
+}
+
static int create_region_validate_config(struct cxl_ctx *ctx,
struct parsed_params *p)
{
@@ -507,6 +556,10 @@ found:
return rc;
collect_minsize(ctx, p);
+ rc = create_region_validate_qos_class(ctx, p);
+ if (rc)
+ return rc;
+
return 0;
}
@@ -654,6 +707,7 @@ static int create_region(struct cxl_ctx *ctx, int *count,
return -EOPNOTSUPP;
}
+ cxl_region_qos_class_mismatched_set(region, p->qos_mismatched);
devname = cxl_region_get_devname(region);
rc = cxl_region_determine_granularity(region, p);
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [NDCTL PATCH v7 3/4] ndctl: cxl: add QoS class check for CXL region creation
2024-02-08 20:11 ` [NDCTL PATCH v7 3/4] ndctl: cxl: add QoS class check for CXL region creation Dave Jiang
@ 2024-02-23 22:48 ` Verma, Vishal L
2024-02-26 21:58 ` Dave Jiang
0 siblings, 1 reply; 11+ messages in thread
From: Verma, Vishal L @ 2024-02-23 22:48 UTC (permalink / raw)
To: Jiang, Dave, linux-cxl@vger.kernel.org, nvdimm@lists.linux.dev
Cc: Schofield, Alison
On Thu, 2024-02-08 at 13:11 -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 QoS check during region creation. If --enforce-qos/-Q is set and
> the qos_class mismatches, the region creation will fail.
>
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v7:
> - Add qos_class_mismatched to region for cxl list (Vishal)
> - Add create_region -Q check (Vishal)
> ---
> Documentation/cxl/cxl-create-region.txt | 6 +++
> cxl/json.c | 6 +++
> cxl/lib/libcxl.c | 11 +++++
> cxl/lib/libcxl.sym | 2 +
> cxl/lib/private.h | 1 +
> cxl/libcxl.h | 2 +
> cxl/region.c | 56 ++++++++++++++++++++++++-
> 7 files changed, 83 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
> index f11a412bddfe..b244af60b8a6 100644
> --- a/Documentation/cxl/cxl-create-region.txt
> +++ b/Documentation/cxl/cxl-create-region.txt
> @@ -105,6 +105,12 @@ include::bus-option.txt[]
> supplied, the first cross-host bridge (if available), decoder that
> supports the largest interleave will be chosen.
>
> +-Q::
> +--enforce-qos::
> + Parameter to enforce qos_class mismatch failure. Region create operation
> + will fail of the qos_class of the root decoder and one of the memdev that
> + backs the region mismatches.
> +
> include::human-option.txt[]
>
> include::debug-option.txt[]
> diff --git a/cxl/json.c b/cxl/json.c
> index c8bd8c27447a..27cbacc84f3a 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -1238,6 +1238,12 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
> }
> }
>
> + if (cxl_region_qos_class_mismatched(region)) {
> + jobj = json_object_new_boolean(true);
> + if (jobj)
> + json_object_object_add(jregion, "qos_class_mismatched", jobj);
> + }
> +
> json_object_set_userdata(jregion, region, NULL);
>
>
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 6c293f1dfc91..3461c4de2097 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -414,6 +414,17 @@ CXL_EXPORT int cxl_region_is_enabled(struct cxl_region *region)
> return is_enabled(path);
> }
>
> +CXL_EXPORT void cxl_region_qos_class_mismatched_set(struct cxl_region *region,
> + bool mismatched)
> +{
> + region->qos_mismatched = mismatched;
> +}
This should be called cxl_region_set_qos_class_mismatched() at a
minimum, but..
> +
> +CXL_EXPORT bool cxl_region_qos_class_mismatched(struct cxl_region *region)
> +{
> + return region->qos_mismatched;
> +}
.. I think libcxl always perform its own qos mismatch checking when
this is called and return appropriately, instead of relying on a user-
set flag.
Actually I don't see this interface getting called anywhere. Was there
a patch to cxl_region_to_json() that got dropped?
> +
> CXL_EXPORT int cxl_region_disable(struct cxl_region *region)
> {
> const char *devname = cxl_region_get_devname(region);
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 465c78dc6c70..47a9c3cafc71 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -285,4 +285,6 @@ global:
> cxl_root_decoder_get_qos_class;
> cxl_memdev_get_pmem_qos_class;
> cxl_memdev_get_ram_qos_class;
> + cxl_region_qos_class_mismatched_set;
> + cxl_region_qos_class_mismatched;
> } LIBCXL_7;
> diff --git a/cxl/lib/private.h b/cxl/lib/private.h
> index 07dc8c784f1d..88448d82d53f 100644
> --- a/cxl/lib/private.h
> +++ b/cxl/lib/private.h
> @@ -174,6 +174,7 @@ struct cxl_region {
> struct daxctl_region *dax_region;
> struct kmod_module *module;
> struct list_head mappings;
> + bool qos_mismatched;
> };
>
> struct cxl_memdev_mapping {
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index a180f01cb05e..7795496cdbbd 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -335,6 +335,8 @@ int cxl_region_clear_target(struct cxl_region *region, int position);
> int cxl_region_clear_all_targets(struct cxl_region *region);
> int cxl_region_decode_commit(struct cxl_region *region);
> int cxl_region_decode_reset(struct cxl_region *region);
> +void cxl_region_qos_class_mismatched_set(struct cxl_region *region, bool mismatched);
> +bool cxl_region_qos_class_mismatched(struct cxl_region *region);
>
> #define cxl_region_foreach(decoder, region) \
> for (region = cxl_region_get_first(decoder); region != NULL; \
> diff --git a/cxl/region.c b/cxl/region.c
> index 3a762db4800e..76df177ef246 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -32,6 +32,7 @@ static struct region_params {
> bool force;
> bool human;
> bool debug;
> + bool qos_enforce;
> } param = {
> .ways = INT_MAX,
> .granularity = INT_MAX,
> @@ -49,6 +50,8 @@ struct parsed_params {
> const char **argv;
> struct cxl_decoder *root_decoder;
> enum cxl_decoder_mode mode;
> + bool qos_enforce;
> + bool qos_mismatched;
> };
>
> enum region_actions {
> @@ -81,7 +84,8 @@ OPT_STRING('U', "uuid", ¶m.uuid, \
> "region uuid", "uuid for the new region (default: autogenerate)"), \
> OPT_BOOLEAN('m', "memdevs", ¶m.memdevs, \
> "non-option arguments are memdevs"), \
> -OPT_BOOLEAN('u', "human", ¶m.human, "use human friendly number formats")
> +OPT_BOOLEAN('u', "human", ¶m.human, "use human friendly number formats"), \
> +OPT_BOOLEAN('Q', "enforce-qos", ¶m.qos_enforce, "enforce of qos_class matching")
>
> static const struct option create_options[] = {
> BASE_OPTIONS(),
> @@ -360,6 +364,8 @@ static int parse_create_options(struct cxl_ctx *ctx, int count,
> }
> }
>
> + p->qos_enforce = param.qos_enforce;
> +
> return 0;
>
> err:
> @@ -467,6 +473,49 @@ 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_qos_class(struct cxl_ctx *ctx,
ctx is never used, can be removed.
> + struct parsed_params *p)
> +{
> + int root_qos_class;
> + int qos_class;
> + int i;
> +
> + if (!p->qos_enforce)
> + return 0;
> +
> + 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 == CXL_QOS_CLASS_NONE)
> + break;
> +
> + if (qos_class != root_qos_class) {
> + p->qos_mismatched = true;
> + if (p->qos_enforce) {
> + log_err(&rl, "%s QoS Class mismatches %s\n",
> + cxl_decoder_get_devname(p->root_decoder),
> + cxl_memdev_get_devname(memdev));
> +
> + return -ENXIO;
> + }
> + }
> + }
> +
> + return 0;
> +}
> +
> static int create_region_validate_config(struct cxl_ctx *ctx,
> struct parsed_params *p)
> {
> @@ -507,6 +556,10 @@ found:
> return rc;
>
> collect_minsize(ctx, p);
> + rc = create_region_validate_qos_class(ctx, p);
> + if (rc)
> + return rc;
> +
Maybe this call can be moved into the existing validate_decoder() check
since?
> return 0;
> }
>
> @@ -654,6 +707,7 @@ static int create_region(struct cxl_ctx *ctx, int *count,
> return -EOPNOTSUPP;
> }
>
> + cxl_region_qos_class_mismatched_set(region, p->qos_mismatched);
> devname = cxl_region_get_devname(region);
>
> rc = cxl_region_determine_granularity(region, p);
I think as a future enhancement, it might be nice to add
cxl_filter_walk() smarts to allow it to filter memdevs based on
qos_class. That way, when cxl create-region is called without any
memdev arguments (i.e. it is free to select memdevs), collect_memdevs()
can ask for memdevs that match the qos_class, and see if those can
satisfy the interleave requirements if --enforce-qos is used.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [NDCTL PATCH v7 3/4] ndctl: cxl: add QoS class check for CXL region creation
2024-02-23 22:48 ` Verma, Vishal L
@ 2024-02-26 21:58 ` Dave Jiang
0 siblings, 0 replies; 11+ messages in thread
From: Dave Jiang @ 2024-02-26 21:58 UTC (permalink / raw)
To: Verma, Vishal L, linux-cxl@vger.kernel.org,
nvdimm@lists.linux.dev
Cc: Schofield, Alison
On 2/23/24 3:48 PM, Verma, Vishal L wrote:
> On Thu, 2024-02-08 at 13:11 -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 QoS check during region creation. If --enforce-qos/-Q is set and
>> the qos_class mismatches, the region creation will fail.
>>
>> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> v7:
>> - Add qos_class_mismatched to region for cxl list (Vishal)
>> - Add create_region -Q check (Vishal)
>> ---
>> Documentation/cxl/cxl-create-region.txt | 6 +++
>> cxl/json.c | 6 +++
>> cxl/lib/libcxl.c | 11 +++++
>> cxl/lib/libcxl.sym | 2 +
>> cxl/lib/private.h | 1 +
>> cxl/libcxl.h | 2 +
>> cxl/region.c | 56 ++++++++++++++++++++++++-
>> 7 files changed, 83 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
>> index f11a412bddfe..b244af60b8a6 100644
>> --- a/Documentation/cxl/cxl-create-region.txt
>> +++ b/Documentation/cxl/cxl-create-region.txt
>> @@ -105,6 +105,12 @@ include::bus-option.txt[]
>> supplied, the first cross-host bridge (if available), decoder that
>> supports the largest interleave will be chosen.
>>
>> +-Q::
>> +--enforce-qos::
>> + Parameter to enforce qos_class mismatch failure. Region create operation
>> + will fail of the qos_class of the root decoder and one of the memdev that
>> + backs the region mismatches.
>> +
>> include::human-option.txt[]
>>
>> include::debug-option.txt[]
>> diff --git a/cxl/json.c b/cxl/json.c
>> index c8bd8c27447a..27cbacc84f3a 100644
>> --- a/cxl/json.c
>> +++ b/cxl/json.c
>> @@ -1238,6 +1238,12 @@ struct json_object *util_cxl_region_to_json(struct cxl_region *region,
>> }
>> }
>>
>> + if (cxl_region_qos_class_mismatched(region)) {
>> + jobj = json_object_new_boolean(true);
>> + if (jobj)
>> + json_object_object_add(jregion, "qos_class_mismatched", jobj);
>> + }
>> +
>> json_object_set_userdata(jregion, region, NULL);
>>
>>
>> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
>> index 6c293f1dfc91..3461c4de2097 100644
>> --- a/cxl/lib/libcxl.c
>> +++ b/cxl/lib/libcxl.c
>> @@ -414,6 +414,17 @@ CXL_EXPORT int cxl_region_is_enabled(struct cxl_region *region)
>> return is_enabled(path);
>> }
>>
>> +CXL_EXPORT void cxl_region_qos_class_mismatched_set(struct cxl_region *region,
>> + bool mismatched)
>> +{
>> + region->qos_mismatched = mismatched;
>> +}
>
> This should be called cxl_region_set_qos_class_mismatched() at a
> minimum, but..
>
>> +
>> +CXL_EXPORT bool cxl_region_qos_class_mismatched(struct cxl_region *region)
>> +{
>> + return region->qos_mismatched;
>> +}
>
> .. I think libcxl always perform its own qos mismatch checking when
> this is called and return appropriately, instead of relying on a user-
> set flag.
Ok. I'll add internal compare for libcxl instead and remove the flag.
>
> Actually I don't see this interface getting called anywhere. Was there
> a patch to cxl_region_to_json() that got dropped?
It's the first code chunk above.
>
>> +
>> CXL_EXPORT int cxl_region_disable(struct cxl_region *region)
>> {
>> const char *devname = cxl_region_get_devname(region);
>> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
>> index 465c78dc6c70..47a9c3cafc71 100644
>> --- a/cxl/lib/libcxl.sym
>> +++ b/cxl/lib/libcxl.sym
>> @@ -285,4 +285,6 @@ global:
>> cxl_root_decoder_get_qos_class;
>> cxl_memdev_get_pmem_qos_class;
>> cxl_memdev_get_ram_qos_class;
>> + cxl_region_qos_class_mismatched_set;
>> + cxl_region_qos_class_mismatched;
>> } LIBCXL_7;
>> diff --git a/cxl/lib/private.h b/cxl/lib/private.h
>> index 07dc8c784f1d..88448d82d53f 100644
>> --- a/cxl/lib/private.h
>> +++ b/cxl/lib/private.h
>> @@ -174,6 +174,7 @@ struct cxl_region {
>> struct daxctl_region *dax_region;
>> struct kmod_module *module;
>> struct list_head mappings;
>> + bool qos_mismatched;
>> };
>>
>> struct cxl_memdev_mapping {
>> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
>> index a180f01cb05e..7795496cdbbd 100644
>> --- a/cxl/libcxl.h
>> +++ b/cxl/libcxl.h
>> @@ -335,6 +335,8 @@ int cxl_region_clear_target(struct cxl_region *region, int position);
>> int cxl_region_clear_all_targets(struct cxl_region *region);
>> int cxl_region_decode_commit(struct cxl_region *region);
>> int cxl_region_decode_reset(struct cxl_region *region);
>> +void cxl_region_qos_class_mismatched_set(struct cxl_region *region, bool mismatched);
>> +bool cxl_region_qos_class_mismatched(struct cxl_region *region);
>>
>> #define cxl_region_foreach(decoder, region) \
>> for (region = cxl_region_get_first(decoder); region != NULL; \
>> diff --git a/cxl/region.c b/cxl/region.c
>> index 3a762db4800e..76df177ef246 100644
>> --- a/cxl/region.c
>> +++ b/cxl/region.c
>> @@ -32,6 +32,7 @@ static struct region_params {
>> bool force;
>> bool human;
>> bool debug;
>> + bool qos_enforce;
>> } param = {
>> .ways = INT_MAX,
>> .granularity = INT_MAX,
>> @@ -49,6 +50,8 @@ struct parsed_params {
>> const char **argv;
>> struct cxl_decoder *root_decoder;
>> enum cxl_decoder_mode mode;
>> + bool qos_enforce;
>> + bool qos_mismatched;
>> };
>>
>> enum region_actions {
>> @@ -81,7 +84,8 @@ OPT_STRING('U', "uuid", ¶m.uuid, \
>> "region uuid", "uuid for the new region (default: autogenerate)"), \
>> OPT_BOOLEAN('m', "memdevs", ¶m.memdevs, \
>> "non-option arguments are memdevs"), \
>> -OPT_BOOLEAN('u', "human", ¶m.human, "use human friendly number formats")
>> +OPT_BOOLEAN('u', "human", ¶m.human, "use human friendly number formats"), \
>> +OPT_BOOLEAN('Q', "enforce-qos", ¶m.qos_enforce, "enforce of qos_class matching")
>>
>> static const struct option create_options[] = {
>> BASE_OPTIONS(),
>> @@ -360,6 +364,8 @@ static int parse_create_options(struct cxl_ctx *ctx, int count,
>> }
>> }
>>
>> + p->qos_enforce = param.qos_enforce;
>> +
>> return 0;
>>
>> err:
>> @@ -467,6 +473,49 @@ 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_qos_class(struct cxl_ctx *ctx,
>
> ctx is never used, can be removed.
ok
>
>> + struct parsed_params *p)
>> +{
>> + int root_qos_class;
>> + int qos_class;
>> + int i;
>> +
>> + if (!p->qos_enforce)
>> + return 0;
>> +
>> + 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 == CXL_QOS_CLASS_NONE)
>> + break;
>> +
>> + if (qos_class != root_qos_class) {
>> + p->qos_mismatched = true;
>> + if (p->qos_enforce) {
>> + log_err(&rl, "%s QoS Class mismatches %s\n",
>> + cxl_decoder_get_devname(p->root_decoder),
>> + cxl_memdev_get_devname(memdev));
>> +
>> + return -ENXIO;
>> + }
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> static int create_region_validate_config(struct cxl_ctx *ctx,
>> struct parsed_params *p)
>> {
>> @@ -507,6 +556,10 @@ found:
>> return rc;
>>
>> collect_minsize(ctx, p);
>> + rc = create_region_validate_qos_class(ctx, p);
>> + if (rc)
>> + return rc;
>> +
>
> Maybe this call can be moved into the existing validate_decoder() check
> since?
ok
>
>> return 0;
>> }
>>
>> @@ -654,6 +707,7 @@ static int create_region(struct cxl_ctx *ctx, int *count,
>> return -EOPNOTSUPP;
>> }
>>
>> + cxl_region_qos_class_mismatched_set(region, p->qos_mismatched);
>> devname = cxl_region_get_devname(region);
>>
>> rc = cxl_region_determine_granularity(region, p);
>
> I think as a future enhancement, it might be nice to add
> cxl_filter_walk() smarts to allow it to filter memdevs based on
> qos_class. That way, when cxl create-region is called without any
> memdev arguments (i.e. it is free to select memdevs), collect_memdevs()
> can ask for memdevs that match the qos_class, and see if those can
> satisfy the interleave requirements if --enforce-qos is used.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [NDCTL PATCH v7 4/4] ndctl: add test for qos_class in CXL test suite
2024-02-08 20:11 [NDCTL PATCH v7 4/4] ndctl: Add support of qos_class for CXL CLI Dave Jiang
` (2 preceding siblings ...)
2024-02-08 20:11 ` [NDCTL PATCH v7 3/4] ndctl: cxl: add QoS class check for CXL region creation Dave Jiang
@ 2024-02-08 20:11 ` Dave Jiang
2024-02-22 7:59 ` Verma, Vishal L
2024-02-23 22:07 ` Verma, Vishal L
3 siblings, 2 replies; 11+ messages in thread
From: Dave Jiang @ 2024-02-08 20:11 UTC (permalink / raw)
To: linux-cxl, nvdimm; +Cc: vishal.l.verma
Add tests in cxl-qos-class.sh to verify qos_class are set with the fake
qos_class create by the kernel. Root decoders should have qos_class
attribute set. Memory devices should have ram_qos_class or pmem_qos_class
set depending on which partitions are valid.
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v7:
- Add create_region -Q testing (Vishal)
---
test/common | 4 ++
test/cxl-qos-class.sh | 102 ++++++++++++++++++++++++++++++++++++++++++
test/meson.build | 2 +
3 files changed, 108 insertions(+)
create mode 100755 test/cxl-qos-class.sh
diff --git a/test/common b/test/common
index f1023ef20f7e..5694820c7adc 100644
--- a/test/common
+++ b/test/common
@@ -150,3 +150,7 @@ check_dmesg()
grep -q "Call Trace" <<< $log && err $1
true
}
+
+
+# CXL COMMON
+TEST_QOS_CLASS=42
diff --git a/test/cxl-qos-class.sh b/test/cxl-qos-class.sh
new file mode 100755
index 000000000000..145df6134685
--- /dev/null
+++ b/test/cxl-qos-class.sh
@@ -0,0 +1,102 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (C) 2024 Intel Corporation. All rights reserved.
+
+. $(dirname $0)/common
+
+rc=77
+
+set -ex
+
+trap 'err $LINENO' ERR
+
+check_prereq "jq"
+
+modprobe -r cxl_test
+modprobe cxl_test
+rc=1
+
+check_qos_decoders () {
+ # check root decoders have expected fake qos_class
+ # also make sure the number of root decoders equal to the number
+ # with qos_class found
+ json=$($CXL list -b cxl_test -D -d root)
+ decoders=$(echo "$json" | jq length)
+ count=0
+ while read -r qos_class
+ do
+ ((qos_class == TEST_QOS_CLASS)) || err "$LINENO"
+ count=$((count+1))
+ done <<< "$(echo "$json" | jq -r '.[] | .qos_class')"
+
+ ((count == decoders)) || err "$LINENO";
+}
+
+check_qos_memdevs () {
+ # Check that memdevs that expose ram_qos_class or pmem_qos_class have
+ # expected fake value programmed.
+ json=$(cxl list -b cxl_test -M)
+ readarray -t lines < <(jq ".[] | .ram_size, .pmem_size, .ram_qos_class, .pmem_qos_class" <<<"$json")
+ for (( i = 0; i < ${#lines[@]}; i += 4 ))
+ do
+ ram_size=${lines[i]}
+ pmem_size=${lines[i+1]}
+ ram_qos_class=${lines[i+2]}
+ pmem_qos_class=${lines[i+3]}
+
+ if [[ "$ram_size" != null ]]
+ then
+ ((ram_qos_class == TEST_QOS_CLASS)) || err "$LINENO"
+ fi
+ if [[ "$pmem_size" != null ]]
+ then
+ ((pmem_qos_class == TEST_QOS_CLASS)) || err "$LINENO"
+ fi
+ done
+}
+
+# Based on cxl-create-region.sh create_single()
+destroy_regions()
+{
+ if [[ "$*" ]]; then
+ $CXL destroy-region -f -b cxl_test "$@"
+ else
+ $CXL destroy-region -f -b cxl_test all
+ fi
+}
+
+create_region_check_qos()
+{
+ # the 5th cxl_test decoder is expected to target a single-port
+ # host-bridge. Older cxl_test implementations may not define it,
+ # so skip the test in that case.
+ decoder=$($CXL list -b cxl_test -D -d root |
+ jq -r ".[4] |
+ select(.pmem_capable == true) |
+ select(.nr_targets == 1) |
+ .decoder")
+
+ if [[ ! $decoder ]]; then
+ echo "no single-port host-bridge decoder found, skipping"
+ return
+ fi
+
+ # Send create-region with -Q to enforce qos_class matching
+ region=$($CXL create-region -Q -d "$decoder" | jq -r ".region")
+ if [[ ! $region ]]; then
+ echo "failed to create single-port host-bridge region"
+ err "$LINENO"
+ fi
+
+ destroy_regions "$region"
+}
+
+check_qos_decoders
+
+check_qos_memdevs
+
+create_region_check_qos
+
+check_dmesg "$LINEO"
+
+modprobe -r cxl_test
diff --git a/test/meson.build b/test/meson.build
index 5eb35749a95b..4892df11119f 100644
--- a/test/meson.build
+++ b/test/meson.build
@@ -160,6 +160,7 @@ cxl_events = find_program('cxl-events.sh')
cxl_poison = find_program('cxl-poison.sh')
cxl_sanitize = find_program('cxl-sanitize.sh')
cxl_destroy_region = find_program('cxl-destroy-region.sh')
+cxl_qos_class = find_program('cxl-qos-class.sh')
tests = [
[ 'libndctl', libndctl, 'ndctl' ],
@@ -192,6 +193,7 @@ tests = [
[ 'cxl-poison.sh', cxl_poison, 'cxl' ],
[ 'cxl-sanitize.sh', cxl_sanitize, 'cxl' ],
[ 'cxl-destroy-region.sh', cxl_destroy_region, 'cxl' ],
+ [ 'cxl-qos-class.sh', cxl_qos_class, 'cxl' ],
]
if get_option('destructive').enabled()
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [NDCTL PATCH v7 4/4] ndctl: add test for qos_class in CXL test suite
2024-02-08 20:11 ` [NDCTL PATCH v7 4/4] ndctl: add test for qos_class in CXL test suite Dave Jiang
@ 2024-02-22 7:59 ` Verma, Vishal L
2024-02-26 22:39 ` Dave Jiang
2024-02-23 22:07 ` Verma, Vishal L
1 sibling, 1 reply; 11+ messages in thread
From: Verma, Vishal L @ 2024-02-22 7:59 UTC (permalink / raw)
To: Jiang, Dave, linux-cxl@vger.kernel.org, nvdimm@lists.linux.dev
On Thu, 2024-02-08 at 13:11 -0700, Dave Jiang wrote:
> Add tests in cxl-qos-class.sh to verify qos_class are set with the fake
> qos_class create by the kernel. Root decoders should have qos_class
> attribute set. Memory devices should have ram_qos_class or pmem_qos_class
> set depending on which partitions are valid.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v7:
> - Add create_region -Q testing (Vishal)
> ---
> test/common | 4 ++
> test/cxl-qos-class.sh | 102 ++++++++++++++++++++++++++++++++++++++++++
> test/meson.build | 2 +
> 3 files changed, 108 insertions(+)
> create mode 100755 test/cxl-qos-class.sh
>
> diff --git a/test/common b/test/common
> index f1023ef20f7e..5694820c7adc 100644
> --- a/test/common
> +++ b/test/common
> @@ -150,3 +150,7 @@ check_dmesg()
> grep -q "Call Trace" <<< $log && err $1
> true
> }
> +
> +
> +# CXL COMMON
> +TEST_QOS_CLASS=42
> diff --git a/test/cxl-qos-class.sh b/test/cxl-qos-class.sh
> new file mode 100755
> index 000000000000..145df6134685
> --- /dev/null
> +++ b/test/cxl-qos-class.sh
> @@ -0,0 +1,102 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2024 Intel Corporation. All rights reserved.
> +
> +. $(dirname $0)/common
> +
> +rc=77
> +
> +set -ex
> +
> +trap 'err $LINENO' ERR
> +
> +check_prereq "jq"
> +
> +modprobe -r cxl_test
> +modprobe cxl_test
> +rc=1
> +
> +check_qos_decoders () {
> + # check root decoders have expected fake qos_class
> + # also make sure the number of root decoders equal to the number
> + # with qos_class found
> + json=$($CXL list -b cxl_test -D -d root)
> + decoders=$(echo "$json" | jq length)
> + count=0
> + while read -r qos_class
> + do
> + ((qos_class == TEST_QOS_CLASS)) || err "$LINENO"
> + count=$((count+1))
> + done <<< "$(echo "$json" | jq -r '.[] | .qos_class')"
> +
> + ((count == decoders)) || err "$LINENO";
> +}
> +
> +check_qos_memdevs () {
> + # Check that memdevs that expose ram_qos_class or pmem_qos_class have
> + # expected fake value programmed.
> + json=$(cxl list -b cxl_test -M)
> + readarray -t lines < <(jq ".[] | .ram_size, .pmem_size, .ram_qos_class, .pmem_qos_class" <<<"$json")
> + for (( i = 0; i < ${#lines[@]}; i += 4 ))
> + do
> + ram_size=${lines[i]}
> + pmem_size=${lines[i+1]}
> + ram_qos_class=${lines[i+2]}
> + pmem_qos_class=${lines[i+3]}
> +
> + if [[ "$ram_size" != null ]]
> + then
> + ((ram_qos_class == TEST_QOS_CLASS)) || err "$LINENO"
> + fi
> + if [[ "$pmem_size" != null ]]
> + then
> + ((pmem_qos_class == TEST_QOS_CLASS)) || err "$LINENO"
> + fi
> + done
> +}
> +
> +# Based on cxl-create-region.sh create_single()
> +destroy_regions()
> +{
> + if [[ "$*" ]]; then
> + $CXL destroy-region -f -b cxl_test "$@"
> + else
> + $CXL destroy-region -f -b cxl_test all
> + fi
> +}
> +
> +create_region_check_qos()
> +{
> + # the 5th cxl_test decoder is expected to target a single-port
> + # host-bridge. Older cxl_test implementations may not define it,
> + # so skip the test in that case.
> + decoder=$($CXL list -b cxl_test -D -d root |
> + jq -r ".[4] |
> + select(.pmem_capable == true) |
> + select(.nr_targets == 1) |
> + .decoder")
Instead of assuming the 5th decoder, can we select based on some
property of the decoder or its parentage? This works, but it's a bit
sensitive to future cxl_test topology changes that will easily and
(more importantly) silently break this part of the test (since we skip
but still pass).
> +
> + if [[ ! $decoder ]]; then
> + echo "no single-port host-bridge decoder found, skipping"
> + return
> + fi
I think there's some space/tab mixing going on here.
> +
> + # Send create-region with -Q to enforce qos_class matching
> + region=$($CXL create-region -Q -d "$decoder" | jq -r ".region")
> + if [[ ! $region ]]; then
> + echo "failed to create single-port host-bridge region"
> + err "$LINENO"
> + fi
> +
> + destroy_regions "$region"
> +}
> +
> +check_qos_decoders
> +
> +check_qos_memdevs
> +
> +create_region_check_qos
> +
> +check_dmesg "$LINEO"
> +
> +modprobe -r cxl_test
> diff --git a/test/meson.build b/test/meson.build
> index 5eb35749a95b..4892df11119f 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -160,6 +160,7 @@ cxl_events = find_program('cxl-events.sh')
> cxl_poison = find_program('cxl-poison.sh')
> cxl_sanitize = find_program('cxl-sanitize.sh')
> cxl_destroy_region = find_program('cxl-destroy-region.sh')
> +cxl_qos_class = find_program('cxl-qos-class.sh')
>
> tests = [
> [ 'libndctl', libndctl, 'ndctl' ],
> @@ -192,6 +193,7 @@ tests = [
> [ 'cxl-poison.sh', cxl_poison, 'cxl' ],
> [ 'cxl-sanitize.sh', cxl_sanitize, 'cxl' ],
> [ 'cxl-destroy-region.sh', cxl_destroy_region, 'cxl' ],
> + [ 'cxl-qos-class.sh', cxl_qos_class, 'cxl' ],
> ]
>
> if get_option('destructive').enabled()
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [NDCTL PATCH v7 4/4] ndctl: add test for qos_class in CXL test suite
2024-02-22 7:59 ` Verma, Vishal L
@ 2024-02-26 22:39 ` Dave Jiang
0 siblings, 0 replies; 11+ messages in thread
From: Dave Jiang @ 2024-02-26 22:39 UTC (permalink / raw)
To: Verma, Vishal L, linux-cxl@vger.kernel.org,
nvdimm@lists.linux.dev
On 2/22/24 12:59 AM, Verma, Vishal L wrote:
> On Thu, 2024-02-08 at 13:11 -0700, Dave Jiang wrote:
>> Add tests in cxl-qos-class.sh to verify qos_class are set with the fake
>> qos_class create by the kernel. Root decoders should have qos_class
>> attribute set. Memory devices should have ram_qos_class or pmem_qos_class
>> set depending on which partitions are valid.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> v7:
>> - Add create_region -Q testing (Vishal)
>> ---
>> test/common | 4 ++
>> test/cxl-qos-class.sh | 102 ++++++++++++++++++++++++++++++++++++++++++
>> test/meson.build | 2 +
>> 3 files changed, 108 insertions(+)
>> create mode 100755 test/cxl-qos-class.sh
>>
>> diff --git a/test/common b/test/common
>> index f1023ef20f7e..5694820c7adc 100644
>> --- a/test/common
>> +++ b/test/common
>> @@ -150,3 +150,7 @@ check_dmesg()
>> grep -q "Call Trace" <<< $log && err $1
>> true
>> }
>> +
>> +
>> +# CXL COMMON
>> +TEST_QOS_CLASS=42
>> diff --git a/test/cxl-qos-class.sh b/test/cxl-qos-class.sh
>> new file mode 100755
>> index 000000000000..145df6134685
>> --- /dev/null
>> +++ b/test/cxl-qos-class.sh
>> @@ -0,0 +1,102 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (C) 2024 Intel Corporation. All rights reserved.
>> +
>> +. $(dirname $0)/common
>> +
>> +rc=77
>> +
>> +set -ex
>> +
>> +trap 'err $LINENO' ERR
>> +
>> +check_prereq "jq"
>> +
>> +modprobe -r cxl_test
>> +modprobe cxl_test
>> +rc=1
>> +
>> +check_qos_decoders () {
>> + # check root decoders have expected fake qos_class
>> + # also make sure the number of root decoders equal to the number
>> + # with qos_class found
>> + json=$($CXL list -b cxl_test -D -d root)
>> + decoders=$(echo "$json" | jq length)
>> + count=0
>> + while read -r qos_class
>> + do
>> + ((qos_class == TEST_QOS_CLASS)) || err "$LINENO"
>> + count=$((count+1))
>> + done <<< "$(echo "$json" | jq -r '.[] | .qos_class')"
>> +
>> + ((count == decoders)) || err "$LINENO";
>> +}
>> +
>> +check_qos_memdevs () {
>> + # Check that memdevs that expose ram_qos_class or pmem_qos_class have
>> + # expected fake value programmed.
>> + json=$(cxl list -b cxl_test -M)
>> + readarray -t lines < <(jq ".[] | .ram_size, .pmem_size, .ram_qos_class, .pmem_qos_class" <<<"$json")
>> + for (( i = 0; i < ${#lines[@]}; i += 4 ))
>> + do
>> + ram_size=${lines[i]}
>> + pmem_size=${lines[i+1]}
>> + ram_qos_class=${lines[i+2]}
>> + pmem_qos_class=${lines[i+3]}
>> +
>> + if [[ "$ram_size" != null ]]
>> + then
>> + ((ram_qos_class == TEST_QOS_CLASS)) || err "$LINENO"
>> + fi
>> + if [[ "$pmem_size" != null ]]
>> + then
>> + ((pmem_qos_class == TEST_QOS_CLASS)) || err "$LINENO"
>> + fi
>> + done
>> +}
>> +
>> +# Based on cxl-create-region.sh create_single()
>> +destroy_regions()
>> +{
>> + if [[ "$*" ]]; then
>> + $CXL destroy-region -f -b cxl_test "$@"
>> + else
>> + $CXL destroy-region -f -b cxl_test all
>> + fi
>> +}
>> +
>> +create_region_check_qos()
>> +{
>> + # the 5th cxl_test decoder is expected to target a single-port
>> + # host-bridge. Older cxl_test implementations may not define it,
>> + # so skip the test in that case.
>> + decoder=$($CXL list -b cxl_test -D -d root |
>> + jq -r ".[4] |
>> + select(.pmem_capable == true) |
>> + select(.nr_targets == 1) |
>> + .decoder")
>
> Instead of assuming the 5th decoder, can we select based on some
> property of the decoder or its parentage? This works, but it's a bit
> sensitive to future cxl_test topology changes that will easily and
> (more importantly) silently break this part of the test (since we skip
> but still pass).
I copied it straight from cxl-topology.sh. It can be any really. All cxl_test setup should have qos_class. I would imagine cxl-topology.sh would also break if there's some future change
>
>> +
>> + if [[ ! $decoder ]]; then
>> + echo "no single-port host-bridge decoder found, skipping"
>> + return
>> + fi
>
> I think there's some space/tab mixing going on here.
I'll fix.
>
>> +
>> + # Send create-region with -Q to enforce qos_class matching
>> + region=$($CXL create-region -Q -d "$decoder" | jq -r ".region")
>> + if [[ ! $region ]]; then
>> + echo "failed to create single-port host-bridge region"
>> + err "$LINENO"
>> + fi
>> +
>> + destroy_regions "$region"
>> +}
>> +
>> +check_qos_decoders
>> +
>> +check_qos_memdevs
>> +
>> +create_region_check_qos
>> +
>> +check_dmesg "$LINEO"
>> +
>> +modprobe -r cxl_test
>> diff --git a/test/meson.build b/test/meson.build
>> index 5eb35749a95b..4892df11119f 100644
>> --- a/test/meson.build
>> +++ b/test/meson.build
>> @@ -160,6 +160,7 @@ cxl_events = find_program('cxl-events.sh')
>> cxl_poison = find_program('cxl-poison.sh')
>> cxl_sanitize = find_program('cxl-sanitize.sh')
>> cxl_destroy_region = find_program('cxl-destroy-region.sh')
>> +cxl_qos_class = find_program('cxl-qos-class.sh')
>>
>> tests = [
>> [ 'libndctl', libndctl, 'ndctl' ],
>> @@ -192,6 +193,7 @@ tests = [
>> [ 'cxl-poison.sh', cxl_poison, 'cxl' ],
>> [ 'cxl-sanitize.sh', cxl_sanitize, 'cxl' ],
>> [ 'cxl-destroy-region.sh', cxl_destroy_region, 'cxl' ],
>> + [ 'cxl-qos-class.sh', cxl_qos_class, 'cxl' ],
>> ]
>>
>> if get_option('destructive').enabled()
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [NDCTL PATCH v7 4/4] ndctl: add test for qos_class in CXL test suite
2024-02-08 20:11 ` [NDCTL PATCH v7 4/4] ndctl: add test for qos_class in CXL test suite Dave Jiang
2024-02-22 7:59 ` Verma, Vishal L
@ 2024-02-23 22:07 ` Verma, Vishal L
2024-02-26 23:09 ` Dave Jiang
1 sibling, 1 reply; 11+ messages in thread
From: Verma, Vishal L @ 2024-02-23 22:07 UTC (permalink / raw)
To: Jiang, Dave, linux-cxl@vger.kernel.org, nvdimm@lists.linux.dev
On Thu, 2024-02-08 at 13:11 -0700, Dave Jiang wrote:
> Add tests in cxl-qos-class.sh to verify qos_class are set with the fake
> qos_class create by the kernel. Root decoders should have qos_class
> attribute set. Memory devices should have ram_qos_class or pmem_qos_class
> set depending on which partitions are valid.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v7:
> - Add create_region -Q testing (Vishal)
> ---
> test/common | 4 ++
> test/cxl-qos-class.sh | 102 ++++++++++++++++++++++++++++++++++++++++++
> test/meson.build | 2 +
> 3 files changed, 108 insertions(+)
> create mode 100755 test/cxl-qos-class.sh
>
> diff --git a/test/common b/test/common
> index f1023ef20f7e..5694820c7adc 100644
> --- a/test/common
> +++ b/test/common
> @@ -150,3 +150,7 @@ check_dmesg()
> grep -q "Call Trace" <<< $log && err $1
> true`
> }
> +
> +
> +# CXL COMMON
> +TEST_QOS_CLASS=42
> diff --git a/test/cxl-qos-class.sh b/test/cxl-qos-class.sh
> new file mode 100755
> index 000000000000..145df6134685
> --- /dev/null
> +++ b/test/cxl-qos-class.sh
> @@ -0,0 +1,102 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (C) 2024 Intel Corporation. All rights reserved.
> +
> +. $(dirname $0)/common
> +
> +rc=77
> +
> +set -ex
> +
> +trap 'err $LINENO' ERR
> +
> +check_prereq "jq"
> +
> +modprobe -r cxl_test
> +modprobe cxl_test
> +rc=1
> +
> +check_qos_decoders () {
> + # check root decoders have expected fake qos_class
> + # also make sure the number of root decoders equal to the number
> + # with qos_class found
> + json=$($CXL list -b cxl_test -D -d root)
> + decoders=$(echo "$json" | jq length)
> + count=0
> + while read -r qos_class
> + do
For consistency, the script based tests all have the while..do,
for..do, if..then bits on the same line. Would be nice not to break
that precedent.
> + ((qos_class == TEST_QOS_CLASS)) || err "$LINENO"
> + count=$((count+1))
> + done <<< "$(echo "$json" | jq -r '.[] | .qos_class')"
> +
> + ((count == decoders)) || err "$LINENO";
> +}
> +
> +check_qos_memdevs () {
> + # Check that memdevs that expose ram_qos_class or pmem_qos_class have
> + # expected fake value programmed.
> + json=$(cxl list -b cxl_test -M)
> + readarray -t lines < <(jq ".[] | .ram_size, .pmem_size, .ram_qos_class, .pmem_qos_class" <<<"$json")
> + for (( i = 0; i < ${#lines[@]}; i += 4 ))
> + do
> + ram_size=${lines[i]}
> + pmem_size=${lines[i+1]}
> + ram_qos_class=${lines[i+2]}
> + pmem_qos_class=${lines[i+3]}
Hm instead of splitting into lines, and then looping through them, why
not just invoke jq for each?
ram_size=$(jq ".[] | .ram_size" <<< $json)
pmem_size=$(jq ".[] | .pmem_size" <<< $json)
...etc
> +
> + if [[ "$ram_size" != null ]]
> + then
> + ((ram_qos_class == TEST_QOS_CLASS)) || err "$LINENO"
> + fi
This might be a bit more readable as:
if [[ "$ram_size" != null ]] && ((ram_qos_class != TEST_QOS_CLASS)); then
err "$LINENO"
fi
> + if [[ "$pmem_size" != null ]]
> + then
> + ((pmem_qos_class == TEST_QOS_CLASS)) || err "$LINENO"
> + fi
> + done
> +}
> +
> +# Based on cxl-create-region.sh create_single()
> +destroy_regions()
> +{
> + if [[ "$*" ]]; then
> + $CXL destroy-region -f -b cxl_test "$@"
> + else
> + $CXL destroy-region -f -b cxl_test all
> + fi
> +}
> +
> +create_region_check_qos()
> +{
> + # the 5th cxl_test decoder is expected to target a single-port
> + # host-bridge. Older cxl_test implementations may not define it,
> + # so skip the test in that case.
> + decoder=$($CXL list -b cxl_test -D -d root |
> + jq -r ".[4] |
> + select(.pmem_capable == true) |
> + select(.nr_targets == 1) |
> + .decoder")
> +
> + if [[ ! $decoder ]]; then
> + echo "no single-port host-bridge decoder found, skipping"
> + return
> + fi
> +
> + # Send create-region with -Q to enforce qos_class matching
> + region=$($CXL create-region -Q -d "$decoder" | jq -r ".region")
> + if [[ ! $region ]]; then
> + echo "failed to create single-port host-bridge region"
> + err "$LINENO"
> + fi
> +
> + destroy_regions "$region"
> +}
> +
> +check_qos_decoders
> +
> +check_qos_memdevs
> +
> +create_region_check_qos
> +
> +check_dmesg "$LINEO"
> +
> +modprobe -r cxl_test
> diff --git a/test/meson.build b/test/meson.build
> index 5eb35749a95b..4892df11119f 100644
> --- a/test/meson.build
> +++ b/test/meson.build
> @@ -160,6 +160,7 @@ cxl_events = find_program('cxl-events.sh')
> cxl_poison = find_program('cxl-poison.sh')
> cxl_sanitize = find_program('cxl-sanitize.sh')
> cxl_destroy_region = find_program('cxl-destroy-region.sh')
> +cxl_qos_class = find_program('cxl-qos-class.sh')
>
> tests = [
> [ 'libndctl', libndctl, 'ndctl' ],
> @@ -192,6 +193,7 @@ tests = [
> [ 'cxl-poison.sh', cxl_poison, 'cxl' ],
> [ 'cxl-sanitize.sh', cxl_sanitize, 'cxl' ],
> [ 'cxl-destroy-region.sh', cxl_destroy_region, 'cxl' ],
> + [ 'cxl-qos-class.sh', cxl_qos_class, 'cxl' ],
> ]
>
> if get_option('destructive').enabled()
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [NDCTL PATCH v7 4/4] ndctl: add test for qos_class in CXL test suite
2024-02-23 22:07 ` Verma, Vishal L
@ 2024-02-26 23:09 ` Dave Jiang
0 siblings, 0 replies; 11+ messages in thread
From: Dave Jiang @ 2024-02-26 23:09 UTC (permalink / raw)
To: Verma, Vishal L, linux-cxl@vger.kernel.org,
nvdimm@lists.linux.dev
On 2/23/24 3:07 PM, Verma, Vishal L wrote:
> On Thu, 2024-02-08 at 13:11 -0700, Dave Jiang wrote:
>> Add tests in cxl-qos-class.sh to verify qos_class are set with the fake
>> qos_class create by the kernel. Root decoders should have qos_class
>> attribute set. Memory devices should have ram_qos_class or pmem_qos_class
>> set depending on which partitions are valid.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> v7:
>> - Add create_region -Q testing (Vishal)
>> ---
>> test/common | 4 ++
>> test/cxl-qos-class.sh | 102 ++++++++++++++++++++++++++++++++++++++++++
>> test/meson.build | 2 +
>> 3 files changed, 108 insertions(+)
>> create mode 100755 test/cxl-qos-class.sh
>>
>> diff --git a/test/common b/test/common
>> index f1023ef20f7e..5694820c7adc 100644
>> --- a/test/common
>> +++ b/test/common
>> @@ -150,3 +150,7 @@ check_dmesg()
>> grep -q "Call Trace" <<< $log && err $1
>> true`
>> }
>> +
>> +
>> +# CXL COMMON
>> +TEST_QOS_CLASS=42
>> diff --git a/test/cxl-qos-class.sh b/test/cxl-qos-class.sh
>> new file mode 100755
>> index 000000000000..145df6134685
>> --- /dev/null
>> +++ b/test/cxl-qos-class.sh
>> @@ -0,0 +1,102 @@
>> +#!/bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (C) 2024 Intel Corporation. All rights reserved.
>> +
>> +. $(dirname $0)/common
>> +
>> +rc=77
>> +
>> +set -ex
>> +
>> +trap 'err $LINENO' ERR
>> +
>> +check_prereq "jq"
>> +
>> +modprobe -r cxl_test
>> +modprobe cxl_test
>> +rc=1
>> +
>> +check_qos_decoders () {
>> + # check root decoders have expected fake qos_class
>> + # also make sure the number of root decoders equal to the number
>> + # with qos_class found
>> + json=$($CXL list -b cxl_test -D -d root)
>> + decoders=$(echo "$json" | jq length)
>> + count=0
>> + while read -r qos_class
>> + do
>
> For consistency, the script based tests all have the while..do,
> for..do, if..then bits on the same line. Would be nice not to break
> that precedent.
Will fix. BTW, cxl-topology.sh also deviates.
>
>> + ((qos_class == TEST_QOS_CLASS)) || err "$LINENO"
>> + count=$((count+1))
>> + done <<< "$(echo "$json" | jq -r '.[] | .qos_class')"
>> +
>> + ((count == decoders)) || err "$LINENO";
>> +}
>> +
>> +check_qos_memdevs () {
>> + # Check that memdevs that expose ram_qos_class or pmem_qos_class have
>> + # expected fake value programmed.
>> + json=$(cxl list -b cxl_test -M)
>> + readarray -t lines < <(jq ".[] | .ram_size, .pmem_size, .ram_qos_class, .pmem_qos_class" <<<"$json")
>> + for (( i = 0; i < ${#lines[@]}; i += 4 ))
>> + do
>> + ram_size=${lines[i]}
>> + pmem_size=${lines[i+1]}
>> + ram_qos_class=${lines[i+2]}
>> + pmem_qos_class=${lines[i+3]}
>
> Hm instead of splitting into lines, and then looping through them, why
> not just invoke jq for each?
>
> ram_size=$(jq ".[] | .ram_size" <<< $json)
> pmem_size=$(jq ".[] | .pmem_size" <<< $json)
> ...etc
>
ok
>> +
>> + if [[ "$ram_size" != null ]]
>> + then
>> + ((ram_qos_class == TEST_QOS_CLASS)) || err "$LINENO"
>> + fi
>
> This might be a bit more readable as:
>
> if [[ "$ram_size" != null ]] && ((ram_qos_class != TEST_QOS_CLASS)); then
> err "$LINENO"
> fi
ok
DJ
>
>> + if [[ "$pmem_size" != null ]]
>> + then
>> + ((pmem_qos_class == TEST_QOS_CLASS)) || err "$LINENO"
>> + fi
>> + done
>> +}
>> +
>> +# Based on cxl-create-region.sh create_single()
>> +destroy_regions()
>> +{
>> + if [[ "$*" ]]; then
>> + $CXL destroy-region -f -b cxl_test "$@"
>> + else
>> + $CXL destroy-region -f -b cxl_test all
>> + fi
>> +}
>> +
>> +create_region_check_qos()
>> +{
>> + # the 5th cxl_test decoder is expected to target a single-port
>> + # host-bridge. Older cxl_test implementations may not define it,
>> + # so skip the test in that case.
>> + decoder=$($CXL list -b cxl_test -D -d root |
>> + jq -r ".[4] |
>> + select(.pmem_capable == true) |
>> + select(.nr_targets == 1) |
>> + .decoder")
>> +
>> + if [[ ! $decoder ]]; then
>> + echo "no single-port host-bridge decoder found, skipping"
>> + return
>> + fi
>> +
>> + # Send create-region with -Q to enforce qos_class matching
>> + region=$($CXL create-region -Q -d "$decoder" | jq -r ".region")
>> + if [[ ! $region ]]; then
>> + echo "failed to create single-port host-bridge region"
>> + err "$LINENO"
>> + fi
>> +
>> + destroy_regions "$region"
>> +}
>> +
>> +check_qos_decoders
>> +
>> +check_qos_memdevs
>> +
>> +create_region_check_qos
>> +
>> +check_dmesg "$LINEO"
>> +
>> +modprobe -r cxl_test
>> diff --git a/test/meson.build b/test/meson.build
>> index 5eb35749a95b..4892df11119f 100644
>> --- a/test/meson.build
>> +++ b/test/meson.build
>> @@ -160,6 +160,7 @@ cxl_events = find_program('cxl-events.sh')
>> cxl_poison = find_program('cxl-poison.sh')
>> cxl_sanitize = find_program('cxl-sanitize.sh')
>> cxl_destroy_region = find_program('cxl-destroy-region.sh')
>> +cxl_qos_class = find_program('cxl-qos-class.sh')
>>
>> tests = [
>> [ 'libndctl', libndctl, 'ndctl' ],
>> @@ -192,6 +193,7 @@ tests = [
>> [ 'cxl-poison.sh', cxl_poison, 'cxl' ],
>> [ 'cxl-sanitize.sh', cxl_sanitize, 'cxl' ],
>> [ 'cxl-destroy-region.sh', cxl_destroy_region, 'cxl' ],
>> + [ 'cxl-qos-class.sh', cxl_qos_class, 'cxl' ],
>> ]
>>
>> if get_option('destructive').enabled()
>
^ permalink raw reply [flat|nested] 11+ messages in thread