All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v5 0/3] fit: Image node compression
@ 2019-07-11 20:53 Julius Werner
  2019-07-11 20:53 ` [U-Boot] [PATCH v5 1/3] common: Move bootm_decomp_image() to image.c (as image_decomp()) Julius Werner
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Julius Werner @ 2019-07-11 20:53 UTC (permalink / raw)
  To: u-boot

This patch series adds compression support for non-kernel FIT image
nodes (e.g. FDTs). The first patch is a preparatory refactoring, the
second adds the compression support itself, and the third adds a new
feature to compatible string matching that allows it to be useful
with compressed FDTs.

Sandbox-tested with FIT images with and without compressed FDTs, with
and without overlays, in both compatible string matching and direct
config selection modes. Also expanded the test_fit pytest to include a
case with compressed kernel, FDT and ramdisk.

Julius Werner (3):
  common: Move bootm_decomp_image() to image.c (as image_decomp())
    - First version: v5
  fit: Support compression for non-kernel components (e.g. FDT)
    - Changes for v2:
      - Changed from only supporting compressed FDTs to supporting all
        non-kernel image node types.
    - Changes for v3:
      - Fixed up some debug output that was still written for v1.
      - Fixed a mistake with handling FIT_LOAD_OPTIONAL_NON_ZERO when
        'load' was 0 (i.e. unset).
      - Added compression test case to the test_fit pytest.
    - No changes for v4
    - No changes for v5
  fit: Support compat string property in configuration node
    - No changes for v2
    - No changes for v3
    - Changes for v4:
      - Added documentation for compatible string in config node.
      - Added example .its file for compressed FDT with compat string in
        config node.
    - No changes for v5

 common/bootm.c                            | 148 +++-------------------
 common/image-fit.c                        | 143 ++++++++++++---------
 common/image.c                            | 111 ++++++++++++++++
 doc/uImage.FIT/kernel_fdts_compressed.its |  73 +++++++++++
 doc/uImage.FIT/source_file_format.txt     |   7 +
 include/bootm.h                           |  17 ---
 include/image.h                           |  17 +++
 test/compression.c                        |  24 ++--
 test/py/tests/test_fit.py                 |  29 ++++-
 9 files changed, 351 insertions(+), 218 deletions(-)
 create mode 100644 doc/uImage.FIT/kernel_fdts_compressed.its

-- 
2.20.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 1/3] common: Move bootm_decomp_image() to image.c (as image_decomp())
  2019-07-11 20:53 [U-Boot] [PATCH v5 0/3] fit: Image node compression Julius Werner
@ 2019-07-11 20:53 ` Julius Werner
  2019-07-12  6:09   ` Simon Goldschmidt
  2019-07-24 18:14   ` Tom Rini
  2019-07-11 20:53 ` [U-Boot] [PATCH v5 2/3] fit: Support compression for non-kernel components (e.g. FDT) Julius Werner
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 15+ messages in thread
From: Julius Werner @ 2019-07-11 20:53 UTC (permalink / raw)
  To: u-boot

Upcoming patches want to add decompression to use cases that are no
longer directly related to booting. It makes sense to retain a single
decompression routine, but it should no longer be in bootm.c (which is
not compiled for all configurations). This patch moves
bootm_decomp_image() to image.c and renames it to image_decomp() in
preparation of those upcoming patches.

Signed-off-by: Julius Werner <jwerner@chromium.org>
---
 - First version: v5

 common/bootm.c     | 148 ++++++---------------------------------------
 common/image.c     | 111 ++++++++++++++++++++++++++++++++++
 include/bootm.h    |  17 ------
 include/image.h    |  17 ++++++
 test/compression.c |  24 ++++----
 5 files changed, 160 insertions(+), 157 deletions(-)

diff --git a/common/bootm.c b/common/bootm.c
index d193751647..2e64140df6 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -7,17 +7,12 @@
 #ifndef USE_HOSTCC
 #include <common.h>
 #include <bootstage.h>
-#include <bzlib.h>
 #include <errno.h>
 #include <fdt_support.h>
 #include <lmb.h>
 #include <malloc.h>
 #include <mapmem.h>
 #include <asm/io.h>
-#include <linux/lzo.h>
-#include <lzma/LzmaTypes.h>
-#include <lzma/LzmaDec.h>
-#include <lzma/LzmaTools.h>
 #if defined(CONFIG_CMD_USB)
 #include <usb.h>
 #endif
@@ -299,23 +294,6 @@ static int bootm_find_other(cmd_tbl_t *cmdtp, int flag, int argc,
 }
 #endif /* USE_HOSTC */
 
-/**
- * print_decomp_msg() - Print a suitable decompression/loading message
- *
- * @type:	OS type (IH_OS_...)
- * @comp_type:	Compression type being used (IH_COMP_...)
- * @is_xip:	true if the load address matches the image start
- */
-static void print_decomp_msg(int comp_type, int type, bool is_xip)
-{
-	const char *name = genimg_get_type_name(type);
-
-	if (comp_type == IH_COMP_NONE)
-		printf("   %s %s ... ", is_xip ? "XIP" : "Loading", name);
-	else
-		printf("   Uncompressing %s ... ", name);
-}
-
 /**
  * handle_decomp_error() - display a decompression error
  *
@@ -325,16 +303,18 @@ static void print_decomp_msg(int comp_type, int type, bool is_xip)
  *
  * @comp_type:		Compression type being used (IH_COMP_...)
  * @uncomp_size:	Number of bytes uncompressed
- * @unc_len:		Amount of space available for decompression
- * @ret:		Error code to report
- * @return BOOTM_ERR_RESET, indicating that the board must be reset
+ * @ret:		errno error code received from compression library
+ * @return Appropriate BOOTM_ERR_ error code
  */
-static int handle_decomp_error(int comp_type, size_t uncomp_size,
-			       size_t unc_len, int ret)
+static int handle_decomp_error(int comp_type, size_t uncomp_size, int ret)
 {
 	const char *name = genimg_get_comp_name(comp_type);
 
-	if (uncomp_size >= unc_len)
+	/* ENOSYS means unimplemented compression type, don't reset. */
+	if (ret == -ENOSYS)
+		return BOOTM_ERR_UNIMPLEMENTED;
+
+	if (uncomp_size >= CONFIG_SYS_BOOTM_LEN)
 		printf("Image too large: increase CONFIG_SYS_BOOTM_LEN\n");
 	else
 		printf("%s: uncompress error %d\n", name, ret);
@@ -352,93 +332,6 @@ static int handle_decomp_error(int comp_type, size_t uncomp_size,
 	return BOOTM_ERR_RESET;
 }
 
-int bootm_decomp_image(int comp, ulong load, ulong image_start, int type,
-		       void *load_buf, void *image_buf, ulong image_len,
-		       uint unc_len, ulong *load_end)
-{
-	int ret = 0;
-
-	*load_end = load;
-	print_decomp_msg(comp, type, load == image_start);
-
-	/*
-	 * Load the image to the right place, decompressing if needed. After
-	 * this, image_len will be set to the number of uncompressed bytes
-	 * loaded, ret will be non-zero on error.
-	 */
-	switch (comp) {
-	case IH_COMP_NONE:
-		if (load == image_start)
-			break;
-		if (image_len <= unc_len)
-			memmove_wd(load_buf, image_buf, image_len, CHUNKSZ);
-		else
-			ret = 1;
-		break;
-#ifdef CONFIG_GZIP
-	case IH_COMP_GZIP: {
-		ret = gunzip(load_buf, unc_len, image_buf, &image_len);
-		break;
-	}
-#endif /* CONFIG_GZIP */
-#ifdef CONFIG_BZIP2
-	case IH_COMP_BZIP2: {
-		uint size = unc_len;
-
-		/*
-		 * If we've got less than 4 MB of malloc() space,
-		 * use slower decompression algorithm which requires
-		 *@most 2300 KB of memory.
-		 */
-		ret = BZ2_bzBuffToBuffDecompress(load_buf, &size,
-			image_buf, image_len,
-			CONFIG_SYS_MALLOC_LEN < (4096 * 1024), 0);
-		image_len = size;
-		break;
-	}
-#endif /* CONFIG_BZIP2 */
-#ifdef CONFIG_LZMA
-	case IH_COMP_LZMA: {
-		SizeT lzma_len = unc_len;
-
-		ret = lzmaBuffToBuffDecompress(load_buf, &lzma_len,
-					       image_buf, image_len);
-		image_len = lzma_len;
-		break;
-	}
-#endif /* CONFIG_LZMA */
-#ifdef CONFIG_LZO
-	case IH_COMP_LZO: {
-		size_t size = unc_len;
-
-		ret = lzop_decompress(image_buf, image_len, load_buf, &size);
-		image_len = size;
-		break;
-	}
-#endif /* CONFIG_LZO */
-#ifdef CONFIG_LZ4
-	case IH_COMP_LZ4: {
-		size_t size = unc_len;
-
-		ret = ulz4fn(image_buf, image_len, load_buf, &size);
-		image_len = size;
-		break;
-	}
-#endif /* CONFIG_LZ4 */
-	default:
-		printf("Unimplemented compression type %d\n", comp);
-		return BOOTM_ERR_UNIMPLEMENTED;
-	}
-
-	if (ret)
-		return handle_decomp_error(comp, image_len, unc_len, ret);
-	*load_end = load + image_len;
-
-	puts("OK\n");
-
-	return 0;
-}
-
 #ifndef USE_HOSTCC
 static int bootm_load_os(bootm_headers_t *images, int boot_progress)
 {
@@ -456,10 +349,11 @@ static int bootm_load_os(bootm_headers_t *images, int boot_progress)
 
 	load_buf = map_sysmem(load, 0);
 	image_buf = map_sysmem(os.image_start, image_len);
-	err = bootm_decomp_image(os.comp, load, os.image_start, os.type,
-				 load_buf, image_buf, image_len,
-				 CONFIG_SYS_BOOTM_LEN, &load_end);
+	err = image_decomp(os.comp, load, os.image_start, os.type,
+			   load_buf, image_buf, image_len,
+			   CONFIG_SYS_BOOTM_LEN, &load_end);
 	if (err) {
+		err = handle_decomp_error(os.comp, load_end - load, err);
 		bootstage_error(BOOTSTAGE_ID_DECOMP_IMAGE);
 		return err;
 	}
@@ -919,11 +813,6 @@ void __weak switch_to_non_secure_mode(void)
 
 #else /* USE_HOSTCC */
 
-void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
-{
-	memmove(to, from, len);
-}
-
 #if defined(CONFIG_FIT_SIGNATURE)
 static int bootm_host_load_image(const void *fit, int req_image_type)
 {
@@ -957,13 +846,16 @@ static int bootm_host_load_image(const void *fit, int req_image_type)
 
 	/* Allow the image to expand by a factor of 4, should be safe */
 	load_buf = malloc((1 << 20) + len * 4);
-	ret = bootm_decomp_image(imape_comp, 0, data, image_type, load_buf,
-				 (void *)data, len, CONFIG_SYS_BOOTM_LEN,
-				 &load_end);
+	ret = image_decomp(imape_comp, 0, data, image_type, load_buf,
+			   (void *)data, len, CONFIG_SYS_BOOTM_LEN,
+			   &load_end);
 	free(load_buf);
 
-	if (ret && ret != BOOTM_ERR_UNIMPLEMENTED)
-		return ret;
+	if (ret) {
+		ret = handle_decomp_error(imape_comp, load_end - 0, ret);
+		if (ret != BOOTM_ERR_UNIMPLEMENTED)
+			return ret;
+	}
 
 	return 0;
 }
diff --git a/common/image.c b/common/image.c
index 75b84d5009..04da002fcc 100644
--- a/common/image.c
+++ b/common/image.c
@@ -32,6 +32,12 @@
 #include <linux/errno.h>
 #include <asm/io.h>
 
+#include <bzlib.h>
+#include <linux/lzo.h>
+#include <lzma/LzmaTypes.h>
+#include <lzma/LzmaDec.h>
+#include <lzma/LzmaTools.h>
+
 #ifdef CONFIG_CMD_BDI
 extern int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
 #endif
@@ -375,6 +381,106 @@ void image_print_contents(const void *ptr)
 	}
 }
 
+/**
+ * print_decomp_msg() - Print a suitable decompression/loading message
+ *
+ * @type:	OS type (IH_OS_...)
+ * @comp_type:	Compression type being used (IH_COMP_...)
+ * @is_xip:	true if the load address matches the image start
+ */
+static void print_decomp_msg(int comp_type, int type, bool is_xip)
+{
+	const char *name = genimg_get_type_name(type);
+
+	if (comp_type == IH_COMP_NONE)
+		printf("   %s %s ... ", is_xip ? "XIP" : "Loading", name);
+	else
+		printf("   Uncompressing %s ... ", name);
+}
+
+int image_decomp(int comp, ulong load, ulong image_start, int type,
+		 void *load_buf, void *image_buf, ulong image_len,
+		 uint unc_len, ulong *load_end)
+{
+	int ret = 0;
+
+	*load_end = load;
+	print_decomp_msg(comp, type, load == image_start);
+
+	/*
+	 * Load the image to the right place, decompressing if needed. After
+	 * this, image_len will be set to the number of uncompressed bytes
+	 * loaded, ret will be non-zero on error.
+	 */
+	switch (comp) {
+	case IH_COMP_NONE:
+		if (load == image_start)
+			break;
+		if (image_len <= unc_len)
+			memmove_wd(load_buf, image_buf, image_len, CHUNKSZ);
+		else
+			ret = -ENOSPC;
+		break;
+#ifdef CONFIG_GZIP
+	case IH_COMP_GZIP: {
+		ret = gunzip(load_buf, unc_len, image_buf, &image_len);
+		break;
+	}
+#endif /* CONFIG_GZIP */
+#ifdef CONFIG_BZIP2
+	case IH_COMP_BZIP2: {
+		uint size = unc_len;
+
+		/*
+		 * If we've got less than 4 MB of malloc() space,
+		 * use slower decompression algorithm which requires
+		 *@most 2300 KB of memory.
+		 */
+		ret = BZ2_bzBuffToBuffDecompress(load_buf, &size,
+			image_buf, image_len,
+			CONFIG_SYS_MALLOC_LEN < (4096 * 1024), 0);
+		image_len = size;
+		break;
+	}
+#endif /* CONFIG_BZIP2 */
+#ifdef CONFIG_LZMA
+	case IH_COMP_LZMA: {
+		SizeT lzma_len = unc_len;
+
+		ret = lzmaBuffToBuffDecompress(load_buf, &lzma_len,
+					       image_buf, image_len);
+		image_len = lzma_len;
+		break;
+	}
+#endif /* CONFIG_LZMA */
+#ifdef CONFIG_LZO
+	case IH_COMP_LZO: {
+		size_t size = unc_len;
+
+		ret = lzop_decompress(image_buf, image_len, load_buf, &size);
+		image_len = size;
+		break;
+	}
+#endif /* CONFIG_LZO */
+#ifdef CONFIG_LZ4
+	case IH_COMP_LZ4: {
+		size_t size = unc_len;
+
+		ret = ulz4fn(image_buf, image_len, load_buf, &size);
+		image_len = size;
+		break;
+	}
+#endif /* CONFIG_LZ4 */
+	default:
+		printf("Unimplemented compression type %d\n", comp);
+		return -ENOSYS;
+	}
+
+	*load_end = load + image_len;
+
+	return ret;
+}
+
 
 #ifndef USE_HOSTCC
 #if defined(CONFIG_IMAGE_FORMAT_LEGACY)
@@ -551,6 +657,11 @@ void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
 	memmove(to, from, len);
 #endif	/* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
 }
+#else	/* USE_HOSTCC */
+void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
+{
+	memmove(to, from, len);
+}
 #endif /* !USE_HOSTCC */
 
 void genimg_print_size(uint32_t size)
diff --git a/include/bootm.h b/include/bootm.h
index f771b733f5..edeeacb0df 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -59,23 +59,6 @@ int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
 
 void arch_preboot_os(void);
 
-/**
- * bootm_decomp_image() - decompress the operating system
- *
- * @comp:	Compression algorithm that is used (IH_COMP_...)
- * @load:	Destination load address in U-Boot memory
- * @image_start Image start address (where we are decompressing from)
- * @type:	OS type (IH_OS_...)
- * @load_bug:	Place to decompress to
- * @image_buf:	Address to decompress from
- * @image_len:	Number of bytes in @image_buf to decompress
- * @unc_len:	Available space for decompression
- * @return 0 if OK, -ve on error (BOOTM_ERR_...)
- */
-int bootm_decomp_image(int comp, ulong load, ulong image_start, int type,
-		       void *load_buf, void *image_buf, ulong image_len,
-		       uint unc_len, ulong *load_end);
-
 /*
  * boards should define this to disable devices when EFI exits from boot
  * services.
diff --git a/include/image.h b/include/image.h
index bb7089ef5d..3bdb9f0a97 100644
--- a/include/image.h
+++ b/include/image.h
@@ -849,6 +849,23 @@ static inline int image_check_target_arch(const image_header_t *hdr)
 }
 #endif /* USE_HOSTCC */
 
+/**
+ * image_decomp() - decompress an image
+ *
+ * @comp:	Compression algorithm that is used (IH_COMP_...)
+ * @load:	Destination load address in U-Boot memory
+ * @image_start Image start address (where we are decompressing from)
+ * @type:	OS type (IH_OS_...)
+ * @load_bug:	Place to decompress to
+ * @image_buf:	Address to decompress from
+ * @image_len:	Number of bytes in @image_buf to decompress
+ * @unc_len:	Available space for decompression
+ * @return 0 if OK, -ve on error (BOOTM_ERR_...)
+ */
+int image_decomp(int comp, ulong load, ulong image_start, int type,
+		 void *load_buf, void *image_buf, ulong image_len,
+		 uint unc_len, ulong *load_end);
+
 /**
  * Set up properties in the FDT
  *
diff --git a/test/compression.c b/test/compression.c
index 7bc0f73e09..dc5e94684f 100644
--- a/test/compression.c
+++ b/test/compression.c
@@ -471,15 +471,15 @@ static int run_bootm_test(struct unit_test_state *uts, int comp_type,
 	unc_len = strlen(plain);
 	compress(uts, (void *)plain, unc_len, compress_buff, compress_size,
 		 &compress_size);
-	err = bootm_decomp_image(comp_type, load_addr, image_start,
-				 IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
-				 compress_buff, compress_size, unc_len,
-				 &load_end);
+	err = image_decomp(comp_type, load_addr, image_start,
+			   IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
+			   compress_buff, compress_size, unc_len,
+			   &load_end);
 	ut_assertok(err);
-	err = bootm_decomp_image(comp_type, load_addr, image_start,
-				 IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
-				 compress_buff, compress_size, unc_len - 1,
-				 &load_end);
+	err = image_decomp(comp_type, load_addr, image_start,
+			   IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
+			   compress_buff, compress_size, unc_len - 1,
+			   &load_end);
 	ut_assert(err);
 
 	/* We can't detect corruption when not decompressing */
@@ -487,10 +487,10 @@ static int run_bootm_test(struct unit_test_state *uts, int comp_type,
 		return 0;
 	memset(compress_buff + compress_size / 2, '\x49',
 	       compress_size / 2);
-	err = bootm_decomp_image(comp_type, load_addr, image_start,
-				 IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
-				 compress_buff, compress_size, 0x10000,
-				 &load_end);
+	err = image_decomp(comp_type, load_addr, image_start,
+			   IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
+			   compress_buff, compress_size, 0x10000,
+			   &load_end);
 	ut_assert(err);
 
 	return 0;
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/3] fit: Support compression for non-kernel components (e.g. FDT)
  2019-07-11 20:53 [U-Boot] [PATCH v5 0/3] fit: Image node compression Julius Werner
  2019-07-11 20:53 ` [U-Boot] [PATCH v5 1/3] common: Move bootm_decomp_image() to image.c (as image_decomp()) Julius Werner
