[PATCH v2 1/4] ndctl: add support to alloc_intel_cmd for variable payload

[PATCH v2 1/4] ndctl: add support to alloc_intel_cmd for variable payload

[Dave Jiang]

Certain payloads have variable size. The existing alloc_intel_cmd()
does not take into account of that. Adding additional size for allocation.
We do waste a little bit of the space because of the smart payload.

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

diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
index 3e4260f..b85a682 100644
--- a/ndctl/lib/intel.c
+++ b/ndctl/lib/intel.c
@@ -33,7 +33,7 @@ static struct ndctl_cmd *alloc_intel_cmd(struct ndctl_dimm *dimm,
 		return NULL;
 	}
 
-	size = sizeof(*cmd) + sizeof(struct nd_pkg_intel);
+	size = sizeof(*cmd) + sizeof(struct nd_pkg_intel) + in_size + out_size;
 	cmd = calloc(1, size);
 	if (!cmd)
 		return NULL;

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

[PATCH v2 2/4] ndctl: add firmware download support functions in libndctl

[Dave Jiang]

Adding the DSM commands from Intel DSM v1.6 to support firmware update
sequence in the libndctl library as additional dimm_ops.

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

v2:
Sync'd return types per Dan's comment and removed __uN types.


 0 files changed

diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
index 5e10fde..e3a12e7 100644
--- a/ndctl/lib/Makefile.am
+++ b/ndctl/lib/Makefile.am
@@ -21,6 +21,7 @@ libndctl_la_SOURCES =\
 	hpe1.c \
 	msft.c \
 	ars.c \
+	firmware.c \
 	libndctl.c
 
 libndctl_la_LIBADD =\
diff --git a/ndctl/lib/firmware.c b/ndctl/lib/firmware.c
new file mode 100644
index 0000000..3ec89b7
--- /dev/null
+++ b/ndctl/lib/firmware.c
@@ -0,0 +1,128 @@
+/*
+ * 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 firmware update:
+ */
+NDCTL_EXPORT struct ndctl_cmd *
+ndctl_dimm_cmd_new_fw_get_info(struct ndctl_dimm *dimm)
+{
+	struct ndctl_dimm_ops *ops = dimm->ops;
+
+	if (ops && ops->new_fw_get_info)
+		return ops->new_fw_get_info(dimm);
+	else
+		return NULL;
+}
+
+NDCTL_EXPORT struct ndctl_cmd *
+ndctl_dimm_cmd_new_fw_start_update(struct ndctl_dimm *dimm)
+{
+	struct ndctl_dimm_ops *ops = dimm->ops;
+
+	if (ops && ops->new_fw_start_update)
+		return ops->new_fw_start_update(dimm);
+	else
+		return NULL;
+}
+
+NDCTL_EXPORT struct ndctl_cmd *
+ndctl_dimm_cmd_new_fw_send(struct ndctl_dimm *dimm, unsigned int chunk_size)
+{
+	struct ndctl_dimm_ops *ops = dimm->ops;
+
+	if (ops && ops->new_fw_send)
+		return ops->new_fw_send(dimm, chunk_size);
+	else
+		return NULL;
+}
+
+NDCTL_EXPORT struct ndctl_cmd *
+ndctl_dimm_cmd_new_fw_finish(struct ndctl_dimm *dimm)
+{
+	struct ndctl_dimm_ops *ops = dimm->ops;
+
+	if (ops && ops->new_fw_finish)
+		return ops->new_fw_finish(dimm);
+	else
+		return NULL;
+}
+
+NDCTL_EXPORT struct ndctl_cmd *
+ndctl_dimm_cmd_new_fw_finish_query(struct ndctl_dimm *dimm)
+{
+	struct ndctl_dimm_ops *ops = dimm->ops;
+
+	if (ops && ops->new_fw_finish_query)
+		return ops->new_fw_finish_query(dimm);
+	else
+		return NULL;
+}
+
+
+#define firmware_cmd_op(op, rettype, defretvalue) \
+NDCTL_EXPORT rettype ndctl_cmd_##op(struct ndctl_cmd *cmd) \
+{ \
+	if (cmd->dimm) { \
+		struct ndctl_dimm_ops *ops = cmd->dimm->ops; \
+		if (ops && ops->op) \
+			return ops->op(cmd); \
+	} \
+	return defretvalue; \
+}
+
+firmware_cmd_op(fw_info_get_storage_size, unsigned int, 0)
+firmware_cmd_op(fw_info_get_max_send_len, unsigned int, 0)
+firmware_cmd_op(fw_info_get_query_interval, unsigned int, 0)
+firmware_cmd_op(fw_info_get_max_query_time, unsigned int, 0)
+firmware_cmd_op(fw_info_get_fis_version, unsigned int, 0)
+firmware_cmd_op(fw_info_get_run_version, unsigned long, 0)
+firmware_cmd_op(fw_info_get_updated_version, unsigned long, 0)
+firmware_cmd_op(fw_info_get_update_cap, unsigned char, 0)
+firmware_cmd_op(fw_start_get_context, unsigned int, 0)
+firmware_cmd_op(fw_fquery_get_fw_rev, unsigned long, 0)
+
+#define firmware_cmd_set_op(op, intype) \
+NDCTL_EXPORT int ndctl_cmd_##op(struct ndctl_cmd *cmd, intype val) \
+{ \
+	if (cmd->dimm) { \
+		struct ndctl_dimm_ops *ops = cmd->dimm->ops; \
+		if (ops && ops->op) \
+			return ops->op(cmd, val); \
+	} \
+	return -ENXIO; \
+}
+
+firmware_cmd_set_op(fw_send_set_context, unsigned int)
+firmware_cmd_set_op(fw_send_set_offset, unsigned int)
+firmware_cmd_set_op(fw_send_set_length, unsigned int)
+firmware_cmd_set_op(fw_finish_set_ctrl_flags, unsigned char)
+firmware_cmd_set_op(fw_finish_set_context, unsigned int)
+firmware_cmd_set_op(fw_fquery_set_context, unsigned int)
+
+NDCTL_EXPORT int
+ndctl_cmd_fw_send_set_data(struct ndctl_cmd *cmd, void *buf, size_t len)
+{
+	if (cmd->dimm) {
+		struct ndctl_dimm_ops *ops = cmd->dimm->ops;
+		if (ops && ops->fw_send_set_data)
+			return ops->fw_send_set_data(cmd, buf, len);
+	}
+	return -ENXIO;
+}
+
diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
index b85a682..3ac4d3b 100644
--- a/ndctl/lib/intel.c
+++ b/ndctl/lib/intel.c
@@ -288,6 +288,11 @@ static const char *intel_cmd_desc(int fn)
 	static const char *descs[] = {
 		[ND_INTEL_SMART] = "smart",
 		[ND_INTEL_SMART_THRESHOLD] = "smart_thresh",
+		[ND_INTEL_FW_GET_INFO] = "firmware_get_info",
+		[ND_INTEL_FW_START_UPDATE] = "firmware_start_update",
+		[ND_INTEL_FW_SEND_DATA] = "firmware_send_data",
+		[ND_INTEL_FW_FINISH_UPDATE] = "firmware_finish_update",
+		[ND_INTEL_FW_FINISH_STATUS_QUERY] = "firmware_finish_query",
 		[ND_INTEL_SMART_SET_THRESHOLD] = "smart_set_thresh",
 	};
 	const char *desc = descs[fn];
@@ -299,6 +304,238 @@ static const char *intel_cmd_desc(int fn)
 	return desc;
 }
 
