The ALIGN() is available at imagetool.h, no need to self define one.

Signed-off-by: Kever Yang kever.yang@rock-chips.com Reviewed-by: Punit Agrawal punit1.agrawal@toshiba.co.jp Reviewed-by: Tom Rini trini@konsulko.com ---

Changes in v4: None Changes in v3: None Changes in v2: None

tools/aisimage.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/aisimage.c b/tools/aisimage.c index 4cd76ab843..b8b3ee3207 100644 --- a/tools/aisimage.c +++ b/tools/aisimage.c @@ -10,7 +10,6 @@

#define IS_FNC_EXEC(c) (cmd_table[c].AIS_cmd == AIS_CMD_FNLOAD) #define WORD_ALIGN0 4 -#define WORD_ALIGN(len) (((len)+WORD_ALIGN0-1) & ~(WORD_ALIGN0-1)) #define MAX_CMD_BUFFER 4096

static uint32_t ais_img_size; @@ -202,8 +201,9 @@ static uint32_t *ais_alloc_buffer(struct image_tool_params *params) * is not left to the main program, because after the datafile * the header must be terminated with the Jump & Close command. */ - ais_img_size = WORD_ALIGN(sbuf.st_size) + MAX_CMD_BUFFER; - ptr = (uint32_t *)malloc(WORD_ALIGN(sbuf.st_size) + MAX_CMD_BUFFER); + ais_img_size = ALIGN(sbuf.st_size, WORD_ALIGN0) + MAX_CMD_BUFFER; + ptr = (uint32_t *)malloc(ALIGN(sbuf.st_size, WORD_ALIGN0) + + MAX_CMD_BUFFER); if (!ptr) { fprintf(stderr, "%s: malloc return failure: %s\n", params->cmdname, strerror(errno)); @@ -242,7 +242,7 @@ static uint32_t *ais_copy_image(struct image_tool_params *params, *aisptr++ = params->ep; *aisptr++ = sbuf.st_size; memcpy((void *)aisptr, ptr, sbuf.st_size); - aisptr += WORD_ALIGN(sbuf.st_size) / sizeof(uint32_t); + aisptr += ALIGN(sbuf.st_size, WORD_ALIGN0) / sizeof(uint32_t);

(void) munmap((void *)ptr, sbuf.st_size); (void) close(dfd);

The ALIGN() is now available at imagetool.h, migrate to use it.

Signed-off-by: Kever Yang kever.yang@rock-chips.com Reviewed-by: Punit Agrawal punit1.agrawal@toshiba.co.jp Reviewed-by: Tom Rini trini@konsulko.com ---

Changes in v4: None Changes in v3: None Changes in v2: None

tools/mkimage.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/mkimage.c b/tools/mkimage.c index 5f51d2cc89..0279f6867e 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -7,6 +7,7 @@ * Wolfgang Denk, wd@denx.de */

+#include "imagetool.h" #include "mkimage.h" #include "imximage.h" #include <image.h> @@ -557,8 +558,8 @@ int main(int argc, char **argv) } if (params.type == IH_TYPE_FIRMWARE_IVT) { /* Add alignment and IVT */ - uint32_t aligned_filesize = (params.file_size + 0x1000 - - 1) & ~(0x1000 - 1); + uint32_t aligned_filesize = ALIGN(params.file_size, + 0x1000); flash_header_v2_t ivt_header = { { 0xd1, 0x2000, 0x40 }, params.addr, 0, 0, 0, params.addr + aligned_filesize

The ALIGN() is now available at imagetool.h, migrate to use it.

Signed-off-by: Kever Yang kever.yang@rock-chips.com Reviewed-by: Punit Agrawal punit1.agrawal@toshiba.co.jp Reviewed-by: Tom Rini trini@konsulko.com Reviewed-by: Stefan Roese sr@denx.de ---

Changes in v4: None Changes in v3: None Changes in v2: None

tools/kwbimage.c | 8 ++++---- tools/kwbimage.h | 2 -- 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/tools/kwbimage.c b/tools/kwbimage.c index b8f8d38212..02fd0c949f 100644 --- a/tools/kwbimage.c +++ b/tools/kwbimage.c @@ -1015,7 +1015,7 @@ static size_t image_headersz_v1(int *hasext) * The payload should be aligned on some reasonable * boundary */ - return ALIGN_SUP(headersz, 4096); + return ALIGN(headersz, 4096); }

int add_binary_header_v1(uint8_t *cur) @@ -1058,7 +1058,7 @@ int add_binary_header_v1(uint8_t *cur) * up to a 4-byte boundary. Plus 4 bytes for the * next-header byte and 3-byte alignment at the end. */ - binhdrsz = ALIGN_SUP(binhdrsz, 4) + 4; + binhdrsz = ALIGN(binhdrsz, 4) + 4; hdr->headersz_lsb = cpu_to_le16(binhdrsz & 0xFFFF); hdr->headersz_msb = (binhdrsz & 0xFFFF0000) >> 16;

@@ -1082,7 +1082,7 @@ int add_binary_header_v1(uint8_t *cur)

fclose(bin);

- cur += ALIGN_SUP(s.st_size, 4); + cur += ALIGN(s.st_size, 4);

/* * For now, we don't support more than one binary @@ -1548,7 +1548,7 @@ static void kwbimage_set_header(void *ptr, struct stat *sbuf, int ifd, }

/* The MVEBU BootROM does not allow non word aligned payloads */ - sbuf->st_size = ALIGN_SUP(sbuf->st_size, 4); + sbuf->st_size = ALIGN(sbuf->st_size, 4);

version = image_get_version(); switch (version) { diff --git a/tools/kwbimage.h b/tools/kwbimage.h index 25bc08c5ce..0b6d05bef1 100644 --- a/tools/kwbimage.h +++ b/tools/kwbimage.h @@ -29,8 +29,6 @@ #define IBR_HDR_UART_ID 0x69 #define IBR_DEF_ATTRIB 0x00

-#define ALIGN_SUP(x, a) (((x) + (a - 1)) & ~(a - 1)) - /* Structure of the main header, version 0 (Kirkwood, Dove) */ struct main_hdr_v0 { uint8_t blockid; /* 0x0 */

The align for fit_size has been done twice, remove the first one for it does not make any sense.

Signed-off-by: Kever Yang kever.yang@rock-chips.com Reviewed-by: Punit Agrawal punit1.agrawal@toshiba.co.jp Reviewed-by: Tom Rini trini@konsulko.com ---

Changes in v4: None Changes in v3: None Changes in v2: None

tools/imx8mimage.c | 1 - 1 file changed, 1 deletion(-)

diff --git a/tools/imx8mimage.c b/tools/imx8mimage.c index 7defb13962..bc4ee793cb 100644 --- a/tools/imx8mimage.c +++ b/tools/imx8mimage.c @@ -341,7 +341,6 @@ static int generate_ivt_for_fit(int fd, int fit_offset, uint32_t ep, }

fit_size = fdt_totalsize(&image_header); - fit_size = (fit_size + 3) & ~3;

fit_size = ALIGN(fit_size, ALIGN_SIZE);

Use the ALIGN() for size align so that the code is more readable.

Signed-off-by: Kever Yang kever.yang@rock-chips.com Reviewed-by: Punit Agrawal punit1.agrawal@toshiba.co.jp Reviewed-by: Tom Rini trini@konsulko.com Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de ---

Changes in v4: None Changes in v3: None Changes in v2: None

tools/fit_image.c | 2 +- tools/socfpgaimage.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/fit_image.c b/tools/fit_image.c index dd61a816c9..b7d615f8c8 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -547,7 +547,7 @@ static int fit_import_data(struct image_tool_params *params, const char *fname) if (fd < 0) return -EIO; fit_size = fdt_totalsize(old_fdt); - data_base = (fit_size + 3) & ~3; + data_base = ALIGN(fit_size, 4);

/* Allocate space to hold the new FIT */ size = sbuf.st_size + 16384; diff --git a/tools/socfpgaimage.c b/tools/socfpgaimage.c index 8fa098338b..6dfd64e31d 100644 --- a/tools/socfpgaimage.c +++ b/tools/socfpgaimage.c @@ -203,7 +203,7 @@ static int sfp_sign_buffer(uint8_t *buf, uint8_t ver, uint8_t flags, uint32_t calc_crc;

/* Align the length up */ - len = (len + 3) & ~3; + len = ALIGN(len, 4);

/* Build header, adding 4 bytes to length to hold the CRC32. */ sfp_build_header(buf + HEADER_OFFSET, ver, flags, len + 4);

The tool need to use fdtdec_get_child_count(), make it available for HOST_CC.

Signed-off-by: Kever Yang kever.yang@rock-chips.com Reviewed-by: Punit Agrawal punit1.agrawal@toshiba.co.jp ---

Changes in v4: - add function comment for fdtdec_get_child_count() in fdt_support.h

Changes in v3: None Changes in v2: None

include/fdt_support.h | 9 +++++++++ lib/fdtdec.c | 11 ----------- lib/fdtdec_common.c | 11 +++++++++++ 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/include/fdt_support.h b/include/fdt_support.h index ba14acd7f6..2eff311fa4 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -343,6 +343,15 @@ int fdt_get_cells_len(const void *blob, char *nr_cells_name); #ifdef USE_HOSTCC int fdtdec_get_int(const void *blob, int node, const char *prop_name, int default_val); + +/* + * Count child nodes of one parent node. + * + * @param blob FDT blob + * @param node parent node + * @return number of child node; 0 if there is not child node + */ +int fdtdec_get_child_count(const void *blob, int node); #endif #ifdef CONFIG_FMAN_ENET int fdt_update_ethernet_dt(void *blob); diff --git a/lib/fdtdec.c b/lib/fdtdec.c index eb11fc898e..e13af283a1 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -810,17 +810,6 @@ int fdtdec_parse_phandle_with_args(const void *blob, int src_node, return rc; }

-int fdtdec_get_child_count(const void *blob, int node) -{ - int subnode; - int num = 0; - - fdt_for_each_subnode(subnode, blob, node) - num++; - - return num; -} - int fdtdec_get_byte_array(const void *blob, int node, const char *prop_name, u8 *array, int count) { diff --git a/lib/fdtdec_common.c b/lib/fdtdec_common.c index 088e9e9063..5775992ef3 100644 --- a/lib/fdtdec_common.c +++ b/lib/fdtdec_common.c @@ -53,3 +53,14 @@ unsigned int fdtdec_get_uint(const void *blob, int node, const char *prop_name, debug("(not found)\n"); return default_val; } + +int fdtdec_get_child_count(const void *blob, int node) +{ + int subnode; + int num = 0; + + fdt_for_each_subnode(subnode, blob, node) + num++; + + return num; +}

The image is usually stored in block device like emmc, SD card, make the offset of image data aligned to block(512 byte) can avoid data copy during boot process. eg. SPL boot from FIT image with external data: - SPL read the first block of FIT image, and then parse the header; - SPL read image data separately; - The first image offset is the base_offset which is the header size; - The second image offset is just after the first image; - If the offset of imge does not aligned, SPL will do memcpy; The header size is a ramdon number, which is very possible not aligned, so add '-B size'to specify the align size in hex for better performance.

example usage: ./tools/mkimage -E -f u-boot.its -B 0x200 u-boot.itb

Signed-off-by: Kever Yang kever.yang@rock-chips.com Reviewed-by: Punit Agrawal punit1.agrawal@toshiba.co.jp ---

Changes in v4: - update to clarify the size in hex format

Changes in v3: - add common ALIGN() at imagetool.h - migrate to use imagetool.h for all other files under tools/

Changes in v2: - use '-B' to take a argument as a align block lenth; - add new variable to indecate align_size - address commens from Heinrich, Rasmus, Tom, Punit;

doc/uImage.FIT/source_file_format.txt | 5 ++++ tools/fit_image.c | 33 ++++++++++++++++----------- tools/imagetool.h | 1 + tools/mkimage.c | 14 ++++++++++-- 4 files changed, 38 insertions(+), 15 deletions(-)

diff --git a/doc/uImage.FIT/source_file_format.txt b/doc/uImage.FIT/source_file_format.txt index 18d2aedcb7..884a58456f 100644 --- a/doc/uImage.FIT/source_file_format.txt +++ b/doc/uImage.FIT/source_file_format.txt @@ -304,6 +304,11 @@ Normal kernel FIT image has data embedded within FIT structure. U-Boot image for SPL boot has external data. Existence of 'data-offset' can be used to identify which format is used.

+For FIT image with external data, it would be better to align each blob of data +to block(512 byte) for block device, so that we don't need to do the copy when +read the image data in SPL. Pass '-B 0x200' to mkimage to align the FIT +structure and data to 512 byte, other values available for other align size. + 9) Examples -----------

diff --git a/tools/fit_image.c b/tools/fit_image.c index b7d615f8c8..06c32b3e71 100644 --- a/tools/fit_image.c +++ b/tools/fit_image.c @@ -422,7 +422,7 @@ err_buf: */ static int fit_extract_data(struct image_tool_params *params, const char *fname) { - void *buf; + void *buf = NULL; int buf_ptr; int fit_size, new_size; int fd; @@ -431,26 +431,33 @@ static int fit_extract_data(struct image_tool_params *params, const char *fname) int ret; int images; int node; + int image_number; + int align_size;

+ align_size = params->bl_len ? params->bl_len : 4; fd = mmap_fdt(params->cmdname, fname, 0, &fdt, &sbuf, false, false); if (fd < 0) return -EIO; fit_size = fdt_totalsize(fdt);

- /* Allocate space to hold the image data we will extract */ - buf = malloc(fit_size); - if (!buf) { - ret = -ENOMEM; - goto err_munmap; - } - buf_ptr = 0; - images = fdt_path_offset(fdt, FIT_IMAGES_PATH); if (images < 0) { debug("%s: Cannot find /images node: %d\n", __func__, images); ret = -EINVAL; goto err_munmap; } + image_number = fdtdec_get_child_count(fdt, images); + + /* + * Allocate space to hold the image data we will extract, + * extral space allocate for image alignment to prevent overflow. + */ + buf = malloc(fit_size + (align_size * image_number)); + if (!buf) { + ret = -ENOMEM; + goto err_munmap; + } + buf_ptr = 0;

for (node = fdt_first_subnode(fdt, images); node >= 0; @@ -478,17 +485,17 @@ static int fit_extract_data(struct image_tool_params *params, const char *fname) buf_ptr); } fdt_setprop_u32(fdt, node, FIT_DATA_SIZE_PROP, len); - - buf_ptr += (len + 3) & ~3; + buf_ptr += ALIGN(len, align_size); }

/* Pack the FDT and place the data after it */ fdt_pack(fdt);

+ new_size = fdt_totalsize(fdt); + new_size = ALIGN(new_size, align_size); + fdt_set_totalsize(fdt, new_size); debug("Size reduced from %x to %x\n", fit_size, fdt_totalsize(fdt)); debug("External data size %x\n", buf_ptr); - new_size = fdt_totalsize(fdt); - new_size = (new_size + 3) & ~3; munmap(fdt, sbuf.st_size);

if (ftruncate(fd, new_size)) { diff --git a/tools/imagetool.h b/tools/imagetool.h index 81e5cd0c5c..f54809cd57 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -79,6 +79,7 @@ struct image_tool_params { bool external_data; /* Store data outside the FIT */ bool quiet; /* Don't output text in normal operation */ unsigned int external_offset; /* Add padding to external data */ + int bl_len; /* Block length in byte for external data */ const char *engine_id; /* Engine to use for signing */ };

diff --git a/tools/mkimage.c b/tools/mkimage.c index 0279f6867e..336376f8d0 100644 --- a/tools/mkimage.c +++ b/tools/mkimage.c @@ -98,8 +98,9 @@ static void usage(const char *msg) " -i => input filename for ramdisk file\n"); #ifdef CONFIG_FIT_SIGNATURE fprintf(stderr, - "Signing / verified boot options: [-E] [-k keydir] [-K dtb] [ -c <comment>] [-p addr] [-r] [-N engine]\n" + "Signing / verified boot options: [-E] [-B size] [-k keydir] [-K dtb] [ -c <comment>] [-p addr] [-r] [-N engine]\n" " -E => place data outside of the FIT structure\n" + " -B => align size in hex for FIT structure and header\n" " -k => set directory containing private keys\n" " -K => write public keys to this .dtb file\n" " -c => add comment in signature node\n" @@ -144,7 +145,7 @@ static void process_args(int argc, char **argv) int opt;

while ((opt = getopt(argc, argv, - "a:A:b:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qsT:vVx")) != -1) { + "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qsT:vVx")) != -1) { switch (opt) { case 'a': params.addr = strtoull(optarg, &ptr, 16); @@ -168,6 +169,15 @@ static void process_args(int argc, char **argv) params.cmdname, optarg); exit(EXIT_FAILURE); } + break; + case 'B': + params.bl_len = strtoull(optarg, &ptr, 16); + if (*ptr) { + fprintf(stderr, "%s: invalid block length %s\n", + params.cmdname, optarg); + exit(EXIT_FAILURE); + } + break; case 'c': params.comment = optarg;