diff mbox series

[v4,8/8] mkimage: fit_image: Add option to make fit header align

Message ID 20200330035625.25164-8-kever.yang@rock-chips.com
State Accepted
Commit ebfe611be91e0075c040588a30a9996519d30aa6
Delegated to: Tom Rini
Headers show
Series [v4,1/8] tool: Move ALIGN_MASK to header as common MACRO | expand

Commit Message

Kever Yang March 30, 2020, 3:56 a.m. UTC
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(-)

Comments

Tom Rini March 30, 2020, 11:30 p.m. UTC | #1
On Mon, Mar 30, 2020 at 11:56:24AM +0800, Kever Yang wrote:

> 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>

Reviewed-by: Tom Rini <trini@konsulko.com>
Kever Yang April 24, 2020, 6:52 a.m. UTC | #2
Hi Tom,

     Who is suppose to pick up this series patches? Any thing I need to 
update?


Thanks,

- Kever

On 2020/3/31 上午7:30, Tom Rini wrote:
> On Mon, Mar 30, 2020 at 11:56:24AM +0800, Kever Yang wrote:
>
>> 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>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
Tom Rini April 24, 2020, 5:10 p.m. UTC | #3
On Mon, Mar 30, 2020 at 11:56:24AM +0800, Kever Yang wrote:

> 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>
> Reviewed-by: Tom Rini <trini@konsulko.com>

Applied to u-boot/master, thanks!
diff mbox series

Patch

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;