+static struct ndctl_cmd *intel_dimm_cmd_new_fw_get_info(struct ndctl_dimm *dimm)
+{
+	struct ndctl_cmd *cmd;
+
+	BUILD_ASSERT(sizeof(struct nd_intel_fw_info) == 44);
+
+	cmd = alloc_intel_cmd(dimm, ND_INTEL_FW_GET_INFO,
+			0, sizeof(cmd->intel->info));
+	if (!cmd)
+		return NULL;
+
+	cmd->firmware_status = &cmd->intel->info.status;
+	return cmd;
+}
+
+static int intel_fw_get_info_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_FW_GET_INFO)
+		return -EINVAL;
+	return 0;
+}
+
+#define intel_fw_info_get_field32(cmd, field) \
+static unsigned int intel_cmd_fw_info_get_##field( \
+			struct ndctl_cmd *cmd) \
+{ \
+	if (intel_fw_get_info_valid(cmd) < 0) \
+		return -EINVAL; \
+	return cmd->intel->info.field; \
+}
+
+#define intel_fw_info_get_field64(cmd, field) \
+static unsigned long intel_cmd_fw_info_get_##field( \
+			struct ndctl_cmd *cmd) \
+{ \
+	if (intel_fw_get_info_valid(cmd) < 0) \
+		return -EINVAL; \
+	return cmd->intel->info.field; \
+}
+
+static unsigned char intel_cmd_fw_info_get_update_cap(struct ndctl_cmd *cmd)
+{
+	if (intel_fw_get_info_valid(cmd) < 0)
+		return -EINVAL;
+	return cmd->intel->info.update_cap;
+}
+
+intel_fw_info_get_field32(cmd, storage_size)
+intel_fw_info_get_field32(cmd, max_send_len)
+intel_fw_info_get_field32(cmd, query_interval)
+intel_fw_info_get_field32(cmd, max_query_time);
+intel_fw_info_get_field32(cmd, fis_version);
+intel_fw_info_get_field64(cmd, run_version);
+intel_fw_info_get_field64(cmd, updated_version);
+
+static struct ndctl_cmd *intel_dimm_cmd_new_fw_start(struct ndctl_dimm *dimm)
+{
+	struct ndctl_cmd *cmd;
+
+	BUILD_ASSERT(sizeof(struct nd_intel_fw_start) == 8);
+
+	cmd = alloc_intel_cmd(dimm, ND_INTEL_FW_START_UPDATE,
+			sizeof(cmd->intel->start) - 4, 4);
+	if (!cmd)
+		return NULL;
+
+	cmd->firmware_status = &cmd->intel->start.status;
+	return cmd;
+}
+
+static int intel_fw_start_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_FW_START_UPDATE)
+		return -EINVAL;
+	return 0;
+}
+
+static unsigned int intel_cmd_fw_start_get_context(struct ndctl_cmd *cmd)
+{
+	if (intel_fw_start_valid(cmd) < 0)
+		return -EINVAL;
+	return cmd->intel->start.context;
+}
+
+static struct ndctl_cmd *intel_dimm_cmd_new_fw_send(struct ndctl_dimm *dimm,
+		unsigned int chunk_size)
+{
+	struct ndctl_cmd *cmd;
+
+	BUILD_ASSERT(sizeof(struct nd_intel_fw_send_data) == 12);
+
+	cmd = alloc_intel_cmd(dimm, ND_INTEL_FW_SEND_DATA,
+		sizeof(cmd->intel->send) + chunk_size, 4);
+	if (!cmd)
+		return NULL;
+
+	/* the last dword is reserved for status */
+	cmd->firmware_status =
+		(unsigned int *)(&cmd->intel->send.data[0] + chunk_size);
+	return cmd;
+}
+
+static int intel_fw_send_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_FW_SEND_DATA)
+		return -EINVAL;
+	return 0;
+}
+
+#define intel_fw_send_set_field(field) \
+static int intel_cmd_fw_send_set_##field(struct ndctl_cmd *cmd, unsigned int val) \
+{ \
+	if (intel_fw_send_valid(cmd) < 0) \
+		return -EINVAL; \
+	cmd->intel->send.field = val; \
+	return 0; \
+}
+
+intel_fw_send_set_field(context)
+intel_fw_send_set_field(offset)
+intel_fw_send_set_field(length)
+
+static int
+intel_cmd_fw_send_set_data(struct ndctl_cmd *cmd, void *buf, size_t len)
+{
+	if (intel_fw_send_valid(cmd) < 0)
+		return -EINVAL;
+
+	memcpy(cmd->intel->send.data, buf, len);
+	return 0;
+}
+
+static struct ndctl_cmd *intel_dimm_cmd_new_fw_finish(struct ndctl_dimm *dimm)
+{
+	struct ndctl_cmd *cmd;
+
+	BUILD_ASSERT(sizeof(struct nd_intel_fw_finish_update) == 12);
+
+	cmd = alloc_intel_cmd(dimm, ND_INTEL_FW_FINISH_UPDATE,
+			sizeof(cmd->intel->finish) - 4, 4);
+	if (!cmd)
+		return NULL;
+
+	cmd->firmware_status = &cmd->intel->finish.status;
+	return cmd;
+}
+
+static int intel_fw_finish_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_FW_FINISH_UPDATE)
+		return -EINVAL;
+	return 0;
+}
+
+static int
+intel_cmd_fw_finish_set_ctrl_flags(struct ndctl_cmd *cmd, unsigned char ctrl)
+{
+	if (intel_fw_finish_valid(cmd) < 0)
+		return -EINVAL;
+	cmd->intel->finish.ctrl_flags = ctrl;
+	return 0;
+}
+
+static int
+intel_cmd_fw_finish_set_context(struct ndctl_cmd *cmd, unsigned int context)
+{
+	if (intel_fw_finish_valid(cmd) < 0)
+		return -EINVAL;
+	cmd->intel->finish.context = context;
+	return 0;
+}
+
+static struct ndctl_cmd *
+intel_dimm_cmd_new_fw_finish_query(struct ndctl_dimm *dimm)
+{
+	struct ndctl_cmd *cmd;
+
+	BUILD_ASSERT(sizeof(struct nd_intel_fw_finish_update) == 12);
+
+	cmd = alloc_intel_cmd(dimm, ND_INTEL_FW_FINISH_UPDATE,
+			4, sizeof(cmd->intel->fquery) - 4);
+	if (!cmd)
+		return NULL;
+
+	cmd->firmware_status = &cmd->intel->finish.status;
+	return cmd;
+
+}
+
+static int intel_fw_fquery_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_FW_FINISH_UPDATE)
+		return -EINVAL;
+	return 0;
+}
+
+static int
+intel_cmd_fw_fquery_set_context(struct ndctl_cmd *cmd, unsigned int context)
+{
+	if (intel_fw_fquery_valid(cmd) < 0)
+		return -EINVAL;
+	cmd->intel->fquery.context = context;
+	return 0;
+}
+
+static unsigned long intel_cmd_fw_fquery_get_fw_rev(struct ndctl_cmd *cmd)
+{
+	if (intel_fw_start_valid(cmd) < 0)
+		return -EINVAL;
+	return cmd->intel->fquery.updated_fw_rev;
+}
+
 struct ndctl_dimm_ops * const intel_dimm_ops = &(struct ndctl_dimm_ops) {
 	.cmd_desc = intel_cmd_desc,
 	.new_smart = intel_dimm_cmd_new_smart,
@@ -331,4 +568,26 @@ struct ndctl_dimm_ops * const intel_dimm_ops = &(struct ndctl_dimm_ops) {
 	.smart_threshold_set_ctrl_temperature
 		= intel_cmd_smart_threshold_set_ctrl_temperature,
 	.smart_threshold_set_spares = intel_cmd_smart_threshold_set_spares,
+	.new_fw_get_info = intel_dimm_cmd_new_fw_get_info,
+	.fw_info_get_storage_size = intel_cmd_fw_info_get_storage_size,
+	.fw_info_get_max_send_len = intel_cmd_fw_info_get_max_send_len,
+	.fw_info_get_query_interval = intel_cmd_fw_info_get_query_interval,
+	.fw_info_get_max_query_time = intel_cmd_fw_info_get_max_query_time,
+	.fw_info_get_update_cap = intel_cmd_fw_info_get_update_cap,
+	.fw_info_get_fis_version = intel_cmd_fw_info_get_fis_version,
+	.fw_info_get_run_version = intel_cmd_fw_info_get_run_version,
+	.fw_info_get_updated_version = intel_cmd_fw_info_get_updated_version,
+	.new_fw_start_update = intel_dimm_cmd_new_fw_start,
+	.fw_start_get_context = intel_cmd_fw_start_get_context,
+	.new_fw_send = intel_dimm_cmd_new_fw_send,
+	.fw_send_set_context = intel_cmd_fw_send_set_context,
+	.fw_send_set_offset = intel_cmd_fw_send_set_offset,
+	.fw_send_set_length = intel_cmd_fw_send_set_length,
+	.fw_send_set_data = intel_cmd_fw_send_set_data,
+	.new_fw_finish = intel_dimm_cmd_new_fw_finish,
+	.fw_finish_set_ctrl_flags = intel_cmd_fw_finish_set_ctrl_flags,
+	.fw_finish_set_context = intel_cmd_fw_finish_set_context,
+	.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,
 };
diff --git a/ndctl/lib/intel.h b/ndctl/lib/intel.h
index 9e63985..080e37b 100644
--- a/ndctl/lib/intel.h
+++ b/ndctl/lib/intel.h
@@ -5,6 +5,12 @@
 #define __INTEL_H__
 #define ND_INTEL_SMART 1
 #define ND_INTEL_SMART_THRESHOLD 2
+
+#define ND_INTEL_FW_GET_INFO 12
+#define ND_INTEL_FW_START_UPDATE 13
+#define ND_INTEL_FW_SEND_DATA 14
+#define ND_INTEL_FW_FINISH_UPDATE 15
+#define ND_INTEL_FW_FINISH_STATUS_QUERY 16
 #define ND_INTEL_SMART_SET_THRESHOLD 17
 
 #define ND_INTEL_SMART_HEALTH_VALID             (1 << 0)
@@ -71,12 +77,58 @@ struct nd_intel_smart_set_threshold {
 	__u32 status;
 } __attribute__((packed));
 
+struct nd_intel_fw_info {
+	__u32 status;
+	__u32 storage_size;
+	__u32 max_send_len;
+	__u32 query_interval;
+	__u32 max_query_time;
+	__u8 update_cap;
+	__u8 reserved[3];
+	__u32 fis_version;
+	__u64 run_version;
+	__u64 updated_version;
+} __attribute__((packed));
+
+struct nd_intel_fw_start {
+	__u32 status;
+	__u32 context;
+} __attribute__((packed));
+
+/* this one has the output first because the variable input data size */
+struct nd_intel_fw_send_data {
+	__u32 context;
+	__u32 offset;
+	__u32 length;
+	__u8 data[0];
+/* reserving last 4 bytes as status */
+/*	__u32 status; */
+} __attribute__((packed));
+
+struct nd_intel_fw_finish_update {
+	__u8 ctrl_flags;
+	__u8 reserved[3];
+	__u32 context;
+	__u32 status;
+} __attribute__((packed));
+
+struct nd_intel_fw_finish_query {
+	__u32 context;
+	__u32 status;
+	__u64 updated_fw_rev;
+} __attribute__((packed));
+
 struct nd_pkg_intel {
 	struct nd_cmd_pkg gen;
 	union {
 		struct nd_intel_smart smart;
 		struct nd_intel_smart_threshold	thresh;
 		struct nd_intel_smart_set_threshold set_thresh;
+		struct nd_intel_fw_info info;
+		struct nd_intel_fw_start start;
+		struct nd_intel_fw_send_data send;
+		struct nd_intel_fw_finish_update finish;
+		struct nd_intel_fw_finish_query fquery;
 	};
 };
 #endif /* __INTEL_H__ */
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 2ace942..2e248ab 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -322,4 +322,26 @@ global:
 	ndctl_cmd_smart_threshold_set_ctrl_temperature;
 	ndctl_cmd_smart_threshold_set_spares;
 	ndctl_decode_smart_temperature;
+	ndctl_dimm_cmd_new_fw_get_info;
+	ndctl_dimm_cmd_new_fw_start_update;
+	ndctl_dimm_cmd_new_fw_send;
+	ndctl_dimm_cmd_new_fw_finish;
+	ndctl_dimm_cmd_new_fw_finish_query;
+	ndctl_cmd_fw_info_get_storage_size;
+	ndctl_cmd_fw_info_get_max_send_len;
+	ndctl_cmd_fw_info_get_query_interval;
+	ndctl_cmd_fw_info_get_max_query_time;
+	ndctl_cmd_fw_info_get_run_version;
+	ndctl_cmd_fw_info_get_fis_version;
+	ndctl_cmd_fw_info_get_updated_version;
+	ndctl_cmd_fw_info_get_update_cap;
+	ndctl_cmd_fw_start_get_context;
+	ndctl_cmd_fw_fquery_get_fw_rev;
+	ndctl_cmd_fw_send_set_context;
+	ndctl_cmd_fw_send_set_offset;
+	ndctl_cmd_fw_send_set_length;
+	ndctl_cmd_fw_send_set_data;
+	ndctl_cmd_fw_finish_set_ctrl_flags;
+	ndctl_cmd_fw_finish_set_context;
+	ndctl_cmd_fw_fquery_set_context;
 } LIBNDCTL_13;
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 6726097..20f9e6e 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -303,6 +303,28 @@ struct ndctl_dimm_ops {
 	int (*smart_threshold_set_media_temperature)(struct ndctl_cmd *, unsigned int);
 	int (*smart_threshold_set_ctrl_temperature)(struct ndctl_cmd *, unsigned int);
 	int (*smart_threshold_set_spares)(struct ndctl_cmd *, unsigned int);
+	struct ndctl_cmd *(*new_fw_get_info)(struct ndctl_dimm *);
+	unsigned int (*fw_info_get_storage_size)(struct ndctl_cmd *);
+	unsigned int (*fw_info_get_max_send_len)(struct ndctl_cmd *);
+	unsigned int (*fw_info_get_query_interval)(struct ndctl_cmd *);
+	unsigned int (*fw_info_get_max_query_time)(struct ndctl_cmd *);
+	unsigned char (*fw_info_get_update_cap)(struct ndctl_cmd *);
+	unsigned int (*fw_info_get_fis_version)(struct ndctl_cmd *);
+	unsigned long (*fw_info_get_run_version)(struct ndctl_cmd *);
+	unsigned long (*fw_info_get_updated_version)(struct ndctl_cmd *);
+	struct ndctl_cmd *(*new_fw_start_update)(struct ndctl_dimm *);
+	unsigned int (*fw_start_get_context)(struct ndctl_cmd *);
+	struct ndctl_cmd *(*new_fw_send)(struct ndctl_dimm *, unsigned int chunk_size);
+	int (*fw_send_set_context)(struct ndctl_cmd *, unsigned int context);
+	int (*fw_send_set_offset)(struct ndctl_cmd *, unsigned int offset);
+	int (*fw_send_set_length)(struct ndctl_cmd *, unsigned int len);
+	int (*fw_send_set_data)(struct ndctl_cmd *, void *buf, size_t len);
+	struct ndctl_cmd *(*new_fw_finish)(struct ndctl_dimm *);
+	int (*fw_finish_set_ctrl_flags)(struct ndctl_cmd *, unsigned char flags);
+	int (*fw_finish_set_context)(struct ndctl_cmd *, unsigned int context);
+	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_dimm_ops * const intel_dimm_ops;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index e9da20e..64a4e99 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -15,6 +15,7 @@
 
 #include <stdbool.h>
 #include <stdarg.h>
+#include <stdint.h>
 #include <unistd.h>
 #include <errno.h>
 
@@ -581,6 +582,31 @@ int ndctl_dax_delete(struct ndctl_dax *dax);
 int ndctl_dax_is_configured(struct ndctl_dax *dax);
 struct daxctl_region *ndctl_dax_get_daxctl_region(struct ndctl_dax *dax);
 
+struct ndctl_cmd *ndctl_dimm_cmd_new_fw_get_info(struct ndctl_dimm *dimm);
+struct ndctl_cmd *ndctl_dimm_cmd_new_fw_start_update(struct ndctl_dimm *dimm);
+struct ndctl_cmd *ndctl_dimm_cmd_new_fw_send(struct ndctl_dimm *dimm,
+		unsigned int chunk_size);
+struct ndctl_cmd *ndctl_dimm_cmd_new_fw_finish(struct ndctl_dimm *dimm);
+struct ndctl_cmd *ndctl_dimm_cmd_new_fw_finish_query(struct ndctl_dimm *dimm);
+unsigned int ndctl_cmd_fw_info_get_storage_size(struct ndctl_cmd *cmd);
+unsigned int ndctl_cmd_fw_info_get_max_send_len(struct ndctl_cmd *cmd);
+unsigned int ndctl_cmd_fw_info_get_query_interval(struct ndctl_cmd *cmd);
+unsigned int ndctl_cmd_fw_info_get_max_query_time(struct ndctl_cmd *cmd);
+unsigned char ndctl_cmd_fw_info_get_update_cap(struct ndctl_cmd *cmd);
+unsigned int ndctl_cmd_fw_info_get_fis_version(struct ndctl_cmd *cmd);
+unsigned long ndctl_cmd_fw_info_get_run_version(struct ndctl_cmd *cmd);
+unsigned long ndctl_cmd_fw_info_get_updated_version(struct ndctl_cmd *cmd);
+unsigned char ndctl_cmd_fw_info_get_update_cap(struct ndctl_cmd *cmd);
+unsigned int ndctl_cmd_fw_start_get_context(struct ndctl_cmd *cmd);
+unsigned long ndctl_cmd_fw_fquery_get_fw_rev(struct ndctl_cmd *cmd);
+int ndctl_cmd_fw_send_set_context(struct ndctl_cmd *cmd, unsigned int context);
+int ndctl_cmd_fw_send_set_offset(struct ndctl_cmd *cmd, unsigned int offset);
+int ndctl_cmd_fw_send_set_length(struct ndctl_cmd *cmd, unsigned int len);
+int ndctl_cmd_fw_send_set_data(struct ndctl_cmd *cmd, void *buf, size_t len);
+int ndctl_cmd_fw_finish_set_ctrl_flags(struct ndctl_cmd *cmd, unsigned char flags);
+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);
+
 #ifdef __cplusplus
 } /* extern "C" */
 #endif

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

[PATCH v2 3/4] ndctl: add firmware update command option for ndctl

[Dave Jiang]

Adding option "update-firmware" to ndctl for update firmware support from
Intel DSM v1.6. ndctl update-firmware takes an option of -f for a firmware
binary and a -d for the DIMM name:
ndctl update-firmware -d nmem0 -f new_firmware.bin

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

diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index 615baf0..27b2076 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -31,6 +31,7 @@ man1_MANS = \
 	ndctl-destroy-namespace.1 \
 	ndctl-check-namespace.1 \
 	ndctl-inject-error.1 \
+	ndctl-update-firmware.1 \
 	ndctl-list.1
 
 CLEANFILES = $(man1_MANS)
diff --git a/Documentation/ndctl/ndctl-update-firmware.txt b/Documentation/ndctl/ndctl-update-firmware.txt
new file mode 100644
index 0000000..d742302
--- /dev/null
+++ b/Documentation/ndctl/ndctl-update-firmware.txt
@@ -0,0 +1,18 @@
+ndctl-update-firmware(1)
+========================
+
+NAME
+----
+ndctl-update-firmware - provides updating of NVDIMM firmware
+
+SYNOPSIS
+--------
+[verse]
+'ndctl update-firmware' -f <firmware_file> -d <dimm name>
+
+COPYRIGHT
+---------
+Copyright (c) 2016 - 2017, Intel Corporation. License GPLv2: GNU GPL
+version 2 <http://gnu.org/licenses/gpl.html>.  This is free software:
+you are free to change and redistribute it.  There is NO WARRANTY, to
+the extent permitted by law.
diff --git a/builtin.h b/builtin.h
index 5e1b7ef..1f423dc 100644
--- a/builtin.h
+++ b/builtin.h
@@ -43,4 +43,5 @@ int cmd_test(int argc, const char **argv, void *ctx);
 #ifdef ENABLE_DESTRUCTIVE
 int cmd_bat(int argc, const char **argv, void *ctx);
 #endif
+int cmd_update_firmware(int argc, const char **argv, void *ctx);
 #endif /* _NDCTL_BUILTIN_H_ */
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index 6677607..5cd8678 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -13,7 +13,8 @@ ndctl_SOURCES = ndctl.c \
 		test.c \
 		../util/json.c \
 		util/json-smart.c \
-		inject-error.c
+		inject-error.c \
+		update.c
 
 if ENABLE_DESTRUCTIVE
 ndctl_SOURCES += ../test/blk_namespaces.c \
diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
index 3ac4d3b..6d26a6c 100644
--- a/ndctl/lib/intel.c
+++ b/ndctl/lib/intel.c
@@ -323,7 +323,7 @@ static int intel_fw_get_info_valid(struct ndctl_cmd *cmd)
 {
 	struct nd_pkg_intel *pkg = cmd->intel;
 
-	if (cmd->type != ND_CMD_CALL || cmd->status != 1
+	if (cmd->type != ND_CMD_CALL || cmd->status != 0
 			|| pkg->gen.nd_family != NVDIMM_FAMILY_INTEL
 			|| pkg->gen.nd_command != ND_INTEL_FW_GET_INFO)
 		return -EINVAL;
@@ -382,7 +382,7 @@ static int intel_fw_start_valid(struct ndctl_cmd *cmd)
 {
 	struct nd_pkg_intel *pkg = cmd->intel;
 
-	if (cmd->type != ND_CMD_CALL || cmd->status != 1
+	if (cmd->type != ND_CMD_CALL || cmd->status != 0
 			|| pkg->gen.nd_family != NVDIMM_FAMILY_INTEL
 			|| pkg->gen.nd_command != ND_INTEL_FW_START_UPDATE)
 		return -EINVAL;
@@ -414,7 +414,7 @@ static struct ndctl_cmd *intel_dimm_cmd_new_fw_send(struct ndctl_dimm *dimm,
 	return cmd;
 }
 
-static int intel_fw_send_valid(struct ndctl_cmd *cmd)
+static int intel_fw_send_set_valid(struct ndctl_cmd *cmd)
 {
 	struct nd_pkg_intel *pkg = cmd->intel;
 
@@ -428,7 +428,7 @@ static int intel_fw_send_valid(struct ndctl_cmd *cmd)
 #define intel_fw_send_set_field(field) \
 static int intel_cmd_fw_send_set_##field(struct ndctl_cmd *cmd, unsigned int val) \
 { \
-	if (intel_fw_send_valid(cmd) < 0) \
+	if (intel_fw_send_set_valid(cmd) < 0) \
 		return -EINVAL; \
 	cmd->intel->send.field = val; \
 	return 0; \
@@ -441,7 +441,7 @@ intel_fw_send_set_field(length)
 static int
 intel_cmd_fw_send_set_data(struct ndctl_cmd *cmd, void *buf, size_t len)
 {
-	if (intel_fw_send_valid(cmd) < 0)
+	if (intel_fw_send_set_valid(cmd) < 0)
 		return -EINVAL;
 
 	memcpy(cmd->intel->send.data, buf, len);
@@ -455,7 +455,7 @@ static struct ndctl_cmd *intel_dimm_cmd_new_fw_finish(struct ndctl_dimm *dimm)
 	BUILD_ASSERT(sizeof(struct nd_intel_fw_finish_update) == 12);
 
 	cmd = alloc_intel_cmd(dimm, ND_INTEL_FW_FINISH_UPDATE,
-			sizeof(cmd->intel->finish) - 4, 4);
+			offsetof(struct nd_intel_fw_finish_update, status), 4);
 	if (!cmd)
 		return NULL;
 
@@ -463,7 +463,7 @@ static struct ndctl_cmd *intel_dimm_cmd_new_fw_finish(struct ndctl_dimm *dimm)
 	return cmd;
 }
 
-static int intel_fw_finish_valid(struct ndctl_cmd *cmd)
+static int intel_fw_finish_set_valid(struct ndctl_cmd *cmd)
 {
 	struct nd_pkg_intel *pkg = cmd->intel;
 
@@ -477,7 +477,7 @@ static int intel_fw_finish_valid(struct ndctl_cmd *cmd)
 static int
 intel_cmd_fw_finish_set_ctrl_flags(struct ndctl_cmd *cmd, unsigned char ctrl)
 {
-	if (intel_fw_finish_valid(cmd) < 0)
+	if (intel_fw_finish_set_valid(cmd) < 0)
 		return -EINVAL;
 	cmd->intel->finish.ctrl_flags = ctrl;
 	return 0;
@@ -486,7 +486,7 @@ intel_cmd_fw_finish_set_ctrl_flags(struct ndctl_cmd *cmd, unsigned char ctrl)
 static int
 intel_cmd_fw_finish_set_context(struct ndctl_cmd *cmd, unsigned int context)
 {
-	if (intel_fw_finish_valid(cmd) < 0)
+	if (intel_fw_finish_set_valid(cmd) < 0)
 		return -EINVAL;
 	cmd->intel->finish.context = context;
 	return 0;
@@ -497,14 +497,14 @@ intel_dimm_cmd_new_fw_finish_query(struct ndctl_dimm *dimm)
 {
 	struct ndctl_cmd *cmd;
 
-	BUILD_ASSERT(sizeof(struct nd_intel_fw_finish_update) == 12);
+	BUILD_ASSERT(sizeof(struct nd_intel_fw_finish_query) == 16);
 
-	cmd = alloc_intel_cmd(dimm, ND_INTEL_FW_FINISH_UPDATE,
+	cmd = alloc_intel_cmd(dimm, ND_INTEL_FW_FINISH_STATUS_QUERY,
 			4, sizeof(cmd->intel->fquery) - 4);
 	if (!cmd)
 		return NULL;
 
-	cmd->firmware_status = &cmd->intel->finish.status;
+	cmd->firmware_status = &cmd->intel->fquery.status;
 	return cmd;
 
 }
@@ -513,9 +513,20 @@ static int intel_fw_fquery_valid(struct ndctl_cmd *cmd)
 {
 	struct nd_pkg_intel *pkg = cmd->intel;
 
+	if (cmd->type != ND_CMD_CALL || cmd->status != 0
+			|| pkg->gen.nd_family != NVDIMM_FAMILY_INTEL
+			|| pkg->gen.nd_command != ND_INTEL_FW_FINISH_STATUS_QUERY)
+		return -EINVAL;
+	return 0;
+}
+
+static int intel_fw_fquery_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_FW_FINISH_UPDATE)
+			|| pkg->gen.nd_command != ND_INTEL_FW_FINISH_STATUS_QUERY)
 		return -EINVAL;
 	return 0;
 }
@@ -523,7 +534,7 @@ static int intel_fw_fquery_valid(struct ndctl_cmd *cmd)
 static int
 intel_cmd_fw_fquery_set_context(struct ndctl_cmd *cmd, unsigned int context)
 {
-	if (intel_fw_fquery_valid(cmd) < 0)
+	if (intel_fw_fquery_set_valid(cmd) < 0)
 		return -EINVAL;
 	cmd->intel->fquery.context = context;
 	return 0;
@@ -531,7 +542,7 @@ intel_cmd_fw_fquery_set_context(struct ndctl_cmd *cmd, unsigned int context)
 
 static unsigned long intel_cmd_fw_fquery_get_fw_rev(struct ndctl_cmd *cmd)
 {
-	if (intel_fw_start_valid(cmd) < 0)
+	if (intel_fw_fquery_valid(cmd) < 0)
 		return -EINVAL;
 	return cmd->intel->fquery.updated_fw_rev;
 }
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 0f748e1..a0e5153 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -84,6 +84,7 @@ static struct cmd_struct commands[] = {
 	{ "init-labels", cmd_init_labels },
 	{ "check-labels", cmd_check_labels },
 	{ "inject-error", cmd_inject_error },
+	{ "update-firmware", cmd_update_firmware },
 	{ "list", cmd_list },
 	{ "help", cmd_help },
 	#ifdef ENABLE_TEST
diff --git a/ndctl/update.c b/ndctl/update.c
new file mode 100644
index 0000000..2ac6d21
--- /dev/null
+++ b/ndctl/update.c
@@ -0,0 +1,567 @@
+/*
+ * 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 <errno.h>
+#include <stdlib.h>
+#include <limits.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <syslog.h>
+#include <time.h>
+#include <sys/time.h>
+
+#include <util/log.h>
+#include <util/size.h>
+#include <util/util.h>
+#include <uuid/uuid.h>
+#include <util/json.h>
+#include <util/filter.h>
+#include <json-c/json.h>
+#include <util/fletcher.h>
+#include <ndctl/libndctl.h>
+#include <ndctl/namespace.h>
+#include <util/parse-options.h>
+#include <ccan/minmax/minmax.h>
+#include <ccan/array_size/array_size.h>
+#ifdef HAVE_NDCTL_H
+#include <linux/ndctl.h>
+#else
+#include <ndctl.h>
+#endif
+
+#include <libndctl-nfit.h>
+#include "private.h"
+#include <builtin.h>
+#include <test.h>
+
+#define ND_CMD_STATUS_SUCCESS	0
+#define ND_CMD_STATUS_NOTSUPP	1
+#define	ND_CMD_STATUS_NOTEXIST	2
+#define ND_CMD_STATUS_INVALPARM	3
+#define ND_CMD_STATUS_HWERR	4
+#define ND_CMD_STATUS_RETRY	5
+#define ND_CMD_STATUS_UNKNOWN	6
+#define ND_CMD_STATUS_EXTEND	7
+#define ND_CMD_STATUS_NORES	8
+#define ND_CMD_STATUS_NOTREADY	9
+
+#define ND_CMD_STATUS_START_BUSY	0x10000
+#define ND_CMD_STATUS_SEND_CTXINVAL	0x10000
+#define ND_CMD_STATUS_FIN_CTXINVAL	0x10000
+#define ND_CMD_STATUS_FIN_DONE		0x20000
+#define ND_CMD_STATUS_FIN_BAD		0x30000
+#define ND_CMD_STATUS_FIN_ABORTED	0x40000
+#define ND_CMD_STATUS_FQ_CTXINVAL	0x10000
+#define ND_CMD_STATUS_FQ_BUSY		0x20000
+#define ND_CMD_STATUS_FQ_BAD		0x30000
+#define ND_CMD_STATUS_FQ_ORDER		0x40000
+
+struct fw_info {
+	uint32_t store_size;
+	uint32_t update_size;
+	uint32_t query_interval;
+	uint32_t max_query;
+	uint8_t update_cap;
+	uint32_t fis_version;
+	uint64_t run_version;
+	uint32_t context;
+};
+
+struct update_context {
+	int fw_fd;
+	size_t fw_size;
+	const char *fw_path;
+	const char *dimm_id;
+	struct ndctl_dimm *dimm;
+	struct fw_info dimm_fw;
+};
+
+/*
+ * updating firmware consists of performing the following steps:
+ * 1. Call GET_FIMRWARE_INFO DSM. The return results provide:
+ *	A. Size of the firmware storage area
+ *	B. Max size per send command
+ *	C. Polling interval for check finish status
+ *	D. Max time for finish update poll
+ *	E. Update capabilities
+ *	F. Running FIS version
+ *	G. Running FW revision
+ *	H. Updated FW revision. Only valid after firmware update done.
+ * 2. Call START_FW_UPDATE. The return results provide:
+ *	A. Ready to start status
+ *	B. Valid FW update context
+ * 3. Call SEND_FW_UPDATE_DATA with valid payload
+ *    Repeat until done.
+ * 4. Call FINISH_FW_UPDATE
+ * 5. Poll with QUERY_FINISH_UPDATE success or failure
+ */
+
+static int verify_fw_size(struct update_context *uctx)
+{
+	struct fw_info *fw = &uctx->dimm_fw;
+
+	if (uctx->fw_size > fw->store_size) {
+		error("Firmware file size greater than DIMM store\n");
+		return -E2BIG;
+	}
+
+	return 0;
+}
+
+static int submit_get_firmware_info(struct update_context *uctx)
+{
+	struct ndctl_cmd *cmd;
+	int rc;
+	uint32_t status;
+	struct fw_info *fw = &uctx->dimm_fw;
+
+	cmd = ndctl_dimm_cmd_new_fw_get_info(uctx->dimm);
+	if (!cmd)
+		return -ENXIO;
+
+	rc = ndctl_cmd_submit(cmd);
+	if (rc < 0)
+		return rc;
+
+	status = ndctl_cmd_get_firmware_status(cmd);
+	if (status != 0) {
+		error("GET FIRMWARE INFO failed: %#x\n", status);
+		return -ENXIO;
+	}
+
+	fw->store_size = ndctl_cmd_fw_info_get_storage_size(cmd);
+	if (fw->store_size == UINT_MAX)
+		return -ENXIO;
+
+	fw->update_size = ndctl_cmd_fw_info_get_max_send_len(cmd);
+	if (fw->update_size == UINT_MAX)
+		return -ENXIO;
+
+	fw->query_interval = ndctl_cmd_fw_info_get_query_interval(cmd);
+	if (fw->query_interval == UINT_MAX)
+		return -ENXIO;
+
+	fw->max_query = ndctl_cmd_fw_info_get_max_query_time(cmd);
+	if (fw->max_query == UINT_MAX)
+		return -ENXIO;
+
+	fw->update_cap = ndctl_cmd_fw_info_get_update_cap(cmd);
+	if (fw->update_cap == UCHAR_MAX)
+		return -ENXIO;
+
+	fw->fis_version = ndctl_cmd_fw_info_get_fis_version(cmd);
+	if (fw->fis_version == UINT_MAX)
+		return -ENXIO;
+
+	fw->run_version = ndctl_cmd_fw_info_get_run_version(cmd);
+	if (fw->run_version == ULLONG_MAX)
+		return -ENXIO;
+
+	rc = verify_fw_size(uctx);
+	ndctl_cmd_unref(cmd);
+	return rc;
+}
+
+static int submit_start_firmware_upload(struct update_context *uctx)
+{
+	struct ndctl_cmd *cmd;
+	int rc;
+	uint32_t status;
+	struct fw_info *fw = &uctx->dimm_fw;
+
+	cmd = ndctl_dimm_cmd_new_fw_start_update(uctx->dimm);
+	if (!cmd)
+		return -ENXIO;
+
+	rc = ndctl_cmd_submit(cmd);
+	if (rc < 0)
+		return rc;
+
+	status = ndctl_cmd_get_firmware_status(cmd);
+	if (status != 0) {
+		error("START FIRMWARE UPDATE failed: %#x\n", status);
+		if (status & ND_CMD_STATUS_EXTEND &&
+				status & ND_CMD_STATUS_START_BUSY)
+			error("Another firmware upload in progress or finished.\n");
+		return -ENXIO;
+	}
+
+	fw->context = ndctl_cmd_fw_start_get_context(cmd);
+	if (fw->context == UINT_MAX)
+		return -ENXIO;
+
+	ndctl_cmd_unref(cmd);
+	return 0;
+}
+
+static int get_fw_data_from_file(int fd, void *buf, uint32_t len,
+		uint32_t offset)
+{
+	ssize_t rc, total = len;
+
+	while (len) {
+		rc = pread(fd, buf, len, offset);
+		if (rc < 0)
+			return -errno;
+		len -= rc;
+	}
+
+	return total;
+}
+
+static int send_firmware(struct update_context *uctx)
+{
+	struct ndctl_cmd *cmd = NULL;
+	ssize_t read;
+	int rc;
+	uint32_t status;
+	struct fw_info *fw = &uctx->dimm_fw;
+	uint32_t copied = 0, len, remain;
+	void *buf;
+
+	buf = malloc(fw->update_size);
+	if (!buf)
+		return -ENOMEM;
+
+	remain = uctx->fw_size;
+
+	while (remain) {
+		len = min(fw->update_size, remain);
+		read = get_fw_data_from_file(uctx->fw_fd, buf, len, copied);
+		if (read < 0) {
+			rc = read;
+			goto cleanup;
+		}
+
+		cmd = ndctl_dimm_cmd_new_fw_send(uctx->dimm, read);
+		if (!cmd) {
+			rc = -ENXIO;
+			goto cleanup;
+		}
+
+		rc = ndctl_cmd_fw_send_set_context(cmd, fw->context);
+		if (rc < 0)
+			goto cleanup;
+
+		rc = ndctl_cmd_fw_send_set_offset(cmd, copied);
+		if (rc < 0)
+			goto cleanup;
+
+		rc = ndctl_cmd_fw_send_set_length(cmd, read);
+		if (rc < 0)
+			goto cleanup;
+
+		rc = ndctl_cmd_fw_send_set_data(cmd, buf, read);
+		if (rc < 0)
+			goto cleanup;
+
+		rc = ndctl_cmd_submit(cmd);
+		if (rc < 0)
+			goto cleanup;
+
+		status = ndctl_cmd_get_firmware_status(cmd);
+		if (status != 0) {
+			error("SEND FIRMWARE failed: %#x\n", status);
+			rc = -ENXIO;
+			goto cleanup;
+		}
+
+		copied += read;
+		remain -= read;
+
+		ndctl_cmd_unref(cmd);
+		cmd = NULL;
+	}
+
+cleanup:
+	if (cmd)
+		ndctl_cmd_unref(cmd);
+	free(buf);
+	return rc;
+}
+
+static int submit_finish_firmware(struct update_context *uctx, int err)
+{
+	struct ndctl_cmd *cmd;
+	int rc;
+	uint32_t status;
+	struct fw_info *fw = &uctx->dimm_fw;
+	int abort;
+
+	abort = err < 0 ? 1 : 0;
+
+	cmd = ndctl_dimm_cmd_new_fw_finish(uctx->dimm);
+	if (!cmd)
+		return -ENXIO;
+
+	rc = ndctl_cmd_fw_finish_set_context(cmd, fw->context);
+	if (rc < 0)
+		goto out;
+
+	rc = ndctl_cmd_fw_finish_set_ctrl_flags(cmd, abort);
+	if (rc < 0)
+		goto out;
+
+	rc = ndctl_cmd_submit(cmd);
+	if (rc < 0)
+		goto out;
+
+	status = ndctl_cmd_get_firmware_status(cmd);
+	if (status != 0 && !abort) {
+		error("FINISH FIRMWARE UPDATE failed: %#x\n", status);
+		rc = -ENXIO;
+		goto out;
+	}
+
+	if (abort && !(status & ND_CMD_STATUS_FIN_ABORTED)) {
+		error("FW update abort failed: %#x\n", status);
+		rc = -ENXIO;
+		goto out;
+	}
+
+out:
+	ndctl_cmd_unref(cmd);
+	return rc;
+}
+
+static int query_fw_finish_status(struct update_context *uctx)
+{
+	struct ndctl_cmd *cmd;
+	int rc;
+	uint32_t status;
+	struct fw_info *fw = &uctx->dimm_fw;
+	bool done = false;
+	struct timeval before, after;
+	struct timespec now;
+	uint64_t ver;
+
+	cmd = ndctl_dimm_cmd_new_fw_finish_query(uctx->dimm);
+	if (!cmd)
+		return -ENXIO;
+
+	rc = ndctl_cmd_fw_fquery_set_context(cmd, fw->context);
+	if (rc < 0)
+		goto out;
+
+	rc = gettimeofday(&before, NULL);
+	if (rc < 0)
+		goto out;
+
+	now.tv_nsec = fw->query_interval / 1000;
+	now.tv_sec = 0;
+
+	do {
+		rc = ndctl_cmd_submit(cmd);
+		if (rc < 0)
+			break;
+
+		status = ndctl_cmd_get_firmware_status(cmd);
+		switch (status & 0xffff) {
+		case ND_CMD_STATUS_SUCCESS:
+			ver = ndctl_cmd_fw_fquery_get_fw_rev(cmd);
+			if (ver == 0) {
+				printf("No firmware updated\n");
+				rc = -ENXIO;
+				goto out;
+			}
+
+			printf("Image %s updated successfully to DIMM %s\n",
+					uctx->fw_path, uctx->dimm_id);
+			printf("Firmware version %#lx.\n", ver);
+			printf("Reboot to activate.\n");
+			done = true;
+			rc = 0;
+			break;
+		case ND_CMD_STATUS_EXTEND:
+			/* Check extended status */
+			switch (status & 0xffff0000) {
+			case ND_CMD_STATUS_FQ_BUSY:
+				/* Still on going, continue */
+				rc = gettimeofday(&after, NULL);
+				if (rc < 0) {
+					rc = -errno;
+					goto out;
+				}
+
+				/*
+				 * If we expire max query time,
+				 * we timed out
+				 */
+				if (after.tv_sec - before.tv_sec >
+						fw->max_query / 1000000) {
+					rc = -ETIMEDOUT;
+					goto out;
+				}
+
+				/*
+				 * Sleep the interval dictated by firmware
+				 * before query again.
+				 */
+				rc = nanosleep(&now, NULL);
+				if (rc < 0) {
+					rc = -errno;
+					goto out;
+				}
+				break;
+			case ND_CMD_STATUS_FQ_BAD:
+				printf("Image failed to verify by DIMM\n");
+			case ND_CMD_STATUS_FQ_CTXINVAL:
+			case ND_CMD_STATUS_FQ_ORDER:
+			default:
+				done = true;
+				rc = -ENXIO;
+				goto out;
+			}
+			break;
+		case ND_CMD_STATUS_NORES:
+			printf("Firmware update sequence timed out\n");
+			rc = -ETIMEDOUT;
+			done = true;
+			goto out;
+		default:
+			rc = -EINVAL;
+			done = true;
+			goto out;
+		}
+	} while (!done);
+
+out:
+	ndctl_cmd_unref(cmd);
+	return rc;
+}
+
+static int update_firmware(struct update_context *uctx)
+{
+	int rc;
+
+	rc = submit_get_firmware_info(uctx);
+	if (rc < 0)
+		return rc;
+
+	rc = submit_start_firmware_upload(uctx);
+	if (rc < 0)
+		return rc;
+
+	printf("Uploading %s to DIMM %s\n", uctx->fw_path, uctx->dimm_id);
+
+	rc = send_firmware(uctx);
+	if (rc < 0) {
+		error("Firmware send failed. Aborting...\n");
+		rc = submit_finish_firmware(uctx, 1);
+		if (rc < 0)
+			error("Aborting update sequence failed\n");
+		return rc;
+	}
+
+	rc = submit_finish_firmware(uctx, rc);
+	if (rc < 0) {
+		error("Unable to end update sequence\n");
+		rc = submit_finish_firmware(uctx, 1);
+		if (rc < 0)
+			error("Aborting update sequence failed\n");
+		return rc;
+	}
+
+	rc = query_fw_finish_status(uctx);
+	if (rc < 0)
+		return rc;
+
+	return 0;
+}
+
+static int get_ndctl_dimm(struct update_context *uctx, void *ctx)
+{
+	struct ndctl_dimm *dimm;
+	struct ndctl_bus *bus;
+
+	ndctl_bus_foreach(ctx, bus)
+		ndctl_dimm_foreach(bus, dimm) {
+			if (!util_dimm_filter(dimm, uctx->dimm_id))
+				continue;
+			uctx->dimm = dimm;
+			return 0;
+		}
+
+	return -ENODEV;
+}
+
+static int verify_fw_file(struct update_context *uctx)
+{
+	struct stat st;
+
+	if (stat(uctx->fw_path, &st) < 0)
+		return -errno;
+	if (!S_ISREG(st.st_mode))
+		return -EINVAL;
+
+	uctx->fw_size = st.st_size;
+	if (uctx->fw_size == 0)
+		return -EINVAL;
+
+	uctx->fw_fd = open(uctx->fw_path, O_RDONLY);
+	if (uctx->fw_fd < 0)
+		return -errno;
+
+	return 0;
+}
+
+int cmd_update_firmware(int argc, const char **argv, void *ctx)
+{
+	struct update_context uctx = { 0 };
+	const struct option options[] = {
+		OPT_STRING('f', "firmware", &uctx.fw_path,
+				"file-name", "name of firmware"),
+		OPT_STRING('d', "dimm", &uctx.dimm_id, "dimm-id",
+				"dimm to be updated"),
+		OPT_END(),
+	};
+	const char * const u[] = {
+		"ndctl update_firmware [<options>]",
+		NULL
+	};
+	int i, rc;
+
+	argc = parse_options(argc, argv, options, u, 0);
+	for (i = 0; i < argc; i++)
+		error("unknown parameter \"%s\"\n", argv[i]);
+	if (argc)
+		usage_with_options(u, options);
+
+	if (!uctx.fw_path) {
+		error("No firmware file provided\n");
+		usage_with_options(u, options);
+		return -EINVAL;
+	}
+
+	if (!uctx.dimm_id) {
+		error("No DIMM ID provided\n");
+		usage_with_options(u, options);
+		return -EINVAL;
+	}
+
+	rc = verify_fw_file(&uctx);
+	if (rc < 0)
+		return rc;
+
+	rc = get_ndctl_dimm(&uctx, ctx);
+	if (rc < 0)
+		return rc;
+
+	rc = update_firmware(&uctx);
+	if (rc < 0)
+		return rc;
+
+	return 0;
+}

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

[PATCH v2 4/4] ndctl, test: firmware update unit test

[Dave Jiang]

Adding a unit test that will use nfit_test kernel module to test the
firmware update sequence.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 test/Makefile.am        |    3 +-
 test/firmware-update.sh |   74 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100755 test/firmware-update.sh

diff --git a/test/Makefile.am b/test/Makefile.am
index d4c2bd6..d5ef648 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -16,7 +16,8 @@ TESTS =\
 	blk-exhaust.sh \
 	sector-mode.sh \
 	inject-error.sh \
-	btt-errors.sh
+	btt-errors.sh \
+	firmware-update.sh
 
 check_PROGRAMS =\
 	libndctl \
diff --git a/test/firmware-update.sh b/test/firmware-update.sh
new file mode 100755
index 0000000..ff777b9
--- /dev/null
+++ b/test/firmware-update.sh
@@ -0,0 +1,74 @@
+#!/bin/bash -Ex
+
+# Copyright(c) 2017 Intel Corporation. All rights reserved.
+#
+# This program is free software; you can redistribute it and/or modify it
+# under the terms of version 2 of the GNU General Public License as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# General Public License for more details.
+
+[ -f "../ndctl/ndctl" ] && [ -x "../ndctl/ndctl" ] && ndctl="../ndctl/ndctl"
+[ -f "./ndctl/ndctl" ] && [ -x "./ndctl/ndctl" ] && ndctl="./ndctl/ndctl"
+[ -z "$ndctl" ] && echo "Couldn't find an ndctl binary" && exit 1
+bus="nfit_test.0"
+json2var="s/[{}\",]//g; s/:/=/g"
+rc=77
+dev=""
+image="update-fw.img"
+
+trap 'err $LINENO' ERR
+
+# $1: Line number
+# $2: exit code
+err()
+{
+	[ -n "$2" ] && rc="$2"
+	echo "test/firmware-update.sh: failed at line $1"
+	exit "$rc"
+}
+
+check_min_kver()
+{
+	local ver="$1"
+	: "${KVER:=$(uname -r)}"
+
+	[ -n "$ver" ] || return 1
+	[[ "$ver" == "$(echo -e "$ver\n$KVER" | sort -V | head -1)" ]]
+}
+
+reset()
+{
+	$ndctl disable-region -b "$bus" all
+	$ndctl zero-labels -b "$bus" all
+	$ndctl enable-region -b "$bus" all
+	if [ -f $image ]; then
+		rm -f $image
+	fi
+}
+
+detect()
+{
+	dev=$($ndctl list -b "$bus" -D | jq .[0].dev | tr -d '"')
+	[ -n "$dev" ] || err "$LINENO" 2
+}
+
+do_tests()
+{
+	fallocate -l 196608 $image
+	$ndctl update-firmware -d $dev -f $image
+}
+
+check_min_kver "4.16" || { echo "kernel $KVER may lack firmware update test handling"; exit
+$rc; }
+modprobe nfit_test
+rc=1
+reset
+detect
+do_tests
+reset
+exit 0
+

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

Re: [PATCH v2 2/4] ndctl: add firmware download support functions in libndctl

[Dan Williams]

On Wed, Dec 6, 2017 at 2:47 PM, Dave Jiang <dave.jiang@intel.com> wrote:
> Adding the DSM commands from Intel DSM v1.6 to support firmware update
> sequence in the libndctl library as additional dimm_ops.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Some more type clarifications and api fixups to make sure this
approach/functionality can be reused for other DIMMs.

> ---
>
> v2:
> Sync'd return types per Dan's comment and removed __uN types.
>
>
>  0 files changed
>
> diff --git a/ndctl/lib/Makefile.am b/ndctl/lib/Makefile.am
> index 5e10fde..e3a12e7 100644
> --- a/ndctl/lib/Makefile.am
> +++ b/ndctl/lib/Makefile.am
> @@ -21,6 +21,7 @@ libndctl_la_SOURCES =\
>         hpe1.c \
>         msft.c \
>         ars.c \
> +       firmware.c \
>         libndctl.c
>
>  libndctl_la_LIBADD =\
> diff --git a/ndctl/lib/firmware.c b/ndctl/lib/firmware.c
> new file mode 100644
> index 0000000..3ec89b7
> --- /dev/null
> +++ b/ndctl/lib/firmware.c
> @@ -0,0 +1,128 @@
> +/*
> + * 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 firmware update:
> + */
> +NDCTL_EXPORT struct ndctl_cmd *
> +ndctl_dimm_cmd_new_fw_get_info(struct ndctl_dimm *dimm)
> +{
> +       struct ndctl_dimm_ops *ops = dimm->ops;
> +
> +       if (ops && ops->new_fw_get_info)
> +               return ops->new_fw_get_info(dimm);
> +       else
> +               return NULL;
> +}
> +
> +NDCTL_EXPORT struct ndctl_cmd *
> +ndctl_dimm_cmd_new_fw_start_update(struct ndctl_dimm *dimm)
> +{
> +       struct ndctl_dimm_ops *ops = dimm->ops;
> +
> +       if (ops && ops->new_fw_start_update)
> +               return ops->new_fw_start_update(dimm);
> +       else
> +               return NULL;
> +}
> +
> +NDCTL_EXPORT struct ndctl_cmd *
> +ndctl_dimm_cmd_new_fw_send(struct ndctl_dimm *dimm, unsigned int chunk_size)
> +{
> +       struct ndctl_dimm_ops *ops = dimm->ops;
> +
> +       if (ops && ops->new_fw_send)
> +               return ops->new_fw_send(dimm, chunk_size);
> +       else
> +               return NULL;
> +}
> +
> +NDCTL_EXPORT struct ndctl_cmd *
> +ndctl_dimm_cmd_new_fw_finish(struct ndctl_dimm *dimm)
> +{
> +       struct ndctl_dimm_ops *ops = dimm->ops;
> +
> +       if (ops && ops->new_fw_finish)
> +               return ops->new_fw_finish(dimm);
> +       else
> +               return NULL;
> +}
> +
> +NDCTL_EXPORT struct ndctl_cmd *
> +ndctl_dimm_cmd_new_fw_finish_query(struct ndctl_dimm *dimm)
> +{
> +       struct ndctl_dimm_ops *ops = dimm->ops;
> +
> +       if (ops && ops->new_fw_finish_query)
> +               return ops->new_fw_finish_query(dimm);
> +       else
> +               return NULL;
> +}
> +
> +
> +#define firmware_cmd_op(op, rettype, defretvalue) \
> +NDCTL_EXPORT rettype ndctl_cmd_##op(struct ndctl_cmd *cmd) \
> +{ \
> +       if (cmd->dimm) { \
> +               struct ndctl_dimm_ops *ops = cmd->dimm->ops; \
> +               if (ops && ops->op) \
> +                       return ops->op(cmd); \
> +       } \
> +       return defretvalue; \
> +}
> +
> +firmware_cmd_op(fw_info_get_storage_size, unsigned int, 0)
> +firmware_cmd_op(fw_info_get_max_send_len, unsigned int, 0)
> +firmware_cmd_op(fw_info_get_query_interval, unsigned int, 0)
> +firmware_cmd_op(fw_info_get_max_query_time, unsigned int, 0)
> +firmware_cmd_op(fw_info_get_fis_version, unsigned int, 0)
> +firmware_cmd_op(fw_info_get_run_version, unsigned long, 0)
> +firmware_cmd_op(fw_info_get_updated_version, unsigned long, 0)
> +firmware_cmd_op(fw_info_get_update_cap, unsigned char, 0)
> +firmware_cmd_op(fw_start_get_context, unsigned int, 0)
> +firmware_cmd_op(fw_fquery_get_fw_rev, unsigned long, 0)

The 64-bit ones should be using "unsigned long long" as the return value.

> +
> +#define firmware_cmd_set_op(op, intype) \
> +NDCTL_EXPORT int ndctl_cmd_##op(struct ndctl_cmd *cmd, intype val) \
> +{ \
> +       if (cmd->dimm) { \
> +               struct ndctl_dimm_ops *ops = cmd->dimm->ops; \
> +               if (ops && ops->op) \
> +                       return ops->op(cmd, val); \
> +       } \
> +       return -ENXIO; \
> +}
> +
> +firmware_cmd_set_op(fw_send_set_context, unsigned int)
> +firmware_cmd_set_op(fw_send_set_offset, unsigned int)
> +firmware_cmd_set_op(fw_send_set_length, unsigned int)
> +firmware_cmd_set_op(fw_finish_set_ctrl_flags, unsigned char)
> +firmware_cmd_set_op(fw_finish_set_context, unsigned int)
> +firmware_cmd_set_op(fw_fquery_set_context, unsigned int)
> +
> +NDCTL_EXPORT int
> +ndctl_cmd_fw_send_set_data(struct ndctl_cmd *cmd, void *buf, size_t len)
> +{
> +       if (cmd->dimm) {
> +               struct ndctl_dimm_ops *ops = cmd->dimm->ops;
> +               if (ops && ops->fw_send_set_data)
> +                       return ops->fw_send_set_data(cmd, buf, len);
> +       }
> +       return -ENXIO;
> +}
> +
> diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
> index b85a682..3ac4d3b 100644
> --- a/ndctl/lib/intel.c
> +++ b/ndctl/lib/intel.c
> @@ -288,6 +288,11 @@ static const char *intel_cmd_desc(int fn)
>         static const char *descs[] = {
>                 [ND_INTEL_SMART] = "smart",
>                 [ND_INTEL_SMART_THRESHOLD] = "smart_thresh",
> +               [ND_INTEL_FW_GET_INFO] = "firmware_get_info",
> +               [ND_INTEL_FW_START_UPDATE] = "firmware_start_update",
> +               [ND_INTEL_FW_SEND_DATA] = "firmware_send_data",
> +               [ND_INTEL_FW_FINISH_UPDATE] = "firmware_finish_update",
> +               [ND_INTEL_FW_FINISH_STATUS_QUERY] = "firmware_finish_query",
>                 [ND_INTEL_SMART_SET_THRESHOLD] = "smart_set_thresh",
>         };
>         const char *desc = descs[fn];
> @@ -299,6 +304,238 @@ static const char *intel_cmd_desc(int fn)
>         return desc;
>  }
>
> +static struct ndctl_cmd *intel_dimm_cmd_new_fw_get_info(struct ndctl_dimm *dimm)
> +{
> +       struct ndctl_cmd *cmd;
> +
> +       BUILD_ASSERT(sizeof(struct nd_intel_fw_info) == 44);
> +
> +       cmd = alloc_intel_cmd(dimm, ND_INTEL_FW_GET_INFO,
> +                       0, sizeof(cmd->intel->info));
> +       if (!cmd)
> +               return NULL;
> +
> +       cmd->firmware_status = &cmd->intel->info.status;
> +       return cmd;
> +}
> +
> +static int intel_fw_get_info_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_FW_GET_INFO)
> +               return -EINVAL;
> +       return 0;
> +}
> +
> +#define intel_fw_info_get_field32(cmd, field) \
> +static unsigned int intel_cmd_fw_info_get_##field( \
> +                       struct ndctl_cmd *cmd) \
> +{ \
> +       if (intel_fw_get_info_valid(cmd) < 0) \
> +               return -EINVAL; \
> +       return cmd->intel->info.field; \

This should be returning UINT_MAX in the error case.

> +}
> +
> +#define intel_fw_info_get_field64(cmd, field) \
> +static unsigned long intel_cmd_fw_info_get_##field( \

Return unsigned long long.

> +                       struct ndctl_cmd *cmd) \
> +{ \
> +       if (intel_fw_get_info_valid(cmd) < 0) \
> +               return -EINVAL; \
> +       return cmd->intel->info.field; \
> +}

Return ULLONG_MAX in the error case.

> +
> +static unsigned char intel_cmd_fw_info_get_update_cap(struct ndctl_cmd *cmd)

just make this unsigned int

> +{
> +       if (intel_fw_get_info_valid(cmd) < 0)
> +               return -EINVAL;

UINT_MAX

> +       return cmd->intel->info.update_cap;
> +}
> +
> +intel_fw_info_get_field32(cmd, storage_size)
> +intel_fw_info_get_field32(cmd, max_send_len)
> +intel_fw_info_get_field32(cmd, query_interval)
> +intel_fw_info_get_field32(cmd, max_query_time);
> +intel_fw_info_get_field32(cmd, fis_version);
> +intel_fw_info_get_field64(cmd, run_version);
> +intel_fw_info_get_field64(cmd, updated_version);
> +
> +static struct ndctl_cmd *intel_dimm_cmd_new_fw_start(struct ndctl_dimm *dimm)
> +{
> +       struct ndctl_cmd *cmd;
> +
> +       BUILD_ASSERT(sizeof(struct nd_intel_fw_start) == 8);
> +
> +       cmd = alloc_intel_cmd(dimm, ND_INTEL_FW_START_UPDATE,
> +                       sizeof(cmd->intel->start) - 4, 4);
> +       if (!cmd)
> +               return NULL;
> +
> +       cmd->firmware_status = &cmd->intel->start.status;
> +       return cmd;
> +}
> +
> +static int intel_fw_start_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_FW_START_UPDATE)
> +               return -EINVAL;
> +       return 0;
> +}
> +
> +static unsigned int intel_cmd_fw_start_get_context(struct ndctl_cmd *cmd)
> +{
> +       if (intel_fw_start_valid(cmd) < 0)
> +               return -EINVAL;

UINT_MAX

> +       return cmd->intel->start.context;
> +}
> +
> +static struct ndctl_cmd *intel_dimm_cmd_new_fw_send(struct ndctl_dimm *dimm,
> +               unsigned int chunk_size)
> +{
> +       struct ndctl_cmd *cmd;
> +
> +       BUILD_ASSERT(sizeof(struct nd_intel_fw_send_data) == 12);
> +
> +       cmd = alloc_intel_cmd(dimm, ND_INTEL_FW_SEND_DATA,
> +               sizeof(cmd->intel->send) + chunk_size, 4);
> +       if (!cmd)
> +               return NULL;
> +
> +       /* the last dword is reserved for status */
> +       cmd->firmware_status =
> +               (unsigned int *)(&cmd->intel->send.data[0] + chunk_size);
> +       return cmd;
> +}
> +
> +static int intel_fw_send_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_FW_SEND_DATA)
> +               return -EINVAL;
> +       return 0;
> +}
> +
> +#define intel_fw_send_set_field(field) \
> +static int intel_cmd_fw_send_set_##field(struct ndctl_cmd *cmd, unsigned int val) \
> +{ \
> +       if (intel_fw_send_valid(cmd) < 0) \
> +               return -EINVAL; \
> +       cmd->intel->send.field = val; \
> +       return 0; \
> +}
> +
> +intel_fw_send_set_field(context)
> +intel_fw_send_set_field(offset)
> +intel_fw_send_set_field(length)
> +
> +static int
> +intel_cmd_fw_send_set_data(struct ndctl_cmd *cmd, void *buf, size_t len)
> +{
> +       if (intel_fw_send_valid(cmd) < 0)
> +               return -EINVAL;
> +
> +       memcpy(cmd->intel->send.data, buf, len);
> +       return 0;
> +}
> +
> +static struct ndctl_cmd *intel_dimm_cmd_new_fw_finish(struct ndctl_dimm *dimm)
> +{
> +       struct ndctl_cmd *cmd;
> +
> +       BUILD_ASSERT(sizeof(struct nd_intel_fw_finish_update) == 12);
> +
> +       cmd = alloc_intel_cmd(dimm, ND_INTEL_FW_FINISH_UPDATE,
> +                       sizeof(cmd->intel->finish) - 4, 4);
> +       if (!cmd)
> +               return NULL;
> +
> +       cmd->firmware_status = &cmd->intel->finish.status;
> +       return cmd;
> +}
> +
> +static int intel_fw_finish_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_FW_FINISH_UPDATE)
> +               return -EINVAL;
> +       return 0;
> +}
> +
> +static int
> +intel_cmd_fw_finish_set_ctrl_flags(struct ndctl_cmd *cmd, unsigned char ctrl)
> +{
> +       if (intel_fw_finish_valid(cmd) < 0)
> +               return -EINVAL;
> +       cmd->intel->finish.ctrl_flags = ctrl;
> +       return 0;
> +}
> +
> +static int
> +intel_cmd_fw_finish_set_context(struct ndctl_cmd *cmd, unsigned int context)
> +{
> +       if (intel_fw_finish_valid(cmd) < 0)
> +               return -EINVAL;
> +       cmd->intel->finish.context = context;
> +       return 0;
> +}
> +
> +static struct ndctl_cmd *
> +intel_dimm_cmd_new_fw_finish_query(struct ndctl_dimm *dimm)
> +{
> +       struct ndctl_cmd *cmd;
> +
> +       BUILD_ASSERT(sizeof(struct nd_intel_fw_finish_update) == 12);
> +
> +       cmd = alloc_intel_cmd(dimm, ND_INTEL_FW_FINISH_UPDATE,
> +                       4, sizeof(cmd->intel->fquery) - 4);
> +       if (!cmd)
> +               return NULL;
> +
> +       cmd->firmware_status = &cmd->intel->finish.status;
> +       return cmd;
> +
> +}
> +
> +static int intel_fw_fquery_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_FW_FINISH_UPDATE)
> +               return -EINVAL;
> +       return 0;
> +}
> +
> +static int
> +intel_cmd_fw_fquery_set_context(struct ndctl_cmd *cmd, unsigned int context)
> +{
> +       if (intel_fw_fquery_valid(cmd) < 0)
> +               return -EINVAL;
> +       cmd->intel->fquery.context = context;
> +       return 0;
> +}
> +
> +static unsigned long intel_cmd_fw_fquery_get_fw_rev(struct ndctl_cmd *cmd)

Is this another 64-bit field? If so, should be unsigned long long

> +{
> +       if (intel_fw_start_valid(cmd) < 0)
> +               return -EINVAL;

...and ULLONG_MAX if it's 64-bit

> +       return cmd->intel->fquery.updated_fw_rev;
> +}
> +
>  struct ndctl_dimm_ops * const intel_dimm_ops = &(struct ndctl_dimm_ops) {
>         .cmd_desc = intel_cmd_desc,
>         .new_smart = intel_dimm_cmd_new_smart,
> @@ -331,4 +568,26 @@ struct ndctl_dimm_ops * const intel_dimm_ops = &(struct ndctl_dimm_ops) {
>         .smart_threshold_set_ctrl_temperature
>                 = intel_cmd_smart_threshold_set_ctrl_temperature,
>         .smart_threshold_set_spares = intel_cmd_smart_threshold_set_spares,
> +       .new_fw_get_info = intel_dimm_cmd_new_fw_get_info,
> +       .fw_info_get_storage_size = intel_cmd_fw_info_get_storage_size,
> +       .fw_info_get_max_send_len = intel_cmd_fw_info_get_max_send_len,
> +       .fw_info_get_query_interval = intel_cmd_fw_info_get_query_interval,
> +       .fw_info_get_max_query_time = intel_cmd_fw_info_get_max_query_time,
> +       .fw_info_get_update_cap = intel_cmd_fw_info_get_update_cap,
> +       .fw_info_get_fis_version = intel_cmd_fw_info_get_fis_version,
> +       .fw_info_get_run_version = intel_cmd_fw_info_get_run_version,
> +       .fw_info_get_updated_version = intel_cmd_fw_info_get_updated_version,
> +       .new_fw_start_update = intel_dimm_cmd_new_fw_start,
> +       .fw_start_get_context = intel_cmd_fw_start_get_context,
> +       .new_fw_send = intel_dimm_cmd_new_fw_send,
> +       .fw_send_set_context = intel_cmd_fw_send_set_context,
> +       .fw_send_set_offset = intel_cmd_fw_send_set_offset,
> +       .fw_send_set_length = intel_cmd_fw_send_set_length,
> +       .fw_send_set_data = intel_cmd_fw_send_set_data,
> +       .new_fw_finish = intel_dimm_cmd_new_fw_finish,
> +       .fw_finish_set_ctrl_flags = intel_cmd_fw_finish_set_ctrl_flags,
> +       .fw_finish_set_context = intel_cmd_fw_finish_set_context,
> +       .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,
>  };
> diff --git a/ndctl/lib/intel.h b/ndctl/lib/intel.h
> index 9e63985..080e37b 100644
> --- a/ndctl/lib/intel.h
> +++ b/ndctl/lib/intel.h
> @@ -5,6 +5,12 @@
>  #define __INTEL_H__
>  #define ND_INTEL_SMART 1
>  #define ND_INTEL_SMART_THRESHOLD 2
> +
> +#define ND_INTEL_FW_GET_INFO 12
> +#define ND_INTEL_FW_START_UPDATE 13
> +#define ND_INTEL_FW_SEND_DATA 14
> +#define ND_INTEL_FW_FINISH_UPDATE 15
> +#define ND_INTEL_FW_FINISH_STATUS_QUERY 16
>  #define ND_INTEL_SMART_SET_THRESHOLD 17
>
>  #define ND_INTEL_SMART_HEALTH_VALID             (1 << 0)
> @@ -71,12 +77,58 @@ struct nd_intel_smart_set_threshold {
>         __u32 status;
>  } __attribute__((packed));
>
> +struct nd_intel_fw_info {
> +       __u32 status;
> +       __u32 storage_size;
> +       __u32 max_send_len;
> +       __u32 query_interval;
> +       __u32 max_query_time;
> +       __u8 update_cap;
> +       __u8 reserved[3];
> +       __u32 fis_version;
> +       __u64 run_version;
> +       __u64 updated_version;
> +} __attribute__((packed));
> +
> +struct nd_intel_fw_start {
> +       __u32 status;
> +       __u32 context;
> +} __attribute__((packed));
> +
> +/* this one has the output first because the variable input data size */
> +struct nd_intel_fw_send_data {
> +       __u32 context;
> +       __u32 offset;
> +       __u32 length;
> +       __u8 data[0];
> +/* reserving last 4 bytes as status */
> +/*     __u32 status; */
> +} __attribute__((packed));
> +
> +struct nd_intel_fw_finish_update {
> +       __u8 ctrl_flags;
> +       __u8 reserved[3];
> +       __u32 context;
> +       __u32 status;
> +} __attribute__((packed));
> +
> +struct nd_intel_fw_finish_query {
> +       __u32 context;
> +       __u32 status;
> +       __u64 updated_fw_rev;
> +} __attribute__((packed));
> +
>  struct nd_pkg_intel {
>         struct nd_cmd_pkg gen;
>         union {
>                 struct nd_intel_smart smart;
>                 struct nd_intel_smart_threshold thresh;
>                 struct nd_intel_smart_set_threshold set_thresh;
> +               struct nd_intel_fw_info info;
> +               struct nd_intel_fw_start start;
> +               struct nd_intel_fw_send_data send;
> +               struct nd_intel_fw_finish_update finish;
> +               struct nd_intel_fw_finish_query fquery;
>         };
>  };
>  #endif /* __INTEL_H__ */
> diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
> index 2ace942..2e248ab 100644
> --- a/ndctl/lib/libndctl.sym
> +++ b/ndctl/lib/libndctl.sym
> @@ -322,4 +322,26 @@ global:
>         ndctl_cmd_smart_threshold_set_ctrl_temperature;
>         ndctl_cmd_smart_threshold_set_spares;
>         ndctl_decode_smart_temperature;
> +       ndctl_dimm_cmd_new_fw_get_info;
> +       ndctl_dimm_cmd_new_fw_start_update;
> +       ndctl_dimm_cmd_new_fw_send;
> +       ndctl_dimm_cmd_new_fw_finish;
> +       ndctl_dimm_cmd_new_fw_finish_query;
> +       ndctl_cmd_fw_info_get_storage_size;
> +       ndctl_cmd_fw_info_get_max_send_len;
> +       ndctl_cmd_fw_info_get_query_interval;
> +       ndctl_cmd_fw_info_get_max_query_time;
> +       ndctl_cmd_fw_info_get_run_version;
> +       ndctl_cmd_fw_info_get_fis_version;
> +       ndctl_cmd_fw_info_get_updated_version;
> +       ndctl_cmd_fw_info_get_update_cap;
> +       ndctl_cmd_fw_start_get_context;
> +       ndctl_cmd_fw_fquery_get_fw_rev;
> +       ndctl_cmd_fw_send_set_context;
> +       ndctl_cmd_fw_send_set_offset;
> +       ndctl_cmd_fw_send_set_length;
> +       ndctl_cmd_fw_send_set_data;
> +       ndctl_cmd_fw_finish_set_ctrl_flags;
> +       ndctl_cmd_fw_finish_set_context;
> +       ndctl_cmd_fw_fquery_set_context;
>  } LIBNDCTL_13;
> diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
> index 6726097..20f9e6e 100644
> --- a/ndctl/lib/private.h
> +++ b/ndctl/lib/private.h
> @@ -303,6 +303,28 @@ struct ndctl_dimm_ops {
>         int (*smart_threshold_set_media_temperature)(struct ndctl_cmd *, unsigned int);
>         int (*smart_threshold_set_ctrl_temperature)(struct ndctl_cmd *, unsigned int);
>         int (*smart_threshold_set_spares)(struct ndctl_cmd *, unsigned int);
> +       struct ndctl_cmd *(*new_fw_get_info)(struct ndctl_dimm *);
> +       unsigned int (*fw_info_get_storage_size)(struct ndctl_cmd *);
> +       unsigned int (*fw_info_get_max_send_len)(struct ndctl_cmd *);
> +       unsigned int (*fw_info_get_query_interval)(struct ndctl_cmd *);
> +       unsigned int (*fw_info_get_max_query_time)(struct ndctl_cmd *);
> +       unsigned char (*fw_info_get_update_cap)(struct ndctl_cmd *);
> +       unsigned int (*fw_info_get_fis_version)(struct ndctl_cmd *);
> +       unsigned long (*fw_info_get_run_version)(struct ndctl_cmd *);
> +       unsigned long (*fw_info_get_updated_version)(struct ndctl_cmd *);
> +       struct ndctl_cmd *(*new_fw_start_update)(struct ndctl_dimm *);
> +       unsigned int (*fw_start_get_context)(struct ndctl_cmd *);
> +       struct ndctl_cmd *(*new_fw_send)(struct ndctl_dimm *, unsigned int chunk_size);
> +       int (*fw_send_set_context)(struct ndctl_cmd *, unsigned int context);
> +       int (*fw_send_set_offset)(struct ndctl_cmd *, unsigned int offset);
> +       int (*fw_send_set_length)(struct ndctl_cmd *, unsigned int len);
> +       int (*fw_send_set_data)(struct ndctl_cmd *, void *buf, size_t len);
> +       struct ndctl_cmd *(*new_fw_finish)(struct ndctl_dimm *);
> +       int (*fw_finish_set_ctrl_flags)(struct ndctl_cmd *, unsigned char flags);
> +       int (*fw_finish_set_context)(struct ndctl_cmd *, unsigned int context);
> +       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_dimm_ops * const intel_dimm_ops;
> diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
> index e9da20e..64a4e99 100644
> --- a/ndctl/libndctl.h
> +++ b/ndctl/libndctl.h
> @@ -15,6 +15,7 @@
>
>  #include <stdbool.h>
>  #include <stdarg.h>
> +#include <stdint.h>
>  #include <unistd.h>
>  #include <errno.h>
>
> @@ -581,6 +582,31 @@ int ndctl_dax_delete(struct ndctl_dax *dax);
>  int ndctl_dax_is_configured(struct ndctl_dax *dax);
>  struct daxctl_region *ndctl_dax_get_daxctl_region(struct ndctl_dax *dax);
>
> +struct ndctl_cmd *ndctl_dimm_cmd_new_fw_get_info(struct ndctl_dimm *dimm);
> +struct ndctl_cmd *ndctl_dimm_cmd_new_fw_start_update(struct ndctl_dimm *dimm);
> +struct ndctl_cmd *ndctl_dimm_cmd_new_fw_send(struct ndctl_dimm *dimm,
> +               unsigned int chunk_size);
> +struct ndctl_cmd *ndctl_dimm_cmd_new_fw_finish(struct ndctl_dimm *dimm);
> +struct ndctl_cmd *ndctl_dimm_cmd_new_fw_finish_query(struct ndctl_dimm *dimm);
> +unsigned int ndctl_cmd_fw_info_get_storage_size(struct ndctl_cmd *cmd);
> +unsigned int ndctl_cmd_fw_info_get_max_send_len(struct ndctl_cmd *cmd);
> +unsigned int ndctl_cmd_fw_info_get_query_interval(struct ndctl_cmd *cmd);
> +unsigned int ndctl_cmd_fw_info_get_max_query_time(struct ndctl_cmd *cmd);
> +unsigned char ndctl_cmd_fw_info_get_update_cap(struct ndctl_cmd *cmd);

The update cap is a field that has a binary representation for the
Intel case, I don't think this is the api want at the generic level.
Lets drop this one.

> +unsigned int ndctl_cmd_fw_info_get_fis_version(struct ndctl_cmd *cmd);

This one is Intel specific and shouldn't be in the public interface.

> +unsigned long ndctl_cmd_fw_info_get_run_version(struct ndctl_cmd *cmd);
> +unsigned long ndctl_cmd_fw_info_get_updated_version(struct ndctl_cmd *cmd);
> +unsigned char ndctl_cmd_fw_info_get_update_cap(struct ndctl_cmd *cmd);

Duplicate?

> +unsigned int ndctl_cmd_fw_start_get_context(struct ndctl_cmd *cmd);
> +unsigned long ndctl_cmd_fw_fquery_get_fw_rev(struct ndctl_cmd *cmd);
> +int ndctl_cmd_fw_send_set_context(struct ndctl_cmd *cmd, unsigned int context);
> +int ndctl_cmd_fw_send_set_offset(struct ndctl_cmd *cmd, unsigned int offset);
> +int ndctl_cmd_fw_send_set_length(struct ndctl_cmd *cmd, unsigned int len);
> +int ndctl_cmd_fw_send_set_data(struct ndctl_cmd *cmd, void *buf, size_t len);
> +int ndctl_cmd_fw_finish_set_ctrl_flags(struct ndctl_cmd *cmd, unsigned char flags);

I think we need explicit 'finish" and "abort" apis since other DIMMs
may not use flags for this and certainly not the same binary
definition of the flags.

> +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);
> +
>  #ifdef __cplusplus
>  } /* extern "C" */
>  #endif
>
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 2/4] ndctl: add firmware download support functions in libndctl

[Dan Williams]

On Wed, Dec 6, 2017 at 2:47 PM, Dave Jiang <dave.jiang@intel.com> wrote:
> Adding the DSM commands from Intel DSM v1.6 to support firmware update
> sequence in the libndctl library as additional dimm_ops.
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
[..]
> +int ndctl_cmd_fw_send_set_context(struct ndctl_cmd *cmd, unsigned int context);
> +int ndctl_cmd_fw_send_set_offset(struct ndctl_cmd *cmd, unsigned int offset);
> +int ndctl_cmd_fw_send_set_length(struct ndctl_cmd *cmd, unsigned int len);
> +int ndctl_cmd_fw_send_set_data(struct ndctl_cmd *cmd, void *buf, size_t len);

These should all be input parameters to ndctl_dimm_cmd_new_fw_send()

> +int ndctl_cmd_fw_finish_set_ctrl_flags(struct ndctl_cmd *cmd, unsigned char flags);
> +int ndctl_cmd_fw_finish_set_context(struct ndctl_cmd *cmd, unsigned int context);

These should be input parameters to ndctl_dimm_cmd_new_fw_finish()

The reason the smart commands don't do this is that there may be many
and variable number of DIMM specific fields that could be set for
different DIMM types. For firmware update these parameters seem
generic / static and can be safely specified at command creation time.

> +int ndctl_cmd_fw_fquery_set_context(struct ndctl_cmd *cmd, unsigned int context);

We shouldn't need to pass around the context value it should be
inherited from the initial ndctl_dimm_cmd_new_fw_start_update()
command.

You can see an example of inheriting implied parameters from other
commands in the case of read-modify-write operations like
intel_dimm_cmd_new_smart_set_threshold() that inherits the defaults
from the result of intel_dimm_cmd_new_smart_threshold().
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 3/4] ndctl: add firmware update command option for ndctl

[Dan Williams]

On Wed, Dec 6, 2017 at 2:47 PM, Dave Jiang <dave.jiang@intel.com> wrote:
> Adding option "update-firmware" to ndctl for update firmware support from
> Intel DSM v1.6. ndctl update-firmware takes an option of -f for a firmware
> binary and a -d for the DIMM name:
> ndctl update-firmware -d nmem0 -f new_firmware.bin
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
[..]
> diff --git a/ndctl/update.c b/ndctl/update.c
> new file mode 100644
> index 0000000..2ac6d21
> --- /dev/null
> +++ b/ndctl/update.c
[..]
> +#define ND_CMD_STATUS_SUCCESS  0
> +#define ND_CMD_STATUS_NOTSUPP  1
> +#define        ND_CMD_STATUS_NOTEXIST  2
> +#define ND_CMD_STATUS_INVALPARM        3
> +#define ND_CMD_STATUS_HWERR    4
> +#define ND_CMD_STATUS_RETRY    5
> +#define ND_CMD_STATUS_UNKNOWN  6
> +#define ND_CMD_STATUS_EXTEND   7
> +#define ND_CMD_STATUS_NORES    8
> +#define ND_CMD_STATUS_NOTREADY 9
> +
> +#define ND_CMD_STATUS_START_BUSY       0x10000
> +#define ND_CMD_STATUS_SEND_CTXINVAL    0x10000
> +#define ND_CMD_STATUS_FIN_CTXINVAL     0x10000
> +#define ND_CMD_STATUS_FIN_DONE         0x20000
> +#define ND_CMD_STATUS_FIN_BAD          0x30000
> +#define ND_CMD_STATUS_FIN_ABORTED      0x40000
> +#define ND_CMD_STATUS_FQ_CTXINVAL      0x10000
> +#define ND_CMD_STATUS_FQ_BUSY          0x20000
> +#define ND_CMD_STATUS_FQ_BAD           0x30000
> +#define ND_CMD_STATUS_FQ_ORDER         0x40000

I think we need to have the DIMM implementation translate these statuses...

> +static int query_fw_finish_status(struct update_context *uctx)
> +{
> +       struct ndctl_cmd *cmd;
> +       int rc;
> +       uint32_t status;
> +       struct fw_info *fw = &uctx->dimm_fw;
> +       bool done = false;
> +       struct timeval before, after;
> +       struct timespec now;
> +       uint64_t ver;
> +
> +       cmd = ndctl_dimm_cmd_new_fw_finish_query(uctx->dimm);
> +       if (!cmd)
> +               return -ENXIO;
> +
> +       rc = ndctl_cmd_fw_fquery_set_context(cmd, fw->context);
> +       if (rc < 0)
> +               goto out;
> +
> +       rc = gettimeofday(&before, NULL);
> +       if (rc < 0)
> +               goto out;
> +
> +       now.tv_nsec = fw->query_interval / 1000;
> +       now.tv_sec = 0;
> +
> +       do {
> +               rc = ndctl_cmd_submit(cmd);
> +               if (rc < 0)
> +                       break;
> +
> +               status = ndctl_cmd_get_firmware_status(cmd);

I think we need a ndctl_cmd_xlat_firmware_status() to translate the
DIMM specific error code into a common error number otherwise this is
immediately not portable to another DIMM's firmware update commands.
For example: https://msdn.microsoft.com/en-us/library/windows/hardware/mt604709(v=vs.85).aspx
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH v2 3/4] ndctl: add firmware update command option for ndctl

[Dan Williams]

On Wed, Dec 6, 2017 at 2:47 PM, Dave Jiang <dave.jiang@intel.com> wrote:
> Adding option "update-firmware" to ndctl for update firmware support from
> Intel DSM v1.6. ndctl update-firmware takes an option of -f for a firmware
> binary and a -d for the DIMM name:
> ndctl update-firmware -d nmem0 -f new_firmware.bin
>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
[..]
> diff --git a/ndctl/lib/intel.c b/ndctl/lib/intel.c
> index 3ac4d3b..6d26a6c 100644
> --- a/ndctl/lib/intel.c
> +++ b/ndctl/lib/intel.c

These changes should be folded into the previous patch.

> @@ -323,7 +323,7 @@ static int intel_fw_get_info_valid(struct ndctl_cmd *cmd)
>  {
>         struct nd_pkg_intel *pkg = cmd->intel;
>
> -       if (cmd->type != ND_CMD_CALL || cmd->status != 1
> +       if (cmd->type != ND_CMD_CALL || cmd->status != 0
>                         || pkg->gen.nd_family != NVDIMM_FAMILY_INTEL
>                         || pkg->gen.nd_command != ND_INTEL_FW_GET_INFO)
>                 return -EINVAL;
> @@ -382,7 +382,7 @@ static int intel_fw_start_valid(struct ndctl_cmd *cmd)
>  {
>         struct nd_pkg_intel *pkg = cmd->intel;
>
> -       if (cmd->type != ND_CMD_CALL || cmd->status != 1
> +       if (cmd->type != ND_CMD_CALL || cmd->status != 0
>                         || pkg->gen.nd_family != NVDIMM_FAMILY_INTEL
>                         || pkg->gen.nd_command != ND_INTEL_FW_START_UPDATE)
>                 return -EINVAL;
> @@ -414,7 +414,7 @@ static struct ndctl_cmd *intel_dimm_cmd_new_fw_send(struct ndctl_dimm *dimm,
>         return cmd;
>  }
>
> -static int intel_fw_send_valid(struct ndctl_cmd *cmd)
> +static int intel_fw_send_set_valid(struct ndctl_cmd *cmd)

Why the name change? The way you initially had it is what I would expect.
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm