* [ndctl PATCH 01/16] ndctl/build: Fix zero-length array warnings
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
@ 2020-07-07 2:40 ` Dan Williams
2020-07-07 2:40 ` [ndctl PATCH 02/16] ndctl/dimm: Fix chatty status messages Dan Williams
` (14 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:40 UTC (permalink / raw)
To: linux-nvdimm; +Cc: vishal.l.verma
GCC10 emits warnings like:
msft.c: In function ‘msft_cmd_smart_get_media_temperature’:
msft.c:146:28: warning: array subscript 0 is outside the bounds of an interior zero-length array ‘struct ndn_msft_smart_data[0]’ [-Wzero-length-bounds]
hpe1.c: In function ‘hpe1_cmd_smart_get_flags’:
hpe1.c:111:33: warning: array subscript 0 is outside the bounds of an interior zero-length array ‘struct ndn_hpe1_smart_data[0]’ [-Wzero-length-bounds]
ars.c: In function ‘ndctl_cmd_ars_get_record_addr’:
ars.c:274:38: warning: array subscript ‘(<unknown>) + 4294967295’ is outside the bounds of an interior zero-length array ‘struct nd_ars_record[0]’ [-Wzero-length-bounds]
In the case of the 'msft' and 'hpe1' implementation the zero-length array
is not needed because they are declared with a union of a buffer of the
same size as a single element. Fix those cases by just declaring a single
element array.
The ARS case is different, it's complaining about an internal zero-length
member. Switch to the recommended [1] flexible-array syntax for that case.
[1]: https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
ndctl/lib/hpe1.h | 4 ++--
ndctl/lib/msft.h | 2 +-
ndctl/ndctl.h | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/ndctl/lib/hpe1.h b/ndctl/lib/hpe1.h
index b050831ec2c4..1afa54f127a6 100644
--- a/ndctl/lib/hpe1.h
+++ b/ndctl/lib/hpe1.h
@@ -111,7 +111,7 @@ struct ndn_hpe1_smart {
__u32 status;
union {
__u8 buf[124];
- struct ndn_hpe1_smart_data data[0];
+ struct ndn_hpe1_smart_data data[1];
};
} __attribute__((packed));
@@ -136,7 +136,7 @@ struct ndn_hpe1_smart_threshold {
__u32 status;
union {
__u8 buf[32];
- struct ndn_hpe1_smart_threshold_data data[0];
+ struct ndn_hpe1_smart_threshold_data data[1];
};
} __attribute__((packed));
diff --git a/ndctl/lib/msft.h b/ndctl/lib/msft.h
index 0a1c7c6a0907..c45981edd8d7 100644
--- a/ndctl/lib/msft.h
+++ b/ndctl/lib/msft.h
@@ -46,7 +46,7 @@ struct ndn_msft_smart {
__u32 status;
union {
__u8 buf[9];
- struct ndn_msft_smart_data data[0];
+ struct ndn_msft_smart_data data[1];
};
} __attribute__((packed));
diff --git a/ndctl/ndctl.h b/ndctl/ndctl.h
index 008f81cdeb9f..e3605b3d64b4 100644
--- a/ndctl/ndctl.h
+++ b/ndctl/ndctl.h
@@ -91,7 +91,7 @@ struct nd_cmd_ars_status {
__u32 reserved;
__u64 err_address;
__u64 length;
- } __attribute__((packed)) records[0];
+ } __attribute__((packed)) records[];
} __attribute__((packed));
struct nd_cmd_clear_error {
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 17+ messages in thread* [ndctl PATCH 02/16] ndctl/dimm: Fix chatty status messages
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
2020-07-07 2:40 ` [ndctl PATCH 01/16] ndctl/build: Fix zero-length array warnings Dan Williams
@ 2020-07-07 2:40 ` Dan Williams
2020-07-07 2:40 ` [ndctl PATCH 03/16] ndctl/list: Indicate firmware update capability Dan Williams
` (13 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:40 UTC (permalink / raw)
To: linux-nvdimm; +Cc: Dave Jiang, vishal.l.verma
Firmware update and overwrite violate the general design principle of ndctl
that all stdout messages are json formatted. Move the raw status messages
to stderr. Some of the messages are obviously debug and should be moved
under a verbose option, while others confuse use of error() vs
fprintf(stderr, ...). The error() helper is preferred for messages
indicating command failures vs notices.
Cc: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
ndctl/dimm.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 3db01431d618..8523ead36eb5 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -795,15 +795,16 @@ static int update_firmware(struct ndctl_dimm *dimm,
if (rc < 0)
return rc;
- printf("Uploading firmware to DIMM %s.\n",
- ndctl_dimm_get_devname(dimm));
+ if (param.verbose)
+ fprintf(stderr, "Uploading firmware to DIMM %s.\n",
+ ndctl_dimm_get_devname(dimm));
rc = send_firmware(dimm, actx);
if (rc < 0) {
- fprintf(stderr, "Firmware send failed. Aborting!\n");
+ error("Firmware send failed. Aborting!\n");
rc = submit_abort_firmware(dimm, actx);
if (rc < 0)
- fprintf(stderr, "Aborting update sequence failed.\n");
+ error("Aborting update sequence failed.\n");
return rc;
}
@@ -945,7 +946,8 @@ static int action_sanitize_dimm(struct ndctl_dimm *dimm,
*/
if (!param.crypto_erase && !param.overwrite) {
param.crypto_erase = true;
- printf("No santize method passed in, default to crypto-erase\n");
+ if (param.verbose)
+ fprintf(stderr, "No santize method passed in, default to crypto-erase\n");
}
if (param.crypto_erase) {
@@ -982,8 +984,8 @@ static int action_wait_overwrite(struct ndctl_dimm *dimm,
}
rc = ndctl_dimm_wait_overwrite(dimm);
- if (rc == 1)
- printf("%s: overwrite completed.\n",
+ if (rc == 1 && param.verbose)
+ fprintf(stderr, "%s: overwrite completed.\n",
ndctl_dimm_get_devname(dimm));
return rc;
}
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 17+ messages in thread* [ndctl PATCH 03/16] ndctl/list: Indicate firmware update capability
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
2020-07-07 2:40 ` [ndctl PATCH 01/16] ndctl/build: Fix zero-length array warnings Dan Williams
2020-07-07 2:40 ` [ndctl PATCH 02/16] ndctl/dimm: Fix chatty status messages Dan Williams
@ 2020-07-07 2:40 ` Dan Williams
2020-07-07 2:40 ` [ndctl PATCH 04/16] ndctl/dimm: Detect firmware-update vs ARS conflict Dan Williams
` (12 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:40 UTC (permalink / raw)
To: linux-nvdimm; +Cc: vishal.l.verma
Given all the components that need to be lined up to support firmware
update, add a "can_update" flag to indicate that all the proper plumbing is
in place.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Documentation/ndctl/ndctl-update-firmware.txt | 21 ++++++++++++++++++---
ndctl/util/json-firmware.c | 5 +++++
2 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/Documentation/ndctl/ndctl-update-firmware.txt b/Documentation/ndctl/ndctl-update-firmware.txt
index 1aa7fee502ca..f93da6bf15e7 100644
--- a/Documentation/ndctl/ndctl-update-firmware.txt
+++ b/Documentation/ndctl/ndctl-update-firmware.txt
@@ -5,7 +5,7 @@ ndctl-update-firmware(1)
NAME
----
-ndctl-update-firmware - provides for updating the firmware on an NVDIMM
+ndctl-update-firmware - update the firmware the given device
SYNOPSIS
--------
@@ -15,9 +15,24 @@ SYNOPSIS
DESCRIPTION
-----------
Provide a generic interface for updating NVDIMM firmware. The use of this
-depends on support from the underlying libndctl, kernel, as well as the
-platform itself.
+depends on support for the NVDIMM "family" in libndctl, the kernel needs
+to enable that command set, and the device itself needs to implement the
+command. Use "ndctl list -DF" to interrogate if firmware
+update is enabled. For example:
+[verse]
+ndctl list -DFu -d nmem1
+{
+ "dev":"nmem1",
+ "id":"cdab-0a-07e0-ffffffff",
+ "handle":"0",
+ "phys_id":"0",
+ "security":"disabled",
+ "firmware":{
+ "current_version":"0",
+ "can_update":true
+ }
+}
OPTIONS
-------
diff --git a/ndctl/util/json-firmware.c b/ndctl/util/json-firmware.c
index f7150d82d174..9a9db064d851 100644
--- a/ndctl/util/json-firmware.c
+++ b/ndctl/util/json-firmware.c
@@ -46,6 +46,11 @@ struct json_object *util_dimm_firmware_to_json(struct ndctl_dimm *dimm,
if (jobj)
json_object_object_add(jfirmware, "current_version", jobj);
+ rc = ndctl_dimm_fw_update_supported(dimm);
+ jobj = json_object_new_boolean(rc == 0);
+ 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);
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 17+ messages in thread* [ndctl PATCH 04/16] ndctl/dimm: Detect firmware-update vs ARS conflict
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
` (2 preceding siblings ...)
2020-07-07 2:40 ` [ndctl PATCH 03/16] ndctl/list: Indicate firmware update capability Dan Williams
@ 2020-07-07 2:40 ` Dan Williams
2020-07-07 2:40 ` [ndctl PATCH 05/16] ndctl/dimm: Improve firmware-update failure message Dan Williams
` (11 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:40 UTC (permalink / raw)
To: linux-nvdimm; +Cc: vishal.l.verma
Prevent firmware-update while ARS is active unless --force is supplied.
Translate the FW_ERETRY return code as a conflict with a background DIMM
process.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Documentation/ndctl/ndctl-update-firmware.txt | 5 ++++
ndctl/dimm.c | 32 ++++++++++++++++++-------
2 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/Documentation/ndctl/ndctl-update-firmware.txt b/Documentation/ndctl/ndctl-update-firmware.txt
index f93da6bf15e7..bcf61abaa989 100644
--- a/Documentation/ndctl/ndctl-update-firmware.txt
+++ b/Documentation/ndctl/ndctl-update-firmware.txt
@@ -47,6 +47,11 @@ include::xable-bus-options.txt[]
--firmware::
firmware file used to perform the update
+-i::
+--force::
+ Ignore in-progress Address Range Scrub and try to submit the
+ firmware update.
+
-v::
--verbose::
Emit debug messages for the namespace check process.
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 8523ead36eb5..f67f78e6c420 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -18,7 +18,6 @@
#include <unistd.h>
#include <limits.h>
#include <syslog.h>
-#include <util/log.h>
#include <util/size.h>
#include <uuid/uuid.h>
#include <util/json.h>
@@ -33,6 +32,11 @@
#include <ndctl/firmware-update.h>
#include <util/keys.h>
+static const char *cmd_name = "dimm";
+static int err_count;
+#define err(fmt, ...) \
+ ({ err_count++; error("%s: " fmt, cmd_name, ##__VA_ARGS__); })
+
struct action_context {
struct json_object *jdimms;
enum ndctl_namespace_version labelversion;
@@ -832,24 +836,30 @@ static int update_firmware(struct ndctl_dimm *dimm,
static int action_update(struct ndctl_dimm *dimm, struct action_context *actx)
{
+ struct ndctl_bus *bus = ndctl_dimm_get_bus(dimm);
+ const char *devname = ndctl_dimm_get_devname(dimm);
int rc;
rc = ndctl_dimm_fw_update_supported(dimm);
switch (rc) {
case -ENOTTY:
- error("%s: firmware update not supported by ndctl.",
- ndctl_dimm_get_devname(dimm));
+ err("%s: firmware update not supported by ndctl.", devname);
return rc;
case -EOPNOTSUPP:
- error("%s: firmware update not supported by the kernel",
- ndctl_dimm_get_devname(dimm));
+ err("%s: firmware update not supported by the kernel", devname);
return rc;
case -EIO:
- error("%s: firmware update not supported by either platform firmware or the kernel.",
- ndctl_dimm_get_devname(dimm));
+ err("%s: firmware update not supported by either platform firmware or the kernel.",
+ devname);
return rc;
}
+ if (ndctl_bus_get_scrub_state(bus) > 0 && !param.force) {
+ err("%s: scrub active, retry after 'ndctl wait-scrub'",
+ devname);
+ return -EBUSY;
+ }
+
rc = update_verify_input(actx);
if (rc < 0)
return rc;
@@ -1073,7 +1083,8 @@ OPT_STRING('i', "input", ¶m.infile, "input-file", \
#define UPDATE_OPTIONS() \
OPT_STRING('f', "firmware", ¶m.infile, "firmware-file", \
- "firmware filename for update")
+ "firmware filename for update"), \
+OPT_BOOLEAN('i', "force", ¶m.force, "ignore ARS status, try to force update")
#define INIT_OPTIONS() \
OPT_BOOLEAN('f', "force", ¶m.force, \
@@ -1386,7 +1397,10 @@ int cmd_enable_dimm(int argc, const char **argv, struct ndctl_ctx *ctx)
int cmd_update_firmware(int argc, const char **argv, struct ndctl_ctx *ctx)
{
- int count = dimm_action(argc, argv, ctx, action_update, update_options,
+ int count;
+
+ cmd_name = "update firmware";
+ count = dimm_action(argc, argv, ctx, action_update, update_options,
"ndctl update-firmware <nmem0> [<nmem1>..<nmemN>] [<options>]");
fprintf(stderr, "updated %d nmem%s.\n", count >= 0 ? count : 0,
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 17+ messages in thread* [ndctl PATCH 05/16] ndctl/dimm: Improve firmware-update failure message
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
` (3 preceding siblings ...)
2020-07-07 2:40 ` [ndctl PATCH 04/16] ndctl/dimm: Detect firmware-update vs ARS conflict Dan Williams
@ 2020-07-07 2:40 ` Dan Williams
2020-07-07 2:40 ` [ndctl PATCH 06/16] ndctl/dimm: Prepare to emit dimm json object after firmware update Dan Williams
` (10 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:40 UTC (permalink / raw)
To: linux-nvdimm; +Cc: Dave Jiang, vishal.l.verma
If ARS is active while firmware update is attempted the DIMM may fail the
update. In ndctl reports:
FINISH FIRMWARE UPDATE on DIMM nmem0 failed: 0x5
...which is not very helpful. Improve this and other error messages to
indicate the likely error where possible and make sure error messages are
consistently emitting the affected dimm.
Cc: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
ndctl/dimm.c | 108 ++++++++++++++++++++++++++++++----------------------------
1 file changed, 55 insertions(+), 53 deletions(-)
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index f67f78e6c420..5ecee033cd63 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -460,6 +460,7 @@ static int verify_fw_size(struct update_context *uctx)
static int submit_get_firmware_info(struct ndctl_dimm *dimm,
struct action_context *actx)
{
+ const char *devname = ndctl_dimm_get_devname(dimm);
struct update_context *uctx = &actx->update;
struct fw_info *fw = &uctx->dimm_fw;
struct ndctl_cmd *cmd;
@@ -477,8 +478,7 @@ static int submit_get_firmware_info(struct ndctl_dimm *dimm,
rc = -ENXIO;
status = ndctl_cmd_fw_xlat_firmware_status(cmd);
if (status != FW_SUCCESS) {
- fprintf(stderr, "GET FIRMWARE INFO on DIMM %s failed: %#x\n",
- ndctl_dimm_get_devname(dimm), status);
+ err("%s: failed to retrieve firmware info", devname);
goto out;
}
@@ -512,6 +512,7 @@ out:
static int submit_start_firmware_upload(struct ndctl_dimm *dimm,
struct action_context *actx)
{
+ const char *devname = ndctl_dimm_get_devname(dimm);
struct update_context *uctx = &actx->update;
struct fw_info *fw = &uctx->dimm_fw;
struct ndctl_cmd *cmd;
@@ -527,21 +528,18 @@ static int submit_start_firmware_upload(struct ndctl_dimm *dimm,
return rc;
status = ndctl_cmd_fw_xlat_firmware_status(cmd);
+ if (status == FW_EBUSY) {
+ err("%s: busy with another firmware update", devname);
+ return -EBUSY;
+ }
if (status != FW_SUCCESS) {
- fprintf(stderr,
- "START FIRMWARE UPDATE on DIMM %s failed: %#x\n",
- ndctl_dimm_get_devname(dimm), status);
- if (status == FW_EBUSY)
- fprintf(stderr, "Another firmware upload in progress"
- " or firmware already updated.\n");
+ err("%s: failed to create start context", devname);
return -ENXIO;
}
fw->context = ndctl_cmd_fw_start_get_context(cmd);
if (fw->context == UINT_MAX) {
- fprintf(stderr,
- "Retrieved firmware context invalid on DIMM %s\n",
- ndctl_dimm_get_devname(dimm));
+ err("%s: failed to retrieve start context", devname);
return -ENXIO;
}
@@ -567,16 +565,16 @@ static int get_fw_data_from_file(FILE *file, void *buf, uint32_t len)
return len;
}
-static int send_firmware(struct ndctl_dimm *dimm,
- struct action_context *actx)
+static int send_firmware(struct ndctl_dimm *dimm, struct action_context *actx)
{
+ const char *devname = ndctl_dimm_get_devname(dimm);
struct update_context *uctx = &actx->update;
struct fw_info *fw = &uctx->dimm_fw;
+ uint32_t copied = 0, len, remain;
struct ndctl_cmd *cmd = NULL;
- ssize_t read;
- int rc = -ENXIO;
enum ND_FW_STATUS status;
- uint32_t copied = 0, len, remain;
+ int rc = -ENXIO;
+ ssize_t read;
void *buf;
buf = malloc(fw->update_size);
@@ -596,18 +594,22 @@ static int send_firmware(struct ndctl_dimm *dimm,
cmd = ndctl_dimm_cmd_new_fw_send(uctx->start, copied, read,
buf);
if (!cmd) {
- rc = -ENXIO;
+ rc = -ENOMEM;
goto cleanup;
}
rc = ndctl_cmd_submit(cmd);
- if (rc < 0)
+ if (rc < 0) {
+ err("%s: failed to issue firmware transmit: %d",
+ devname, rc);
goto cleanup;
+ }
status = ndctl_cmd_fw_xlat_firmware_status(cmd);
if (status != FW_SUCCESS) {
- error("SEND FIRMWARE failed: %#x\n", status);
- rc = -ENXIO;
+ err("%s: failed to transmit firmware: %d",
+ devname, status);
+ rc = -EIO;
goto cleanup;
}
@@ -627,10 +629,11 @@ cleanup:
static int submit_finish_firmware(struct ndctl_dimm *dimm,
struct action_context *actx)
{
+ const char *devname = ndctl_dimm_get_devname(dimm);
struct update_context *uctx = &actx->update;
- struct ndctl_cmd *cmd;
- int rc;
enum ND_FW_STATUS status;
+ struct ndctl_cmd *cmd;
+ int rc = -ENXIO;
cmd = ndctl_dimm_cmd_new_fw_finish(uctx->start);
if (!cmd)
@@ -641,12 +644,19 @@ static int submit_finish_firmware(struct ndctl_dimm *dimm,
goto out;
status = ndctl_cmd_fw_xlat_firmware_status(cmd);
- if (status != FW_SUCCESS) {
- fprintf(stderr,
- "FINISH FIRMWARE UPDATE on DIMM %s failed: %#x\n",
- ndctl_dimm_get_devname(dimm), status);
- rc = -ENXIO;
- goto out;
+ switch (status) {
+ case FW_SUCCESS:
+ rc = 0;
+ break;
+ case FW_ERETRY:
+ err("%s: device busy with other operation (ARS?)", devname);
+ break;
+ case FW_EBADFW:
+ err("%s: firmware image rejected", devname);
+ break;
+ default:
+ err("%s: update failed: error code: %d", devname, status);
+ break;
}
out:
@@ -687,13 +697,14 @@ out:
static int query_fw_finish_status(struct ndctl_dimm *dimm,
struct action_context *actx)
{
+ const char *devname = ndctl_dimm_get_devname(dimm);
struct update_context *uctx = &actx->update;
struct fw_info *fw = &uctx->dimm_fw;
- struct ndctl_cmd *cmd;
- int rc;
- enum ND_FW_STATUS status;
struct timespec now, before, after;
+ enum ND_FW_STATUS status;
+ struct ndctl_cmd *cmd;
uint64_t ver;
+ int rc;
cmd = ndctl_dimm_cmd_new_fw_finish_query(uctx->start);
if (!cmd)
@@ -747,8 +758,8 @@ again:
case FW_SUCCESS:
ver = ndctl_cmd_fw_fquery_get_fw_rev(cmd);
if (ver == 0) {
- fprintf(stderr, "No firmware updated.\n");
- rc = -ENXIO;
+ err("%s: new firmware not found after update", devname);
+ rc = -EIO;
goto unref;
}
@@ -759,25 +770,16 @@ again:
rc = 0;
break;
case FW_EBADFW:
- fprintf(stderr,
- "Firmware failed to verify by DIMM %s.\n",
- ndctl_dimm_get_devname(dimm));
- /* FALLTHROUGH */
- case FW_EINVAL_CTX:
- case FW_ESEQUENCE:
- rc = -ENXIO;
+ err("%s: firmware verification failure", devname);
+ rc = -EINVAL;
break;
case FW_ENORES:
- fprintf(stderr,
- "Firmware update sequence timed out: %s\n",
- ndctl_dimm_get_devname(dimm));
+ err("%s: timeout awaiting update", devname);
rc = -ETIMEDOUT;
break;
default:
- fprintf(stderr,
- "Unknown update status: %#x on DIMM %s\n",
- status, ndctl_dimm_get_devname(dimm));
- rc = -EINVAL;
+ err("%s: unhandled error %d", devname, status);
+ rc = -EIO;
break;
}
@@ -789,6 +791,7 @@ unref:
static int update_firmware(struct ndctl_dimm *dimm,
struct action_context *actx)
{
+ const char *devname = ndctl_dimm_get_devname(dimm);
int rc;
rc = submit_get_firmware_info(dimm, actx);
@@ -800,15 +803,14 @@ static int update_firmware(struct ndctl_dimm *dimm,
return rc;
if (param.verbose)
- fprintf(stderr, "Uploading firmware to DIMM %s.\n",
- ndctl_dimm_get_devname(dimm));
+ fprintf(stderr, "%s: uploading firmware\n", devname);
rc = send_firmware(dimm, actx);
if (rc < 0) {
- error("Firmware send failed. Aborting!\n");
+ err("%s: firmware send failed", devname);
rc = submit_abort_firmware(dimm, actx);
if (rc < 0)
- error("Aborting update sequence failed.\n");
+ err("%s: abort failed", devname);
return rc;
}
@@ -820,10 +822,10 @@ static int update_firmware(struct ndctl_dimm *dimm,
rc = submit_finish_firmware(dimm, actx);
if (rc < 0) {
- fprintf(stderr, "Unable to end update sequence.\n");
+ err("%s: failed to finish update sequence", devname);
rc = submit_abort_firmware(dimm, actx);
if (rc < 0)
- fprintf(stderr, "Aborting update sequence failed.\n");
+ err("%s: failed to abort update", devname);
return rc;
}
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 17+ messages in thread* [ndctl PATCH 06/16] ndctl/dimm: Prepare to emit dimm json object after firmware update
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
` (4 preceding siblings ...)
2020-07-07 2:40 ` [ndctl PATCH 05/16] ndctl/dimm: Improve firmware-update failure message Dan Williams
@ 2020-07-07 2:40 ` Dan Williams
2020-07-07 2:40 ` [ndctl PATCH 07/16] ndctl/dimm: Emit dimm firmware details after update Dan Williams
` (9 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:40 UTC (permalink / raw)
To: linux-nvdimm; +Cc: vishal.l.verma
Move util_dimm_firmware_to_json() internal to util_dimm_to_json().
Introduce a new UTIL_JSON_FIRMWARE flag to optionally dump firmware info
when listing the dimm from either 'ndctl list', or after 'ndctl
update-firmware'.
Move util_dimm_firmware_to_json() out of ndctl/util/json-firmware.c into
the core util/json.c.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
ndctl/Makefile.am | 1 -
ndctl/list.c | 10 +----
ndctl/util/json-firmware.c | 85 --------------------------------------------
util/json.c | 84 +++++++++++++++++++++++++++++++++++++++++++
util/json.h | 17 +++++----
5 files changed, 95 insertions(+), 102 deletions(-)
delete mode 100644 ndctl/util/json-firmware.c
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index 49c6c4ab4498..a63b1e0b8a01 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -25,7 +25,6 @@ ndctl_SOURCES = ndctl.c \
../util/json.c \
../util/json.h \
util/json-smart.c \
- util/json-firmware.c \
util/keys.h \
inject-error.c \
inject-smart.c \
diff --git a/ndctl/list.c b/ndctl/list.c
index 31fb1b9593a2..1f7cc8ee1deb 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -59,6 +59,8 @@ static unsigned long listopts_to_flags(void)
flags |= UTIL_JSON_VERBOSE;
if (list.capabilities)
flags |= UTIL_JSON_CAPABILITIES;
+ if (list.firmware)
+ flags |= UTIL_JSON_FIRMWARE;
return flags;
}
@@ -367,14 +369,6 @@ static void filter_dimm(struct ndctl_dimm *dimm, struct util_filter_ctx *ctx)
}
}
- if (list.firmware) {
- struct json_object *jfirmware;
-
- jfirmware = util_dimm_firmware_to_json(dimm, lfa->flags);
- if (jfirmware)
- json_object_object_add(jdimm, "firmware", jfirmware);
- }
-
/*
* Without a bus we are collecting dimms anonymously across the
* platform.
diff --git a/ndctl/util/json-firmware.c b/ndctl/util/json-firmware.c
deleted file mode 100644
index 9a9db064d851..000000000000
--- a/ndctl/util/json-firmware.c
+++ /dev/null
@@ -1,85 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
-#include <limits.h>
-#include <util/json.h>
-#include <uuid/uuid.h>
-#include <json-c/json.h>
-#include <ndctl/libndctl.h>
-#include <ccan/array_size/array_size.h>
-#include <ndctl.h>
-
-struct json_object *util_dimm_firmware_to_json(struct ndctl_dimm *dimm,
- unsigned long flags)
-{
- struct json_object *jfirmware = json_object_new_object();
- struct json_object *jobj;
- struct ndctl_cmd *cmd;
- int rc;
- uint64_t run, next;
-
- if (!jfirmware)
- return NULL;
-
- cmd = ndctl_dimm_cmd_new_fw_get_info(dimm);
- if (!cmd)
- goto err;
-
- rc = ndctl_cmd_submit(cmd);
- if ((rc < 0) || ndctl_cmd_fw_xlat_firmware_status(cmd) != FW_SUCCESS) {
- jobj = util_json_object_hex(-1, flags);
- if (jobj)
- json_object_object_add(jfirmware, "current_version",
- jobj);
- goto out;
- }
-
- run = ndctl_cmd_fw_info_get_run_version(cmd);
- if (run == ULLONG_MAX) {
- jobj = util_json_object_hex(-1, flags);
- if (jobj)
- json_object_object_add(jfirmware, "current_version",
- jobj);
- goto out;
- }
-
- jobj = util_json_object_hex(run, flags);
- if (jobj)
- json_object_object_add(jfirmware, "current_version", jobj);
-
- rc = ndctl_dimm_fw_update_supported(dimm);
- jobj = json_object_new_boolean(rc == 0);
- 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);
- if (jobj)
- json_object_object_add(jfirmware, "next_version",
- jobj);
- goto out;
- }
-
- if (next != 0) {
- jobj = util_json_object_hex(next, flags);
- if (jobj)
- json_object_object_add(jfirmware,
- "next_version", jobj);
-
- jobj = json_object_new_boolean(true);
- if (jobj)
- json_object_object_add(jfirmware,
- "need_powercycle", jobj);
- }
-
- ndctl_cmd_unref(cmd);
- return jfirmware;
-
-err:
- json_object_put(jfirmware);
- jfirmware = NULL;
-out:
- if (cmd)
- ndctl_cmd_unref(cmd);
- return jfirmware;
-}
diff --git a/util/json.c b/util/json.c
index 21ab25674624..59a3d07cc4a6 100644
--- a/util/json.c
+++ b/util/json.c
@@ -156,6 +156,82 @@ struct json_object *util_bus_to_json(struct ndctl_bus *bus)
return NULL;
}
+struct json_object *util_dimm_firmware_to_json(struct ndctl_dimm *dimm,
+ unsigned long flags)
+{
+ struct json_object *jfirmware = json_object_new_object();
+ struct json_object *jobj;
+ struct ndctl_cmd *cmd;
+ int rc;
+ uint64_t run, next;
+
+ if (!jfirmware)
+ return NULL;
+
+ cmd = ndctl_dimm_cmd_new_fw_get_info(dimm);
+ if (!cmd)
+ goto err;
+
+ rc = ndctl_cmd_submit(cmd);
+ if ((rc < 0) || ndctl_cmd_fw_xlat_firmware_status(cmd) != FW_SUCCESS) {
+ jobj = util_json_object_hex(-1, flags);
+ if (jobj)
+ json_object_object_add(jfirmware, "current_version",
+ jobj);
+ goto out;
+ }
+
+ run = ndctl_cmd_fw_info_get_run_version(cmd);
+ if (run == ULLONG_MAX) {
+ jobj = util_json_object_hex(-1, flags);
+ if (jobj)
+ json_object_object_add(jfirmware, "current_version",
+ jobj);
+ goto out;
+ }
+
+ jobj = util_json_object_hex(run, flags);
+ if (jobj)
+ json_object_object_add(jfirmware, "current_version", jobj);
+
+ rc = ndctl_dimm_fw_update_supported(dimm);
+ jobj = json_object_new_boolean(rc == 0);
+ 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);
+ if (jobj)
+ json_object_object_add(jfirmware, "next_version",
+ jobj);
+ goto out;
+ }
+
+ if (next != 0) {
+ jobj = util_json_object_hex(next, flags);
+ if (jobj)
+ json_object_object_add(jfirmware,
+ "next_version", jobj);
+
+ jobj = json_object_new_boolean(true);
+ if (jobj)
+ json_object_object_add(jfirmware,
+ "need_powercycle", jobj);
+ }
+
+ ndctl_cmd_unref(cmd);
+ return jfirmware;
+
+err:
+ json_object_put(jfirmware);
+ jfirmware = NULL;
+out:
+ if (cmd)
+ ndctl_cmd_unref(cmd);
+ return jfirmware;
+}
+
struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm,
unsigned long flags)
{
@@ -266,6 +342,14 @@ struct json_object *util_dimm_to_json(struct ndctl_dimm *dimm,
json_object_object_add(jdimm, "security_frozen", jobj);
}
+ if (flags & UTIL_JSON_FIRMWARE) {
+ struct json_object *jfirmware;
+
+ jfirmware = util_dimm_firmware_to_json(dimm, flags);
+ if (jfirmware)
+ json_object_object_add(jdimm, "firmware", jfirmware);
+ }
+
return jdimm;
err:
json_object_put(jdimm);
diff --git a/util/json.h b/util/json.h
index 6d39d3aa4693..fc91a8db034f 100644
--- a/util/json.h
+++ b/util/json.h
@@ -18,14 +18,15 @@
#include <ccan/short_types/short_types.h>
enum util_json_flags {
- UTIL_JSON_IDLE = (1 << 0),
- UTIL_JSON_MEDIA_ERRORS = (1 << 1),
- UTIL_JSON_DAX = (1 << 2),
- UTIL_JSON_DAX_DEVS = (1 << 3),
- UTIL_JSON_HUMAN = (1 << 4),
- UTIL_JSON_VERBOSE = (1 << 5),
- UTIL_JSON_CAPABILITIES = (1 << 6),
- UTIL_JSON_CONFIGURED = (1 << 7),
+ UTIL_JSON_IDLE = (1 << 0),
+ UTIL_JSON_MEDIA_ERRORS = (1 << 1),
+ UTIL_JSON_DAX = (1 << 2),
+ UTIL_JSON_DAX_DEVS = (1 << 3),
+ UTIL_JSON_HUMAN = (1 << 4),
+ UTIL_JSON_VERBOSE = (1 << 5),
+ UTIL_JSON_CAPABILITIES = (1 << 6),
+ UTIL_JSON_CONFIGURED = (1 << 7),
+ UTIL_JSON_FIRMWARE = (1 << 8),
};
struct json_object;
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 17+ messages in thread* [ndctl PATCH 07/16] ndctl/dimm: Emit dimm firmware details after update
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
` (5 preceding siblings ...)
2020-07-07 2:40 ` [ndctl PATCH 06/16] ndctl/dimm: Prepare to emit dimm json object after firmware update Dan Williams
@ 2020-07-07 2:40 ` Dan Williams
2020-07-07 2:41 ` [ndctl PATCH 08/16] ndctl/list: Add firmware activation enumeration Dan Williams
` (8 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:40 UTC (permalink / raw)
To: linux-nvdimm; +Cc: vishal.l.verma
Replace the status messages at the end of 'ndctl update-firmware' with a
json representation of the dimm-firmware state.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
ndctl/dimm.c | 23 ++++++++++++++++-------
ndctl/firmware-update.h | 1 +
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/ndctl/dimm.c b/ndctl/dimm.c
index 5ecee033cd63..e02f5dfdb889 100644
--- a/ndctl/dimm.c
+++ b/ndctl/dimm.c
@@ -701,8 +701,10 @@ 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;
@@ -763,10 +765,12 @@ again:
goto unref;
}
- fprintf(stderr, "Image updated successfully to DIMM %s.\n",
- ndctl_dimm_get_devname(dimm));
- fprintf(stderr, "Firmware version %#lx.\n", ver);
- fprintf(stderr, "Cold reboot to activate.\n");
+ 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);
rc = 0;
break;
case FW_EBADFW:
@@ -1202,7 +1206,7 @@ static int dimm_action(int argc, const char **argv, struct ndctl_ctx *ctx,
return -EINVAL;
}
- json = param.json || param.human;
+ json = param.json || param.human || action == action_update;
if (action == action_read && json && (param.len || param.offset)) {
fprintf(stderr, "--size and --offset are incompatible with --json\n");
usage_with_options(u, options);
@@ -1308,8 +1312,13 @@ static int dimm_action(int argc, const char **argv, struct ndctl_ctx *ctx,
rc = action(single, &actx);
}
- if (actx.jdimms)
- util_display_json_array(actx.f_out, actx.jdimms, 0);
+ if (actx.jdimms && json_object_array_length(actx.jdimms) > 0) {
+ unsigned long flags = 0;
+
+ if (actx.f_out == stdout && isatty(1))
+ flags |= UTIL_JSON_HUMAN;
+ util_display_json_array(actx.f_out, actx.jdimms, flags);
+ }
if (actx.f_out != stdout)
fclose(actx.f_out);
diff --git a/ndctl/firmware-update.h b/ndctl/firmware-update.h
index a7576889f739..a4386d6089d2 100644
--- a/ndctl/firmware-update.h
+++ b/ndctl/firmware-update.h
@@ -40,6 +40,7 @@ struct update_context {
size_t fw_size;
struct fw_info dimm_fw;
struct ndctl_cmd *start;
+ struct json_object *jdimms;
};
#endif
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 17+ messages in thread* [ndctl PATCH 08/16] ndctl/list: Add firmware activation enumeration
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
` (6 preceding siblings ...)
2020-07-07 2:40 ` [ndctl PATCH 07/16] ndctl/dimm: Emit dimm firmware details after update Dan Williams
@ 2020-07-07 2:41 ` Dan Williams
2020-07-07 2:41 ` [ndctl PATCH 09/16] ndctl/dimm: Auto-arm firmware activation Dan Williams
` (7 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:41 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 | 148 ++++++++++++++++++++++++++++++++++++
ndctl/lib/libndctl.sym | 7 ++
ndctl/lib/private.h | 3 +
ndctl/libndctl.h | 28 +++++++
ndctl/list.c | 3 -
util/json.c | 117 ++++++++++++++++++++++++++--
util/json.h | 3 -
9 files changed, 325 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..f03635e99a83 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -819,6 +819,19 @@ 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 void *add_bus(void *parent, int id, const char *ctl_base)
{
char buf[SYSFS_ATTR_SIZE];
@@ -880,6 +893,12 @@ 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);
+
bus->bus_path = parent_dev_path("char", bus->major, bus->minor);
if (!bus->bus_path)
goto err_dev_path;
@@ -1436,6 +1455,74 @@ 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)
+{
+ struct ndctl_ctx *ctx = ndctl_bus_get_ctx(bus);
+ char *path = bus->bus_buf;
+ int len = bus->buf_len;
+ struct stat st;
+ int rc;
+
+ if (bus->fwa_state == NDCTL_FWA_INVALID)
+ return NDCTL_FWA_METHOD_RESET;
+
+ 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;
+ }
+
+ rc = stat(path, &st);
+ if (rc < 0)
+ return NDCTL_FWA_METHOD_RESET;
+ if ((st.st_mode & 0600) == 0600)
+ return NDCTL_FWA_METHOD_LIVE;
+ if ((st.st_mode & 0400) == 0400)
+ return NDCTL_FWA_METHOD_SUSPEND;
+ return NDCTL_FWA_METHOD_RESET;
+}
+
+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 +1602,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_activate_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 +2097,55 @@ 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_activate_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..e1d61f52fed5 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,7 @@ struct ndctl_bus {
char *scrub_path;
unsigned long cmd_mask;
unsigned long nfit_dsm_mask;
+ enum ndctl_fwa_state fwa_state;
};
/**
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 related [flat|nested] 17+ messages in thread* [ndctl PATCH 09/16] ndctl/dimm: Auto-arm firmware activation
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
` (7 preceding siblings ...)
2020-07-07 2:41 ` [ndctl PATCH 08/16] ndctl/list: Add firmware activation enumeration Dan Williams
@ 2020-07-07 2:41 ` Dan Williams
2020-07-07 2:41 ` [ndctl PATCH 10/16] ndctl/bus: Add 'activate-firmware' command Dan Williams
` (6 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:41 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 | 33 +++++++
ndctl/lib/libndctl.sym | 2
ndctl/libndctl.h | 2
5 files changed, 166 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", ¶m.infile, "input-file", \
#define UPDATE_OPTIONS() \
OPT_STRING('f', "firmware", ¶m.infile, "firmware-file", \
"firmware filename for update"), \
-OPT_BOOLEAN('i', "force", ¶m.force, "ignore ARS status, try to force update")
+OPT_BOOLEAN('i', "force", ¶m.force, "ignore ARS / arm status, try to force update"), \
+OPT_BOOLEAN_SET('A', "arm", ¶m.arm, ¶m.arm_set, \
+ "arm device for firmware activation (default)"), \
+OPT_BOOLEAN_SET('D', "disarm", ¶m.disarm, ¶m.disarm_set, \
+ "disarm device for firmware activation")
#define INIT_OPTIONS() \
OPT_BOOLEAN('f', "force", ¶m.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 f03635e99a83..d11b05f113d5 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -2097,6 +2097,39 @@ 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 related [flat|nested] 17+ messages in thread* [ndctl PATCH 10/16] ndctl/bus: Add 'activate-firmware' command
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
` (8 preceding siblings ...)
2020-07-07 2:41 ` [ndctl PATCH 09/16] ndctl/dimm: Auto-arm firmware activation Dan Williams
@ 2020-07-07 2:41 ` Dan Williams
2020-07-07 2:41 ` [ndctl PATCH 11/16] ndctl/docs: Update copyright date Dan Williams
` (5 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:41 UTC (permalink / raw)
To: linux-nvdimm; +Cc: vishal.l.verma
For platforms where firmware can be activated 'live' at runtime, trigger
that activation. There may also be buses that allow live activation to be
forced with the caveat that it may inflict undefined behavior on bus master
devices actively transmitting data over the activation / platform quiesce
event. In this case the platform makes a best effort to increase
completion timeouts, but if the timeout is violated it device specific how
a device may behave after it has signaled a timeout. Use "--force" with
caution.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Documentation/ndctl/Makefile.am | 3
Documentation/ndctl/ndctl-activate-firmware.txt | 130 +++++++++++++++++
ndctl/action.h | 1
ndctl/builtin.h | 1
ndctl/bus.c | 173 ++++++++++++++++++++++-
ndctl/lib/libndctl.c | 77 ++++++++++
ndctl/lib/libndctl.sym | 5 +
ndctl/libndctl.h | 5 +
ndctl/ndctl.c | 1
9 files changed, 388 insertions(+), 8 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..635299fbdc8e
--- /dev/null
+++ b/Documentation/ndctl/ndctl-activate-firmware.txt
@@ -0,0 +1,130 @@
+// 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 then the default policy for
+firmware activation is to wait for a suspend/resume cycle to quiesce the
+OS before the hard queisce is injected by the platform.
+
+*DANGER* the activate-firmware command includes a --force option to
+attempt to override this "suspend-required" policy. That option should
+only be used in emergencies, or with express knowledge that the platform
+quiesce time would not trigger completion timeout violations for any
+devices in the system.
+
+EXAMPLES
+--------
+Check the activation method 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.
+nfit_test.0: ndbus2: requires a platform suspend event to activate firmware
+error activating firmware: Operation not supported
+----
+
+Check that the suspend requirement can be overridden 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":[
+...
+]
+----
+
+In this case you see that the dry-run activation result is displayed
+rather than the error message. 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 include 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
+ exceeds the time needed to perform the activation. This
+ validation step can be overridden by specifying --no-idle.
+
+-f::
+--force::
+ 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 completion timeouts are
+ violated for in-flight memory operations. Alternatively if
+ "activate_method" reports "live" in the absence of an override
+ it is asserting that the driver detects no adverse quiesce
+ delays are injected to activate firmware.
+
+-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..62f1303830a1 100644
--- a/ndctl/bus.c
+++ b/ndctl/bus.c
@@ -16,8 +16,13 @@
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", ¶m.poll_interval, "poll interval (seconds)")
+#define ACTIVATE_OPTIONS() \
+ OPT_BOOLEAN('I', "idle", ¶m.idle, \
+ "attempt platform idle over activate (default)"), \
+ OPT_BOOLEAN('f', "force", ¶m.force, "try to force live activation"), \
+ OPT_BOOLEAN('n', "dry-run", ¶m.dryrun, \
+ "perform all setup/validation steps, skip the activate")
+
static const struct option start_options[] = {
BASE_OPTIONS(),
OPT_END(),
@@ -37,6 +49,99 @@ 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);
+ bool do_clear_nosuspend = false;
+ 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) {
+ fprintf(stderr, "%s: %s: requires a platform suspend event to activate firmware\n",
+ provider, devname);
+ rc = -EOPNOTSUPP;
+ goto out;
+ }
+
+ if (method == NDCTL_FWA_METHOD_SUSPEND) {
+ rc = ndctl_bus_set_fw_activate_nosuspend(bus);
+ if (rc) {
+ fprintf(stderr, "%s: %s: failed to force live activation.\n",
+ provider, devname);
+ goto out;
+ }
+ do_clear_nosuspend = true;
+ }
+
+ 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);
+ }
+
+ if (rc) {
+ fprintf(stderr, "%s: %s: live firmware activation failed (%s)\n",
+ provider, devname, strerror(-rc));
+ goto out;
+ }
+
+out:
+ if (do_clear_noidle)
+ ndctl_bus_clear_fw_activate_noidle(bus);
+ if (do_clear_nosuspend)
+ ndctl_bus_clear_fw_activate_nosuspend(bus);
+ return rc;
+}
+
static int scrub_action(struct ndctl_bus *bus, enum device_action action)
{
switch (action) {
@@ -50,6 +155,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 +193,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 +224,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 +286,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 d11b05f113d5..d44ca784e631 100644
--- a/ndctl/lib/libndctl.c
+++ b/ndctl/lib/libndctl.c
@@ -1508,6 +1508,83 @@ NDCTL_EXPORT enum ndctl_fwa_method ndctl_bus_get_fw_activate_method(
return NDCTL_FWA_METHOD_RESET;
}
+NDCTL_EXPORT int ndctl_bus_activate_firmware(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 (snprintf(path, len, "%s/firmware_activate", bus->bus_path) >= len) {
+ err(ctx, "%s: buffer too small!\n", ndctl_bus_get_devname(bus));
+ return -ENOMEM;
+ }
+
+ sprintf(buf, "activate\n");
+
+ 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..97979e6cf5aa 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);
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 related [flat|nested] 17+ messages in thread* [ndctl PATCH 11/16] ndctl/docs: Update copyright date
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
` (9 preceding siblings ...)
2020-07-07 2:41 ` [ndctl PATCH 10/16] ndctl/bus: Add 'activate-firmware' command Dan Williams
@ 2020-07-07 2:41 ` Dan Williams
2020-07-07 2:41 ` [ndctl PATCH 12/16] ndctl/test: Test firmware-activation interface Dan Williams
` (4 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:41 UTC (permalink / raw)
To: linux-nvdimm; +Cc: vishal.l.verma
Happy New Year!
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
Documentation/copyright.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/copyright.txt b/Documentation/copyright.txt
index 2820a50d2980..760a5be9779e 100644
--- a/Documentation/copyright.txt
+++ b/Documentation/copyright.txt
@@ -2,7 +2,7 @@
COPYRIGHT
---------
-Copyright (c) 2016 - 2019, Intel Corporation. License GPLv2: GNU GPL
+Copyright (c) 2016 - 2020, 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.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 17+ messages in thread* [ndctl PATCH 12/16] ndctl/test: Test firmware-activation interface
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
` (10 preceding siblings ...)
2020-07-07 2:41 ` [ndctl PATCH 11/16] ndctl/docs: Update copyright date Dan Williams
@ 2020-07-07 2:41 ` Dan Williams
2020-07-07 2:41 ` [ndctl PATCH 13/16] test: Validate strict iomem protections of pmem Dan Williams
` (3 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:41 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 | 50 ++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 45 insertions(+), 5 deletions(-)
diff --git a/test/firmware-update.sh b/test/firmware-update.sh
index ed7d7e53772c..098dd92b4eb3 100755
--- a/test/firmware-update.sh
+++ b/test/firmware-update.sh
@@ -22,23 +22,63 @@ 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"
+
+ # check that unforced activation fails
+ [ $NDCTL activate-firmware -b $NFIT_TEST_BUS0 ] && err "$LINENO"
+
+ # force activation
+ json=$($NDCTL activate-firmware $NFIT_TEST_BUS0 --force)
+ 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 related [flat|nested] 17+ messages in thread* [ndctl PATCH 13/16] test: Validate strict iomem protections of pmem
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
` (11 preceding siblings ...)
2020-07-07 2:41 ` [ndctl PATCH 12/16] ndctl/test: Test firmware-activation interface Dan Williams
@ 2020-07-07 2:41 ` Dan Williams
2020-07-07 2:41 ` [ndctl PATCH 14/16] ndctl: Refactor nfit.h to acpi.h Dan Williams
` (2 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:41 UTC (permalink / raw)
To: linux-nvdimm; +Cc: vishal.l.verma
---
test/Makefile.am | 9 +++
test/revoke-devmem.c | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 152 insertions(+)
create mode 100644 test/revoke-devmem.c
diff --git a/test/Makefile.am b/test/Makefile.am
index 1d24a65ded8b..1dcaa1eb6da5 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -56,6 +56,7 @@ TESTS +=\
dax-xfs.sh \
align.sh \
device-dax \
+ revoke-devmem \
device-dax-fio.sh \
daxctl-devices.sh \
dm.sh \
@@ -71,6 +72,7 @@ check_PROGRAMS +=\
dax-dev \
dax-pmd \
device-dax \
+ revoke-devmem \
mmap
endif
@@ -154,6 +156,13 @@ device_dax_LDADD = \
$(UUID_LIBS) \
../libutil.a
+revoke_devmem_SOURCES = \
+ revoke-devmem.c \
+ dax-dev.c \
+ $(testcore)
+
+revoke_devmem_LDADD = $(LIBNDCTL_LIB)
+
smart_notify_SOURCES = smart-notify.c
smart_notify_LDADD = $(LIBNDCTL_LIB)
smart_listen_SOURCES = smart-listen.c
diff --git a/test/revoke-devmem.c b/test/revoke-devmem.c
new file mode 100644
index 000000000000..ffa509e2d7d1
--- /dev/null
+++ b/test/revoke-devmem.c
@@ -0,0 +1,143 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2020 Intel Corporation. All rights reserved. */
+#include <fcntl.h>
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <syslog.h>
+#include <string.h>
+#include <signal.h>
+#include <setjmp.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <util/size.h>
+#include <linux/falloc.h>
+#include <linux/version.h>
+#include <ndctl/libndctl.h>
+#include <ccan/array_size/array_size.h>
+
+#include <builtin.h>
+#include <test.h>
+
+static sigjmp_buf sj_env;
+
+static void sigbus(int sig, siginfo_t *siginfo, void *d)
+{
+ siglongjmp(sj_env, 1);
+}
+
+#define err(fmt, ...) \
+ fprintf(stderr, "%s: " fmt, __func__, ##__VA_ARGS__)
+
+static int test_devmem(int loglevel, struct ndctl_test *test,
+ struct ndctl_ctx *ctx)
+{
+ void *buf;
+ int fd, rc;
+ struct sigaction act;
+ unsigned long long resource;
+ struct ndctl_namespace *ndns;
+
+ ndctl_set_log_priority(ctx, loglevel);
+
+ /* iostrict devmem started in kernel 4.5 */
+ if (!ndctl_test_attempt(test, KERNEL_VERSION(4, 5, 0)))
+ return 77;
+
+ ndns = ndctl_get_test_dev(ctx);
+ if (!ndns) {
+ err("failed to find suitable namespace\n");
+ return 77;
+ }
+
+ resource = ndctl_namespace_get_resource(ndns);
+ if (resource == ULLONG_MAX) {
+ err("failed to retrieve resource base\n");
+ return 77;
+ }
+
+ rc = ndctl_namespace_disable(ndns);
+ if (rc) {
+ err("failed to disable namespace\n");
+ return rc;
+ }
+
+ /* establish a devmem mapping of the namespace memory */
+ fd = open("/dev/mem", O_RDWR);
+ if (fd < 0) {
+ err("failed to open /dev/mem: %s\n", strerror(errno));
+ rc = -errno;
+ goto out_devmem;
+ }
+
+ buf = mmap(NULL, SZ_2M, PROT_READ|PROT_WRITE, MAP_SHARED, fd, resource);
+ if (buf == MAP_FAILED) {
+ err("failed to map /dev/mem: %s\n", strerror(errno));
+ rc = -errno;
+ goto out_mmap;
+ }
+
+ /* populate and write, should not fail */
+ memset(buf, 0, SZ_2M);
+
+ memset(&act, 0, sizeof(act));
+ act.sa_sigaction = sigbus;
+ act.sa_flags = SA_SIGINFO;
+ if (sigaction(SIGBUS, &act, 0)) {
+ perror("sigaction");
+ rc = EXIT_FAILURE;
+ goto out_sigaction;
+ }
+
+ /* test fault due to devmem revocation */
+ if (sigsetjmp(sj_env, 1)) {
+ /* got sigbus, success */
+ fprintf(stderr, "devmem revoked!\n");
+ rc = 0;
+ goto out_sigaction;
+ }
+
+ rc = ndctl_namespace_enable(ndns);
+ if (rc) {
+ err("failed to enable namespace\n");
+ goto out_sigaction;
+ }
+
+ /* write, should sigbus */
+ memset(buf, 0, SZ_2M);
+
+ err("kernel failed to prevent write after namespace enabled\n");
+ rc = -ENXIO;
+
+out_sigaction:
+ munmap(buf, SZ_2M);
+out_mmap:
+ close(fd);
+out_devmem:
+ if (ndctl_namespace_enable(ndns) != 0)
+ err("failed to re-enable namespace\n");
+ return rc;
+}
+
+int main(int argc, char *argv[])
+{
+ struct ndctl_test *test = ndctl_test_new(0);
+ struct ndctl_ctx *ctx;
+ int rc;
+
+ if (!test) {
+ fprintf(stderr, "failed to initialize test\n");
+ return EXIT_FAILURE;
+ }
+
+ rc = ndctl_new(&ctx);
+ if (rc < 0)
+ return ndctl_test_result(test, rc);
+
+ rc = test_devmem(LOG_DEBUG, test, ctx);
+ ndctl_unref(ctx);
+ return ndctl_test_result(test, rc);
+}
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 17+ messages in thread* [ndctl PATCH 14/16] ndctl: Refactor nfit.h to acpi.h
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
` (12 preceding siblings ...)
2020-07-07 2:41 ` [ndctl PATCH 13/16] test: Validate strict iomem protections of pmem Dan Williams
@ 2020-07-07 2:41 ` Dan Williams
2020-07-07 2:41 ` [ndctl PATCH 15/16] daxctl: Add 'split-acpi' command to generate custom ACPI tables Dan Williams
2020-07-07 2:41 ` [ndctl PATCH 16/16] test/ndctl: mremap pmd confusion Dan Williams
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:41 UTC (permalink / raw)
To: linux-nvdimm; +Cc: vishal.l.verma
In preparation for adding other acpi table creation helper utilities
make a common acpi.h header with common helpers and definitions.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
acpi.h | 110 +++++++++++++++++++++++++++++++++++++++++++++++++++
ndctl/create-nfit.c | 66 ++++++-------------------------
nfit.h | 65 ------------------------------
3 files changed, 124 insertions(+), 117 deletions(-)
create mode 100644 acpi.h
delete mode 100644 nfit.h
diff --git a/acpi.h b/acpi.h
new file mode 100644
index 000000000000..06685aa2c90a
--- /dev/null
+++ b/acpi.h
@@ -0,0 +1,110 @@
+/*
+ * ACPI Table Definitions
+ *
+ * Copyright(c) 2013-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.
+ */
+#ifndef __ACPI_H__
+#define __ACPI_H__
+#include <stdint.h>
+#include <linux/uuid.h>
+
+static inline void nfit_spa_uuid_pm(void *uuid)
+{
+ uuid_le uuid_pm = UUID_LE(0x66f0d379, 0xb4f3, 0x4074, 0xac, 0x43, 0x0d,
+ 0x33, 0x18, 0xb7, 0x8c, 0xdb);
+ memcpy(uuid, &uuid_pm, 16);
+}
+
+enum {
+ NFIT_TABLE_SPA = 0,
+ SRAT_TABLE_MEM = 1,
+ SRAT_MEM_ENABLED = (1<<0),
+ SRAT_MEM_HOT_PLUGGABLE = (1<<1),
+ SRAT_MEM_NON_VOLATILE = (1<<2),
+};
+
+/**
+ * struct nfit - Nvdimm Firmware Interface Table
+ * @signature: "ACPI"
+ * @length: sum of size of this table plus all appended subtables
+ */
+struct acpi_header {
+ uint8_t signature[4];
+ uint32_t length;
+ uint8_t revision;
+ uint8_t checksum;
+ uint8_t oemid[6];
+ uint64_t oem_tbl_id;
+ uint32_t oem_revision;
+ uint32_t asl_id;
+ uint32_t asl_revision;
+} __attribute__((packed));
+
+struct nfit {
+ struct acpi_header h;
+ uint32_t reserved;
+} __attribute__((packed));
+
+/**
+ * struct nfit_spa - System Physical Address Range Descriptor Table
+ */
+struct nfit_spa {
+ uint16_t type;
+ uint16_t length;
+ uint16_t range_index;
+ uint16_t flags;
+ uint32_t reserved;
+ uint32_t proximity_domain;
+ uint8_t type_uuid[16];
+ uint64_t spa_base;
+ uint64_t spa_length;
+ uint64_t mem_attr;
+} __attribute__((packed));
+
+static inline unsigned char acpi_checksum(void *buf, size_t size)
+{
+ unsigned char sum, *data = buf;
+ size_t i;
+
+ for (sum = 0, i = 0; i < size; i++)
+ sum += data[i];
+ return 0 - sum;
+}
+
+static inline void writeq(uint64_t v, void *a)
+{
+ uint64_t *p = a;
+
+ *p = htole64(v);
+}
+
+static inline void writel(uint32_t v, void *a)
+{
+ uint32_t *p = a;
+
+ *p = htole32(v);
+}
+
+static inline void writew(unsigned short v, void *a)
+{
+ unsigned short *p = a;
+
+ *p = htole16(v);
+}
+
+static inline void writeb(unsigned char v, void *a)
+{
+ unsigned char *p = a;
+
+ *p = v;
+}
+#endif /* __ACPI_H__ */
diff --git a/ndctl/create-nfit.c b/ndctl/create-nfit.c
index 53319182d8a4..8f05eab81494 100644
--- a/ndctl/create-nfit.c
+++ b/ndctl/create-nfit.c
@@ -22,7 +22,7 @@
#include <util/parse-options.h>
#include <util/size.h>
-#include <nfit.h>
+#include <acpi.h>
#define DEFAULT_NFIT "local_nfit.bin"
static const char *nfit_file = DEFAULT_NFIT;
@@ -68,44 +68,6 @@ static int parse_add_spa(const struct option *option, const char *__arg, int uns
return rc;
}
-static unsigned char nfit_checksum(void *buf, size_t size)
-{
- unsigned char sum, *data = buf;
- size_t i;
-
- for (sum = 0, i = 0; i < size; i++)
- sum += data[i];
- return 0 - sum;
-}
-
-static void writeq(unsigned long long v, void *a)
-{
- unsigned long long *p = a;
-
- *p = htole64(v);
-}
-
-static void writel(unsigned long v, void *a)
-{
- unsigned long *p = a;
-
- *p = htole32(v);
-}
-
-static void writew(unsigned short v, void *a)
-{
- unsigned short *p = a;
-
- *p = htole16(v);
-}
-
-static void writeb(unsigned char v, void *a)
-{
- unsigned char *p = a;
-
- *p = v;
-}
-
static struct nfit *create_nfit(struct list_head *spa_list)
{
struct nfit_spa *nfit_spa;
@@ -124,15 +86,15 @@ static struct nfit *create_nfit(struct list_head *spa_list)
return NULL;
/* nfit header */
- nfit = (struct nfit *) buf;
- memcpy(nfit->signature, "NFIT", 4);
- writel(size, &nfit->length);
- writeb(1, &nfit->revision);
- memcpy(nfit->oemid, "LOCAL", 6);
- writew(1, &nfit->oem_tbl_id);
- writel(1, &nfit->oem_revision);
- writel(0x80860000, &nfit->creator_id);
- writel(1, &nfit->creator_revision);
+ nfit = (typeof(nfit)) buf;
+ memcpy(nfit->h.signature, "NFIT", 4);
+ writel(size, &nfit->h.length);
+ writeb(1, &nfit->h.revision);
+ memcpy(nfit->h.oemid, "LOCAL", 6);
+ writew(1, &nfit->h.oem_tbl_id);
+ writel(1, &nfit->h.oem_revision);
+ writel(0x80860000, &nfit->h.asl_id);
+ writel(1, &nfit->h.asl_revision);
nfit_spa = (struct nfit_spa *) (buf + sizeof(*nfit));
i = 1;
@@ -146,7 +108,7 @@ static struct nfit *create_nfit(struct list_head *spa_list)
nfit_spa++;
}
- writeb(nfit_checksum(buf, size), &nfit->checksum);
+ writeb(acpi_checksum(buf, size), &nfit->h.checksum);
return nfit;
}
@@ -170,7 +132,7 @@ static int write_nfit(struct nfit *nfit, const char *file, int force)
return -errno;
}
- rc = write(fd, nfit, le32toh(nfit->length));
+ rc = write(fd, nfit, le32toh(nfit->h.length));
close(fd);
return rc;
}
@@ -210,9 +172,9 @@ int cmd_create_nfit(int argc, const char **argv, struct ndctl_ctx *ctx)
goto out;
rc = write_nfit(nfit, nfit_file, force);
- if ((unsigned int) rc == le32toh(nfit->length)) {
+ if ((unsigned int) rc == le32toh(nfit->h.length)) {
fprintf(stderr, "wrote %d bytes to %s\n",
- le32toh(nfit->length), nfit_file);
+ le32toh(nfit->h.length), nfit_file);
rc = 0;
}
diff --git a/nfit.h b/nfit.h
deleted file mode 100644
index 9815d2143a9e..000000000000
--- a/nfit.h
+++ /dev/null
@@ -1,65 +0,0 @@
-/*
- * NVDIMM Firmware Interface Table - NFIT
- *
- * Copyright(c) 2013-2016 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.
- */
-#ifndef __NFIT_H__
-#define __NFIT_H__
-#include <stdint.h>
-#include <linux/uuid.h>
-
-static inline void nfit_spa_uuid_pm(void *uuid)
-{
- uuid_le uuid_pm = UUID_LE(0x66f0d379, 0xb4f3, 0x4074, 0xac, 0x43, 0x0d,
- 0x33, 0x18, 0xb7, 0x8c, 0xdb);
- memcpy(uuid, &uuid_pm, 16);
-}
-
-enum {
- NFIT_TABLE_SPA = 0,
-};
-
-/**
- * struct nfit - Nvdimm Firmware Interface Table
- * @signature: "NFIT"
- * @length: sum of size of this table plus all appended subtables
- */
-struct nfit {
- uint8_t signature[4];
- uint32_t length;
- uint8_t revision;
- uint8_t checksum;
- uint8_t oemid[6];
- uint64_t oem_tbl_id;
- uint32_t oem_revision;
- uint32_t creator_id;
- uint32_t creator_revision;
- uint32_t reserved;
-} __attribute__((packed));
-
-/**
- * struct nfit_spa - System Physical Address Range Descriptor Table
- */
-struct nfit_spa {
- uint16_t type;
- uint16_t length;
- uint16_t range_index;
- uint16_t flags;
- uint32_t reserved;
- uint32_t proximity_domain;
- uint8_t type_uuid[16];
- uint64_t spa_base;
- uint64_t spa_length;
- uint64_t mem_attr;
-} __attribute__((packed));
-
-#endif /* __NFIT_H__ */
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 17+ messages in thread* [ndctl PATCH 15/16] daxctl: Add 'split-acpi' command to generate custom ACPI tables
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
` (13 preceding siblings ...)
2020-07-07 2:41 ` [ndctl PATCH 14/16] ndctl: Refactor nfit.h to acpi.h Dan Williams
@ 2020-07-07 2:41 ` Dan Williams
2020-07-07 2:41 ` [ndctl PATCH 16/16] test/ndctl: mremap pmd confusion Dan Williams
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:41 UTC (permalink / raw)
To: linux-nvdimm; +Cc: vishal.l.verma
While the numa_emulation (fake-numa) driver allows *all* physical nodes to
be split by a given ratio / size, sometimes what is desired is to split a
single memory range into multiple nodes. Also, numa_emulation can only do
this for injecting a fake SRAT, but splitting a NUMA node may also involve
splitting the NFIT. ACPI table injection is more flexible and capable than
what can be described on the kernel command line to the numa_emulation
driver.
Add support to the daxctl utility for splitting SRAT, SLIT, and NFIT by
proximity domain. Yes, this might be a better fit in acpica-tools, but out
of laziness and familiarity I picked daxctl.
Also, for simplicity, this only supports dividing a node by a power-of-2
factor. Conceivably this could later be extended for custom splits, but for
now this covers the most likely common case of "split in half".
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
acpi.h | 132 ++++++++
daxctl/Makefile.am | 1
daxctl/acpi.c | 870 ++++++++++++++++++++++++++++++++++++++++++++++++++++
daxctl/builtin.h | 1
daxctl/daxctl.c | 1
5 files changed, 1002 insertions(+), 3 deletions(-)
create mode 100644 daxctl/acpi.c
diff --git a/acpi.h b/acpi.h
index 06685aa2c90a..e714e28e2354 100644
--- a/acpi.h
+++ b/acpi.h
@@ -17,11 +17,12 @@
#include <stdint.h>
#include <linux/uuid.h>
+static const uuid_le uuid_pmem = UUID_LE(0x66f0d379, 0xb4f3, 0x4074, 0xac, 0x43, 0x0d,
+ 0x33, 0x18, 0xb7, 0x8c, 0xdb);
+
static inline void nfit_spa_uuid_pm(void *uuid)
{
- uuid_le uuid_pm = UUID_LE(0x66f0d379, 0xb4f3, 0x4074, 0xac, 0x43, 0x0d,
- 0x33, 0x18, 0xb7, 0x8c, 0xdb);
- memcpy(uuid, &uuid_pm, 16);
+ memcpy(uuid, &uuid_pmem, 16);
}
enum {
@@ -54,6 +55,18 @@ struct nfit {
uint32_t reserved;
} __attribute__((packed));
+enum acpi_nfit_type {
+ ACPI_NFIT_TYPE_SYSTEM_ADDRESS = 0,
+ ACPI_NFIT_TYPE_MEMORY_MAP = 1,
+ ACPI_NFIT_TYPE_INTERLEAVE = 2,
+ ACPI_NFIT_TYPE_SMBIOS = 3,
+ ACPI_NFIT_TYPE_CONTROL_REGION = 4,
+ ACPI_NFIT_TYPE_DATA_REGION = 5,
+ ACPI_NFIT_TYPE_FLUSH_ADDRESS = 6,
+ ACPI_NFIT_TYPE_CAPABILITIES = 7,
+ ACPI_NFIT_TYPE_RESERVED = 8 /* 8 and greater are reserved */
+};
+
/**
* struct nfit_spa - System Physical Address Range Descriptor Table
*/
@@ -70,6 +83,91 @@ struct nfit_spa {
uint64_t mem_attr;
} __attribute__((packed));
+struct nfit_map {
+ uint16_t type;
+ uint16_t length;
+ uint32_t device_handle;
+ uint16_t physical_id;
+ uint16_t region_id;
+ uint16_t range_index;
+ uint16_t region_index;
+ uint64_t region_size;
+ uint64_t region_offset;
+ uint64_t address;
+ uint16_t interleave_index;
+ uint16_t interleave_ways;
+ uint16_t flags;
+ uint16_t reserved; /* Reserved, must be zero */
+} __attribute__((packed));
+
+struct srat {
+ struct acpi_header h;
+ uint32_t revision;
+ uint64_t reserved;
+} __attribute__((packed));
+
+enum acpi_srat_type {
+ ACPI_SRAT_TYPE_CPU_AFFINITY = 0,
+ ACPI_SRAT_TYPE_MEMORY_AFFINITY = 1,
+ ACPI_SRAT_TYPE_X2APIC_CPU_AFFINITY = 2,
+ ACPI_SRAT_TYPE_GICC_AFFINITY = 3,
+ ACPI_SRAT_TYPE_GIC_ITS_AFFINITY = 4, /* ACPI 6.2 */
+ ACPI_SRAT_TYPE_GENERIC_AFFINITY = 5, /* ACPI 6.3 */
+ ACPI_SRAT_TYPE_RESERVED = 6 /* 5 and greater are reserved */
+};
+
+struct srat_cpu {
+ uint8_t type;
+ uint8_t length;
+ uint8_t proximity_domain_lo;
+ uint8_t apic_id;
+ uint32_t flags;
+ uint8_t local_sapic_eid;
+ uint8_t proximity_domain_hi[3];
+ uint32_t clock_domain;
+} __attribute__((packed));
+
+struct srat_generic {
+ uint8_t type;
+ uint8_t length;
+ uint8_t reserved;
+ uint8_t device_handle_type;
+ uint32_t proximity_domain;
+ uint8_t device_handle[16];
+ uint32_t flags;
+ uint32_t reserved1;
+} __attribute__((packed));
+
+struct srat_mem {
+ uint8_t type;
+ uint8_t length;
+ uint32_t proximity_domain;
+ uint16_t reserved;
+ uint64_t spa_base;
+ uint64_t spa_length;
+ uint32_t reserved1;
+ uint32_t flags;
+ uint64_t reserved2;
+} __attribute__((packed));
+
+struct acpi_subtable8 {
+ uint8_t type;
+ uint8_t length;
+ uint8_t buf[];
+} __attribute__((packed));
+
+struct acpi_subtable16 {
+ uint16_t type;
+ uint16_t length;
+ uint8_t buf[];
+} __attribute__((packed));
+
+struct slit {
+ struct acpi_header h;
+ uint64_t count;
+ uint8_t entry[]; /* size = count^2 */
+} __attribute__((packed));
+
static inline unsigned char acpi_checksum(void *buf, size_t size)
{
unsigned char sum, *data = buf;
@@ -107,4 +205,32 @@ static inline void writeb(unsigned char v, void *a)
*p = v;
}
+
+static inline uint64_t readq(void *a)
+{
+ uint64_t *p = a;
+
+ return le64toh(*p);
+}
+
+static inline uint32_t readl(void *a)
+{
+ uint32_t *p = a;
+
+ return le32toh(*p);
+}
+
+static inline uint16_t readw(void *a)
+{
+ uint16_t *p = a;
+
+ return le16toh(*p);
+}
+
+static inline uint8_t readb(void *a)
+{
+ uint8_t *p = a;
+
+ return *p;
+}
#endif /* __ACPI_H__ */
diff --git a/daxctl/Makefile.am b/daxctl/Makefile.am
index ca1b86748bfb..9b1313ac547f 100644
--- a/daxctl/Makefile.am
+++ b/daxctl/Makefile.am
@@ -13,6 +13,7 @@ config.h: $(srcdir)/Makefile.am
daxctl_SOURCES =\
daxctl.c \
+ acpi.c \
list.c \
migrate.c \
device.c \
diff --git a/daxctl/acpi.c b/daxctl/acpi.c
new file mode 100644
index 000000000000..5d0e3df15aa9
--- /dev/null
+++ b/daxctl/acpi.c
@@ -0,0 +1,870 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2017-2020 Intel Corporation. All rights reserved. */
+#include <stdio.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <endian.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <ccan/list/list.h>
+#include <util/bitmap.h>
+#include <ccan/minmax/minmax.h>
+#include <util/parse-options.h>
+#include <util/size.h>
+
+#include <acpi.h>
+
+static bool verbose;
+
+struct srat_container {
+ struct srat *srat;
+ struct list_head ents;
+};
+
+struct srat_ent {
+ struct list_node list;
+ struct acpi_subtable8 *tbl;
+};
+
+struct nfit_container {
+ struct nfit *nfit;
+ struct list_head ents;
+};
+
+struct nfit_ent {
+ struct list_node list;
+ struct acpi_subtable16 *tbl;
+};
+
+static void free_srat_container(struct srat_container *container)
+{
+ struct srat_ent *ent, *_e;
+
+ if (!container)
+ return;
+
+ list_for_each_safe(&container->ents, ent, _e, list) {
+ list_del_from(&container->ents, &ent->list);
+ free(ent);
+ }
+ free(container->srat);
+ free(container);
+}
+
+static void free_nfit_container(struct nfit_container *container)
+{
+ struct nfit_ent *ent, *_e;
+
+ if (!container)
+ return;
+
+ list_for_each_safe(&container->ents, ent, _e, list) {
+ list_del_from(&container->ents, &ent->list);
+ free(ent);
+ }
+ free(container->nfit);
+ free(container);
+}
+
+static void *read_table(int fd, const char *sig)
+{
+ int rc, len;
+ uint8_t checksum;
+ struct acpi_header hdr;
+ struct acpi_header *data = NULL;
+
+ rc = read(fd, &hdr, sizeof(hdr));
+ if (rc < (int) sizeof(hdr)) {
+ error("failed to read header\n");
+ rc = rc < 0 ? -errno : -EINVAL;
+ goto out;
+ }
+
+ if (strncmp((char *) hdr.signature, sig, 4) != 0) {
+ error("invalid %s header\n", sig);
+ rc = -EINVAL;
+ goto out;
+ }
+
+ data = calloc(1, hdr.length);
+ if (!data) {
+ error("failed to alloc %d bytes\n", hdr.length);
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ for (len = hdr.length; len > 0;) {
+ int offset = hdr.length - len;
+
+ rc = pread(fd, ((char *) data) + offset, len, offset);
+ if (rc < 0)
+ break;
+ len -= rc;
+ }
+
+ if (rc < 0) {
+ error("failed to read %s\n", sig);
+ rc = rc < 0 ? -errno : -EINVAL;
+ goto out;
+ }
+
+ checksum = data->checksum;
+ data->checksum = 0;
+ if (acpi_checksum(data, data->length) != checksum) {
+ error("bad %s checksum\n", sig);
+ rc = -EINVAL;
+ goto out;
+ }
+out:
+ close(fd);
+ if (rc < 0) {
+ free(data);
+ data = NULL;
+ }
+ return data;
+}
+
+static struct nfit_container *read_nfit(int fd)
+{
+ void *buf;
+ int rc = 0;
+ unsigned int length;
+ struct nfit *nfit = NULL;
+ struct nfit_container *container = NULL;
+
+ nfit = read_table(fd, "NFIT");
+ if (!nfit)
+ return NULL;
+
+ container = calloc(1, sizeof(*container));
+ if (!container) {
+ error("failed to alloc %d bytes\n", nfit->h.length);
+ rc = -ENOMEM;
+ goto out;
+ }
+ list_head_init(&container->ents);
+ container->nfit = nfit;
+
+ length = nfit->h.length - sizeof(*nfit);
+ if (!length) {
+ error("no sub-tables found in SRAT\n");
+ rc = -EINVAL;
+ goto out;
+ }
+
+ buf = nfit + 1;
+ while (length) {
+ struct nfit_ent *ent = calloc(1, sizeof(*ent));
+
+ if (!ent) {
+ error("failed to alloc %zd bytes\n", sizeof(*ent));
+ rc = -ENOMEM;
+ goto out;
+ }
+ ent->tbl = (struct acpi_subtable16 *) buf;
+ list_add_tail(&container->ents, &ent->list);
+ if (readw(&ent->tbl->length) > length
+ || !readw(&ent->tbl->length)) {
+ error("failed to validate all SRAT entries\n");
+ rc = -EINVAL;
+ goto out;
+ }
+ length -= readw(&ent->tbl->length);
+ buf += readw(&ent->tbl->length);
+ }
+out:
+ if (rc < 0) {
+ if (container)
+ free_nfit_container(container);
+ else
+ free(nfit);
+ container = NULL;
+ }
+ return container;
+}
+
+static struct srat_container *read_srat(int fd)
+{
+ void *buf;
+ int rc = 0;
+ unsigned int length;
+ struct srat *srat = NULL;
+ struct srat_container *container = NULL;
+
+ srat = read_table(fd, "SRAT");
+ if (!srat)
+ return NULL;
+
+ container = calloc(1, sizeof(*container));
+ if (!container) {
+ error("failed to alloc %d bytes\n", srat->h.length);
+ rc = -ENOMEM;
+ goto out;
+ }
+ list_head_init(&container->ents);
+ container->srat = srat;
+
+ length = srat->h.length - sizeof(*srat);
+ if (!length) {
+ error("no sub-tables found in SRAT\n");
+ rc = -EINVAL;
+ goto out;
+ }
+
+ buf = srat + 1;
+ while (length) {
+ struct srat_ent *ent = calloc(1, sizeof(*ent));
+
+ if (!ent) {
+ error("failed to alloc %zd bytes\n", sizeof(*ent));
+ rc = -ENOMEM;
+ goto out;
+ }
+ ent->tbl = (struct acpi_subtable8 *) buf;
+ list_add_tail(&container->ents, &ent->list);
+ if (readb(&ent->tbl->length) > length
+ || !readb(&ent->tbl->length)) {
+ error("failed to validate all SRAT entries\n");
+ rc = -EINVAL;
+ goto out;
+ }
+ length -= readb(&ent->tbl->length);
+ buf += readb(&ent->tbl->length);
+ }
+out:
+ if (rc < 0) {
+ if (container)
+ free_srat_container(container);
+ else
+ free(srat);
+ container = NULL;
+ }
+ return container;
+}
+
+enum acpi_table {
+ ACPI_SRAT,
+ ACPI_SLIT,
+ ACPI_NFIT,
+ ACPI_TABLES,
+};
+
+static const char *acpi_table_name(enum acpi_table id)
+{
+ const char *names[ACPI_TABLES] = {
+ [ACPI_SRAT] = "srat",
+ [ACPI_SLIT] = "slit",
+ [ACPI_NFIT] = "nfit",
+ };
+
+ return names[id];
+}
+
+struct parameters {
+ char *table[ACPI_TABLES];
+ char *new_table[ACPI_TABLES];
+ int in_fd[ACPI_TABLES];
+ int out_fd[ACPI_TABLES];
+ int nodes;
+ int pxm;
+ const char *path;
+} param = {
+ .nodes = 2,
+};
+
+struct split_context {
+ uint64_t address;
+ uint64_t length;
+ int max_pxm;
+ int max_region_id;
+ int max_range_index;
+};
+
+static int create_nfit(struct parameters *p, struct nfit_container *container,
+ struct list_head *mems)
+{
+ unsigned int oem_revision;
+ size_t orig_size, size;
+ struct nfit_ent *ent;
+ struct nfit *nfit;
+ void *buf;
+ int rc;
+
+ orig_size = readl(&container->nfit->h.length);
+ size = orig_size;
+ list_for_each(mems, ent, list)
+ size += readw(&ent->tbl->length);
+
+ buf = calloc(1, size);
+ if (!buf)
+ return -ENOMEM;
+
+ nfit = buf;
+ memcpy(nfit, container->nfit, sizeof(*nfit));
+ writel(size, &nfit->h.length);
+ oem_revision = readl(&nfit->h.oem_revision);
+ writel(oem_revision + 1, &nfit->h.oem_revision);
+ buf += sizeof(*nfit);
+
+ list_append_list(&container->ents, mems);
+ list_for_each(&container->ents, ent, list) {
+ memcpy(buf, ent->tbl, readw(&ent->tbl->length));
+ buf += readw(&ent->tbl->length);
+ }
+
+ writeb(acpi_checksum(nfit, size), &nfit->h.checksum);
+
+ rc = write(p->out_fd[ACPI_NFIT], nfit, size);
+ free(nfit);
+
+ if (rc < 0)
+ return -errno;
+ return 0;
+}
+
+static int create_srat(struct parameters *p, struct srat_container *container,
+ struct list_head *mems)
+{
+ unsigned int oem_revision;
+ size_t orig_size, size;
+ struct srat_ent *ent;
+ struct srat *srat;
+ void *buf;
+ int rc;
+
+ orig_size = readl(&container->srat->h.length);
+ size = orig_size;
+ list_for_each(mems, ent, list)
+ size += readb(&ent->tbl->length);
+
+ buf = calloc(1, size);
+ if (!buf)
+ return -ENOMEM;
+
+ srat = buf;
+ memcpy(srat, container->srat, sizeof(*srat));
+ writel(size, &srat->h.length);
+ oem_revision = readl(&srat->h.oem_revision);
+ writel(oem_revision + 1, &srat->h.oem_revision);
+ buf += sizeof(*srat);
+
+ list_append_list(&container->ents, mems);
+ list_for_each(&container->ents, ent, list) {
+ memcpy(buf, ent->tbl, readb(&ent->tbl->length));
+ buf += readb(&ent->tbl->length);
+ }
+
+ writeb(acpi_checksum(srat, size), &srat->h.checksum);
+
+ rc = write(p->out_fd[ACPI_SRAT], srat, size);
+ free(srat);
+
+ if (rc < 0)
+ return -errno;
+ return 0;
+}
+
+#define dbg(fmt, ...) \
+ ({if (verbose) { \
+ fprintf(stderr, fmt, ##__VA_ARGS__); \
+ } else { \
+ do { } while (0); \
+ }})
+
+static int split_srat(struct parameters *p, struct split_context *split)
+{
+ struct srat_container *srat = read_srat(p->in_fd[ACPI_SRAT]);
+ struct srat_ent *ent, *found_ent = NULL;
+ int count = 0, max_pxm = 0, i, rc;
+ uint64_t length, address;
+ struct srat_mem *m;
+ LIST_HEAD(mems);
+
+ list_for_each(&srat->ents, ent, list) {
+ struct srat_generic *g;
+ struct srat_cpu *c;
+ int pxm, type;
+
+ type = readb(&ent->tbl->type);
+ switch (type) {
+ case ACPI_SRAT_TYPE_MEMORY_AFFINITY:
+ m = (struct srat_mem *) ent->tbl;
+ pxm = readl(&m->proximity_domain);
+ break;
+ case ACPI_SRAT_TYPE_CPU_AFFINITY:
+ c = (struct srat_cpu *) ent->tbl;
+ pxm = readb(&c->proximity_domain_lo);
+ pxm |= readw(&c->proximity_domain_hi[0]) << 8;
+ pxm |= readb(&c->proximity_domain_hi[2]) << 24;
+ break;
+ case ACPI_SRAT_TYPE_GENERIC_AFFINITY:
+ g = (struct srat_generic *) ent->tbl;
+ pxm = readl(&g->proximity_domain);
+ break;
+ default:
+ pxm = -1;
+ break;
+ }
+ max_pxm = max(pxm, max_pxm);
+
+ if (type != ACPI_SRAT_TYPE_MEMORY_AFFINITY)
+ continue;
+
+ if (p->pxm == pxm) {
+ found_ent = ent;
+ count++;
+ }
+
+ if (count > 1) {
+ error("SRAT: no support for splitting multiple entry proximity domains\n");
+ return -ENXIO;
+ }
+
+ }
+
+ if (!found_ent) {
+ error("SRAT: proximity domain to split not found\n");
+ free_srat_container(srat);
+ return -ENOENT;
+ }
+ ent = found_ent;
+ m = (struct srat_mem *) ent->tbl;
+ address = readq(&m->spa_base);
+ length = readq(&m->spa_length);
+
+ *split = (struct split_context) {
+ .address = address,
+ .length = length,
+ .max_pxm = max_pxm,
+ };
+
+ length /= p->nodes;
+ writeq(length, &m->spa_length);
+ dbg("SRAT: edit: %#llx@%#llx pxm: %d\n", (unsigned long long) length,
+ (unsigned long long) address, p->pxm);
+
+ address += length;
+
+ for (i = 0; i < p->nodes - 1; i++) {
+ struct srat_mem *srat_mem = calloc(1, sizeof(*srat_mem));
+
+ if (!srat_mem) {
+ error("failed to alloc srat entry\n");
+ return -ENOMEM;
+ }
+
+ ent = calloc(1, sizeof(*ent));
+ if (!ent) {
+ error("failed to alloc srat entry\n");
+ free(srat_mem);
+ return -ENOMEM;
+ }
+
+ ent->tbl = (struct acpi_subtable8 *) srat_mem;
+ writeb(ACPI_SRAT_TYPE_MEMORY_AFFINITY, &srat_mem->type);
+ writeb(sizeof(*srat_mem), &srat_mem->length);
+ writel(max_pxm + 1 + i, &srat_mem->proximity_domain);
+ writeq(address, &srat_mem->spa_base);
+ writeq(length, &srat_mem->spa_length);
+ srat_mem->flags = m->flags;
+ dbg("SRAT: add: %#llx@%#llx pxm: %d\n",
+ (unsigned long long) length,
+ (unsigned long long) address, max_pxm + 1 + i);
+
+ address += length;
+ list_add_tail(&mems, &ent->list);
+ }
+
+ rc = create_srat(p, srat, &mems);
+ free_srat_container(srat);
+ if (rc < 0)
+ return rc;
+ return max_pxm;
+}
+
+static int split_slit(struct parameters *p, struct split_context *split)
+{
+ unsigned int oem_revision;
+ int max_pxm = split->max_pxm;
+ int nodes = max_pxm + p->nodes;
+ struct slit *slit, *slit_old;
+ int old_nodes, rc, i, j;
+ size_t size;
+
+ size = sizeof(*slit) + nodes * nodes;
+ slit = calloc(1, size);
+ if (!slit) {
+ error("failed to allocated %zd bytes\n", size);
+ return -ENOMEM;
+ }
+
+ slit_old = read_table(p->in_fd[ACPI_SLIT], "SLIT");
+ if (!slit_old) {
+ error("failed to read SLIT\n");
+ free(slit);
+ return -ENOMEM;
+ }
+
+ *slit = *slit_old;
+ old_nodes = readq(&slit_old->count);
+ writeq(nodes, &slit->count);
+ writel(size, &slit->h.length);
+ oem_revision = readl(&slit->h.oem_revision);
+ writel(oem_revision + 1, &slit->h.oem_revision);
+ for (i = 0; i < nodes; i++)
+ for (j = 0; j < nodes; j++) {
+ u8 val = 10;
+
+ if (i > max_pxm && j > max_pxm)
+ val = 10;
+ else if (i <= max_pxm && j <= max_pxm)
+ val = slit_old->entry[i * old_nodes + j];
+ else if (i > max_pxm)
+ val = slit_old->entry[p->pxm * old_nodes + j];
+ else if (j > max_pxm)
+ val = slit_old->entry[i * old_nodes + p->pxm];
+
+ /*
+ * Linux requires distance 10 for the i == j
+ * case and rejects distance 10 rejects the SLIT
+ * if 10 is found anywhere else. Fixup val per
+ * these constraints.
+ */
+ if (val == 10 && i != j)
+ val = 11;
+
+ slit->entry[i * nodes + j] = val;
+ }
+ writeb(acpi_checksum(slit, size), &slit->h.checksum);
+
+ rc = write(p->out_fd[ACPI_SLIT], slit, size);
+ free(slit);
+ free(slit_old);
+ return rc;
+}
+
+static int split_nfit_map(struct parameters *p, struct nfit_map *map,
+ struct list_head *maps, struct split_context *split)
+{
+ int rc, i, max_region_id = split->max_region_id,
+ max_range_index = split->max_range_index;
+ uint64_t region_offset, region_size;
+ struct nfit_ent *ent, *_ent;
+
+ region_offset = readq(&map->region_offset);
+ region_size = readq(&map->region_size);
+ region_size /= p->nodes;
+ writeq(region_size, &map->region_size);
+ dbg("NFIT: edit: %#llx@%#llx region_id: %d\n",
+ (unsigned long long) region_size,
+ (unsigned long long) region_offset,
+ readw(&map->region_id));
+ region_offset += region_size;
+
+ for (i = 0; i < p->nodes - 1; i++) {
+ struct nfit_map *nfit_map = calloc(1, sizeof(*nfit_map));
+
+ if (!nfit_map) {
+ error("failed to alloc nfit entry\n");
+ rc = -ENOMEM;
+ break;
+ }
+
+ ent = calloc(1, sizeof(*ent));
+ if (!ent) {
+ error("failed to alloc nfit entry\n");
+ free(nfit_map);
+ rc = -ENOMEM;
+ break;
+ }
+
+ ent->tbl = (struct acpi_subtable16 *) nfit_map;
+ *nfit_map = *map;
+ writew(max_region_id + 1 + i, &nfit_map->region_id);
+ writew(max_range_index + 1 + i, &nfit_map->range_index);
+ writeq(region_size, &nfit_map->region_size);
+ writeq(region_offset, &nfit_map->region_offset);
+
+ dbg("NFIT: add: %#llx@%#llx region_id: %d\n",
+ (unsigned long long) region_size,
+ (unsigned long long) region_offset,
+ max_region_id + 1 + i);
+
+ region_offset += region_size;
+ list_add_tail(maps, &ent->list);
+ }
+
+ if (i < p->nodes - 1)
+ list_for_each_safe(maps, ent, _ent, list) {
+ list_del(&ent->list);
+ free(ent->tbl);
+ free(ent);
+ return rc;
+ }
+
+ split->max_region_id = max_region_id + i;
+ return 0;
+}
+
+static int split_nfit(struct parameters *p, struct split_context *split)
+{
+ int count = 0, max_pxm = split->max_pxm, i, rc, max_range_index = 0,
+ max_region_id = 0;
+ struct nfit_container *nfit = read_nfit(p->in_fd[ACPI_NFIT]);
+ struct nfit_ent *ent, *_ent, *found_ent = NULL;
+ uint64_t length, address;
+ struct nfit_spa *spa;
+ struct nfit_map *map;
+ LIST_HEAD(new_maps);
+ LIST_HEAD(mems);
+ LIST_HEAD(maps);
+
+ list_for_each(&nfit->ents, ent, list) {
+ int pxm, type, range_index, region_id;
+
+ type = readw(&ent->tbl->type);
+ if (type == ACPI_NFIT_TYPE_MEMORY_MAP) {
+ map = (struct nfit_map *) ent->tbl;
+ region_id = readw(&map->region_id);
+ max_region_id = max(max_region_id, region_id);
+ continue;
+ }
+
+ if (type != ACPI_NFIT_TYPE_SYSTEM_ADDRESS)
+ continue;
+
+ spa = (struct nfit_spa *) ent->tbl;
+ range_index = readw(&spa->range_index);
+ max_range_index = max(range_index, max_range_index);
+
+ if (memcmp(&spa->type_uuid, &uuid_pmem, sizeof(uuid_pmem)) != 0)
+ continue;
+
+ pxm = readl(&spa->proximity_domain);
+ if (pxm != p->pxm)
+ continue;
+
+ if (split->address != readq(&spa->spa_base))
+ continue;
+
+ if (split->length != readq(&spa->spa_length))
+ continue;
+
+ found_ent = ent;
+ count++;
+
+ if (count > 1) {
+ error("NFIT: no support for splitting multiple entry proximity domains\n");
+ return -ENXIO;
+ }
+ }
+
+ if (!found_ent) {
+ dbg("NFIT: proximity domain to split not found\n");
+ free_nfit_container(nfit);
+ return -ENOENT;
+ }
+ ent = found_ent;
+ spa = (struct nfit_spa *) ent->tbl;
+ address = readq(&spa->spa_base);
+ length = readq(&spa->spa_length) / p->nodes;
+ writeq(length, &spa->spa_length);
+ dbg("NFIT: edit: %#llx@%#llx pxm: %d\n", (unsigned long long) length,
+ (unsigned long long) address, p->pxm);
+ address += length;
+
+ for (i = 0; i < p->nodes - 1; i++) {
+ struct nfit_spa *nfit_spa = calloc(1, sizeof(*nfit_spa));
+
+ if (!nfit_spa) {
+ error("failed to alloc nfit entry\n");
+ rc = -ENOMEM;
+ break;
+ }
+
+ ent = calloc(1, sizeof(*ent));
+ if (!ent) {
+ error("failed to alloc nfit entry\n");
+ free(nfit_spa);
+ rc = -ENOMEM;
+ break;
+ }
+
+ ent->tbl = (struct acpi_subtable16 *) nfit_spa;
+ *nfit_spa = *spa;
+ writew(max_range_index + i + 1, &nfit_spa->range_index);
+ writel(max_pxm + 1 + i, &nfit_spa->proximity_domain);
+ writeq(address, &nfit_spa->spa_base);
+ writeq(length, &nfit_spa->spa_length);
+
+ dbg("NFIT: add: %#llx@%#llx pxm: %d\n",
+ (unsigned long long) length,
+ (unsigned long long) address, max_pxm + 1 + i);
+
+ address += length;
+ list_add_tail(&mems, &ent->list);
+ }
+
+ if (i < p->nodes - 1)
+ list_for_each_safe(&mems, ent, _ent, list) {
+ list_del(&ent->list);
+ free(ent->tbl);
+ free(ent);
+ return rc;
+ }
+
+ /*
+ * Find and split the maps that might be referring to split
+ * address range.
+ */
+ split->max_region_id = max_region_id;
+ split->max_range_index = max_range_index;
+ list_for_each_safe(&nfit->ents, ent, _ent, list) {
+ unsigned int type;
+
+ type = readw(&ent->tbl->type);
+ if (type != ACPI_NFIT_TYPE_MEMORY_MAP)
+ continue;
+ map = (struct nfit_map *) ent->tbl;
+ if (map->range_index != spa->range_index)
+ continue;
+ list_del_from(&nfit->ents, &ent->list);
+ list_add_tail(&maps, &ent->list);
+ }
+
+ list_for_each(&maps, ent, list) {
+ map = (struct nfit_map *) ent->tbl;
+ rc = split_nfit_map(p, map, &new_maps, split);
+ if (rc)
+ return rc;
+ }
+
+ list_append_list(&maps, &new_maps);
+ list_append_list(&mems, &maps);
+
+ rc = create_nfit(p, nfit, &mems);
+ free_nfit_container(nfit);
+ if (rc < 0)
+ return rc;
+ return max_pxm;
+}
+
+static int do_split(struct parameters *p)
+{
+ struct split_context split;
+ int rc = split_srat(p, &split);
+
+ if (rc < 0)
+ return rc;
+ fprintf(stderr, "created: %s\n", p->new_table[ACPI_SRAT]);
+
+ rc = split_slit(p, &split);
+ if (rc < 0)
+ return rc;
+ fprintf(stderr, "created: %s\n", p->new_table[ACPI_SLIT]);
+
+ rc = split_nfit(p, &split);
+ if (rc == -ENOENT) {
+ unlink(p->new_table[ACPI_NFIT]);
+ return 0;
+ }
+
+ if (rc < 0)
+ return rc;
+
+ fprintf(stderr, "created: %s\n", p->new_table[ACPI_NFIT]);
+ return 0;
+}
+
+int cmd_split_acpi(int argc, const char **argv, void *ctx)
+{
+ int i, rc = 0;
+ const char * const u[] = {
+ "daxctl split-acpi <options>",
+ NULL
+ };
+ const struct option options[] = {
+ OPT_STRING('d', "directory", ¶m.path, "path",
+ "Path to ACPI tables dumped by \"acpixtract -a\""),
+ OPT_INTEGER('p', "pxm", ¶m.pxm,
+ "Proximity domain to split"),
+ OPT_INTEGER('n', "nodes", ¶m.nodes,
+ "Number of nodes to split capacity (default 2)"),
+ OPT_BOOLEAN('v', "verbose", &verbose, "Enable verbose output"),
+ OPT_END(),
+ };
+
+ argc = parse_options(argc, argv, options, u, 0);
+
+ for (i = 0; i < argc; i++) {
+ error("unknown parameter \"%s\"\n", argv[i]);
+ rc = -EINVAL;
+ }
+
+ if (param.nodes < 2) {
+ error("--nodes=%d, must be greater than 2\n", param.nodes);
+ rc = -EINVAL;
+ }
+
+ if (!is_power_of_2(param.nodes)) {
+ error("--nodes=%d, must be power of 2\n", param.nodes);
+ rc = -EINVAL;
+ }
+
+ if (rc)
+ usage_with_options(u, options);
+
+ for (i = 0; i < ACPI_TABLES; i++) {
+ rc = asprintf(¶m.table[i], "%s/%s.dat", param.path
+ ? param.path : ".", acpi_table_name(i));
+ if (rc < 0) {
+ error("failed to allocate path for %s\n",
+ acpi_table_name(i));
+ break;
+ }
+
+ rc = open(param.table[i], O_RDONLY);
+ if (rc < 0 && i > ACPI_SLIT) {
+ error("failed to open required %s\n", param.table[i]);
+ break;
+ }
+
+ if (rc < 0)
+ continue;
+ param.in_fd[i] = rc;
+
+ rc = asprintf(¶m.new_table[i], "%s/%s.dat.new", param.path
+ ? param.path : ".", acpi_table_name(i));
+ if (rc < 0) {
+ error("failed to allocate path for %s.new\n",
+ acpi_table_name(i));
+ break;
+ }
+
+ rc = open(param.new_table[i], O_RDWR | O_TRUNC | O_CREAT, 0640);
+ if (rc < 0 && i <= ACPI_SLIT) {
+ error("failed to open %s\n", param.new_table[i]);
+ break;
+ }
+ param.out_fd[i] = rc;
+ }
+
+ if (rc < 0) {
+ rc = EXIT_FAILURE;
+ goto out;
+ }
+
+ rc = do_split(¶m);
+out:
+ for (i = 0; i < ACPI_TABLES; i++) {
+ free(param.table[i]);
+ free(param.new_table[i]);
+ if (param.in_fd[i] > 0)
+ close(param.in_fd[i]);
+ if (param.out_fd[i] > 0)
+ close(param.out_fd[i]);
+ }
+ return rc;
+}
diff --git a/daxctl/builtin.h b/daxctl/builtin.h
index f5a0147f0e11..cbee984971cf 100644
--- a/daxctl/builtin.h
+++ b/daxctl/builtin.h
@@ -9,4 +9,5 @@ int cmd_migrate(int argc, const char **argv, struct daxctl_ctx *ctx);
int cmd_reconfig_device(int argc, const char **argv, struct daxctl_ctx *ctx);
int cmd_online_memory(int argc, const char **argv, struct daxctl_ctx *ctx);
int cmd_offline_memory(int argc, const char **argv, struct daxctl_ctx *ctx);
+int cmd_split_acpi(int argc, const char **argv, struct daxctl_ctx *ctx);
#endif /* _DAXCTL_BUILTIN_H_ */
diff --git a/daxctl/daxctl.c b/daxctl/daxctl.c
index 1ab073200313..ec366dd71707 100644
--- a/daxctl/daxctl.c
+++ b/daxctl/daxctl.c
@@ -70,6 +70,7 @@ static struct cmd_struct commands[] = {
{ "version", .d_fn = cmd_version },
{ "list", .d_fn = cmd_list },
{ "help", .d_fn = cmd_help },
+ { "split-acpi", .d_fn = cmd_split_acpi, },
{ "migrate-device-model", .d_fn = cmd_migrate },
{ "reconfigure-device", .d_fn = cmd_reconfig_device },
{ "online-memory", .d_fn = cmd_online_memory },
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 17+ messages in thread* [ndctl PATCH 16/16] test/ndctl: mremap pmd confusion
2020-07-07 2:40 [ndctl PATCH 00/16] Firmware Activation and Test Updates Dan Williams
` (14 preceding siblings ...)
2020-07-07 2:41 ` [ndctl PATCH 15/16] daxctl: Add 'split-acpi' command to generate custom ACPI tables Dan Williams
@ 2020-07-07 2:41 ` Dan Williams
15 siblings, 0 replies; 17+ messages in thread
From: Dan Williams @ 2020-07-07 2:41 UTC (permalink / raw)
To: linux-nvdimm; +Cc: vishal.l.verma
mremap() fails to check for pmd_devmap() entries in some kernels leading to
a case where move_page_tables() interprets a pmd_devmap() entry as a page
table and not a leaf entry. Reproduce this failing condition across
device-dax and fileystem-dax.
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
test.h | 2 +
test/dax-dev.c | 4 ++
test/dax-pmd.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++--
test/device-dax.c | 7 ++++
4 files changed, 108 insertions(+), 4 deletions(-)
diff --git a/test.h b/test.h
index fa0c0cff9daf..3f6212e067fc 100644
--- a/test.h
+++ b/test.h
@@ -38,6 +38,8 @@ struct ndctl_ctx;
int test_parent_uuid(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx);
int test_multi_pmem(int loglevel, struct ndctl_test *test, struct ndctl_ctx *ctx);
int test_dax_directio(int dax_fd, unsigned long align, void *dax_addr, off_t offset);
+int test_dax_remap(struct ndctl_test *test, int dax_fd, unsigned long align, void *dax_addr,
+ off_t offset, bool fsdax);
#ifdef ENABLE_POISON
int test_dax_poison(struct ndctl_test *test, int dax_fd, unsigned long align,
void *dax_addr, off_t offset, bool fsdax);
diff --git a/test/dax-dev.c b/test/dax-dev.c
index 49ccaa334e31..ab6b35a67183 100644
--- a/test/dax-dev.c
+++ b/test/dax-dev.c
@@ -60,6 +60,10 @@ struct ndctl_namespace *ndctl_get_test_dev(struct ndctl_ctx *ctx)
if (!ndns)
goto out;
+ rc = ndctl_namespace_enable(ndns);
+ if (rc)
+ goto out;
+
mode = ndctl_namespace_get_mode(ndns);
if (mode >= 0 && mode != NDCTL_NS_MODE_MEMORY)
goto out;
diff --git a/test/dax-pmd.c b/test/dax-pmd.c
index 0c95b20707c2..df3219639a6d 100644
--- a/test/dax-pmd.c
+++ b/test/dax-pmd.c
@@ -27,6 +27,7 @@
#include <test.h>
#include <util/size.h>
#include <linux/fiemap.h>
+#include <linux/version.h>
#define NUM_EXTENTS 5
#define fail() fprintf(stderr, "%s: failed at: %d (%s)\n", \
@@ -35,6 +36,92 @@
__func__, __LINE__, i, strerror(errno))
#define TEST_FILE "test_dax_data"
+#define REGION_MEM_SIZE 4096*4
+#define REGION_PM_SIZE 4096*512
+#define REMAP_SIZE 4096
+
+static sigjmp_buf sj_env;
+
+static void sigbus(int sig, siginfo_t *siginfo, void *d)
+{
+ siglongjmp(sj_env, 1);
+}
+
+int test_dax_remap(struct ndctl_test *test, int dax_fd, unsigned long align, void *dax_addr,
+ off_t offset, bool fsdax)
+{
+ void *anon, *remap, *addr;
+ struct sigaction act;
+ int rc, val;
+
+ if ((fsdax || align == SZ_2M) && !ndctl_test_attempt(test, KERNEL_VERSION(5, 8, 0))) {
+ /* kernel's prior to 5.8 may crash on this test */
+ fprintf(stderr, "%s: SKIP mremap() test\n", __func__);
+ return 0;
+ }
+
+ anon = mmap(NULL, REGION_MEM_SIZE, PROT_READ|PROT_WRITE,
+ MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
+
+ addr = mmap(dax_addr, 2*align,
+ PROT_READ|PROT_WRITE, MAP_SHARED, dax_fd, offset);
+
+ fprintf(stderr, "%s: addr: %p size: %#lx\n", __func__, addr, 2*align);
+
+ if (addr == MAP_FAILED) {
+ rc = -errno;
+ faili(0);
+ return rc;
+ }
+
+ memset(anon, 'a', REGION_MEM_SIZE);
+ memset(addr, 'i', align*2);
+
+ remap = mremap(addr, REMAP_SIZE, REMAP_SIZE, MREMAP_MAYMOVE|MREMAP_FIXED, anon);
+
+ if (remap != anon) {
+ rc = -ENXIO;
+ perror("mremap");
+ faili(1);
+ return rc;
+ }
+
+ fprintf(stderr, "%s: addr: %p size: %#x\n", __func__, remap, REMAP_SIZE);
+
+ memset(&act, 0, sizeof(act));
+ act.sa_sigaction = sigbus;
+ act.sa_flags = SA_SIGINFO;
+ if (sigaction(SIGBUS, &act, 0)) {
+ perror("sigaction");
+ rc = EXIT_FAILURE;
+ goto out;
+ }
+
+ /* test fault after device-dax instance disabled */
+ if (sigsetjmp(sj_env, 1)) {
+ if (!fsdax && align > SZ_4K) {
+ fprintf(stderr, "got expected SIGBUS after mremap() of device-dax\n");
+ rc = 0;
+ } else {
+ fprintf(stderr, "unpexpected SIGBUS after mremap()\n");
+ rc = -EIO;
+ }
+ goto out;
+ }
+
+ *(int *) anon = 0xAA;
+ val = *(int *) anon;
+
+ if (val != 0xAA) {
+ faili(2);
+ return -ENXIO;
+ }
+
+ rc = 0;
+out:
+ return rc;
+}
+
int test_dax_directio(int dax_fd, unsigned long align, void *dax_addr, off_t offset)
{
int i, rc = -ENXIO;
@@ -254,15 +341,19 @@ static int test_pmd(struct ndctl_test *test, int fd)
pmd_addr = (char *) base + m_align;
pmd_off = ext->fe_logical + p_align;
+ rc = test_dax_remap(test, fd, HPAGE_SIZE, pmd_addr, pmd_off, fsdax);
+ if (rc)
+ goto err_test;
+
rc = test_dax_directio(fd, HPAGE_SIZE, pmd_addr, pmd_off);
if (rc)
- goto err_directio;
+ goto err_test;
rc = test_dax_poison(test, fd, HPAGE_SIZE, pmd_addr, pmd_off, fsdax);
- err_directio:
- err_extent:
- err_mmap:
+err_test:
+err_extent:
+err_mmap:
free(map);
return rc;
}
diff --git a/test/device-dax.c b/test/device-dax.c
index b19c1ed0b535..9de10682e34d 100644
--- a/test/device-dax.c
+++ b/test/device-dax.c
@@ -268,6 +268,13 @@ static int __test_device_dax(unsigned long align, int loglevel,
ndctl_namespace_get_devname(ndns));
goto out;
}
+
+ rc = test_dax_remap(test, fd, align, NULL, 0, devdax);
+ if (rc) {
+ fprintf(stderr, "%s: failed dax remap\n",
+ ndctl_namespace_get_devname(ndns));
+ goto out;
+ }
close(fd);
fprintf(stderr, "%s: test dax poison\n",
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
^ permalink raw reply related [flat|nested] 17+ messages in thread