Re: [PATCH v2] ndctl: add option to list firmware information for a DIMM
On 02/13/2018 12:03 PM, Jeff Moyer wrote:
> Dave Jiang writes:
>
>> Adding firmware output of firmware information when ndctl list -D -F is used.
>> Components displayed are current firmware version, updated firmware version,
>> and if a coldboot is required (firmware updated).
>
> So now I don't get any output if I run:
>
> ndctl list -DF
>
> on an NVDIMM-N system. I would expect the list to at least work, but no
> firmware information to be provided. Is that not what you would expect?
Yeah I know what the issue is.
>
> Cheers,
> Jeff
>
>>
>> Signed-off-by: Dave Jiang
>> ---
>>
>> v2:
>> - Added copyright
>> - Added support for human readable option (hex) for versions
>> - Removed check against CMD_CALL as it's not useful
>>
>> Documentation/ndctl/ndctl-list.txt | 13 +
>> ndctl/Makefile.am |1
>> ndctl/list.c | 15 ++
>> ndctl/util/json-firmware.c | 90
>>
>> util/json.h|2 +
>> 5 files changed, 121 insertions(+)
>> create mode 100644 ndctl/util/json-firmware.c
>>
>> diff --git a/Documentation/ndctl/ndctl-list.txt
>> b/Documentation/ndctl/ndctl-list.txt
>> index fc07a71..85b87d4 100644
>> --- a/Documentation/ndctl/ndctl-list.txt
>> +++ b/Documentation/ndctl/ndctl-list.txt
>> @@ -110,6 +110,19 @@ include::xable-region-options.txt[]
>>}
>> }
>>
>> +-F::
>> +--firmware::
>> +Include dimm firmware info in the listing. For example:
>> +[verse]
>> +{
>> + "dev":"nmem0",
>> + "firmware":{
>> + "current_version":0,
>> + "next_version":1,
>> + "coldboot_required":true
>> + }
>> +}
>> +
>> -X::
>> --device-dax::
>> Include device-dax ("daxregion") details when a namespace is in
>> diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
>> index 2054c1a..e0db97b 100644
>> --- a/ndctl/Makefile.am
>> +++ b/ndctl/Makefile.am
>> @@ -13,6 +13,7 @@ ndctl_SOURCES = ndctl.c \
>> test.c \
>> ../util/json.c \
>> util/json-smart.c \
>> +util/json-firmware.c \
>> inject-error.c \
>> update.c \
>> inject-smart.c
>> diff --git a/ndctl/list.c b/ndctl/list.c
>> index 37a224a..2aaa248 100644
>> --- a/ndctl/list.c
>> +++ b/ndctl/list.c
>> @@ -35,6 +35,7 @@ static struct {
>> bool dax;
>> bool media_errors;
>> bool human;
>> +bool firmware;
>> } list;
>>
>> static unsigned long listopts_to_flags(void)
>> @@ -277,6 +278,7 @@ int cmd_list(int argc, const char **argv, void *ctx)
>> "filter by region-type"),
>> OPT_BOOLEAN('B', "buses", &list.buses, "include bus info"),
>> OPT_BOOLEAN('D', "dimms", &list.dimms, "include dimm info"),
>> +OPT_BOOLEAN('F', "firmware", &list.firmware, "include firmware
>> info"),
>> OPT_BOOLEAN('H', "health", &list.health, "include dimm health"),
>> OPT_BOOLEAN('R', "regions", &list.regions,
>> "include region info"),
>> @@ -420,6 +422,19 @@ int cmd_list(int argc, const char **argv, void *ctx)
>> }
>> }
>>
>> +if (list.firmware) {
>> +struct json_object *jfirmware;
>> +
>> +jfirmware = util_dimm_firmware_to_json(dimm,
>> +listopts_to_flags());
>> +if (jfirmware)
>> +json_object_object_add(jdimm,
>> +"firmware",
>> +jfirmware);
>> +else
>> +continue;
>> +}
>> +
>> /*
>> * 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
>> new file mode 100644
>> index 000..a6c33d8
>> --- /dev/null
>> +++ b/ndctl/util/json-firmware.c
>> @@ -0,0 +1,90 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +
>> +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, updated;
>> +bool reboot = false;
>> +
>> +if (!jfirmware)
>> +return NULL;
>> +
>> +cmd = ndctl_dimm_cmd_new_fw_get_info(dimm);
>> +if (!cmd)
>> +goto err;
>> +
>> +rc = ndctl_cmd_submit(cmd);
>> +if (rc || ndctl_cmd_fw_xlat_firmware_status(cmd) !=
Re: [PATCH v2] ndctl: add option to list firmware information for a DIMM
On Tue, Feb 13, 2018 at 11:03 AM, Jeff Moyer wrote: > Dave Jiang writes: > >> Adding firmware output of firmware information when ndctl list -D -F is used. >> Components displayed are current firmware version, updated firmware version, >> and if a coldboot is required (firmware updated). > > So now I don't get any output if I run: > > ndctl list -DF > > on an NVDIMM-N system. I would expect the list to at least work, but no > firmware information to be provided. Is that not what you would expect? Right, "ndctl list -DF" should be equivalent to "ndctl list -D" on DIMMs where ndctl does not know how to retrieve the firmware details. ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH v2] ndctl: add option to list firmware information for a DIMM
Dave Jiang writes:
> Adding firmware output of firmware information when ndctl list -D -F is used.
> Components displayed are current firmware version, updated firmware version,
> and if a coldboot is required (firmware updated).
So now I don't get any output if I run:
ndctl list -DF
on an NVDIMM-N system. I would expect the list to at least work, but no
firmware information to be provided. Is that not what you would expect?
Cheers,
Jeff
>
> Signed-off-by: Dave Jiang
> ---
>
> v2:
> - Added copyright
> - Added support for human readable option (hex) for versions
> - Removed check against CMD_CALL as it's not useful
>
> Documentation/ndctl/ndctl-list.txt | 13 +
> ndctl/Makefile.am |1
> ndctl/list.c | 15 ++
> ndctl/util/json-firmware.c | 90
>
> util/json.h|2 +
> 5 files changed, 121 insertions(+)
> create mode 100644 ndctl/util/json-firmware.c
>
> diff --git a/Documentation/ndctl/ndctl-list.txt
> b/Documentation/ndctl/ndctl-list.txt
> index fc07a71..85b87d4 100644
> --- a/Documentation/ndctl/ndctl-list.txt
> +++ b/Documentation/ndctl/ndctl-list.txt
> @@ -110,6 +110,19 @@ include::xable-region-options.txt[]
>}
> }
>
> +-F::
> +--firmware::
> + Include dimm firmware info in the listing. For example:
> +[verse]
> +{
> + "dev":"nmem0",
> + "firmware":{
> + "current_version":0,
> + "next_version":1,
> + "coldboot_required":true
> + }
> +}
> +
> -X::
> --device-dax::
> Include device-dax ("daxregion") details when a namespace is in
> diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
> index 2054c1a..e0db97b 100644
> --- a/ndctl/Makefile.am
> +++ b/ndctl/Makefile.am
> @@ -13,6 +13,7 @@ ndctl_SOURCES = ndctl.c \
> test.c \
> ../util/json.c \
> util/json-smart.c \
> + util/json-firmware.c \
> inject-error.c \
> update.c \
> inject-smart.c
> diff --git a/ndctl/list.c b/ndctl/list.c
> index 37a224a..2aaa248 100644
> --- a/ndctl/list.c
> +++ b/ndctl/list.c
> @@ -35,6 +35,7 @@ static struct {
> bool dax;
> bool media_errors;
> bool human;
> + bool firmware;
> } list;
>
> static unsigned long listopts_to_flags(void)
> @@ -277,6 +278,7 @@ int cmd_list(int argc, const char **argv, void *ctx)
> "filter by region-type"),
> OPT_BOOLEAN('B', "buses", &list.buses, "include bus info"),
> OPT_BOOLEAN('D', "dimms", &list.dimms, "include dimm info"),
> + OPT_BOOLEAN('F', "firmware", &list.firmware, "include firmware
> info"),
> OPT_BOOLEAN('H', "health", &list.health, "include dimm health"),
> OPT_BOOLEAN('R', "regions", &list.regions,
> "include region info"),
> @@ -420,6 +422,19 @@ int cmd_list(int argc, const char **argv, void *ctx)
> }
> }
>
> + if (list.firmware) {
> + struct json_object *jfirmware;
> +
> + jfirmware = util_dimm_firmware_to_json(dimm,
> + listopts_to_flags());
> + if (jfirmware)
> + json_object_object_add(jdimm,
> + "firmware",
> + jfirmware);
> + else
> + continue;
> + }
> +
> /*
>* 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
> new file mode 100644
> index 000..a6c33d8
> --- /dev/null
> +++ b/ndctl/util/json-firmware.c
> @@ -0,0 +1,90 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +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, updated;
> + bool reboot = false;
> +
> + if (!jfirmware)
> + return NULL;
> +
> + cmd = ndctl_dimm_cmd_new_fw_get_info(dimm);
> + if (!cmd)
> + goto err;
> +
> + rc = ndctl_cmd_submit(cmd);
> + if (rc || ndctl_cmd_fw_xlat_firmware_status(cmd) != FW_SUCCESS) {
> + jobj = json_object_new_string("unknown");
> + if (jobj)
> + json_object_object_add(jfirmware, "current_version",
> +
[PATCH v2] ndctl: add option to list firmware information for a DIMM
Adding firmware output of firmware information when ndctl list -D -F is used.
Components displayed are current firmware version, updated firmware version,
and if a coldboot is required (firmware updated).
Signed-off-by: Dave Jiang
---
v2:
- Added copyright
- Added support for human readable option (hex) for versions
- Removed check against CMD_CALL as it's not useful
Documentation/ndctl/ndctl-list.txt | 13 +
ndctl/Makefile.am |1
ndctl/list.c | 15 ++
ndctl/util/json-firmware.c | 90
util/json.h|2 +
5 files changed, 121 insertions(+)
create mode 100644 ndctl/util/json-firmware.c
diff --git a/Documentation/ndctl/ndctl-list.txt
b/Documentation/ndctl/ndctl-list.txt
index fc07a71..85b87d4 100644
--- a/Documentation/ndctl/ndctl-list.txt
+++ b/Documentation/ndctl/ndctl-list.txt
@@ -110,6 +110,19 @@ include::xable-region-options.txt[]
}
}
+-F::
+--firmware::
+ Include dimm firmware info in the listing. For example:
+[verse]
+{
+ "dev":"nmem0",
+ "firmware":{
+ "current_version":0,
+ "next_version":1,
+ "coldboot_required":true
+ }
+}
+
-X::
--device-dax::
Include device-dax ("daxregion") details when a namespace is in
diff --git a/ndctl/Makefile.am b/ndctl/Makefile.am
index 2054c1a..e0db97b 100644
--- a/ndctl/Makefile.am
+++ b/ndctl/Makefile.am
@@ -13,6 +13,7 @@ ndctl_SOURCES = ndctl.c \
test.c \
../util/json.c \
util/json-smart.c \
+ util/json-firmware.c \
inject-error.c \
update.c \
inject-smart.c
diff --git a/ndctl/list.c b/ndctl/list.c
index 37a224a..2aaa248 100644
--- a/ndctl/list.c
+++ b/ndctl/list.c
@@ -35,6 +35,7 @@ static struct {
bool dax;
bool media_errors;
bool human;
+ bool firmware;
} list;
static unsigned long listopts_to_flags(void)
@@ -277,6 +278,7 @@ int cmd_list(int argc, const char **argv, void *ctx)
"filter by region-type"),
OPT_BOOLEAN('B', "buses", &list.buses, "include bus info"),
OPT_BOOLEAN('D', "dimms", &list.dimms, "include dimm info"),
+ OPT_BOOLEAN('F', "firmware", &list.firmware, "include firmware
info"),
OPT_BOOLEAN('H', "health", &list.health, "include dimm health"),
OPT_BOOLEAN('R', "regions", &list.regions,
"include region info"),
@@ -420,6 +422,19 @@ int cmd_list(int argc, const char **argv, void *ctx)
}
}
+ if (list.firmware) {
+ struct json_object *jfirmware;
+
+ jfirmware = util_dimm_firmware_to_json(dimm,
+ listopts_to_flags());
+ if (jfirmware)
+ json_object_object_add(jdimm,
+ "firmware",
+ jfirmware);
+ else
+ continue;
+ }
+
/*
* 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
new file mode 100644
index 000..a6c33d8
--- /dev/null
+++ b/ndctl/util/json-firmware.c
@@ -0,0 +1,90 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright(c) 2018 Intel Corporation. All rights reserved. */
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+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, updated;
+ bool reboot = false;
+
+ if (!jfirmware)
+ return NULL;
+
+ cmd = ndctl_dimm_cmd_new_fw_get_info(dimm);
+ if (!cmd)
+ goto err;
+
+ rc = ndctl_cmd_submit(cmd);
+ if (rc || ndctl_cmd_fw_xlat_firmware_status(cmd) != FW_SUCCESS) {
+ jobj = json_object_new_string("unknown");
+ 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 = json_object_new_string("unknown");
+ if (jobj)
+ json_object_object_add(jfirmware, "current_version",
+ jobj);
+ goto out;
+ }
