| Message ID | 1488335129-12363-17-git-send-email-andre.przywara@arm.com |
|---|---|
| State | Accepted |
| Commit | 7f0ef5a945a0a9cef773c2b05b157d2fef3ea7ad |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
Hi Andre, On 28 February 2017 at 19:25, Andre Przywara <andre.przywara@arm.com> wrote: > From: Siarhei Siamashka <siarhei.siamashka@gmail.com> > > 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. > > 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. > [Andre: split patch to remove OF_LIST hash feature] > > Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com> > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > arch/arm/include/asm/arch-sunxi/spl.h | 19 +++++++++++--- > include/configs/sunxi-common.h | 8 +++--- > scripts/Makefile.spl | 3 ++- > tools/mksunxiboot.c | 49 ++++++++++++++++++++++++++++++++--- > 4 files changed, 67 insertions(+), 12 deletions(-) Can this code move into mkimage as a new image type? This is what rockchip does. It feels like this tool should be subsumed. If that doesn't work, perhaps binman? - Simon
On 08/03/17 21:01, Simon Glass wrote: Hi Simon, many thanks for the review, finally found some time to look at this. I have finished the needed rework (including documentation) and will post something after some testing and some sleep ;-) > On 28 February 2017 at 19:25, Andre Przywara <andre.przywara@arm.com> wrote: >> From: Siarhei Siamashka <siarhei.siamashka@gmail.com> >> >> 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. >> >> 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. >> [Andre: split patch to remove OF_LIST hash feature] >> >> Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> --- >> arch/arm/include/asm/arch-sunxi/spl.h | 19 +++++++++++--- >> include/configs/sunxi-common.h | 8 +++--- >> scripts/Makefile.spl | 3 ++- >> tools/mksunxiboot.c | 49 ++++++++++++++++++++++++++++++++--- >> 4 files changed, 67 insertions(+), 12 deletions(-) > > Can this code move into mkimage as a new image type? This is what > rockchip does. It feels like this tool should be subsumed. If that > doesn't work, perhaps binman? Interesting, I wasn't aware that mkimage can do more than legacy and FIT. Indeed that sounds useful, especially as mkimage seems to be packaged separately and is available in some distros. So I hacked something up, but that needs some more love. I am tempted to drop (or split) this patch from this series for now, since this extension here and the move to mkimage could be treated separately from the SPL FIT code. Cheers, Andre.
Hi Andre, On 26 March 2017 at 19:18, André Przywara <andre.przywara@arm.com> wrote: > On 08/03/17 21:01, Simon Glass wrote: > > Hi Simon, > > many thanks for the review, finally found some time to look at this. > I have finished the needed rework (including documentation) and will > post something after some testing and some sleep ;-) > >> On 28 February 2017 at 19:25, Andre Przywara <andre.przywara@arm.com> wrote: >>> From: Siarhei Siamashka <siarhei.siamashka@gmail.com> >>> >>> 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. >>> >>> 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. >>> [Andre: split patch to remove OF_LIST hash feature] >>> >>> Signed-off-by: Siarhei Siamashka <siarhei.siamashka@gmail.com> >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >>> --- >>> arch/arm/include/asm/arch-sunxi/spl.h | 19 +++++++++++--- >>> include/configs/sunxi-common.h | 8 +++--- >>> scripts/Makefile.spl | 3 ++- >>> tools/mksunxiboot.c | 49 ++++++++++++++++++++++++++++++++--- >>> 4 files changed, 67 insertions(+), 12 deletions(-) >> >> Can this code move into mkimage as a new image type? This is what >> rockchip does. It feels like this tool should be subsumed. If that >> doesn't work, perhaps binman? > > Interesting, I wasn't aware that mkimage can do more than legacy and > FIT. Indeed that sounds useful, especially as mkimage seems to be > packaged separately and is available in some distros. > > So I hacked something up, but that needs some more love. > I am tempted to drop (or split) this patch from this series for now, > since this extension here and the move to mkimage could be treated > separately from the SPL FIT code. Yes that's fine, whatever works for you. I like using mkimage since it is supposed to me the tool we use to make binary files (with binman as a way of building the whole firmware image). Regards, SImon
diff --git a/arch/arm/include/asm/arch-sunxi/spl.h b/arch/arm/include/asm/arch-sunxi/spl.h index 831d0c0..9358397 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 #ifdef CONFIG_SUNXI_HIGH_SRAM #define SPL_ADDR 0x10000 @@ -58,11 +58,24 @@ struct boot_file_head { * compatible format, ready to be imported via "env import -t". */ uint32_t fel_uEnv_length; - uint32_t reserved1[2]; + /* + * 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 dt_name_offset; + uint32_t reserved1; uint32_t boot_media; /* written here by the boot ROM */ - uint32_t reserved2[5]; /* padding, align to 64 bytes */ + /* 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) */ }; +/* Compile time check to assure proper alignment of structure */ +typedef char boot_file_head_not_multiple_of_32[1 - 2*(sizeof(struct boot_file_head) % 32)]; + #define is_boot0_magic(addr) (memcmp((void *)addr, BOOT0_MAGIC, 8) == 0) #endif diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h index 06d03d4..e37bf6a 100644 --- a/include/configs/sunxi-common.h +++ b/include/configs/sunxi-common.h @@ -188,8 +188,8 @@ #endif #ifdef CONFIG_SUNXI_HIGH_SRAM -#define CONFIG_SPL_TEXT_BASE 0x10040 /* sram start+header */ -#define CONFIG_SPL_MAX_SIZE 0x7fc0 /* 32 KiB */ +#define CONFIG_SPL_TEXT_BASE 0x10060 /* sram start+header */ +#define CONFIG_SPL_MAX_SIZE 0x7fa0 /* 32 KiB */ #ifdef CONFIG_ARM64 /* end of SRAM A2 for now, as SRAM A1 is pretty tight for an ARM64 build */ #define LOW_LEVEL_SRAM_STACK 0x00054000 @@ -197,8 +197,8 @@ #define LOW_LEVEL_SRAM_STACK 0x00018000 #endif /* !CONFIG_ARM64 */ #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 */ #define LOW_LEVEL_SRAM_STACK 0x00008000 /* End of sram */ #endif diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index b52f996..fe827ce 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -285,7 +285,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) $< $@ $(obj)/sunxi-spl.bin: $(obj)/$(SPL_BIN).bin FORCE $(call if_changed,mksunxiboot) diff --git a/tools/mksunxiboot.c b/tools/mksunxiboot.c index 6bb649c..4ac2791 100644 --- a/tools/mksunxiboot.c +++ b/tools/mksunxiboot.c @@ -70,11 +70,40 @@ int main(int argc, char *argv[]) struct boot_img img; unsigned file_size; int count; + char *tool_name = argv[0]; + char *default_dt = NULL; - 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]); + /* 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"); + 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 { + fprintf(stderr, "ERROR: bad option '%s'\n", argv[1]); + return EXIT_FAILURE; + } + } + + 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"); return EXIT_FAILURE; } @@ -122,6 +151,18 @@ 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); + img.header.dt_name_offset = + 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"); + } + } + gen_check_sum(&img.header); count = write(fd_out, &img, le32_to_cpu(img.header.length));