[PATCH v2 0/2] tools/sunxi: Use mkimage for SPL generation

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH v2 0/2] tools/sunxi: Use mkimage for SPL generation

André Przywara
Hi,

digging this out after two years [1] ;-)
I fixed the minor comments from v1 and rebased it, then repeated
the tests. Apart from two trivial fixes there were no changes, so
I kept the R-b: tags. For a changelog see below.

==================

So far creating a bootable SPL image for Allwinner based boards uses
the mksunxiboot tool. Most other platforms seemed to have integrated this
kind of functionality into the common mkimage tool.

Since there is nothing special about the Allwinner image in this respect,
just add support for the so-called "eGON" image type into mkimage. If there
was a particular reason this hasn't been done before, please let me know.

This will eventually allow us to remove mksunxiboot, but I leave it around
for now in case of regressions and since some people depend on it from
external projects.

Patch 1/2 adds the actual support to mkimage, patch 2/2 then switches
the Makefile to use mkimage instead of mksunxiboot.

I tested all 152 Allwinner boards by building each
u-boot-sunxi-with-spl.bin and comparing them against the version created
using mksunxiboot (using SOURCE_DATE_EPOCH and .scmversion to create
reproducible builds, and by reverting just patch 2/2).
All files before and after were identical.

Cheers,
Andre

[1] https://lists.denx.de/pipermail/u-boot/2018-December/352798.html

Changelog v1 .. v2:
- Drop already merged cleanup patch (v1 1/3)
- replace relative include path
- remove already defined ALIGN macro
- rebase against current master

Andre Przywara (2):
  tools: mkimage: Add Allwinner eGON support
  sunxi: Use mkimage for SPL boot image generation

 common/image.c       |   1 +
 include/image.h      |   1 +
 scripts/Makefile.spl |   8 +--
 tools/Makefile       |   1 +
 tools/sunxi_egon.c   | 133 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 140 insertions(+), 4 deletions(-)
 create mode 100644 tools/sunxi_egon.c

--
2.17.5

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 1/2] tools: mkimage: Add Allwinner eGON support

André Przywara
So far we used the separate mksunxiboot tool for generating a bootable
image for Allwinner SPLs, probably just for historical reasons.

Use the mkimage framework to generate a so called eGON image the
Allwinner BROM expects.
The new image type is called "sunxi_egon", to differentiate it
from the (still to be implemented) secure boot TOC0 image.

Signed-off-by: Andre Przywara <[hidden email]>
Reviewed-by: Simon Glass <[hidden email]>
Reviewed-by: Jagan Teki <[hidden email]>
Tested-by: Jagan Teki <[hidden email]> # BPI-M64

---
 common/image.c     |   1 +
 include/image.h    |   1 +
 tools/Makefile     |   1 +
 tools/sunxi_egon.c | 133 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 136 insertions(+)
 create mode 100644 tools/sunxi_egon.c

diff --git a/common/image.c b/common/image.c
index 451fc689a89..6923dac7c07 100644
--- a/common/image.c
+++ b/common/image.c
@@ -189,6 +189,7 @@ static const table_entry_t uimage_type[] = {
  { IH_TYPE_STM32IMAGE, "stm32image", "STMicroelectronics STM32 Image" },
  { IH_TYPE_MTKIMAGE,   "mtk_image",   "MediaTek BootROM loadable Image" },
  { IH_TYPE_COPRO, "copro", "Coprocessor Image"},
+ { IH_TYPE_SUNXI_EGON, "sunxi_egon",  "Allwinner eGON Boot Image" },
  { -1,    "",  "", },
 };
 
diff --git a/include/image.h b/include/image.h
index 00bc03bebec..89bfca2155a 100644
--- a/include/image.h
+++ b/include/image.h
@@ -308,6 +308,7 @@ enum {
  IH_TYPE_IMX8MIMAGE, /* Freescale IMX8MBoot Image */
  IH_TYPE_IMX8IMAGE, /* Freescale IMX8Boot Image */
  IH_TYPE_COPRO, /* Coprocessor Image for remoteproc*/
+ IH_TYPE_SUNXI_EGON, /* Allwinner eGON Boot Image */
 
  IH_TYPE_COUNT, /* Number of image types */
 };