@ 2019-07-11 20:53 ` Julius Werner
  2019-07-12  6:10   ` Simon Goldschmidt
  2019-07-11 20:53 ` [U-Boot] [PATCH v5 3/3] fit: Support compat string property in configuration node Julius Werner
  2019-07-12  6:02 ` [U-Boot] [PATCH v5 0/3] fit: Image node compression Simon Goldschmidt
  3 siblings, 1 reply; 15+ messages in thread
From: Julius Werner @ 2019-07-11 20:53 UTC (permalink / raw)
  To: u-boot

This patch adds support for compressing non-kernel image nodes in a FIT
image (kernel nodes could already be compressed previously). This can
reduce the size of FIT images and therefore improve boot times
(especially when an image bundles many different kernel FDTs). The
images will automatically be decompressed on load.

This patch does not support extracting compatible strings from
compressed FDTs, so it's not very helpful in conjunction with
CONFIG_FIT_BEST_MATCH yet, but it can already be used in environments
that select the configuration to load explicitly.

Signed-off-by: Julius Werner <jwerner@chromium.org>
---
 - Changes for v2:
   - Changed from only supporting compressed FDTs to supporting all
     non-kernel image node types.
 - Changes for v3:
   - Fixed up some debug output that was still written for v1.
   - Fixed a mistake with handling FIT_LOAD_OPTIONAL_NON_ZERO when
     'load' was 0 (i.e. unset).
   - Added compression test case to the test_fit pytest.
 - No changes for v4
 - No changes for v5

 common/image-fit.c        | 86 +++++++++++++++++++++++----------------
 test/py/tests/test_fit.py | 29 +++++++++++--
 2 files changed, 77 insertions(+), 38 deletions(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index a74b44f298..c9ffc441aa 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -22,6 +22,7 @@
 DECLARE_GLOBAL_DATA_PTR;
 #endif /* !USE_HOSTCC*/
 
+#include <bootm.h>
 #include <image.h>
 #include <bootstage.h>
 #include <u-boot/crc.h>
@@ -1576,6 +1577,13 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
 			      kfdt_name);
 			continue;
 		}
+
+		if (!fit_image_check_comp(fit, kfdt_noffset, IH_COMP_NONE)) {
+			debug("Can't extract compat from \"%s\" (compressed)\n",
+			      kfdt_name);
+			continue;
+		}
+
 		/*
 		 * Get a pointer to this configuration's fdt.
 		 */
@@ -1795,11 +1803,12 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 	const char *fit_uname_config;
 	const char *fit_base_uname_config;
 	const void *fit;
-	const void *buf;
+	void *buf;
+	void *loadbuf;
 	size_t size;
 	int type_ok, os_ok;
-	ulong load, data, len;
-	uint8_t os;
+	ulong load, load_end, data, len;
+	uint8_t os, comp;
 #ifndef USE_HOSTCC
 	uint8_t os_arch;
 #endif
@@ -1895,12 +1904,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 	images->os.arch = os_arch;
 #endif
 
-	if (image_type == IH_TYPE_FLATDT &&
-	    !fit_image_check_comp(fit, noffset, IH_COMP_NONE)) {
-		puts("FDT image is compressed");
-		return -EPROTONOSUPPORT;
-	}
-
 	bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL);
 	type_ok = fit_image_check_type(fit, noffset, image_type) ||
 		  fit_image_check_type(fit, noffset, IH_TYPE_FIRMWARE) ||
@@ -1931,7 +1934,8 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 	bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL_OK);
 
 	/* get image data address and length */
-	if (fit_image_get_data_and_size(fit, noffset, &buf, &size)) {
+	if (fit_image_get_data_and_size(fit, noffset,
+					(const void **)&buf, &size)) {
 		printf("Could not find %s subimage data!\n", prop_name);
 		bootstage_error(bootstage_id + BOOTSTAGE_SUB_GET_DATA);
 		return -ENOENT;
@@ -1939,30 +1943,15 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 
 #if !defined(USE_HOSTCC) && defined(CONFIG_FIT_IMAGE_POST_PROCESS)
 	/* perform any post-processing on the image data */
-	board_fit_image_post_process((void **)&buf, &size);
+	board_fit_image_post_process(&buf, &size);
 #endif
 
 	len = (ulong)size;
 
-	/* verify that image data is a proper FDT blob */
-	if (image_type == IH_TYPE_FLATDT && fdt_check_header(buf)) {
-		puts("Subimage data is not a FDT");
-		return -ENOEXEC;
-	}
-
 	bootstage_mark(bootstage_id + BOOTSTAGE_SUB_GET_DATA_OK);
 
-	/*
-	 * Work-around for eldk-4.2 which gives this warning if we try to
-	 * cast in the unmap_sysmem() call:
-	 * warning: initialization discards qualifiers from pointer target type
-	 */
-	{
-		void *vbuf = (void *)buf;
-
-		data = map_to_sysmem(vbuf);
-	}
-
+	data = map_to_sysmem(buf);
+	load = data;
 	if (load_op == FIT_LOAD_IGNORED) {
 		/* Don't load */
 	} else if (fit_image_get_load(fit, noffset, &load)) {
@@ -1974,8 +1963,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 		}
 	} else if (load_op != FIT_LOAD_OPTIONAL_NON_ZERO || load) {
 		ulong image_start, image_end;
-		ulong load_end;
-		void *dst;
 
 		/*
 		 * move image data to the load address,
@@ -1993,14 +1980,45 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
 
 		printf("   Loading %s from 0x%08lx to 0x%08lx\n",
 		       prop_name, data, load);
+	} else {
+		load = data;	/* No load address specified */
+	}
+
+	comp = IH_COMP_NONE;
+	loadbuf = buf;
+	/* Kernel images get decompressed later in bootm_load_os(). */
+	if (!(image_type == IH_TYPE_KERNEL ||
+	      image_type == IH_TYPE_KERNEL_NOLOAD) &&
+	    !fit_image_get_comp(fit, noffset, &comp) &&
+	    comp != IH_COMP_NONE) {
+		ulong max_decomp_len = len * 20;
+		if (load == data) {
+			loadbuf = malloc(max_decomp_len);
+			load = map_to_sysmem(loadbuf);
+		} else {
+			loadbuf = map_sysmem(load, max_decomp_len);
+		}
+		if (image_decomp(comp, load, data, image_type,
+				loadbuf, buf, len, max_decomp_len, &load_end)) {
+			printf("Error decompressing %s\n", prop_name);
 
-		dst = map_sysmem(load, len);
-		memmove(dst, buf, len);
-		data = load;
+			return -ENOEXEC;
+		}
+		len = load_end - load;
+	} else if (load != data) {
+		loadbuf = map_sysmem(load, len);
+		memcpy(loadbuf, buf, len);
 	}
+
+	/* verify that image data is a proper FDT blob */
+	if (image_type == IH_TYPE_FLATDT && fdt_check_header(loadbuf)) {
+		puts("Subimage data is not a FDT");
+		return -ENOEXEC;
+	}
+
 	bootstage_mark(bootstage_id + BOOTSTAGE_SUB_LOAD);
 
-	*datap = data;
+	*datap = load;
 	*lenp = len;
 	if (fit_unamep)
 		*fit_unamep = (char *)fit_uname;
diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py
index 49d6fea571..8009d2907b 100755
--- a/test/py/tests/test_fit.py
+++ b/test/py/tests/test_fit.py
@@ -24,7 +24,7 @@ base_its = '''
                         type = "kernel";
                         arch = "sandbox";
                         os = "linux";
-                        compression = "none";
+                        compression = "%(compression)s";
                         load = <0x40000>;
                         entry = <0x8>;
                 };
@@ -39,11 +39,11 @@ base_its = '''
                 };
                 fdt at 1 {
                         description = "snow";
-                        data = /incbin/("u-boot.dtb");
+                        data = /incbin/("%(fdt)s");
                         type = "flat_dt";
                         arch = "sandbox";
                         %(fdt_load)s
-                        compression = "none";
+                        compression = "%(compression)s";
                         signature at 1 {
                                 algo = "sha1,rsa2048";
                                 key-name-hint = "dev";
@@ -56,7 +56,7 @@ base_its = '''
                         arch = "sandbox";
                         os = "linux";
                         %(ramdisk_load)s
-                        compression = "none";
+                        compression = "%(compression)s";
                 };
                 ramdisk at 2 {
                         description = "snow";
@@ -221,6 +221,10 @@ def test_fit(u_boot_console):
             print(data, file=fd)
         return fname
 
+    def make_compressed(filename):
+        util.run_and_log(cons, ['gzip', '-f', '-k', filename])
+        return filename + '.gz'
+
     def find_matching(text, match):
         """Find a match in a line of text, and return the unmatched line portion
 
@@ -312,6 +316,7 @@ def test_fit(u_boot_console):
         loadables1 = make_kernel('test-loadables1.bin', 'lenrek')
         loadables2 = make_ramdisk('test-loadables2.bin', 'ksidmar')
         kernel_out = make_fname('kernel-out.bin')
+        fdt = make_fname('u-boot.dtb')
         fdt_out = make_fname('fdt-out.dtb')
         ramdisk_out = make_fname('ramdisk-out.bin')
         loadables1_out = make_fname('loadables1-out.bin')
@@ -326,6 +331,7 @@ def test_fit(u_boot_console):
             'kernel_addr' : 0x40000,
             'kernel_size' : filesize(kernel),
 
+            'fdt' : fdt,
             'fdt_out' : fdt_out,
             'fdt_addr' : 0x80000,
             'fdt_size' : filesize(control_dtb),
@@ -351,6 +357,7 @@ def test_fit(u_boot_console):
             'loadables2_load' : '',
 
             'loadables_config' : '',
+            'compression' : 'none',
         }
 
         # Make a basic FIT and a script to load it
@@ -417,6 +424,20 @@ def test_fit(u_boot_console):
             check_equal(loadables2, loadables2_out,
                         'Loadables2 (ramdisk) not loaded')
 
+        # Kernel, FDT and Ramdisk all compressed
+        with cons.log.section('(Kernel + FDT + Ramdisk) compressed'):
+            params['compression'] = 'gzip'
+            params['kernel'] = make_compressed(kernel)
+            params['fdt'] = make_compressed(fdt)
+            params['ramdisk'] = make_compressed(ramdisk)
+            fit = make_fit(mkimage, params)
+            cons.restart_uboot()
+            output = cons.run_command_list(cmd.splitlines())
+            check_equal(kernel, kernel_out, 'Kernel not loaded')
+            check_equal(control_dtb, fdt_out, 'FDT not loaded')
+            check_equal(ramdisk, ramdisk_out, 'Ramdisk not loaded')
+
+
     cons = u_boot_console
     try:
         # We need to use our own device tree file. Remember to restore it
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 3/3] fit: Support compat string property in configuration node
  2019-07-11 20:53 [U-Boot] [PATCH v5 0/3] fit: Image node compression Julius Werner
  2019-07-11 20:53 ` [U-Boot] [PATCH v5 1/3] common: Move bootm_decomp_image() to image.c (as image_decomp()) Julius Werner
  2019-07-11 20:53 ` [U-Boot] [PATCH v5 2/3] fit: Support compression for non-kernel components (e.g. FDT) Julius Werner
@ 2019-07-11 20:53 ` Julius Werner
  2019-07-12  6:10   ` Simon Goldschmidt
  2019-07-12  6:02 ` [U-Boot] [PATCH v5 0/3] fit: Image node compression Simon Goldschmidt
  3 siblings, 1 reply; 15+ messages in thread
From: Julius Werner @ 2019-07-11 20:53 UTC (permalink / raw)
  To: u-boot

This patch adds support for an optional optimization to compatible
string matching where the compatible string property from the root node
of the kernel FDT can be copied into the configuration node of the FIT
image. This is most useful when using compressed FDTs or when using FDT
overlays, where the traditional extraction of the compatible string from
the kernel FDT itself is not easily possible.

Signed-off-by: Julius Werner <jwerner@chromium.org>
---
 - No changes for v2
 - No changes for v3
 - Changes for v4:
   - Added documentation for compatible string in config node.
   - Added example .its file for compressed FDT with compat string in
     config node.
 - No changes for v5

 common/image-fit.c                        | 67 ++++++++++++---------
 doc/uImage.FIT/kernel_fdts_compressed.its | 73 +++++++++++++++++++++++
 doc/uImage.FIT/source_file_format.txt     |  7 +++
 3 files changed, 119 insertions(+), 28 deletions(-)
 create mode 100644 doc/uImage.FIT/kernel_fdts_compressed.its

diff --git a/common/image-fit.c b/common/image-fit.c
index c9ffc441aa..e346fed550 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1522,6 +1522,10 @@ int fit_check_format(const void *fit)
  * compatible list, "foo,bar", matches a compatible string in the root of fdt1.
  * "bim,bam" in fdt2 matches the second string which isn't as good as fdt1.
  *
+ * As an optimization, the compatible property from the FDT's root node can be
+ * copied into the configuration node in the FIT image. This is required to
+ * match configurations with compressed FDTs.
+ *
  * returns:
  *     offset to the configuration to use if one was found
  *     -1 otherwise
@@ -1554,55 +1558,62 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
 	for (noffset = fdt_next_node(fit, confs_noffset, &ndepth);
 			(noffset >= 0) && (ndepth > 0);
 			noffset = fdt_next_node(fit, noffset, &ndepth)) {
-		const void *kfdt;
+		const void *fdt;
 		const char *kfdt_name;
-		int kfdt_noffset;
+		int kfdt_noffset, compat_noffset;
 		const char *cur_fdt_compat;
 		int len;
-		size_t size;
+		size_t sz;
 		int i;
 
 		if (ndepth > 1)
 			continue;
 
-		kfdt_name = fdt_getprop(fit, noffset, "fdt", &len);
-		if (!kfdt_name) {
-			debug("No fdt property found.\n");
-			continue;
-		}
-		kfdt_noffset = fdt_subnode_offset(fit, images_noffset,
-						  kfdt_name);
-		if (kfdt_noffset < 0) {
-			debug("No image node named \"%s\" found.\n",
-			      kfdt_name);
-			continue;
-		}
+		/* If there's a compat property in the config node, use that. */
+		if (fdt_getprop(fit, noffset, "compatible", NULL)) {
+			fdt = fit;		  /* search in FIT image */
+			compat_noffset = noffset; /* search under config node */
+		} else {	/* Otherwise extract it from the kernel FDT. */
+			kfdt_name = fdt_getprop(fit, noffset, "fdt", &len);
+			if (!kfdt_name) {
+				debug("No fdt property found.\n");
+				continue;
+			}
+			kfdt_noffset = fdt_subnode_offset(fit, images_noffset,
+							  kfdt_name);
+			if (kfdt_noffset < 0) {
+				debug("No image node named \"%s\" found.\n",
+				      kfdt_name);
+				continue;
+			}
 
-		if (!fit_image_check_comp(fit, kfdt_noffset, IH_COMP_NONE)) {
-			debug("Can't extract compat from \"%s\" (compressed)\n",
-			      kfdt_name);
-			continue;
-		}
+			if (!fit_image_check_comp(fit, kfdt_noffset,
+						  IH_COMP_NONE)) {
+				debug("Can't extract compat from \"%s\" "
+				      "(compressed)\n", kfdt_name);
+				continue;
+			}
 
-		/*
-		 * Get a pointer to this configuration's fdt.
-		 */
-		if (fit_image_get_data(fit, kfdt_noffset, &kfdt, &size)) {
-			debug("Failed to get fdt \"%s\".\n", kfdt_name);
-			continue;
+			/* search in this config's kernel FDT */
+			if (fit_image_get_data(fit, kfdt_noffset, &fdt, &sz)) {
+				debug("Failed to get fdt \"%s\".\n", kfdt_name);
+				continue;
+			}
+
+			compat_noffset = 0;  /* search kFDT under root node */
 		}
 
 		len = fdt_compat_len;
 		cur_fdt_compat = fdt_compat;
 		/*
 		 * Look for a match for each U-Boot compatibility string in
-		 * turn in this configuration's fdt.
+		 * turn in the compat string property.
 		 */
 		for (i = 0; len > 0 &&
 		     (!best_match_offset || best_match_pos > i); i++) {
 			int cur_len = strlen(cur_fdt_compat) + 1;
 
-			if (!fdt_node_check_compatible(kfdt, 0,
+			if (!fdt_node_check_compatible(fdt, compat_noffset,
 						       cur_fdt_compat)) {
 				best_match_offset = noffset;
 				best_match_pos = i;
diff --git a/doc/uImage.FIT/kernel_fdts_compressed.its b/doc/uImage.FIT/kernel_fdts_compressed.its
new file mode 100644
index 0000000000..8f81106efc
--- /dev/null
+++ b/doc/uImage.FIT/kernel_fdts_compressed.its
@@ -0,0 +1,73 @@
+/*
+ * U-Boot uImage source file with a kernel and multiple compressed FDT blobs.
+ * Since the FDTs are compressed, configurations must provide a compatible
+ * string to match directly.
+ */
+
+/dts-v1/;
+
+/ {
+	description = "Image with single Linux kernel and compressed FDT blobs";
+	#address-cells = <1>;
+
+	images {
+		kernel {
+			description = "Vanilla Linux kernel";
+			data = /incbin/("./vmlinux.bin.gz");
+			type = "kernel";
+			arch = "ppc";
+			os = "linux";
+			compression = "gzip";
+			load = <00000000>;
+			entry = <00000000>;
+			hash-1 {
+				algo = "crc32";
+			};
+			hash-2 {
+				algo = "sha1";
+			};
+		};
+		fdt at 1 {
+			description = "Flattened Device Tree blob 1";
+			data = /incbin/("./myboard-var1.dtb");
+			type = "flat_dt";
+			arch = "ppc";
+			compression = "gzip";
+			hash-1 {
+				algo = "crc32";
+			};
+			hash-2 {
+				algo = "sha1";
+			};
+		};
+		fdt at 2 {
+			description = "Flattened Device Tree blob 2";
+			data = /incbin/("./myboard-var2.dtb");
+			type = "flat_dt";
+			arch = "ppc";
+			compression = "lzma";
+			hash-1 {
+				algo = "crc32";
+			};
+			hash-2 {
+				algo = "sha1";
+			};
+		};
+	};
+
+	configurations {
+		default = "conf at 1";
+		conf at 1 {
+			description = "Boot Linux kernel with FDT blob 1";
+			kernel = "kernel";
+			fdt = "fdt at 1";
+			compatible = "myvendor,myboard-variant1";
+		};
+		conf at 2 {
+			description = "Boot Linux kernel with FDT blob 2";
+			kernel = "kernel";
+			fdt = "fdt at 2";
+			compatible = "myvendor,myboard-variant2";
+		};
+	};
+};
diff --git a/doc/uImage.FIT/source_file_format.txt b/doc/uImage.FIT/source_file_format.txt
index d701b9bb76..f8e27ed34e 100644
--- a/doc/uImage.FIT/source_file_format.txt
+++ b/doc/uImage.FIT/source_file_format.txt
@@ -240,6 +240,7 @@ o config-1
   |- fdt = "fdt sub-node unit-name" [, "fdt overlay sub-node unit-name", ...]
   |- fpga = "fpga sub-node unit-name"
   |- loadables = "loadables sub-node unit-name"
+  |- compatible = "vendor,board-style device tree compatible string"
 
 
   Mandatory properties:
@@ -263,6 +264,12 @@ o config-1
     of strings. U-Boot will load each binary at its given start-address and
     may optionaly invoke additional post-processing steps on this binary based
     on its component image node type.
+  - compatible : The root compatible string of the U-Boot device tree that
+    this configuration shall automatically match when CONFIG_FIT_BEST_MATCH is
+    enabled. If this property is not provided, the compatible string will be
+    extracted from the fdt blob instead. This is only possible if the fdt is
+    not compressed, so images with compressed fdts that want to use compatible
+    string matching must always provide this property.
 
 The FDT blob is required to properly boot FDT based kernel, so the minimal
 configuration for 2.6 FDT kernel is (kernel, fdt) pair.
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 0/3] fit: Image node compression
  2019-07-11 20:53 [U-Boot] [PATCH v5 0/3] fit: Image node compression Julius Werner
                   ` (2 preceding siblings ...)
  2019-07-11 20:53 ` [U-Boot] [PATCH v5 3/3] fit: Support compat string property in configuration node Julius Werner
@ 2019-07-12  6:02 ` Simon Goldschmidt
  2019-07-12 18:27   ` Julius Werner
  3 siblings, 1 reply; 15+ messages in thread
From: Simon Goldschmidt @ 2019-07-12  6:02 UTC (permalink / raw)
  To: u-boot

On Thu, Jul 11, 2019 at 10:53 PM Julius Werner <jwerner@chromium.org> wrote:
>
> This patch series adds compression support for non-kernel FIT image
> nodes (e.g. FDTs). The first patch is a preparatory refactoring, the
> second adds the compression support itself, and the third adds a new
> feature to compatible string matching that allows it to be useful
> with compressed FDTs.
>
> Sandbox-tested with FIT images with and without compressed FDTs, with
> and without overlays, in both compatible string matching and direct
> config selection modes. Also expanded the test_fit pytest to include a
> case with compressed kernel, FDT and ramdisk.
>
> Julius Werner (3):
>   common: Move bootm_decomp_image() to image.c (as image_decomp())
>     - First version: v5

This kind of interleaved change log is kind of hard to read. A higher-level,
manually written changelog for the cover letter would be better I think.

Regards,
Simon

>   fit: Support compression for non-kernel components (e.g. FDT)
>     - Changes for v2:
>       - Changed from only supporting compressed FDTs to supporting all
>         non-kernel image node types.
>     - Changes for v3:
>       - Fixed up some debug output that was still written for v1.
>       - Fixed a mistake with handling FIT_LOAD_OPTIONAL_NON_ZERO when
>         'load' was 0 (i.e. unset).
>       - Added compression test case to the test_fit pytest.
>     - No changes for v4
>     - No changes for v5
>   fit: Support compat string property in configuration node
>     - No changes for v2
>     - No changes for v3
>     - Changes for v4:
>       - Added documentation for compatible string in config node.
>       - Added example .its file for compressed FDT with compat string in
>         config node.
>     - No changes for v5
>
>  common/bootm.c                            | 148 +++-------------------
>  common/image-fit.c                        | 143 ++++++++++++---------
>  common/image.c                            | 111 ++++++++++++++++
>  doc/uImage.FIT/kernel_fdts_compressed.its |  73 +++++++++++
>  doc/uImage.FIT/source_file_format.txt     |   7 +
>  include/bootm.h                           |  17 ---
>  include/image.h                           |  17 +++
>  test/compression.c                        |  24 ++--
>  test/py/tests/test_fit.py                 |  29 ++++-
>  9 files changed, 351 insertions(+), 218 deletions(-)
>  create mode 100644 doc/uImage.FIT/kernel_fdts_compressed.its
>
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 1/3] common: Move bootm_decomp_image() to image.c (as image_decomp())
  2019-07-11 20:53 ` [U-Boot] [PATCH v5 1/3] common: Move bootm_decomp_image() to image.c (as image_decomp()) Julius Werner
@ 2019-07-12  6:09   ` Simon Goldschmidt
  2019-07-12 18:33     ` Julius Werner
  2019-07-24 18:14   ` Tom Rini
  1 sibling, 1 reply; 15+ messages in thread
From: Simon Goldschmidt @ 2019-07-12  6:09 UTC (permalink / raw)
  To: u-boot

On Thu, Jul 11, 2019 at 10:53 PM Julius Werner <jwerner@chromium.org> wrote:
>
> Upcoming patches want to add decompression to use cases that are no
> longer directly related to booting. It makes sense to retain a single
> decompression routine, but it should no longer be in bootm.c (which is
> not compiled for all configurations). This patch moves
> bootm_decomp_image() to image.c and renames it to image_decomp() in
> preparation of those upcoming patches.

I'd like to review this, but maybe you can help me speed up the process and tell
us if the move has been a 1:1 code move or if you had to adapt things to the new
location (other than the function being renamed)?

Thanks,
Simon

>
> Signed-off-by: Julius Werner <jwerner@chromium.org>
> ---
>  - First version: v5
>
>  common/bootm.c     | 148 ++++++---------------------------------------
>  common/image.c     | 111 ++++++++++++++++++++++++++++++++++
>  include/bootm.h    |  17 ------
>  include/image.h    |  17 ++++++
>  test/compression.c |  24 ++++----
>  5 files changed, 160 insertions(+), 157 deletions(-)
>
> diff --git a/common/bootm.c b/common/bootm.c
> index d193751647..2e64140df6 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -7,17 +7,12 @@
>  #ifndef USE_HOSTCC
>  #include <common.h>
>  #include <bootstage.h>
> -#include <bzlib.h>
>  #include <errno.h>
>  #include <fdt_support.h>
>  #include <lmb.h>
>  #include <malloc.h>
>  #include <mapmem.h>
>  #include <asm/io.h>
> -#include <linux/lzo.h>
> -#include <lzma/LzmaTypes.h>
> -#include <lzma/LzmaDec.h>
> -#include <lzma/LzmaTools.h>
>  #if defined(CONFIG_CMD_USB)
>  #include <usb.h>
>  #endif
> @@ -299,23 +294,6 @@ static int bootm_find_other(cmd_tbl_t *cmdtp, int flag, int argc,
>  }
>  #endif /* USE_HOSTC */
>
> -/**
> - * print_decomp_msg() - Print a suitable decompression/loading message
> - *
> - * @type:      OS type (IH_OS_...)
> - * @comp_type: Compression type being used (IH_COMP_...)
> - * @is_xip:    true if the load address matches the image start
> - */
> -static void print_decomp_msg(int comp_type, int type, bool is_xip)
> -{
> -       const char *name = genimg_get_type_name(type);
> -
> -       if (comp_type == IH_COMP_NONE)
> -               printf("   %s %s ... ", is_xip ? "XIP" : "Loading", name);
> -       else
> -               printf("   Uncompressing %s ... ", name);
> -}
> -
>  /**
>   * handle_decomp_error() - display a decompression error
>   *
> @@ -325,16 +303,18 @@ static void print_decomp_msg(int comp_type, int type, bool is_xip)
>   *
>   * @comp_type:         Compression type being used (IH_COMP_...)
>   * @uncomp_size:       Number of bytes uncompressed
> - * @unc_len:           Amount of space available for decompression
> - * @ret:               Error code to report
> - * @return BOOTM_ERR_RESET, indicating that the board must be reset
> + * @ret:               errno error code received from compression library
> + * @return Appropriate BOOTM_ERR_ error code
>   */
> -static int handle_decomp_error(int comp_type, size_t uncomp_size,
> -                              size_t unc_len, int ret)
> +static int handle_decomp_error(int comp_type, size_t uncomp_size, int ret)
>  {
>         const char *name = genimg_get_comp_name(comp_type);
>
> -       if (uncomp_size >= unc_len)
> +       /* ENOSYS means unimplemented compression type, don't reset. */
> +       if (ret == -ENOSYS)
> +               return BOOTM_ERR_UNIMPLEMENTED;
> +
> +       if (uncomp_size >= CONFIG_SYS_BOOTM_LEN)
>                 printf("Image too large: increase CONFIG_SYS_BOOTM_LEN\n");
>         else
>                 printf("%s: uncompress error %d\n", name, ret);
> @@ -352,93 +332,6 @@ static int handle_decomp_error(int comp_type, size_t uncomp_size,
>         return BOOTM_ERR_RESET;
>  }
>
> -int bootm_decomp_image(int comp, ulong load, ulong image_start, int type,
> -                      void *load_buf, void *image_buf, ulong image_len,
> -                      uint unc_len, ulong *load_end)
> -{
> -       int ret = 0;
> -
> -       *load_end = load;
> -       print_decomp_msg(comp, type, load == image_start);
> -
> -       /*
> -        * Load the image to the right place, decompressing if needed. After
> -        * this, image_len will be set to the number of uncompressed bytes
> -        * loaded, ret will be non-zero on error.
> -        */
> -       switch (comp) {
> -       case IH_COMP_NONE:
> -               if (load == image_start)
> -                       break;
> -               if (image_len <= unc_len)
> -                       memmove_wd(load_buf, image_buf, image_len, CHUNKSZ);
> -               else
> -                       ret = 1;
> -               break;
> -#ifdef CONFIG_GZIP
> -       case IH_COMP_GZIP: {
> -               ret = gunzip(load_buf, unc_len, image_buf, &image_len);
> -               break;
> -       }
> -#endif /* CONFIG_GZIP */
> -#ifdef CONFIG_BZIP2
> -       case IH_COMP_BZIP2: {
> -               uint size = unc_len;
> -
> -               /*
> -                * If we've got less than 4 MB of malloc() space,
> -                * use slower decompression algorithm which requires
> -                * at most 2300 KB of memory.
> -                */
> -               ret = BZ2_bzBuffToBuffDecompress(load_buf, &size,
> -                       image_buf, image_len,
> -                       CONFIG_SYS_MALLOC_LEN < (4096 * 1024), 0);
> -               image_len = size;
> -               break;
> -       }
> -#endif /* CONFIG_BZIP2 */
> -#ifdef CONFIG_LZMA
> -       case IH_COMP_LZMA: {
> -               SizeT lzma_len = unc_len;
> -
> -               ret = lzmaBuffToBuffDecompress(load_buf, &lzma_len,
> -                                              image_buf, image_len);
> -               image_len = lzma_len;
> -               break;
> -       }
> -#endif /* CONFIG_LZMA */
> -#ifdef CONFIG_LZO
> -       case IH_COMP_LZO: {
> -               size_t size = unc_len;
> -
> -               ret = lzop_decompress(image_buf, image_len, load_buf, &size);
> -               image_len = size;
> -               break;
> -       }
> -#endif /* CONFIG_LZO */
> -#ifdef CONFIG_LZ4
> -       case IH_COMP_LZ4: {
> -               size_t size = unc_len;
> -
> -               ret = ulz4fn(image_buf, image_len, load_buf, &size);
> -               image_len = size;
> -               break;
> -       }
> -#endif /* CONFIG_LZ4 */
> -       default:
> -               printf("Unimplemented compression type %d\n", comp);
> -               return BOOTM_ERR_UNIMPLEMENTED;
> -       }
> -
> -       if (ret)
> -               return handle_decomp_error(comp, image_len, unc_len, ret);
> -       *load_end = load + image_len;
> -
> -       puts("OK\n");
> -
> -       return 0;
> -}
> -
>  #ifndef USE_HOSTCC
>  static int bootm_load_os(bootm_headers_t *images, int boot_progress)
>  {
> @@ -456,10 +349,11 @@ static int bootm_load_os(bootm_headers_t *images, int boot_progress)
>
>         load_buf = map_sysmem(load, 0);
>         image_buf = map_sysmem(os.image_start, image_len);
> -       err = bootm_decomp_image(os.comp, load, os.image_start, os.type,
> -                                load_buf, image_buf, image_len,
> -                                CONFIG_SYS_BOOTM_LEN, &load_end);
> +       err = image_decomp(os.comp, load, os.image_start, os.type,
> +                          load_buf, image_buf, image_len,
> +                          CONFIG_SYS_BOOTM_LEN, &load_end);
>         if (err) {
> +               err = handle_decomp_error(os.comp, load_end - load, err);
>                 bootstage_error(BOOTSTAGE_ID_DECOMP_IMAGE);
>                 return err;
>         }
> @@ -919,11 +813,6 @@ void __weak switch_to_non_secure_mode(void)
>
>  #else /* USE_HOSTCC */
>
> -void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
> -{
> -       memmove(to, from, len);
> -}
> -
>  #if defined(CONFIG_FIT_SIGNATURE)
>  static int bootm_host_load_image(const void *fit, int req_image_type)
>  {
> @@ -957,13 +846,16 @@ static int bootm_host_load_image(const void *fit, int req_image_type)
>
>         /* Allow the image to expand by a factor of 4, should be safe */
>         load_buf = malloc((1 << 20) + len * 4);
> -       ret = bootm_decomp_image(imape_comp, 0, data, image_type, load_buf,
> -                                (void *)data, len, CONFIG_SYS_BOOTM_LEN,
> -                                &load_end);
> +       ret = image_decomp(imape_comp, 0, data, image_type, load_buf,
> +                          (void *)data, len, CONFIG_SYS_BOOTM_LEN,
> +                          &load_end);
>         free(load_buf);
>
> -       if (ret && ret != BOOTM_ERR_UNIMPLEMENTED)
> -               return ret;
> +       if (ret) {
> +               ret = handle_decomp_error(imape_comp, load_end - 0, ret);
> +               if (ret != BOOTM_ERR_UNIMPLEMENTED)
> +                       return ret;
> +       }
>
>         return 0;
>  }
> diff --git a/common/image.c b/common/image.c
> index 75b84d5009..04da002fcc 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -32,6 +32,12 @@
>  #include <linux/errno.h>
>  #include <asm/io.h>
>
> +#include <bzlib.h>
> +#include <linux/lzo.h>
> +#include <lzma/LzmaTypes.h>
> +#include <lzma/LzmaDec.h>
> +#include <lzma/LzmaTools.h>
> +
>  #ifdef CONFIG_CMD_BDI
>  extern int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
>  #endif
> @@ -375,6 +381,106 @@ void image_print_contents(const void *ptr)
>         }
>  }
>
> +/**
> + * print_decomp_msg() - Print a suitable decompression/loading message
> + *
> + * @type:      OS type (IH_OS_...)
> + * @comp_type: Compression type being used (IH_COMP_...)
> + * @is_xip:    true if the load address matches the image start
> + */
> +static void print_decomp_msg(int comp_type, int type, bool is_xip)
> +{
> +       const char *name = genimg_get_type_name(type);
> +
> +       if (comp_type == IH_COMP_NONE)
> +               printf("   %s %s ... ", is_xip ? "XIP" : "Loading", name);
> +       else
> +               printf("   Uncompressing %s ... ", name);
> +}
> +
> +int image_decomp(int comp, ulong load, ulong image_start, int type,
> +                void *load_buf, void *image_buf, ulong image_len,
> +                uint unc_len, ulong *load_end)
> +{
> +       int ret = 0;
> +
> +       *load_end = load;
> +       print_decomp_msg(comp, type, load == image_start);
> +
> +       /*
> +        * Load the image to the right place, decompressing if needed. After
> +        * this, image_len will be set to the number of uncompressed bytes
> +        * loaded, ret will be non-zero on error.
> +        */
> +       switch (comp) {
> +       case IH_COMP_NONE:
> +               if (load == image_start)
> +                       break;
> +               if (image_len <= unc_len)
> +                       memmove_wd(load_buf, image_buf, image_len, CHUNKSZ);
> +               else
> +                       ret = -ENOSPC;
> +               break;
> +#ifdef CONFIG_GZIP
> +       case IH_COMP_GZIP: {
> +               ret = gunzip(load_buf, unc_len, image_buf, &image_len);
> +               break;
> +       }
> +#endif /* CONFIG_GZIP */
> +#ifdef CONFIG_BZIP2
> +       case IH_COMP_BZIP2: {
> +               uint size = unc_len;
> +
> +               /*
> +                * If we've got less than 4 MB of malloc() space,
> +                * use slower decompression algorithm which requires
> +                * at most 2300 KB of memory.
> +                */
> +               ret = BZ2_bzBuffToBuffDecompress(load_buf, &size,
> +                       image_buf, image_len,
> +                       CONFIG_SYS_MALLOC_LEN < (4096 * 1024), 0);
> +               image_len = size;
> +               break;
> +       }
> +#endif /* CONFIG_BZIP2 */
> +#ifdef CONFIG_LZMA
> +       case IH_COMP_LZMA: {
> +               SizeT lzma_len = unc_len;
> +
> +               ret = lzmaBuffToBuffDecompress(load_buf, &lzma_len,
> +                                              image_buf, image_len);
> +               image_len = lzma_len;
> +               break;
> +       }
> +#endif /* CONFIG_LZMA */
> +#ifdef CONFIG_LZO
> +       case IH_COMP_LZO: {
> +               size_t size = unc_len;
> +
> +               ret = lzop_decompress(image_buf, image_len, load_buf, &size);
> +               image_len = size;
> +               break;
> +       }
> +#endif /* CONFIG_LZO */
> +#ifdef CONFIG_LZ4
> +       case IH_COMP_LZ4: {
> +               size_t size = unc_len;
> +
> +               ret = ulz4fn(image_buf, image_len, load_buf, &size);
> +               image_len = size;
> +               break;
> +       }
> +#endif /* CONFIG_LZ4 */
> +       default:
> +               printf("Unimplemented compression type %d\n", comp);
> +               return -ENOSYS;
> +       }
> +
> +       *load_end = load + image_len;
> +
> +       return ret;
> +}
> +
>
>  #ifndef USE_HOSTCC
>  #if defined(CONFIG_IMAGE_FORMAT_LEGACY)
> @@ -551,6 +657,11 @@ void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
>         memmove(to, from, len);
>  #endif /* CONFIG_HW_WATCHDOG || CONFIG_WATCHDOG */
>  }
> +#else  /* USE_HOSTCC */
> +void memmove_wd(void *to, void *from, size_t len, ulong chunksz)
> +{
> +       memmove(to, from, len);
> +}
>  #endif /* !USE_HOSTCC */
>
>  void genimg_print_size(uint32_t size)
> diff --git a/include/bootm.h b/include/bootm.h
> index f771b733f5..edeeacb0df 100644
> --- a/include/bootm.h
> +++ b/include/bootm.h
> @@ -59,23 +59,6 @@ int do_bootm_states(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[],
>
>  void arch_preboot_os(void);
>
> -/**
> - * bootm_decomp_image() - decompress the operating system
> - *
> - * @comp:      Compression algorithm that is used (IH_COMP_...)
> - * @load:      Destination load address in U-Boot memory
> - * @image_start Image start address (where we are decompressing from)
> - * @type:      OS type (IH_OS_...)
> - * @load_bug:  Place to decompress to
> - * @image_buf: Address to decompress from
> - * @image_len: Number of bytes in @image_buf to decompress
> - * @unc_len:   Available space for decompression
> - * @return 0 if OK, -ve on error (BOOTM_ERR_...)
> - */
> -int bootm_decomp_image(int comp, ulong load, ulong image_start, int type,
> -                      void *load_buf, void *image_buf, ulong image_len,
> -                      uint unc_len, ulong *load_end);
> -
>  /*
>   * boards should define this to disable devices when EFI exits from boot
>   * services.
> diff --git a/include/image.h b/include/image.h
> index bb7089ef5d..3bdb9f0a97 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -849,6 +849,23 @@ static inline int image_check_target_arch(const image_header_t *hdr)
>  }
>  #endif /* USE_HOSTCC */
>
> +/**
> + * image_decomp() - decompress an image
> + *
> + * @comp:      Compression algorithm that is used (IH_COMP_...)
> + * @load:      Destination load address in U-Boot memory
> + * @image_start Image start address (where we are decompressing from)
> + * @type:      OS type (IH_OS_...)
> + * @load_bug:  Place to decompress to
> + * @image_buf: Address to decompress from
> + * @image_len: Number of bytes in @image_buf to decompress
> + * @unc_len:   Available space for decompression
> + * @return 0 if OK, -ve on error (BOOTM_ERR_...)
> + */
> +int image_decomp(int comp, ulong load, ulong image_start, int type,
> +                void *load_buf, void *image_buf, ulong image_len,
> +                uint unc_len, ulong *load_end);
> +
>  /**
>   * Set up properties in the FDT
>   *
> diff --git a/test/compression.c b/test/compression.c
> index 7bc0f73e09..dc5e94684f 100644
> --- a/test/compression.c
> +++ b/test/compression.c
> @@ -471,15 +471,15 @@ static int run_bootm_test(struct unit_test_state *uts, int comp_type,
>         unc_len = strlen(plain);
>         compress(uts, (void *)plain, unc_len, compress_buff, compress_size,
>                  &compress_size);
> -       err = bootm_decomp_image(comp_type, load_addr, image_start,
> -                                IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
> -                                compress_buff, compress_size, unc_len,
> -                                &load_end);
> +       err = image_decomp(comp_type, load_addr, image_start,
> +                          IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
> +                          compress_buff, compress_size, unc_len,
> +                          &load_end);
>         ut_assertok(err);
> -       err = bootm_decomp_image(comp_type, load_addr, image_start,
> -                                IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
> -                                compress_buff, compress_size, unc_len - 1,
> -                                &load_end);
> +       err = image_decomp(comp_type, load_addr, image_start,
> +                          IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
> +                          compress_buff, compress_size, unc_len - 1,
> +                          &load_end);
>         ut_assert(err);
>
>         /* We can't detect corruption when not decompressing */
> @@ -487,10 +487,10 @@ static int run_bootm_test(struct unit_test_state *uts, int comp_type,
>                 return 0;
>         memset(compress_buff + compress_size / 2, '\x49',
>                compress_size / 2);
> -       err = bootm_decomp_image(comp_type, load_addr, image_start,
> -                                IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
> -                                compress_buff, compress_size, 0x10000,
> -                                &load_end);
> +       err = image_decomp(comp_type, load_addr, image_start,
> +                          IH_TYPE_KERNEL, map_sysmem(load_addr, 0),
> +                          compress_buff, compress_size, 0x10000,
> +                          &load_end);
>         ut_assert(err);
>
>         return 0;
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/3] fit: Support compression for non-kernel components (e.g. FDT)
  2019-07-11 20:53 ` [U-Boot] [PATCH v5 2/3] fit: Support compression for non-kernel components (e.g. FDT) Julius Werner
@ 2019-07-12  6:10   ` Simon Goldschmidt
  2019-07-12 18:25     ` Julius Werner
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Goldschmidt @ 2019-07-12  6:10 UTC (permalink / raw)
  To: u-boot

On Thu, Jul 11, 2019 at 10:53 PM Julius Werner <jwerner@chromium.org> wrote:
>
> This patch adds support for compressing non-kernel image nodes in a FIT
> image (kernel nodes could already be compressed previously). This can
> reduce the size of FIT images and therefore improve boot times
> (especially when an image bundles many different kernel FDTs). The
> images will automatically be decompressed on load.
>
> This patch does not support extracting compatible strings from
> compressed FDTs, so it's not very helpful in conjunction with
> CONFIG_FIT_BEST_MATCH yet, but it can already be used in environments
> that select the configuration to load explicitly.
>
> Signed-off-by: Julius Werner <jwerner@chromium.org>
> ---
>  - Changes for v2:
>    - Changed from only supporting compressed FDTs to supporting all
>      non-kernel image node types.
>  - Changes for v3:
>    - Fixed up some debug output that was still written for v1.
>    - Fixed a mistake with handling FIT_LOAD_OPTIONAL_NON_ZERO when
>      'load' was 0 (i.e. unset).
>    - Added compression test case to the test_fit pytest.
>  - No changes for v4
>  - No changes for v5

If there haven't been any changes from v4, why did you drop Simon Glass's
Reviewed-by tag?

Regards,
Simon

>
>  common/image-fit.c        | 86 +++++++++++++++++++++++----------------
>  test/py/tests/test_fit.py | 29 +++++++++++--
>  2 files changed, 77 insertions(+), 38 deletions(-)
>
> diff --git a/common/image-fit.c b/common/image-fit.c
> index a74b44f298..c9ffc441aa 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -22,6 +22,7 @@
>  DECLARE_GLOBAL_DATA_PTR;
>  #endif /* !USE_HOSTCC*/
>
> +#include <bootm.h>
>  #include <image.h>
>  #include <bootstage.h>
>  #include <u-boot/crc.h>
> @@ -1576,6 +1577,13 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
>                               kfdt_name);
>                         continue;
>                 }
> +
> +               if (!fit_image_check_comp(fit, kfdt_noffset, IH_COMP_NONE)) {
> +                       debug("Can't extract compat from \"%s\" (compressed)\n",
> +                             kfdt_name);
> +                       continue;
> +               }
> +
>                 /*
>                  * Get a pointer to this configuration's fdt.
>                  */
> @@ -1795,11 +1803,12 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>         const char *fit_uname_config;
>         const char *fit_base_uname_config;
>         const void *fit;
> -       const void *buf;
> +       void *buf;
> +       void *loadbuf;
>         size_t size;
>         int type_ok, os_ok;
> -       ulong load, data, len;
> -       uint8_t os;
> +       ulong load, load_end, data, len;
> +       uint8_t os, comp;
>  #ifndef USE_HOSTCC
>         uint8_t os_arch;
>  #endif
> @@ -1895,12 +1904,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>         images->os.arch = os_arch;
>  #endif
>
> -       if (image_type == IH_TYPE_FLATDT &&
> -           !fit_image_check_comp(fit, noffset, IH_COMP_NONE)) {
> -               puts("FDT image is compressed");
> -               return -EPROTONOSUPPORT;
> -       }
> -
>         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL);
>         type_ok = fit_image_check_type(fit, noffset, image_type) ||
>                   fit_image_check_type(fit, noffset, IH_TYPE_FIRMWARE) ||
> @@ -1931,7 +1934,8 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL_OK);
>
>         /* get image data address and length */
> -       if (fit_image_get_data_and_size(fit, noffset, &buf, &size)) {
> +       if (fit_image_get_data_and_size(fit, noffset,
> +                                       (const void **)&buf, &size)) {
>                 printf("Could not find %s subimage data!\n", prop_name);
>                 bootstage_error(bootstage_id + BOOTSTAGE_SUB_GET_DATA);
>                 return -ENOENT;
> @@ -1939,30 +1943,15 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>
>  #if !defined(USE_HOSTCC) && defined(CONFIG_FIT_IMAGE_POST_PROCESS)
>         /* perform any post-processing on the image data */
> -       board_fit_image_post_process((void **)&buf, &size);
> +       board_fit_image_post_process(&buf, &size);
>  #endif
>
>         len = (ulong)size;
>
> -       /* verify that image data is a proper FDT blob */
> -       if (image_type == IH_TYPE_FLATDT && fdt_check_header(buf)) {
> -               puts("Subimage data is not a FDT");
> -               return -ENOEXEC;
> -       }
> -
>         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_GET_DATA_OK);
>
> -       /*
> -        * Work-around for eldk-4.2 which gives this warning if we try to
> -        * cast in the unmap_sysmem() call:
> -        * warning: initialization discards qualifiers from pointer target type
> -        */
> -       {
> -               void *vbuf = (void *)buf;
> -
> -               data = map_to_sysmem(vbuf);
> -       }
> -
> +       data = map_to_sysmem(buf);
> +       load = data;
>         if (load_op == FIT_LOAD_IGNORED) {
>                 /* Don't load */
>         } else if (fit_image_get_load(fit, noffset, &load)) {
> @@ -1974,8 +1963,6 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>                 }
>         } else if (load_op != FIT_LOAD_OPTIONAL_NON_ZERO || load) {
>                 ulong image_start, image_end;
> -               ulong load_end;
> -               void *dst;
>
>                 /*
>                  * move image data to the load address,
> @@ -1993,14 +1980,45 @@ int fit_image_load(bootm_headers_t *images, ulong addr,
>
>                 printf("   Loading %s from 0x%08lx to 0x%08lx\n",
>                        prop_name, data, load);
> +       } else {
> +               load = data;    /* No load address specified */
> +       }
> +
> +       comp = IH_COMP_NONE;
> +       loadbuf = buf;
> +       /* Kernel images get decompressed later in bootm_load_os(). */
> +       if (!(image_type == IH_TYPE_KERNEL ||
> +             image_type == IH_TYPE_KERNEL_NOLOAD) &&
> +           !fit_image_get_comp(fit, noffset, &comp) &&
> +           comp != IH_COMP_NONE) {
> +               ulong max_decomp_len = len * 20;
> +               if (load == data) {
> +                       loadbuf = malloc(max_decomp_len);
> +                       load = map_to_sysmem(loadbuf);
> +               } else {
> +                       loadbuf = map_sysmem(load, max_decomp_len);
> +               }
> +               if (image_decomp(comp, load, data, image_type,
> +                               loadbuf, buf, len, max_decomp_len, &load_end)) {
> +                       printf("Error decompressing %s\n", prop_name);
>
> -               dst = map_sysmem(load, len);
> -               memmove(dst, buf, len);
> -               data = load;
> +                       return -ENOEXEC;
> +               }
> +               len = load_end - load;
> +       } else if (load != data) {
> +               loadbuf = map_sysmem(load, len);
> +               memcpy(loadbuf, buf, len);
>         }
> +
> +       /* verify that image data is a proper FDT blob */
> +       if (image_type == IH_TYPE_FLATDT && fdt_check_header(loadbuf)) {
> +               puts("Subimage data is not a FDT");
> +               return -ENOEXEC;
> +       }
> +
>         bootstage_mark(bootstage_id + BOOTSTAGE_SUB_LOAD);
>
> -       *datap = data;
> +       *datap = load;
>         *lenp = len;
>         if (fit_unamep)
>                 *fit_unamep = (char *)fit_uname;
> diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py
> index 49d6fea571..8009d2907b 100755
> --- a/test/py/tests/test_fit.py
> +++ b/test/py/tests/test_fit.py
> @@ -24,7 +24,7 @@ base_its = '''
>                          type = "kernel";
>                          arch = "sandbox";
>                          os = "linux";
> -                        compression = "none";
> +                        compression = "%(compression)s";
>                          load = <0x40000>;
>                          entry = <0x8>;
>                  };
> @@ -39,11 +39,11 @@ base_its = '''
>                  };
>                  fdt at 1 {
>                          description = "snow";
> -                        data = /incbin/("u-boot.dtb");
> +                        data = /incbin/("%(fdt)s");
>                          type = "flat_dt";
>                          arch = "sandbox";
>                          %(fdt_load)s
> -                        compression = "none";
> +                        compression = "%(compression)s";
>                          signature at 1 {
>                                  algo = "sha1,rsa2048";
>                                  key-name-hint = "dev";
> @@ -56,7 +56,7 @@ base_its = '''
>                          arch = "sandbox";
>                          os = "linux";
>                          %(ramdisk_load)s
> -                        compression = "none";
> +                        compression = "%(compression)s";
>                  };
>                  ramdisk at 2 {
>                          description = "snow";
> @@ -221,6 +221,10 @@ def test_fit(u_boot_console):
>              print(data, file=fd)
>          return fname
>
> +    def make_compressed(filename):
> +        util.run_and_log(cons, ['gzip', '-f', '-k', filename])
> +        return filename + '.gz'
> +
>      def find_matching(text, match):
>          """Find a match in a line of text, and return the unmatched line portion
>
> @@ -312,6 +316,7 @@ def test_fit(u_boot_console):
>          loadables1 = make_kernel('test-loadables1.bin', 'lenrek')
>          loadables2 = make_ramdisk('test-loadables2.bin', 'ksidmar')
>          kernel_out = make_fname('kernel-out.bin')
> +        fdt = make_fname('u-boot.dtb')
>          fdt_out = make_fname('fdt-out.dtb')
>          ramdisk_out = make_fname('ramdisk-out.bin')
>          loadables1_out = make_fname('loadables1-out.bin')
> @@ -326,6 +331,7 @@ def test_fit(u_boot_console):
>              'kernel_addr' : 0x40000,
>              'kernel_size' : filesize(kernel),
>
> +            'fdt' : fdt,
>              'fdt_out' : fdt_out,
>              'fdt_addr' : 0x80000,
>              'fdt_size' : filesize(control_dtb),
> @@ -351,6 +357,7 @@ def test_fit(u_boot_console):
>              'loadables2_load' : '',
>
>              'loadables_config' : '',
> +            'compression' : 'none',
>          }
>
>          # Make a basic FIT and a script to load it
> @@ -417,6 +424,20 @@ def test_fit(u_boot_console):
>              check_equal(loadables2, loadables2_out,
>                          'Loadables2 (ramdisk) not loaded')
>
> +        # Kernel, FDT and Ramdisk all compressed
> +        with cons.log.section('(Kernel + FDT + Ramdisk) compressed'):
> +            params['compression'] = 'gzip'
> +            params['kernel'] = make_compressed(kernel)
> +            params['fdt'] = make_compressed(fdt)
> +            params['ramdisk'] = make_compressed(ramdisk)
> +            fit = make_fit(mkimage, params)
> +            cons.restart_uboot()
> +            output = cons.run_command_list(cmd.splitlines())
> +            check_equal(kernel, kernel_out, 'Kernel not loaded')
> +            check_equal(control_dtb, fdt_out, 'FDT not loaded')
> +            check_equal(ramdisk, ramdisk_out, 'Ramdisk not loaded')
> +
> +
>      cons = u_boot_console
>      try:
>          # We need to use our own device tree file. Remember to restore it
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 3/3] fit: Support compat string property in configuration node
  2019-07-11 20:53 ` [U-Boot] [PATCH v5 3/3] fit: Support compat string property in configuration node Julius Werner
@ 2019-07-12  6:10   ` Simon Goldschmidt
  2019-07-12 18:26     ` Julius Werner
  0 siblings, 1 reply; 15+ messages in thread
From: Simon Goldschmidt @ 2019-07-12  6:10 UTC (permalink / raw)
  To: u-boot

On Thu, Jul 11, 2019 at 10:53 PM Julius Werner <jwerner@chromium.org> wrote:
>
> This patch adds support for an optional optimization to compatible
> string matching where the compatible string property from the root node
> of the kernel FDT can be copied into the configuration node of the FIT
> image. This is most useful when using compressed FDTs or when using FDT
> overlays, where the traditional extraction of the compatible string from
> the kernel FDT itself is not easily possible.
>
> Signed-off-by: Julius Werner <jwerner@chromium.org>
> ---
>  - No changes for v2
>  - No changes for v3
>  - Changes for v4:
>    - Added documentation for compatible string in config node.
>    - Added example .its file for compressed FDT with compat string in
>      config node.
>  - No changes for v5

If there haven't been any changes from v4, why did you drop Simon Glass's
Reviewed-by tag?

Regards,
Simon

>
>  common/image-fit.c                        | 67 ++++++++++++---------
>  doc/uImage.FIT/kernel_fdts_compressed.its | 73 +++++++++++++++++++++++
>  doc/uImage.FIT/source_file_format.txt     |  7 +++
>  3 files changed, 119 insertions(+), 28 deletions(-)
>  create mode 100644 doc/uImage.FIT/kernel_fdts_compressed.its
>
> diff --git a/common/image-fit.c b/common/image-fit.c
> index c9ffc441aa..e346fed550 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -1522,6 +1522,10 @@ int fit_check_format(const void *fit)
>   * compatible list, "foo,bar", matches a compatible string in the root of fdt1.
>   * "bim,bam" in fdt2 matches the second string which isn't as good as fdt1.
>   *
> + * As an optimization, the compatible property from the FDT's root node can be
> + * copied into the configuration node in the FIT image. This is required to
> + * match configurations with compressed FDTs.
> + *
>   * returns:
>   *     offset to the configuration to use if one was found
>   *     -1 otherwise
> @@ -1554,55 +1558,62 @@ int fit_conf_find_compat(const void *fit, const void *fdt)
>         for (noffset = fdt_next_node(fit, confs_noffset, &ndepth);
>                         (noffset >= 0) && (ndepth > 0);
>                         noffset = fdt_next_node(fit, noffset, &ndepth)) {
> -               const void *kfdt;
> +               const void *fdt;
>                 const char *kfdt_name;
> -               int kfdt_noffset;
> +               int kfdt_noffset, compat_noffset;
>                 const char *cur_fdt_compat;
>                 int len;
> -               size_t size;
> +               size_t sz;
>                 int i;
>
>                 if (ndepth > 1)
>                         continue;
>
> -               kfdt_name = fdt_getprop(fit, noffset, "fdt", &len);
> -               if (!kfdt_name) {
> -                       debug("No fdt property found.\n");
> -                       continue;
> -               }
> -               kfdt_noffset = fdt_subnode_offset(fit, images_noffset,
> -                                                 kfdt_name);
> -               if (kfdt_noffset < 0) {
> -                       debug("No image node named \"%s\" found.\n",
> -                             kfdt_name);
> -                       continue;
> -               }
> +               /* If there's a compat property in the config node, use that. */
> +               if (fdt_getprop(fit, noffset, "compatible", NULL)) {
> +                       fdt = fit;                /* search in FIT image */
> +                       compat_noffset = noffset; /* search under config node */
> +               } else {        /* Otherwise extract it from the kernel FDT. */
> +                       kfdt_name = fdt_getprop(fit, noffset, "fdt", &len);
> +                       if (!kfdt_name) {
> +                               debug("No fdt property found.\n");
> +                               continue;
> +                       }
> +                       kfdt_noffset = fdt_subnode_offset(fit, images_noffset,
> +                                                         kfdt_name);
> +                       if (kfdt_noffset < 0) {
> +                               debug("No image node named \"%s\" found.\n",
> +                                     kfdt_name);
> +                               continue;
> +                       }
>
> -               if (!fit_image_check_comp(fit, kfdt_noffset, IH_COMP_NONE)) {
> -                       debug("Can't extract compat from \"%s\" (compressed)\n",
> -                             kfdt_name);
> -                       continue;
> -               }
> +                       if (!fit_image_check_comp(fit, kfdt_noffset,
> +                                                 IH_COMP_NONE)) {
> +                               debug("Can't extract compat from \"%s\" "
> +                                     "(compressed)\n", kfdt_name);
> +                               continue;
> +                       }
>
> -               /*
> -                * Get a pointer to this configuration's fdt.
> -                */
> -               if (fit_image_get_data(fit, kfdt_noffset, &kfdt, &size)) {
> -                       debug("Failed to get fdt \"%s\".\n", kfdt_name);
> -                       continue;
> +                       /* search in this config's kernel FDT */
> +                       if (fit_image_get_data(fit, kfdt_noffset, &fdt, &sz)) {
> +                               debug("Failed to get fdt \"%s\".\n", kfdt_name);
> +                               continue;
> +                       }
> +
> +                       compat_noffset = 0;  /* search kFDT under root node */
>                 }
>
>                 len = fdt_compat_len;
>                 cur_fdt_compat = fdt_compat;
>                 /*
>                  * Look for a match for each U-Boot compatibility string in
> -                * turn in this configuration's fdt.
> +                * turn in the compat string property.
>                  */
>                 for (i = 0; len > 0 &&
>                      (!best_match_offset || best_match_pos > i); i++) {
>                         int cur_len = strlen(cur_fdt_compat) + 1;
>
> -                       if (!fdt_node_check_compatible(kfdt, 0,
> +                       if (!fdt_node_check_compatible(fdt, compat_noffset,
>                                                        cur_fdt_compat)) {
>                                 best_match_offset = noffset;
>                                 best_match_pos = i;
> diff --git a/doc/uImage.FIT/kernel_fdts_compressed.its b/doc/uImage.FIT/kernel_fdts_compressed.its
> new file mode 100644
> index 0000000000..8f81106efc
> --- /dev/null
> +++ b/doc/uImage.FIT/kernel_fdts_compressed.its
> @@ -0,0 +1,73 @@
> +/*
> + * U-Boot uImage source file with a kernel and multiple compressed FDT blobs.
> + * Since the FDTs are compressed, configurations must provide a compatible
> + * string to match directly.
> + */
> +
> +/dts-v1/;
> +
> +/ {
> +       description = "Image with single Linux kernel and compressed FDT blobs";
> +       #address-cells = <1>;
> +
> +       images {
> +               kernel {
> +                       description = "Vanilla Linux kernel";
> +                       data = /incbin/("./vmlinux.bin.gz");
> +                       type = "kernel";
> +                       arch = "ppc";
> +                       os = "linux";
> +                       compression = "gzip";
> +                       load = <00000000>;
> +                       entry = <00000000>;
> +                       hash-1 {
> +                               algo = "crc32";
> +                       };
> +                       hash-2 {
> +                               algo = "sha1";
> +                       };
> +               };
> +               fdt at 1 {
> +                       description = "Flattened Device Tree blob 1";
> +                       data = /incbin/("./myboard-var1.dtb");
> +                       type = "flat_dt";
> +                       arch = "ppc";
> +                       compression = "gzip";
> +                       hash-1 {
> +                               algo = "crc32";
> +                       };
> +                       hash-2 {
> +                               algo = "sha1";
> +                       };
> +               };
> +               fdt at 2 {
> +                       description = "Flattened Device Tree blob 2";
> +                       data = /incbin/("./myboard-var2.dtb");
> +                       type = "flat_dt";
> +                       arch = "ppc";
> +                       compression = "lzma";
> +                       hash-1 {
> +                               algo = "crc32";
> +                       };
> +                       hash-2 {
> +                               algo = "sha1";
> +                       };
> +               };
> +       };
> +
> +       configurations {
> +               default = "conf at 1";
> +               conf at 1 {
> +                       description = "Boot Linux kernel with FDT blob 1";
> +                       kernel = "kernel";
> +                       fdt = "fdt at 1";
> +                       compatible = "myvendor,myboard-variant1";
> +               };
> +               conf at 2 {
> +                       description = "Boot Linux kernel with FDT blob 2";
> +                       kernel = "kernel";
> +                       fdt = "fdt at 2";
> +                       compatible = "myvendor,myboard-variant2";
> +               };
> +       };
> +};
> diff --git a/doc/uImage.FIT/source_file_format.txt b/doc/uImage.FIT/source_file_format.txt
> index d701b9bb76..f8e27ed34e 100644
> --- a/doc/uImage.FIT/source_file_format.txt
> +++ b/doc/uImage.FIT/source_file_format.txt
> @@ -240,6 +240,7 @@ o config-1
>    |- fdt = "fdt sub-node unit-name" [, "fdt overlay sub-node unit-name", ...]
>    |- fpga = "fpga sub-node unit-name"
>    |- loadables = "loadables sub-node unit-name"
> +  |- compatible = "vendor,board-style device tree compatible string"
>
>
>    Mandatory properties:
> @@ -263,6 +264,12 @@ o config-1
>      of strings. U-Boot will load each binary at its given start-address and
>      may optionaly invoke additional post-processing steps on this binary based
>      on its component image node type.
> +  - compatible : The root compatible string of the U-Boot device tree that
> +    this configuration shall automatically match when CONFIG_FIT_BEST_MATCH is
> +    enabled. If this property is not provided, the compatible string will be
> +    extracted from the fdt blob instead. This is only possible if the fdt is
> +    not compressed, so images with compressed fdts that want to use compatible
> +    string matching must always provide this property.
>
>  The FDT blob is required to properly boot FDT based kernel, so the minimal
>  configuration for 2.6 FDT kernel is (kernel, fdt) pair.
> --
> 2.20.1
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/3] fit: Support compression for non-kernel components (e.g. FDT)
  2019-07-12  6:10   ` Simon Goldschmidt
@ 2019-07-12 18:25     ` Julius Werner
  2019-07-12 18:31       ` Simon Goldschmidt
  0 siblings, 1 reply; 15+ messages in thread
From: Julius Werner @ 2019-07-12 18:25 UTC (permalink / raw)
  To: u-boot

> If there haven't been any changes from v4, why did you drop Simon Glass's
> Reviewed-by tag?

Sorry, I wasn't aware I was supposed to preserve those manually. Yes,
there were no changes so this should still be considered

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 3/3] fit: Support compat string property in configuration node
  2019-07-12  6:10   ` Simon Goldschmidt
@ 2019-07-12 18:26     ` Julius Werner
  0 siblings, 0 replies; 15+ messages in thread
From: Julius Werner @ 2019-07-12 18:26 UTC (permalink / raw)
  To: u-boot

> If there haven't been any changes from v4, why did you drop Simon Glass's
> Reviewed-by tag?

Same here:

Reviewed-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 0/3] fit: Image node compression
  2019-07-12  6:02 ` [U-Boot] [PATCH v5 0/3] fit: Image node compression Simon Goldschmidt
@ 2019-07-12 18:27   ` Julius Werner
  0 siblings, 0 replies; 15+ messages in thread
From: Julius Werner @ 2019-07-12 18:27 UTC (permalink / raw)
  To: u-boot

> This kind of interleaved change log is kind of hard to read. A higher-level,
> manually written changelog for the cover letter would be better I think.

Sorry, will rewrite the cover letter next time. For v5 I just added
the new patch and didn't change the other two.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 2/3] fit: Support compression for non-kernel components (e.g. FDT)
  2019-07-12 18:25     ` Julius Werner
@ 2019-07-12 18:31       ` Simon Goldschmidt
  0 siblings, 0 replies; 15+ messages in thread
From: Simon Goldschmidt @ 2019-07-12 18:31 UTC (permalink / raw)
  To: u-boot

Am 12.07.2019 um 20:25 schrieb Julius Werner:
>> If there haven't been any changes from v4, why did you drop Simon Glass's
>> Reviewed-by tag?
> 
> Sorry, I wasn't aware I was supposed to preserve those manually. Yes,
> there were no changes so this should still be considered

I was suprised by that, too, in the beginning. But then, how should 
patchwork know you haven't changed anything in patches between versions 
or if the RB is still valid...

Regards,
Simon

> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 1/3] common: Move bootm_decomp_image() to image.c (as image_decomp())
  2019-07-12  6:09   ` Simon Goldschmidt
@ 2019-07-12 18:33     ` Julius Werner
  0 siblings, 0 replies; 15+ messages in thread
From: Julius Werner @ 2019-07-12 18:33 UTC (permalink / raw)
  To: u-boot

> I'd like to review this, but maybe you can help me speed up the process and tell
> us if the move has been a 1:1 code move or if you had to adapt things to the new
> location (other than the function being renamed)?

print_decomp_msg() is a 1:1 move. bootm_decomp_image() is mostly 1:1
move, except that I changed the return code for unimplemented
compression type (from BOOTM_ERR_UNIMPLEMENTED to -ENOSYS) and removed
the call to handle_decomp_error() (handle_decomp_error() is specific
to the BOOTM_ERR error types which are only used within bootm.c, so I
decoupled it from image_decomp() and made all the call sites in
bootm.c call it explicitly).

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 1/3] common: Move bootm_decomp_image() to image.c (as image_decomp())
  2019-07-11 20:53 ` [U-Boot] [PATCH v5 1/3] common: Move bootm_decomp_image() to image.c (as image_decomp()) Julius Werner
  2019-07-12  6:09   ` Simon Goldschmidt
@ 2019-07-24 18:14   ` Tom Rini
  2019-07-25  2:40     ` Julius Werner
  1 sibling, 1 reply; 15+ messages in thread
From: Tom Rini @ 2019-07-24 18:14 UTC (permalink / raw)
  To: u-boot

On Thu, Jul 11, 2019 at 01:53:18PM -0700, Julius Werner wrote:

> Upcoming patches want to add decompression to use cases that are no
> longer directly related to booting. It makes sense to retain a single
> decompression routine, but it should no longer be in bootm.c (which is
> not compiled for all configurations). This patch moves
> bootm_decomp_image() to image.c and renames it to image_decomp() in
> preparation of those upcoming patches.
> 
> Signed-off-by: Julius Werner <jwerner@chromium.org>
> ---
>  - First version: v5

OK, this now causes 'make tests' to fail on the FIT image tests, please
look.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190724/06c33003/attachment.sig>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH v5 1/3] common: Move bootm_decomp_image() to image.c (as image_decomp())
  2019-07-24 18:14   ` Tom Rini
@ 2019-07-25  2:40     ` Julius Werner
  0 siblings, 0 replies; 15+ messages in thread
From: Julius Werner @ 2019-07-25  2:40 UTC (permalink / raw)
  To: u-boot

> OK, this now causes 'make tests' to fail on the FIT image tests, please
> look.  Thanks!

Whoops, sorry, should have run that again. This was just a printf()
without a newline at the end, apparently pytest really doesn't like
that. (I took out the "OK" at the end of the "Loading kernel..."
message since with this patch decompression errors are supposed to be
handled by the callers and there are separate error messages in those
anyway.)

Fixed in v6.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2019-07-25  2:40 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-11 20:53 [U-Boot] [PATCH v5 0/3] fit: Image node compression Julius Werner
2019-07-11 20:53 ` [U-Boot] [PATCH v5 1/3] common: Move bootm_decomp_image() to image.c (as image_decomp()) Julius Werner
2019-07-12  6:09   ` Simon Goldschmidt
2019-07-12 18:33     ` Julius Werner
2019-07-24 18:14   ` Tom Rini
2019-07-25  2:40     ` Julius Werner
2019-07-11 20:53 ` [U-Boot] [PATCH v5 2/3] fit: Support compression for non-kernel components (e.g. FDT) Julius Werner
2019-07-12  6:10   ` Simon Goldschmidt
2019-07-12 18:25     ` Julius Werner
2019-07-12 18:31       ` Simon Goldschmidt
2019-07-11 20:53 ` [U-Boot] [PATCH v5 3/3] fit: Support compat string property in configuration node Julius Werner
2019-07-12  6:10   ` Simon Goldschmidt
2019-07-12 18:26     ` Julius Werner
2019-07-12  6:02 ` [U-Boot] [PATCH v5 0/3] fit: Image node compression Simon Goldschmidt
2019-07-12 18:27   ` Julius Werner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.