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

2020-11-16 Thread Samuel Holland
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 
> Reviewed-by: Simon Glass 
> Reviewed-by: Jagan Teki 
> Tested-by: Jagan Teki  # 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 000..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 
> +
> +#include 
> +
> +/* 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) & 0xff00) != 0xea00)
> + 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 

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

2020-11-11 Thread Andre 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 
Reviewed-by: Simon Glass 
Reviewed-by: Jagan Teki 
Tested-by: Jagan Teki  # 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 000..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 
+
+#include 
+
+/* 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) & 0xff00) != 0xea00)
+   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 = 0xea00 | (sizeof(struct boot_file_head) / 4 - 2);
+   header->b_instruction = cpu_to_le32(value);
+
+