diff --git a/tools/Makefile b/tools/Makefile
index 51123fd9298..2c9a0c4c4e7 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -104,6 +104,7 @@ dumpimage-mkimage-objs := aisimage.o \
  stm32image.o \
  $(ROCKCHIP_OBS) \
  socfpgaimage.o \
+ sunxi_egon.o \
  lib/crc16.o \
  lib/sha1.o \
  lib/sha256.o \
diff --git a/tools/sunxi_egon.c b/tools/sunxi_egon.c
new file mode 100644
index 00000000000..8dcc4c87413
--- /dev/null
+++ b/tools/sunxi_egon.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2018 Arm Ltd.
+ */
+
+#include "imagetool.h"
+#include <image.h>
+
+#include <asm/arch-sunxi/spl.h>
+
+/* checksum initialiser value */
+#define STAMP_VALUE                     0x5F0A6C39
+
+/*
+ * NAND requires 8K padding. SD/eMMC gets away with 512 bytes,
+ * but let's use the larger padding to cover both.
+ */
+#define PAD_SIZE 8192
+
+static int egon_check_params(struct image_tool_params *params)
+{
+ /* We just need a binary image file. */
+ return !params->dflag;
+}
+
+static int egon_verify_header(unsigned char *ptr, int image_size,
+      struct image_tool_params *params)
+{
+ const struct boot_file_head *header = (void *)ptr;
+ uint32_t length;
+
+ /* First 4 bytes must be an ARM branch instruction. */
+ if ((le32_to_cpu(header->b_instruction) & 0xff000000) != 0xea000000)
+ return EXIT_FAILURE;
+
+ if (memcmp(header->magic, BOOT0_MAGIC, 8))
+ return EXIT_FAILURE;
+
+ length = le32_to_cpu(header->length);
+ /* Must be at least 512 byte aligned. */
+ if (length & 511)
+ return EXIT_FAILURE;
+
+ /*
+ * Image could also contain U-Boot proper, so could be bigger.
+ * But it must not be shorter.
+ */
+ if (image_size < length)
+ return EXIT_FAILURE;
+
+ return EXIT_SUCCESS;
+}
+
+static void egon_print_header(const void *buf)
+{
+ const struct boot_file_head *header = buf;
+
+ printf("Allwinner eGON header, size: %d bytes\n",
+       le32_to_cpu(header->length));
+ if (!memcmp(header->spl_signature, SPL_SIGNATURE, 3))
+ printf("\tSPL header version %d.%d\n",
+       header->spl_signature[3] >> SPL_MINOR_BITS,
+       header->spl_signature[3] & ((1U << SPL_MINOR_BITS) - 1));
+ if (header->spl_signature[3] >= SPL_DT_HEADER_VERSION)
+ printf("\tDT name: %s\n",
+       (char *)buf + le32_to_cpu(header->dt_name_offset));
+}
+
+static void egon_set_header(void *buf, struct stat *sbuf, int infd,
+    struct image_tool_params *params)
+{
+ struct boot_file_head *header = buf;
+ uint32_t *buf32 = buf;
+ uint32_t checksum = 0, value;
+ int i;
+
+ /* Generate an ARM branch instruction to jump over the header. */
+ value = 0xea000000 | (sizeof(struct boot_file_head) / 4 - 2);
+ header->b_instruction = cpu_to_le32(value);
+
+ memcpy(header->magic, BOOT0_MAGIC, 8);
+ header->check_sum = cpu_to_le32(STAMP_VALUE);
+ header->length = cpu_to_le32(ALIGN(sbuf->st_size, PAD_SIZE));
+
+ memcpy(header->spl_signature, SPL_SIGNATURE, 3);
+
+ /* If an image name has been provided, use it as the DT name. */
+ if (params->imagename && params->imagename[0]) {
+ header->spl_signature[3] = SPL_DT_HEADER_VERSION;
+
+ value = offsetof(struct boot_file_head, string_pool);
+ header->dt_name_offset = cpu_to_le32(value);
+
+ strncpy((char *)header->string_pool, params->imagename, 52);
+ /* Make sure we have a terminating zero byte. */
+ ((char *)header->string_pool)[51] = 0;
+ } else
+ header->spl_signature[3] = SPL_ENV_HEADER_VERSION;
+
+ /* Calculate the checksum. Yes, it's that simple. */
+ for (i = 0; i < sbuf->st_size / 4; i++)
+ checksum += le32_to_cpu(buf32[i]);
+ header->check_sum = cpu_to_le32(checksum);
+}
+
+static int egon_check_image_type(uint8_t type)
+{
+ return type == IH_TYPE_SUNXI_EGON ? 0 : 1;
+}
+
+static int egon_vrec_header(struct image_tool_params *params,
+    struct image_type_params *tparams)
+{
+ tparams->hdr = calloc(sizeof(struct boot_file_head), 1);
+
+ /* Return padding to 8K blocks. */
+ return PAD_SIZE - (params->file_size & (PAD_SIZE - 1));
+}
+
+U_BOOT_IMAGE_TYPE(
+ sunxi_egon,
+ "Allwinner eGON Boot Image support",
+ sizeof(struct boot_file_head),
+ NULL,
+ egon_check_params,
+ egon_verify_header,
+ egon_print_header,
+ egon_set_header,
+ NULL,
+ egon_check_image_type,
+ NULL,
+ egon_vrec_header
+);
--
2.17.5

