diff mbox series

[NDCTL,v6,3/4] ndctl: cxl: add QoS class check for CXL region creation

Message ID 20240207172055.1882900-4-dave.jiang@intel.com (mailing list archive)
State Superseded
Headers show
Series ndctl: Add support of qos_class for CXL CLI | expand

Commit Message

Dave Jiang Feb. 7, 2024, 5:19 p.m. UTC 
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. 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.

Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
v6:
- Check return value of create_region_validate_qos_class() (Wonjae)
---
 Documentation/cxl/cxl-create-region.txt |  9 ++++
 cxl/region.c                            | 58 ++++++++++++++++++++++++-
 2 files changed, 66 insertions(+), 1 deletion(-)

Comments

Verma, Vishal L Feb. 7, 2024, 9:02 p.m. UTC | #1 
On Wed, 2024-02-07 at 10:19 -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. 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.
> 
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
> v6:
> - Check return value of create_region_validate_qos_class() (Wonjae)
> ---
>  Documentation/cxl/cxl-create-region.txt |  9 ++++
>  cxl/region.c                            | 58 ++++++++++++++++++++++++-
>  2 files changed, 66 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
> index f11a412bddfe..d5e34cf38236 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 qos_class mismatches region creation will fail.
> +
> +-q::
> +--no-enforce-qos::
> +	Parameter to bypass qos_class mismatch failure. Will only emit warning.

Hm, -q is usually synonymous with --quiet, it might be nice to reserve
it for that in case we ever need to add a quiet mode. Maybe use -Q?
Dave Jiang Feb. 7, 2024, 11:32 p.m. UTC | #2 
On 2/7/24 2:02 PM, Verma, Vishal L wrote:
> On Wed, 2024-02-07 at 10:19 -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. 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.
>>
>> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>> v6:
>> - Check return value of create_region_validate_qos_class() (Wonjae)
>> ---
>>  Documentation/cxl/cxl-create-region.txt |  9 ++++
>>  cxl/region.c                            | 58 ++++++++++++++++++++++++-
>>  2 files changed, 66 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
>> index f11a412bddfe..d5e34cf38236 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 qos_class mismatches region creation will fail.
>> +
>> +-q::
>> +--no-enforce-qos::
>> +	Parameter to bypass qos_class mismatch failure. Will only emit warning.
> 
> Hm, -q is usually synonymous with --quiet, it might be nice to reserve
> it for that in case we ever need to add a quiet mode. Maybe use -Q?

Sure. I'll change it. 
> 
> 
>
diff mbox series

Patch

diff --git a/Documentation/cxl/cxl-create-region.txt b/Documentation/cxl/cxl-create-region.txt
index f11a412bddfe..d5e34cf38236 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 qos_class mismatches region creation will fail.
+
+-q::
+--no-enforce-qos::
+	Parameter to bypass qos_class 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..0468f5fa06ee 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_qos;
 } 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_qos;
 };
 
 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-qos", &param.no_qos, "no enforce of qos_class")
 
 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_qos = param.no_qos;
+
 	return 0;
 
 err:
@@ -467,6 +476,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;
+
+	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) {
+			if (p->strict && !p->no_qos) {
+				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 +559,10 @@  found:
 		return rc;
 
 	collect_minsize(ctx, p);
+	rc = create_region_validate_qos_class(ctx, p);
+	if (rc)
+		return rc;
+
 	return 0;
 }