NVDIMM Device and Persistent Memory development
 help / color / mirror / Atom feed
* [ndctl PATCH v3 0/3] Fix accessors for negative fields and error checking for health info
       [not found] <CGME20230810082057epcas2p2978eb4ca2b1665b99fa5f84518d1b5c7@epcas2p2.samsung.com>
@ 2023-08-10  8:23 ` Jehoon Park
       [not found]   ` <CGME20230810082108epcas2p49dce460a55b44ece76c8df4122313514@epcas2p4.samsung.com>
                     ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jehoon Park @ 2023-08-10  8:23 UTC (permalink / raw
  To: linux-cxl
  Cc: nvdimm, Alison Schofield, Vishal Verma, Ira Weiny, Dan Williams,
	Dave Jiang, Davidlohr Bueso, Jonathan Cameron, Kyungsan Kim,
	Junhyeok Im, Jehoon Park

In CXL 3.0 SPEC, 8.2.9.8.3.1 and 8.2.9.8.3.2 define temperature fields
as a 2's complement value. However, they are retrieved by the same accessor
for unsigned value. This causes inaccuracy when the value is negative.

The first patch updates the pre-defined value for device temperature field of
the Get Health Info command when it is not implemented. (CXL 2.0 Errata F38)

The second patch fixes accessors for temperature fields.
Add a new payload accessor for a signed value, then use it for retrieving
temperature properly. INT_MAX is used to indicate errors because negative
errno codes are not distinguishable from the retrieved values when they are
negative. Caller should check errno to know what kind of error occurs.

The third patch fixes the error checking logic when listing device's health
info.

Changes in v3:
- Correct the revision history in the patch description (Jonathan)
- Add review tag (Jonathan)
- Revert unrelated change (Jonathan)
- Move caller side change to proper patch (Jonathan)
- Link to v2: https://lore.kernel.org/r/20230807063549.5942-1-jehoon.park@samsung.com/

Changes in v2:
- Rebase on the latest pending branch
- Remove dbg() messages in libcxl accessors (Vishal)
- Make signed value accessors to return INT_MAX when error occurs and set
  errno as proper errno codes (Vishal)
- Use proper value for checking "life_used" and "device_temperature" fields
  are implemented
- Link to v1: https://lore.kernel.org/r/20230717062908.8292-1-jehoon.park@samsung.com/

Jehoon Park (3):
  libcxl: Update a outdated value to the latest revision
  libcxl: Fix accessors for temperature field to support negative value
  cxl/json.c: Fix the error checking logic when listing device's health
    info

 cxl/json.c        |  9 +++++----
 cxl/lib/libcxl.c  | 30 +++++++++++++++++++++---------
 cxl/lib/private.h |  2 +-
 3 files changed, 27 insertions(+), 14 deletions(-)


base-commit: a871e6153b11fe63780b37cdcb1eb347b296095c
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [ndctl PATCH v3 1/3] libcxl: Update a outdated value to the latest revision
       [not found]   ` <CGME20230810082108epcas2p49dce460a55b44ece76c8df4122313514@epcas2p4.samsung.com>
@ 2023-08-10  8:23     ` Jehoon Park
  0 siblings, 0 replies; 4+ messages in thread
From: Jehoon Park @ 2023-08-10  8:23 UTC (permalink / raw
  To: linux-cxl
  Cc: nvdimm, Alison Schofield, Vishal Verma, Ira Weiny, Dan Williams,
	Dave Jiang, Davidlohr Bueso, Jonathan Cameron, Kyungsan Kim,
	Junhyeok Im, Jehoon Park

Update the predefined value for device temperature field when it is not
implemented. (Revised in CXL 2.0 Errata F38)

Signed-off-by: Jehoon Park <jehoon.park@samsung.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 cxl/lib/private.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cxl/lib/private.h b/cxl/lib/private.h
index a641727..a692fd5 100644
--- a/cxl/lib/private.h
+++ b/cxl/lib/private.h
@@ -360,7 +360,7 @@ struct cxl_cmd_set_partition {
 #define CXL_CMD_HEALTH_INFO_EXT_CORRECTED_PERSISTENT_WARNING		(1)
 
 #define CXL_CMD_HEALTH_INFO_LIFE_USED_NOT_IMPL				0xff
-#define CXL_CMD_HEALTH_INFO_TEMPERATURE_NOT_IMPL			0xffff
+#define CXL_CMD_HEALTH_INFO_TEMPERATURE_NOT_IMPL			0x7fff
 
 static inline int check_kmod(struct kmod_ctx *kmod_ctx)
 {
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [ndctl PATCH v3 2/3] libcxl: Fix accessors for temperature field to support negative value
       [not found]   ` <CGME20230810082115epcas2p31c2f26887e54c9fdeab0215bbde49f0a@epcas2p3.samsung.com>
@ 2023-08-10  8:23     ` Jehoon Park
  0 siblings, 0 replies; 4+ messages in thread
From: Jehoon Park @ 2023-08-10  8:23 UTC (permalink / raw
  To: linux-cxl
  Cc: nvdimm, Alison Schofield, Vishal Verma, Ira Weiny, Dan Williams,
	Dave Jiang, Davidlohr Bueso, Jonathan Cameron, Kyungsan Kim,
	Junhyeok Im, Jehoon Park

Add a new macro function to retrieve a signed value such as a temperature.
Modify accessors for signed value to return INT_MAX when error occurs and
set errno to corresponding errno codes.
Fix the error checking value of the temperature accessor in cxl/json.c.

Signed-off-by: Jehoon Park <jehoon.park@samsung.com>
---
 cxl/json.c       |  2 +-
 cxl/lib/libcxl.c | 30 +++++++++++++++++++++---------
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/cxl/json.c b/cxl/json.c
index 7678d02..89e5fd0 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -246,7 +246,7 @@ static struct json_object *util_cxl_memdev_health_to_json(
 	}
 
 	field = cxl_cmd_health_info_get_temperature(cmd);
-	if (field != 0xffff) {
+	if (field != INT_MAX) {
 		jobj = json_object_new_int(field);
 		if (jobj)
 			json_object_object_add(jhealth, "temperature", jobj);
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index af4ca44..53c9b25 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -3661,11 +3661,23 @@ cxl_cmd_alert_config_get_life_used_prog_warn_threshold(struct cxl_cmd *cmd)
 			 life_used_prog_warn_threshold);
 }
 
+#define cmd_get_field_s16(cmd, n, N, field)				\
+do {									\
+	struct cxl_cmd_##n *c =						\
+		(struct cxl_cmd_##n *)cmd->send_cmd->out.payload;	\
+	int rc = cxl_cmd_validate_status(cmd, CXL_MEM_COMMAND_ID_##N);	\
+	if (rc)	{							\
+		errno = -rc;						\
+		return INT_MAX;						\
+	}								\
+	return (int16_t)le16_to_cpu(c->field);				\
+} while(0)
+
 CXL_EXPORT int
 cxl_cmd_alert_config_get_dev_over_temperature_crit_alert_threshold(
 	struct cxl_cmd *cmd)
 {
-	cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG,
+	cmd_get_field_s16(cmd, get_alert_config, GET_ALERT_CONFIG,
 			  dev_over_temperature_crit_alert_threshold);
 }
 
@@ -3673,7 +3685,7 @@ CXL_EXPORT int
 cxl_cmd_alert_config_get_dev_under_temperature_crit_alert_threshold(
 	struct cxl_cmd *cmd)
 {
-	cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG,
+	cmd_get_field_s16(cmd, get_alert_config, GET_ALERT_CONFIG,
 			  dev_under_temperature_crit_alert_threshold);
 }
 
@@ -3681,7 +3693,7 @@ CXL_EXPORT int
 cxl_cmd_alert_config_get_dev_over_temperature_prog_warn_threshold(
 	struct cxl_cmd *cmd)
 {
-	cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG,
+	cmd_get_field_s16(cmd, get_alert_config, GET_ALERT_CONFIG,
 			  dev_over_temperature_prog_warn_threshold);
 }
 
@@ -3689,7 +3701,7 @@ CXL_EXPORT int
 cxl_cmd_alert_config_get_dev_under_temperature_prog_warn_threshold(
 	struct cxl_cmd *cmd)
 {
-	cmd_get_field_u16(cmd, get_alert_config, GET_ALERT_CONFIG,
+	cmd_get_field_s16(cmd, get_alert_config, GET_ALERT_CONFIG,
 			  dev_under_temperature_prog_warn_threshold);
 }
 
@@ -3914,7 +3926,7 @@ CXL_EXPORT int cxl_cmd_health_info_get_life_used(struct cxl_cmd *cmd)
 
 static int health_info_get_temperature_raw(struct cxl_cmd *cmd)
 {
-	cmd_get_field_u16(cmd, get_health_info, GET_HEALTH_INFO,
+	cmd_get_field_s16(cmd, get_health_info, GET_HEALTH_INFO,
 				 temperature);
 }
 
@@ -3922,10 +3934,10 @@ CXL_EXPORT int cxl_cmd_health_info_get_temperature(struct cxl_cmd *cmd)
 {
 	int rc = health_info_get_temperature_raw(cmd);
 
-	if (rc < 0)
-		return rc;
-	if (rc == CXL_CMD_HEALTH_INFO_TEMPERATURE_NOT_IMPL)
-		return -EOPNOTSUPP;
+	if (rc == CXL_CMD_HEALTH_INFO_TEMPERATURE_NOT_IMPL) {
+		errno = EOPNOTSUPP;
+		return INT_MAX;
+	}
 	return rc;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [ndctl PATCH v3 3/3] cxl/json.c: Fix the error checking logic when listing device's health info
       [not found]   ` <CGME20230810082121epcas2p3eb7acc9bdf13cafe9ef79a6fdd06b679@epcas2p3.samsung.com>
@ 2023-08-10  8:23     ` Jehoon Park
  0 siblings, 0 replies; 4+ messages in thread
From: Jehoon Park @ 2023-08-10  8:23 UTC (permalink / raw
  To: linux-cxl
  Cc: nvdimm, Alison Schofield, Vishal Verma, Ira Weiny, Dan Williams,
	Dave Jiang, Davidlohr Bueso, Jonathan Cameron, Kyungsan Kim,
	Junhyeok Im, Jehoon Park

Change the variable type for storing result of life used field accessor
: u32 to int.
Fix the value for checking device's life used field is implemented.

Signed-off-by: Jehoon Park <jehoon.park@samsung.com>
---
 cxl/json.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/cxl/json.c b/cxl/json.c
index 89e5fd0..102bfaf 100644
--- a/cxl/json.c
+++ b/cxl/json.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 // Copyright (C) 2015-2021 Intel Corporation. All rights reserved.
+#include <errno.h>
 #include <limits.h>
 #include <util/json.h>
 #include <uuid/uuid.h>
@@ -238,9 +239,9 @@ static struct json_object *util_cxl_memdev_health_to_json(
 		json_object_object_add(jhealth, "ext_corrected_persistent", jobj);
 
 	/* other fields */
-	field = cxl_cmd_health_info_get_life_used(cmd);
-	if (field != 0xff) {
-		jobj = json_object_new_int(field);
+	rc = cxl_cmd_health_info_get_life_used(cmd);
+	if (rc != -EOPNOTSUPP) {
+		jobj = json_object_new_int(rc);
 		if (jobj)
 			json_object_object_add(jhealth, "life_used_percent", jobj);
 	}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-08-10  8:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20230810082057epcas2p2978eb4ca2b1665b99fa5f84518d1b5c7@epcas2p2.samsung.com>
2023-08-10  8:23 ` [ndctl PATCH v3 0/3] Fix accessors for negative fields and error checking for health info Jehoon Park
     [not found]   ` <CGME20230810082108epcas2p49dce460a55b44ece76c8df4122313514@epcas2p4.samsung.com>
2023-08-10  8:23     ` [ndctl PATCH v3 1/3] libcxl: Update a outdated value to the latest revision Jehoon Park
     [not found]   ` <CGME20230810082115epcas2p31c2f26887e54c9fdeab0215bbde49f0a@epcas2p3.samsung.com>
2023-08-10  8:23     ` [ndctl PATCH v3 2/3] libcxl: Fix accessors for temperature field to support negative value Jehoon Park
     [not found]   ` <CGME20230810082121epcas2p3eb7acc9bdf13cafe9ef79a6fdd06b679@epcas2p3.samsung.com>
2023-08-10  8:23     ` [ndctl PATCH v3 3/3] cxl/json.c: Fix the error checking logic when listing device's health info Jehoon Park

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).