Am Mittwoch, den 28.10.2020, 15:10 +0100 schrieb Stefan Roese: > Add a tool to update or insert an Octeon specific header into the U-Boot > image. This is needed e.g. for booting via SPI NOR, eMMC and NAND. > > While working on this, move enum cvmx_board_types_enum and > cvmx_board_type_to_string() to cvmx-bootloader.h and remove the > unreferenced (unsupported) board definition. > > Signed-off-by: Stefan Roese <s...@denx.de> > Cc: Aaron Williams <awilli...@marvell.com> > Cc: Chandrakala Chavva <ccha...@marvell.com> > Cc: Daniel Schwierzeck <daniel.schwierz...@gmail.com> > --- > v2: > - No change > - Azure build success here: > https://dev.azure.com/sr0718/u-boot/_build/results?buildId=59&view=results > > .../mach-octeon/include/mach/cvmx-bootinfo.h | 222 --------- > .../include/mach/cvmx-bootloader.h | 172 +++++++ > tools/Makefile | 3 + > tools/update_octeon_header.c | 450 ++++++++++++++++++ > 4 files changed, 625 insertions(+), 222 deletions(-) > create mode 100644 arch/mips/mach-octeon/include/mach/cvmx-bootloader.h > create mode 100644 tools/update_octeon_header.c > ...
> diff --git a/tools/Makefile b/tools/Makefile > index 51123fd929..253a6b9706 100644 > --- a/tools/Makefile > +++ b/tools/Makefile > @@ -206,6 +206,9 @@ hostprogs-y += proftool > hostprogs-$(CONFIG_STATIC_RELA) += relocate-rela > hostprogs-$(CONFIG_RISCV) += prelink-riscv > > +hostprogs-$(CONFIG_ARCH_OCTEON) += update_octeon_header > +update_octeon_header-objs := update_octeon_header.o lib/crc32.o entry in tools/.gitignore is missing > + > hostprogs-y += fdtgrep > fdtgrep-objs += $(LIBFDT_OBJS) common/fdt_region.o fdtgrep.o > > diff --git a/tools/update_octeon_header.c b/tools/update_octeon_header.c > new file mode 100644 > index 0000000000..964036216d > --- /dev/null > +++ b/tools/update_octeon_header.c > @@ -0,0 +1,450 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2020 Marvell International Ltd. > + */ > + > +#include <stdio.h> > +#include <stdint.h> > +#include <stddef.h> > +#include <sys/types.h> > +#include <sys/stat.h> > +#include <fcntl.h> > +#include <unistd.h> > +#include <stdlib.h> > +#include <string.h> > +#include <getopt.h> > +#include <arpa/inet.h> > + > +#include <u-boot/crc.h> > + > +#include "mkimage.h" > + > +#include "../arch/mips/mach-octeon/include/mach/cvmx-bootloader.h" > + > +void usage(void); this needs some code cleanup: - drop the forward declarations and reorder the functions when necessary - make all functions static - drop dead code - drop debug() macros convert the useful ones to printf - maybe use fprintf(STDERR, ...) for error messages there are also some bugs: - tool crashes with segfault when called without arguments - tool crashes when called with a long option but no value - the bugs above will also be detected by clang-tidy, suggestions for fixing below > + > +/* > + * Do some funky stuff here to get a compile time error if the size of the > + * bootloader_header structure has changed. > + */ > +#define compile_time_assert(cond, msg) char msg[(cond) ? 1 : -1] > +compile_time_assert(sizeof(struct bootloader_header) == 192, > + bootloader_header_size_changed); there is BUILD_BUG_ON_MSG() in linux/build_bug.h or compiletime_assert() in linux/compiler.h > + > +#define BUF_SIZE (16 * 1024) > +#define NAME_LEN 100 > + > +#ifndef WOFFSETOF > +/* word offset */ > +#define WOFFSETOF(type, elem) (offsetof(type, elem) / 4) > +#endif > + > +#define cvmx_cpu_to_be64(x) (x) > + > +static int stage2_flag; > +static int stage_1_5_flag; > +static int stage_1_flag; > + > +int lookup_board_type(char *board_name) > +{ > + int i; > + int board_type = 0; > + char *substr = NULL; > + > + /* Detect stage 2 bootloader boards */ > + if (strcasestr(board_name, "_stage2")) { > + debug("Stage 2 bootloader detected from substring %s in name > %s\n", > + "_stage2", board_name); > + stage2_flag = 1; > + } else { > + debug("Stage 2 bootloader NOT detected from name \"%s\"\n", > + board_name); > + } > + > + if (strcasestr(board_name, "_stage1")) { > + debug("Stage 1 bootloader detected from substring %s in name > %s\n", > + "_stage1", board_name); > + stage_1_flag = 1; > + } > + > + /* Generic is a special case since there are numerous sub-types */ > + if (!strncasecmp("generic", board_name, strlen("generic"))) > + return CVMX_BOARD_TYPE_GENERIC; > + > + /* > + * If we're an eMMC stage 2 bootloader, cut off the _emmc_stage2 > + * part of the name. > + */ > + substr = strcasestr(board_name, "_emmc_stage2"); > + if (substr && (substr[strlen("_emmc_stage2")] == '\0')) { > + /*return CVMX_BOARD_TYPE_GENERIC;*/ > + > + printf(" Converting board name %s to ", board_name); > + *substr = '\0'; > + printf("%s\n", board_name); > + } > + > + /* > + * If we're a NAND stage 2 bootloader, cut off the _nand_stage2 > + * part of the name. > + */ > + substr = strcasestr(board_name, "_nand_stage2"); > + if (substr && (substr[strlen("_nand_stage2")] == '\0')) { > + /*return CVMX_BOARD_TYPE_GENERIC;*/ > + > + printf(" Converting board name %s to ", board_name); > + *substr = '\0'; > + printf("%s\n", board_name); > + } > + > + /* > + * If we're a SPI stage 2 bootloader, cut off the _spi_stage2 > + * part of the name. > + */ > + substr = strcasestr(board_name, "_spi_stage2"); > + if (substr && (substr[strlen("_spi_stage2")] == '\0')) { > + printf(" Converting board name %s to ", board_name); > + *substr = '\0'; > + printf("%s\n", board_name); > + } > + > + for (i = CVMX_BOARD_TYPE_NULL; i < CVMX_BOARD_TYPE_MAX; i++) > + if (!strcasecmp(cvmx_board_type_to_string(i), board_name)) > + board_type = i; > + > + for (i = CVMX_BOARD_TYPE_CUST_DEFINED_MIN; > + i < CVMX_BOARD_TYPE_CUST_DEFINED_MAX; i++) > + if (!strncasecmp(cvmx_board_type_to_string(i), board_name, > + strlen(cvmx_board_type_to_string(i)))) > + board_type = i; > + > + for (i = CVMX_BOARD_TYPE_CUST_PRIVATE_MIN; > + i < CVMX_BOARD_TYPE_CUST_PRIVATE_MAX; i++) > + if (!strncasecmp(cvmx_board_type_to_string(i), board_name, > + strlen(cvmx_board_type_to_string(i)))) > + board_type = i; > + > + return board_type; > +} > + > +/* Getoptions variables must be global */ > +static int failsafe_flag; > +static int pciboot_flag; > +static int env_flag; > + > +int main(int argc, char *argv[]) > +{ > + int fd; > + uint8_t buf[BUF_SIZE]; > + uint32_t data_crc = 0; > + int len; > + int data_len = 0; > + struct bootloader_header header; > + char filename[NAME_LEN]; > + int i; > + int option_index = 0; /* getopt_long stores the option index here. */ > + char board_name[NAME_LEN] = { 0 }; > + char tmp_board_name[NAME_LEN] = { 0 }; > + int c; > + int board_type = 0; > + unsigned long long address = 0; > + ssize_t ret; > + const char *type_str = NULL; > + int hdr_size = sizeof(struct bootloader_header); the tool has two positional arguments, so you should bail out early. This will also fix the segfault if (argc < 3) { usage(); return -1; } > + > + debug("header size is: %d bytes\n", hdr_size); > + > + /* Parse command line options using getopt_long */ > + while (1) { > + static struct option long_options[] = { > + /* These options set a flag. */ > + {"failsafe", no_argument, &failsafe_flag, 1}, > + {"pciboot", no_argument, &pciboot_flag, 1}, > + {"nandstage2", no_argument, &stage2_flag, 1}, > + {"spistage2", no_argument, &stage2_flag, 1}, > + {"norstage2", no_argument, &stage2_flag, 1}, > + {"stage2", no_argument, &stage2_flag, 1}, > + {"stage1.5", no_argument, &stage_1_5_flag, 1}, > + {"stage1", no_argument, &stage_1_flag, 1}, > + {"environment", no_argument, &env_flag, 1}, > + /* These options don't set a flag. > + * We distinguish them by their indices. > + */ > + {"board", required_argument, 0, 0}, > + {"text_base", required_argument, 0, 0}, > + {0, 0, 0, 0} > + }; this should be moved out of main() and marked as static const > + > + c = getopt_long(argc, (char *const *)argv, "h", cast is not needed > + long_options, &option_index); > + > + /* Detect the end of the options. */ > + if (c == -1) > + break; > + > + switch (c) { > + /* All long options handled in case 0 */ > + case 0: > + /* If this option set a flag, do nothing else now. */ > + if (long_options[option_index].flag != 0) > + break; > + debug("option(l) %s", long_options[option_index].name); > + if (optarg) > + debug(" with arg %s", optarg); > + debug("\n"); should be if (!optarg) { usage(); return -1; } debug(" with arg %s\n", optarg); the code below depends on optarg and passing a long option without value is an error and should be notified to the user > + > + if (!strcmp(long_options[option_index].name, "board")) { > + if (strlen(optarg) >= NAME_LEN) { > + printf("strncpy() issue detected!"); > + exit(-1); > + } > + strncpy(board_name, optarg, NAME_LEN); > + > + debug("Using user supplied board name: %s\n", > + board_name); > + } else if (!strcmp(long_options[option_index].name, > + "text_base")) { > + address = strtoull(optarg, NULL, 0); > + debug("Address of image is: 0x%llx\n", > + (unsigned long long)address); > + if (!(address & 0xFFFFFFFFULL << 32)) { > + if (address & 1 << 31) { > + address |= 0xFFFFFFFFULL << 32; > + debug("Converting address to 64 > bit compatibility space: 0x%llx\n", > + address); > + } > + } > + } > + break; > + > + case 'h': > + case '?': > + /* getopt_long already printed an error message. */ > + usage(); > + return -1; > + > + default: > + abort(); > + } > + } > + > + if (optind < argc) { > + /* > + * We only support one argument - an optional bootloader > + * file name > + */ > + if (argc - optind > 2) { > + debug("non-option ARGV-elements: "); > + while (optind < argc) > + debug("%s ", argv[optind++]); > + debug("\n"); > + usage(); > + return -1; > + } > + } > + > + if (strlen(argv[optind]) >= NAME_LEN) { > + printf("strncpy() issue detected!"); > + exit(-1); > + } > + strncpy(filename, argv[optind], NAME_LEN); > + > + if (board_name[0] == '\0') { > + if (strlen(argv[optind + 1]) >= NAME_LEN) { > + printf("strncpy() issue detected!"); > + exit(-1); > + } > + strncpy(board_name, argv[optind + 1], NAME_LEN); > + } > + > + if (strlen(board_name) >= NAME_LEN) { > + printf("strncpy() issue detected!"); > + exit(-1); > + } > + strncpy(tmp_board_name, board_name, NAME_LEN); > + > + fd = open(filename, O_RDWR); > + if (fd < 0) { > + printf("Unable to open file: %s\n", filename); > + exit(-1); > + } > + > + if (failsafe_flag) > + debug("Setting failsafe flag\n"); > + > + if (strlen(board_name)) { > + int offset = 0; > + > + debug("Supplied board name of: %s\n", board_name); > + > + if (strstr(board_name, "failsafe")) { > + failsafe_flag = 1; > + debug("Setting failsafe flag based on board name\n"); > + } > + /* Skip leading octeon_ if present. */ > + if (!strncmp(board_name, "octeon_", 7)) > + offset = 7; > + > + /* > + * Check to see if 'failsafe' is in the name. If so, set the > + * failsafe flag. Also, ignore extra trailing characters on > + * passed parameter when comparing against board names. > + * We actually use the configuration name from u-boot, so it > + * may have some other variant names. Variants other than > + * failsafe _must_ be passed to this program explicitly > + */ > + > + board_type = lookup_board_type(board_name + offset); > + if (!board_type) { > + /* Retry with 'cust_' prefix to catch boards that are > + * in the customer section (such as nb5) > + */ > + sprintf(tmp_board_name, "cust_%s", board_name + offset); > + board_type = lookup_board_type(tmp_board_name); > + } > + > + /* reset to original value */ > + strncpy(tmp_board_name, board_name, NAME_LEN); > + if (!board_type) { > + /* > + * Retry with 'cust_private_' prefix to catch boards > + * that are in the customer private section > + */ > + sprintf(tmp_board_name, "cust_private_%s", > + board_name + offset); > + board_type = lookup_board_type(tmp_board_name); > + } > + > + if (!board_type) { > + printf("ERROR: unable to determine board type\n"); > + exit(-1); > + } > + debug("Board type is: %d: %s\n", board_type, > + cvmx_board_type_to_string(board_type)); > + } else { > + printf("Board name must be specified!\n"); > + exit(-1); > + } > + > + /* > + * Check to see if there is either an existing header, or that there > + * are zero valued bytes where we want to put the header > + */ > + len = read(fd, buf, BUF_SIZE); > + if (len > 0) { > + /* > + * Copy the header, as the first word (jump instruction, needs > + * to remain the same. > + */ > + memcpy(&header, buf, hdr_size); > + /* > + * Check to see if we have zero bytes (excluding first 4, which > + * are the jump instruction) > + */ > + for (i = 1; i < hdr_size / 4; i++) { > + if (((uint32_t *)buf)[i]) { > + printf("ERROR: non-zero word found %x in > location %d required for header, aborting\n", > + ((uint32_t *)buf)[i], i); > + exit(-1); > + } > + } > + debug("Zero bytes found in header location, adding header.\n"); > + > + } else { > + printf("Unable to read from file %s\n", filename); > + exit(-1); > + } > + > + /* Read data bytes and generate CRC */ > + lseek(fd, hdr_size, SEEK_SET); > + > + while ((len = read(fd, buf, BUF_SIZE)) > 0) { > + data_crc = crc32(data_crc, buf, len); > + data_len += len; > + } > + printf("CRC of data: 0x%x, length: %d\n", data_crc, data_len); > + > + /* Now create the new header */ > + header.magic = htonl(BOOTLOADER_HEADER_MAGIC); > + header.maj_rev = htons(BOOTLOADER_HEADER_CURRENT_MAJOR_REV); > + header.min_rev = htons(BOOTLOADER_HEADER_CURRENT_MINOR_REV); > + header.dlen = htonl(data_len); > + header.dcrc = htonl(data_crc); > + header.board_type = htons(board_type); > + header.address = cvmx_cpu_to_be64(address); cvmx_cpu_to_be64() is a no-op, is this intentional? If yes, this macro can be removed. Otherwise you can use cpu_to_be64(). All cpu_to_be*() functions are automatically available in tools/ directory. -- - Daniel