[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