From 9349ef637d86c2ac07c46d223386c9b5d2e215ff Mon Sep 17 00:00:00 2001 From: Logan Gunthorpe Date: Mon, 31 Oct 2022 09:55:52 -0600 Subject: [PATCH 1/7] lib/platform/linux: Ensure entire switchtec_event_summary struct is set In normal cases, event_summary_copy() will set the entire switchtec_event_summary struct. However, if the library falls back to the smaller SWITCHTEC_IOCTL_EVENT_SUMMARY_LEGACY, then a size less than SWITCHTEC_MAX_PFF_CSR will be passed to the function and a portion of the function will be left unset. There are a number of places in the code where this function gets called pointing to an unitialized structure on the stack. So fix this issue by ensuring event_summary_copy() sets the entire structure. Resolves: #325 ("events command returning 'Invalid argument'") --- lib/platform/linux.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/platform/linux.c b/lib/platform/linux.c index 8fa42be8..c7f63627 100644 --- a/lib/platform/linux.c +++ b/lib/platform/linux.c @@ -829,6 +829,9 @@ static void event_summary_copy(struct switchtec_event_summary *dst, for (i = 0; i < SWITCHTEC_MAX_PFF_CSR && i < size; i++) dst->pff[i] = src->pff[i]; + + for (; i < SWITCHTEC_MAX_PFF_CSR; i++) + dst->pff[i] = 0; } #define EV(t, n)[SWITCHTEC_ ## t ## _EVT_ ## n] = \ From 76b6d4cc3591cd6a93b3e46bdfa4734c4c327484 Mon Sep 17 00:00:00 2001 From: Kelvin Cao Date: Tue, 8 Nov 2022 02:16:52 -0800 Subject: [PATCH 2/7] lib: Fix boot phase setup in set_gen_variant() After the setup of dev->gen and dev->var, continue with the setup of dev->boot_phase. --- lib/switchtec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/switchtec.c b/lib/switchtec.c index 854f5ec7..21886c35 100644 --- a/lib/switchtec.c +++ b/lib/switchtec.c @@ -192,7 +192,7 @@ static int set_gen_variant(struct switchtec_dev * dev) dev->gen = id->gen; dev->var = id->var; - return 0; + break; } id++; From 01f6dd86663fd0392498614d372e808b983d8816 Mon Sep 17 00:00:00 2001 From: Kelvin Cao Date: Tue, 8 Nov 2022 02:35:59 -0800 Subject: [PATCH 3/7] lib: Introduce function switchtec_get_device_id_bl2 Introduce a new function to get device id in BL2, as GAS regions is not populated in this boot phase. --- inc/switchtec/switchtec.h | 2 ++ lib/fw.c | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/inc/switchtec/switchtec.h b/inc/switchtec/switchtec.h index 0bcb317e..8cd1e94a 100644 --- a/inc/switchtec/switchtec.h +++ b/inc/switchtec/switchtec.h @@ -835,6 +835,8 @@ int switchtec_fw_read(struct switchtec_dev *dev, unsigned long addr, size_t len, void *buf); void switchtec_fw_perror(const char *s, int ret); int switchtec_fw_file_info(int fd, struct switchtec_fw_image_info *info); +int switchtec_get_device_id_bl2(struct switchtec_dev *dev, + unsigned short *device_id); int switchtec_fw_file_secure_version_newer(struct switchtec_dev *dev, int img_fd); const char *switchtec_fw_image_type(const struct switchtec_fw_image_info *info); diff --git a/lib/fw.c b/lib/fw.c index 2a4b7560..d7eab3d7 100644 --- a/lib/fw.c +++ b/lib/fw.c @@ -1097,6 +1097,27 @@ static int switchtec_fw_part_info(struct switchtec_dev *dev, int nr_info, return nr_info; } +int switchtec_get_device_id_bl2(struct switchtec_dev *dev, + unsigned short *device_id) +{ + int ret; + uint8_t subcmd = MRPC_PART_INFO_GET_ALL_INFO; + struct switchtec_flash_info_gen4 all_info; + + if (dev->gen != SWITCHTEC_GEN_UNKNOWN) + return -EINVAL; + + ret = switchtec_cmd(dev, MRPC_PART_INFO, &subcmd, + sizeof(subcmd), &all_info, + sizeof(all_info)); + if (ret) + return ret; + + *device_id = le16toh(all_info.device_id); + + return 0; +} + static long multicfg_subcmd(struct switchtec_dev *dev, uint32_t subcmd, uint8_t index) { From 8fa80c9eca23521bdc9bb46622f954846d53f4a4 Mon Sep 17 00:00:00 2001 From: Kelvin Cao Date: Tue, 8 Nov 2022 02:42:39 -0800 Subject: [PATCH 4/7] cli: Fix command info for variant and device ID The info command doesn't show variant and device ID properly in BL2, as current code get device ID from GAS which is not available in BL2. Fallback to MRPC if device ID is not returned. --- lib/switchtec.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/switchtec.c b/lib/switchtec.c index 21886c35..5254c7e7 100644 --- a/lib/switchtec.c +++ b/lib/switchtec.c @@ -186,6 +186,9 @@ static int set_gen_variant(struct switchtec_dev * dev) dev->gen = SWITCHTEC_GEN_UNKNOWN; dev->var = SWITCHTEC_VAR_UNKNOWN; dev->device_id = dev->ops->get_device_id(dev); + if (!dev->device_id) + switchtec_get_device_id_bl2(dev, + (unsigned short *)&dev->device_id); while (id->device_id) { if (id->device_id == dev->device_id) { From 114604bf60a4ec85ba796b2c61d2d4ee0b9ff5c7 Mon Sep 17 00:00:00 2001 From: Kelvin Cao Date: Mon, 14 Nov 2022 06:01:45 -0800 Subject: [PATCH 5/7] lib: Make helper phase_id_to_string global --- cli/mfg.c | 17 ++--------------- inc/switchtec/switchtec.h | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/cli/mfg.c b/cli/mfg.c index b46935ad..dfb1f078 100644 --- a/cli/mfg.c +++ b/cli/mfg.c @@ -64,20 +64,6 @@ static const struct argconfig_choice secure_state_choices[] = { {} }; -static const char* phase_id_to_string(enum switchtec_boot_phase phase_id) -{ - switch(phase_id) { - case SWITCHTEC_BOOT_PHASE_BL1: - return "BL1"; - case SWITCHTEC_BOOT_PHASE_BL2: - return "BL2"; - case SWITCHTEC_BOOT_PHASE_FW: - return "Main Firmware"; - default: - return "Unknown Phase"; - } -} - #define CMD_DESC_PING "ping device and get current boot phase" static int ping(int argc, char **argv) @@ -282,7 +268,8 @@ static int info(int argc, char **argv) } phase_id = switchtec_boot_phase(cfg.dev); - printf("Current Boot Phase: \t\t\t%s\n", phase_id_to_string(phase_id)); + printf("Current Boot Phase: \t\t\t%s\n", + switchtec_phase_id_str(phase_id)); ret = switchtec_sn_ver_get(cfg.dev, &sn_info); if (ret) { diff --git a/inc/switchtec/switchtec.h b/inc/switchtec/switchtec.h index 8cd1e94a..9fa07e2b 100644 --- a/inc/switchtec/switchtec.h +++ b/inc/switchtec/switchtec.h @@ -610,6 +610,24 @@ static inline const char *switchtec_variant_str(struct switchtec_dev *dev) return str; } +/** + * @brief Return the phase string for a phase id. + */ +static inline const char* switchtec_phase_id_str( + enum switchtec_boot_phase phase_id) +{ + switch(phase_id) { + case SWITCHTEC_BOOT_PHASE_BL1: + return "BL1"; + case SWITCHTEC_BOOT_PHASE_BL2: + return "BL2"; + case SWITCHTEC_BOOT_PHASE_FW: + return "Main Firmware"; + default: + return "Unknown Phase"; + } +} + /** @brief Number of GT/s capable for each PCI generation or \p link_rate */ static const float switchtec_gen_transfers[] = {0, 2.5, 5, 8, 16, 32}; /** @brief Number of GB/s capable for each PCI generation or \p link_rate */ From cdb77b2384ad6e7d571eede2a5ec2cd853921c0d Mon Sep 17 00:00:00 2001 From: Kelvin Cao Date: Mon, 14 Nov 2022 06:08:30 -0800 Subject: [PATCH 6/7] cli: Fix command info for version report in BL1 F/W version is not available in BL1. Print 'N/A' for F/W version instead of exiting with error. --- cli/main.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cli/main.c b/cli/main.c index db2eda7a..3f7be042 100644 --- a/cli/main.c +++ b/cli/main.c @@ -194,10 +194,8 @@ static int print_dev_info(struct switchtec_dev *dev) device_id = switchtec_device_id(dev); ret = switchtec_get_fw_version(dev, version, sizeof(version)); - if (ret < 0) { - switchtec_perror("dev info"); - return ret; - } + if (ret < 0) + strcpy(version, "N/A"); ret = switchtec_get_device_info(dev, NULL, NULL, &hw_rev); if (ret) { From 6ff72e7810e1a15e0c2873de4ca3ffe3489d7ab6 Mon Sep 17 00:00:00 2001 From: Kelvin Cao Date: Mon, 14 Nov 2022 06:11:33 -0800 Subject: [PATCH 7/7] cli: Report 'N/A' in command info for N/A information Report 'N/A' instead of '0x0000' or 'Unknown' for Device Id and Variant which are not available in BL1. Also show boot phase to make the reporting of 'N/A' more reasonable. --- cli/main.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/cli/main.c b/cli/main.c index 3f7be042..cfde38e8 100644 --- a/cli/main.c +++ b/cli/main.c @@ -189,6 +189,7 @@ static int print_dev_info(struct switchtec_dev *dev) int ret; int device_id; char version[64]; + enum switchtec_boot_phase phase; enum switchtec_rev hw_rev; device_id = switchtec_device_id(dev); @@ -197,17 +198,22 @@ static int print_dev_info(struct switchtec_dev *dev) if (ret < 0) strcpy(version, "N/A"); - ret = switchtec_get_device_info(dev, NULL, NULL, &hw_rev); + ret = switchtec_get_device_info(dev, &phase, NULL, &hw_rev); if (ret) { switchtec_perror("dev info"); return ret; } - printf("%s:\n", switchtec_name(dev)); + printf("%s (%s):\n", switchtec_name(dev), + switchtec_phase_id_str(phase)); printf(" Generation: %s\n", switchtec_gen_str(dev)); printf(" HW Revision: %s\n", switchtec_rev_str(hw_rev)); - printf(" Variant: %s\n", switchtec_variant_str(dev)); - printf(" Device ID: 0x%04x\n", device_id); + printf(" Variant: %s\n", + device_id ? switchtec_variant_str(dev) : "N/A"); + if (device_id) + printf(" Device ID: 0x%04x\n", device_id); + else + printf(" Device ID: %s\n", "N/A"); printf(" FW Version: %s\n", version); return 0;