diff mbox series

[ndctl,1/3] libndctl, intel: Add infrastructure for firmware_status translation

Message ID 20190111225339.6939-2-vishal.l.verma@intel.com (mailing list archive)
State Superseded
Headers show
Series Add missing firmware_status checks | expand

Commit Message

Verma, Vishal L Jan. 11, 2019, 10:53 p.m. UTC 
Add a new routine to ndctl_dimm_ops that allows a DSM family to provide
a translation routine that will translate the status codes of the result
of a DSM to generic errno style error codes. To use this routine
effectively, add a new wrapper around ndctl_cmd_submit (called
ndctl_cmd_submit_xlat) that submits the command, and also runs it
through the above translator dimm_op (if one is is defined).

Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
---
 ndctl/lib/intel.c      | 49 ++++++++++++++++++++++++++++++++++++++++++
 ndctl/lib/intel.h      |  1 +
 ndctl/lib/libndctl.c   | 26 ++++++++++++++++++++++
 ndctl/lib/libndctl.sym |  6 ++++++
 ndctl/lib/private.h    |  1 +
 ndctl/libndctl.h       |  3 +++
 6 files changed, 86 insertions(+)

Comments

Dan Williams Jan. 12, 2019, 12:19 a.m. UTC | #1 
On Fri, Jan 11, 2019 at 2:53 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
>
> Add a new routine to ndctl_dimm_ops that allows a DSM family to provide
> a translation routine that will translate the status codes of the result
> of a DSM to generic errno style error codes. To use this routine
> effectively, add a new wrapper around ndctl_cmd_submit (called
> ndctl_cmd_submit_xlat) that submits the command, and also runs it
> through the above translator dimm_op (if one is is defined).
>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
[..]
> +NDCTL_EXPORT int ndctl_cmd_submit_xlat(struct ndctl_cmd *cmd)
> +{
> +       int rc;
> +
> +       rc = ndctl_cmd_submit(cmd);
> +       if (rc < 0)
> +               return rc;
> +
> +       /*
> +        * NOTE: This loses a positive rc which happens in the case of a
> +        * buffer underrun. If the caller cares about that (usually not very
> +        * useful), then the xlat function is available separately as well.
> +        */

Hmm, yes not very useful in general but if
ndctl_cmd_xlat_firmware_status() returns 0, why not preserve the
positive value if available? Just to keep the 2 interfaces more
closely aligned.
Verma, Vishal L Jan. 12, 2019, 12:24 a.m. UTC | #2 
On Fri, 2019-01-11 at 16:19 -0800, Dan Williams wrote:
> On Fri, Jan 11, 2019 at 2:53 PM Vishal Verma <vishal.l.verma@intel.com> wrote:
> > 
> > Add a new routine to ndctl_dimm_ops that allows a DSM family to provide
> > a translation routine that will translate the status codes of the result
> > of a DSM to generic errno style error codes. To use this routine
> > effectively, add a new wrapper around ndctl_cmd_submit (called
> > ndctl_cmd_submit_xlat) that submits the command, and also runs it
> > through the above translator dimm_op (if one is is defined).
> > 
> > Cc: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com>
> 
> [..]
> > +NDCTL_EXPORT int ndctl_cmd_submit_xlat(struct ndctl_cmd *cmd)
> > +{
> > +       int rc;
> > +
> > +       rc = ndctl_cmd_submit(cmd);
> > +       if (rc < 0)
> > +               return rc;
> > +
> > +       /*
> > +        * NOTE: This loses a positive rc which happens in the case of a
> > +        * buffer underrun. If the caller cares about that (usually not very
> > +        * useful), then the xlat function is available separately as well.
> > +        */
> 
> Hmm, yes not very useful in general but if
> ndctl_cmd_xlat_firmware_status() returns 0, why not preserve the
> positive value if available? Just to keep the 2 interfaces more
> closely aligned.

Good idea. We'd still lose the positive rc if there was an underrun &&
firmware_status was non-zero, but that is fine (I'll still keep this
note for that case).
diff mbox series

Patch

diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
index 744386f..d684bac 100644
--- a/ndctl/lib/intel.c
+++ b/ndctl/lib/intel.c
@@ -16,6 +16,54 @@ 
 #include <ndctl/libndctl.h>
 #include "private.h"
 
+static int intel_cmd_xlat_firmware_status(struct ndctl_cmd *cmd)
+{
+	struct nd_pkg_intel *pkg = cmd->intel;
+	unsigned int status, ext_status;
+
+	status = (*cmd->firmware_status) & ND_INTEL_STATUS_MASK;
+	ext_status = (*cmd->firmware_status) & ND_INTEL_STATUS_EXTEND_MASK;
+
+	/* Common statuses */
+	switch (status) {
+	case ND_INTEL_STATUS_SUCCESS:
+		return 0;
+	case ND_INTEL_STATUS_NOTSUPP:
+		return -EOPNOTSUPP;
+	case ND_INTEL_STATUS_NOTEXIST:
+		return -ENXIO;
+	case ND_INTEL_STATUS_INVALPARM:
+		return -EINVAL;
+	case ND_INTEL_STATUS_HWERR:
+		return -EIO;
+	case ND_INTEL_STATUS_RETRY:
+		return -EAGAIN;
+	case ND_INTEL_STATUS_EXTEND:
+		/* refer to extended status, break out of this */
+		break;
+	case ND_INTEL_STATUS_NORES:
+		return -EAGAIN;
+	case ND_INTEL_STATUS_NOTREADY:
+		return -EBUSY;
+	}
+
+	/* Extended status is command specific */
+	switch (pkg->gen.nd_command) {
+	case ND_INTEL_SMART:
+	case ND_INTEL_SMART_THRESHOLD:
+	case ND_INTEL_SMART_SET_THRESHOLD:
+		/* ext status not specified */
+		break;
+	case ND_INTEL_SMART_INJECT:
+		/* smart injection not enabled */
+		if (ext_status == ND_INTEL_STATUS_INJ_DISABLED)
+			return -ENXIO;
+		break;
+	}
+
+	return -ENOMSG;
+}
+
 static struct ndctl_cmd *alloc_intel_cmd(struct ndctl_dimm *dimm,
 		unsigned func, size_t in_size, size_t out_size)
 {
@@ -780,4 +828,5 @@  struct ndctl_dimm_ops * const intel_dimm_ops = &(struct ndctl_dimm_ops) {
 	.fw_xlat_firmware_status = intel_cmd_fw_xlat_firmware_status,
 	.new_ack_shutdown_count = intel_dimm_cmd_new_lss,
 	.fw_update_supported = intel_dimm_fw_update_supported,
+	.xlat_firmware_status = intel_cmd_xlat_firmware_status,
 };
diff --git a/ndctl/lib/intel.h b/ndctl/lib/intel.h
index 3b01bba..530c996 100644
--- a/ndctl/lib/intel.h
+++ b/ndctl/lib/intel.h
@@ -179,5 +179,6 @@  struct nd_pkg_intel {
 #define ND_INTEL_STATUS_FQ_BUSY		0x20000
 #define ND_INTEL_STATUS_FQ_BAD		0x30000
 #define ND_INTEL_STATUS_FQ_ORDER	0x40000
+#define ND_INTEL_STATUS_INJ_DISABLED	0x10000
 
 #endif /* __INTEL_H__ */
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index e82a08d..5f1fd84 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2704,6 +2704,16 @@  static const char *ndctl_dimm_get_cmd_subname(struct ndctl_cmd *cmd)
 	return ops->cmd_desc(cmd->pkg->nd_command);
 }
 
+NDCTL_EXPORT int ndctl_cmd_xlat_firmware_status(struct ndctl_cmd *cmd)
+{
+	struct ndctl_dimm *dimm = cmd->dimm;
+	struct ndctl_dimm_ops *ops = dimm ? dimm->ops : NULL;
+
+	if (!dimm || !ops || !ops->xlat_firmware_status)
+		return -ENOMSG;
+	return ops->xlat_firmware_status(cmd);
+}
+
 static int do_cmd(int fd, int ioctl_cmd, struct ndctl_cmd *cmd)
 {
 	int rc;
@@ -2819,6 +2829,22 @@  NDCTL_EXPORT int ndctl_cmd_submit(struct ndctl_cmd *cmd)
 	return rc;
 }
 
+NDCTL_EXPORT int ndctl_cmd_submit_xlat(struct ndctl_cmd *cmd)
+{
+	int rc;
+
+	rc = ndctl_cmd_submit(cmd);
+	if (rc < 0)
+		return rc;
+
+	/*
+	 * NOTE: This loses a positive rc which happens in the case of a
+	 * buffer underrun. If the caller cares about that (usually not very
+	 * useful), then the xlat function is available separately as well.
+	 */
+	return ndctl_cmd_xlat_firmware_status(cmd);
+}
+
 NDCTL_EXPORT int ndctl_cmd_get_status(struct ndctl_cmd *cmd)
 {
 	return cmd->status;
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 6c4c8b4..275db92 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -385,3 +385,9 @@  global:
 	ndctl_namespace_get_next_badblock;
 	ndctl_dimm_get_dirty_shutdown;
 } LIBNDCTL_17;
+
+LIBNDCTL_19 {
+global:
+	ndctl_cmd_xlat_firmware_status;
+	ndctl_cmd_submit_xlat;
+} LIBNDCTL_18;
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 4fbea56..a387b0b 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -338,6 +338,7 @@  struct ndctl_dimm_ops {
 	enum ND_FW_STATUS (*fw_xlat_firmware_status)(struct ndctl_cmd *);
 	struct ndctl_cmd *(*new_ack_shutdown_count)(struct ndctl_dimm *);
 	int (*fw_update_supported)(struct ndctl_dimm *);
+	int (*xlat_firmware_status)(struct ndctl_cmd *);
 };
 
 struct ndctl_dimm_ops * const intel_dimm_ops;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index c81cc03..e55a593 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -681,6 +681,9 @@  enum ND_FW_STATUS ndctl_cmd_fw_xlat_firmware_status(struct ndctl_cmd *cmd);
 struct ndctl_cmd *ndctl_dimm_cmd_new_ack_shutdown_count(struct ndctl_dimm *dimm);
 int ndctl_dimm_fw_update_supported(struct ndctl_dimm *dimm);
 
+int ndctl_cmd_xlat_firmware_status(struct ndctl_cmd *cmd);
+int ndctl_cmd_submit_xlat(struct ndctl_cmd *cmd);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif