From 5fab572e6665dc01f8e3ca703401f837f6eb6ff5 Mon Sep 17 00:00:00 2001 From: jeff-lien-wdc Date: Tue, 5 Mar 2024 15:22:00 -0600 Subject: [PATCH 1/2] wdc: Fix timestamp displayed by vs-firmware-activate-history command The actual timestamp value is in bytes 0:5 of the 8 byte field. The code was using all 8 bytes so a change was made to mask off the top 2 bytes. The timestamp calculations were also simplified to be more maintainable. Signed-off-by: jeff-lien-wdc --- plugins/wdc/wdc-nvme.c | 198 +++++++++++++++++------------------------ plugins/wdc/wdc-nvme.h | 2 +- 2 files changed, 81 insertions(+), 119 deletions(-) diff --git a/plugins/wdc/wdc-nvme.c b/plugins/wdc/wdc-nvme.c index c8323c6c5a..678e2ef463 100644 --- a/plugins/wdc/wdc-nvme.c +++ b/plugins/wdc/wdc-nvme.c @@ -1383,6 +1383,11 @@ struct __packed wdc_fw_act_history_log_format_c2 { __u8 log_page_guid[WDC_C2_GUID_LENGTH]; }; +static __u8 ocp_C2_guid[WDC_C2_GUID_LENGTH] = { + 0x6D, 0x79, 0x9A, 0x76, 0xB4, 0xDA, 0xF6, 0xA3, + 0xE2, 0x4D, 0xB2, 0x8A, 0xAC, 0xF3, 0x1C, 0xD1 +}; + #define WDC_OCP_C4_GUID_LENGTH 16 #define WDC_DEV_CAP_LOG_BUF_LEN 4096 #define WDC_DEV_CAP_LOG_ID 0xC4 @@ -5531,8 +5536,7 @@ static void wdc_print_fw_act_history_log_normal(__u8 *data, int num_entries, if (data[0] == WDC_NVME_GET_FW_ACT_HISTORY_C2_LOG_ID) { printf(" Firmware Activate History Log\n"); if (cust_id == WDC_CUSTOMER_ID_0x1005 || - vendor_id == WDC_NVME_SNDK_VID || - wdc_is_sn861(device_id)) { + vendor_id == WDC_NVME_SNDK_VID) { printf(" Power on Hour Power Cycle Previous New\n"); printf(" Entry hh:mm:ss Count Firmware Firmware Slot Action Result\n"); printf(" ----- ----------------- ----------------- --------- --------- ----- ------ -------\n"); @@ -5573,48 +5577,33 @@ static void wdc_print_fw_act_history_log_normal(__u8 *data, int num_entries, memcpy(new_fw, null_fw, 8); printf("%5"PRIu16"", (uint16_t)le16_to_cpu(fw_act_history_entry->entry[entryIdx].fw_act_hist_entries)); + + uint64_t timestamp = (0x0000FFFFFFFFFFFF & + le64_to_cpu( + fw_act_history_entry->entry[entryIdx].timestamp)); + __u64 timestamp_sec = le64_to_cpu(timestamp) / 1000; if (cust_id == WDC_CUSTOMER_ID_0x1005) { printf(" "); memset((void *)time_str, 0, 9); - sprintf((char *)time_str, "%04d:%02d:%02d", (int)(le64_to_cpu(fw_act_history_entry->entry[entryIdx].timestamp)/3600), - (int)((le64_to_cpu(fw_act_history_entry->entry[entryIdx].timestamp%3600)/60)), - (int)(le64_to_cpu(fw_act_history_entry->entry[entryIdx].timestamp%60))); + sprintf((char *)time_str, "%04d:%02d:%02d", + (int)(le64_to_cpu(timestamp_sec)/3600), + (int)((le64_to_cpu(timestamp_sec%3600)/60)), + (int)(le64_to_cpu(timestamp_sec%60))); printf("%s", time_str); printf(" "); } else if (vendor_id == WDC_NVME_SNDK_VID) { printf(" "); - uint64_t timestamp = (0x0000FFFFFFFFFFFF & le64_to_cpu(fw_act_history_entry->entry[entryIdx].timestamp)); memset((void *)time_str, 0, 9); - sprintf((char *)time_str, "%04d:%02d:%02d", (int)((timestamp/(3600*1000))%24), (int)((timestamp/(1000*60))%60), - (int)((timestamp/1000)%60)); + sprintf((char *)time_str, "%04d:%02d:%02d", + (int)((le64_to_cpu(timestamp_sec)/3600)%24), + (int)((le64_to_cpu(timestamp_sec)/60)%60), + (int)(le64_to_cpu(timestamp_sec)%60)); printf("%s", time_str); printf(" "); - } else if (wdc_is_sn861(device_id)) { - printf(" "); - char timestamp[20]; - __u64 hour; - __u8 min; - __u8 sec; - __u64 timestamp_sec; - - timestamp_sec = - le64_to_cpu(fw_act_history_entry->entry[entryIdx].timestamp) - / 1000; - hour = timestamp_sec / 3600; - min = (timestamp_sec % 3600) / 60; - sec = timestamp_sec % 60; - - sprintf(timestamp, - "%"PRIu64":%02"PRIu8":%02"PRIu8, - (uint64_t)hour, min, sec); - printf("%-11s", timestamp); - printf(" "); } else { printf(" "); - uint64_t timestamp = (0x0000FFFFFFFFFFFF & le64_to_cpu(fw_act_history_entry->entry[entryIdx].timestamp)); - printf("%16"PRIu64"", timestamp); printf(" "); } @@ -5765,33 +5754,25 @@ static void wdc_print_fw_act_history_log_json(__u8 *data, int num_entries, json_object_add_value_int(root, "Entry", le16_to_cpu(fw_act_history_entry->entry[entryIdx].fw_act_hist_entries)); + uint64_t timestamp = (0x0000FFFFFFFFFFFF & + le64_to_cpu( + fw_act_history_entry->entry[entryIdx].timestamp)); + __u64 timestamp_sec = le64_to_cpu(timestamp)/1000; if (cust_id == WDC_CUSTOMER_ID_0x1005) { - sprintf((char *)time_str, "%04d:%02d:%02d", (int)(le64_to_cpu(fw_act_history_entry->entry[entryIdx].timestamp)/3600), - (int)((le64_to_cpu(fw_act_history_entry->entry[entryIdx].timestamp%3600)/60)), - (int)(le64_to_cpu(fw_act_history_entry->entry[entryIdx].timestamp%60))); + sprintf((char *)time_str, "%04d:%02d:%02d", + (int)(le64_to_cpu(timestamp_sec)/3600), + (int)((le64_to_cpu(timestamp_sec)%3600/60)), + (int)(le64_to_cpu(timestamp_sec)%60)); json_object_add_value_string(root, "Power on Hour", time_str); } else if (vendor_id == WDC_NVME_SNDK_VID) { - uint64_t timestamp = (0x0000FFFFFFFFFFFF & le64_to_cpu(fw_act_history_entry->entry[entryIdx].timestamp)); - - sprintf((char *)time_str, "%04d:%02d:%02d", (int)((timestamp/(3600*1000))%24), (int)((timestamp/(1000*60))%60), - (int)((timestamp/1000)%60)); + sprintf((char *)time_str, "%04d:%02d:%02d", + (int)((le64_to_cpu(timestamp_sec)/3600)%24), + (int)((le64_to_cpu(timestamp_sec)/60)%60), + (int)(le64_to_cpu(timestamp_sec)%60)); json_object_add_value_string(root, "Power on Hour", time_str); - } else if (wdc_is_sn861(device_id)) { - __u64 timestamp_sec = - le64_to_cpu(fw_act_history_entry->entry[entryIdx].timestamp) - / 1000; - - sprintf((char *)ext_time_str, - "%"PRIu64":%02"PRIu8":%02"PRIu8, - (uint64_t)(__u64)(timestamp_sec/3600), - (__u8)((timestamp_sec%3600)/60), - (__u8)(timestamp_sec%60)); - json_object_add_value_string(root, "Power on Hour", ext_time_str); } else { - uint64_t timestamp = (0x0000FFFFFFFFFFFF & le64_to_cpu(fw_act_history_entry->entry[entryIdx].timestamp)); - json_object_add_value_uint64(root, "Timestamp", timestamp); } @@ -9034,6 +9015,8 @@ static int wdc_get_fw_act_history_C2(nvme_root_t r, struct nvme_dev *dev, enum nvme_print_flags fmt; __u8 *data; int ret; + int i; + bool c2GuidMatch = false; if (!wdc_check_device(r, dev)) return -1; @@ -9062,29 +9045,46 @@ static int wdc_get_fw_act_history_C2(nvme_root_t r, struct nvme_dev *dev, nvme_show_status(ret); if (!ret) { - /* parse the data */ + /* Get the log page data and verify the GUID */ fw_act_history_log = (struct wdc_fw_act_history_log_format_c2 *)(data); - tot_entries = le32_to_cpu(fw_act_history_log->num_entries); - if (tot_entries > 0) { - /* get the FW customer id */ - if (!wdc_is_sn861(device_id)) { - cust_id = wdc_get_fw_cust_id(r, dev); - if (cust_id == WDC_INVALID_CUSTOMER_ID) { - fprintf(stderr, - "%s: ERROR: WDC: invalid customer id\n", - __func__); - ret = -1; - goto freeData; + for (i = 0; i < 16; i++) { + if (ocp_C2_guid[i] != fw_act_history_log->log_page_guid[i]) { + c2GuidMatch = false; + break; + } + } + + if (i == 16) + c2GuidMatch = true; + + if (c2GuidMatch) { + /* parse the data */ + tot_entries = le32_to_cpu(fw_act_history_log->num_entries); + + if (tot_entries > 0) { + /* get the FW customer id */ + if (!wdc_is_sn861(device_id)) { + cust_id = wdc_get_fw_cust_id(r, dev); + if (cust_id == WDC_INVALID_CUSTOMER_ID) { + fprintf(stderr, + "%s: ERROR: WDC: invalid customer id\n", + __func__); + ret = -1; + goto freeData; + } } + num_entries = (tot_entries < WDC_MAX_NUM_ACT_HIST_ENTRIES) ? + tot_entries : WDC_MAX_NUM_ACT_HIST_ENTRIES; + ret = wdc_print_fw_act_history_log(data, num_entries, + fmt, cust_id, vendor_id, device_id); + } else { + fprintf(stderr, "INFO: WDC: No entries found.\n"); + ret = 0; } - num_entries = (tot_entries < WDC_MAX_NUM_ACT_HIST_ENTRIES) ? tot_entries : - WDC_MAX_NUM_ACT_HIST_ENTRIES; - ret = wdc_print_fw_act_history_log(data, num_entries, - fmt, cust_id, vendor_id, device_id); - } else { - fprintf(stderr, "INFO: WDC: No FW Activate History entries found.\n"); - ret = 0; + } else { + fprintf(stderr, "ERROR: WDC: Invalid C2 log page GUID\n"); + ret = -1; } } else { fprintf(stderr, "ERROR: WDC: Unable to read FW Activate History Log Page data\n"); @@ -9103,7 +9103,7 @@ static int wdc_vs_fw_activate_history(int argc, char **argv, struct command *com __u64 capabilities = 0; struct nvme_dev *dev; nvme_root_t r; - int ret; + int ret = -1; struct config { char *output_format; @@ -9131,61 +9131,23 @@ static int wdc_vs_fw_activate_history(int argc, char **argv, struct command *com } if (capabilities & WDC_DRIVE_CAP_FW_ACTIVATE_HISTORY) { - int uuid_index = 0; - bool c0GuidMatch = false; - __u8 *data; - int i; - - /* - * check for the GUID in the 0xC0 log page to determine which log page to use to - * retrieve fw activate history data - */ - data = (__u8 *)malloc(sizeof(__u8) * WDC_NVME_SMART_CLOUD_ATTR_LEN); - if (!data) { - fprintf(stderr, "ERROR: WDC: malloc: %s\n", strerror(errno)); + __u32 cust_fw_id = 0; + /* get the FW customer id */ + cust_fw_id = wdc_get_fw_cust_id(r, dev); + if (cust_fw_id == WDC_INVALID_CUSTOMER_ID) { + fprintf(stderr, "%s: ERROR: WDC: invalid customer id\n", __func__); ret = -1; goto out; } - /* Get the 0xC0 log data */ - struct nvme_get_log_args args = { - .args_size = sizeof(args), - .fd = dev_fd(dev), - .lid = WDC_NVME_GET_SMART_CLOUD_ATTR_LOG_ID, - .nsid = 0xFFFFFFFF, - .lpo = 0, - .lsp = NVME_LOG_LSP_NONE, - .lsi = 0, - .rae = false, - .uuidx = uuid_index, - .csi = NVME_CSI_NVM, - .ot = false, - .len = WDC_NVME_SMART_CLOUD_ATTR_LEN, - .log = data, - .timeout = NVME_DEFAULT_IOCTL_TIMEOUT, - .result = NULL, - }; - ret = nvme_get_log(&args); - - if (!ret) { - /* Verify GUID matches */ - for (i = 0; i < 16; i++) { - if (scao_guid[i] != data[SCAO_LPG + i]) { - c0GuidMatch = false; - break; - } - } - - if (i == 16) - c0GuidMatch = true; - } - - free(data); - if (c0GuidMatch) + if ((cust_fw_id == WDC_CUSTOMER_ID_0x1004) || + (cust_fw_id == WDC_CUSTOMER_ID_0x1008) || + (cust_fw_id == WDC_CUSTOMER_ID_0x1005) || + (cust_fw_id == WDC_CUSTOMER_ID_0x1304)) ret = wdc_get_fw_act_history_C2(r, dev, cfg.output_format); else ret = wdc_get_fw_act_history(r, dev, cfg.output_format); - } else { + } else if (capabilities & WDC_DRIVE_CAP_FW_ACTIVATE_HISTORY_C2 { ret = wdc_get_fw_act_history_C2(r, dev, cfg.output_format); } diff --git a/plugins/wdc/wdc-nvme.h b/plugins/wdc/wdc-nvme.h index d1f87406bd..65d2de3a3a 100644 --- a/plugins/wdc/wdc-nvme.h +++ b/plugins/wdc/wdc-nvme.h @@ -5,7 +5,7 @@ #if !defined(WDC_NVME) || defined(CMD_HEADER_MULTI_READ) #define WDC_NVME -#define WDC_PLUGIN_VERSION "2.8.0" +#define WDC_PLUGIN_VERSION "2.8.1" #include "cmd.h" PLUGIN(NAME("wdc", "Western Digital vendor specific extensions", WDC_PLUGIN_VERSION), From e287f2553a3d83eb508638329a1080974dbf49ec Mon Sep 17 00:00:00 2001 From: jeff-lien-wdc Date: Tue, 12 Mar 2024 20:23:09 -0500 Subject: [PATCH 2/2] wdc: Review changes and build fixes Changes based on review comments. Fixed build errors building timestamp string. Fixed lines over the 100 char limit. Signed-off-by: jeff-lien-wdc --- plugins/wdc/wdc-nvme.c | 67 ++++++++++++++++++++---------------------- 1 file changed, 32 insertions(+), 35 deletions(-) diff --git a/plugins/wdc/wdc-nvme.c b/plugins/wdc/wdc-nvme.c index 678e2ef463..8ea98b8ff2 100644 --- a/plugins/wdc/wdc-nvme.c +++ b/plugins/wdc/wdc-nvme.c @@ -5527,11 +5527,13 @@ static void wdc_print_fw_act_history_log_normal(__u8 *data, int num_entries, char previous_fw[9]; char new_fw[9]; char commit_action_bin[8]; - char time_str[11]; + char time_str[100]; __u16 oldestEntryIdx = 0, entryIdx = 0; + uint64_t timestamp; + __u64 timestamp_sec; char *null_fw = "--------"; - memset((void *)time_str, 0, 11); + memset((void *)time_str, '\0', 100); if (data[0] == WDC_NVME_GET_FW_ACT_HISTORY_C2_LOG_ID) { printf(" Firmware Activate History Log\n"); @@ -5578,17 +5580,17 @@ static void wdc_print_fw_act_history_log_normal(__u8 *data, int num_entries, printf("%5"PRIu16"", (uint16_t)le16_to_cpu(fw_act_history_entry->entry[entryIdx].fw_act_hist_entries)); - uint64_t timestamp = (0x0000FFFFFFFFFFFF & + timestamp = (0x0000FFFFFFFFFFFF & le64_to_cpu( fw_act_history_entry->entry[entryIdx].timestamp)); - __u64 timestamp_sec = le64_to_cpu(timestamp) / 1000; + timestamp_sec = timestamp / 1000; if (cust_id == WDC_CUSTOMER_ID_0x1005) { printf(" "); memset((void *)time_str, 0, 9); - sprintf((char *)time_str, "%04d:%02d:%02d", - (int)(le64_to_cpu(timestamp_sec)/3600), - (int)((le64_to_cpu(timestamp_sec%3600)/60)), - (int)(le64_to_cpu(timestamp_sec%60))); + sprintf((char *)time_str, "%"PRIu32":%u:%u", + (__u32)(timestamp_sec/3600), + (__u8)(timestamp_sec%3600/60), + (__u8)(timestamp_sec%60)); printf("%s", time_str); printf(" "); @@ -5596,10 +5598,10 @@ static void wdc_print_fw_act_history_log_normal(__u8 *data, int num_entries, printf(" "); memset((void *)time_str, 0, 9); - sprintf((char *)time_str, "%04d:%02d:%02d", - (int)((le64_to_cpu(timestamp_sec)/3600)%24), - (int)((le64_to_cpu(timestamp_sec)/60)%60), - (int)(le64_to_cpu(timestamp_sec)%60)); + sprintf((char *)time_str, "%"PRIu32":%u:%u", + (__u32)((timestamp_sec/3600)%24), + (__u8)((timestamp_sec/60)%60), + (__u8)(timestamp_sec%60)); printf("%s", time_str); printf(" "); } else { @@ -5708,13 +5710,15 @@ static void wdc_print_fw_act_history_log_json(__u8 *data, int num_entries, char new_fw[9]; char commit_action_bin[8]; char fail_str[32]; - char time_str[11]; + char time_str[100]; char ext_time_str[20]; + uint64_t timestamp; + __u64 timestamp_sec; memset((void *)previous_fw, 0, 9); memset((void *)new_fw, 0, 9); memset((void *)commit_action_bin, 0, 8); - memset((void *)time_str, 0, 11); + memset((void *)time_str, '\0', 100); memset((void *)ext_time_str, 0, 20); memset((void *)fail_str, 0, 11); char *null_fw = "--------"; @@ -5754,23 +5758,23 @@ static void wdc_print_fw_act_history_log_json(__u8 *data, int num_entries, json_object_add_value_int(root, "Entry", le16_to_cpu(fw_act_history_entry->entry[entryIdx].fw_act_hist_entries)); - uint64_t timestamp = (0x0000FFFFFFFFFFFF & + timestamp = (0x0000FFFFFFFFFFFF & le64_to_cpu( fw_act_history_entry->entry[entryIdx].timestamp)); - __u64 timestamp_sec = le64_to_cpu(timestamp)/1000; + timestamp_sec = timestamp / 1000; if (cust_id == WDC_CUSTOMER_ID_0x1005) { - sprintf((char *)time_str, "%04d:%02d:%02d", - (int)(le64_to_cpu(timestamp_sec)/3600), - (int)((le64_to_cpu(timestamp_sec)%3600/60)), - (int)(le64_to_cpu(timestamp_sec)%60)); + sprintf((char *)time_str, "%"PRIu32":%u:%u", + (__u32)(timestamp_sec/3600), + (__u8)(timestamp_sec%3600/60), + (__u8)(timestamp_sec%60)); json_object_add_value_string(root, "Power on Hour", time_str); } else if (vendor_id == WDC_NVME_SNDK_VID) { - sprintf((char *)time_str, "%04d:%02d:%02d", - (int)((le64_to_cpu(timestamp_sec)/3600)%24), - (int)((le64_to_cpu(timestamp_sec)/60)%60), - (int)(le64_to_cpu(timestamp_sec)%60)); + sprintf((char *)time_str, "%"PRIu32":%u:%u", + (__u32)((timestamp_sec/3600)%24), + (__u8)((timestamp_sec/60)%60), + (__u8)(timestamp_sec%60)); json_object_add_value_string(root, "Power on Hour", time_str); } else { json_object_add_value_uint64(root, "Timestamp", timestamp); @@ -9015,7 +9019,6 @@ static int wdc_get_fw_act_history_C2(nvme_root_t r, struct nvme_dev *dev, enum nvme_print_flags fmt; __u8 *data; int ret; - int i; bool c2GuidMatch = false; if (!wdc_check_device(r, dev)) @@ -9048,15 +9051,9 @@ static int wdc_get_fw_act_history_C2(nvme_root_t r, struct nvme_dev *dev, /* Get the log page data and verify the GUID */ fw_act_history_log = (struct wdc_fw_act_history_log_format_c2 *)(data); - for (i = 0; i < 16; i++) { - if (ocp_C2_guid[i] != fw_act_history_log->log_page_guid[i]) { - c2GuidMatch = false; - break; - } - } - - if (i == 16) - c2GuidMatch = true; + c2GuidMatch = !memcmp(ocp_C2_guid, + fw_act_history_log->log_page_guid, + WDC_C2_GUID_LENGTH); if (c2GuidMatch) { /* parse the data */ @@ -9147,7 +9144,7 @@ static int wdc_vs_fw_activate_history(int argc, char **argv, struct command *com ret = wdc_get_fw_act_history_C2(r, dev, cfg.output_format); else ret = wdc_get_fw_act_history(r, dev, cfg.output_format); - } else if (capabilities & WDC_DRIVE_CAP_FW_ACTIVATE_HISTORY_C2 { + } else if (capabilities & WDC_DRIVE_CAP_FW_ACTIVATE_HISTORY_C2) { ret = wdc_get_fw_act_history_C2(r, dev, cfg.output_format); }