Reply | Threaded
Open this post in threaded view
|

[PATCH v2 2/2] sunxi: Use mkimage for SPL boot image generation

André Przywara
In reply to this post by André Przywara
Switch the SPL boot image generation from using mksunxiboot to the new
sunxi_egon format of mkimage.

Verified to create identical results for all 152 Allwinner boards.

Signed-off-by: Andre Przywara <[hidden email]>
Reviewed-by: Simon Glass <[hidden email]>

---
 scripts/Makefile.spl | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 9f1f7445d71..db95bd998a8 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -382,11 +382,11 @@ endif
 $(obj)/$(SPL_BIN).sfp: $(obj)/$(SPL_BIN).bin FORCE
  $(call if_changed,mkimage)
 
-quiet_cmd_mksunxiboot = MKSUNXI $@
-cmd_mksunxiboot = $(objtree)/tools/mksunxiboot \
- --default-dt $(CONFIG_DEFAULT_DEVICE_TREE) $< $@
+MKIMAGEFLAGS_sunxi-spl.bin = -T sunxi_egon \
+ -n $(CONFIG_DEFAULT_DEVICE_TREE)
+
 $(obj)/sunxi-spl.bin: $(obj)/$(SPL_BIN).bin FORCE
- $(call if_changed,mksunxiboot)
+ $(call if_changed,mkimage)
 
 quiet_cmd_sunxi_spl_image_builder = SUNXI_SPL_IMAGE_BUILDER $@
 cmd_sunxi_spl_image_builder = $(objtree)/tools/sunxi-spl-image-builder \
--
2.17.5

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 2/2] sunxi: Use mkimage for SPL boot image generation

Jagan Teki-3
On Wed, Nov 11, 2020 at 6:23 PM Andre Przywara <[hidden email]> wrote:
>
> Switch the SPL boot image generation from using mksunxiboot to the new
> sunxi_egon format of mkimage.
>
> Verified to create identical results for all 152 Allwinner boards.
>
> Signed-off-by: Andre Przywara <[hidden email]>
> Reviewed-by: Simon Glass <[hidden email]>

Reviewed-by: Jagan Teki <[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v2 1/2] tools: mkimage: Add Allwinner eGON support

Samuel Holland
In reply to this post by André Przywara
Hi Andre,

On 11/11/20 6:52 AM, Andre Przywara wrote:

> So far we used the separate mksunxiboot tool for generating a bootable
> image for Allwinner SPLs, probably just for historical reasons.
>
> Use the mkimage framework to generate a so called eGON image the
> Allwinner BROM expects.
> The new image type is called "sunxi_egon", to differentiate it
> from the (still to be implemented) secure boot TOC0 image.
>
> Signed-off-by: Andre Przywara <[hidden email]>
> Reviewed-by: Simon Glass <[hidden email]>
> Reviewed-by: Jagan Teki <[hidden email]>
> Tested-by: Jagan Teki <[hidden email]> # BPI-M64
>
> ---
>  common/image.c     |   1 +
>  include/image.h    |   1 +
>  tools/Makefile     |   1 +
>  tools/sunxi_egon.c | 133 +++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 136 insertions(+)
>  create mode 100644 tools/sunxi_egon.c
>
> diff --git a/common/image.c b/common/image.c
> index 451fc689a89..6923dac7c07 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -189,6 +189,7 @@ static const table_entry_t uimage_type[] = {
>   { IH_TYPE_STM32IMAGE, "stm32image", "STMicroelectronics STM32 Image" },
>   { IH_TYPE_MTKIMAGE,   "mtk_image",   "MediaTek BootROM loadable Image" },
>   { IH_TYPE_COPRO, "copro", "Coprocessor Image"},
> + { IH_TYPE_SUNXI_EGON, "sunxi_egon",  "Allwinner eGON Boot Image" },
>   { -1,    "",  "", },
>  };
>  
> diff --git a/include/image.h b/include/image.h
> index 00bc03bebec..89bfca2155a 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -308,6 +308,7 @@ enum {
>   IH_TYPE_IMX8MIMAGE, /* Freescale IMX8MBoot Image */
>   IH_TYPE_IMX8IMAGE, /* Freescale IMX8Boot Image */
>   IH_TYPE_COPRO, /* Coprocessor Image for remoteproc*/
> + IH_TYPE_SUNXI_EGON, /* Allwinner eGON Boot Image */
>  
>   IH_TYPE_COUNT, /* Number of image types */
>  };
> diff --git a/tools/Makefile b/tools/Makefile
> index 51123fd9298..2c9a0c4c4e7 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -104,6 +104,7 @@ dumpimage-mkimage-objs := aisimage.o \
>   stm32image.o \
>   $(ROCKCHIP_OBS) \
>   socfpgaimage.o \
> + sunxi_egon.o \
>   lib/crc16.o \
>   lib/sha1.o \
>   lib/sha256.o \
> diff --git a/tools/sunxi_egon.c b/tools/sunxi_egon.c
> new file mode 100644
> index 00000000000..8dcc4c87413
> --- /dev/null
> +++ b/tools/sunxi_egon.c
> @@ -0,0 +1,133 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * (C) Copyright 2018 Arm Ltd.
> + */
> +
> +#include "imagetool.h"
> +#include <image.h>
> +
> +#include <asm/arch-sunxi/spl.h>
> +
> +/* checksum initialiser value */
> +#define STAMP_VALUE                     0x5F0A6C39

Using spaces here seems unintentional.

> +
> +/*
> + * NAND requires 8K padding. SD/eMMC gets away with 512 bytes,
> + * but let's use the larger padding to cover both.
> + */
> +#define PAD_SIZE 8192
> +
> +static int egon_check_params(struct image_tool_params *params)
> +{
> + /* We just need a binary image file. */
> + return !params->dflag;
> +}
> +
> +static int egon_verify_header(unsigned char *ptr, int image_size,
> +      struct image_tool_params *params)
> +{
> + const struct boot_file_head *header = (void *)ptr;
> + uint32_t length;
> +
> + /* First 4 bytes must be an ARM branch instruction. */
> + if ((le32_to_cpu(header->b_instruction) & 0xff000000) != 0xea000000)
> + return EXIT_FAILURE;
> +
> + if (memcmp(header->magic, BOOT0_MAGIC, 8))

sizeof(header->magic)? (same for the memcpy below)

> + return EXIT_FAILURE;
> +
> + length = le32_to_cpu(header->length);
> + /* Must be at least 512 byte aligned. */
> + if (length & 511)
> + return EXIT_FAILURE;
> +
> + /*
> + * Image could also contain U-Boot proper, so could be bigger.
> + * But it must not be shorter.
> + */
> + if (image_size < length)
> + return EXIT_FAILURE;
> +
> + return EXIT_SUCCESS;
> +}
> +
> +static void egon_print_header(const void *buf)
> +{
> + const struct boot_file_head *header = buf;
> +
> + printf("Allwinner eGON header, size: %d bytes\n",
> +       le32_to_cpu(header->length));

While the data being printed comes from the header, it describes the image /
image type as a whole, so "image" seems more appropriate.

> + if (!memcmp(header->spl_signature, SPL_SIGNATURE, 3))
> + printf("\tSPL header version %d.%d\n",
> +       header->spl_signature[3] >> SPL_MINOR_BITS,
> +       header->spl_signature[3] & ((1U << SPL_MINOR_BITS) - 1));
> + if (header->spl_signature[3] >= SPL_DT_HEADER_VERSION)
> + printf("\tDT name: %s\n",
> +       (char *)buf + le32_to_cpu(header->dt_name_offset));

This needs to be inside the SPL_SIGNATURE check, and the DT name is only valid
if dt_name_offset is nonzero.

Maybe use a local variable for the version?

> +}
> +
> +static void egon_set_header(void *buf, struct stat *sbuf, int infd,
> +    struct image_tool_params *params)
> +{
> + struct boot_file_head *header = buf;
> + uint32_t *buf32 = buf;
> + uint32_t checksum = 0, value;
> + int i;
> +
> + /* Generate an ARM branch instruction to jump over the header. */
> + value = 0xea000000 | (sizeof(struct boot_file_head) / 4 - 2);
> + header->b_instruction = cpu_to_le32(value);
> +
> + memcpy(header->magic, BOOT0_MAGIC, 8);
> + header->check_sum = cpu_to_le32(STAMP_VALUE);
> + header->length = cpu_to_le32(ALIGN(sbuf->st_size, PAD_SIZE));

When this function runs, padding has already been applied, so you can use simply
sbuf->st_size (or params->file_size).

> +
> + memcpy(header->spl_signature, SPL_SIGNATURE, 3);
> +
> + /* If an image name has been provided, use it as the DT name. */
> + if (params->imagename && params->imagename[0]) {
> + header->spl_signature[3] = SPL_DT_HEADER_VERSION;
> +
> + value = offsetof(struct boot_file_head, string_pool);
> + header->dt_name_offset = cpu_to_le32(value);
> +
> + strncpy((char *)header->string_pool, params->imagename, 52);
> + /* Make sure we have a terminating zero byte. */
> + ((char *)header->string_pool)[51] = 0;
> + } else
> + header->spl_signature[3] = SPL_ENV_HEADER_VERSION;
> +
> + /* Calculate the checksum. Yes, it's that simple. */
> + for (i = 0; i < sbuf->st_size / 4; i++)
> + checksum += le32_to_cpu(buf32[i]);
> + header->check_sum = cpu_to_le32(checksum);
> +}
> +
> +static int egon_check_image_type(uint8_t type)
> +{
> + return type == IH_TYPE_SUNXI_EGON ? 0 : 1;
> +}
> +
> +static int egon_vrec_header(struct image_tool_params *params,
> +    struct image_type_params *tparams)
> +{
> + tparams->hdr = calloc(sizeof(struct boot_file_head), 1);
> +
> + /* Return padding to 8K blocks. */
> + return PAD_SIZE - (params->file_size & (PAD_SIZE - 1));

I think "ALIGN(params->file_size, PAD_SIZE) - params->file_size;" is much clearer.

And for both patches:

Tested-by: Samuel Holland <[hidden email]>

Cheers,
Samuel

> +}
> +
> +U_BOOT_IMAGE_TYPE(
> + sunxi_egon,
> + "Allwinner eGON Boot Image support",
> + sizeof(struct boot_file_head),
> + NULL,
> + egon_check_params,
> + egon_verify_header,
> + egon_print_header,
> + egon_set_header,
> + NULL,
> + egon_check_image_type,
> + NULL,
> + egon_vrec_header
> +);
>