* [ndctl PATCH 0/2] add support for CCI command Get Alert Configuration
@ 2022-10-03 23:23 Jonathan Zhang
2022-10-03 23:23 ` [PATCH 1/2] libcxl: add accessors for Get Alert Configuration CCI command output Jonathan Zhang
2022-10-03 23:23 ` [PATCH 2/2] cxl: display alert configuratin fields in list command Jonathan Zhang
0 siblings, 2 replies; 6+ messages in thread
From: Jonathan Zhang @ 2022-10-03 23:23 UTC (permalink / raw)
To: linux-cxl, vishal.l.verma, wzhang; +Cc: Jonathan Zhang
From: Jonathan Zhang <jonzhang@meta.com>
CXL spec 3.0 section 8.2.9.8.3.2 defines CCI command
Get Alert Configuration.
This patchset adds support for it.
Jonathan Zhang (2):
libcxl: add accessors for Get Alert Configuration CCI command output
cxl: display alert configuratin fields in list command
Documentation/cxl/cxl-list.txt | 33 ++++++++
Documentation/cxl/lib/libcxl.txt | 1 +
cxl/filter.c | 2 +
cxl/filter.h | 1 +
cxl/json.c | 131 +++++++++++++++++++++++++++++++
cxl/lib/libcxl.c | 120 ++++++++++++++++++++++++++++
cxl/lib/libcxl.sym | 23 ++++++
cxl/lib/private.h | 28 +++++++
cxl/libcxl.h | 19 +++++
cxl/list.c | 3 +
util/json.h | 1 +
11 files changed, 362 insertions(+)
--
2.30.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] libcxl: add accessors for Get Alert Configuration CCI command output
2022-10-03 23:23 [ndctl PATCH 0/2] add support for CCI command Get Alert Configuration Jonathan Zhang
@ 2022-10-03 23:23 ` Jonathan Zhang
2022-10-21 7:30 ` Verma, Vishal L
2022-10-03 23:23 ` [PATCH 2/2] cxl: display alert configuratin fields in list command Jonathan Zhang
1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Zhang @ 2022-10-03 23:23 UTC (permalink / raw)
To: linux-cxl, vishal.l.verma, wzhang; +Cc: Jonathan Zhang
From: Jonathan Zhang <jonzhang@meta.com>
CXL 3.0 spec section 8.2.9.8.3.2 "Get Alert Configuration
(Opcode 4201h) defines the get-alert-config command to
retrieve the devices's critical alert and programmable
warning configuration.
Add the methods to issue the command and get the fields
defined.
Signed-off-by: Jonathan Zhang <jonzhang@meta.com>
---
Documentation/cxl/lib/libcxl.txt | 1 +
cxl/lib/libcxl.c | 120 +++++++++++++++++++++++++++++++
cxl/lib/libcxl.sym | 23 ++++++
cxl/lib/private.h | 28 ++++++++
cxl/libcxl.h | 19 +++++
5 files changed, 191 insertions(+)
diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
index fd2962a..dec3641 100644
--- a/Documentation/cxl/lib/libcxl.txt
+++ b/Documentation/cxl/lib/libcxl.txt
@@ -121,6 +121,7 @@ information this call requires root / CAP_SYS_ADMIN.
struct cxl_cmd *cxl_cmd_new_raw(struct cxl_memdev *memdev, int opcode);
struct cxl_cmd *cxl_cmd_new_identify(struct cxl_memdev *memdev);
struct cxl_cmd *cxl_cmd_new_get_health_info(struct cxl_memdev *memdev);
+struct cxl_cmd *cxl_cmd_new_get_alert_config(struct cxl_memdev *memdev);
struct cxl_cmd *cxl_cmd_new_read_label(struct cxl_memdev *memdev,
unsigned int offset, unsigned int length);
struct cxl_cmd *cxl_cmd_new_write_label(struct cxl_memdev *memdev, void *buf,
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index e8c5d44..ed5616c 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -3140,6 +3140,126 @@ do { \
return !!(c->field & mask); \
} while(0)
+CXL_EXPORT struct cxl_cmd *cxl_cmd_new_get_alert_config(
+ struct cxl_memdev *memdev)
+{
+ return cxl_cmd_new_generic(memdev, CXL_MEM_COMMAND_ID_GET_ALERT_CONFIG);
+}
+
+#define cmd_alert_get_valid_alerts_field(c, m) \
+ cmd_get_field_u8_mask(c, get_alert_config, GET_ALERT_CONFIG, valid_alerts, m)
+
+CXL_EXPORT int cxl_cmd_alert_config_get_life_used_prog_warn_threshold_valid(struct cxl_cmd *cmd)
+{
+ cmd_alert_get_valid_alerts_field(cmd,
+ CXL_CMD_ALERT_CONFIG_VALID_ALERTS_LIFE_USED_PROG_WARN_THRESHOLD_MASK);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_valid(struct cxl_cmd *cmd)
+{
+ cmd_alert_get_valid_alerts_field(cmd,
+ CXL_CMD_ALERT_CONFIG_VALID_ALERTS_DEV_OVER_TEMP_PROG_WARN_THRESHOLD_MASK);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_valid(struct cxl_cmd *cmd)
+{
+ cmd_alert_get_valid_alerts_field(cmd,
+ CXL_CMD_ALERT_CONFIG_VALID_ALERTS_DEV_UNDER_TEMP_PROG_WARN_THRESHOLD_MASK);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_valid(struct cxl_cmd *cmd)
+{
+ cmd_alert_get_valid_alerts_field(cmd,
+ CXL_CMD_ALERT_CONFIG_VALID_ALERTS_CORR_VOL_MEM_ERR_PROG_WARN_THRESHOLD_MASK);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_valid(struct cxl_cmd *cmd)
+{
+ cmd_alert_get_valid_alerts_field(cmd,
+ CXL_CMD_ALERT_CONFIG_VALID_ALERTS_CORR_PERS_MEM_ERR_PROG_WARN_THRESHOLD_MASK);
+}
+
+#define cmd_alert_get_prog_alerts_field(c, m) \
+ cmd_get_field_u8_mask(c, get_alert_config, GET_ALERT_CONFIG, programmable_alerts, m)
+
+CXL_EXPORT int cxl_cmd_alert_config_get_life_used_prog_warn_threshold_prog(struct cxl_cmd *cmd)
+{
+ cmd_alert_get_prog_alerts_field(cmd,
+ CXL_CMD_ALERT_CONFIG_PROG_ALERTS_LIEF_USED_PROG_WARN_THRESHOLD_MASK);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_prog(struct cxl_cmd *cmd)
+{
+ cmd_alert_get_prog_alerts_field(cmd,
+ CXL_CMD_ALERT_CONFIG_PROG_ALERTS_DEV_OVER_TEMP_PROG_WARN_THRESHOLD_MASK);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_prog(struct cxl_cmd *cmd)
+{
+ cmd_alert_get_prog_alerts_field(cmd,
+ CXL_CMD_ALERT_CONFIG_PROG_ALERTS_DEV_UNDER_TEMP_PROG_WARN_THRESHOLD_MASK);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_prog(struct cxl_cmd *cmd)
+{
+ cmd_alert_get_prog_alerts_field(cmd,
+ CXL_CMD_ALERT_CONFIG_PROG_ALERTS_CORR_VOL_MEM_ERR_PROG_WARN_THRESHOLD_MASK);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_prog(struct cxl_cmd *cmd)
+{
+ cmd_alert_get_prog_alerts_field(cmd,
+ CXL_CMD_ALERT_CONFIG_PROG_ALERTS_CORR_PERS_MEM_ERR_PROG_WARN_THRESHOLD_MASK);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_life_used_crit_alert_threshold(struct cxl_cmd *cmd)
+{
+ cmd_get_field_u8(cmd, get_alert_config, GET_ALERT_CONFIG,
+ life_used_crit_alert_threshold);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_life_used_prog_warn_threshold(struct cxl_cmd *cmd)
+{
+ cmd_get_field_u8(cmd, get_alert_config, GET_ALERT_CONFIG,
+ life_used_prog_warn_threshold);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_dev_over_temp_crit_alert_threshold(struct cxl_cmd *cmd)
+{
+ cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG,
+ dev_over_temp_crit_alert_threshold);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_dev_under_temp_crit_alert_threshold(struct cxl_cmd *cmd)
+{
+ cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG,
+ dev_under_temp_crit_alert_threshold);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold(struct cxl_cmd *cmd)
+{
+ cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG,
+ dev_over_temp_prog_warn_threshold);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold(struct cxl_cmd *cmd)
+{
+ cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG,
+ dev_under_temp_prog_warn_threshold);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold(struct cxl_cmd *cmd)
+{
+ cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG,
+ corr_vol_mem_err_prog_warn_threshold);
+}
+
+CXL_EXPORT int cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold(struct cxl_cmd *cmd)
+{
+ cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG,
+ corr_pers_mem_err_prog_warn_threshold);
+}
+
CXL_EXPORT struct cxl_cmd *cxl_cmd_new_get_health_info(
struct cxl_memdev *memdev)
{
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 8bb91e0..7c1e261 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -217,3 +217,26 @@ global:
cxl_decoder_get_max_available_extent;
cxl_decoder_get_region;
} LIBCXL_2;
+
+LIBCXL_4 {
+global:
+ cxl_cmd_new_get_alert_config;
+ cxl_cmd_alert_config_get_life_used_prog_warn_threshold_valid;
+ cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_valid;
+ cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_valid;
+ cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_valid;
+ cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_valid;
+ cxl_cmd_alert_config_get_life_used_prog_warn_threshold_prog;
+ cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_prog;
+ cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_prog;
+ cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_prog;
+ cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_prog;
+ cxl_cmd_alert_config_get_life_used_crit_alert_threshold;
+ cxl_cmd_alert_config_get_life_used_prog_warn_threshold;
+ cxl_cmd_alert_config_get_dev_over_temp_crit_alert_threshold;
+ cxl_cmd_alert_config_get_dev_under_temp_crit_alert_threshold;
+ cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold;
+ cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold;
+ cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold;
+ cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold;
+} LIBCXL_3;
diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index 437eade..5a1a786 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -228,6 +228,34 @@ struct cxl_cmd_get_health_info {
le32 pmem_errors;
} __attribute__((packed));
+/* CXL 3.0 8.2.9.8.3.2 Get Alert Configuration */
+struct cxl_cmd_get_alert_config {
+ u8 valid_alerts;
+ u8 programmable_alerts;
+ u8 life_used_crit_alert_threshold;
+ u8 life_used_prog_warn_threshold;
+ le16 dev_over_temp_crit_alert_threshold;
+ le16 dev_under_temp_crit_alert_threshold;
+ le16 dev_over_temp_prog_warn_threshold;
+ le16 dev_under_temp_prog_warn_threshold;
+ le16 corr_vol_mem_err_prog_warn_threshold;
+ le16 corr_pers_mem_err_prog_warn_threshold;
+} __attribute__((packed));
+
+/* CXL 3.0 8.2.9.8.3.2 Get Alert Configuration Byte 0 Valid Alerts */
+#define CXL_CMD_ALERT_CONFIG_VALID_ALERTS_LIFE_USED_PROG_WARN_THRESHOLD_MASK BIT(0)
+#define CXL_CMD_ALERT_CONFIG_VALID_ALERTS_DEV_OVER_TEMP_PROG_WARN_THRESHOLD_MASK BIT(1)
+#define CXL_CMD_ALERT_CONFIG_VALID_ALERTS_DEV_UNDER_TEMP_PROG_WARN_THRESHOLD_MASK BIT(2)
+#define CXL_CMD_ALERT_CONFIG_VALID_ALERTS_CORR_VOL_MEM_ERR_PROG_WARN_THRESHOLD_MASK BIT(3)
+#define CXL_CMD_ALERT_CONFIG_VALID_ALERTS_CORR_PERS_MEM_ERR_PROG_WARN_THRESHOLD_MASK BIT(4)
+
+/* CXL 3.0 8.2.9.8.3.2 Get Alert Configuration Byte 1 Programmable Alerts */
+#define CXL_CMD_ALERT_CONFIG_PROG_ALERTS_LIEF_USED_PROG_WARN_THRESHOLD_MASK BIT(0)
+#define CXL_CMD_ALERT_CONFIG_PROG_ALERTS_DEV_OVER_TEMP_PROG_WARN_THRESHOLD_MASK BIT(1)
+#define CXL_CMD_ALERT_CONFIG_PROG_ALERTS_DEV_UNDER_TEMP_PROG_WARN_THRESHOLD_MASK BIT(2)
+#define CXL_CMD_ALERT_CONFIG_PROG_ALERTS_CORR_VOL_MEM_ERR_PROG_WARN_THRESHOLD_MASK BIT(3)
+#define CXL_CMD_ALERT_CONFIG_PROG_ALERTS_CORR_PERS_MEM_ERR_PROG_WARN_THRESHOLD_MASK BIT(4)
+
struct cxl_cmd_get_partition {
le64 active_volatile;
le64 active_persistent;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 9fe4e99..8aa1c46 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -354,6 +354,25 @@ int cxl_cmd_health_info_get_temperature(struct cxl_cmd *cmd);
int cxl_cmd_health_info_get_dirty_shutdowns(struct cxl_cmd *cmd);
int cxl_cmd_health_info_get_volatile_errors(struct cxl_cmd *cmd);
int cxl_cmd_health_info_get_pmem_errors(struct cxl_cmd *cmd);
+struct cxl_cmd *cxl_cmd_new_get_alert_config(struct cxl_memdev *memdev);
+int cxl_cmd_alert_config_get_life_used_prog_warn_threshold_valid(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_valid(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_valid(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_valid(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_valid(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_life_used_prog_warn_threshold_prog(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_prog(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_prog(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_prog(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_prog(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_life_used_crit_alert_threshold(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_life_used_prog_warn_threshold(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_dev_over_temp_crit_alert_threshold(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_dev_under_temp_crit_alert_threshold(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold(struct cxl_cmd *cmd);
+int cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold(struct cxl_cmd *cmd);
struct cxl_cmd *cxl_cmd_new_read_label(struct cxl_memdev *memdev,
unsigned int offset, unsigned int length);
ssize_t cxl_cmd_read_label_get_payload(struct cxl_cmd *cmd, void *buf,
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] cxl: display alert configuratin fields in list command
2022-10-03 23:23 [ndctl PATCH 0/2] add support for CCI command Get Alert Configuration Jonathan Zhang
2022-10-03 23:23 ` [PATCH 1/2] libcxl: add accessors for Get Alert Configuration CCI command output Jonathan Zhang
@ 2022-10-03 23:23 ` Jonathan Zhang
2022-10-21 7:30 ` Verma, Vishal L
1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Zhang @ 2022-10-03 23:23 UTC (permalink / raw)
To: linux-cxl, vishal.l.verma, wzhang; +Cc: Jonathan Zhang
From: Jonathan Zhang <jonzhang@meta.com>
When list commands are issued, display alert config:
Signed-off-by: Jonathan Zhang <jonzhang@meta.com>
---
Documentation/cxl/cxl-list.txt | 33 +++++++++
cxl/filter.c | 2 +
cxl/filter.h | 1 +
cxl/json.c | 131 +++++++++++++++++++++++++++++++++
cxl/list.c | 3 +
util/json.h | 1 +
6 files changed, 171 insertions(+)
diff --git a/Documentation/cxl/cxl-list.txt b/Documentation/cxl/cxl-list.txt
index 14a2b4b..66dfa15 100644
--- a/Documentation/cxl/cxl-list.txt
+++ b/Documentation/cxl/cxl-list.txt
@@ -219,6 +219,39 @@ OPTIONS
}
]
----
+-A::
+--alert::
+ Include alert configuration in the memdev listing. Example listing:
+----
+# cxl list -m mem0 -A
+[
+ {
+ "memdev":"mem0",
+ "pmem_size":0,
+ "ram_size":273535729664,
+ "alert":{
+ "life_used_prog_warn_threshold_valid":false,
+ "dev_over_temp_prog_warn_threshold_valid":false,
+ "dev_under_temp_prog_warn_threshold_valid":false,
+ "corr_vol_mem_err_prog_warn_threshold_valid":true,
+ "corr_pers_mem_err_prog_warn_threshold_valid":false,
+ "life_used_prog_warn_threshold_programmable":false,
+ "dev_over_temp_prog_warn_threshold_programmable":false,
+ "dev_under_temp_prog_warn_threshold_programmable":false,
+ "corr_vol_mem_err_prog_warn_threshold_programmable":true,
+ "corr_pers_mem_err_prog_warn_threshold_programmable":false,
+ "life_used_crit_alert_threshold":0,
+ "life_used_prog_warn_threshold":0,
+ "dev_over_temp_crit_alert_threshold":0,
+ "dev_under_temp_crit_alert_threshold":0,
+ "dev_over_temp_prog_warn_threshold":0,
+ "dev_under_temp_prog_warn_threshold":0,
+ "corr_vol_mem_err_prog_warn_threshold":0,
+ "corr_pers_mem_err_prog_warn_threshold":0
+ },
+ }
+]
+----
-B::
--buses::
diff --git a/cxl/filter.c b/cxl/filter.c
index 56c6599..6d71c1c 100644
--- a/cxl/filter.c
+++ b/cxl/filter.c
@@ -686,6 +686,8 @@ static unsigned long params_to_flags(struct cxl_filter_params *param)
flags |= UTIL_JSON_TARGETS;
if (param->partition)
flags |= UTIL_JSON_PARTITION;
+ if (param->alert)
+ flags |= UTIL_JSON_ALERT;
return flags;
}
diff --git a/cxl/filter.h b/cxl/filter.h
index 256df49..ec0f74c 100644
--- a/cxl/filter.h
+++ b/cxl/filter.h
@@ -26,6 +26,7 @@ struct cxl_filter_params {
bool human;
bool health;
bool partition;
+ bool alert;
int verbose;
struct log_ctx ctx;
};
diff --git a/cxl/json.c b/cxl/json.c
index 63c1751..dd83ed6 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -185,6 +185,131 @@ err_jobj:
return NULL;
}
+static struct json_object *util_cxl_memdev_alert_to_json(
+ struct cxl_memdev *memdev, unsigned long flags)
+{
+ struct json_object *jalert;
+ struct json_object *jobj;
+ struct cxl_cmd *cmd;
+ int rc;
+
+ jalert = json_object_new_object();
+ if (!jalert)
+ return NULL;
+ if (!memdev)
+ goto err_jobj;
+
+ cmd = cxl_cmd_new_get_alert_config(memdev);
+ if (!cmd)
+ goto err_jobj;
+
+ rc = cxl_cmd_submit(cmd);
+ if (rc < 0)
+ goto err_cmd;
+ rc = cxl_cmd_get_mbox_status(cmd);
+ if (rc != 0)
+ goto err_cmd;
+
+ rc = cxl_cmd_alert_config_get_life_used_prog_warn_threshold_valid(cmd);
+ jobj = json_object_new_boolean(rc);
+ if (jobj)
+ json_object_object_add(jalert, "life_used_prog_warn_threshold_valid", jobj);
+
+ rc = cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_valid(cmd);
+ jobj = json_object_new_boolean(rc);
+ if (jobj)
+ json_object_object_add(jalert, "dev_over_temp_prog_warn_threshold_valid", jobj);
+
+ rc = cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_valid(cmd);
+ jobj = json_object_new_boolean(rc);
+ if (jobj)
+ json_object_object_add(jalert, "dev_under_temp_prog_warn_threshold_valid", jobj);
+
+ rc = cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_valid(cmd);
+ jobj = json_object_new_boolean(rc);
+ if (jobj)
+ json_object_object_add(jalert, "corr_vol_mem_err_prog_warn_threshold_valid", jobj);
+
+ rc = cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_valid(cmd);
+ jobj = json_object_new_boolean(rc);
+ if (jobj)
+ json_object_object_add(jalert, "corr_pers_mem_err_prog_warn_threshold_valid", jobj);
+
+ rc = cxl_cmd_alert_config_get_life_used_prog_warn_threshold_prog(cmd);
+ jobj = json_object_new_boolean(rc);
+ if (jobj)
+ json_object_object_add(jalert, "life_used_prog_warn_threshold_programmable", jobj);
+
+ rc = cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_prog(cmd);
+ jobj = json_object_new_boolean(rc);
+ if (jobj)
+ json_object_object_add(jalert, "dev_over_temp_prog_warn_threshold_programmable", jobj);
+
+ rc = cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_prog(cmd);
+ jobj = json_object_new_boolean(rc);
+ if (jobj)
+ json_object_object_add(jalert, "dev_under_temp_prog_warn_threshold_programmable", jobj);
+
+ rc = cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_prog(cmd);
+ jobj = json_object_new_boolean(rc);
+ if (jobj)
+ json_object_object_add(jalert, "corr_vol_mem_err_prog_warn_threshold_programmable", jobj);
+
+ rc = cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_prog(cmd);
+ jobj = json_object_new_boolean(rc);
+ if (jobj)
+ json_object_object_add(jalert, "corr_pers_mem_err_prog_warn_threshold_programmable", jobj);
+
+ rc = cxl_cmd_alert_config_get_life_used_crit_alert_threshold(cmd);
+ jobj = json_object_new_int(rc);
+ if (jobj)
+ json_object_object_add(jalert, "life_used_crit_alert_threshold", jobj);
+
+ rc = cxl_cmd_alert_config_get_life_used_prog_warn_threshold(cmd);
+ jobj = json_object_new_int(rc);
+ if (jobj)
+ json_object_object_add(jalert, "life_used_prog_warn_threshold", jobj);
+
+ rc = cxl_cmd_alert_config_get_dev_over_temp_crit_alert_threshold(cmd);
+ jobj = json_object_new_int(rc);
+ if (jobj)
+ json_object_object_add(jalert, "dev_over_temp_crit_alert_threshold", jobj);
+
+ rc = cxl_cmd_alert_config_get_dev_under_temp_crit_alert_threshold(cmd);
+ jobj = json_object_new_int(rc);
+ if (jobj)
+ json_object_object_add(jalert, "dev_under_temp_crit_alert_threshold", jobj);
+
+ rc = cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold(cmd);
+ jobj = json_object_new_int(rc);
+ if (jobj)
+ json_object_object_add(jalert, "dev_over_temp_prog_warn_threshold", jobj);
+
+ rc = cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold(cmd);
+ jobj = json_object_new_int(rc);
+ if (jobj)
+ json_object_object_add(jalert, "dev_under_temp_prog_warn_threshold", jobj);
+
+ rc = cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold(cmd);
+ jobj = json_object_new_int(rc);
+ if (jobj)
+ json_object_object_add(jalert, "corr_vol_mem_err_prog_warn_threshold", jobj);
+
+ rc = cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold(cmd);
+ jobj = json_object_new_int(rc);
+ if (jobj)
+ json_object_object_add(jalert, "corr_pers_mem_err_prog_warn_threshold", jobj);
+
+ cxl_cmd_unref(cmd);
+ return jalert;
+
+err_cmd:
+ cxl_cmd_unref(cmd);
+err_jobj:
+ json_object_put(jalert);
+ return NULL;
+}
+
/*
* Present complete view of memdev partition by presenting fields from
* both GET_PARTITION_INFO and IDENTIFY mailbox commands.
@@ -330,6 +455,12 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
json_object_object_add(jdev, "health", jobj);
}
+ if (flags & UTIL_JSON_ALERT) {
+ jobj = util_cxl_memdev_alert_to_json(memdev, flags);
+ if (jobj)
+ json_object_object_add(jdev, "alert", jobj);
+ }
+
serial = cxl_memdev_get_serial(memdev);
if (serial < ULLONG_MAX) {
jobj = util_json_object_hex(serial, flags);
diff --git a/cxl/list.c b/cxl/list.c
index 8c48fbb..6ec4403 100644
--- a/cxl/list.c
+++ b/cxl/list.c
@@ -52,6 +52,8 @@ static const struct option options[] = {
"include memory device health information"),
OPT_BOOLEAN('I', "partition", ¶m.partition,
"include memory device partition information"),
+ OPT_BOOLEAN('A', "alert", ¶m.alert,
+ "include alert configuration information"),
OPT_INCR('v', "verbose", ¶m.verbose,
"increase output detail"),
#ifdef ENABLE_DEBUG
@@ -113,6 +115,7 @@ int cmd_list(int argc, const char **argv, struct cxl_ctx *ctx)
case 3:
param.health = true;
param.partition = true;
+ param.alert = true;
/* fallthrough */
case 2:
param.idle = true;
diff --git a/util/json.h b/util/json.h
index 73bb9f0..06594b5 100644
--- a/util/json.h
+++ b/util/json.h
@@ -20,6 +20,7 @@ enum util_json_flags {
UTIL_JSON_HEALTH = (1 << 10),
UTIL_JSON_TARGETS = (1 << 11),
UTIL_JSON_PARTITION = (1 << 12),
+ UTIL_JSON_ALERT = (1 << 13),
};
void util_display_json_array(FILE *f_out, struct json_object *jarray,
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] libcxl: add accessors for Get Alert Configuration CCI command output
2022-10-03 23:23 ` [PATCH 1/2] libcxl: add accessors for Get Alert Configuration CCI command output Jonathan Zhang
@ 2022-10-21 7:30 ` Verma, Vishal L
2022-10-21 7:35 ` Verma, Vishal L
0 siblings, 1 reply; 6+ messages in thread
From: Verma, Vishal L @ 2022-10-21 7:30 UTC (permalink / raw)
To: wzhang@meta.com, linux-cxl@vger.kernel.org, Zhang, Jonathan
Cc: jonzhang@meta.com
On Mon, 2022-10-03 at 16:23 -0700, Jonathan Zhang wrote:
> From: Jonathan Zhang <jonzhang@meta.com>
>
> CXL 3.0 spec section 8.2.9.8.3.2 "Get Alert Configuration
> (Opcode 4201h) defines the get-alert-config command to
> retrieve the devices's critical alert and programmable
> warning configuration.
>
> Add the methods to issue the command and get the fields
> defined.
>
> Signed-off-by: Jonathan Zhang <jonzhang@meta.com>
> ---
> Documentation/cxl/lib/libcxl.txt | 1 +
> cxl/lib/libcxl.c | 120 +++++++++++++++++++++++++++++++
> cxl/lib/libcxl.sym | 23 ++++++
> cxl/lib/private.h | 28 ++++++++
> cxl/libcxl.h | 19 +++++
> 5 files changed, 191 insertions(+)
Hi Jonathan,
Sorry for the delay in getting to these - a few small comments below.
>
> diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
> index fd2962a..dec3641 100644
> --- a/Documentation/cxl/lib/libcxl.txt
> +++ b/Documentation/cxl/lib/libcxl.txt
> @@ -121,6 +121,7 @@ information this call requires root / CAP_SYS_ADMIN.
> struct cxl_cmd *cxl_cmd_new_raw(struct cxl_memdev *memdev, int opcode);
> struct cxl_cmd *cxl_cmd_new_identify(struct cxl_memdev *memdev);
> struct cxl_cmd *cxl_cmd_new_get_health_info(struct cxl_memdev *memdev);
> +struct cxl_cmd *cxl_cmd_new_get_alert_config(struct cxl_memdev *memdev);
> struct cxl_cmd *cxl_cmd_new_read_label(struct cxl_memdev *memdev,
> unsigned int offset, unsigned int length);
> struct cxl_cmd *cxl_cmd_new_write_label(struct cxl_memdev *memdev, void *buf,
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index e8c5d44..ed5616c 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -3140,6 +3140,126 @@ do { \
> return !!(c->field & mask); \
> } while(0)
>
> +CXL_EXPORT struct cxl_cmd *cxl_cmd_new_get_alert_config(
> + struct cxl_memdev *memdev)
> +{
> + return cxl_cmd_new_generic(memdev, CXL_MEM_COMMAND_ID_GET_ALERT_CONFIG);
> +}
> +
> +#define cmd_alert_get_valid_alerts_field(c, m) \
> + cmd_get_field_u8_mask(c, get_alert_config, GET_ALERT_CONFIG, valid_alerts, m)
> +
> +CXL_EXPORT int cxl_cmd_alert_config_get_life_used_prog_warn_threshold_valid(struct cxl_cmd *cmd)
Generally throughout these patches, because of the (unavoidably) long
names, these lines are well over 80 chars. For cases where a #define
for example can't be broken down further, that's okay. But a lot of
these can be split. We have a .clang-format for the project, and that
should do the right thing in all these cases.
<..>
>
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 8bb91e0..7c1e261 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -217,3 +217,26 @@ global:
> cxl_decoder_get_max_available_extent;
> cxl_decoder_get_region;
> } LIBCXL_2;
> +
> +LIBCXL_4 {
> +global:
> + cxl_cmd_new_get_alert_config;
> + cxl_cmd_alert_config_get_life_used_prog_warn_threshold_valid;
> + cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_valid;
> + cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_valid;
> + cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_valid;
> + cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_valid;
I think for all the 'valid' accessors, the 'get' can be dropped.
cxl_cmd_alert_config_<something>_valid is a reasonable API that asks
whether something is valid.
> + cxl_cmd_alert_config_get_life_used_prog_warn_threshold_prog;
> + cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_prog;
> + cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_prog;
> + cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_prog;
> + cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_prog;
> + cxl_cmd_alert_config_get_life_used_crit_alert_threshold;
> + cxl_cmd_alert_config_get_life_used_prog_warn_threshold;
> + cxl_cmd_alert_config_get_dev_over_temp_crit_alert_threshold;
> + cxl_cmd_alert_config_get_dev_under_temp_crit_alert_threshold;
> + cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold;
> + cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold;
> + cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold;
> + cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold;
These are okay to have the get, since we are getting a threshold value.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] cxl: display alert configuratin fields in list command
2022-10-03 23:23 ` [PATCH 2/2] cxl: display alert configuratin fields in list command Jonathan Zhang
@ 2022-10-21 7:30 ` Verma, Vishal L
0 siblings, 0 replies; 6+ messages in thread
From: Verma, Vishal L @ 2022-10-21 7:30 UTC (permalink / raw)
To: wzhang@meta.com, linux-cxl@vger.kernel.org, Zhang, Jonathan
Cc: jonzhang@meta.com
On Mon, 2022-10-03 at 16:23 -0700, Jonathan Zhang wrote:
> From: Jonathan Zhang <jonzhang@meta.com>
>
> When list commands are issued, display alert config:
>
> Signed-off-by: Jonathan Zhang <jonzhang@meta.com>
> ---
> Documentation/cxl/cxl-list.txt | 33 +++++++++
> cxl/filter.c | 2 +
> cxl/filter.h | 1 +
> cxl/json.c | 131 +++++++++++++++++++++++++++++++++
> cxl/list.c | 3 +
> util/json.h | 1 +
> 6 files changed, 171 insertions(+)
>
> diff --git a/Documentation/cxl/cxl-list.txt b/Documentation/cxl/cxl-list.txt
> index 14a2b4b..66dfa15 100644
> --- a/Documentation/cxl/cxl-list.txt
> +++ b/Documentation/cxl/cxl-list.txt
> @@ -219,6 +219,39 @@ OPTIONS
> }
> ]
> ----
> +-A::
> +--alert::
> + Include alert configuration in the memdev listing. Example listing:
> +----
> +# cxl list -m mem0 -A
> +[
> + {
> + "memdev":"mem0",
> + "pmem_size":0,
> + "ram_size":273535729664,
> + "alert":{
> + "life_used_prog_warn_threshold_valid":false,
> + "dev_over_temp_prog_warn_threshold_valid":false,
> + "dev_under_temp_prog_warn_threshold_valid":false,
> + "corr_vol_mem_err_prog_warn_threshold_valid":true,
> + "corr_pers_mem_err_prog_warn_threshold_valid":false,
> + "life_used_prog_warn_threshold_programmable":false,
I feel the 'prog' and 'programmabnle' conflict a bit. How about:
"life_used_prog_warn_threshold_writable":false,
> + "dev_over_temp_prog_warn_threshold_programmable":false,
> + "dev_under_temp_prog_warn_threshold_programmable":false,
> + "corr_vol_mem_err_prog_warn_threshold_programmable":true,
> + "corr_pers_mem_err_prog_warn_threshold_programmable":false,
The health info output uses long form 'corrected', and 'volatile'. Even
though the final string becomes a bit longer, I think it would be
better to maintain consistency with health_info on these. Same for
'temp' actually - use 'temperature', especially so it isn't confused
with 'temporary'. And for persistent, we've used 'pmem' everywhere
else, so this should become something like:
"corrected_pmem_err_prog_warn_threshold_writable":false,
> + "life_used_crit_alert_threshold":0,
> + "life_used_prog_warn_threshold":0,
> + "dev_over_temp_crit_alert_threshold":0,
> + "dev_under_temp_crit_alert_threshold":0,
> + "dev_over_temp_prog_warn_threshold":0,
> + "dev_under_temp_prog_warn_threshold":0,
> + "corr_vol_mem_err_prog_warn_threshold":0,
> + "corr_pers_mem_err_prog_warn_threshold":0
> + },
> + }
> +]
> +----
>
> -B::
> --buses::
> diff --git a/cxl/filter.c b/cxl/filter.c
> index 56c6599..6d71c1c 100644
> --- a/cxl/filter.c
> +++ b/cxl/filter.c
> @@ -686,6 +686,8 @@ static unsigned long params_to_flags(struct
> cxl_filter_params *param)
> flags |= UTIL_JSON_TARGETS;
> if (param->partition)
> flags |= UTIL_JSON_PARTITION;
> + if (param->alert)
> + flags |= UTIL_JSON_ALERT;
Maybe call this flag UTIL_JSON_ALERT_CONFIG. Just ALERT might be a bit
too generic, since JSON flags are shared among ndctl and daxctl as
well.
> return flags;
> }
>
> diff --git a/cxl/filter.h b/cxl/filter.h
> index 256df49..ec0f74c 100644
> --- a/cxl/filter.h
> +++ b/cxl/filter.h
> @@ -26,6 +26,7 @@ struct cxl_filter_params {
> bool human;
> bool health;
> bool partition;
> + bool alert;
> int verbose;
> struct log_ctx ctx;
> };
> diff --git a/cxl/json.c b/cxl/json.c
> index 63c1751..dd83ed6 100644
> --- a/cxl/json.c
> +++ b/cxl/json.c
> @@ -185,6 +185,131 @@ err_jobj:
> return NULL;
> }
>
> +static struct json_object *util_cxl_memdev_alert_to_json(
> + struct cxl_memdev *memdev, unsigned long flags)
> +{
> + struct json_object *jalert;
> + struct json_object *jobj;
> + struct cxl_cmd *cmd;
> + int rc;
> +
> + jalert = json_object_new_object();
> + if (!jalert)
> + return NULL;
> + if (!memdev)
> + goto err_jobj;
> +
> + cmd = cxl_cmd_new_get_alert_config(memdev);
> + if (!cmd)
> + goto err_jobj;
> +
> + rc = cxl_cmd_submit(cmd);
> + if (rc < 0)
> + goto err_cmd;
> + rc = cxl_cmd_get_mbox_status(cmd);
> + if (rc != 0)
> + goto err_cmd;
> +
> + rc = cxl_cmd_alert_config_get_life_used_prog_warn_threshold_valid(cmd);
> + jobj = json_object_new_boolean(rc);
> + if (jobj)
> + json_object_object_add(jalert, "life_used_prog_warn_threshold_valid", jobj);
Same comment as patch 1 about long lines.
> +
> + rc = cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_valid(cmd);
> + jobj = json_object_new_boolean(rc);
> + if (jobj)
> + json_object_object_add(jalert, "dev_over_temp_prog_warn_threshold_valid", jobj);
> +
> + rc = cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_valid(cmd);
> + jobj = json_object_new_boolean(rc);
> + if (jobj)
> + json_object_object_add(jalert, "dev_under_temp_prog_warn_threshold_valid", jobj);
> +
> + rc = cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_valid(cmd);
> + jobj = json_object_new_boolean(rc);
> + if (jobj)
> + json_object_object_add(jalert, "corr_vol_mem_err_prog_warn_threshold_valid", jobj);
> +
> + rc = cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_valid(cmd);
> + jobj = json_object_new_boolean(rc);
> + if (jobj)
> + json_object_object_add(jalert, "corr_pers_mem_err_prog_warn_threshold_valid", jobj);
> +
> + rc = cxl_cmd_alert_config_get_life_used_prog_warn_threshold_prog(cmd);
> + jobj = json_object_new_boolean(rc);
> + if (jobj)
> + json_object_object_add(jalert, "life_used_prog_warn_threshold_programmable", jobj);
> +
> + rc = cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_prog(cmd);
> + jobj = json_object_new_boolean(rc);
> + if (jobj)
> + json_object_object_add(jalert, "dev_over_temp_prog_warn_threshold_programmable", jobj);
> +
> + rc = cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_prog(cmd);
> + jobj = json_object_new_boolean(rc);
> + if (jobj)
> + json_object_object_add(jalert, "dev_under_temp_prog_warn_threshold_programmable", jobj);
> +
> + rc = cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_prog(cmd);
> + jobj = json_object_new_boolean(rc);
> + if (jobj)
> + json_object_object_add(jalert, "corr_vol_mem_err_prog_warn_threshold_programmable", jobj);
> +
> + rc = cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_prog(cmd);
> + jobj = json_object_new_boolean(rc);
> + if (jobj)
> + json_object_object_add(jalert, "corr_pers_mem_err_prog_warn_threshold_programmable", jobj);
> +
> + rc = cxl_cmd_alert_config_get_life_used_crit_alert_threshold(cmd);
> + jobj = json_object_new_int(rc);
> + if (jobj)
> + json_object_object_add(jalert, "life_used_crit_alert_threshold", jobj);
> +
> + rc = cxl_cmd_alert_config_get_life_used_prog_warn_threshold(cmd);
> + jobj = json_object_new_int(rc);
> + if (jobj)
> + json_object_object_add(jalert, "life_used_prog_warn_threshold", jobj);
> +
> + rc = cxl_cmd_alert_config_get_dev_over_temp_crit_alert_threshold(cmd);
> + jobj = json_object_new_int(rc);
> + if (jobj)
> + json_object_object_add(jalert, "dev_over_temp_crit_alert_threshold", jobj);
> +
> + rc = cxl_cmd_alert_config_get_dev_under_temp_crit_alert_threshold(cmd);
> + jobj = json_object_new_int(rc);
> + if (jobj)
> + json_object_object_add(jalert, "dev_under_temp_crit_alert_threshold", jobj);
> +
> + rc = cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold(cmd);
> + jobj = json_object_new_int(rc);
> + if (jobj)
> + json_object_object_add(jalert, "dev_over_temp_prog_warn_threshold", jobj);
> +
> + rc = cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold(cmd);
> + jobj = json_object_new_int(rc);
> + if (jobj)
> + json_object_object_add(jalert, "dev_under_temp_prog_warn_threshold", jobj);
> +
> + rc = cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold(cmd);
> + jobj = json_object_new_int(rc);
> + if (jobj)
> + json_object_object_add(jalert, "corr_vol_mem_err_prog_warn_threshold", jobj);
> +
> + rc = cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold(cmd);
> + jobj = json_object_new_int(rc);
> + if (jobj)
> + json_object_object_add(jalert, "corr_pers_mem_err_prog_warn_threshold", jobj);
> +
> + cxl_cmd_unref(cmd);
> + return jalert;
> +
> +err_cmd:
> + cxl_cmd_unref(cmd);
> +err_jobj:
> + json_object_put(jalert);
> + return NULL;
> +}
> +
> /*
> * Present complete view of memdev partition by presenting fields from
> * both GET_PARTITION_INFO and IDENTIFY mailbox commands.
> @@ -330,6 +455,12 @@ struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
> json_object_object_add(jdev, "health", jobj);
> }
>
> + if (flags & UTIL_JSON_ALERT) {
> + jobj = util_cxl_memdev_alert_to_json(memdev, flags);
> + if (jobj)
> + json_object_object_add(jdev, "alert", jobj);
> + }
> +
> serial = cxl_memdev_get_serial(memdev);
> if (serial < ULLONG_MAX) {
> jobj = util_json_object_hex(serial, flags);
> diff --git a/cxl/list.c b/cxl/list.c
> index 8c48fbb..6ec4403 100644
> --- a/cxl/list.c
> +++ b/cxl/list.c
> @@ -52,6 +52,8 @@ static const struct option options[] = {
> "include memory device health information"),
> OPT_BOOLEAN('I', "partition", ¶m.partition,
> "include memory device partition information"),
> + OPT_BOOLEAN('A', "alert", ¶m.alert,
> + "include alert configuration information"),
This should be --alert-config - in case there is a future need to list,
e.g. alerts that have triggered. And regardless of the future need,
we're listing the alert config, not alerts themselves. Likewise rename
param.alert to param.alert_config too.
> OPT_INCR('v', "verbose", ¶m.verbose,
> "increase output detail"),
> #ifdef ENABLE_DEBUG
> @@ -113,6 +115,7 @@ int cmd_list(int argc, const char **argv, struct cxl_ctx *ctx)
> case 3:
> param.health = true;
> param.partition = true;
> + param.alert = true;
> /* fallthrough */
> case 2:
> param.idle = true;
> diff --git a/util/json.h b/util/json.h
> index 73bb9f0..06594b5 100644
> --- a/util/json.h
> +++ b/util/json.h
> @@ -20,6 +20,7 @@ enum util_json_flags {
> UTIL_JSON_HEALTH = (1 << 10),
> UTIL_JSON_TARGETS = (1 << 11),
> UTIL_JSON_PARTITION = (1 << 12),
> + UTIL_JSON_ALERT = (1 << 13),
> };
>
> void util_display_json_array(FILE *f_out, struct json_object *jarray,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] libcxl: add accessors for Get Alert Configuration CCI command output
2022-10-21 7:30 ` Verma, Vishal L
@ 2022-10-21 7:35 ` Verma, Vishal L
0 siblings, 0 replies; 6+ messages in thread
From: Verma, Vishal L @ 2022-10-21 7:35 UTC (permalink / raw)
To: wzhang@meta.com, linux-cxl@vger.kernel.org, Zhang, Jonathan
Cc: jonzhang@meta.com
On Fri, 2022-10-21 at 07:30 +0000, Verma, Vishal L wrote:
> On Mon, 2022-10-03 at 16:23 -0700, Jonathan Zhang wrote:
> > From: Jonathan Zhang <jonzhang@meta.com>
> >
> > CXL 3.0 spec section 8.2.9.8.3.2 "Get Alert Configuration
> > (Opcode 4201h) defines the get-alert-config command to
> > retrieve the devices's critical alert and programmable
> > warning configuration.
> >
> > Add the methods to issue the command and get the fields
> > defined.
> >
> > Signed-off-by: Jonathan Zhang <jonzhang@meta.com>
> > ---
> > Documentation/cxl/lib/libcxl.txt | 1 +
> > cxl/lib/libcxl.c | 120 +++++++++++++++++++++++++++++++
> > cxl/lib/libcxl.sym | 23 ++++++
> > cxl/lib/private.h | 28 ++++++++
> > cxl/libcxl.h | 19 +++++
> > 5 files changed, 191 insertions(+)
>
> Hi Jonathan,
>
> Sorry for the delay in getting to these - a few small comments below.
>
> >
> > diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
> > index fd2962a..dec3641 100644
> > --- a/Documentation/cxl/lib/libcxl.txt
> > +++ b/Documentation/cxl/lib/libcxl.txt
> > @@ -121,6 +121,7 @@ information this call requires root / CAP_SYS_ADMIN.
> > struct cxl_cmd *cxl_cmd_new_raw(struct cxl_memdev *memdev, int opcode);
> > struct cxl_cmd *cxl_cmd_new_identify(struct cxl_memdev *memdev);
> > struct cxl_cmd *cxl_cmd_new_get_health_info(struct cxl_memdev *memdev);
> > +struct cxl_cmd *cxl_cmd_new_get_alert_config(struct cxl_memdev *memdev);
> > struct cxl_cmd *cxl_cmd_new_read_label(struct cxl_memdev *memdev,
> > unsigned int offset, unsigned int length);
> > struct cxl_cmd *cxl_cmd_new_write_label(struct cxl_memdev *memdev, void *buf,
> > diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> > index e8c5d44..ed5616c 100644
> > --- a/cxl/lib/libcxl.c
> > +++ b/cxl/lib/libcxl.c
> > @@ -3140,6 +3140,126 @@ do { \
> > return !!(c->field & mask); \
> > } while(0)
> >
> > +CXL_EXPORT struct cxl_cmd *cxl_cmd_new_get_alert_config(
> > + struct cxl_memdev *memdev)
> > +{
> > + return cxl_cmd_new_generic(memdev, CXL_MEM_COMMAND_ID_GET_ALERT_CONFIG);
> > +}
> > +
> > +#define cmd_alert_get_valid_alerts_field(c, m) \
> > + cmd_get_field_u8_mask(c, get_alert_config, GET_ALERT_CONFIG, valid_alerts, m)
> > +
> > +CXL_EXPORT int cxl_cmd_alert_config_get_life_used_prog_warn_threshold_valid(struct cxl_cmd *cmd)
>
> Generally throughout these patches, because of the (unavoidably) long
> names, these lines are well over 80 chars. For cases where a #define
> for example can't be broken down further, that's okay. But a lot of
> these can be split. We have a .clang-format for the project, and that
> should do the right thing in all these cases.
>
> <..>
> >
> > diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> > index 8bb91e0..7c1e261 100644
> > --- a/cxl/lib/libcxl.sym
> > +++ b/cxl/lib/libcxl.sym
> > @@ -217,3 +217,26 @@ global:
> > cxl_decoder_get_max_available_extent;
> > cxl_decoder_get_region;
> > } LIBCXL_2;
> > +
> > +LIBCXL_4 {
> > +global:
> > + cxl_cmd_new_get_alert_config;
> > + cxl_cmd_alert_config_get_life_used_prog_warn_threshold_valid;
> > + cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_valid;
> > + cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_valid;
> > + cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_valid;
> > + cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_valid;
>
> I think for all the 'valid' accessors, the 'get' can be dropped.
> cxl_cmd_alert_config_<something>_valid is a reasonable API that asks
> whether something is valid.
>
> > + cxl_cmd_alert_config_get_life_used_prog_warn_threshold_prog;
> > + cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold_prog;
> > + cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold_prog;
> > + cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold_prog;
> > + cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold_prog;
Only realized this after hitting send - same comment here about the
double 'prog', 'pmem' etc (from my patch 2 email) here. Additionally
the 'get' can be dropped since the responds with a boolean. So this can
be:
cxl_cmd_alert_config_corrected_pmem_error_prog_warn_threshold_writable
> > + cxl_cmd_alert_config_get_life_used_crit_alert_threshold;
> > + cxl_cmd_alert_config_get_life_used_prog_warn_threshold;
> > + cxl_cmd_alert_config_get_dev_over_temp_crit_alert_threshold;
> > + cxl_cmd_alert_config_get_dev_under_temp_crit_alert_threshold;
> > + cxl_cmd_alert_config_get_dev_over_temp_prog_warn_threshold;
> > + cxl_cmd_alert_config_get_dev_under_temp_prog_warn_threshold;
> > + cxl_cmd_alert_config_get_corr_vol_mem_err_prog_warn_threshold;
> > + cxl_cmd_alert_config_get_corr_pers_mem_err_prog_warn_threshold;
>
> These are okay to have the get, since we are getting a threshold value.
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-21 7:35 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-03 23:23 [ndctl PATCH 0/2] add support for CCI command Get Alert Configuration Jonathan Zhang
2022-10-03 23:23 ` [PATCH 1/2] libcxl: add accessors for Get Alert Configuration CCI command output Jonathan Zhang
2022-10-21 7:30 ` Verma, Vishal L
2022-10-21 7:35 ` Verma, Vishal L
2022-10-03 23:23 ` [PATCH 2/2] cxl: display alert configuratin fields in list command Jonathan Zhang
2022-10-21 7:30 ` Verma, Vishal L
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox