[PATCH 1/2] ndctl: add support to enable latch system shutdown status

[PATCH 1/2] ndctl: add support to enable latch system shutdown status

[Dave Jiang]

Adding the Enable Latch System Shutdown Status (Function Index 10) for
DSM v1.6 spec.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 ndctl/lib/Makefile.am  |    1 +
 ndctl/lib/intel.c      |   38 ++++++++++++++++++++++++++++++++++++++
 ndctl/lib/intel.h      |    7 +++++++
 ndctl/lib/libndctl.sym |    2 ++
 ndctl/lib/lss.c        |   43 +++++++++++++++++++++++++++++++++++++++++++
 ndctl/lib/private.h    |    2 ++
 6 files changed, 93 insertions(+)
 create mode 100644 ndctl/lib/lss.c

diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
index e3a12e7..c1ea371 100644
--- a/ndctl/lib/Makefile.am
+++ b/ndctl/lib/Makefile.am
@@ -22,6 +22,7 @@ libndctl_la_SOURCES =\
 	msft.c \
 	ars.c \
 	firmware.c \
+	lss.c \
 	libndctl.c
 
 libndctl_la_LIBADD =\
diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
index 6d26a6c..10a0881 100644
--- a/ndctl/lib/intel.c
+++ b/ndctl/lib/intel.c
@@ -547,6 +547,42 @@ static unsigned long intel_cmd_fw_fquery_get_fw_rev(struct ndctl_cmd *cmd)
 	return cmd->intel->fquery.updated_fw_rev;
 }
 
+static struct ndctl_cmd *
+intel_dimm_cmd_new_lss_enable(struct ndctl_dimm *dimm)
+{
+	struct ndctl_cmd *cmd;
+
+	BUILD_ASSERT(sizeof(struct nd_intel_lss) == 5);
+
+	cmd = alloc_intel_cmd(dimm, ND_INTEL_ENABLE_LSS_STATUS, 1, 4);
+	if (!cmd)
+		return NULL;
+
+	cmd->firmware_status = &cmd->intel->lss.status;
+	return cmd;
+}
+
+static int intel_lss_set_valid(struct ndctl_cmd *cmd)
+{
+	struct nd_pkg_intel *pkg = cmd->intel;
+
+	if (cmd->type != ND_CMD_CALL || cmd->status != 1
+			|| pkg->gen.nd_family != NVDIMM_FAMILY_INTEL
+			|| pkg->gen.nd_command != ND_INTEL_ENABLE_LSS_STATUS)
+		return -EINVAL;
+	return 0;
+}
+
+static int
+intel_cmd_lss_set_enable(struct ndctl_cmd *cmd, unsigned char enable)
+{
+	if (intel_lss_set_valid(cmd) < 0)
+		return -EINVAL;
+	cmd->intel->lss.enable = enable;
+	return 0;
+}
+
+
 struct ndctl_dimm_ops * const intel_dimm_ops = &(struct ndctl_dimm_ops) {
 	.cmd_desc = intel_cmd_desc,
 	.new_smart = intel_dimm_cmd_new_smart,
@@ -601,4 +637,6 @@ struct ndctl_dimm_ops * const intel_dimm_ops = &(struct ndctl_dimm_ops) {
 	.new_fw_finish_query = intel_dimm_cmd_new_fw_finish_query,
 	.fw_fquery_set_context = intel_cmd_fw_fquery_set_context,
 	.fw_fquery_get_fw_rev = intel_cmd_fw_fquery_get_fw_rev,
+	.new_lss_enable = intel_dimm_cmd_new_lss_enable,
+	.lss_set_enable = intel_cmd_lss_set_enable,
 };
diff --git a/ndctl/lib/intel.h b/ndctl/lib/intel.h
index 080e37b..92bed53 100644
--- a/ndctl/lib/intel.h
+++ b/ndctl/lib/intel.h
@@ -6,6 +6,7 @@
 #define ND_INTEL_SMART 1
 #define ND_INTEL_SMART_THRESHOLD 2
 
+#define ND_INTEL_ENABLE_LSS_STATUS 10
 #define ND_INTEL_FW_GET_INFO 12
 #define ND_INTEL_FW_START_UPDATE 13
 #define ND_INTEL_FW_SEND_DATA 14
@@ -118,6 +119,11 @@ struct nd_intel_fw_finish_query {
 	__u64 updated_fw_rev;
 } __attribute__((packed));
 
+struct nd_intel_lss {
+	__u8 enable;
+	__u32 status;
+} __attribute__((packed));
+
 struct nd_pkg_intel {
 	struct nd_cmd_pkg gen;
 	union {
@@ -129,6 +135,7 @@ struct nd_pkg_intel {
 		struct nd_intel_fw_send_data send;
 		struct nd_intel_fw_finish_update finish;
 		struct nd_intel_fw_finish_query fquery;
+		struct nd_intel_lss lss;
 	};
 };
 #endif /* __INTEL_H__ */
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 2e248ab..65673ad 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -344,4 +344,6 @@ global:
 	ndctl_cmd_fw_finish_set_ctrl_flags;
 	ndctl_cmd_fw_finish_set_context;
 	ndctl_cmd_fw_fquery_set_context;
+	ndctl_dimm_cmd_new_lss_enable;
+	ndctl_cmd_lss_set_enable;
 } LIBNDCTL_13;
diff --git a/ndctl/lib/lss.c b/ndctl/lib/lss.c
new file mode 100644
index 0000000..fbeeec5
--- /dev/null
+++ b/ndctl/lib/lss.c
@@ -0,0 +1,43 @@
+/*
+ * Copyright (c) 2017, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU Lesser General Public License,
+ * version 2.1, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for
+ * more details.
+ */
+#include <stdlib.h>
+#include <limits.h>
+#include <util/log.h>
+#include <ndctl/libndctl.h>
+#include "private.h"
+
+/*
+ * Define the wrappers around the ndctl_dimm_ops for LSS DSM
+ */
+NDCTL_EXPORT struct ndctl_cmd *
+ndctl_dimm_cmd_new_lss_enable(struct ndctl_dimm *dimm)
+{
+	struct ndctl_dimm_ops *ops = dimm->ops;
+
+	if (ops && ops->new_lss_enable)
+		return ops->new_lss_enable(dimm);
+	else
+		return NULL;
+}
+
+NDCTL_EXPORT int
+ndctl_cmd_lss_set_enable(struct ndctl_cmd *cmd, unsigned char enable)
+{
+	if (cmd->dimm) {
+		struct ndctl_dimm_ops *ops = cmd->dimm->ops;
+
+		if (ops && ops->lss_set_enable)
+			return ops->lss_set_enable(cmd, enable);
+	}
+	return -ENXIO;
+}
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 20f9e6e..4035e11 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -325,6 +325,8 @@ struct ndctl_dimm_ops {
 	struct ndctl_cmd *(*new_fw_finish_query)(struct ndctl_dimm *);
 	int (*fw_fquery_set_context)(struct ndctl_cmd *, unsigned int context);
 	unsigned long (*fw_fquery_get_fw_rev)(struct ndctl_cmd *);
+	struct ndctl_cmd *(*new_lss_enable)(struct ndctl_dimm *);
+	int (*lss_set_enable)(struct ndctl_cmd *, unsigned char enable);
 };
 
 struct ndctl_dimm_ops * const intel_dimm_ops;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 64a4e99..24dc7a3 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -607,6 +607,9 @@ int ndctl_cmd_fw_finish_set_ctrl_flags(struct ndctl_cmd *cmd, unsigned char flag
 int ndctl_cmd_fw_finish_set_context(struct ndctl_cmd *cmd, unsigned int context);
 int ndctl_cmd_fw_fquery_set_context(struct ndctl_cmd *cmd, unsigned int context);
 
+struct ndctl_cmd *ndctl_dimm_cmd_new_lss_enable(struct ndctl_dimm *dimm);
+int ndctl_cmd_lss_set_enable(struct ndctl_cmd *cmd, unsigned char enable);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

[PATCH 2/2] ndctl: lss latch unit test

[Dave Jiang]

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 0 files changed

diff --git a/test/Makefile.am b/test/Makefile.am
index d5ef648..7f70e63 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -17,7 +17,8 @@ TESTS =\
 	sector-mode.sh \
 	inject-error.sh \
 	btt-errors.sh \
-	firmware-update.sh
+	firmware-update.sh \
+	lss-status-set
 
 check_PROGRAMS =\
 	libndctl \
@@ -28,7 +29,8 @@ check_PROGRAMS =\
 	dax-errors \
 	smart-notify \
 	smart-listen \
-	daxdev-errors
+	daxdev-errors \
+	lss-status-set
 
 if ENABLE_DESTRUCTIVE
 TESTS +=\
@@ -67,6 +69,12 @@ dsm_fail_SOURCES =\
 
 dsm_fail_LDADD = $(LIBNDCTL_LIB) $(KMOD_LIBS)
 
+lss_status_set_SOURCES =\
+	lss-status-set.c \
+	$(testcore)
+
+lss_status_set_LDADD = $(LIBNDCTL_LIB) $(KMOD_LIBS)
+
 blk_ns_SOURCES = blk_namespaces.c $(testcore)
 blk_ns_LDADD = $(LIBNDCTL_LIB) $(KMOD_LIBS)
 
diff --git a/test/lss-status-set.c b/test/lss-status-set.c
new file mode 100644
index 0000000..5056a8f
--- /dev/null
+++ b/test/lss-status-set.c
@@ -0,0 +1,157 @@
+/*
+ * Copyright (c) 2017, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU Lesser General Public License,
+ * version 2.1, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT ANY
+ * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+ * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for
+ * more details.
+ */
+#include <stdio.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <string.h>
+#include <ctype.h>
+#include <errno.h>
+#include <syslog.h>
+#include <libkmod.h>
+#include <util/log.h>
+#include <util/sysfs.h>
+#include <linux/version.h>
+
+#include <ccan/array_size/array_size.h>
+#include <ndctl/libndctl.h>
+#ifdef HAVE_NDCTL_H
+#include <linux/ndctl.h>
+#else
+#include <ndctl.h>
+#endif
+#include <test.h>
+
+static int test_dimm(struct ndctl_dimm *dimm)
+{
+	struct ndctl_cmd *cmd;
+	int rc = 0;
+
+	cmd = ndctl_dimm_cmd_new_lss_enable(dimm);
+	if (!cmd)
+		return -ENOMEM;
+
+	rc = ndctl_cmd_lss_set_enable(cmd, 1);
+	if (rc < 0)
+		goto out;
+
+	rc = ndctl_cmd_submit(cmd);
+	if (rc < 0)
+		goto out;
+
+	rc = ndctl_cmd_get_firmware_status(cmd);
+	if (rc != 0) {
+		fprintf(stderr, "dimm %s LSS enable set failed\n",
+				ndctl_dimm_get_devname(dimm));
+		goto out;
+	}
+
+	printf("DIMM %s LSS enable set\n", ndctl_dimm_get_devname(dimm));
+
+out:
+	ndctl_cmd_unref(cmd);
+	return rc;
+}
+
+static void reset_bus(struct ndctl_bus *bus)
+{
+	struct ndctl_region *region;
+	struct ndctl_dimm *dimm;
+
+	/* disable all regions so that set_config_data commands are permitted */
+	ndctl_region_foreach(bus, region)
+		ndctl_region_disable_invalidate(region);
+
+	ndctl_dimm_foreach(bus, dimm)
+		ndctl_dimm_zero_labels(dimm);
+}
+
+static int do_test(struct ndctl_ctx *ctx, struct ndctl_test *test)
+{
+	struct ndctl_bus *bus = ndctl_bus_get_by_provider(ctx, "nfit_test.0");
+	struct ndctl_dimm *dimm;
+	struct ndctl_region *region;
+	struct log_ctx log_ctx;
+	int rc = 0;
+
+	if (!ndctl_test_attempt(test, KERNEL_VERSION(4, 15, 0)))
+		return 77;
+
+	if (!bus)
+		return -ENXIO;
+
+	log_init(&log_ctx, "test/lss-status-set", "NDCTL_TEST");
+
+	ndctl_bus_wait_probe(bus);
+
+	ndctl_region_foreach(bus, region)
+		ndctl_region_disable_invalidate(region);
+
+	ndctl_dimm_foreach(bus, dimm) {
+		fprintf(stderr, "Testing dimm: %s\n",
+				ndctl_dimm_get_devname(dimm));
+		rc = test_dimm(dimm);
+		if (rc < 0) {
+			fprintf(stderr, "dimm %s failed\n",
+				ndctl_dimm_get_devname(dimm));
+			goto out;
+		}
+	}
+
+out:
+	reset_bus(bus);
+	return rc;
+}
+
+static int test_lss_status_set(int loglevel, struct ndctl_test *test,
+		struct ndctl_ctx *ctx)
+{
+	struct kmod_module *mod;
+	struct kmod_ctx *kmod_ctx;
+	int result = EXIT_FAILURE, err;
+
+	ndctl_set_log_priority(ctx, loglevel);
+	err = nfit_test_init(&kmod_ctx, &mod, NULL, loglevel, test);
+	if (err < 0) {
+		result = 77;
+		ndctl_test_skip(test);
+		fprintf(stderr, "%s unavailable skipping tests\n",
+				"nfit_test");
+		return result;
+	}
+
+	result = do_test(ctx, test);
+	kmod_module_remove_module(mod, 0);
+
+	kmod_unref(kmod_ctx);
+	return result;
+}
+
+int main(int argc, char *argv[])
+{
+	struct ndctl_test *test = ndctl_test_new(0);
+	struct ndctl_ctx *ctx;
+	int rc;
+
+	if (!test) {
+		fprintf(stderr, "failed to initialize test\n");
+		return EXIT_FAILURE;
+	}
+
+	rc = ndctl_new(&ctx);
+	if (rc)
+		return ndctl_test_result(test, rc);
+	rc = test_lss_status_set(LOG_DEBUG, test, ctx);
+	ndctl_unref(ctx);
+
+	return ndctl_test_result(test, rc);
+}

_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH 1/2] ndctl: add support to enable latch system shutdown status

[Dan Williams]

On Wed, Dec 6, 2017 at 2:53 PM, Dave Jiang <dave.jiang@intel.com> wrote:
> Adding the Enable Latch System Shutdown Status (Function Index 10) for
> DSM v1.6 spec.

Whoops, this got missed for ndctl-v59, some small comments so we can
get this moving for v60.

>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  ndctl/lib/Makefile.am  |    1 +
>  ndctl/lib/intel.c      |   38 ++++++++++++++++++++++++++++++++++++++
>  ndctl/lib/intel.h      |    7 +++++++
>  ndctl/lib/libndctl.sym |    2 ++
>  ndctl/lib/lss.c        |   43 +++++++++++++++++++++++++++++++++++++++++++
>  ndctl/lib/private.h    |    2 ++
>  6 files changed, 93 insertions(+)
>  create mode 100644 ndctl/lib/lss.c
>
> diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
> index e3a12e7..c1ea371 100644
> --- a/ndctl/lib/Makefile.am
> +++ b/ndctl/lib/Makefile.am
> @@ -22,6 +22,7 @@ libndctl_la_SOURCES =\
>         msft.c \
>         ars.c \
>         firmware.c \
> +       lss.c \

I don't think LSS deserves its own file. Let's just add this routines
to ndctl/lib/smart.c.

>         libndctl.c
>
>  libndctl_la_LIBADD =\
> diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
> index 6d26a6c..10a0881 100644
> --- a/ndctl/lib/intel.c
> +++ b/ndctl/lib/intel.c
> @@ -547,6 +547,42 @@ static unsigned long intel_cmd_fw_fquery_get_fw_rev(struct ndctl_cmd *cmd)
>         return cmd->intel->fquery.updated_fw_rev;
>  }
>
> +static struct ndctl_cmd *
> +intel_dimm_cmd_new_lss_enable(struct ndctl_dimm *dimm)
> +{
> +       struct ndctl_cmd *cmd;
> +
> +       BUILD_ASSERT(sizeof(struct nd_intel_lss) == 5);
> +
> +       cmd = alloc_intel_cmd(dimm, ND_INTEL_ENABLE_LSS_STATUS, 1, 4);
> +       if (!cmd)
> +               return NULL;
> +
> +       cmd->firmware_status = &cmd->intel->lss.status;
> +       return cmd;
> +}
> +
> +static int intel_lss_set_valid(struct ndctl_cmd *cmd)
> +{
> +       struct nd_pkg_intel *pkg = cmd->intel;
> +
> +       if (cmd->type != ND_CMD_CALL || cmd->status != 1
> +                       || pkg->gen.nd_family != NVDIMM_FAMILY_INTEL
> +                       || pkg->gen.nd_command != ND_INTEL_ENABLE_LSS_STATUS)
> +               return -EINVAL;
> +       return 0;
> +}
> +
> +static int
> +intel_cmd_lss_set_enable(struct ndctl_cmd *cmd, unsigned char enable)
> +{
> +       if (intel_lss_set_valid(cmd) < 0)
> +               return -EINVAL;
> +       cmd->intel->lss.enable = enable;
> +       return 0;
> +}
> +
> +
>  struct ndctl_dimm_ops * const intel_dimm_ops = &(struct ndctl_dimm_ops) {
>         .cmd_desc = intel_cmd_desc,
>         .new_smart = intel_dimm_cmd_new_smart,
> @@ -601,4 +637,6 @@ struct ndctl_dimm_ops * const intel_dimm_ops = &(struct ndctl_dimm_ops) {
>         .new_fw_finish_query = intel_dimm_cmd_new_fw_finish_query,
>         .fw_fquery_set_context = intel_cmd_fw_fquery_set_context,
>         .fw_fquery_get_fw_rev = intel_cmd_fw_fquery_get_fw_rev,
> +       .new_lss_enable = intel_dimm_cmd_new_lss_enable,
> +       .lss_set_enable = intel_cmd_lss_set_enable,
>  };
> diff --git a/ndctl/lib/intel.h b/ndctl/lib/intel.h
> index 080e37b..92bed53 100644
> --- a/ndctl/lib/intel.h
> +++ b/ndctl/lib/intel.h
> @@ -6,6 +6,7 @@
>  #define ND_INTEL_SMART 1
>  #define ND_INTEL_SMART_THRESHOLD 2
>
> +#define ND_INTEL_ENABLE_LSS_STATUS 10
>  #define ND_INTEL_FW_GET_INFO 12
>  #define ND_INTEL_FW_START_UPDATE 13
>  #define ND_INTEL_FW_SEND_DATA 14
> @@ -118,6 +119,11 @@ struct nd_intel_fw_finish_query {
>         __u64 updated_fw_rev;
>  } __attribute__((packed));
>
> +struct nd_intel_lss {
> +       __u8 enable;
> +       __u32 status;
> +} __attribute__((packed));
> +
>  struct nd_pkg_intel {
>         struct nd_cmd_pkg gen;
>         union {
> @@ -129,6 +135,7 @@ struct nd_pkg_intel {
>                 struct nd_intel_fw_send_data send;
>                 struct nd_intel_fw_finish_update finish;
>                 struct nd_intel_fw_finish_query fquery;
> +               struct nd_intel_lss lss;
>         };
>  };
>  #endif /* __INTEL_H__ */
> diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
> index 2e248ab..65673ad 100644
> --- a/ndctl/lib/libndctl.sym
> +++ b/ndctl/lib/libndctl.sym
> @@ -344,4 +344,6 @@ global:
>         ndctl_cmd_fw_finish_set_ctrl_flags;
>         ndctl_cmd_fw_finish_set_context;
>         ndctl_cmd_fw_fquery_set_context;
> +       ndctl_dimm_cmd_new_lss_enable;
> +       ndctl_cmd_lss_set_enable;
>  } LIBNDCTL_13;
> diff --git a/ndctl/lib/lss.c b/ndctl/lib/lss.c
> new file mode 100644
> index 0000000..fbeeec5
> --- /dev/null
> +++ b/ndctl/lib/lss.c
> @@ -0,0 +1,43 @@
> +/*
> + * Copyright (c) 2017, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU Lesser General Public License,
> + * version 2.1, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT ANY
> + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> + * FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License for
> + * more details.
> + */
> +#include <stdlib.h>
> +#include <limits.h>
> +#include <util/log.h>
> +#include <ndctl/libndctl.h>
> +#include "private.h"
> +
> +/*
> + * Define the wrappers around the ndctl_dimm_ops for LSS DSM
> + */
> +NDCTL_EXPORT struct ndctl_cmd *
> +ndctl_dimm_cmd_new_lss_enable(struct ndctl_dimm *dimm)
> +{
> +       struct ndctl_dimm_ops *ops = dimm->ops;
> +
> +       if (ops && ops->new_lss_enable)
> +               return ops->new_lss_enable(dimm);
> +       else
> +               return NULL;
> +}

The "LSS" acronym is Intel specific. Let's use a neutral name for
this. I propose _ack_shutdown_count() to go along with the
_{get,set}_shutdown count namespace.

> +
> +NDCTL_EXPORT int
> +ndctl_cmd_lss_set_enable(struct ndctl_cmd *cmd, unsigned char enable)
> +{
> +       if (cmd->dimm) {
> +               struct ndctl_dimm_ops *ops = cmd->dimm->ops;
> +
> +               if (ops && ops->lss_set_enable)
> +                       return ops->lss_set_enable(cmd, enable);
> +       }
> +       return -ENXIO;
> +}

Do we need this routine? Is there a use case for sending 0? In fact
the spec says, "All other values are reserved.", so I assume that
means sending 0 is invalid.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm