Linux-NVDIMM Archive on lore.kernel.org
 help / color / Atom feed
* [ndctl PATCH v2 0/4] Firmware Activation and Test Updates
@ 2020-07-20 23:37 Dan Williams
  2020-07-20 23:37 ` [ndctl PATCH v2 1/4] ndctl/list: Add firmware activation enumeration Dan Williams
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Dan Williams @ 2020-07-20 23:37 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Changes since v1 [1]:
- Update the firmware-activation patches per v3 of the kernel support
  series. Specifically handle the rename of
  {ndbusX,nmemX}/firmware_activate to {ndbusX,nmemX}/firmware/activate,
  add support for ndbusX/firmware/capability, and account for ability to
  specify "quiesce" or "live" to ndbusX/firmware/activate to select the
  activation method.

- This series replaces patch 8, 9, 10, and 12 from the v1 posting.

[1]: http://lore.kernel.org/r/159408961822.2386154.888266173771881937.stgit@dwillia2-desk3.amr.corp.intel.com

---

Some persistent memory devices run a firmware locally on the device /
"DIMM" to perform tasks like media management, capacity provisioning,
and health monitoring. The process of updating that firmware typically
involves a reboot because it has implications for in-flight memory
transactions. However, reboots are disruptive and at least the Intel
persistent memory platform implementation, described by the Intel ACPI
DSM specification [2], has added support for activating firmware at
runtime.

As mentioned in the kernel patches adding support for firmware-activate
[3], ndctl is extended with the following functionality:

1/ The existing update-firmware command will 'arm' devices where the
   firmware image is staged by default.

    ndctl update-firmware all -f firmware_image.bin

2/ The existing ability to enumerate firmware-update capabilities now
   includes firmware activate capabilities at the 'bus' and 'dimm/device'
   level:

    ndctl list -BDF -b nfit_test.0
    [
      {
        "provider":"nfit_test.0",
        "dev":"ndbus2",
        "scrub_state":"idle",
        "firmware":{
          "activate_method":"suspend",
          "activate_state":"idle"
        },
        "dimms":[
          {
            "dev":"nmem1",
            "id":"cdab-0a-07e0-ffffffff",
            "handle":0,
            "phys_id":0,
            "security":"disabled",
            "firmware":{
              "current_version":0,
              "can_update":true
            }
          },
    ...


3/ The new activate-firmware command triggers firmware activation per
   the platform enumerated context, "suspend" vs "live", or can be forced
   to "live" if there is explicit knowledge that allowing applications
   and devices to race the quiesce timeout will have no adverse effects.

    ndctl activate-firmware nfit_test.0 [--force]

[2]: https://pmem.io/documents/IntelOptanePMem_DSM_Interface-V2.0.pdf
[3]: http://lore.kernel.org/r/159528284411.993790.11733759435137949717.stgit@dwillia2-desk3.amr.corp.intel.com 

---

Dan Williams (4):
      ndctl/list: Add firmware activation enumeration
      ndctl/dimm: Auto-arm firmware activation
      ndctl/bus: Add 'activate-firmware' command
      ndctl/test: Test firmware-activation interface


 Documentation/ndctl/Makefile.am                 |    3 
 Documentation/ndctl/ndctl-activate-firmware.txt |  146 +++++++++++++
 Documentation/ndctl/ndctl-list.txt              |   39 +++
 Documentation/ndctl/ndctl-update-firmware.txt   |   16 +
 ndctl/action.h                                  |    1 
 ndctl/builtin.h                                 |    1 
 ndctl/bus.c                                     |  158 +++++++++++++-
 ndctl/dimm.c                                    |  125 ++++++++++-
 ndctl/lib/libndctl.c                            |  257 +++++++++++++++++++++++
 ndctl/lib/libndctl.sym                          |   14 +
 ndctl/lib/private.h                             |    4 
 ndctl/libndctl.h                                |   35 +++
 ndctl/list.c                                    |    3 
 ndctl/ndctl.c                                   |    1 
 test/firmware-update.sh                         |   47 ++++
 util/json.c                                     |  117 +++++++++-
 util/json.h                                     |    3 
 17 files changed, 920 insertions(+), 50 deletions(-)
 create mode 100644 Documentation/ndctl/ndctl-activate-firmware.txt
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH v2 1/4] ndctl/list: Add firmware activation enumeration
  2020-07-20 23:37 [ndctl PATCH v2 0/4] Firmware Activation and Test Updates Dan Williams
@ 2020-07-20 23:37 ` Dan Williams
  2020-07-20 23:38 ` [ndctl PATCH v2 2/4] ndctl/dimm: Auto-arm firmware activation Dan Williams
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Dan Williams @ 2020-07-20 23:37 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

When firmware is staged check if the platform / kernel also supports live
firmware activation and include that state at both the dimm and bus level.
The "last activate result" is included only as a hint to whether a
powercycle is required to activate the current image. If firmware
activation fails the "need_powercycle" flag will appear, if firmware
activation is successful then the "current_version" field will be
up-to-date.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ndctl/ndctl-list.txt |   39 +++++++---
 ndctl/bus.c                        |    2 -
 ndctl/lib/libndctl.c               |  140 ++++++++++++++++++++++++++++++++++++
 ndctl/lib/libndctl.sym             |    7 ++
 ndctl/lib/private.h                |    4 +
 ndctl/libndctl.h                   |   28 +++++++
 ndctl/list.c                       |    3 +
 util/json.c                        |  117 +++++++++++++++++++++++++++---
 util/json.h                        |    3 +
 9 files changed, 318 insertions(+), 25 deletions(-)

diff --git a/Documentation/ndctl/ndctl-list.txt b/Documentation/ndctl/ndctl-list.txt
index 7c7e3ac9d05c..b8d517d784b4 100644
--- a/Documentation/ndctl/ndctl-list.txt
+++ b/Documentation/ndctl/ndctl-list.txt
@@ -132,16 +132,35 @@ include::xable-bus-options.txt[]
 
 -F::
 --firmware::
-	Include dimm firmware info in the listing. For example:
-[verse]
-{
-  "dev":"nmem0",
-  "firmware":{
-      "current_version":0,
-      "next_version":1,
-      "need_powercycle":true
-  }
-}
+	Include firmware info in the listing, including the state and
+	capability of runtime firmware activation:
+
+----
+# ndctl list -BDF
+[
+  {
+    "provider":"nfit_test.0",
+    "dev":"ndbus2",
+    "scrub_state":"idle",
+    "firmware":{
+      "activate_method":"suspend",
+      "activate_state":"idle"
+    },
+    "dimms":[
+      {
+        "dev":"nmem1",
+        "id":"cdab-0a-07e0-ffffffff",
+        "handle":0,
+        "phys_id":0,
+        "security":"disabled",
+        "firmware":{
+          "current_version":0,
+          "can_update":true
+        }
+      },
+...
+]
+----
 
 -X::
 --device-dax::
diff --git a/ndctl/bus.c b/ndctl/bus.c
index 86bbd5178df9..6d5bafb86fe4 100644
--- a/ndctl/bus.c
+++ b/ndctl/bus.c
@@ -92,7 +92,7 @@ static int bus_action(int argc, const char **argv, const char *usage,
 			rc = scrub_action(bus, action);
 			if (rc == 0) {
 				success++;
-				jbus = util_bus_to_json(bus);
+				jbus = util_bus_to_json(bus, 0);
 				if (jbus)
 					json_object_array_add(jbuses, jbus);
 			} else if (!fail)
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index ee737cbbfe3e..628bb9c0cffa 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -819,6 +819,31 @@ static void parse_dimm_flags(struct ndctl_dimm *dimm, char *flags)
 				ndctl_dimm_get_devname(dimm), flags);
 }
 
+static enum ndctl_fwa_state fwa_to_state(const char *fwa)
+{
+	if (strcmp(fwa, "idle") == 0)
+		return NDCTL_FWA_IDLE;
+	if (strcmp(fwa, "busy") == 0)
+		return NDCTL_FWA_BUSY;
+	if (strcmp(fwa, "armed") == 0)
+		return NDCTL_FWA_ARMED;
+	if (strcmp(fwa, "overflow") == 0)
+		return NDCTL_FWA_ARM_OVERFLOW;
+	return NDCTL_FWA_INVALID;
+}
+
+static enum ndctl_fwa_method fwa_method_to_method(const char *fwa_method)
+{
+	if (!fwa_method)
+		return NDCTL_FWA_METHOD_RESET;
+
+	if (strcmp(fwa_method, "quiesce") == 0)
+		return NDCTL_FWA_METHOD_SUSPEND;
+	if (strcmp(fwa_method, "live") == 0)
+		return NDCTL_FWA_METHOD_LIVE;
+	return NDCTL_FWA_METHOD_RESET;
+}
+
 static void *add_bus(void *parent, int id, const char *ctl_base)
 {
 	char buf[SYSFS_ATTR_SIZE];
@@ -880,6 +905,19 @@ static void *add_bus(void *parent, int id, const char *ctl_base)
 	if (!bus->scrub_path)
 		goto err_read;
 
+	sprintf(path, "%s/device/firmware/activate", ctl_base);
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		bus->fwa_state = NDCTL_FWA_INVALID;
+	else
+		bus->fwa_state = fwa_to_state(buf);
+
+	sprintf(path, "%s/device/firmware/capability", ctl_base);
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		bus->fwa_method = fwa_method_to_method(NULL);
+	else
+		bus->fwa_method = fwa_method_to_method(buf);
+
+
 	bus->bus_path = parent_dev_path("char", bus->major, bus->minor);
 	if (!bus->bus_path)
 		goto err_dev_path;
@@ -1436,6 +1474,51 @@ NDCTL_EXPORT int ndctl_bus_wait_for_scrub_completion(struct ndctl_bus *bus)
 	return ndctl_bus_poll_scrub_completion(bus, 0, 0);
 }
 
+NDCTL_EXPORT enum ndctl_fwa_state ndctl_bus_get_fw_activate_state(
+		struct ndctl_bus *bus)
+{
+	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
+	char *path = bus->bus_buf;
+	char buf[SYSFS_ATTR_SIZE];
+	int len = bus->buf_len;
+
+	if (bus->fwa_state == NDCTL_FWA_INVALID)
+		return NDCTL_FWA_INVALID;
+
+	if (snprintf(path, len, "%s/firmware/activate", bus->bus_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+				ndctl_bus_get_devname(bus));
+		return NDCTL_FWA_INVALID;
+	}
+
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		return NDCTL_FWA_INVALID;
+
+	bus->fwa_state = fwa_to_state(buf);
+
+	return bus->fwa_state;
+}
+
+NDCTL_EXPORT enum ndctl_fwa_method ndctl_bus_get_fw_activate_method(struct ndctl_bus *bus)
+{
+	return bus->fwa_method;
+}
+
+static enum ndctl_fwa_result fwa_result_to_result(const char *result)
+{
+	if (strcmp(result, "none") == 0)
+		return NDCTL_FWA_RESULT_NONE;
+	if (strcmp(result, "success") == 0)
+		return NDCTL_FWA_RESULT_SUCCESS;
+	if (strcmp(result, "fail") == 0)
+		return NDCTL_FWA_RESULT_FAIL;
+	if (strcmp(result, "not_staged") == 0)
+		return NDCTL_FWA_RESULT_NOTSTAGED;
+	if (strcmp(result, "need_reset") == 0)
+		return NDCTL_FWA_RESULT_NEEDRESET;
+	return NDCTL_FWA_RESULT_INVALID;
+}
+
 static int ndctl_bind(struct ndctl_ctx *ctx, struct kmod_module *module,
 		const char *devname);
 static int ndctl_unbind(struct ndctl_ctx *ctx, const char *devpath);
@@ -1515,6 +1598,18 @@ static void *add_dimm(void *parent, int id, const char *dimm_base)
 	} else
 		parse_dimm_flags(dimm, buf);
 
+	sprintf(path, "%s/firmware/activate", dimm_base);
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		dimm->fwa_state = NDCTL_FWA_INVALID;
+	else
+		dimm->fwa_state = fwa_to_state(buf);
+
+	sprintf(path, "%s/firmware/result", dimm_base);
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		dimm->fwa_result = NDCTL_FWA_RESULT_INVALID;
+	else
+		dimm->fwa_result = fwa_result_to_result(buf);
+
 	if (!ndctl_bus_has_nfit(bus))
 		goto out;
 
@@ -1998,6 +2093,51 @@ NDCTL_EXPORT int ndctl_dimm_enable(struct ndctl_dimm *dimm)
 	return 0;
 }
 
+NDCTL_EXPORT enum ndctl_fwa_state ndctl_dimm_get_fw_activate_state(
+		struct ndctl_dimm *dimm)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	char *path = dimm->dimm_buf;
+	char buf[SYSFS_ATTR_SIZE];
+	int len = dimm->buf_len;
+
+	if (dimm->fwa_state == NDCTL_FWA_INVALID)
+		return NDCTL_FWA_INVALID;
+
+	if (snprintf(path, len, "%s/firmware/activate", dimm->dimm_path) >= len) {
+		err(ctx, "%s: buffer too small!\n", ndctl_dimm_get_devname(dimm));
+		return NDCTL_FWA_INVALID;
+	}
+
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		return NDCTL_FWA_INVALID;
+
+	dimm->fwa_state = fwa_to_state(buf);
+	return dimm->fwa_state;
+}
+
+NDCTL_EXPORT enum ndctl_fwa_result ndctl_dimm_get_fw_activate_result(
+		struct ndctl_dimm *dimm)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	char *path = dimm->dimm_buf;
+	char buf[SYSFS_ATTR_SIZE];
+	int len = dimm->buf_len;
+
+	if (dimm->fwa_result == NDCTL_FWA_RESULT_INVALID)
+		return NDCTL_FWA_RESULT_INVALID;
+
+	if (snprintf(path, len, "%s/firmware/result", dimm->dimm_path) >= len) {
+		err(ctx, "%s: buffer too small!\n", ndctl_dimm_get_devname(dimm));
+		return NDCTL_FWA_RESULT_INVALID;
+	}
+
+	if (sysfs_read_attr(ctx, path, buf) < 0)
+		return NDCTL_FWA_RESULT_INVALID;
+
+	return fwa_result_to_result(buf);
+}
+
 NDCTL_EXPORT struct ndctl_dimm *ndctl_dimm_get_by_handle(struct ndctl_bus *bus,
 		unsigned int handle)
 {
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index ac575a23d035..37217036b0d8 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -431,3 +431,10 @@ LIBNDCTL_23 {
 	ndctl_region_get_align;
 	ndctl_region_set_align;
 } LIBNDCTL_22;
+
+LIBNDCTL_24 {
+	ndctl_dimm_get_fw_activate_state;
+	ndctl_dimm_get_fw_activate_result;
+	ndctl_bus_get_fw_activate_state;
+	ndctl_bus_get_fw_activate_method;
+} LIBNDCTL_23;
diff --git a/ndctl/lib/private.h b/ndctl/lib/private.h
index 2e537f0a8649..02391631d85e 100644
--- a/ndctl/lib/private.h
+++ b/ndctl/lib/private.h
@@ -79,6 +79,8 @@ struct ndctl_dimm {
 	unsigned long cmd_mask;
 	unsigned long nfit_dsm_mask;
 	long long dirty_shutdown;
+	enum ndctl_fwa_state fwa_state;
+	enum ndctl_fwa_result fwa_result;
 	char *unique_id;
 	char *dimm_path;
 	char *dimm_buf;
@@ -174,6 +176,8 @@ struct ndctl_bus {
 	char *scrub_path;
 	unsigned long cmd_mask;
 	unsigned long nfit_dsm_mask;
+	enum ndctl_fwa_state fwa_state;
+	enum ndctl_fwa_method fwa_method;
 };
 
 /**
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 2580f433ade8..e66a52029481 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -110,6 +110,20 @@ enum ndctl_persistence_domain {
 	PERSISTENCE_UNKNOWN = INT_MAX,
 };
 
+enum ndctl_fwa_state {
+	NDCTL_FWA_INVALID,
+	NDCTL_FWA_IDLE,
+	NDCTL_FWA_ARMED,
+	NDCTL_FWA_BUSY,
+	NDCTL_FWA_ARM_OVERFLOW,
+};
+
+enum ndctl_fwa_method {
+	NDCTL_FWA_METHOD_RESET,
+	NDCTL_FWA_METHOD_SUSPEND,
+	NDCTL_FWA_METHOD_LIVE,
+};
+
 struct ndctl_bus;
 struct ndctl_bus *ndctl_bus_get_first(struct ndctl_ctx *ctx);
 struct ndctl_bus *ndctl_bus_get_next(struct ndctl_bus *bus);
@@ -139,6 +153,8 @@ unsigned int ndctl_bus_get_scrub_count(struct ndctl_bus *bus);
 int ndctl_bus_get_scrub_state(struct ndctl_bus *bus);
 int ndctl_bus_start_scrub(struct ndctl_bus *bus);
 int ndctl_bus_has_error_injection(struct ndctl_bus *bus);
+enum ndctl_fwa_state ndctl_bus_get_fw_activate_state(struct ndctl_bus *bus);
+enum ndctl_fwa_method ndctl_bus_get_fw_activate_method(struct ndctl_bus *bus);
 
 struct ndctl_dimm;
 struct ndctl_dimm *ndctl_dimm_get_first(struct ndctl_bus *bus);
@@ -702,6 +718,18 @@ enum ND_FW_STATUS ndctl_cmd_fw_xlat_firmware_status(struct ndctl_cmd *cmd);
 struct ndctl_cmd *ndctl_dimm_cmd_new_ack_shutdown_count(struct ndctl_dimm *dimm);
 int ndctl_dimm_fw_update_supported(struct ndctl_dimm *dimm);
 
+enum ndctl_fwa_result {
+        NDCTL_FWA_RESULT_INVALID,
+        NDCTL_FWA_RESULT_NONE,
+        NDCTL_FWA_RESULT_SUCCESS,
+        NDCTL_FWA_RESULT_NOTSTAGED,
+        NDCTL_FWA_RESULT_NEEDRESET,
+        NDCTL_FWA_RESULT_FAIL,
+};
+
+enum ndctl_fwa_state ndctl_dimm_get_fw_activate_state(struct ndctl_dimm *dimm);
+enum ndctl_fwa_result ndctl_dimm_get_fw_activate_result(struct ndctl_dimm *dimm);
+
 int ndctl_cmd_xlat_firmware_status(struct ndctl_cmd *cmd);
 int ndctl_cmd_submit_xlat(struct ndctl_cmd *cmd);
 
diff --git a/ndctl/list.c b/ndctl/list.c
index 1f7cc8ee1deb..f98148aea479 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -403,11 +403,12 @@ static bool filter_bus(struct ndctl_bus *bus, struct util_filter_ctx *ctx)
 		}
 	}
 
-	lfa->jbus = util_bus_to_json(bus);
+	lfa->jbus = util_bus_to_json(bus, lfa->flags);
 	if (!lfa->jbus) {
 		fail("\n");
 		return false;
 	}
+
 	json_object_array_add(lfa->jbuses, lfa->jbus);
 	return true;
 }
diff --git a/util/json.c b/util/json.c
index 59a3d07cc4a6..77bd4781551d 100644
--- a/util/json.c
+++ b/util/json.c
@@ -122,10 +122,10 @@ void util_display_json_array(FILE *f_out, struct json_object *jarray,
 	json_object_put(jarray);
 }
 
-struct json_object *util_bus_to_json(struct ndctl_bus *bus)
+struct json_object *util_bus_to_json(struct ndctl_bus *bus, unsigned long flags)
 {
 	struct json_object *jbus = json_object_new_object();
-	struct json_object *jobj;
+	struct json_object *jobj, *fw_obj = NULL;
 	int scrub;
 
 	if (!jbus)
@@ -150,6 +150,49 @@ struct json_object *util_bus_to_json(struct ndctl_bus *bus)
 		goto err;
 	json_object_object_add(jbus, "scrub_state", jobj);
 
+	if (flags & UTIL_JSON_FIRMWARE) {
+		struct ndctl_dimm *dimm;
+
+		/*
+		 * Skip displaying firmware activation capability if no
+		 * DIMMs support firmware update.
+		 */
+		ndctl_dimm_foreach(bus, dimm)
+			if (ndctl_dimm_fw_update_supported(dimm) == 0) {
+				fw_obj = json_object_new_object();
+				break;
+			}
+	}
+
+	if (fw_obj) {
+		enum ndctl_fwa_state state;
+		enum ndctl_fwa_method method;
+
+		jobj = NULL;
+		method = ndctl_bus_get_fw_activate_method(bus);
+		if (method == NDCTL_FWA_METHOD_RESET)
+			jobj = json_object_new_string("reset");
+		if (method == NDCTL_FWA_METHOD_SUSPEND)
+			jobj = json_object_new_string("suspend");
+		if (method == NDCTL_FWA_METHOD_LIVE)
+			jobj = json_object_new_string("live");
+		if (jobj)
+			json_object_object_add(fw_obj, "activate_method", jobj);
+
+		jobj = NULL;
+		state = ndctl_bus_get_fw_activate_state(bus);
+		if (state == NDCTL_FWA_ARMED)
+			jobj = json_object_new_string("armed");
+		if (state == NDCTL_FWA_IDLE)
+			jobj = json_object_new_string("idle");
+		if (state == NDCTL_FWA_ARM_OVERFLOW)
+			jobj = json_object_new_string("overflow");
+		if (jobj)
+			json_object_object_add(fw_obj, "activate_state", jobj);
+
+		json_object_object_add(jbus, "firmware", fw_obj);
+	}
+
 	return jbus;
  err:
 	json_object_put(jbus);
@@ -160,10 +203,13 @@ struct json_object *util_dimm_firmware_to_json(struct ndctl_dimm *dimm,
 		unsigned long flags)
 {
 	struct json_object *jfirmware = json_object_new_object();
+	bool can_update, need_powercycle;
+	enum ndctl_fwa_result result;
+	enum ndctl_fwa_state state;
 	struct json_object *jobj;
 	struct ndctl_cmd *cmd;
-	int rc;
 	uint64_t run, next;
+	int rc;
 
 	if (!jfirmware)
 		return NULL;
@@ -195,10 +241,12 @@ struct json_object *util_dimm_firmware_to_json(struct ndctl_dimm *dimm,
 		json_object_object_add(jfirmware, "current_version", jobj);
 
 	rc = ndctl_dimm_fw_update_supported(dimm);
-	jobj = json_object_new_boolean(rc == 0);
+	can_update = rc == 0;
+	jobj = json_object_new_boolean(can_update);
 	if (jobj)
 		json_object_object_add(jfirmware, "can_update", jobj);
 
+
 	next = ndctl_cmd_fw_info_get_updated_version(cmd);
 	if (next == ULLONG_MAX) {
 		jobj = util_json_object_hex(-1, flags);
@@ -208,16 +256,61 @@ struct json_object *util_dimm_firmware_to_json(struct ndctl_dimm *dimm,
 		goto out;
 	}
 
-	if (next != 0) {
-		jobj = util_json_object_hex(next, flags);
-		if (jobj)
-			json_object_object_add(jfirmware,
-					"next_version", jobj);
+	if (!next)
+		goto out;
+
+	jobj = util_json_object_hex(next, flags);
+	if (jobj)
+		json_object_object_add(jfirmware,
+				"next_version", jobj);
+
+	state = ndctl_dimm_get_fw_activate_state(dimm);
+	switch (state) {
+	case NDCTL_FWA_IDLE:
+		jobj = json_object_new_string("idle");
+		break;
+	case NDCTL_FWA_ARMED:
+		jobj = json_object_new_string("armed");
+		break;
+	case NDCTL_FWA_BUSY:
+		jobj = json_object_new_string("busy");
+		break;
+	default:
+		jobj = NULL;
+		break;
+	}
+	if (jobj)
+		json_object_object_add(jfirmware, "activate_state", jobj);
+
+	result = ndctl_dimm_get_fw_activate_result(dimm);
+	switch (result) {
+	case NDCTL_FWA_RESULT_NONE:
+	case NDCTL_FWA_RESULT_SUCCESS:
+	case NDCTL_FWA_RESULT_NOTSTAGED:
+		/*
+		 * If a 'next' firmware version is staged then this
+		 * result is stale, if the activation succeeds that is
+		 * indicated by not finding a 'next' entry.
+		 */
+		need_powercycle = false;
+		break;
+	case NDCTL_FWA_RESULT_NEEDRESET:
+	case NDCTL_FWA_RESULT_FAIL:
+	default:
+		/*
+		 * If the last activation failed, or if the activation
+		 * result is unavailable it is always the case that the
+		 * only remediation is powercycle.
+		 */
+		need_powercycle = true;
+		break;
+	}
 
+	if (need_powercycle) {
 		jobj = json_object_new_boolean(true);
-		if (jobj)
-			json_object_object_add(jfirmware,
-					"need_powercycle", jobj);
+		if (!jobj)
+			goto out;
+		json_object_object_add(jfirmware, "need_powercycle", jobj);
 	}
 
 	ndctl_cmd_unref(cmd);
diff --git a/util/json.h b/util/json.h
index fc91a8db034f..39a33789bac9 100644
--- a/util/json.h
+++ b/util/json.h
@@ -32,7 +32,8 @@ enum util_json_flags {
 struct json_object;
 void util_display_json_array(FILE *f_out, struct json_object *jarray,
 		unsigned long flags);
-struct json_object *util_bus_to_json(struct ndctl_bus *bus);
+struct json_object *util_bus_to_json(struct ndctl_bus *bus,
+		unsigned long flags);
 struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm,
 		unsigned long flags);
 struct json_object *util_mapping_to_json(struct ndctl_mapping *mapping,
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH v2 2/4] ndctl/dimm: Auto-arm firmware activation
  2020-07-20 23:37 [ndctl PATCH v2 0/4] Firmware Activation and Test Updates Dan Williams
  2020-07-20 23:37 ` [ndctl PATCH v2 1/4] ndctl/list: Add firmware activation enumeration Dan Williams
@ 2020-07-20 23:38 ` Dan Williams
  2020-07-20 23:38 ` [ndctl PATCH v2 3/4] ndctl/bus: Add 'activate-firmware' command Dan Williams
  2020-07-20 23:38 ` [ndctl PATCH v2 4/4] ndctl/test: Test firmware-activation interface Dan Williams
  3 siblings, 0 replies; 5+ messages in thread
From: Dan Williams @ 2020-07-20 23:38 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Add an option to control firmware-activation arming and enable it by
default. The arming process checks for the "arm overflow" condition and
disarms dimms if the last arming caused the overflow to be indicated. The
--force option skips checking for arm-overflow. The --disarm option toggles
arming off and can be specified without a firmware-image to just perform
the disarm operation in isolation.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ndctl/ndctl-update-firmware.txt |   16 +++
 ndctl/dimm.c                                  |  125 +++++++++++++++++++++++--
 ndctl/lib/libndctl.c                          |   31 ++++++
 ndctl/lib/libndctl.sym                        |    2 
 ndctl/libndctl.h                              |    2 
 5 files changed, 164 insertions(+), 12 deletions(-)

diff --git a/Documentation/ndctl/ndctl-update-firmware.txt b/Documentation/ndctl/ndctl-update-firmware.txt
index bcf61abaa989..1080d62a20b9 100644
--- a/Documentation/ndctl/ndctl-update-firmware.txt
+++ b/Documentation/ndctl/ndctl-update-firmware.txt
@@ -50,7 +50,21 @@ include::xable-bus-options.txt[]
 -i::
 --force::
 	Ignore in-progress Address Range Scrub and try to submit the
-	firmware update.
+	firmware update, or ignore firmware activate arm overflows and
+	force-arm devices.
+
+-A::
+--arm::
+	Arm a device for firmware activation. This is enabled by default
+	when a firmware image is specified. Specify --no-arm to disable
+	this default. Otherwise, without a firmware image, this option can be
+	used to manually arm a device for firmware activate.
+
+-D::
+--disarm::
+	Disarm devices after uploading the firmware file, or manually
+	disarm devices when a firmware image is not specified.
+	--no-disarm is not accepted.
 
 -v::
 --verbose::
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index e02f5dfdb889..90eb0b8013ae 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -59,10 +59,15 @@ static struct parameters {
 	bool master_pass;
 	bool human;
 	bool force;
+	bool arm;
+	bool arm_set;
+	bool disarm;
+	bool disarm_set;
 	bool index;
 	bool json;
 	bool verbose;
 } param = {
+	.arm = true,
 	.labelversion = "1.1",
 };
 
@@ -694,6 +699,72 @@ out:
 	return rc;
 }
 
+static enum ndctl_fwa_state fw_update_arm(struct ndctl_dimm *dimm)
+{
+	struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
+	const char *devname = ndctl_dimm_get_devname(dimm);
+	enum ndctl_fwa_state state = ndctl_bus_get_fw_activate_state(bus);
+
+	if (state == NDCTL_FWA_INVALID) {
+		if (param.verbose)
+			err("%s: firmware activate capability not found\n",
+					devname);
+		return NDCTL_FWA_INVALID;
+	}
+
+	if (state == NDCTL_FWA_ARM_OVERFLOW && !param.force) {
+		err("%s: overflow detected skip arm\n", devname);
+		return NDCTL_FWA_INVALID;
+	}
+
+	state = ndctl_dimm_fw_activate_arm(dimm);
+	if (state != NDCTL_FWA_ARMED) {
+		err("%s: failed to arm\n", devname);
+		return NDCTL_FWA_INVALID;
+	}
+
+	if (param.force)
+		return state;
+
+	state = ndctl_bus_get_fw_activate_state(bus);
+	if (state == NDCTL_FWA_ARM_OVERFLOW) {
+		err("%s: arm aborted, tripped overflow\n", devname);
+		ndctl_dimm_fw_activate_disarm(dimm);
+		return NDCTL_FWA_INVALID;
+	}
+	return NDCTL_FWA_ARMED;
+}
+
+#define ARM_FAILURE_FATAL (1)
+#define ARM_FAILURE_OK (0)
+
+static int fw_update_toggle_arm(struct ndctl_dimm *dimm,
+		struct json_object *jdimms, bool arm_fatal)
+{
+	enum ndctl_fwa_state state;
+	struct json_object *jobj;
+	unsigned long flags;
+
+	if (param.disarm)
+		state = ndctl_dimm_fw_activate_disarm(dimm);
+	else if (param.arm)
+		state = fw_update_arm(dimm);
+	else
+		state = NDCTL_FWA_INVALID;
+
+	if (state == NDCTL_FWA_INVALID && arm_fatal)
+		return -ENXIO;
+
+	flags = UTIL_JSON_FIRMWARE;
+	if (isatty(1))
+		flags |= UTIL_JSON_HUMAN;
+	jobj = util_dimm_to_json(dimm, flags);
+	if (jobj)
+		json_object_array_add(jdimms, jobj);
+
+	return 0;
+}
+
 static int query_fw_finish_status(struct ndctl_dimm *dimm,
 		struct action_context *actx)
 {
@@ -701,10 +772,8 @@ static int query_fw_finish_status(struct ndctl_dimm *dimm,
 	struct update_context *uctx = &actx->update;
 	struct fw_info *fw = &uctx->dimm_fw;
 	struct timespec now, before, after;
-	struct json_object *jobj;
 	enum ND_FW_STATUS status;
 	struct ndctl_cmd *cmd;
-	unsigned long flags;
 	uint64_t ver;
 	int rc;
 
@@ -765,12 +834,13 @@ again:
 			goto unref;
 		}
 
-		flags = UTIL_JSON_FIRMWARE;
-		if (isatty(1))
-			flags |= UTIL_JSON_HUMAN;
-		jobj = util_dimm_to_json(dimm, flags);
-		if (jobj)
-			json_object_array_add(actx->jdimms, jobj);
+		/*
+		 * Now try to arm/disarm firmware activation if
+		 * requested.  Failure to toggle the arm state is not
+		 * fatal, the success / failure will be inferred from
+		 * the emitted json state.
+		 */
+		fw_update_toggle_arm(dimm, actx->jdimms, ARM_FAILURE_OK);
 		rc = 0;
 		break;
 	case FW_EBADFW:
@@ -846,6 +916,10 @@ static int action_update(struct ndctl_dimm *dimm, struct action_context *actx)
 	const char *devname = ndctl_dimm_get_devname(dimm);
 	int rc;
 
+	if (!param.infile)
+		return fw_update_toggle_arm(dimm, actx->jdimms,
+				ARM_FAILURE_FATAL);
+
 	rc = ndctl_dimm_fw_update_supported(dimm);
 	switch (rc) {
 	case -ENOTTY:
@@ -1090,7 +1164,11 @@ OPT_STRING('i', "input", &param.infile, "input-file", \
 #define UPDATE_OPTIONS() \
 OPT_STRING('f', "firmware", &param.infile, "firmware-file", \
 	"firmware filename for update"), \
-OPT_BOOLEAN('i', "force", &param.force, "ignore ARS status, try to force update")
+OPT_BOOLEAN('i', "force", &param.force, "ignore ARS / arm status, try to force update"), \
+OPT_BOOLEAN_SET('A', "arm", &param.arm, &param.arm_set, \
+	"arm device for firmware activation (default)"), \
+OPT_BOOLEAN_SET('D', "disarm", &param.disarm, &param.disarm_set, \
+	"disarm device for firmware activation")
 
 #define INIT_OPTIONS() \
 OPT_BOOLEAN('f', "force", &param.force, \
@@ -1237,10 +1315,35 @@ static int dimm_action(int argc, const char **argv, struct ndctl_ctx *ctx,
 		}
 	}
 
+	if (param.arm_set && param.disarm_set) {
+		fprintf(stderr, "set either --arm, or --disarm, not both\n");
+		usage_with_options(u, options);
+	}
+
+	if (param.disarm_set && !param.disarm) {
+		fprintf(stderr, "--no-disarm syntax not supported\n");
+		usage_with_options(u, options);
+		return -EINVAL;
+	}
+
 	if (!param.infile) {
+		/*
+		 * Update needs an infile unless we are only being
+		 * called to toggle the arm state. Other actions either
+		 * do no need an input file, or are prepared for stdin.
+		 */
 		if (action == action_update) {
-			usage_with_options(u, options);
-			return -EINVAL;
+			if (!param.arm_set && !param.disarm_set) {
+				fprintf(stderr, "require --arm, or --disarm\n");
+				usage_with_options(u, options);
+				return -EINVAL;
+			}
+
+			if (param.arm_set && !param.arm) {
+				fprintf(stderr, "--no-arm syntax not supported\n");
+				usage_with_options(u, options);
+				return -EINVAL;
+			}
 		}
 		actx.f_in = stdin;
 	} else {
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index 628bb9c0cffa..a4bca6d05022 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2093,6 +2093,37 @@ NDCTL_EXPORT int ndctl_dimm_enable(struct ndctl_dimm *dimm)
 	return 0;
 }
 
+static int dimm_set_arm(struct ndctl_dimm *dimm, bool arm)
+{
+	struct ndctl_ctx *ctx = ndctl_dimm_get_ctx(dimm);
+	char *path = dimm->dimm_buf;
+	int len = dimm->buf_len;
+
+	if (dimm->fwa_state == NDCTL_FWA_INVALID)
+		return NDCTL_FWA_INVALID;
+
+	if (snprintf(path, len, "%s/firmware/activate", dimm->dimm_path) >= len) {
+		err(ctx, "%s: buffer too small!\n", ndctl_dimm_get_devname(dimm));
+		return NDCTL_FWA_INVALID;
+	}
+
+	if (sysfs_write_attr(ctx, path, arm ? "arm" : "disarm") < 0)
+		return NDCTL_FWA_INVALID;
+	return NDCTL_FWA_ARMED;
+}
+
+NDCTL_EXPORT enum ndctl_fwa_state ndctl_dimm_fw_activate_disarm(
+		struct ndctl_dimm *dimm)
+{
+	return dimm_set_arm(dimm, false);
+}
+
+NDCTL_EXPORT enum ndctl_fwa_state ndctl_dimm_fw_activate_arm(
+		struct ndctl_dimm *dimm)
+{
+	return dimm_set_arm(dimm, true);
+}
+
 NDCTL_EXPORT enum ndctl_fwa_state ndctl_dimm_get_fw_activate_state(
 		struct ndctl_dimm *dimm)
 {
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 37217036b0d8..269ac8693304 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -437,4 +437,6 @@ LIBNDCTL_24 {
 	ndctl_dimm_get_fw_activate_result;
 	ndctl_bus_get_fw_activate_state;
 	ndctl_bus_get_fw_activate_method;
+	ndctl_dimm_fw_activate_disarm;
+	ndctl_dimm_fw_activate_arm;
 } LIBNDCTL_23;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index e66a52029481..04ca127767ac 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -729,6 +729,8 @@ enum ndctl_fwa_result {
 
 enum ndctl_fwa_state ndctl_dimm_get_fw_activate_state(struct ndctl_dimm *dimm);
 enum ndctl_fwa_result ndctl_dimm_get_fw_activate_result(struct ndctl_dimm *dimm);
+enum ndctl_fwa_state ndctl_dimm_fw_activate_disarm(struct ndctl_dimm *dimm);
+enum ndctl_fwa_state ndctl_dimm_fw_activate_arm(struct ndctl_dimm *dimm);
 
 int ndctl_cmd_xlat_firmware_status(struct ndctl_cmd *cmd);
 int ndctl_cmd_submit_xlat(struct ndctl_cmd *cmd);
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH v2 3/4] ndctl/bus: Add 'activate-firmware' command
  2020-07-20 23:37 [ndctl PATCH v2 0/4] Firmware Activation and Test Updates Dan Williams
  2020-07-20 23:37 ` [ndctl PATCH v2 1/4] ndctl/list: Add firmware activation enumeration Dan Williams
  2020-07-20 23:38 ` [ndctl PATCH v2 2/4] ndctl/dimm: Auto-arm firmware activation Dan Williams
@ 2020-07-20 23:38 ` Dan Williams
  2020-07-20 23:38 ` [ndctl PATCH v2 4/4] ndctl/test: Test firmware-activation interface Dan Williams
  3 siblings, 0 replies; 5+ messages in thread
From: Dan Williams @ 2020-07-20 23:38 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

For platforms where firmware can be activated at runtime, trigger
that activation. The activation method, or system context while the
activation is performed is determined by the driver between "suspend" and
"live". Where "live" has no memory controller side-effects during the
activation and "suspend" indicates that the platform will take the memory
controller offline for a short period of time.

An override, to attempt "live" activation, is implemented in the --force
option to activate-firmware.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 Documentation/ndctl/Makefile.am                 |    3 
 Documentation/ndctl/ndctl-activate-firmware.txt |  146 +++++++++++++++++++++
 ndctl/action.h                                  |    1 
 ndctl/builtin.h                                 |    1 
 ndctl/bus.c                                     |  158 ++++++++++++++++++++++-
 ndctl/lib/libndctl.c                            |   86 +++++++++++++
 ndctl/lib/libndctl.sym                          |    5 +
 ndctl/libndctl.h                                |    5 +
 ndctl/ndctl.c                                   |    1 
 9 files changed, 397 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/ndctl/ndctl-activate-firmware.txt

diff --git a/Documentation/ndctl/Makefile.am b/Documentation/ndctl/Makefile.am
index b8e239107ff9..0278c783ea66 100644
--- a/Documentation/ndctl/Makefile.am
+++ b/Documentation/ndctl/Makefile.am
@@ -57,7 +57,8 @@ man1_MANS = \
 	ndctl-load-keys.1 \
 	ndctl-wait-overwrite.1 \
 	ndctl-read-infoblock.1 \
-	ndctl-write-infoblock.1
+	ndctl-write-infoblock.1 \
+	ndctl-activate-firmware.1
 
 EXTRA_DIST = $(man1_MANS)
 
diff --git a/Documentation/ndctl/ndctl-activate-firmware.txt b/Documentation/ndctl/ndctl-activate-firmware.txt
new file mode 100644
index 000000000000..bff534ed358a
--- /dev/null
+++ b/Documentation/ndctl/ndctl-activate-firmware.txt
@@ -0,0 +1,146 @@
+// SPDX-License-Identifier: GPL-2.0
+
+ndctl-activate-firmware(1)
+==========================
+
+NAME
+----
+ndctl-activate-firmware - activate staged firmware on memory devices
+
+SYNOPSIS
+--------
+[verse]
+'ndctl activate-firmware' [<bus-id> <bus-id2> ... <bus-idN>] [<options>]
+
+Some persistent memory devices run a firmware locally on the device /
+"DIMM" to perform tasks like media management, capacity provisioning,
+and health monitoring. The process of updating that firmware typically
+involves a reboot because it has implications for in-flight memory
+transactions. However, reboots can be costly for systems that can not
+tolerate extended downtime.
+
+The kernel detects platforms that expose support for
+runtime-firmware-activation (FWA). The 'ndctl update-firmware' stages
+new firmware binaries, but if the platform supports FWA it will
+additionally arm the devices for activation. Then 'ndctl
+activate-firmware' may attempt to activate the firmware live. However,
+if the platform indicates that the memory controller will be taken
+off-line for the duration of the update "activate_method == suspend"
+then the default policy for firmware activation is to inject a truncated
+hibernate cycle to freeze devices and applications before the hard
+quiesce is injected by the platform, and then resume the system.
+
+*DANGER* the activate-firmware command includes a --force option to tell
+the driver bypass the hibernation cycle and perform the update "live".
+I.e. it arranges for applications and devices to race the platform
+injected quiesce period. This option should only be used explicit
+knowledge that the platform quiesce time will not trigger completion
+timeout violations for any devices in the system.
+
+EXAMPLES
+--------
+
+Check for any buses that support activation without triggering an
+activation:
+
+----
+# ndctl activate-firmware all --dry-run
+ACPI.NFIT: ndbus1: has no devices that support firmware update.
+nfit_test.1: ndbus3: has no devices that support firmware update.
+e820: ndbus0: has no devices that support firmware update.
+[
+  {
+    "provider":"nfit_test.0",
+    "dev":"ndbus1",
+    "scrub_state":"idle",
+    "firmware":{
+      "activate_method":"suspend",
+      "activate_state":"idle"
+    },
+    "dimms":[
+      {
+...
+----
+
+
+Check that a specific bus supports activation without performing an activation:
+
+----
+# ndctl activate-firmware nfit_test.0 --dry-run --force
+[
+  {
+    "provider":"nfit_test.0",
+    "dev":"ndbus2",
+    "scrub_state":"idle",
+    "firmware":{
+      "activate_method":"suspend",
+      "activate_state":"idle"
+    },
+    "dimms":[
+...
+]
+----
+
+The result is equivalent to 'ndctl list -BFDu' upon successful
+activation.
+
+The 'ndctl list' command can also enumerate the default activation
+method:
+
+----
+# ndctl list -b nfit_test.0 -BF
+[
+  {
+    "provider":"nfit_test.0",
+    "dev":"ndbus2",
+    "scrub_state":"idle",
+    "firmware":{
+      "activate_method":"suspend",
+      "activate_state":"idle"
+    }
+  }
+]
+----
+
+OPTIONS
+-------
+-n::
+--dry-run::
+	Perform all actions related to activation including honoring
+	--idle and --force, but skip the final execution of the
+	activation. The overrides are undone before the command
+	completes. Any failed overrides will be reported as error
+	messages.
+
+-I::
+--idle::
+	Implied by default, this option controls whether the platform
+	will attempt to increase the completion timeout of all devices
+	in the system and validate that the max completion timeout
+	satisfies the time needed to perform the activation. This
+	validation step can be overridden by specifying --no-idle.
+
+-f::
+--force::
+	The activation method defaults to the reported
+	"bus.firmware.activate_method" property. When the method is
+	"live" then this --force option is ignored. When the method is
+	"reset" no runtime activation is attempted. When the method is
+	"suspend" this option indicates to the driver to bypass the
+	hibernate cycle to activate firmware.  in the bus When the
+	reported "activate_method" is "suspend" the kernel driver may
+	support overriding the suspend requirement and instead issue the
+	firmware-activation live. *CAUTION* this may lead to undefined
+	system behavior if device completion timeouts are violated for
+	in-flight memory operations.
+
+-v::
+--verbose::
+	Emit debug messages for the firmware activation procedure
+
+include::../copyright.txt[]
+
+SEE ALSO
+--------
+linkndctl:ndctl-update-firmware[1],
+https://pmem.io/documents/IntelOptanePMem_DSM_Interface-V2.0.pdf[Intel Optane PMem DSM Interface]
diff --git a/ndctl/action.h b/ndctl/action.h
index bcf6bf3196c6..51f8ee6f4bce 100644
--- a/ndctl/action.h
+++ b/ndctl/action.h
@@ -14,6 +14,7 @@ enum device_action {
 	ACTION_WAIT,
 	ACTION_START,
 	ACTION_CLEAR,
+	ACTION_ACTIVATE,
 	ACTION_READ_INFOBLOCK,
 	ACTION_WRITE_INFOBLOCK,
 };
diff --git a/ndctl/builtin.h b/ndctl/builtin.h
index 8aeada86c1a7..5de7379ce1b4 100644
--- a/ndctl/builtin.h
+++ b/ndctl/builtin.h
@@ -24,6 +24,7 @@ int cmd_init_labels(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_check_labels(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_inject_error(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_wait_scrub(int argc, const char **argv, struct ndctl_ctx *ctx);
+int cmd_activate_firmware(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_start_scrub(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_list(int argc, const char **argv, struct ndctl_ctx *ctx);
 int cmd_monitor(int argc, const char **argv, struct ndctl_ctx *ctx);
diff --git a/ndctl/bus.c b/ndctl/bus.c
index 6d5bafb86fe4..47053c8af389 100644
--- a/ndctl/bus.c
+++ b/ndctl/bus.c
@@ -10,14 +10,19 @@
 #include <util/json.h>
 #include <util/filter.h>
 #include <json-c/json.h>
-#include <util/parse-options.h>
 #include <ndctl/libndctl.h>
+#include <util/parse-options.h>
 #include <ccan/array_size/array_size.h>
 
 static struct {
 	bool verbose;
+	bool force;
+	bool idle;
+	bool dryrun;
 	unsigned int poll_interval;
-} param;
+} param = {
+	.idle = true,
+};
 
 
 #define BASE_OPTIONS() \
@@ -26,6 +31,13 @@ static struct {
 #define WAIT_OPTIONS() \
 	OPT_UINTEGER('p', "poll", &param.poll_interval, "poll interval (seconds)")
 
+#define ACTIVATE_OPTIONS() \
+	OPT_BOOLEAN('I', "idle", &param.idle, \
+			"allow platform-injected idle over activate (default)"), \
+	OPT_BOOLEAN('f', "force", &param.force, "try to force live activation"), \
+	OPT_BOOLEAN('n', "dry-run", &param.dryrun, \
+			"perform all setup/validation steps, skip the activate")
+
 static const struct option start_options[] = {
 	BASE_OPTIONS(),
 	OPT_END(),
@@ -37,6 +49,82 @@ static const struct option wait_options[] = {
 	OPT_END(),
 };
 
+static const struct option activate_options[] = {
+	BASE_OPTIONS(),
+	ACTIVATE_OPTIONS(),
+	OPT_END(),
+};
+
+static int activate_firmware(struct ndctl_bus *bus)
+{
+	const char *provider = ndctl_bus_get_provider(bus);
+	const char *devname = ndctl_bus_get_devname(bus);
+	enum ndctl_fwa_method method;
+	bool do_clear_noidle = false;
+	enum ndctl_fwa_state state;
+	struct ndctl_dimm *dimm;
+	bool has_fwupd = false;
+	int rc;
+
+	ndctl_dimm_foreach(bus, dimm) {
+		rc = ndctl_dimm_fw_update_supported(dimm);
+		if (rc == 0) {
+			has_fwupd = true;
+			break;
+		}
+	}
+
+	if (!has_fwupd) {
+		fprintf(stderr, "%s: %s: has no devices that support firmware update.\n",
+				provider, devname);
+		return -EOPNOTSUPP;
+	}
+
+	method = ndctl_bus_get_fw_activate_method(bus);
+	if (method == NDCTL_FWA_METHOD_RESET) {
+		fprintf(stderr, "%s: %s: requires a platform reset to activate firmware\n",
+				provider, devname);
+		return -EOPNOTSUPP;
+	}
+
+	if (!param.idle) {
+		rc = ndctl_bus_set_fw_activate_noidle(bus);
+		if (rc) {
+			fprintf(stderr, "%s: %s: failed to disable platform idling.\n",
+					provider, devname);
+			/* not fatal, continue... */
+		}
+		do_clear_noidle = true;
+	}
+
+	if (method == NDCTL_FWA_METHOD_SUSPEND && param.force)
+		method = NDCTL_FWA_METHOD_LIVE;
+
+	rc = 0;
+	if (!param.dryrun) {
+		state = ndctl_bus_get_fw_activate_state(bus);
+		if (state != NDCTL_FWA_ARMED && state != NDCTL_FWA_ARM_OVERFLOW) {
+			fprintf(stderr, "%s: %s: no devices armed\n",
+					provider, devname);
+			rc = -ENXIO;
+			goto out;
+		}
+
+		rc = ndctl_bus_activate_firmware(bus, method);
+	}
+
+	if (rc) {
+		fprintf(stderr, "%s: %s: firmware activation failed (%s)\n",
+				provider, devname, strerror(-rc));
+		goto out;
+	}
+
+out:
+	if (do_clear_noidle)
+		ndctl_bus_clear_fw_activate_noidle(bus);
+	return rc;
+}
+
 static int scrub_action(struct ndctl_bus *bus, enum device_action action)
 {
 	switch (action) {
@@ -50,6 +138,36 @@ static int scrub_action(struct ndctl_bus *bus, enum device_action action)
 	}
 }
 
+static void collect_result(struct json_object *jbuses, struct ndctl_bus *bus,
+		enum device_action action)
+{
+	unsigned long flags = UTIL_JSON_FIRMWARE | UTIL_JSON_HUMAN;
+	struct json_object *jbus, *jdimms;
+	struct ndctl_dimm *dimm;
+
+	jbus = util_bus_to_json(bus, flags);
+	if (jbus)
+		json_object_array_add(jbuses, jbus);
+	if (action != ACTION_ACTIVATE)
+		return;
+
+	jdimms = json_object_new_array();
+	if (!jdimms)
+		return;
+
+	ndctl_dimm_foreach(bus, dimm) {
+		struct json_object *jdimm;
+
+		jdimm = util_dimm_to_json(dimm, flags);
+		if (jdimm)
+			json_object_array_add(jdimms, jdimm);
+	}
+	if (json_object_array_length(jdimms) > 0)
+		json_object_object_add(jbus, "dimms", jdimms);
+	else
+		json_object_put(jdimms);
+}
+
 static int bus_action(int argc, const char **argv, const char *usage,
 		const struct option *options, enum device_action action,
 		struct ndctl_ctx *ctx)
@@ -58,8 +176,8 @@ static int bus_action(int argc, const char **argv, const char *usage,
 		usage,
 		NULL
 	};
-	struct json_object *jbuses, *jbus;
 	int i, rc, success = 0, fail = 0;
+	struct json_object *jbuses;
 	struct ndctl_bus *bus;
 	const char *all = "all";
 
@@ -89,21 +207,31 @@ static int bus_action(int argc, const char **argv, const char *usage,
 			if (!util_bus_filter(bus, argv[i]))
 				continue;
 			found++;
-			rc = scrub_action(bus, action);
+			switch (action) {
+			case ACTION_WAIT:
+			case ACTION_START:
+				rc = scrub_action(bus, action);
+				break;
+			case ACTION_ACTIVATE:
+				rc = activate_firmware(bus);
+				break;
+			default:
+				rc = -EINVAL;
+			}
+
 			if (rc == 0) {
 				success++;
-				jbus = util_bus_to_json(bus, 0);
-				if (jbus)
-					json_object_array_add(jbuses, jbus);
+				collect_result(jbuses, bus, action);
 			} else if (!fail)
 				fail = rc;
+
 		}
 		if (!found && param.verbose)
 			fprintf(stderr, "no bus matches id: %s\n", argv[i]);
 	}
 
 	if (success)
-		util_display_json_array(stdout, jbuses, 0);
+		util_display_json_array(stdout, jbuses, UTIL_JSON_FIRMWARE);
 	else
 		json_object_put(jbuses);
 
@@ -141,3 +269,17 @@ int cmd_wait_scrub(int argc, const char **argv, struct ndctl_ctx *ctx)
 		return 0;
 	}
 }
+
+int cmd_activate_firmware(int argc, const char **argv, struct ndctl_ctx *ctx)
+{
+	char *usage = "ndctl activate-firmware[<bus-id> <bus-id2> ... <bus-idN>] [<options>]";
+	int rc = bus_action(argc, argv, usage, activate_options,
+			ACTION_ACTIVATE, ctx);
+
+	if (rc <= 0) {
+		fprintf(stderr, "error activating firmware: %s\n",
+				strerror(-rc));
+		return rc;
+	}
+	return 0;
+}
diff --git a/ndctl/lib/libndctl.c b/ndctl/lib/libndctl.c
index a4bca6d05022..0c9ca0763082 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -1504,6 +1504,92 @@ NDCTL_EXPORT enum ndctl_fwa_method ndctl_bus_get_fw_activate_method(struct ndctl
 	return bus->fwa_method;
 }
 
+NDCTL_EXPORT int ndctl_bus_activate_firmware(struct ndctl_bus *bus, enum ndctl_fwa_method method)
+{
+	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
+	char *path = bus->bus_buf;
+	char buf[SYSFS_ATTR_SIZE];
+	int len = bus->buf_len;
+
+	if (snprintf(path, len, "%s/firmware/activate", bus->bus_path) >= len) {
+		err(ctx, "%s: buffer too small!\n", ndctl_bus_get_devname(bus));
+		return -ENOMEM;
+	}
+
+	switch (method) {
+	case NDCTL_FWA_METHOD_LIVE:
+	case NDCTL_FWA_METHOD_SUSPEND:
+		break;
+	default:
+		err(ctx, "%s: method: %d invalid\n", ndctl_bus_get_devname(bus), method);
+		return -EINVAL;
+	}
+
+	sprintf(buf, "%s\n", method == NDCTL_FWA_METHOD_LIVE ? "live" : "quiesce");
+
+	return sysfs_write_attr(ctx, path, buf);
+}
+
+static int write_fw_activate_noidle(struct ndctl_bus *bus, int arg)
+{
+	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
+	char *path = bus->bus_buf;
+	char buf[SYSFS_ATTR_SIZE];
+	int len = bus->buf_len;
+
+	if (!ndctl_bus_has_nfit(bus))
+		return -EOPNOTSUPP;
+
+	if (snprintf(path, len, "%s/nfit/firmware_activate_noidle", bus->bus_path) >= len) {
+		err(ctx, "%s: buffer too small!\n", ndctl_bus_get_devname(bus));
+		return -ENOMEM;
+	}
+
+	sprintf(buf, "%d\n", arg);
+
+	return sysfs_write_attr(ctx, path, buf);
+}
+
+NDCTL_EXPORT int ndctl_bus_set_fw_activate_noidle(struct ndctl_bus *bus)
+{
+	return write_fw_activate_noidle(bus, 1);
+}
+
+NDCTL_EXPORT int ndctl_bus_clear_fw_activate_noidle(struct ndctl_bus *bus)
+{
+	return write_fw_activate_noidle(bus, 0);
+}
+
+static int write_fw_activate_nosuspend(struct ndctl_bus *bus, int arg)
+{
+	struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
+	char *path = bus->bus_buf;
+	char buf[SYSFS_ATTR_SIZE];
+	int len = bus->buf_len;
+
+	if (!ndctl_bus_has_nfit(bus))
+		return -EOPNOTSUPP;
+
+	if (snprintf(path, len, "%s/nfit/firmware_activate_nosuspend", bus->bus_path) >= len) {
+		err(ctx, "%s: buffer too small!\n", ndctl_bus_get_devname(bus));
+		return -ENOMEM;
+	}
+
+	sprintf(buf, "%d\n", arg);
+
+	return sysfs_write_attr(ctx, path, buf);
+}
+
+NDCTL_EXPORT int ndctl_bus_set_fw_activate_nosuspend(struct ndctl_bus *bus)
+{
+	return write_fw_activate_nosuspend(bus, 1);
+}
+
+NDCTL_EXPORT int ndctl_bus_clear_fw_activate_nosuspend(struct ndctl_bus *bus)
+{
+	return write_fw_activate_nosuspend(bus, 0);
+}
+
 static enum ndctl_fwa_result fwa_result_to_result(const char *result)
 {
 	if (strcmp(result, "none") == 0)
diff --git a/ndctl/lib/libndctl.sym b/ndctl/lib/libndctl.sym
index 269ac8693304..97353fe071e7 100644
--- a/ndctl/lib/libndctl.sym
+++ b/ndctl/lib/libndctl.sym
@@ -439,4 +439,9 @@ LIBNDCTL_24 {
 	ndctl_bus_get_fw_activate_method;
 	ndctl_dimm_fw_activate_disarm;
 	ndctl_dimm_fw_activate_arm;
+	ndctl_bus_set_fw_activate_noidle;
+	ndctl_bus_clear_fw_activate_noidle;
+	ndctl_bus_set_fw_activate_nosuspend;
+	ndctl_bus_clear_fw_activate_nosuspend;
+	ndctl_bus_activate_firmware;
 } LIBNDCTL_23;
diff --git a/ndctl/libndctl.h b/ndctl/libndctl.h
index 04ca127767ac..9491b2602254 100644
--- a/ndctl/libndctl.h
+++ b/ndctl/libndctl.h
@@ -155,6 +155,11 @@ int ndctl_bus_start_scrub(struct ndctl_bus *bus);
 int ndctl_bus_has_error_injection(struct ndctl_bus *bus);
 enum ndctl_fwa_state ndctl_bus_get_fw_activate_state(struct ndctl_bus *bus);
 enum ndctl_fwa_method ndctl_bus_get_fw_activate_method(struct ndctl_bus *bus);
+int ndctl_bus_set_fw_activate_noidle(struct ndctl_bus *bus);
+int ndctl_bus_clear_fw_activate_noidle(struct ndctl_bus *bus);
+int ndctl_bus_set_fw_activate_nosuspend(struct ndctl_bus *bus);
+int ndctl_bus_clear_fw_activate_nosuspend(struct ndctl_bus *bus);
+int ndctl_bus_activate_firmware(struct ndctl_bus *bus, enum ndctl_fwa_method method);
 
 struct ndctl_dimm;
 struct ndctl_dimm *ndctl_dimm_get_first(struct ndctl_bus *bus);
diff --git a/ndctl/ndctl.c b/ndctl/ndctl.c
index 58cc9c7bb07e..eb5d8392d8e4 100644
--- a/ndctl/ndctl.c
+++ b/ndctl/ndctl.c
@@ -90,6 +90,7 @@ static struct cmd_struct commands[] = {
 	{ "update-firmware", { cmd_update_firmware } },
 	{ "inject-smart", { cmd_inject_smart } },
 	{ "wait-scrub", { cmd_wait_scrub } },
+	{ "activate-firmware", { cmd_activate_firmware } },
 	{ "start-scrub", { cmd_start_scrub } },
 	{ "setup-passphrase", { cmd_setup_passphrase } },
 	{ "update-passphrase", { cmd_update_passphrase } },
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

* [ndctl PATCH v2 4/4] ndctl/test: Test firmware-activation interface
  2020-07-20 23:37 [ndctl PATCH v2 0/4] Firmware Activation and Test Updates Dan Williams
                   ` (2 preceding siblings ...)
  2020-07-20 23:38 ` [ndctl PATCH v2 3/4] ndctl/bus: Add 'activate-firmware' command Dan Williams
@ 2020-07-20 23:38 ` Dan Williams
  3 siblings, 0 replies; 5+ messages in thread
From: Dan Williams @ 2020-07-20 23:38 UTC (permalink / raw)
  To: linux-nvdimm; +Cc: vishal.l.verma

Use the nfit_test firmware-update+activation emulation to validate the
operation of the kernel sysfs attributes, libndctl, ndctl update-firmware,
and ndctl activate-firmware.

Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 test/firmware-update.sh |   47 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 42 insertions(+), 5 deletions(-)

diff --git a/test/firmware-update.sh b/test/firmware-update.sh
index ed7d7e53772c..284b8268dbdd 100755
--- a/test/firmware-update.sh
+++ b/test/firmware-update.sh
@@ -22,23 +22,60 @@ reset()
 
 detect()
 {
-	dev=$($NDCTL list -b $NFIT_TEST_BUS0 -D | jq .[0].dev | tr -d '"')
-	[ -n "$dev" ] || err "$LINENO"
+	$NDCTL wait-scrub $NFIT_TEST_BUS0
+	fwa=$($NDCTL list -b $NFIT_TEST_BUS0 -F | jq -r '.[0].firmware.activate_method')
+	[ $fwa = "suspend" ] || err "$LINENO"
+	count=$($NDCTL list -b $NFIT_TEST_BUS0 -D | jq length)
+	[ $((count)) -eq 4 ] || err "$LINENO"
 }
 
 do_tests()
 {
+	# create a dummy image file, try to update all 4 dimms on
+	# nfit_test.0, validate that all get staged, validate that all
+	# but one get armed relative to an overflow error.
 	truncate -s 196608 $image
-	$NDCTL update-firmware -f $image $dev
+	json=$($NDCTL update-firmware -b $NFIT_TEST_BUS0 -f $image all)
+	count=$(jq 'map(select(.firmware.activate_state == "armed")) | length' <<< $json)
+	[ $((count)) -eq 3 ] || err "$LINENO"
+	count=$(jq 'map(select(.firmware.activate_state == "idle")) | length' <<< $json)
+	[ $((count)) -eq 1 ] || err "$LINENO"
+
+	# validate that the overflow dimm can be force armed
+	dev=$(jq -r '.[] | select(.firmware.activate_state == "idle").dev' <<< $json)
+	json=$($NDCTL update-firmware -b $NFIT_TEST_BUS0 $dev -A --force)
+	state=$(jq -r '.[0].firmware.activate_state' <<< $json)
+	[ $state = "armed" ] || err "$LINENO"
+
+	# validate that the bus indicates overflow
+	fwa=$($NDCTL list -b $NFIT_TEST_BUS0 -F | jq -r '.[0].firmware.activate_state')
+	[ $fwa = "overflow" ] || err "$LINENO"
+
+	# validate that all devices can be disarmed, and the bus goes idle
+	json=$($NDCTL update-firmware -b $NFIT_TEST_BUS0 -D all)
+	count=$(jq 'map(select(.firmware.activate_state == "idle")) | length' <<< $json)
+	[ $((count)) -eq 4 ] || err "$LINENO"
+	fwa=$($NDCTL list -b $NFIT_TEST_BUS0 -F | jq -r '.[0].firmware.activate_state')
+	[ $fwa = "idle" ] || err "$LINENO"
+
+	# re-arm all DIMMs
+	json=$($NDCTL update-firmware -b $NFIT_TEST_BUS0 -A --force all)
+	count=$(jq 'map(select(.firmware.activate_state == "armed")) | length' <<< $json)
+	[ $((count)) -eq 4 ] || err "$LINENO"
+
+	# trigger activation via suspend
+	json=$($NDCTL activate-firmware -v $NFIT_TEST_BUS0)
+	idle_count=$(jq '.[].dimms | map(select(.firmware.activate_state == "idle")) | length' <<< $json)
+	busy_count=$(jq '.[].dimms | map(select(.firmware.activate_state == "busy")) | length' <<< $json)
+	[ $((idle_count)) -eq 4 -o $((busy_count)) -eq 4 ] || err "$LINENO"
 }
 
 check_min_kver "4.16" || do_skip "may lack firmware update test handling"
 
 modprobe nfit_test
-rc=1
 reset
-rc=2
 detect
+rc=1
 do_tests
 rm -f $image
 _cleanup
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

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

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-20 23:37 [ndctl PATCH v2 0/4] Firmware Activation and Test Updates Dan Williams
2020-07-20 23:37 ` [ndctl PATCH v2 1/4] ndctl/list: Add firmware activation enumeration Dan Williams
2020-07-20 23:38 ` [ndctl PATCH v2 2/4] ndctl/dimm: Auto-arm firmware activation Dan Williams
2020-07-20 23:38 ` [ndctl PATCH v2 3/4] ndctl/bus: Add 'activate-firmware' command Dan Williams
2020-07-20 23:38 ` [ndctl PATCH v2 4/4] ndctl/test: Test firmware-activation interface Dan Williams

Linux-NVDIMM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nvdimm/0 linux-nvdimm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nvdimm linux-nvdimm/ https://lore.kernel.org/linux-nvdimm \
		linux-nvdimm@lists.01.org
	public-inbox-index linux-nvdimm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.01.lists.linux-nvdimm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git