[U-Boot] [RFC] sunxi: Store the device tree name in the SPL header
Bernhard Nortmann
bernhard.nortmann at web.de
Fri Jun 3 09:02:37 CEST 2016
Am 02.06.2016 um 12:32 schrieb Siarhei Siamashka:
> This patch updates the mksunxiboot tool to optionally add
> the default device tree name string to the SPL header. This
> information can be used by the firmware upgrade tools to
> protect users from harming themselves by trying to upgrade
> to an incompatible bootloader.
>
> The primary use case here is a non-removable bootable media
> (such as NAND, eMMC or SPI flash), which already may have
> a properly working, but a little bit outdated bootloader
> installed. For example, the user may download or build a
> new U-Boot image for "Cubieboard", and then attemept to
> install it on a "Cubieboard2" hardware by mistake as a
> replacement for the already existing bootloader. If this
> happens, the flash programming tool can identify this
> problem and warn the user.
>
> CONFIG_OF_LIST may provide more than one compatible device
> tree name when supporting multiple boards by a single
> defconfig. In such cases, the right device tree name is
> getting identified at runtime from the list of multiple
> options. The patch calculates a 32-bit hash value (crc32)
> of this list and stores it to the SPL header too, because
> this is better than nothing and still can be used to warn
> about potentially incompatible upgrades.
>
> The size of the SPL header is also increased from 64 bytes
> to 96 bytes to provide enough space for the device tree name
> string.
>
> Signed-off-by: Siarhei Siamashka <siarhei.siamashka at gmail.com>
> ---
> arch/arm/include/asm/arch-sunxi/spl.h | 26 +++++++++++--
> include/configs/sunxi-common.h | 12 +++---
> scripts/Makefile.spl | 3 +-
> tools/Makefile | 1 +
> tools/mksunxiboot.c | 71 +++++++++++++++++++++++++++++++++--
> 5 files changed, 98 insertions(+), 15 deletions(-)
Even with this, I guess there are still (slim) chances that users get
themselves into trouble - e.g. by having 'bricked' their devices after
they flashed an incompatible SPL/U-Boot, and then trying to repair the
damage via "FEL recovery". (The existing SPL would carry the wrong info.)
But it seems a useful approach, as long as the flasher only warns and
allows to continue - possibly requiring confirmation (enter "yes" to
proceed) or the use of some "--force" option. In general, I like the
idea.
> diff --git a/arch/arm/include/asm/arch-sunxi/spl.h b/arch/arm/include/asm/arch-sunxi/spl.h
> index a0f33b0..f370222 100644
> --- a/arch/arm/include/asm/arch-sunxi/spl.h
> +++ b/arch/arm/include/asm/arch-sunxi/spl.h
> @@ -10,7 +10,7 @@
>
> #define BOOT0_MAGIC "eGON.BT0"
> #define SPL_SIGNATURE "SPL" /* marks "sunxi" SPL header */
> -#define SPL_HEADER_VERSION 1
> +#define SPL_HEADER_VERSION 2
>
> #if defined(CONFIG_MACH_SUN9I) || defined(CONFIG_MACH_SUN50I)
> #define SPL_ADDR 0x10000
> @@ -49,9 +49,27 @@ struct boot_file_head {
> uint8_t spl_signature[4];
> };
> uint32_t fel_script_address;
> - uint32_t reserved1[3];
> - uint32_t boot_media; /* written here by the boot ROM */
> - uint32_t reserved2[5]; /* padding, align to 64 bytes */
> + uint32_t reserved1[1];
There's no "reserved2" any more, and no need to keep that an array,
so just "uint32_t reserved;"?
> + /*
> + * Offset of an ASCIIZ string (relative to the SPL header), which
> + * contains the default device tree name (CONFIG_DEFAULT_DEVICE_TREE).
> + * This is optional and may be set to NULL. Is intended to be used
> + * by flash programming tools for providing nice informative messages
> + * to the users.
> + */
> + uint32_t offset_of_the_default_device_tree_name;
> + /*
> + * A 32-bit hash value, calculated from the list of compatible device
> + * tree names (CONFIG_OF_LIST or CONFIG_DEFAULT_DEVICE_TREE). Is
> + * intended to be used by flash programming tools for safeguearding
> + * against upgrading to an incompatible firmware.
> + */
> + uint32_t crc32_of_the_compatible_device_tree_list;
> + /* The 'boot_media' code is written here by the boot ROM */
> + uint32_t boot_media;
> + /* A padding area (may be used for storing text strings) */
> + uint32_t string_pool[13];
> + /* The header must be a multiple of 32 bytes (for VBAR alignment) */
I see no particular reason why this should be uint32_t[13] instead of
char[52], with a suitable (multiple of 4) array size to ensure proper
header alignment. You're introducing a check / safeguard for the header
size anyway, and char[] saves us from having to do a typecast later on.
> };
>
> #define is_boot0_magic(addr) (memcmp((void *)addr, BOOT0_MAGIC, 8) == 0)
> diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
> index b33cfb8..e184d0c 100644
> --- a/include/configs/sunxi-common.h
> +++ b/include/configs/sunxi-common.h
> @@ -189,14 +189,14 @@
> #define CONFIG_SPL_BOARD_LOAD_IMAGE
>
> #if defined(CONFIG_MACH_SUN9I)
> -#define CONFIG_SPL_TEXT_BASE 0x10040 /* sram start+header */
> -#define CONFIG_SPL_MAX_SIZE 0x5fc0 /* ? KiB on sun9i */
> +#define CONFIG_SPL_TEXT_BASE 0x10060 /* sram start+header */
> +#define CONFIG_SPL_MAX_SIZE 0x5fa0 /* ? KiB on sun9i */
> #elif defined(CONFIG_MACH_SUN50I)
> -#define CONFIG_SPL_TEXT_BASE 0x10040 /* sram start+header */
> -#define CONFIG_SPL_MAX_SIZE 0x7fc0 /* 32 KiB on sun50i */
> +#define CONFIG_SPL_TEXT_BASE 0x10060 /* sram start+header */
> +#define CONFIG_SPL_MAX_SIZE 0x7fa0 /* 32 KiB on sun50i */
> #else
> -#define CONFIG_SPL_TEXT_BASE 0x40 /* sram start+header */
> -#define CONFIG_SPL_MAX_SIZE 0x5fc0 /* 24KB on sun4i/sun7i */
> +#define CONFIG_SPL_TEXT_BASE 0x60 /* sram start+header */
> +#define CONFIG_SPL_MAX_SIZE 0x5fa0 /* 24KB on sun4i/sun7i */
> #endif
>
> #define CONFIG_SPL_LIBDISK_SUPPORT
> diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
> index 6d2017d..54d8a2e 100644
> --- a/scripts/Makefile.spl
> +++ b/scripts/Makefile.spl
> @@ -240,7 +240,8 @@ $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin FORCE
> $(call if_changed,mkimage)
>
> quiet_cmd_mksunxiboot = MKSUNXI $@
> -cmd_mksunxiboot = $(objtree)/tools/mksunxiboot $< $@
> +cmd_mksunxiboot = $(objtree)/tools/mksunxiboot --default-dt "$(CONFIG_DEFAULT_DEVICE_TREE)" \
> + --compatible-dt-list "$(CONFIG_OF_LIST)" $< $@
> $(obj)/sunxi-spl.bin: $(obj)/$(SPL_BIN).bin FORCE
> $(call if_changed,mksunxiboot)
>
> diff --git a/tools/Makefile b/tools/Makefile
> index 63355aa..e6c0481 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -155,6 +155,7 @@ hostprogs-$(CONFIG_MX28) += mxsboot
> HOSTCFLAGS_mxsboot.o := -pedantic
>
> hostprogs-$(CONFIG_SUNXI) += mksunxiboot
> +mksunxiboot-objs := mksunxiboot.o lib/crc32.o
>
> hostprogs-$(CONFIG_NETCONSOLE) += ncb
> hostprogs-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1
> diff --git a/tools/mksunxiboot.c b/tools/mksunxiboot.c
> index 9c1c5b7..b45acec 100644
> --- a/tools/mksunxiboot.c
> +++ b/tools/mksunxiboot.c
> @@ -17,6 +17,8 @@
> #include <sys/stat.h>
> #include "asm/arch/spl.h"
>
> +#include <u-boot/crc.h>
> +
> #define STAMP_VALUE 0x5F0A6C39
>
> /* check sum functon from sun4i boot code */
> @@ -70,11 +72,55 @@ int main(int argc, char *argv[])
> struct boot_img img;
> unsigned file_size;
> int count;
> + char *tool_name = argv[0];
> + char *default_dt = NULL;
> + char *compatible_dt_list = NULL;
> +
> + /* a sanity check */
> + if ((sizeof(img.header) % 32) != 0) {
> + fprintf(stderr, "ERROR: the SPL header must be a multiple ");
> + fprintf(stderr, "of 32 bytes.\n");
Nitpick / coding style: You're using multiple fprintf(stderr, ...)
where a simple string concatenation would do. Same applies for some
printf() further down. Not a real concern though.
> + return EXIT_FAILURE;
> + }
> +
> + /* process optional command line switches */
> + while (argc >= 2 && argv[1][0] == '-') {
> + if (strcmp(argv[1], "--default-dt") == 0) {
> + if (argc >= 3) {
> + default_dt = argv[2];
> + argv += 2;
> + argc -= 2;
> + continue;
> + }
> + fprintf(stderr, "ERROR: no --default-dt arg\n");
> + return EXIT_FAILURE;
> + } else if (strcmp(argv[1], "--compatible-dt-list") == 0) {
> + if (argc >= 3) {
> + compatible_dt_list = argv[2];
> + argv += 2;
> + argc -= 2;
> + continue;
> + }
> + fprintf(stderr, "ERROR: no --compatible-dt-list arg\n");
> + return EXIT_FAILURE;
> + } else {
> + fprintf(stderr, "ERROR: bad option '%s'\n", argv[1]);
> + return EXIT_FAILURE;
> + }
> + }
>
> - if (argc < 2) {
> - printf("\tThis program makes an input bin file to sun4i " \
> - "bootable image.\n" \
> - "\tUsage: %s input_file out_putfile\n", argv[0]);
> + if (!compatible_dt_list || strlen(compatible_dt_list) == 0)
> + compatible_dt_list = default_dt;
> +
> + if (argc < 3) {
> + printf("This program converts an input binary file to a sunxi bootable image.\n");
> + printf("\nUsage: %s [options] input_file output_file\n",
> + tool_name);
> + printf("Where [options] may be:\n");
> + printf(" --default-dt arg - 'arg' is the default device tree name\n");
> + printf(" (CONFIG_DEFAULT_DEVICE_TREE).\n");
> + printf(" --compatible-dt-list arg - 'arg' is the space separated list of the\n");
> + printf(" compatible device tree names (CONFIG_OF_LIST).\n");
> return EXIT_FAILURE;
> }
>
> @@ -122,6 +168,23 @@ int main(int argc, char *argv[])
> memcpy(img.header.spl_signature, SPL_SIGNATURE, 3); /* "sunxi" marker */
> img.header.spl_signature[3] = SPL_HEADER_VERSION;
>
> + if (default_dt) {
> + if (strlen(default_dt) + 1 <= sizeof(img.header.string_pool)) {
> + strcpy((char *)img.header.string_pool, default_dt);
(The cast would be obsolete with a "char stringpool[];" type.)
> + img.header.offset_of_the_default_device_tree_name =
> + cpu_to_le32(offsetof(struct boot_file_head,
> + string_pool));
> + } else {
> + printf("WARNING: The SPL header is too small\n");
> + printf(" and has no space to store the dt name.\n");
> + }
> + }
> + if (compatible_dt_list) {
> + img.header.crc32_of_the_compatible_device_tree_list =
> + cpu_to_le32(crc32(0, (uint8_t *)compatible_dt_list,
> + strlen(compatible_dt_list)));
> + }
> +
> gen_check_sum(&img.header);
>
> count = write(fd_out, &img, le32_to_cpu(img.header.length));
Regards, B. Nortmann
More information about the U-Boot
mailing list