[U-Boot] [RFC/RFT PATCH v4 0/3] Add compressed Image booting support

[U-Boot] [RFC/RFT PATCH v4 0/3] Add compressed Image booting support

[Atish Patra]

This patch series extends booti to support compressed images
as well. Following compressed images are supported for now. 

lzma, lzo, bzip2, gz.

Other compression methods can easily be supported if required.
The above compression methods are common ones that Linux kernel
(ARM64/RISC-V) and U-Boot supports.

Changes from v3->v4
1. Removed CONFIG_SYS_BOOTM_LEN usage.

Atish Patra (3):
lib: kconfig: Add option to set BZIP2 compression method
image: Add a common compression type detection function.
image: Add compressed Image parsing support in booti.

cmd/booti.c                | 40 ++++++++++++++++++++++++++-
common/image.c             | 23 ++++++++++++++++
doc/README.distro          | 12 +++++++++
doc/board/sifive/fu540.rst | 55 ++++++++++++++++++++++++++++++++++++++
include/image.h            | 21 +++++++++++++++
lib/Kconfig                |  5 ++++
6 files changed, 155 insertions(+), 1 deletion(-)

--
2.24.0

[U-Boot] [RFC/RFT PATCH v4 1/3] lib: kconfig: Add option to set BZIP2 compression method

[Atish Patra]

There is no way to select BZIP2 compression method.
Add it under library/compression config where all other
compression related configs are present.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
---
 lib/Kconfig | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/Kconfig b/lib/Kconfig
index 3da45a5ec322..b5dcdba23014 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -385,6 +385,11 @@ config GZIP
 	help
 	  This enables support for GZIP compression algorithm.
 
+config BZIP2
+	bool "Enable bzip2 decompression support"
+	help
+	  This enables support for BZIP2 compression algorithm.
+
 config ZLIB
 	bool
 	default y
-- 
2.24.0

[U-Boot] [RFC/RFT PATCH v4 2/3] image: Add a common compression type detection function.

[Atish Patra]

Currently, there is no method that can detect compression types
given a file. This is very useful where a compressed kernel image
is loaded directly to the memory.

Inspect initial few bytes to figure out compression type of the
image. It will be used in booti method for now but can be reused
any other function in future as well.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
---
 common/image.c  | 23 +++++++++++++++++++++++
 include/image.h | 21 +++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/common/image.c b/common/image.c
index 179eef0bd2dc..3043f0a1c2fc 100644
--- a/common/image.c
+++ b/common/image.c
@@ -196,6 +196,14 @@ struct table_info {
 	const table_entry_t *table;
 };
 
+static const struct comp_magic_map image_comp[] = {
+	{	IH_COMP_BZIP2,	"bzip2",	{0x42, 0x5a},},
+	{	IH_COMP_GZIP,	"gzip",		{0x1f, 0x8b},},
+	{	IH_COMP_LZMA,	"lzma",		{0x5d, 0x00},},
+	{	IH_COMP_LZO,	"lzo",		{0x89, 0x4c},},
+	{	IH_COMP_NONE,	"none",		{},	},
+};
+
 static const struct table_info table_info[IH_COUNT] = {
 	{ "architecture", IH_ARCH_COUNT, uimage_arch },
 	{ "compression", IH_COMP_COUNT, uimage_comp },
@@ -401,6 +409,21 @@ static void print_decomp_msg(int comp_type, int type, bool is_xip)
 		printf("   Uncompressing %s\n", name);
 }
 
+int image_decomp_type(const unsigned char *buf, ulong len)
+{
+	const struct comp_magic_map *cmagic = image_comp;
+
+	if (len < 2)
+		return -EINVAL;
+
+	for (; cmagic->comp_id > 0; cmagic++) {
+		if (!memcmp(buf, cmagic->magic, 2))
+			break;
+	}
+
+	return cmagic->comp_id;
+}
+
 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)
diff --git a/include/image.h b/include/image.h
index c1065c06f9bd..733a6a107da8 100644
--- a/include/image.h
+++ b/include/image.h
@@ -447,6 +447,15 @@ typedef struct table_entry {
 	char	*lname;		/* long (output) name to print for messages */
 } table_entry_t;
 
+/*
+ * Compression type and magic number mapping table.
+ */
+struct comp_magic_map {
+	int		comp_id;
+	const char	*name;
+	unsigned char	magic[2];
+};
+
 /*
  * get_table_entry_id() scans the translation table trying to find an
  * entry that matches the given short name. If a matching entry is
@@ -851,6 +860,18 @@ static inline int image_check_target_arch(const image_header_t *hdr)
 }
 #endif /* USE_HOSTCC */
 
+/**
+ * image_decomp_type() - Find out compression type of an image
+ *
+ * @buf:	Address in U-Boot memory where image is loaded.
+ * @len:	Length of the compressed image.
+ * @return	compression type or IH_COMP_NONE if not compressed.
+ *
+ * Note: Only following compression types are supported now.
+ * lzo, lzma, gzip, bzip2
+ */
+int image_decomp_type(const unsigned char *buf, ulong len);
+
 /**
  * image_decomp() - decompress an image
  *
-- 
2.24.0

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[Atish Patra]

Add compressed Image parsing support so that booti can parse both
flat and compressed Image to boot Linux. Currently, it is difficult
to calculate a safe address for every board where the compressed
image can be decompressed. It is also not possible to figure out the
size of the compressed file as well. Thus, user need to set two
additional environment variables kernel_comp_addr_r and filesize to
make this work.

Following compression methods are supported for now.
lzma, lzo, bzip2, gzip.

lz4 support is not added as ARM64 kernel generates a lz4 compressed
image with legacy header which U-Boot doesn't know how to parse and
decompress.

Tested on HiFive Unleashed and Qemu for RISC-V.
Tested on Qemu for ARM64.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
I could not test this patch on any ARM64 boards due to lack of
access to any ARM64 board. If anybody can test it on ARM64, that
would be great.
---
 cmd/booti.c                | 40 ++++++++++++++++++++++++++-
 doc/README.distro          | 12 +++++++++
 doc/board/sifive/fu540.rst | 55 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 106 insertions(+), 1 deletion(-)

diff --git a/cmd/booti.c b/cmd/booti.c
index c36b0235df8c..cd8670a9a8db 100644
--- a/cmd/booti.c
+++ b/cmd/booti.c
@@ -13,6 +13,7 @@
 #include <linux/kernel.h>
 #include <linux/sizes.h>
 
+DECLARE_GLOBAL_DATA_PTR;
 /*
  * Image booting support
  */
@@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int flag, int argc,
 	ulong ld;
 	ulong relocated_addr;
 	ulong image_size;
+	uint8_t *temp;
+	ulong dest;
+	ulong dest_end;
+	unsigned long comp_len;
+	unsigned long decomp_len;
+	int ctype;
 
 	ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START,
 			      images, 1);
@@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int flag, int argc,
 		debug("*  kernel: cmdline image address = 0x%08lx\n", ld);
 	}
 
+	temp = map_sysmem(ld, 0);
+	ctype = image_decomp_type(temp, 2);
+	if (ctype > 0) {
+		dest = env_get_ulong("kernel_comp_addr_r", 16, 0);
+		comp_len = env_get_ulong("filesize", 16, 0);
+		if (!dest || !comp_len) {
+			puts("kernel_comp_addr_r or filesize is not provided!\n");
+			return -EINVAL;
+		}
+		if (dest < gd->ram_base || dest > gd->ram_top) {
+			puts("kernel_comp_addr_r is outside of DRAM range!\n");
+			return -EINVAL;
+		}
+
+		debug("kernel image compression type %d size = 0x%08lx address = 0x%08lx\n",
+			ctype, comp_len, (ulong)dest);
+		decomp_len = comp_len * 10;
+		ret = image_decomp(ctype, 0, ld, IH_TYPE_KERNEL,
+				 (void *)dest, (void *)ld, comp_len,
+				 decomp_len, &dest_end);
+		if (ret)
+			return ret;
+		/* dest_end contains the uncompressed Image size */
+		memmove((void *) ld, (void *)dest, dest_end);
+	}
+	unmap_sysmem((void *)ld);
+
 	ret = booti_setup(ld, &relocated_addr, &image_size, false);
 	if (ret != 0)
 		return 1;
@@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 #ifdef CONFIG_SYS_LONGHELP
 static char booti_help_text[] =
 	"[addr [initrd[:size]] [fdt]]\n"
-	"    - boot Linux 'Image' stored at 'addr'\n"
+	"    - boot Linux flat or compressed 'Image' stored at 'addr'\n"
 	"\tThe argument 'initrd' is optional and specifies the address\n"
 	"\tof an initrd in memory. The optional parameter ':size' allows\n"
 	"\tspecifying the size of a RAW initrd.\n"
+	"\tCurrently only booting from gz, bz2, lzma and lz4 compression\n"
+	"\ttypes are supported. In order to boot from any of these compressed\n"
+	"\timages, user have to set kernel_comp_addr_r and filesize enviornment\n"
+	"\tvariables beforehand.\n"
 #if defined(CONFIG_OF_LIBFDT)
 	"\tSince booting a Linux kernel requires a flat device-tree, a\n"
 	"\tthird argument providing the address of the device-tree blob\n"
diff --git a/doc/README.distro b/doc/README.distro
index ab6e6f4e74be..67b49e1e4b6a 100644
--- a/doc/README.distro
+++ b/doc/README.distro
@@ -246,6 +246,18 @@ kernel_addr_r:
 
   A size of 16MB for the kernel is likely adequate.
 
+kernel_comp_addr_r:
+  Optional. This is only required if user wants to boot Linux from a compressed
+  Image(.gz, .bz2, .lzma, .lzo) using booti command. It represents the location
+  in RAM where the compressed Image will be decompressed temporarily. Once the
+  decompression is complete, decompressed data will be moved kernel_addr_r for
+  booting.
+
+filesize:
+  Optional. This is only required if user wants to boot Linux from a compressed
+  Image using booti command. It represents the size of the compressed file. The
+  size has to at least the size of loaded image for decompression to succeed.
+
 pxefile_addr_r:
 
   Mandatory. The location in RAM where extlinux.conf will be loaded to prior
diff --git a/doc/board/sifive/fu540.rst b/doc/board/sifive/fu540.rst
index 7807f5b2c128..df2c5ad8d3e3 100644
--- a/doc/board/sifive/fu540.rst
+++ b/doc/board/sifive/fu540.rst
@@ -138,6 +138,10 @@ load uImage.
    => setenv netmask 255.255.252.0
    => setenv serverip 10.206.4.143
    => setenv gateway 10.206.4.1
+
+If you want to use a flat kernel image such as Image file
+
+.. code-block:: none
    => tftpboot ${kernel_addr_r} /sifive/fu540/Image
    ethernet at 10090000: PHY present at 0
    ethernet at 10090000: Starting autonegotiation...
@@ -177,6 +181,57 @@ load uImage.
             1.2 MiB/s
    done
    Bytes transferred = 8867100 (874d1c hex)
+
+Or if you want to use a compressed kernel image file such as Image.gz
+
+.. code-block:: none
+   => tftpboot ${kernel_addr_r} /sifive/fu540/Image.gz
+   ethernet at 10090000: PHY present at 0
+   ethernet at 10090000: Starting autonegotiation...
+   ethernet at 10090000: Autonegotiation complete
+   ethernet at 10090000: link up, 1000Mbps full-duplex (lpa: 0x3c00)
+   Using ethernet at 10090000 device
+   TFTP from server 10.206.4.143; our IP address is 10.206.7.133
+   Filename '/sifive/fu540/Image.gz'.
+   Load address: 0x84000000
+   Loading: #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            #################################################################
+            ##########################################
+            1.2 MiB/s
+   done
+   Bytes transferred = 4809458 (4962f2 hex)
+   =>setenv kernel_comp_addr_r 0x90000000
+   =>setenv filesize 0x500000
+
+By this time, correct kernel image is loaded and required enviornment variables
+are set. You can proceed to load the ramdisk and device tree from the tftp server
+as well.
+
+.. code-block:: none
    => tftpboot ${ramdisk_addr_r} /sifive/fu540/uRamdisk
    ethernet at 10090000: PHY present at 0
    ethernet at 10090000: Starting autonegotiation...
-- 
2.24.0

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[David Abdurachmanov]

On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com> wrote:
>
> Add compressed Image parsing support so that booti can parse both
> flat and compressed Image to boot Linux. Currently, it is difficult
> to calculate a safe address for every board where the compressed
> image can be decompressed. It is also not possible to figure out the
> size of the compressed file as well. Thus, user need to set two
> additional environment variables kernel_comp_addr_r and filesize to
> make this work.
>
> Following compression methods are supported for now.
> lzma, lzo, bzip2, gzip.
>
> lz4 support is not added as ARM64 kernel generates a lz4 compressed
> image with legacy header which U-Boot doesn't know how to parse and
> decompress.
>
> Tested on HiFive Unleashed and Qemu for RISC-V.
> Tested on Qemu for ARM64.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> ---
> I could not test this patch on any ARM64 boards due to lack of
> access to any ARM64 board. If anybody can test it on ARM64, that
> would be great.
> ---
>  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
>  doc/README.distro          | 12 +++++++++
>  doc/board/sifive/fu540.rst | 55 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 106 insertions(+), 1 deletion(-)
>
> diff --git a/cmd/booti.c b/cmd/booti.c
> index c36b0235df8c..cd8670a9a8db 100644
> --- a/cmd/booti.c
> +++ b/cmd/booti.c
> @@ -13,6 +13,7 @@
>  #include <linux/kernel.h>
>  #include <linux/sizes.h>
>
> +DECLARE_GLOBAL_DATA_PTR;
>  /*
>   * Image booting support
>   */
> @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int flag, int argc,
>         ulong ld;
>         ulong relocated_addr;
>         ulong image_size;
> +       uint8_t *temp;
> +       ulong dest;
> +       ulong dest_end;
> +       unsigned long comp_len;
> +       unsigned long decomp_len;
> +       int ctype;
>
>         ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START,
>                               images, 1);
> @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int flag, int argc,
>                 debug("*  kernel: cmdline image address = 0x%08lx\n", ld);
>         }
>
> +       temp = map_sysmem(ld, 0);
> +       ctype = image_decomp_type(temp, 2);
> +       if (ctype > 0) {
> +               dest = env_get_ulong("kernel_comp_addr_r", 16, 0);
> +               comp_len = env_get_ulong("filesize", 16, 0);
> +               if (!dest || !comp_len) {
> +                       puts("kernel_comp_addr_r or filesize is not provided!\n");
> +                       return -EINVAL;
> +               }
> +               if (dest < gd->ram_base || dest > gd->ram_top) {
> +                       puts("kernel_comp_addr_r is outside of DRAM range!\n");
> +                       return -EINVAL;
> +               }
> +
> +               debug("kernel image compression type %d size = 0x%08lx address = 0x%08lx\n",
> +                       ctype, comp_len, (ulong)dest);
> +               decomp_len = comp_len * 10;
> +               ret = image_decomp(ctype, 0, ld, IH_TYPE_KERNEL,
> +                                (void *)dest, (void *)ld, comp_len,
> +                                decomp_len, &dest_end);
> +               if (ret)
> +                       return ret;
> +               /* dest_end contains the uncompressed Image size */
> +               memmove((void *) ld, (void *)dest, dest_end);
> +       }
> +       unmap_sysmem((void *)ld);
> +
>         ret = booti_setup(ld, &relocated_addr, &image_size, false);
>         if (ret != 0)
>                 return 1;
> @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>  #ifdef CONFIG_SYS_LONGHELP
>  static char booti_help_text[] =
>         "[addr [initrd[:size]] [fdt]]\n"
> -       "    - boot Linux 'Image' stored at 'addr'\n"
> +       "    - boot Linux flat or compressed 'Image' stored at 'addr'\n"
>         "\tThe argument 'initrd' is optional and specifies the address\n"
>         "\tof an initrd in memory. The optional parameter ':size' allows\n"
>         "\tspecifying the size of a RAW initrd.\n"
> +       "\tCurrently only booting from gz, bz2, lzma and lz4 compression\n"
> +       "\ttypes are supported. In order to boot from any of these compressed\n"
> +       "\timages, user have to set kernel_comp_addr_r and filesize enviornment\n"
> +       "\tvariables beforehand.\n"
>  #if defined(CONFIG_OF_LIBFDT)
>         "\tSince booting a Linux kernel requires a flat device-tree, a\n"
>         "\tthird argument providing the address of the device-tree blob\n"
> diff --git a/doc/README.distro b/doc/README.distro
> index ab6e6f4e74be..67b49e1e4b6a 100644
> --- a/doc/README.distro
> +++ b/doc/README.distro
> @@ -246,6 +246,18 @@ kernel_addr_r:
>
>    A size of 16MB for the kernel is likely adequate.
>
> +kernel_comp_addr_r:
> +  Optional. This is only required if user wants to boot Linux from a compressed
> +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It represents the location
> +  in RAM where the compressed Image will be decompressed temporarily. Once the
> +  decompression is complete, decompressed data will be moved kernel_addr_r for
> +  booting.
> +
> +filesize:
> +  Optional. This is only required if user wants to boot Linux from a compressed
> +  Image using booti command. It represents the size of the compressed file. The
> +  size has to at least the size of loaded image for decompression to succeed.
> +

I am not sure I like using filesize here compared to kernel_gz_size.
It's true that $filesize will be set once your load the binary, e.g. from mmc.
But in EXTLINUX/PXE situation $filesize could be wrong.

The load happens in this order:
- initrd
- kernel
- fdt

So if I specify FDT in my EXTLINUX/PXE config the $filesize will be wrong
while attempting to boot the kernel. Unless we save filesize of kernel after
loading and set it again before calling do_booti() in pxe.c.

Currently I set kernel_gz_size in board environment, which is set to max
allowed size.

david

>  pxefile_addr_r:
>
>    Mandatory. The location in RAM where extlinux.conf will be loaded to prior
> diff --git a/doc/board/sifive/fu540.rst b/doc/board/sifive/fu540.rst
> index 7807f5b2c128..df2c5ad8d3e3 100644
> --- a/doc/board/sifive/fu540.rst
> +++ b/doc/board/sifive/fu540.rst
> @@ -138,6 +138,10 @@ load uImage.
>     => setenv netmask 255.255.252.0
>     => setenv serverip 10.206.4.143
>     => setenv gateway 10.206.4.1
> +
> +If you want to use a flat kernel image such as Image file
> +
> +.. code-block:: none
>     => tftpboot ${kernel_addr_r} /sifive/fu540/Image
>     ethernet at 10090000: PHY present at 0
>     ethernet at 10090000: Starting autonegotiation...
> @@ -177,6 +181,57 @@ load uImage.
>              1.2 MiB/s
>     done
>     Bytes transferred = 8867100 (874d1c hex)
> +
> +Or if you want to use a compressed kernel image file such as Image.gz
> +
> +.. code-block:: none
> +   => tftpboot ${kernel_addr_r} /sifive/fu540/Image.gz
> +   ethernet at 10090000: PHY present at 0
> +   ethernet at 10090000: Starting autonegotiation...
> +   ethernet at 10090000: Autonegotiation complete
> +   ethernet at 10090000: link up, 1000Mbps full-duplex (lpa: 0x3c00)
> +   Using ethernet at 10090000 device
> +   TFTP from server 10.206.4.143; our IP address is 10.206.7.133
> +   Filename '/sifive/fu540/Image.gz'.
> +   Load address: 0x84000000
> +   Loading: #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            #################################################################
> +            ##########################################
> +            1.2 MiB/s
> +   done
> +   Bytes transferred = 4809458 (4962f2 hex)
> +   =>setenv kernel_comp_addr_r 0x90000000
> +   =>setenv filesize 0x500000
> +
> +By this time, correct kernel image is loaded and required enviornment variables
> +are set. You can proceed to load the ramdisk and device tree from the tftp server
> +as well.
> +
> +.. code-block:: none
>     => tftpboot ${ramdisk_addr_r} /sifive/fu540/uRamdisk
>     ethernet at 10090000: PHY present at 0
>     ethernet at 10090000: Starting autonegotiation...
> --
> 2.24.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[Atish Patra]

On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com>
> wrote:
> > Add compressed Image parsing support so that booti can parse both
> > flat and compressed Image to boot Linux. Currently, it is difficult
> > to calculate a safe address for every board where the compressed
> > image can be decompressed. It is also not possible to figure out
> > the
> > size of the compressed file as well. Thus, user need to set two
> > additional environment variables kernel_comp_addr_r and filesize to
> > make this work.
> > 
> > Following compression methods are supported for now.
> > lzma, lzo, bzip2, gzip.
> > 
> > lz4 support is not added as ARM64 kernel generates a lz4 compressed
> > image with legacy header which U-Boot doesn't know how to parse and
> > decompress.
> > 
> > Tested on HiFive Unleashed and Qemu for RISC-V.
> > Tested on Qemu for ARM64.
> > 
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > ---
> > I could not test this patch on any ARM64 boards due to lack of
> > access to any ARM64 board. If anybody can test it on ARM64, that
> > would be great.
> > ---
> >  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
> >  doc/README.distro          | 12 +++++++++
> >  doc/board/sifive/fu540.rst | 55
> > ++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 106 insertions(+), 1 deletion(-)
> > 
> > diff --git a/cmd/booti.c b/cmd/booti.c
> > index c36b0235df8c..cd8670a9a8db 100644
> > --- a/cmd/booti.c
> > +++ b/cmd/booti.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/kernel.h>
> >  #include <linux/sizes.h>
> > 
> > +DECLARE_GLOBAL_DATA_PTR;
> >  /*
> >   * Image booting support
> >   */
> > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > flag, int argc,
> >         ulong ld;
> >         ulong relocated_addr;
> >         ulong image_size;
> > +       uint8_t *temp;
> > +       ulong dest;
> > +       ulong dest_end;
> > +       unsigned long comp_len;
> > +       unsigned long decomp_len;
> > +       int ctype;
> > 
> >         ret = do_bootm_states(cmdtp, flag, argc, argv,
> > BOOTM_STATE_START,
> >                               images, 1);
> > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > flag, int argc,
> >                 debug("*  kernel: cmdline image address =
> > 0x%08lx\n", ld);
> >         }
> > 
> > +       temp = map_sysmem(ld, 0);
> > +       ctype = image_decomp_type(temp, 2);
> > +       if (ctype > 0) {
> > +               dest = env_get_ulong("kernel_comp_addr_r", 16, 0);
> > +               comp_len = env_get_ulong("filesize", 16, 0);
> > +               if (!dest || !comp_len) {
> > +                       puts("kernel_comp_addr_r or filesize is not
> > provided!\n");
> > +                       return -EINVAL;
> > +               }
> > +               if (dest < gd->ram_base || dest > gd->ram_top) {
> > +                       puts("kernel_comp_addr_r is outside of DRAM
> > range!\n");
> > +                       return -EINVAL;
> > +               }
> > +
> > +               debug("kernel image compression type %d size =
> > 0x%08lx address = 0x%08lx\n",
> > +                       ctype, comp_len, (ulong)dest);
> > +               decomp_len = comp_len * 10;
> > +               ret = image_decomp(ctype, 0, ld, IH_TYPE_KERNEL,
> > +                                (void *)dest, (void *)ld,
> > comp_len,
> > +                                decomp_len, &dest_end);
> > +               if (ret)
> > +                       return ret;
> > +               /* dest_end contains the uncompressed Image size */
> > +               memmove((void *) ld, (void *)dest, dest_end);
> > +       }
> > +       unmap_sysmem((void *)ld);
> > +
> >         ret = booti_setup(ld, &relocated_addr, &image_size, false);
> >         if (ret != 0)
> >                 return 1;
> > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag, int
> > argc, char * const argv[])
> >  #ifdef CONFIG_SYS_LONGHELP
> >  static char booti_help_text[] =
> >         "[addr [initrd[:size]] [fdt]]\n"
> > -       "    - boot Linux 'Image' stored at 'addr'\n"
> > +       "    - boot Linux flat or compressed 'Image' stored at
> > 'addr'\n"
> >         "\tThe argument 'initrd' is optional and specifies the
> > address\n"
> >         "\tof an initrd in memory. The optional parameter ':size'
> > allows\n"
> >         "\tspecifying the size of a RAW initrd.\n"
> > +       "\tCurrently only booting from gz, bz2, lzma and lz4
> > compression\n"
> > +       "\ttypes are supported. In order to boot from any of these
> > compressed\n"
> > +       "\timages, user have to set kernel_comp_addr_r and filesize
> > enviornment\n"
> > +       "\tvariables beforehand.\n"
> >  #if defined(CONFIG_OF_LIBFDT)
> >         "\tSince booting a Linux kernel requires a flat device-
> > tree, a\n"
> >         "\tthird argument providing the address of the device-tree
> > blob\n"
> > diff --git a/doc/README.distro b/doc/README.distro
> > index ab6e6f4e74be..67b49e1e4b6a 100644
> > --- a/doc/README.distro
> > +++ b/doc/README.distro
> > @@ -246,6 +246,18 @@ kernel_addr_r:
> > 
> >    A size of 16MB for the kernel is likely adequate.
> > 
> > +kernel_comp_addr_r:
> > +  Optional. This is only required if user wants to boot Linux from
> > a compressed
> > +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It represents
> > the location
> > +  in RAM where the compressed Image will be decompressed
> > temporarily. Once the
> > +  decompression is complete, decompressed data will be moved
> > kernel_addr_r for
> > +  booting.
> > +
> > +filesize:
> > +  Optional. This is only required if user wants to boot Linux from
> > a compressed
> > +  Image using booti command. It represents the size of the
> > compressed file. The
> > +  size has to at least the size of loaded image for decompression
> > to succeed.
> > +
> 
> I am not sure I like using filesize here compared to kernel_gz_size.
> It's true that $filesize will be set once your load the binary, e.g.
> from mmc.
> But in EXTLINUX/PXE situation $filesize could be wrong.
> 
> The load happens in this order:
> - initrd
> - kernel
> - fdt
> 
> So if I specify FDT in my EXTLINUX/PXE config the $filesize will be
> wrong
> while attempting to boot the kernel. Unless we save filesize of
> kernel after
> loading and set it again before calling do_booti() in pxe.c.
> 
> Currently I set kernel_gz_size in board environment, which is set to
> max
> allowed size.
> 
> david
> 

Ahh okay. There are two ways to fix it.

1. Fix the pxe implementation but save the filesize to be used later.

But some other user may fall into the same issue without realizing it
if the user loads any other image after compressed kernel Image.

We need clear documentation saying that, compressed kernel Image should
be loaded last before executing booti or $filesize must be set manually
before calling booti.

2. Just change it back to kernel_comp_size (compressed kernel image
size) to avoid any confusion.

Any preference ?

> >  pxefile_addr_r:
> > 
> >    Mandatory. The location in RAM where extlinux.conf will be
> > loaded to prior
> > diff --git a/doc/board/sifive/fu540.rst
> > b/doc/board/sifive/fu540.rst
> > index 7807f5b2c128..df2c5ad8d3e3 100644
> > --- a/doc/board/sifive/fu540.rst
> > +++ b/doc/board/sifive/fu540.rst
> > @@ -138,6 +138,10 @@ load uImage.
> >     => setenv netmask 255.255.252.0
> >     => setenv serverip 10.206.4.143
> >     => setenv gateway 10.206.4.1
> > +
> > +If you want to use a flat kernel image such as Image file
> > +
> > +.. code-block:: none
> >     => tftpboot ${kernel_addr_r} /sifive/fu540/Image
> >     ethernet at 10090000: PHY present at 0
> >     ethernet at 10090000: Starting autonegotiation...
> > @@ -177,6 +181,57 @@ load uImage.
> >              1.2 MiB/s
> >     done
> >     Bytes transferred = 8867100 (874d1c hex)
> > +
> > +Or if you want to use a compressed kernel image file such as
> > Image.gz
> > +
> > +.. code-block:: none
> > +   => tftpboot ${kernel_addr_r} /sifive/fu540/Image.gz
> > +   ethernet at 10090000: PHY present at 0
> > +   ethernet at 10090000: Starting autonegotiation...
> > +   ethernet at 10090000: Autonegotiation complete
> > +   ethernet at 10090000: link up, 1000Mbps full-duplex (lpa: 0x3c00)
> > +   Using ethernet at 10090000 device
> > +   TFTP from server 10.206.4.143; our IP address is 10.206.7.133
> > +   Filename '/sifive/fu540/Image.gz'.
> > +   Load address: 0x84000000
> > +   Loading:
> > #################################################################
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ######################################################
> > ###########
> > +            ##########################################
> > +            1.2 MiB/s
> > +   done
> > +   Bytes transferred = 4809458 (4962f2 hex)
> > +   =>setenv kernel_comp_addr_r 0x90000000
> > +   =>setenv filesize 0x500000
> > +
> > +By this time, correct kernel image is loaded and required
> > enviornment variables
> > +are set. You can proceed to load the ramdisk and device tree from
> > the tftp server
> > +as well.
> > +
> > +.. code-block:: none
> >     => tftpboot ${ramdisk_addr_r} /sifive/fu540/uRamdisk
> >     ethernet at 10090000: PHY present at 0
> >     ethernet at 10090000: Starting autonegotiation...
> > --
> > 2.24.0
> > 
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot

-- 
Regards,
Atish

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[Atish Patra]

On Wed, 2019-11-13 at 11:47 -0800, Atish Patra wrote:
> On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> > On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com>
> > wrote:
> > > Add compressed Image parsing support so that booti can parse both
> > > flat and compressed Image to boot Linux. Currently, it is
> > > difficult
> > > to calculate a safe address for every board where the compressed
> > > image can be decompressed. It is also not possible to figure out
> > > the
> > > size of the compressed file as well. Thus, user need to set two
> > > additional environment variables kernel_comp_addr_r and filesize
> > > to
> > > make this work.
> > > 
> > > Following compression methods are supported for now.
> > > lzma, lzo, bzip2, gzip.
> > > 
> > > lz4 support is not added as ARM64 kernel generates a lz4
> > > compressed
> > > image with legacy header which U-Boot doesn't know how to parse
> > > and
> > > decompress.
> > > 
> > > Tested on HiFive Unleashed and Qemu for RISC-V.
> > > Tested on Qemu for ARM64.
> > > 
> > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > ---
> > > I could not test this patch on any ARM64 boards due to lack of
> > > access to any ARM64 board. If anybody can test it on ARM64, that
> > > would be great.
> > > ---
> > >  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
> > >  doc/README.distro          | 12 +++++++++
> > >  doc/board/sifive/fu540.rst | 55
> > > ++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 106 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > index c36b0235df8c..cd8670a9a8db 100644
> > > --- a/cmd/booti.c
> > > +++ b/cmd/booti.c
> > > @@ -13,6 +13,7 @@
> > >  #include <linux/kernel.h>
> > >  #include <linux/sizes.h>
> > > 
> > > +DECLARE_GLOBAL_DATA_PTR;
> > >  /*
> > >   * Image booting support
> > >   */
> > > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > flag, int argc,
> > >         ulong ld;
> > >         ulong relocated_addr;
> > >         ulong image_size;
> > > +       uint8_t *temp;
> > > +       ulong dest;
> > > +       ulong dest_end;
> > > +       unsigned long comp_len;
> > > +       unsigned long decomp_len;
> > > +       int ctype;
> > > 
> > >         ret = do_bootm_states(cmdtp, flag, argc, argv,
> > > BOOTM_STATE_START,
> > >                               images, 1);
> > > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > flag, int argc,
> > >                 debug("*  kernel: cmdline image address =
> > > 0x%08lx\n", ld);
> > >         }
> > > 
> > > +       temp = map_sysmem(ld, 0);
> > > +       ctype = image_decomp_type(temp, 2);
> > > +       if (ctype > 0) {
> > > +               dest = env_get_ulong("kernel_comp_addr_r", 16,
> > > 0);
> > > +               comp_len = env_get_ulong("filesize", 16, 0);
> > > +               if (!dest || !comp_len) {
> > > +                       puts("kernel_comp_addr_r or filesize is
> > > not
> > > provided!\n");
> > > +                       return -EINVAL;
> > > +               }
> > > +               if (dest < gd->ram_base || dest > gd->ram_top) {
> > > +                       puts("kernel_comp_addr_r is outside of
> > > DRAM
> > > range!\n");
> > > +                       return -EINVAL;
> > > +               }
> > > +
> > > +               debug("kernel image compression type %d size =
> > > 0x%08lx address = 0x%08lx\n",
> > > +                       ctype, comp_len, (ulong)dest);
> > > +               decomp_len = comp_len * 10;
> > > +               ret = image_decomp(ctype, 0, ld, IH_TYPE_KERNEL,
> > > +                                (void *)dest, (void *)ld,
> > > comp_len,
> > > +                                decomp_len, &dest_end);
> > > +               if (ret)
> > > +                       return ret;
> > > +               /* dest_end contains the uncompressed Image size
> > > */
> > > +               memmove((void *) ld, (void *)dest, dest_end);
> > > +       }
> > > +       unmap_sysmem((void *)ld);
> > > +
> > >         ret = booti_setup(ld, &relocated_addr, &image_size,
> > > false);
> > >         if (ret != 0)
> > >                 return 1;
> > > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag, int
> > > argc, char * const argv[])
> > >  #ifdef CONFIG_SYS_LONGHELP
> > >  static char booti_help_text[] =
> > >         "[addr [initrd[:size]] [fdt]]\n"
> > > -       "    - boot Linux 'Image' stored at 'addr'\n"
> > > +       "    - boot Linux flat or compressed 'Image' stored at
> > > 'addr'\n"
> > >         "\tThe argument 'initrd' is optional and specifies the
> > > address\n"
> > >         "\tof an initrd in memory. The optional parameter ':size'
> > > allows\n"
> > >         "\tspecifying the size of a RAW initrd.\n"
> > > +       "\tCurrently only booting from gz, bz2, lzma and lz4
> > > compression\n"
> > > +       "\ttypes are supported. In order to boot from any of
> > > these
> > > compressed\n"
> > > +       "\timages, user have to set kernel_comp_addr_r and
> > > filesize
> > > enviornment\n"
> > > +       "\tvariables beforehand.\n"
> > >  #if defined(CONFIG_OF_LIBFDT)
> > >         "\tSince booting a Linux kernel requires a flat device-
> > > tree, a\n"
> > >         "\tthird argument providing the address of the device-
> > > tree
> > > blob\n"
> > > diff --git a/doc/README.distro b/doc/README.distro
> > > index ab6e6f4e74be..67b49e1e4b6a 100644
> > > --- a/doc/README.distro
> > > +++ b/doc/README.distro
> > > @@ -246,6 +246,18 @@ kernel_addr_r:
> > > 
> > >    A size of 16MB for the kernel is likely adequate.
> > > 
> > > +kernel_comp_addr_r:
> > > +  Optional. This is only required if user wants to boot Linux
> > > from
> > > a compressed
> > > +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It
> > > represents
> > > the location
> > > +  in RAM where the compressed Image will be decompressed
> > > temporarily. Once the
> > > +  decompression is complete, decompressed data will be moved
> > > kernel_addr_r for
> > > +  booting.
> > > +
> > > +filesize:
> > > +  Optional. This is only required if user wants to boot Linux
> > > from
> > > a compressed
> > > +  Image using booti command. It represents the size of the
> > > compressed file. The
> > > +  size has to at least the size of loaded image for
> > > decompression
> > > to succeed.
> > > +
> > 
> > I am not sure I like using filesize here compared to
> > kernel_gz_size.
> > It's true that $filesize will be set once your load the binary,
> > e.g.
> > from mmc.
> > But in EXTLINUX/PXE situation $filesize could be wrong.
> > 
> > The load happens in this order:
> > - initrd
> > - kernel
> > - fdt
> > 
> > So if I specify FDT in my EXTLINUX/PXE config the $filesize will be
> > wrong
> > while attempting to boot the kernel. Unless we save filesize of
> > kernel after
> > loading and set it again before calling do_booti() in pxe.c.
> > 
> > Currently I set kernel_gz_size in board environment, which is set
> > to
> > max
> > allowed size.
> > 
> > david
> > 
> 
> Ahh okay. There are two ways to fix it.
> 
> 1. Fix the pxe implementation but save the filesize to be used later.
> 
> But some other user may fall into the same issue without realizing it
> if the user loads any other image after compressed kernel Image.
> 
> We need clear documentation saying that, compressed kernel Image
> should
> be loaded last before executing booti or $filesize must be set
> manually
> before calling booti.
> 
> 2. Just change it back to kernel_comp_size (compressed kernel image
> size) to avoid any confusion.
> 
> Any preference ?
> 

Any thoughts ?

> > >  pxefile_addr_r:
> > > 
> > >    Mandatory. The location in RAM where extlinux.conf will be
> > > loaded to prior
> > > diff --git a/doc/board/sifive/fu540.rst
> > > b/doc/board/sifive/fu540.rst
> > > index 7807f5b2c128..df2c5ad8d3e3 100644
> > > --- a/doc/board/sifive/fu540.rst
> > > +++ b/doc/board/sifive/fu540.rst
> > > @@ -138,6 +138,10 @@ load uImage.
> > >     => setenv netmask 255.255.252.0
> > >     => setenv serverip 10.206.4.143
> > >     => setenv gateway 10.206.4.1
> > > +
> > > +If you want to use a flat kernel image such as Image file
> > > +
> > > +.. code-block:: none
> > >     => tftpboot ${kernel_addr_r} /sifive/fu540/Image
> > >     ethernet at 10090000: PHY present at 0
> > >     ethernet at 10090000: Starting autonegotiation...
> > > @@ -177,6 +181,57 @@ load uImage.
> > >              1.2 MiB/s
> > >     done
> > >     Bytes transferred = 8867100 (874d1c hex)
> > > +
> > > +Or if you want to use a compressed kernel image file such as
> > > Image.gz
> > > +
> > > +.. code-block:: none
> > > +   => tftpboot ${kernel_addr_r} /sifive/fu540/Image.gz
> > > +   ethernet at 10090000: PHY present at 0
> > > +   ethernet at 10090000: Starting autonegotiation...
> > > +   ethernet at 10090000: Autonegotiation complete
> > > +   ethernet at 10090000: link up, 1000Mbps full-duplex (lpa:
> > > 0x3c00)
> > > +   Using ethernet at 10090000 device
> > > +   TFTP from server 10.206.4.143; our IP address is 10.206.7.133
> > > +   Filename '/sifive/fu540/Image.gz'.
> > > +   Load address: 0x84000000
> > > +   Loading:
> > > #################################################################
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ####################################################
> > > ##
> > > ###########
> > > +            ##########################################
> > > +            1.2 MiB/s
> > > +   done
> > > +   Bytes transferred = 4809458 (4962f2 hex)
> > > +   =>setenv kernel_comp_addr_r 0x90000000
> > > +   =>setenv filesize 0x500000
> > > +
> > > +By this time, correct kernel image is loaded and required
> > > enviornment variables
> > > +are set. You can proceed to load the ramdisk and device tree
> > > from
> > > the tftp server
> > > +as well.
> > > +
> > > +.. code-block:: none
> > >     => tftpboot ${ramdisk_addr_r} /sifive/fu540/uRamdisk
> > >     ethernet at 10090000: PHY present at 0
> > >     ethernet at 10090000: Starting autonegotiation...
> > > --
> > > 2.24.0
> > > 
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot at lists.denx.de
> > > https://lists.denx.de/listinfo/u-boot

-- 
Regards,
Atish

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[Atish Patra]

On Fri, 2019-11-22 at 18:19 -0800, Atish Patra wrote:
> On Wed, 2019-11-13 at 11:47 -0800, Atish Patra wrote:
> > On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> > > On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com>
> > > wrote:
> > > > Add compressed Image parsing support so that booti can parse
> > > > both
> > > > flat and compressed Image to boot Linux. Currently, it is
> > > > difficult
> > > > to calculate a safe address for every board where the
> > > > compressed
> > > > image can be decompressed. It is also not possible to figure
> > > > out
> > > > the
> > > > size of the compressed file as well. Thus, user need to set two
> > > > additional environment variables kernel_comp_addr_r and
> > > > filesize
> > > > to
> > > > make this work.
> > > > 
> > > > Following compression methods are supported for now.
> > > > lzma, lzo, bzip2, gzip.
> > > > 
> > > > lz4 support is not added as ARM64 kernel generates a lz4
> > > > compressed
> > > > image with legacy header which U-Boot doesn't know how to parse
> > > > and
> > > > decompress.
> > > > 
> > > > Tested on HiFive Unleashed and Qemu for RISC-V.
> > > > Tested on Qemu for ARM64.
> > > > 
> > > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > > ---
> > > > I could not test this patch on any ARM64 boards due to lack of
> > > > access to any ARM64 board. If anybody can test it on ARM64,
> > > > that
> > > > would be great.
> > > > ---
> > > >  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
> > > >  doc/README.distro          | 12 +++++++++
> > > >  doc/board/sifive/fu540.rst | 55
> > > > ++++++++++++++++++++++++++++++++++++++
> > > >  3 files changed, 106 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > index c36b0235df8c..cd8670a9a8db 100644
> > > > --- a/cmd/booti.c
> > > > +++ b/cmd/booti.c
> > > > @@ -13,6 +13,7 @@
> > > >  #include <linux/kernel.h>
> > > >  #include <linux/sizes.h>
> > > > 
> > > > +DECLARE_GLOBAL_DATA_PTR;
> > > >  /*
> > > >   * Image booting support
> > > >   */
> > > > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > flag, int argc,
> > > >         ulong ld;
> > > >         ulong relocated_addr;
> > > >         ulong image_size;
> > > > +       uint8_t *temp;
> > > > +       ulong dest;
> > > > +       ulong dest_end;
> > > > +       unsigned long comp_len;
> > > > +       unsigned long decomp_len;
> > > > +       int ctype;
> > > > 
> > > >         ret = do_bootm_states(cmdtp, flag, argc, argv,
> > > > BOOTM_STATE_START,
> > > >                               images, 1);
> > > > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > flag, int argc,
> > > >                 debug("*  kernel: cmdline image address =
> > > > 0x%08lx\n", ld);
> > > >         }
> > > > 
> > > > +       temp = map_sysmem(ld, 0);
> > > > +       ctype = image_decomp_type(temp, 2);
> > > > +       if (ctype > 0) {
> > > > +               dest = env_get_ulong("kernel_comp_addr_r", 16,
> > > > 0);
> > > > +               comp_len = env_get_ulong("filesize", 16, 0);
> > > > +               if (!dest || !comp_len) {
> > > > +                       puts("kernel_comp_addr_r or filesize is
> > > > not
> > > > provided!\n");
> > > > +                       return -EINVAL;
> > > > +               }
> > > > +               if (dest < gd->ram_base || dest > gd->ram_top)
> > > > {
> > > > +                       puts("kernel_comp_addr_r is outside of
> > > > DRAM
> > > > range!\n");
> > > > +                       return -EINVAL;
> > > > +               }
> > > > +
> > > > +               debug("kernel image compression type %d size =
> > > > 0x%08lx address = 0x%08lx\n",
> > > > +                       ctype, comp_len, (ulong)dest);
> > > > +               decomp_len = comp_len * 10;
> > > > +               ret = image_decomp(ctype, 0, ld,
> > > > IH_TYPE_KERNEL,
> > > > +                                (void *)dest, (void *)ld,
> > > > comp_len,
> > > > +                                decomp_len, &dest_end);
> > > > +               if (ret)
> > > > +                       return ret;
> > > > +               /* dest_end contains the uncompressed Image
> > > > size
> > > > */
> > > > +               memmove((void *) ld, (void *)dest, dest_end);
> > > > +       }
> > > > +       unmap_sysmem((void *)ld);
> > > > +
> > > >         ret = booti_setup(ld, &relocated_addr, &image_size,
> > > > false);
> > > >         if (ret != 0)
> > > >                 return 1;
> > > > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag,
> > > > int
> > > > argc, char * const argv[])
> > > >  #ifdef CONFIG_SYS_LONGHELP
> > > >  static char booti_help_text[] =
> > > >         "[addr [initrd[:size]] [fdt]]\n"
> > > > -       "    - boot Linux 'Image' stored at 'addr'\n"
> > > > +       "    - boot Linux flat or compressed 'Image' stored at
> > > > 'addr'\n"
> > > >         "\tThe argument 'initrd' is optional and specifies the
> > > > address\n"
> > > >         "\tof an initrd in memory. The optional parameter
> > > > ':size'
> > > > allows\n"
> > > >         "\tspecifying the size of a RAW initrd.\n"
> > > > +       "\tCurrently only booting from gz, bz2, lzma and lz4
> > > > compression\n"
> > > > +       "\ttypes are supported. In order to boot from any of
> > > > these
> > > > compressed\n"
> > > > +       "\timages, user have to set kernel_comp_addr_r and
> > > > filesize
> > > > enviornment\n"
> > > > +       "\tvariables beforehand.\n"
> > > >  #if defined(CONFIG_OF_LIBFDT)
> > > >         "\tSince booting a Linux kernel requires a flat device-
> > > > tree, a\n"
> > > >         "\tthird argument providing the address of the device-
> > > > tree
> > > > blob\n"
> > > > diff --git a/doc/README.distro b/doc/README.distro
> > > > index ab6e6f4e74be..67b49e1e4b6a 100644
> > > > --- a/doc/README.distro
> > > > +++ b/doc/README.distro
> > > > @@ -246,6 +246,18 @@ kernel_addr_r:
> > > > 
> > > >    A size of 16MB for the kernel is likely adequate.
> > > > 
> > > > +kernel_comp_addr_r:
> > > > +  Optional. This is only required if user wants to boot Linux
> > > > from
> > > > a compressed
> > > > +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It
> > > > represents
> > > > the location
> > > > +  in RAM where the compressed Image will be decompressed
> > > > temporarily. Once the
> > > > +  decompression is complete, decompressed data will be moved
> > > > kernel_addr_r for
> > > > +  booting.
> > > > +
> > > > +filesize:
> > > > +  Optional. This is only required if user wants to boot Linux
> > > > from
> > > > a compressed
> > > > +  Image using booti command. It represents the size of the
> > > > compressed file. The
> > > > +  size has to at least the size of loaded image for
> > > > decompression
> > > > to succeed.
> > > > +
> > > 
> > > I am not sure I like using filesize here compared to
> > > kernel_gz_size.
> > > It's true that $filesize will be set once your load the binary,
> > > e.g.
> > > from mmc.
> > > But in EXTLINUX/PXE situation $filesize could be wrong.
> > > 
> > > The load happens in this order:
> > > - initrd
> > > - kernel
> > > - fdt
> > > 
> > > So if I specify FDT in my EXTLINUX/PXE config the $filesize will
> > > be
> > > wrong
> > > while attempting to boot the kernel. Unless we save filesize of
> > > kernel after
> > > loading and set it again before calling do_booti() in pxe.c.
> > > 
> > > Currently I set kernel_gz_size in board environment, which is set
> > > to
> > > max
> > > allowed size.
> > > 
> > > david
> > > 
> > 
> > Ahh okay. There are two ways to fix it.
> > 
> > 1. Fix the pxe implementation but save the filesize to be used
> > later.
> > 
> > But some other user may fall into the same issue without realizing
> > it
> > if the user loads any other image after compressed kernel Image.
> > 
> > We need clear documentation saying that, compressed kernel Image
> > should
> > be loaded last before executing booti or $filesize must be set
> > manually
> > before calling booti.
> > 
> > 2. Just change it back to kernel_comp_size (compressed kernel image
> > size) to avoid any confusion.
> > 
> > Any preference ?
> > 
> 
> Any thoughts ?
> 

I think this patch got lost during holidays :). Any suggestion on what
should be the best approach to resolve the issue with pxe
implementation ?


> > > >  pxefile_addr_r:
> > > > 
> > > >    Mandatory. The location in RAM where extlinux.conf will be
> > > > loaded to prior
> > > > diff --git a/doc/board/sifive/fu540.rst
> > > > b/doc/board/sifive/fu540.rst
> > > > index 7807f5b2c128..df2c5ad8d3e3 100644
> > > > --- a/doc/board/sifive/fu540.rst
> > > > +++ b/doc/board/sifive/fu540.rst
> > > > @@ -138,6 +138,10 @@ load uImage.
> > > >     => setenv netmask 255.255.252.0
> > > >     => setenv serverip 10.206.4.143
> > > >     => setenv gateway 10.206.4.1
> > > > +
> > > > +If you want to use a flat kernel image such as Image file
> > > > +
> > > > +.. code-block:: none
> > > >     => tftpboot ${kernel_addr_r} /sifive/fu540/Image
> > > >     ethernet at 10090000: PHY present at 0
> > > >     ethernet at 10090000: Starting autonegotiation...
> > > > @@ -177,6 +181,57 @@ load uImage.
> > > >              1.2 MiB/s
> > > >     done
> > > >     Bytes transferred = 8867100 (874d1c hex)
> > > > +
> > > > +Or if you want to use a compressed kernel image file such as
> > > > Image.gz
> > > > +
> > > > +.. code-block:: none
> > > > +   => tftpboot ${kernel_addr_r} /sifive/fu540/Image.gz
> > > > +   ethernet at 10090000: PHY present at 0
> > > > +   ethernet at 10090000: Starting autonegotiation...
> > > > +   ethernet at 10090000: Autonegotiation complete
> > > > +   ethernet at 10090000: link up, 1000Mbps full-duplex (lpa:
> > > > 0x3c00)
> > > > +   Using ethernet at 10090000 device
> > > > +   TFTP from server 10.206.4.143; our IP address is
> > > > 10.206.7.133
> > > > +   Filename '/sifive/fu540/Image.gz'.
> > > > +   Load address: 0x84000000
> > > > +   Loading:
> > > > ###############################################################
> > > > ##
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##################################################
> > > > ##
> > > > ##
> > > > ###########
> > > > +            ##########################################
> > > > +            1.2 MiB/s
> > > > +   done
> > > > +   Bytes transferred = 4809458 (4962f2 hex)
> > > > +   =>setenv kernel_comp_addr_r 0x90000000
> > > > +   =>setenv filesize 0x500000
> > > > +
> > > > +By this time, correct kernel image is loaded and required
> > > > enviornment variables
> > > > +are set. You can proceed to load the ramdisk and device tree
> > > > from
> > > > the tftp server
> > > > +as well.
> > > > +
> > > > +.. code-block:: none
> > > >     => tftpboot ${ramdisk_addr_r} /sifive/fu540/uRamdisk
> > > >     ethernet at 10090000: PHY present at 0
> > > >     ethernet at 10090000: Starting autonegotiation...
> > > > --
> > > > 2.24.0
> > > > 
> > > > _______________________________________________
> > > > U-Boot mailing list
> > > > U-Boot at lists.denx.de
> > > > https://lists.denx.de/listinfo/u-boot

-- 
Regards,
Atish

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[Tom Rini]

On Wed, Feb 05, 2020 at 12:01:38AM +0000, Atish Patra wrote:
> On Fri, 2019-11-22 at 18:19 -0800, Atish Patra wrote:
> > On Wed, 2019-11-13 at 11:47 -0800, Atish Patra wrote:
> > > On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> > > > On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com>
> > > > wrote:
> > > > > Add compressed Image parsing support so that booti can parse
> > > > > both
> > > > > flat and compressed Image to boot Linux. Currently, it is
> > > > > difficult
> > > > > to calculate a safe address for every board where the
> > > > > compressed
> > > > > image can be decompressed. It is also not possible to figure
> > > > > out
> > > > > the
> > > > > size of the compressed file as well. Thus, user need to set two
> > > > > additional environment variables kernel_comp_addr_r and
> > > > > filesize
> > > > > to
> > > > > make this work.
> > > > > 
> > > > > Following compression methods are supported for now.
> > > > > lzma, lzo, bzip2, gzip.
> > > > > 
> > > > > lz4 support is not added as ARM64 kernel generates a lz4
> > > > > compressed
> > > > > image with legacy header which U-Boot doesn't know how to parse
> > > > > and
> > > > > decompress.
> > > > > 
> > > > > Tested on HiFive Unleashed and Qemu for RISC-V.
> > > > > Tested on Qemu for ARM64.
> > > > > 
> > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > > > ---
> > > > > I could not test this patch on any ARM64 boards due to lack of
> > > > > access to any ARM64 board. If anybody can test it on ARM64,
> > > > > that
> > > > > would be great.
> > > > > ---
> > > > >  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
> > > > >  doc/README.distro          | 12 +++++++++
> > > > >  doc/board/sifive/fu540.rst | 55
> > > > > ++++++++++++++++++++++++++++++++++++++
> > > > >  3 files changed, 106 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > > index c36b0235df8c..cd8670a9a8db 100644
> > > > > --- a/cmd/booti.c
> > > > > +++ b/cmd/booti.c
> > > > > @@ -13,6 +13,7 @@
> > > > >  #include <linux/kernel.h>
> > > > >  #include <linux/sizes.h>
> > > > > 
> > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > >  /*
> > > > >   * Image booting support
> > > > >   */
> > > > > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > flag, int argc,
> > > > >         ulong ld;
> > > > >         ulong relocated_addr;
> > > > >         ulong image_size;
> > > > > +       uint8_t *temp;
> > > > > +       ulong dest;
> > > > > +       ulong dest_end;
> > > > > +       unsigned long comp_len;
> > > > > +       unsigned long decomp_len;
> > > > > +       int ctype;
> > > > > 
> > > > >         ret = do_bootm_states(cmdtp, flag, argc, argv,
> > > > > BOOTM_STATE_START,
> > > > >                               images, 1);
> > > > > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > flag, int argc,
> > > > >                 debug("*  kernel: cmdline image address =
> > > > > 0x%08lx\n", ld);
> > > > >         }
> > > > > 
> > > > > +       temp = map_sysmem(ld, 0);
> > > > > +       ctype = image_decomp_type(temp, 2);
> > > > > +       if (ctype > 0) {
> > > > > +               dest = env_get_ulong("kernel_comp_addr_r", 16,
> > > > > 0);
> > > > > +               comp_len = env_get_ulong("filesize", 16, 0);
> > > > > +               if (!dest || !comp_len) {
> > > > > +                       puts("kernel_comp_addr_r or filesize is
> > > > > not
> > > > > provided!\n");
> > > > > +                       return -EINVAL;
> > > > > +               }
> > > > > +               if (dest < gd->ram_base || dest > gd->ram_top)
> > > > > {
> > > > > +                       puts("kernel_comp_addr_r is outside of
> > > > > DRAM
> > > > > range!\n");
> > > > > +                       return -EINVAL;
> > > > > +               }
> > > > > +
> > > > > +               debug("kernel image compression type %d size =
> > > > > 0x%08lx address = 0x%08lx\n",
> > > > > +                       ctype, comp_len, (ulong)dest);
> > > > > +               decomp_len = comp_len * 10;
> > > > > +               ret = image_decomp(ctype, 0, ld,
> > > > > IH_TYPE_KERNEL,
> > > > > +                                (void *)dest, (void *)ld,
> > > > > comp_len,
> > > > > +                                decomp_len, &dest_end);
> > > > > +               if (ret)
> > > > > +                       return ret;
> > > > > +               /* dest_end contains the uncompressed Image
> > > > > size
> > > > > */
> > > > > +               memmove((void *) ld, (void *)dest, dest_end);
> > > > > +       }
> > > > > +       unmap_sysmem((void *)ld);
> > > > > +
> > > > >         ret = booti_setup(ld, &relocated_addr, &image_size,
> > > > > false);
> > > > >         if (ret != 0)
> > > > >                 return 1;
> > > > > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag,
> > > > > int
> > > > > argc, char * const argv[])
> > > > >  #ifdef CONFIG_SYS_LONGHELP
> > > > >  static char booti_help_text[] =
> > > > >         "[addr [initrd[:size]] [fdt]]\n"
> > > > > -       "    - boot Linux 'Image' stored at 'addr'\n"
> > > > > +       "    - boot Linux flat or compressed 'Image' stored at
> > > > > 'addr'\n"
> > > > >         "\tThe argument 'initrd' is optional and specifies the
> > > > > address\n"
> > > > >         "\tof an initrd in memory. The optional parameter
> > > > > ':size'
> > > > > allows\n"
> > > > >         "\tspecifying the size of a RAW initrd.\n"
> > > > > +       "\tCurrently only booting from gz, bz2, lzma and lz4
> > > > > compression\n"
> > > > > +       "\ttypes are supported. In order to boot from any of
> > > > > these
> > > > > compressed\n"
> > > > > +       "\timages, user have to set kernel_comp_addr_r and
> > > > > filesize
> > > > > enviornment\n"
> > > > > +       "\tvariables beforehand.\n"
> > > > >  #if defined(CONFIG_OF_LIBFDT)
> > > > >         "\tSince booting a Linux kernel requires a flat device-
> > > > > tree, a\n"
> > > > >         "\tthird argument providing the address of the device-
> > > > > tree
> > > > > blob\n"
> > > > > diff --git a/doc/README.distro b/doc/README.distro
> > > > > index ab6e6f4e74be..67b49e1e4b6a 100644
> > > > > --- a/doc/README.distro
> > > > > +++ b/doc/README.distro
> > > > > @@ -246,6 +246,18 @@ kernel_addr_r:
> > > > > 
> > > > >    A size of 16MB for the kernel is likely adequate.
> > > > > 
> > > > > +kernel_comp_addr_r:
> > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > from
> > > > > a compressed
> > > > > +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It
> > > > > represents
> > > > > the location
> > > > > +  in RAM where the compressed Image will be decompressed
> > > > > temporarily. Once the
> > > > > +  decompression is complete, decompressed data will be moved
> > > > > kernel_addr_r for
> > > > > +  booting.
> > > > > +
> > > > > +filesize:
> > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > from
> > > > > a compressed
> > > > > +  Image using booti command. It represents the size of the
> > > > > compressed file. The
> > > > > +  size has to at least the size of loaded image for
> > > > > decompression
> > > > > to succeed.
> > > > > +
> > > > 
> > > > I am not sure I like using filesize here compared to
> > > > kernel_gz_size.
> > > > It's true that $filesize will be set once your load the binary,
> > > > e.g.
> > > > from mmc.
> > > > But in EXTLINUX/PXE situation $filesize could be wrong.
> > > > 
> > > > The load happens in this order:
> > > > - initrd
> > > > - kernel
> > > > - fdt
> > > > 
> > > > So if I specify FDT in my EXTLINUX/PXE config the $filesize will
> > > > be
> > > > wrong
> > > > while attempting to boot the kernel. Unless we save filesize of
> > > > kernel after
> > > > loading and set it again before calling do_booti() in pxe.c.
> > > > 
> > > > Currently I set kernel_gz_size in board environment, which is set
> > > > to
> > > > max
> > > > allowed size.
> > > > 
> > > > david
> > > > 
> > > 
> > > Ahh okay. There are two ways to fix it.
> > > 
> > > 1. Fix the pxe implementation but save the filesize to be used
> > > later.
> > > 
> > > But some other user may fall into the same issue without realizing
> > > it
> > > if the user loads any other image after compressed kernel Image.
> > > 
> > > We need clear documentation saying that, compressed kernel Image
> > > should
> > > be loaded last before executing booti or $filesize must be set
> > > manually
> > > before calling booti.
> > > 
> > > 2. Just change it back to kernel_comp_size (compressed kernel image
> > > size) to avoid any confusion.
> > > 
> > > Any preference ?
> > > 
> > 
> > Any thoughts ?
> > 
> 
> I think this patch got lost during holidays :). Any suggestion on what
> should be the best approach to resolve the issue with pxe
> implementation ?

I think that we really need to be careful when adding "automagic"
features like this.  Thinking harder about it all, we can't re-use any
existing variable as there's going to be some case of a setup breaking
in strange silent ways.

Can we uncompress a single chunk / page / something of the compressed
image to get the header and thus know the uncompressed size?  We would
then know the compressed size is no larger than that and can progress
from there?

If not, then I guess we do need to go back to explicit variables for
compressed image size being used if the size isn't passed in on the
command line.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200213/3d9f06ed/attachment.sig>

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[David Abdurachmanov]

On Thu, Feb 13, 2020 at 6:17 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Feb 05, 2020 at 12:01:38AM +0000, Atish Patra wrote:
> > On Fri, 2019-11-22 at 18:19 -0800, Atish Patra wrote:
> > > On Wed, 2019-11-13 at 11:47 -0800, Atish Patra wrote:
> > > > On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> > > > > On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com>
> > > > > wrote:
> > > > > > Add compressed Image parsing support so that booti can parse
> > > > > > both
> > > > > > flat and compressed Image to boot Linux. Currently, it is
> > > > > > difficult
> > > > > > to calculate a safe address for every board where the
> > > > > > compressed
> > > > > > image can be decompressed. It is also not possible to figure
> > > > > > out
> > > > > > the
> > > > > > size of the compressed file as well. Thus, user need to set two
> > > > > > additional environment variables kernel_comp_addr_r and
> > > > > > filesize
> > > > > > to
> > > > > > make this work.
> > > > > >
> > > > > > Following compression methods are supported for now.
> > > > > > lzma, lzo, bzip2, gzip.
> > > > > >
> > > > > > lz4 support is not added as ARM64 kernel generates a lz4
> > > > > > compressed
> > > > > > image with legacy header which U-Boot doesn't know how to parse
> > > > > > and
> > > > > > decompress.
> > > > > >
> > > > > > Tested on HiFive Unleashed and Qemu for RISC-V.
> > > > > > Tested on Qemu for ARM64.
> > > > > >
> > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > > > > ---
> > > > > > I could not test this patch on any ARM64 boards due to lack of
> > > > > > access to any ARM64 board. If anybody can test it on ARM64,
> > > > > > that
> > > > > > would be great.
> > > > > > ---
> > > > > >  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
> > > > > >  doc/README.distro          | 12 +++++++++
> > > > > >  doc/board/sifive/fu540.rst | 55
> > > > > > ++++++++++++++++++++++++++++++++++++++
> > > > > >  3 files changed, 106 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > > > index c36b0235df8c..cd8670a9a8db 100644
> > > > > > --- a/cmd/booti.c
> > > > > > +++ b/cmd/booti.c
> > > > > > @@ -13,6 +13,7 @@
> > > > > >  #include <linux/kernel.h>
> > > > > >  #include <linux/sizes.h>
> > > > > >
> > > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > > >  /*
> > > > > >   * Image booting support
> > > > > >   */
> > > > > > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > flag, int argc,
> > > > > >         ulong ld;
> > > > > >         ulong relocated_addr;
> > > > > >         ulong image_size;
> > > > > > +       uint8_t *temp;
> > > > > > +       ulong dest;
> > > > > > +       ulong dest_end;
> > > > > > +       unsigned long comp_len;
> > > > > > +       unsigned long decomp_len;
> > > > > > +       int ctype;
> > > > > >
> > > > > >         ret = do_bootm_states(cmdtp, flag, argc, argv,
> > > > > > BOOTM_STATE_START,
> > > > > >                               images, 1);
> > > > > > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > flag, int argc,
> > > > > >                 debug("*  kernel: cmdline image address =
> > > > > > 0x%08lx\n", ld);
> > > > > >         }
> > > > > >
> > > > > > +       temp = map_sysmem(ld, 0);
> > > > > > +       ctype = image_decomp_type(temp, 2);
> > > > > > +       if (ctype > 0) {
> > > > > > +               dest = env_get_ulong("kernel_comp_addr_r", 16,
> > > > > > 0);
> > > > > > +               comp_len = env_get_ulong("filesize", 16, 0);
> > > > > > +               if (!dest || !comp_len) {
> > > > > > +                       puts("kernel_comp_addr_r or filesize is
> > > > > > not
> > > > > > provided!\n");
> > > > > > +                       return -EINVAL;
> > > > > > +               }
> > > > > > +               if (dest < gd->ram_base || dest > gd->ram_top)
> > > > > > {
> > > > > > +                       puts("kernel_comp_addr_r is outside of
> > > > > > DRAM
> > > > > > range!\n");
> > > > > > +                       return -EINVAL;
> > > > > > +               }
> > > > > > +
> > > > > > +               debug("kernel image compression type %d size =
> > > > > > 0x%08lx address = 0x%08lx\n",
> > > > > > +                       ctype, comp_len, (ulong)dest);
> > > > > > +               decomp_len = comp_len * 10;
> > > > > > +               ret = image_decomp(ctype, 0, ld,
> > > > > > IH_TYPE_KERNEL,
> > > > > > +                                (void *)dest, (void *)ld,
> > > > > > comp_len,
> > > > > > +                                decomp_len, &dest_end);
> > > > > > +               if (ret)
> > > > > > +                       return ret;
> > > > > > +               /* dest_end contains the uncompressed Image
> > > > > > size
> > > > > > */
> > > > > > +               memmove((void *) ld, (void *)dest, dest_end);
> > > > > > +       }
> > > > > > +       unmap_sysmem((void *)ld);
> > > > > > +
> > > > > >         ret = booti_setup(ld, &relocated_addr, &image_size,
> > > > > > false);
> > > > > >         if (ret != 0)
> > > > > >                 return 1;
> > > > > > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag,
> > > > > > int
> > > > > > argc, char * const argv[])
> > > > > >  #ifdef CONFIG_SYS_LONGHELP
> > > > > >  static char booti_help_text[] =
> > > > > >         "[addr [initrd[:size]] [fdt]]\n"
> > > > > > -       "    - boot Linux 'Image' stored at 'addr'\n"
> > > > > > +       "    - boot Linux flat or compressed 'Image' stored at
> > > > > > 'addr'\n"
> > > > > >         "\tThe argument 'initrd' is optional and specifies the
> > > > > > address\n"
> > > > > >         "\tof an initrd in memory. The optional parameter
> > > > > > ':size'
> > > > > > allows\n"
> > > > > >         "\tspecifying the size of a RAW initrd.\n"
> > > > > > +       "\tCurrently only booting from gz, bz2, lzma and lz4
> > > > > > compression\n"
> > > > > > +       "\ttypes are supported. In order to boot from any of
> > > > > > these
> > > > > > compressed\n"
> > > > > > +       "\timages, user have to set kernel_comp_addr_r and
> > > > > > filesize
> > > > > > enviornment\n"
> > > > > > +       "\tvariables beforehand.\n"
> > > > > >  #if defined(CONFIG_OF_LIBFDT)
> > > > > >         "\tSince booting a Linux kernel requires a flat device-
> > > > > > tree, a\n"
> > > > > >         "\tthird argument providing the address of the device-
> > > > > > tree
> > > > > > blob\n"
> > > > > > diff --git a/doc/README.distro b/doc/README.distro
> > > > > > index ab6e6f4e74be..67b49e1e4b6a 100644
> > > > > > --- a/doc/README.distro
> > > > > > +++ b/doc/README.distro
> > > > > > @@ -246,6 +246,18 @@ kernel_addr_r:
> > > > > >
> > > > > >    A size of 16MB for the kernel is likely adequate.
> > > > > >
> > > > > > +kernel_comp_addr_r:
> > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > from
> > > > > > a compressed
> > > > > > +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It
> > > > > > represents
> > > > > > the location
> > > > > > +  in RAM where the compressed Image will be decompressed
> > > > > > temporarily. Once the
> > > > > > +  decompression is complete, decompressed data will be moved
> > > > > > kernel_addr_r for
> > > > > > +  booting.
> > > > > > +
> > > > > > +filesize:
> > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > from
> > > > > > a compressed
> > > > > > +  Image using booti command. It represents the size of the
> > > > > > compressed file. The
> > > > > > +  size has to at least the size of loaded image for
> > > > > > decompression
> > > > > > to succeed.
> > > > > > +
> > > > >
> > > > > I am not sure I like using filesize here compared to
> > > > > kernel_gz_size.
> > > > > It's true that $filesize will be set once your load the binary,
> > > > > e.g.
> > > > > from mmc.
> > > > > But in EXTLINUX/PXE situation $filesize could be wrong.
> > > > >
> > > > > The load happens in this order:
> > > > > - initrd
> > > > > - kernel
> > > > > - fdt
> > > > >
> > > > > So if I specify FDT in my EXTLINUX/PXE config the $filesize will
> > > > > be
> > > > > wrong
> > > > > while attempting to boot the kernel. Unless we save filesize of
> > > > > kernel after
> > > > > loading and set it again before calling do_booti() in pxe.c.
> > > > >
> > > > > Currently I set kernel_gz_size in board environment, which is set
> > > > > to
> > > > > max
> > > > > allowed size.
> > > > >
> > > > > david
> > > > >
> > > >
> > > > Ahh okay. There are two ways to fix it.
> > > >
> > > > 1. Fix the pxe implementation but save the filesize to be used
> > > > later.
> > > >
> > > > But some other user may fall into the same issue without realizing
> > > > it
> > > > if the user loads any other image after compressed kernel Image.
> > > >
> > > > We need clear documentation saying that, compressed kernel Image
> > > > should
> > > > be loaded last before executing booti or $filesize must be set
> > > > manually
> > > > before calling booti.
> > > >
> > > > 2. Just change it back to kernel_comp_size (compressed kernel image
> > > > size) to avoid any confusion.
> > > >
> > > > Any preference ?
> > > >
> > >
> > > Any thoughts ?
> > >
> >
> > I think this patch got lost during holidays :). Any suggestion on what
> > should be the best approach to resolve the issue with pxe
> > implementation ?
>
> I think that we really need to be careful when adding "automagic"
> features like this.  Thinking harder about it all, we can't re-use any
> existing variable as there's going to be some case of a setup breaking
> in strange silent ways.
>
> Can we uncompress a single chunk / page / something of the compressed
> image to get the header and thus know the uncompressed size?  We would
> then know the compressed size is no larger than that and can progress
> from there?

After a quick google on various compression formats I get impression this
is not a common thing to record in the headers. The kernels could be
compressed with gzip, bzip2, lz4, lzma and lzo (I only checked Makefile for
 riscv).

>
> If not, then I guess we do need to go back to explicit variables for
> compressed image size being used if the size isn't passed in on the
> command line.
>
> --
> Tom

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[Tom Rini]

On Thu, Feb 13, 2020 at 11:32:52PM +0200, David Abdurachmanov wrote:
> On Thu, Feb 13, 2020 at 6:17 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Feb 05, 2020 at 12:01:38AM +0000, Atish Patra wrote:
> > > On Fri, 2019-11-22 at 18:19 -0800, Atish Patra wrote:
> > > > On Wed, 2019-11-13 at 11:47 -0800, Atish Patra wrote:
> > > > > On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> > > > > > On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com>
> > > > > > wrote:
> > > > > > > Add compressed Image parsing support so that booti can parse
> > > > > > > both
> > > > > > > flat and compressed Image to boot Linux. Currently, it is
> > > > > > > difficult
> > > > > > > to calculate a safe address for every board where the
> > > > > > > compressed
> > > > > > > image can be decompressed. It is also not possible to figure
> > > > > > > out
> > > > > > > the
> > > > > > > size of the compressed file as well. Thus, user need to set two
> > > > > > > additional environment variables kernel_comp_addr_r and
> > > > > > > filesize
> > > > > > > to
> > > > > > > make this work.
> > > > > > >
> > > > > > > Following compression methods are supported for now.
> > > > > > > lzma, lzo, bzip2, gzip.
> > > > > > >
> > > > > > > lz4 support is not added as ARM64 kernel generates a lz4
> > > > > > > compressed
> > > > > > > image with legacy header which U-Boot doesn't know how to parse
> > > > > > > and
> > > > > > > decompress.
> > > > > > >
> > > > > > > Tested on HiFive Unleashed and Qemu for RISC-V.
> > > > > > > Tested on Qemu for ARM64.
> > > > > > >
> > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > > > > > ---
> > > > > > > I could not test this patch on any ARM64 boards due to lack of
> > > > > > > access to any ARM64 board. If anybody can test it on ARM64,
> > > > > > > that
> > > > > > > would be great.
> > > > > > > ---
> > > > > > >  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
> > > > > > >  doc/README.distro          | 12 +++++++++
> > > > > > >  doc/board/sifive/fu540.rst | 55
> > > > > > > ++++++++++++++++++++++++++++++++++++++
> > > > > > >  3 files changed, 106 insertions(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > > > > index c36b0235df8c..cd8670a9a8db 100644
> > > > > > > --- a/cmd/booti.c
> > > > > > > +++ b/cmd/booti.c
> > > > > > > @@ -13,6 +13,7 @@
> > > > > > >  #include <linux/kernel.h>
> > > > > > >  #include <linux/sizes.h>
> > > > > > >
> > > > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > > > >  /*
> > > > > > >   * Image booting support
> > > > > > >   */
> > > > > > > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > flag, int argc,
> > > > > > >         ulong ld;
> > > > > > >         ulong relocated_addr;
> > > > > > >         ulong image_size;
> > > > > > > +       uint8_t *temp;
> > > > > > > +       ulong dest;
> > > > > > > +       ulong dest_end;
> > > > > > > +       unsigned long comp_len;
> > > > > > > +       unsigned long decomp_len;
> > > > > > > +       int ctype;
> > > > > > >
> > > > > > >         ret = do_bootm_states(cmdtp, flag, argc, argv,
> > > > > > > BOOTM_STATE_START,
> > > > > > >                               images, 1);
> > > > > > > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > flag, int argc,
> > > > > > >                 debug("*  kernel: cmdline image address =
> > > > > > > 0x%08lx\n", ld);
> > > > > > >         }
> > > > > > >
> > > > > > > +       temp = map_sysmem(ld, 0);
> > > > > > > +       ctype = image_decomp_type(temp, 2);
> > > > > > > +       if (ctype > 0) {
> > > > > > > +               dest = env_get_ulong("kernel_comp_addr_r", 16,
> > > > > > > 0);
> > > > > > > +               comp_len = env_get_ulong("filesize", 16, 0);
> > > > > > > +               if (!dest || !comp_len) {
> > > > > > > +                       puts("kernel_comp_addr_r or filesize is
> > > > > > > not
> > > > > > > provided!\n");
> > > > > > > +                       return -EINVAL;
> > > > > > > +               }
> > > > > > > +               if (dest < gd->ram_base || dest > gd->ram_top)
> > > > > > > {
> > > > > > > +                       puts("kernel_comp_addr_r is outside of
> > > > > > > DRAM
> > > > > > > range!\n");
> > > > > > > +                       return -EINVAL;
> > > > > > > +               }
> > > > > > > +
> > > > > > > +               debug("kernel image compression type %d size =
> > > > > > > 0x%08lx address = 0x%08lx\n",
> > > > > > > +                       ctype, comp_len, (ulong)dest);
> > > > > > > +               decomp_len = comp_len * 10;
> > > > > > > +               ret = image_decomp(ctype, 0, ld,
> > > > > > > IH_TYPE_KERNEL,
> > > > > > > +                                (void *)dest, (void *)ld,
> > > > > > > comp_len,
> > > > > > > +                                decomp_len, &dest_end);
> > > > > > > +               if (ret)
> > > > > > > +                       return ret;
> > > > > > > +               /* dest_end contains the uncompressed Image
> > > > > > > size
> > > > > > > */
> > > > > > > +               memmove((void *) ld, (void *)dest, dest_end);
> > > > > > > +       }
> > > > > > > +       unmap_sysmem((void *)ld);
> > > > > > > +
> > > > > > >         ret = booti_setup(ld, &relocated_addr, &image_size,
> > > > > > > false);
> > > > > > >         if (ret != 0)
> > > > > > >                 return 1;
> > > > > > > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag,
> > > > > > > int
> > > > > > > argc, char * const argv[])
> > > > > > >  #ifdef CONFIG_SYS_LONGHELP
> > > > > > >  static char booti_help_text[] =
> > > > > > >         "[addr [initrd[:size]] [fdt]]\n"
> > > > > > > -       "    - boot Linux 'Image' stored at 'addr'\n"
> > > > > > > +       "    - boot Linux flat or compressed 'Image' stored at
> > > > > > > 'addr'\n"
> > > > > > >         "\tThe argument 'initrd' is optional and specifies the
> > > > > > > address\n"
> > > > > > >         "\tof an initrd in memory. The optional parameter
> > > > > > > ':size'
> > > > > > > allows\n"
> > > > > > >         "\tspecifying the size of a RAW initrd.\n"
> > > > > > > +       "\tCurrently only booting from gz, bz2, lzma and lz4
> > > > > > > compression\n"
> > > > > > > +       "\ttypes are supported. In order to boot from any of
> > > > > > > these
> > > > > > > compressed\n"
> > > > > > > +       "\timages, user have to set kernel_comp_addr_r and
> > > > > > > filesize
> > > > > > > enviornment\n"
> > > > > > > +       "\tvariables beforehand.\n"
> > > > > > >  #if defined(CONFIG_OF_LIBFDT)
> > > > > > >         "\tSince booting a Linux kernel requires a flat device-
> > > > > > > tree, a\n"
> > > > > > >         "\tthird argument providing the address of the device-
> > > > > > > tree
> > > > > > > blob\n"
> > > > > > > diff --git a/doc/README.distro b/doc/README.distro
> > > > > > > index ab6e6f4e74be..67b49e1e4b6a 100644
> > > > > > > --- a/doc/README.distro
> > > > > > > +++ b/doc/README.distro
> > > > > > > @@ -246,6 +246,18 @@ kernel_addr_r:
> > > > > > >
> > > > > > >    A size of 16MB for the kernel is likely adequate.
> > > > > > >
> > > > > > > +kernel_comp_addr_r:
> > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > from
> > > > > > > a compressed
> > > > > > > +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It
> > > > > > > represents
> > > > > > > the location
> > > > > > > +  in RAM where the compressed Image will be decompressed
> > > > > > > temporarily. Once the
> > > > > > > +  decompression is complete, decompressed data will be moved
> > > > > > > kernel_addr_r for
> > > > > > > +  booting.
> > > > > > > +
> > > > > > > +filesize:
> > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > from
> > > > > > > a compressed
> > > > > > > +  Image using booti command. It represents the size of the
> > > > > > > compressed file. The
> > > > > > > +  size has to at least the size of loaded image for
> > > > > > > decompression
> > > > > > > to succeed.
> > > > > > > +
> > > > > >
> > > > > > I am not sure I like using filesize here compared to
> > > > > > kernel_gz_size.
> > > > > > It's true that $filesize will be set once your load the binary,
> > > > > > e.g.
> > > > > > from mmc.
> > > > > > But in EXTLINUX/PXE situation $filesize could be wrong.
> > > > > >
> > > > > > The load happens in this order:
> > > > > > - initrd
> > > > > > - kernel
> > > > > > - fdt
> > > > > >
> > > > > > So if I specify FDT in my EXTLINUX/PXE config the $filesize will
> > > > > > be
> > > > > > wrong
> > > > > > while attempting to boot the kernel. Unless we save filesize of
> > > > > > kernel after
> > > > > > loading and set it again before calling do_booti() in pxe.c.
> > > > > >
> > > > > > Currently I set kernel_gz_size in board environment, which is set
> > > > > > to
> > > > > > max
> > > > > > allowed size.
> > > > > >
> > > > > > david
> > > > > >
> > > > >
> > > > > Ahh okay. There are two ways to fix it.
> > > > >
> > > > > 1. Fix the pxe implementation but save the filesize to be used
> > > > > later.
> > > > >
> > > > > But some other user may fall into the same issue without realizing
> > > > > it
> > > > > if the user loads any other image after compressed kernel Image.
> > > > >
> > > > > We need clear documentation saying that, compressed kernel Image
> > > > > should
> > > > > be loaded last before executing booti or $filesize must be set
> > > > > manually
> > > > > before calling booti.
> > > > >
> > > > > 2. Just change it back to kernel_comp_size (compressed kernel image
> > > > > size) to avoid any confusion.
> > > > >
> > > > > Any preference ?
> > > > >
> > > >
> > > > Any thoughts ?
> > > >
> > >
> > > I think this patch got lost during holidays :). Any suggestion on what
> > > should be the best approach to resolve the issue with pxe
> > > implementation ?
> >
> > I think that we really need to be careful when adding "automagic"
> > features like this.  Thinking harder about it all, we can't re-use any
> > existing variable as there's going to be some case of a setup breaking
> > in strange silent ways.
> >
> > Can we uncompress a single chunk / page / something of the compressed
> > image to get the header and thus know the uncompressed size?  We would
> > then know the compressed size is no larger than that and can progress
> > from there?
> 
> After a quick google on various compression formats I get impression this
> is not a common thing to record in the headers. The kernels could be
> compressed with gzip, bzip2, lz4, lzma and lzo (I only checked Makefile for
>  riscv).
 
Sorry I wasn't clear enough.  The contents in question here contain a
header at the start which does contain the uncompressed image size.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200214/21d758db/attachment.sig>

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[Atish Patra]

On Fri, Feb 14, 2020 at 8:43 AM Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Feb 13, 2020 at 11:32:52PM +0200, David Abdurachmanov wrote:
> > On Thu, Feb 13, 2020 at 6:17 PM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Wed, Feb 05, 2020 at 12:01:38AM +0000, Atish Patra wrote:
> > > > On Fri, 2019-11-22 at 18:19 -0800, Atish Patra wrote:
> > > > > On Wed, 2019-11-13 at 11:47 -0800, Atish Patra wrote:
> > > > > > On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> > > > > > > On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com>
> > > > > > > wrote:
> > > > > > > > Add compressed Image parsing support so that booti can parse
> > > > > > > > both
> > > > > > > > flat and compressed Image to boot Linux. Currently, it is
> > > > > > > > difficult
> > > > > > > > to calculate a safe address for every board where the
> > > > > > > > compressed
> > > > > > > > image can be decompressed. It is also not possible to figure
> > > > > > > > out
> > > > > > > > the
> > > > > > > > size of the compressed file as well. Thus, user need to set two
> > > > > > > > additional environment variables kernel_comp_addr_r and
> > > > > > > > filesize
> > > > > > > > to
> > > > > > > > make this work.
> > > > > > > >
> > > > > > > > Following compression methods are supported for now.
> > > > > > > > lzma, lzo, bzip2, gzip.
> > > > > > > >
> > > > > > > > lz4 support is not added as ARM64 kernel generates a lz4
> > > > > > > > compressed
> > > > > > > > image with legacy header which U-Boot doesn't know how to parse
> > > > > > > > and
> > > > > > > > decompress.
> > > > > > > >
> > > > > > > > Tested on HiFive Unleashed and Qemu for RISC-V.
> > > > > > > > Tested on Qemu for ARM64.
> > > > > > > >
> > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > > > > > > ---
> > > > > > > > I could not test this patch on any ARM64 boards due to lack of
> > > > > > > > access to any ARM64 board. If anybody can test it on ARM64,
> > > > > > > > that
> > > > > > > > would be great.
> > > > > > > > ---
> > > > > > > >  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
> > > > > > > >  doc/README.distro          | 12 +++++++++
> > > > > > > >  doc/board/sifive/fu540.rst | 55
> > > > > > > > ++++++++++++++++++++++++++++++++++++++
> > > > > > > >  3 files changed, 106 insertions(+), 1 deletion(-)
> > > > > > > >
> > > > > > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > > > > > index c36b0235df8c..cd8670a9a8db 100644
> > > > > > > > --- a/cmd/booti.c
> > > > > > > > +++ b/cmd/booti.c
> > > > > > > > @@ -13,6 +13,7 @@
> > > > > > > >  #include <linux/kernel.h>
> > > > > > > >  #include <linux/sizes.h>
> > > > > > > >
> > > > > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > > > > >  /*
> > > > > > > >   * Image booting support
> > > > > > > >   */
> > > > > > > > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > flag, int argc,
> > > > > > > >         ulong ld;
> > > > > > > >         ulong relocated_addr;
> > > > > > > >         ulong image_size;
> > > > > > > > +       uint8_t *temp;
> > > > > > > > +       ulong dest;
> > > > > > > > +       ulong dest_end;
> > > > > > > > +       unsigned long comp_len;
> > > > > > > > +       unsigned long decomp_len;
> > > > > > > > +       int ctype;
> > > > > > > >
> > > > > > > >         ret = do_bootm_states(cmdtp, flag, argc, argv,
> > > > > > > > BOOTM_STATE_START,
> > > > > > > >                               images, 1);
> > > > > > > > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > flag, int argc,
> > > > > > > >                 debug("*  kernel: cmdline image address =
> > > > > > > > 0x%08lx\n", ld);
> > > > > > > >         }
> > > > > > > >
> > > > > > > > +       temp = map_sysmem(ld, 0);
> > > > > > > > +       ctype = image_decomp_type(temp, 2);
> > > > > > > > +       if (ctype > 0) {
> > > > > > > > +               dest = env_get_ulong("kernel_comp_addr_r", 16,
> > > > > > > > 0);
> > > > > > > > +               comp_len = env_get_ulong("filesize", 16, 0);
> > > > > > > > +               if (!dest || !comp_len) {
> > > > > > > > +                       puts("kernel_comp_addr_r or filesize is
> > > > > > > > not
> > > > > > > > provided!\n");
> > > > > > > > +                       return -EINVAL;
> > > > > > > > +               }
> > > > > > > > +               if (dest < gd->ram_base || dest > gd->ram_top)
> > > > > > > > {
> > > > > > > > +                       puts("kernel_comp_addr_r is outside of
> > > > > > > > DRAM
> > > > > > > > range!\n");
> > > > > > > > +                       return -EINVAL;
> > > > > > > > +               }
> > > > > > > > +
> > > > > > > > +               debug("kernel image compression type %d size =
> > > > > > > > 0x%08lx address = 0x%08lx\n",
> > > > > > > > +                       ctype, comp_len, (ulong)dest);
> > > > > > > > +               decomp_len = comp_len * 10;
> > > > > > > > +               ret = image_decomp(ctype, 0, ld,
> > > > > > > > IH_TYPE_KERNEL,
> > > > > > > > +                                (void *)dest, (void *)ld,
> > > > > > > > comp_len,
> > > > > > > > +                                decomp_len, &dest_end);
> > > > > > > > +               if (ret)
> > > > > > > > +                       return ret;
> > > > > > > > +               /* dest_end contains the uncompressed Image
> > > > > > > > size
> > > > > > > > */
> > > > > > > > +               memmove((void *) ld, (void *)dest, dest_end);
> > > > > > > > +       }
> > > > > > > > +       unmap_sysmem((void *)ld);
> > > > > > > > +
> > > > > > > >         ret = booti_setup(ld, &relocated_addr, &image_size,
> > > > > > > > false);
> > > > > > > >         if (ret != 0)
> > > > > > > >                 return 1;
> > > > > > > > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag,
> > > > > > > > int
> > > > > > > > argc, char * const argv[])
> > > > > > > >  #ifdef CONFIG_SYS_LONGHELP
> > > > > > > >  static char booti_help_text[] =
> > > > > > > >         "[addr [initrd[:size]] [fdt]]\n"
> > > > > > > > -       "    - boot Linux 'Image' stored at 'addr'\n"
> > > > > > > > +       "    - boot Linux flat or compressed 'Image' stored at
> > > > > > > > 'addr'\n"
> > > > > > > >         "\tThe argument 'initrd' is optional and specifies the
> > > > > > > > address\n"
> > > > > > > >         "\tof an initrd in memory. The optional parameter
> > > > > > > > ':size'
> > > > > > > > allows\n"
> > > > > > > >         "\tspecifying the size of a RAW initrd.\n"
> > > > > > > > +       "\tCurrently only booting from gz, bz2, lzma and lz4
> > > > > > > > compression\n"
> > > > > > > > +       "\ttypes are supported. In order to boot from any of
> > > > > > > > these
> > > > > > > > compressed\n"
> > > > > > > > +       "\timages, user have to set kernel_comp_addr_r and
> > > > > > > > filesize
> > > > > > > > enviornment\n"
> > > > > > > > +       "\tvariables beforehand.\n"
> > > > > > > >  #if defined(CONFIG_OF_LIBFDT)
> > > > > > > >         "\tSince booting a Linux kernel requires a flat device-
> > > > > > > > tree, a\n"
> > > > > > > >         "\tthird argument providing the address of the device-
> > > > > > > > tree
> > > > > > > > blob\n"
> > > > > > > > diff --git a/doc/README.distro b/doc/README.distro
> > > > > > > > index ab6e6f4e74be..67b49e1e4b6a 100644
> > > > > > > > --- a/doc/README.distro
> > > > > > > > +++ b/doc/README.distro
> > > > > > > > @@ -246,6 +246,18 @@ kernel_addr_r:
> > > > > > > >
> > > > > > > >    A size of 16MB for the kernel is likely adequate.
> > > > > > > >
> > > > > > > > +kernel_comp_addr_r:
> > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > from
> > > > > > > > a compressed
> > > > > > > > +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It
> > > > > > > > represents
> > > > > > > > the location
> > > > > > > > +  in RAM where the compressed Image will be decompressed
> > > > > > > > temporarily. Once the
> > > > > > > > +  decompression is complete, decompressed data will be moved
> > > > > > > > kernel_addr_r for
> > > > > > > > +  booting.
> > > > > > > > +
> > > > > > > > +filesize:
> > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > from
> > > > > > > > a compressed
> > > > > > > > +  Image using booti command. It represents the size of the
> > > > > > > > compressed file. The
> > > > > > > > +  size has to at least the size of loaded image for
> > > > > > > > decompression
> > > > > > > > to succeed.
> > > > > > > > +
> > > > > > >
> > > > > > > I am not sure I like using filesize here compared to
> > > > > > > kernel_gz_size.
> > > > > > > It's true that $filesize will be set once your load the binary,
> > > > > > > e.g.
> > > > > > > from mmc.
> > > > > > > But in EXTLINUX/PXE situation $filesize could be wrong.
> > > > > > >
> > > > > > > The load happens in this order:
> > > > > > > - initrd
> > > > > > > - kernel
> > > > > > > - fdt
> > > > > > >
> > > > > > > So if I specify FDT in my EXTLINUX/PXE config the $filesize will
> > > > > > > be
> > > > > > > wrong
> > > > > > > while attempting to boot the kernel. Unless we save filesize of
> > > > > > > kernel after
> > > > > > > loading and set it again before calling do_booti() in pxe.c.
> > > > > > >
> > > > > > > Currently I set kernel_gz_size in board environment, which is set
> > > > > > > to
> > > > > > > max
> > > > > > > allowed size.
> > > > > > >
> > > > > > > david
> > > > > > >
> > > > > >
> > > > > > Ahh okay. There are two ways to fix it.
> > > > > >
> > > > > > 1. Fix the pxe implementation but save the filesize to be used
> > > > > > later.
> > > > > >
> > > > > > But some other user may fall into the same issue without realizing
> > > > > > it
> > > > > > if the user loads any other image after compressed kernel Image.
> > > > > >
> > > > > > We need clear documentation saying that, compressed kernel Image
> > > > > > should
> > > > > > be loaded last before executing booti or $filesize must be set
> > > > > > manually
> > > > > > before calling booti.
> > > > > >
> > > > > > 2. Just change it back to kernel_comp_size (compressed kernel image
> > > > > > size) to avoid any confusion.
> > > > > >
> > > > > > Any preference ?
> > > > > >
> > > > >
> > > > > Any thoughts ?
> > > > >
> > > >
> > > > I think this patch got lost during holidays :). Any suggestion on what
> > > > should be the best approach to resolve the issue with pxe
> > > > implementation ?
> > >
> > > I think that we really need to be careful when adding "automagic"
> > > features like this.  Thinking harder about it all, we can't re-use any
> > > existing variable as there's going to be some case of a setup breaking
> > > in strange silent ways.
> > >
> > > Can we uncompress a single chunk / page / something of the compressed
> > > image to get the header and thus know the uncompressed size?  We would
> > > then know the compressed size is no larger than that and can progress
> > > from there?
> >
> > After a quick google on various compression formats I get impression this
> > is not a common thing to record in the headers. The kernels could be
> > compressed with gzip, bzip2, lz4, lzma and lzo (I only checked Makefile for
> >  riscv).
>
> Sorry I wasn't clear enough.  The contents in question here contain a
> header at the start which does contain the uncompressed image size.
>

The objective of the patch was to support all the compressed image
type that kernel supports.
i.e. gz, bz2, lzma, lzo.

uncompressed image size in compressed header in the following file formats:

1. gz: Uncompressed size is at the footer (http://www.zlib.org/rfc-gzip.html)
2. bz2: No information in the header
3. lzma: Present in the header
(https://svn.python.org/projects/external/xz-5.0.3/doc/lzma-file-format.txt)
4. lzo: Present in the header (https://gist.github.com/jledet/1333896)

To summarize, if we want to parse the compressed file to determine the
uncompressed size,
we have to add the support to generic compression library for all file
formats rather than just doing it here.
Additionally, not all format mandates the uncompressed size which
makes it difficult.

I have an alternate idea.
1. Save the compressed image size in an environment variable while
loading the image using tftpboot.
Thus, user doesn't have to save the compressed image size in that
variable. The variable can be just internal
and not exposed to the user.

> --
> Tom



-- 
Regards,
Atish

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[Tom Rini]

On Sun, Feb 16, 2020 at 04:48:22PM -0800, Atish Patra wrote:
> On Fri, Feb 14, 2020 at 8:43 AM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Feb 13, 2020 at 11:32:52PM +0200, David Abdurachmanov wrote:
> > > On Thu, Feb 13, 2020 at 6:17 PM Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Wed, Feb 05, 2020 at 12:01:38AM +0000, Atish Patra wrote:
> > > > > On Fri, 2019-11-22 at 18:19 -0800, Atish Patra wrote:
> > > > > > On Wed, 2019-11-13 at 11:47 -0800, Atish Patra wrote:
> > > > > > > On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> > > > > > > > On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com>
> > > > > > > > wrote:
> > > > > > > > > Add compressed Image parsing support so that booti can parse
> > > > > > > > > both
> > > > > > > > > flat and compressed Image to boot Linux. Currently, it is
> > > > > > > > > difficult
> > > > > > > > > to calculate a safe address for every board where the
> > > > > > > > > compressed
> > > > > > > > > image can be decompressed. It is also not possible to figure
> > > > > > > > > out
> > > > > > > > > the
> > > > > > > > > size of the compressed file as well. Thus, user need to set two
> > > > > > > > > additional environment variables kernel_comp_addr_r and
> > > > > > > > > filesize
> > > > > > > > > to
> > > > > > > > > make this work.
> > > > > > > > >
> > > > > > > > > Following compression methods are supported for now.
> > > > > > > > > lzma, lzo, bzip2, gzip.
> > > > > > > > >
> > > > > > > > > lz4 support is not added as ARM64 kernel generates a lz4
> > > > > > > > > compressed
> > > > > > > > > image with legacy header which U-Boot doesn't know how to parse
> > > > > > > > > and
> > > > > > > > > decompress.
> > > > > > > > >
> > > > > > > > > Tested on HiFive Unleashed and Qemu for RISC-V.
> > > > > > > > > Tested on Qemu for ARM64.
> > > > > > > > >
> > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > > > > > > > ---
> > > > > > > > > I could not test this patch on any ARM64 boards due to lack of
> > > > > > > > > access to any ARM64 board. If anybody can test it on ARM64,
> > > > > > > > > that
> > > > > > > > > would be great.
> > > > > > > > > ---
> > > > > > > > >  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
> > > > > > > > >  doc/README.distro          | 12 +++++++++
> > > > > > > > >  doc/board/sifive/fu540.rst | 55
> > > > > > > > > ++++++++++++++++++++++++++++++++++++++
> > > > > > > > >  3 files changed, 106 insertions(+), 1 deletion(-)
> > > > > > > > >
> > > > > > > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > > > > > > index c36b0235df8c..cd8670a9a8db 100644
> > > > > > > > > --- a/cmd/booti.c
> > > > > > > > > +++ b/cmd/booti.c
> > > > > > > > > @@ -13,6 +13,7 @@
> > > > > > > > >  #include <linux/kernel.h>
> > > > > > > > >  #include <linux/sizes.h>
> > > > > > > > >
> > > > > > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > > > > > >  /*
> > > > > > > > >   * Image booting support
> > > > > > > > >   */
> > > > > > > > > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > > flag, int argc,
> > > > > > > > >         ulong ld;
> > > > > > > > >         ulong relocated_addr;
> > > > > > > > >         ulong image_size;
> > > > > > > > > +       uint8_t *temp;
> > > > > > > > > +       ulong dest;
> > > > > > > > > +       ulong dest_end;
> > > > > > > > > +       unsigned long comp_len;
> > > > > > > > > +       unsigned long decomp_len;
> > > > > > > > > +       int ctype;
> > > > > > > > >
> > > > > > > > >         ret = do_bootm_states(cmdtp, flag, argc, argv,
> > > > > > > > > BOOTM_STATE_START,
> > > > > > > > >                               images, 1);
> > > > > > > > > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > > flag, int argc,
> > > > > > > > >                 debug("*  kernel: cmdline image address =
> > > > > > > > > 0x%08lx\n", ld);
> > > > > > > > >         }
> > > > > > > > >
> > > > > > > > > +       temp = map_sysmem(ld, 0);
> > > > > > > > > +       ctype = image_decomp_type(temp, 2);
> > > > > > > > > +       if (ctype > 0) {
> > > > > > > > > +               dest = env_get_ulong("kernel_comp_addr_r", 16,
> > > > > > > > > 0);
> > > > > > > > > +               comp_len = env_get_ulong("filesize", 16, 0);
> > > > > > > > > +               if (!dest || !comp_len) {
> > > > > > > > > +                       puts("kernel_comp_addr_r or filesize is
> > > > > > > > > not
> > > > > > > > > provided!\n");
> > > > > > > > > +                       return -EINVAL;
> > > > > > > > > +               }
> > > > > > > > > +               if (dest < gd->ram_base || dest > gd->ram_top)
> > > > > > > > > {
> > > > > > > > > +                       puts("kernel_comp_addr_r is outside of
> > > > > > > > > DRAM
> > > > > > > > > range!\n");
> > > > > > > > > +                       return -EINVAL;
> > > > > > > > > +               }
> > > > > > > > > +
> > > > > > > > > +               debug("kernel image compression type %d size =
> > > > > > > > > 0x%08lx address = 0x%08lx\n",
> > > > > > > > > +                       ctype, comp_len, (ulong)dest);
> > > > > > > > > +               decomp_len = comp_len * 10;
> > > > > > > > > +               ret = image_decomp(ctype, 0, ld,
> > > > > > > > > IH_TYPE_KERNEL,
> > > > > > > > > +                                (void *)dest, (void *)ld,
> > > > > > > > > comp_len,
> > > > > > > > > +                                decomp_len, &dest_end);
> > > > > > > > > +               if (ret)
> > > > > > > > > +                       return ret;
> > > > > > > > > +               /* dest_end contains the uncompressed Image
> > > > > > > > > size
> > > > > > > > > */
> > > > > > > > > +               memmove((void *) ld, (void *)dest, dest_end);
> > > > > > > > > +       }
> > > > > > > > > +       unmap_sysmem((void *)ld);
> > > > > > > > > +
> > > > > > > > >         ret = booti_setup(ld, &relocated_addr, &image_size,
> > > > > > > > > false);
> > > > > > > > >         if (ret != 0)
> > > > > > > > >                 return 1;
> > > > > > > > > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag,
> > > > > > > > > int
> > > > > > > > > argc, char * const argv[])
> > > > > > > > >  #ifdef CONFIG_SYS_LONGHELP
> > > > > > > > >  static char booti_help_text[] =
> > > > > > > > >         "[addr [initrd[:size]] [fdt]]\n"
> > > > > > > > > -       "    - boot Linux 'Image' stored at 'addr'\n"
> > > > > > > > > +       "    - boot Linux flat or compressed 'Image' stored at
> > > > > > > > > 'addr'\n"
> > > > > > > > >         "\tThe argument 'initrd' is optional and specifies the
> > > > > > > > > address\n"
> > > > > > > > >         "\tof an initrd in memory. The optional parameter
> > > > > > > > > ':size'
> > > > > > > > > allows\n"
> > > > > > > > >         "\tspecifying the size of a RAW initrd.\n"
> > > > > > > > > +       "\tCurrently only booting from gz, bz2, lzma and lz4
> > > > > > > > > compression\n"
> > > > > > > > > +       "\ttypes are supported. In order to boot from any of
> > > > > > > > > these
> > > > > > > > > compressed\n"
> > > > > > > > > +       "\timages, user have to set kernel_comp_addr_r and
> > > > > > > > > filesize
> > > > > > > > > enviornment\n"
> > > > > > > > > +       "\tvariables beforehand.\n"
> > > > > > > > >  #if defined(CONFIG_OF_LIBFDT)
> > > > > > > > >         "\tSince booting a Linux kernel requires a flat device-
> > > > > > > > > tree, a\n"
> > > > > > > > >         "\tthird argument providing the address of the device-
> > > > > > > > > tree
> > > > > > > > > blob\n"
> > > > > > > > > diff --git a/doc/README.distro b/doc/README.distro
> > > > > > > > > index ab6e6f4e74be..67b49e1e4b6a 100644
> > > > > > > > > --- a/doc/README.distro
> > > > > > > > > +++ b/doc/README.distro
> > > > > > > > > @@ -246,6 +246,18 @@ kernel_addr_r:
> > > > > > > > >
> > > > > > > > >    A size of 16MB for the kernel is likely adequate.
> > > > > > > > >
> > > > > > > > > +kernel_comp_addr_r:
> > > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > > from
> > > > > > > > > a compressed
> > > > > > > > > +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It
> > > > > > > > > represents
> > > > > > > > > the location
> > > > > > > > > +  in RAM where the compressed Image will be decompressed
> > > > > > > > > temporarily. Once the
> > > > > > > > > +  decompression is complete, decompressed data will be moved
> > > > > > > > > kernel_addr_r for
> > > > > > > > > +  booting.
> > > > > > > > > +
> > > > > > > > > +filesize:
> > > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > > from
> > > > > > > > > a compressed
> > > > > > > > > +  Image using booti command. It represents the size of the
> > > > > > > > > compressed file. The
> > > > > > > > > +  size has to at least the size of loaded image for
> > > > > > > > > decompression
> > > > > > > > > to succeed.
> > > > > > > > > +
> > > > > > > >
> > > > > > > > I am not sure I like using filesize here compared to
> > > > > > > > kernel_gz_size.
> > > > > > > > It's true that $filesize will be set once your load the binary,
> > > > > > > > e.g.
> > > > > > > > from mmc.
> > > > > > > > But in EXTLINUX/PXE situation $filesize could be wrong.
> > > > > > > >
> > > > > > > > The load happens in this order:
> > > > > > > > - initrd
> > > > > > > > - kernel
> > > > > > > > - fdt
> > > > > > > >
> > > > > > > > So if I specify FDT in my EXTLINUX/PXE config the $filesize will
> > > > > > > > be
> > > > > > > > wrong
> > > > > > > > while attempting to boot the kernel. Unless we save filesize of
> > > > > > > > kernel after
> > > > > > > > loading and set it again before calling do_booti() in pxe.c.
> > > > > > > >
> > > > > > > > Currently I set kernel_gz_size in board environment, which is set
> > > > > > > > to
> > > > > > > > max
> > > > > > > > allowed size.
> > > > > > > >
> > > > > > > > david
> > > > > > > >
> > > > > > >
> > > > > > > Ahh okay. There are two ways to fix it.
> > > > > > >
> > > > > > > 1. Fix the pxe implementation but save the filesize to be used
> > > > > > > later.
> > > > > > >
> > > > > > > But some other user may fall into the same issue without realizing
> > > > > > > it
> > > > > > > if the user loads any other image after compressed kernel Image.
> > > > > > >
> > > > > > > We need clear documentation saying that, compressed kernel Image
> > > > > > > should
> > > > > > > be loaded last before executing booti or $filesize must be set
> > > > > > > manually
> > > > > > > before calling booti.
> > > > > > >
> > > > > > > 2. Just change it back to kernel_comp_size (compressed kernel image
> > > > > > > size) to avoid any confusion.
> > > > > > >
> > > > > > > Any preference ?
> > > > > > >
> > > > > >
> > > > > > Any thoughts ?
> > > > > >
> > > > >
> > > > > I think this patch got lost during holidays :). Any suggestion on what
> > > > > should be the best approach to resolve the issue with pxe
> > > > > implementation ?
> > > >
> > > > I think that we really need to be careful when adding "automagic"
> > > > features like this.  Thinking harder about it all, we can't re-use any
> > > > existing variable as there's going to be some case of a setup breaking
> > > > in strange silent ways.
> > > >
> > > > Can we uncompress a single chunk / page / something of the compressed
> > > > image to get the header and thus know the uncompressed size?  We would
> > > > then know the compressed size is no larger than that and can progress
> > > > from there?
> > >
> > > After a quick google on various compression formats I get impression this
> > > is not a common thing to record in the headers. The kernels could be
> > > compressed with gzip, bzip2, lz4, lzma and lzo (I only checked Makefile for
> > >  riscv).
> >
> > Sorry I wasn't clear enough.  The contents in question here contain a
> > header at the start which does contain the uncompressed image size.
> >
> 
> The objective of the patch was to support all the compressed image
> type that kernel supports.
> i.e. gz, bz2, lzma, lzo.
> 
> uncompressed image size in compressed header in the following file formats:
> 
> 1. gz: Uncompressed size is at the footer (http://www.zlib.org/rfc-gzip.html)
> 2. bz2: No information in the header
> 3. lzma: Present in the header
> (https://svn.python.org/projects/external/xz-5.0.3/doc/lzma-file-format.txt)
> 4. lzo: Present in the header (https://gist.github.com/jledet/1333896)
> 
> To summarize, if we want to parse the compressed file to determine the
> uncompressed size,
> we have to add the support to generic compression library for all file
> formats rather than just doing it here.
> Additionally, not all format mandates the uncompressed size which
> makes it difficult.

Here's my confusion, and perhaps dumb question.  What happens if we:
1. Uncompress the first "chunk" of the data, which will let us at the
Image header.
2. Given that the Image header tells us the uncompressed size of the
file (image_size field), we know how much space we need in the end.
3. Uncompress to where it needs to go.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200218/d26a313d/attachment.sig>

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[David Abdurachmanov]

On Tue, Feb 18, 2020 at 10:38 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Feb 16, 2020 at 04:48:22PM -0800, Atish Patra wrote:
> > On Fri, Feb 14, 2020 at 8:43 AM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Thu, Feb 13, 2020 at 11:32:52PM +0200, David Abdurachmanov wrote:
> > > > On Thu, Feb 13, 2020 at 6:17 PM Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Wed, Feb 05, 2020 at 12:01:38AM +0000, Atish Patra wrote:
> > > > > > On Fri, 2019-11-22 at 18:19 -0800, Atish Patra wrote:
> > > > > > > On Wed, 2019-11-13 at 11:47 -0800, Atish Patra wrote:
> > > > > > > > On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> > > > > > > > > On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com>
> > > > > > > > > wrote:
> > > > > > > > > > Add compressed Image parsing support so that booti can parse
> > > > > > > > > > both
> > > > > > > > > > flat and compressed Image to boot Linux. Currently, it is
> > > > > > > > > > difficult
> > > > > > > > > > to calculate a safe address for every board where the
> > > > > > > > > > compressed
> > > > > > > > > > image can be decompressed. It is also not possible to figure
> > > > > > > > > > out
> > > > > > > > > > the
> > > > > > > > > > size of the compressed file as well. Thus, user need to set two
> > > > > > > > > > additional environment variables kernel_comp_addr_r and
> > > > > > > > > > filesize
> > > > > > > > > > to
> > > > > > > > > > make this work.
> > > > > > > > > >
> > > > > > > > > > Following compression methods are supported for now.
> > > > > > > > > > lzma, lzo, bzip2, gzip.
> > > > > > > > > >
> > > > > > > > > > lz4 support is not added as ARM64 kernel generates a lz4
> > > > > > > > > > compressed
> > > > > > > > > > image with legacy header which U-Boot doesn't know how to parse
> > > > > > > > > > and
> > > > > > > > > > decompress.
> > > > > > > > > >
> > > > > > > > > > Tested on HiFive Unleashed and Qemu for RISC-V.
> > > > > > > > > > Tested on Qemu for ARM64.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > > > > > > > > ---
> > > > > > > > > > I could not test this patch on any ARM64 boards due to lack of
> > > > > > > > > > access to any ARM64 board. If anybody can test it on ARM64,
> > > > > > > > > > that
> > > > > > > > > > would be great.
> > > > > > > > > > ---
> > > > > > > > > >  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
> > > > > > > > > >  doc/README.distro          | 12 +++++++++
> > > > > > > > > >  doc/board/sifive/fu540.rst | 55
> > > > > > > > > > ++++++++++++++++++++++++++++++++++++++
> > > > > > > > > >  3 files changed, 106 insertions(+), 1 deletion(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > > > > > > > index c36b0235df8c..cd8670a9a8db 100644
> > > > > > > > > > --- a/cmd/booti.c
> > > > > > > > > > +++ b/cmd/booti.c
> > > > > > > > > > @@ -13,6 +13,7 @@
> > > > > > > > > >  #include <linux/kernel.h>
> > > > > > > > > >  #include <linux/sizes.h>
> > > > > > > > > >
> > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > > > > > > >  /*
> > > > > > > > > >   * Image booting support
> > > > > > > > > >   */
> > > > > > > > > > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > > > flag, int argc,
> > > > > > > > > >         ulong ld;
> > > > > > > > > >         ulong relocated_addr;
> > > > > > > > > >         ulong image_size;
> > > > > > > > > > +       uint8_t *temp;
> > > > > > > > > > +       ulong dest;
> > > > > > > > > > +       ulong dest_end;
> > > > > > > > > > +       unsigned long comp_len;
> > > > > > > > > > +       unsigned long decomp_len;
> > > > > > > > > > +       int ctype;
> > > > > > > > > >
> > > > > > > > > >         ret = do_bootm_states(cmdtp, flag, argc, argv,
> > > > > > > > > > BOOTM_STATE_START,
> > > > > > > > > >                               images, 1);
> > > > > > > > > > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > > > flag, int argc,
> > > > > > > > > >                 debug("*  kernel: cmdline image address =
> > > > > > > > > > 0x%08lx\n", ld);
> > > > > > > > > >         }
> > > > > > > > > >
> > > > > > > > > > +       temp = map_sysmem(ld, 0);
> > > > > > > > > > +       ctype = image_decomp_type(temp, 2);
> > > > > > > > > > +       if (ctype > 0) {
> > > > > > > > > > +               dest = env_get_ulong("kernel_comp_addr_r", 16,
> > > > > > > > > > 0);
> > > > > > > > > > +               comp_len = env_get_ulong("filesize", 16, 0);
> > > > > > > > > > +               if (!dest || !comp_len) {
> > > > > > > > > > +                       puts("kernel_comp_addr_r or filesize is
> > > > > > > > > > not
> > > > > > > > > > provided!\n");
> > > > > > > > > > +                       return -EINVAL;
> > > > > > > > > > +               }
> > > > > > > > > > +               if (dest < gd->ram_base || dest > gd->ram_top)
> > > > > > > > > > {
> > > > > > > > > > +                       puts("kernel_comp_addr_r is outside of
> > > > > > > > > > DRAM
> > > > > > > > > > range!\n");
> > > > > > > > > > +                       return -EINVAL;
> > > > > > > > > > +               }
> > > > > > > > > > +
> > > > > > > > > > +               debug("kernel image compression type %d size =
> > > > > > > > > > 0x%08lx address = 0x%08lx\n",
> > > > > > > > > > +                       ctype, comp_len, (ulong)dest);
> > > > > > > > > > +               decomp_len = comp_len * 10;
> > > > > > > > > > +               ret = image_decomp(ctype, 0, ld,
> > > > > > > > > > IH_TYPE_KERNEL,
> > > > > > > > > > +                                (void *)dest, (void *)ld,
> > > > > > > > > > comp_len,
> > > > > > > > > > +                                decomp_len, &dest_end);
> > > > > > > > > > +               if (ret)
> > > > > > > > > > +                       return ret;
> > > > > > > > > > +               /* dest_end contains the uncompressed Image
> > > > > > > > > > size
> > > > > > > > > > */
> > > > > > > > > > +               memmove((void *) ld, (void *)dest, dest_end);
> > > > > > > > > > +       }
> > > > > > > > > > +       unmap_sysmem((void *)ld);
> > > > > > > > > > +
> > > > > > > > > >         ret = booti_setup(ld, &relocated_addr, &image_size,
> > > > > > > > > > false);
> > > > > > > > > >         if (ret != 0)
> > > > > > > > > >                 return 1;
> > > > > > > > > > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag,
> > > > > > > > > > int
> > > > > > > > > > argc, char * const argv[])
> > > > > > > > > >  #ifdef CONFIG_SYS_LONGHELP
> > > > > > > > > >  static char booti_help_text[] =
> > > > > > > > > >         "[addr [initrd[:size]] [fdt]]\n"
> > > > > > > > > > -       "    - boot Linux 'Image' stored at 'addr'\n"
> > > > > > > > > > +       "    - boot Linux flat or compressed 'Image' stored at
> > > > > > > > > > 'addr'\n"
> > > > > > > > > >         "\tThe argument 'initrd' is optional and specifies the
> > > > > > > > > > address\n"
> > > > > > > > > >         "\tof an initrd in memory. The optional parameter
> > > > > > > > > > ':size'
> > > > > > > > > > allows\n"
> > > > > > > > > >         "\tspecifying the size of a RAW initrd.\n"
> > > > > > > > > > +       "\tCurrently only booting from gz, bz2, lzma and lz4
> > > > > > > > > > compression\n"
> > > > > > > > > > +       "\ttypes are supported. In order to boot from any of
> > > > > > > > > > these
> > > > > > > > > > compressed\n"
> > > > > > > > > > +       "\timages, user have to set kernel_comp_addr_r and
> > > > > > > > > > filesize
> > > > > > > > > > enviornment\n"
> > > > > > > > > > +       "\tvariables beforehand.\n"
> > > > > > > > > >  #if defined(CONFIG_OF_LIBFDT)
> > > > > > > > > >         "\tSince booting a Linux kernel requires a flat device-
> > > > > > > > > > tree, a\n"
> > > > > > > > > >         "\tthird argument providing the address of the device-
> > > > > > > > > > tree
> > > > > > > > > > blob\n"
> > > > > > > > > > diff --git a/doc/README.distro b/doc/README.distro
> > > > > > > > > > index ab6e6f4e74be..67b49e1e4b6a 100644
> > > > > > > > > > --- a/doc/README.distro
> > > > > > > > > > +++ b/doc/README.distro
> > > > > > > > > > @@ -246,6 +246,18 @@ kernel_addr_r:
> > > > > > > > > >
> > > > > > > > > >    A size of 16MB for the kernel is likely adequate.
> > > > > > > > > >
> > > > > > > > > > +kernel_comp_addr_r:
> > > > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > > > from
> > > > > > > > > > a compressed
> > > > > > > > > > +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It
> > > > > > > > > > represents
> > > > > > > > > > the location
> > > > > > > > > > +  in RAM where the compressed Image will be decompressed
> > > > > > > > > > temporarily. Once the
> > > > > > > > > > +  decompression is complete, decompressed data will be moved
> > > > > > > > > > kernel_addr_r for
> > > > > > > > > > +  booting.
> > > > > > > > > > +
> > > > > > > > > > +filesize:
> > > > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > > > from
> > > > > > > > > > a compressed
> > > > > > > > > > +  Image using booti command. It represents the size of the
> > > > > > > > > > compressed file. The
> > > > > > > > > > +  size has to at least the size of loaded image for
> > > > > > > > > > decompression
> > > > > > > > > > to succeed.
> > > > > > > > > > +
> > > > > > > > >
> > > > > > > > > I am not sure I like using filesize here compared to
> > > > > > > > > kernel_gz_size.
> > > > > > > > > It's true that $filesize will be set once your load the binary,
> > > > > > > > > e.g.
> > > > > > > > > from mmc.
> > > > > > > > > But in EXTLINUX/PXE situation $filesize could be wrong.
> > > > > > > > >
> > > > > > > > > The load happens in this order:
> > > > > > > > > - initrd
> > > > > > > > > - kernel
> > > > > > > > > - fdt
> > > > > > > > >
> > > > > > > > > So if I specify FDT in my EXTLINUX/PXE config the $filesize will
> > > > > > > > > be
> > > > > > > > > wrong
> > > > > > > > > while attempting to boot the kernel. Unless we save filesize of
> > > > > > > > > kernel after
> > > > > > > > > loading and set it again before calling do_booti() in pxe.c.
> > > > > > > > >
> > > > > > > > > Currently I set kernel_gz_size in board environment, which is set
> > > > > > > > > to
> > > > > > > > > max
> > > > > > > > > allowed size.
> > > > > > > > >
> > > > > > > > > david
> > > > > > > > >
> > > > > > > >
> > > > > > > > Ahh okay. There are two ways to fix it.
> > > > > > > >
> > > > > > > > 1. Fix the pxe implementation but save the filesize to be used
> > > > > > > > later.
> > > > > > > >
> > > > > > > > But some other user may fall into the same issue without realizing
> > > > > > > > it
> > > > > > > > if the user loads any other image after compressed kernel Image.
> > > > > > > >
> > > > > > > > We need clear documentation saying that, compressed kernel Image
> > > > > > > > should
> > > > > > > > be loaded last before executing booti or $filesize must be set
> > > > > > > > manually
> > > > > > > > before calling booti.
> > > > > > > >
> > > > > > > > 2. Just change it back to kernel_comp_size (compressed kernel image
> > > > > > > > size) to avoid any confusion.
> > > > > > > >
> > > > > > > > Any preference ?
> > > > > > > >
> > > > > > >
> > > > > > > Any thoughts ?
> > > > > > >
> > > > > >
> > > > > > I think this patch got lost during holidays :). Any suggestion on what
> > > > > > should be the best approach to resolve the issue with pxe
> > > > > > implementation ?
> > > > >
> > > > > I think that we really need to be careful when adding "automagic"
> > > > > features like this.  Thinking harder about it all, we can't re-use any
> > > > > existing variable as there's going to be some case of a setup breaking
> > > > > in strange silent ways.
> > > > >
> > > > > Can we uncompress a single chunk / page / something of the compressed
> > > > > image to get the header and thus know the uncompressed size?  We would
> > > > > then know the compressed size is no larger than that and can progress
> > > > > from there?
> > > >
> > > > After a quick google on various compression formats I get impression this
> > > > is not a common thing to record in the headers. The kernels could be
> > > > compressed with gzip, bzip2, lz4, lzma and lzo (I only checked Makefile for
> > > >  riscv).
> > >
> > > Sorry I wasn't clear enough.  The contents in question here contain a
> > > header at the start which does contain the uncompressed image size.
> > >
> >
> > The objective of the patch was to support all the compressed image
> > type that kernel supports.
> > i.e. gz, bz2, lzma, lzo.
> >
> > uncompressed image size in compressed header in the following file formats:
> >
> > 1. gz: Uncompressed size is at the footer (http://www.zlib.org/rfc-gzip.html)
> > 2. bz2: No information in the header
> > 3. lzma: Present in the header
> > (https://svn.python.org/projects/external/xz-5.0.3/doc/lzma-file-format.txt)
> > 4. lzo: Present in the header (https://gist.github.com/jledet/1333896)
> >
> > To summarize, if we want to parse the compressed file to determine the
> > uncompressed size,
> > we have to add the support to generic compression library for all file
> > formats rather than just doing it here.
> > Additionally, not all format mandates the uncompressed size which
> > makes it difficult.
>
> Here's my confusion, and perhaps dumb question.  What happens if we:
> 1. Uncompress the first "chunk" of the data, which will let us at the
> Image header.

I still don't think that easy.

- gzip stores the uncompressed size at the end of the file, but that's valid
only for compressed files up to 4G in size. This might not be valid if
compressed with something else (e.g. pigz).
- XZ: https://tukaani.org/xz/xz-file-format-1.0.4.txt
 xz could have multiple streams, each stream could have multiple blocks.
 the uncompressed size is stored in block header (which is optional and
indicated by a flag if field is valid). The format also states that the only
reliable way to get uncompressed size is to uncompress. Quote:

        It should be noted that the only reliable way to determine
        the real uncompressed size is to uncompress the Block,
        because the Block Header and Index fields may contain
        (intentionally or unintentionally) invalid information.

Also the index is also stored at the end of the file.
- LZO: https://www.kernel.org/doc/Documentation/lzo.txt
I don't see anyone mentioning uncompressed size anywhere.
I think kernel uses pure LZO stream without any container (?).
No official spec exist, this is reverse engineered by reading kernel code.
- bzip2: here also an official spec doesn't exist. Reverse engineered
is here: https://github.com/dsnet/compress/blob/master/doc/bzip2-format.pdf
It doesn't store such information.

TL;DR figuring out uncompressed size (especially for multiple containers)
seems to be complicated, and you need to uncompress if you want to be
sure about the uncompressed size. That's the message I get the
more I google about it.

So we either pick a proper variable name (works now, but cannot be $filesize) or
we internally save $filesize after loading kernel binary and pass it
around in C code.

> 2. Given that the Image header tells us the uncompressed size of the
> file (image_size field), we know how much space we need in the end.
> 3. Uncompress to where it needs to go.
>
> --
> Tom

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[Atish Patra]

On Thu, Feb 20, 2020 at 1:14 PM David Abdurachmanov
<david.abdurachmanov@gmail.com> wrote:
>
> On Tue, Feb 18, 2020 at 10:38 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sun, Feb 16, 2020 at 04:48:22PM -0800, Atish Patra wrote:
> > > On Fri, Feb 14, 2020 at 8:43 AM Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Thu, Feb 13, 2020 at 11:32:52PM +0200, David Abdurachmanov wrote:
> > > > > On Thu, Feb 13, 2020 at 6:17 PM Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Wed, Feb 05, 2020 at 12:01:38AM +0000, Atish Patra wrote:
> > > > > > > On Fri, 2019-11-22 at 18:19 -0800, Atish Patra wrote:
> > > > > > > > On Wed, 2019-11-13 at 11:47 -0800, Atish Patra wrote:
> > > > > > > > > On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> > > > > > > > > > On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com>
> > > > > > > > > > wrote:
> > > > > > > > > > > Add compressed Image parsing support so that booti can parse
> > > > > > > > > > > both
> > > > > > > > > > > flat and compressed Image to boot Linux. Currently, it is
> > > > > > > > > > > difficult
> > > > > > > > > > > to calculate a safe address for every board where the
> > > > > > > > > > > compressed
> > > > > > > > > > > image can be decompressed. It is also not possible to figure
> > > > > > > > > > > out
> > > > > > > > > > > the
> > > > > > > > > > > size of the compressed file as well. Thus, user need to set two
> > > > > > > > > > > additional environment variables kernel_comp_addr_r and
> > > > > > > > > > > filesize
> > > > > > > > > > > to
> > > > > > > > > > > make this work.
> > > > > > > > > > >
> > > > > > > > > > > Following compression methods are supported for now.
> > > > > > > > > > > lzma, lzo, bzip2, gzip.
> > > > > > > > > > >
> > > > > > > > > > > lz4 support is not added as ARM64 kernel generates a lz4
> > > > > > > > > > > compressed
> > > > > > > > > > > image with legacy header which U-Boot doesn't know how to parse
> > > > > > > > > > > and
> > > > > > > > > > > decompress.
> > > > > > > > > > >
> > > > > > > > > > > Tested on HiFive Unleashed and Qemu for RISC-V.
> > > > > > > > > > > Tested on Qemu for ARM64.
> > > > > > > > > > >
> > > > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > > > > > > > > > ---
> > > > > > > > > > > I could not test this patch on any ARM64 boards due to lack of
> > > > > > > > > > > access to any ARM64 board. If anybody can test it on ARM64,
> > > > > > > > > > > that
> > > > > > > > > > > would be great.
> > > > > > > > > > > ---
> > > > > > > > > > >  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
> > > > > > > > > > >  doc/README.distro          | 12 +++++++++
> > > > > > > > > > >  doc/board/sifive/fu540.rst | 55
> > > > > > > > > > > ++++++++++++++++++++++++++++++++++++++
> > > > > > > > > > >  3 files changed, 106 insertions(+), 1 deletion(-)
> > > > > > > > > > >
> > > > > > > > > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > > > > > > > > index c36b0235df8c..cd8670a9a8db 100644
> > > > > > > > > > > --- a/cmd/booti.c
> > > > > > > > > > > +++ b/cmd/booti.c
> > > > > > > > > > > @@ -13,6 +13,7 @@
> > > > > > > > > > >  #include <linux/kernel.h>
> > > > > > > > > > >  #include <linux/sizes.h>
> > > > > > > > > > >
> > > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > > > > > > > >  /*
> > > > > > > > > > >   * Image booting support
> > > > > > > > > > >   */
> > > > > > > > > > > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > > > > flag, int argc,
> > > > > > > > > > >         ulong ld;
> > > > > > > > > > >         ulong relocated_addr;
> > > > > > > > > > >         ulong image_size;
> > > > > > > > > > > +       uint8_t *temp;
> > > > > > > > > > > +       ulong dest;
> > > > > > > > > > > +       ulong dest_end;
> > > > > > > > > > > +       unsigned long comp_len;
> > > > > > > > > > > +       unsigned long decomp_len;
> > > > > > > > > > > +       int ctype;
> > > > > > > > > > >
> > > > > > > > > > >         ret = do_bootm_states(cmdtp, flag, argc, argv,
> > > > > > > > > > > BOOTM_STATE_START,
> > > > > > > > > > >                               images, 1);
> > > > > > > > > > > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > > > > flag, int argc,
> > > > > > > > > > >                 debug("*  kernel: cmdline image address =
> > > > > > > > > > > 0x%08lx\n", ld);
> > > > > > > > > > >         }
> > > > > > > > > > >
> > > > > > > > > > > +       temp = map_sysmem(ld, 0);
> > > > > > > > > > > +       ctype = image_decomp_type(temp, 2);
> > > > > > > > > > > +       if (ctype > 0) {
> > > > > > > > > > > +               dest = env_get_ulong("kernel_comp_addr_r", 16,
> > > > > > > > > > > 0);
> > > > > > > > > > > +               comp_len = env_get_ulong("filesize", 16, 0);
> > > > > > > > > > > +               if (!dest || !comp_len) {
> > > > > > > > > > > +                       puts("kernel_comp_addr_r or filesize is
> > > > > > > > > > > not
> > > > > > > > > > > provided!\n");
> > > > > > > > > > > +                       return -EINVAL;
> > > > > > > > > > > +               }
> > > > > > > > > > > +               if (dest < gd->ram_base || dest > gd->ram_top)
> > > > > > > > > > > {
> > > > > > > > > > > +                       puts("kernel_comp_addr_r is outside of
> > > > > > > > > > > DRAM
> > > > > > > > > > > range!\n");
> > > > > > > > > > > +                       return -EINVAL;
> > > > > > > > > > > +               }
> > > > > > > > > > > +
> > > > > > > > > > > +               debug("kernel image compression type %d size =
> > > > > > > > > > > 0x%08lx address = 0x%08lx\n",
> > > > > > > > > > > +                       ctype, comp_len, (ulong)dest);
> > > > > > > > > > > +               decomp_len = comp_len * 10;
> > > > > > > > > > > +               ret = image_decomp(ctype, 0, ld,
> > > > > > > > > > > IH_TYPE_KERNEL,
> > > > > > > > > > > +                                (void *)dest, (void *)ld,
> > > > > > > > > > > comp_len,
> > > > > > > > > > > +                                decomp_len, &dest_end);
> > > > > > > > > > > +               if (ret)
> > > > > > > > > > > +                       return ret;
> > > > > > > > > > > +               /* dest_end contains the uncompressed Image
> > > > > > > > > > > size
> > > > > > > > > > > */
> > > > > > > > > > > +               memmove((void *) ld, (void *)dest, dest_end);
> > > > > > > > > > > +       }
> > > > > > > > > > > +       unmap_sysmem((void *)ld);
> > > > > > > > > > > +
> > > > > > > > > > >         ret = booti_setup(ld, &relocated_addr, &image_size,
> > > > > > > > > > > false);
> > > > > > > > > > >         if (ret != 0)
> > > > > > > > > > >                 return 1;
> > > > > > > > > > > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag,
> > > > > > > > > > > int
> > > > > > > > > > > argc, char * const argv[])
> > > > > > > > > > >  #ifdef CONFIG_SYS_LONGHELP
> > > > > > > > > > >  static char booti_help_text[] =
> > > > > > > > > > >         "[addr [initrd[:size]] [fdt]]\n"
> > > > > > > > > > > -       "    - boot Linux 'Image' stored at 'addr'\n"
> > > > > > > > > > > +       "    - boot Linux flat or compressed 'Image' stored at
> > > > > > > > > > > 'addr'\n"
> > > > > > > > > > >         "\tThe argument 'initrd' is optional and specifies the
> > > > > > > > > > > address\n"
> > > > > > > > > > >         "\tof an initrd in memory. The optional parameter
> > > > > > > > > > > ':size'
> > > > > > > > > > > allows\n"
> > > > > > > > > > >         "\tspecifying the size of a RAW initrd.\n"
> > > > > > > > > > > +       "\tCurrently only booting from gz, bz2, lzma and lz4
> > > > > > > > > > > compression\n"
> > > > > > > > > > > +       "\ttypes are supported. In order to boot from any of
> > > > > > > > > > > these
> > > > > > > > > > > compressed\n"
> > > > > > > > > > > +       "\timages, user have to set kernel_comp_addr_r and
> > > > > > > > > > > filesize
> > > > > > > > > > > enviornment\n"
> > > > > > > > > > > +       "\tvariables beforehand.\n"
> > > > > > > > > > >  #if defined(CONFIG_OF_LIBFDT)
> > > > > > > > > > >         "\tSince booting a Linux kernel requires a flat device-
> > > > > > > > > > > tree, a\n"
> > > > > > > > > > >         "\tthird argument providing the address of the device-
> > > > > > > > > > > tree
> > > > > > > > > > > blob\n"
> > > > > > > > > > > diff --git a/doc/README.distro b/doc/README.distro
> > > > > > > > > > > index ab6e6f4e74be..67b49e1e4b6a 100644
> > > > > > > > > > > --- a/doc/README.distro
> > > > > > > > > > > +++ b/doc/README.distro
> > > > > > > > > > > @@ -246,6 +246,18 @@ kernel_addr_r:
> > > > > > > > > > >
> > > > > > > > > > >    A size of 16MB for the kernel is likely adequate.
> > > > > > > > > > >
> > > > > > > > > > > +kernel_comp_addr_r:
> > > > > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > > > > from
> > > > > > > > > > > a compressed
> > > > > > > > > > > +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It
> > > > > > > > > > > represents
> > > > > > > > > > > the location
> > > > > > > > > > > +  in RAM where the compressed Image will be decompressed
> > > > > > > > > > > temporarily. Once the
> > > > > > > > > > > +  decompression is complete, decompressed data will be moved
> > > > > > > > > > > kernel_addr_r for
> > > > > > > > > > > +  booting.
> > > > > > > > > > > +
> > > > > > > > > > > +filesize:
> > > > > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > > > > from
> > > > > > > > > > > a compressed
> > > > > > > > > > > +  Image using booti command. It represents the size of the
> > > > > > > > > > > compressed file. The
> > > > > > > > > > > +  size has to at least the size of loaded image for
> > > > > > > > > > > decompression
> > > > > > > > > > > to succeed.
> > > > > > > > > > > +
> > > > > > > > > >
> > > > > > > > > > I am not sure I like using filesize here compared to
> > > > > > > > > > kernel_gz_size.
> > > > > > > > > > It's true that $filesize will be set once your load the binary,
> > > > > > > > > > e.g.
> > > > > > > > > > from mmc.
> > > > > > > > > > But in EXTLINUX/PXE situation $filesize could be wrong.
> > > > > > > > > >
> > > > > > > > > > The load happens in this order:
> > > > > > > > > > - initrd
> > > > > > > > > > - kernel
> > > > > > > > > > - fdt
> > > > > > > > > >
> > > > > > > > > > So if I specify FDT in my EXTLINUX/PXE config the $filesize will
> > > > > > > > > > be
> > > > > > > > > > wrong
> > > > > > > > > > while attempting to boot the kernel. Unless we save filesize of
> > > > > > > > > > kernel after
> > > > > > > > > > loading and set it again before calling do_booti() in pxe.c.
> > > > > > > > > >
> > > > > > > > > > Currently I set kernel_gz_size in board environment, which is set
> > > > > > > > > > to
> > > > > > > > > > max
> > > > > > > > > > allowed size.
> > > > > > > > > >
> > > > > > > > > > david
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Ahh okay. There are two ways to fix it.
> > > > > > > > >
> > > > > > > > > 1. Fix the pxe implementation but save the filesize to be used
> > > > > > > > > later.
> > > > > > > > >
> > > > > > > > > But some other user may fall into the same issue without realizing
> > > > > > > > > it
> > > > > > > > > if the user loads any other image after compressed kernel Image.
> > > > > > > > >
> > > > > > > > > We need clear documentation saying that, compressed kernel Image
> > > > > > > > > should
> > > > > > > > > be loaded last before executing booti or $filesize must be set
> > > > > > > > > manually
> > > > > > > > > before calling booti.
> > > > > > > > >
> > > > > > > > > 2. Just change it back to kernel_comp_size (compressed kernel image
> > > > > > > > > size) to avoid any confusion.
> > > > > > > > >
> > > > > > > > > Any preference ?
> > > > > > > > >
> > > > > > > >
> > > > > > > > Any thoughts ?
> > > > > > > >
> > > > > > >
> > > > > > > I think this patch got lost during holidays :). Any suggestion on what
> > > > > > > should be the best approach to resolve the issue with pxe
> > > > > > > implementation ?
> > > > > >
> > > > > > I think that we really need to be careful when adding "automagic"
> > > > > > features like this.  Thinking harder about it all, we can't re-use any
> > > > > > existing variable as there's going to be some case of a setup breaking
> > > > > > in strange silent ways.
> > > > > >
> > > > > > Can we uncompress a single chunk / page / something of the compressed
> > > > > > image to get the header and thus know the uncompressed size?  We would
> > > > > > then know the compressed size is no larger than that and can progress
> > > > > > from there?
> > > > >
> > > > > After a quick google on various compression formats I get impression this
> > > > > is not a common thing to record in the headers. The kernels could be
> > > > > compressed with gzip, bzip2, lz4, lzma and lzo (I only checked Makefile for
> > > > >  riscv).
> > > >
> > > > Sorry I wasn't clear enough.  The contents in question here contain a
> > > > header at the start which does contain the uncompressed image size.
> > > >
> > >
> > > The objective of the patch was to support all the compressed image
> > > type that kernel supports.
> > > i.e. gz, bz2, lzma, lzo.
> > >
> > > uncompressed image size in compressed header in the following file formats:
> > >
> > > 1. gz: Uncompressed size is at the footer (http://www.zlib.org/rfc-gzip.html)
> > > 2. bz2: No information in the header
> > > 3. lzma: Present in the header
> > > (https://svn.python.org/projects/external/xz-5.0.3/doc/lzma-file-format.txt)
> > > 4. lzo: Present in the header (https://gist.github.com/jledet/1333896)
> > >
> > > To summarize, if we want to parse the compressed file to determine the
> > > uncompressed size,
> > > we have to add the support to generic compression library for all file
> > > formats rather than just doing it here.
> > > Additionally, not all format mandates the uncompressed size which
> > > makes it difficult.
> >
> > Here's my confusion, and perhaps dumb question.  What happens if we:
> > 1. Uncompress the first "chunk" of the data, which will let us at the
> > Image header.
>
> I still don't think that easy.
>

David,
I think we misunderstood Tom's feedback. Tom is asking to decompress
initial chunk of
compressed data to so that we have access to kernel Image header.
Uncompressed image
size can be retrieved from header and set it as the compressed data
size which should do the trick.

I think the word "header" was the source of confusion. Thanks for the
suggestion. I will try this at my end.


> - gzip stores the uncompressed size at the end of the file, but that's valid
> only for compressed files up to 4G in size. This might not be valid if
> compressed with something else (e.g. pigz).
> - XZ: https://tukaani.org/xz/xz-file-format-1.0.4.txt
>  xz could have multiple streams, each stream could have multiple blocks.
>  the uncompressed size is stored in block header (which is optional and
> indicated by a flag if field is valid). The format also states that the only
> reliable way to get uncompressed size is to uncompress. Quote:
>
>         It should be noted that the only reliable way to determine
>         the real uncompressed size is to uncompress the Block,
>         because the Block Header and Index fields may contain
>         (intentionally or unintentionally) invalid information.
>
> Also the index is also stored at the end of the file.
> - LZO: https://www.kernel.org/doc/Documentation/lzo.txt
> I don't see anyone mentioning uncompressed size anywhere.
> I think kernel uses pure LZO stream without any container (?).
> No official spec exist, this is reverse engineered by reading kernel code.
> - bzip2: here also an official spec doesn't exist. Reverse engineered
> is here: https://github.com/dsnet/compress/blob/master/doc/bzip2-format.pdf
> It doesn't store such information.
>
> TL;DR figuring out uncompressed size (especially for multiple containers)
> seems to be complicated, and you need to uncompress if you want to be
> sure about the uncompressed size. That's the message I get the
> more I google about it.
>
> So we either pick a proper variable name (works now, but cannot be $filesize) or
> we internally save $filesize after loading kernel binary and pass it
> around in C code.
>
> > 2. Given that the Image header tells us the uncompressed size of the
> > file (image_size field), we know how much space we need in the end.
> > 3. Uncompress to where it needs to go.
> >
> > --
> > Tom



--
Regards,
Atish

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[Atish Patra]

On Thu, Feb 20, 2020 at 2:25 PM Atish Patra <atishp@atishpatra.org> wrote:
>
> On Thu, Feb 20, 2020 at 1:14 PM David Abdurachmanov
> <david.abdurachmanov@gmail.com> wrote:
> >
> > On Tue, Feb 18, 2020 at 10:38 PM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sun, Feb 16, 2020 at 04:48:22PM -0800, Atish Patra wrote:
> > > > On Fri, Feb 14, 2020 at 8:43 AM Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Thu, Feb 13, 2020 at 11:32:52PM +0200, David Abdurachmanov wrote:
> > > > > > On Thu, Feb 13, 2020 at 6:17 PM Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Wed, Feb 05, 2020 at 12:01:38AM +0000, Atish Patra wrote:
> > > > > > > > On Fri, 2019-11-22 at 18:19 -0800, Atish Patra wrote:
> > > > > > > > > On Wed, 2019-11-13 at 11:47 -0800, Atish Patra wrote:
> > > > > > > > > > On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> > > > > > > > > > > On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com>
> > > > > > > > > > > wrote:
> > > > > > > > > > > > Add compressed Image parsing support so that booti can parse
> > > > > > > > > > > > both
> > > > > > > > > > > > flat and compressed Image to boot Linux. Currently, it is
> > > > > > > > > > > > difficult
> > > > > > > > > > > > to calculate a safe address for every board where the
> > > > > > > > > > > > compressed
> > > > > > > > > > > > image can be decompressed. It is also not possible to figure
> > > > > > > > > > > > out
> > > > > > > > > > > > the
> > > > > > > > > > > > size of the compressed file as well. Thus, user need to set two
> > > > > > > > > > > > additional environment variables kernel_comp_addr_r and
> > > > > > > > > > > > filesize
> > > > > > > > > > > > to
> > > > > > > > > > > > make this work.
> > > > > > > > > > > >
> > > > > > > > > > > > Following compression methods are supported for now.
> > > > > > > > > > > > lzma, lzo, bzip2, gzip.
> > > > > > > > > > > >
> > > > > > > > > > > > lz4 support is not added as ARM64 kernel generates a lz4
> > > > > > > > > > > > compressed
> > > > > > > > > > > > image with legacy header which U-Boot doesn't know how to parse
> > > > > > > > > > > > and
> > > > > > > > > > > > decompress.
> > > > > > > > > > > >
> > > > > > > > > > > > Tested on HiFive Unleashed and Qemu for RISC-V.
> > > > > > > > > > > > Tested on Qemu for ARM64.
> > > > > > > > > > > >
> > > > > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > > > > > > > > > > ---
> > > > > > > > > > > > I could not test this patch on any ARM64 boards due to lack of
> > > > > > > > > > > > access to any ARM64 board. If anybody can test it on ARM64,
> > > > > > > > > > > > that
> > > > > > > > > > > > would be great.
> > > > > > > > > > > > ---
> > > > > > > > > > > >  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
> > > > > > > > > > > >  doc/README.distro          | 12 +++++++++
> > > > > > > > > > > >  doc/board/sifive/fu540.rst | 55
> > > > > > > > > > > > ++++++++++++++++++++++++++++++++++++++
> > > > > > > > > > > >  3 files changed, 106 insertions(+), 1 deletion(-)
> > > > > > > > > > > >
> > > > > > > > > > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > > > > > > > > > index c36b0235df8c..cd8670a9a8db 100644
> > > > > > > > > > > > --- a/cmd/booti.c
> > > > > > > > > > > > +++ b/cmd/booti.c
> > > > > > > > > > > > @@ -13,6 +13,7 @@
> > > > > > > > > > > >  #include <linux/kernel.h>
> > > > > > > > > > > >  #include <linux/sizes.h>
> > > > > > > > > > > >
> > > > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > > > > > > > > >  /*
> > > > > > > > > > > >   * Image booting support
> > > > > > > > > > > >   */
> > > > > > > > > > > > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > > > > > flag, int argc,
> > > > > > > > > > > >         ulong ld;
> > > > > > > > > > > >         ulong relocated_addr;
> > > > > > > > > > > >         ulong image_size;
> > > > > > > > > > > > +       uint8_t *temp;
> > > > > > > > > > > > +       ulong dest;
> > > > > > > > > > > > +       ulong dest_end;
> > > > > > > > > > > > +       unsigned long comp_len;
> > > > > > > > > > > > +       unsigned long decomp_len;
> > > > > > > > > > > > +       int ctype;
> > > > > > > > > > > >
> > > > > > > > > > > >         ret = do_bootm_states(cmdtp, flag, argc, argv,
> > > > > > > > > > > > BOOTM_STATE_START,
> > > > > > > > > > > >                               images, 1);
> > > > > > > > > > > > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > > > > > flag, int argc,
> > > > > > > > > > > >                 debug("*  kernel: cmdline image address =
> > > > > > > > > > > > 0x%08lx\n", ld);
> > > > > > > > > > > >         }
> > > > > > > > > > > >
> > > > > > > > > > > > +       temp = map_sysmem(ld, 0);
> > > > > > > > > > > > +       ctype = image_decomp_type(temp, 2);
> > > > > > > > > > > > +       if (ctype > 0) {
> > > > > > > > > > > > +               dest = env_get_ulong("kernel_comp_addr_r", 16,
> > > > > > > > > > > > 0);
> > > > > > > > > > > > +               comp_len = env_get_ulong("filesize", 16, 0);
> > > > > > > > > > > > +               if (!dest || !comp_len) {
> > > > > > > > > > > > +                       puts("kernel_comp_addr_r or filesize is
> > > > > > > > > > > > not
> > > > > > > > > > > > provided!\n");
> > > > > > > > > > > > +                       return -EINVAL;
> > > > > > > > > > > > +               }
> > > > > > > > > > > > +               if (dest < gd->ram_base || dest > gd->ram_top)
> > > > > > > > > > > > {
> > > > > > > > > > > > +                       puts("kernel_comp_addr_r is outside of
> > > > > > > > > > > > DRAM
> > > > > > > > > > > > range!\n");
> > > > > > > > > > > > +                       return -EINVAL;
> > > > > > > > > > > > +               }
> > > > > > > > > > > > +
> > > > > > > > > > > > +               debug("kernel image compression type %d size =
> > > > > > > > > > > > 0x%08lx address = 0x%08lx\n",
> > > > > > > > > > > > +                       ctype, comp_len, (ulong)dest);
> > > > > > > > > > > > +               decomp_len = comp_len * 10;
> > > > > > > > > > > > +               ret = image_decomp(ctype, 0, ld,
> > > > > > > > > > > > IH_TYPE_KERNEL,
> > > > > > > > > > > > +                                (void *)dest, (void *)ld,
> > > > > > > > > > > > comp_len,
> > > > > > > > > > > > +                                decomp_len, &dest_end);
> > > > > > > > > > > > +               if (ret)
> > > > > > > > > > > > +                       return ret;
> > > > > > > > > > > > +               /* dest_end contains the uncompressed Image
> > > > > > > > > > > > size
> > > > > > > > > > > > */
> > > > > > > > > > > > +               memmove((void *) ld, (void *)dest, dest_end);
> > > > > > > > > > > > +       }
> > > > > > > > > > > > +       unmap_sysmem((void *)ld);
> > > > > > > > > > > > +
> > > > > > > > > > > >         ret = booti_setup(ld, &relocated_addr, &image_size,
> > > > > > > > > > > > false);
> > > > > > > > > > > >         if (ret != 0)
> > > > > > > > > > > >                 return 1;
> > > > > > > > > > > > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag,
> > > > > > > > > > > > int
> > > > > > > > > > > > argc, char * const argv[])
> > > > > > > > > > > >  #ifdef CONFIG_SYS_LONGHELP
> > > > > > > > > > > >  static char booti_help_text[] =
> > > > > > > > > > > >         "[addr [initrd[:size]] [fdt]]\n"
> > > > > > > > > > > > -       "    - boot Linux 'Image' stored at 'addr'\n"
> > > > > > > > > > > > +       "    - boot Linux flat or compressed 'Image' stored at
> > > > > > > > > > > > 'addr'\n"
> > > > > > > > > > > >         "\tThe argument 'initrd' is optional and specifies the
> > > > > > > > > > > > address\n"
> > > > > > > > > > > >         "\tof an initrd in memory. The optional parameter
> > > > > > > > > > > > ':size'
> > > > > > > > > > > > allows\n"
> > > > > > > > > > > >         "\tspecifying the size of a RAW initrd.\n"
> > > > > > > > > > > > +       "\tCurrently only booting from gz, bz2, lzma and lz4
> > > > > > > > > > > > compression\n"
> > > > > > > > > > > > +       "\ttypes are supported. In order to boot from any of
> > > > > > > > > > > > these
> > > > > > > > > > > > compressed\n"
> > > > > > > > > > > > +       "\timages, user have to set kernel_comp_addr_r and
> > > > > > > > > > > > filesize
> > > > > > > > > > > > enviornment\n"
> > > > > > > > > > > > +       "\tvariables beforehand.\n"
> > > > > > > > > > > >  #if defined(CONFIG_OF_LIBFDT)
> > > > > > > > > > > >         "\tSince booting a Linux kernel requires a flat device-
> > > > > > > > > > > > tree, a\n"
> > > > > > > > > > > >         "\tthird argument providing the address of the device-
> > > > > > > > > > > > tree
> > > > > > > > > > > > blob\n"
> > > > > > > > > > > > diff --git a/doc/README.distro b/doc/README.distro
> > > > > > > > > > > > index ab6e6f4e74be..67b49e1e4b6a 100644
> > > > > > > > > > > > --- a/doc/README.distro
> > > > > > > > > > > > +++ b/doc/README.distro
> > > > > > > > > > > > @@ -246,6 +246,18 @@ kernel_addr_r:
> > > > > > > > > > > >
> > > > > > > > > > > >    A size of 16MB for the kernel is likely adequate.
> > > > > > > > > > > >
> > > > > > > > > > > > +kernel_comp_addr_r:
> > > > > > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > > > > > from
> > > > > > > > > > > > a compressed
> > > > > > > > > > > > +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It
> > > > > > > > > > > > represents
> > > > > > > > > > > > the location
> > > > > > > > > > > > +  in RAM where the compressed Image will be decompressed
> > > > > > > > > > > > temporarily. Once the
> > > > > > > > > > > > +  decompression is complete, decompressed data will be moved
> > > > > > > > > > > > kernel_addr_r for
> > > > > > > > > > > > +  booting.
> > > > > > > > > > > > +
> > > > > > > > > > > > +filesize:
> > > > > > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > > > > > from
> > > > > > > > > > > > a compressed
> > > > > > > > > > > > +  Image using booti command. It represents the size of the
> > > > > > > > > > > > compressed file. The
> > > > > > > > > > > > +  size has to at least the size of loaded image for
> > > > > > > > > > > > decompression
> > > > > > > > > > > > to succeed.
> > > > > > > > > > > > +
> > > > > > > > > > >
> > > > > > > > > > > I am not sure I like using filesize here compared to
> > > > > > > > > > > kernel_gz_size.
> > > > > > > > > > > It's true that $filesize will be set once your load the binary,
> > > > > > > > > > > e.g.
> > > > > > > > > > > from mmc.
> > > > > > > > > > > But in EXTLINUX/PXE situation $filesize could be wrong.
> > > > > > > > > > >
> > > > > > > > > > > The load happens in this order:
> > > > > > > > > > > - initrd
> > > > > > > > > > > - kernel
> > > > > > > > > > > - fdt
> > > > > > > > > > >
> > > > > > > > > > > So if I specify FDT in my EXTLINUX/PXE config the $filesize will
> > > > > > > > > > > be
> > > > > > > > > > > wrong
> > > > > > > > > > > while attempting to boot the kernel. Unless we save filesize of
> > > > > > > > > > > kernel after
> > > > > > > > > > > loading and set it again before calling do_booti() in pxe.c.
> > > > > > > > > > >
> > > > > > > > > > > Currently I set kernel_gz_size in board environment, which is set
> > > > > > > > > > > to
> > > > > > > > > > > max
> > > > > > > > > > > allowed size.
> > > > > > > > > > >
> > > > > > > > > > > david
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Ahh okay. There are two ways to fix it.
> > > > > > > > > >
> > > > > > > > > > 1. Fix the pxe implementation but save the filesize to be used
> > > > > > > > > > later.
> > > > > > > > > >
> > > > > > > > > > But some other user may fall into the same issue without realizing
> > > > > > > > > > it
> > > > > > > > > > if the user loads any other image after compressed kernel Image.
> > > > > > > > > >
> > > > > > > > > > We need clear documentation saying that, compressed kernel Image
> > > > > > > > > > should
> > > > > > > > > > be loaded last before executing booti or $filesize must be set
> > > > > > > > > > manually
> > > > > > > > > > before calling booti.
> > > > > > > > > >
> > > > > > > > > > 2. Just change it back to kernel_comp_size (compressed kernel image
> > > > > > > > > > size) to avoid any confusion.
> > > > > > > > > >
> > > > > > > > > > Any preference ?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Any thoughts ?
> > > > > > > > >
> > > > > > > >
> > > > > > > > I think this patch got lost during holidays :). Any suggestion on what
> > > > > > > > should be the best approach to resolve the issue with pxe
> > > > > > > > implementation ?
> > > > > > >
> > > > > > > I think that we really need to be careful when adding "automagic"
> > > > > > > features like this.  Thinking harder about it all, we can't re-use any
> > > > > > > existing variable as there's going to be some case of a setup breaking
> > > > > > > in strange silent ways.
> > > > > > >
> > > > > > > Can we uncompress a single chunk / page / something of the compressed
> > > > > > > image to get the header and thus know the uncompressed size?  We would
> > > > > > > then know the compressed size is no larger than that and can progress
> > > > > > > from there?
> > > > > >
> > > > > > After a quick google on various compression formats I get impression this
> > > > > > is not a common thing to record in the headers. The kernels could be
> > > > > > compressed with gzip, bzip2, lz4, lzma and lzo (I only checked Makefile for
> > > > > >  riscv).
> > > > >
> > > > > Sorry I wasn't clear enough.  The contents in question here contain a
> > > > > header at the start which does contain the uncompressed image size.
> > > > >
> > > >
> > > > The objective of the patch was to support all the compressed image
> > > > type that kernel supports.
> > > > i.e. gz, bz2, lzma, lzo.
> > > >
> > > > uncompressed image size in compressed header in the following file formats:
> > > >
> > > > 1. gz: Uncompressed size is at the footer (http://www.zlib.org/rfc-gzip.html)
> > > > 2. bz2: No information in the header
> > > > 3. lzma: Present in the header
> > > > (https://svn.python.org/projects/external/xz-5.0.3/doc/lzma-file-format.txt)
> > > > 4. lzo: Present in the header (https://gist.github.com/jledet/1333896)
> > > >
> > > > To summarize, if we want to parse the compressed file to determine the
> > > > uncompressed size,
> > > > we have to add the support to generic compression library for all file
> > > > formats rather than just doing it here.
> > > > Additionally, not all format mandates the uncompressed size which
> > > > makes it difficult.
> > >
> > > Here's my confusion, and perhaps dumb question.  What happens if we:
> > > 1. Uncompress the first "chunk" of the data, which will let us at the
> > > Image header.
> >

Theoretically that should work. However, current zlib implementation
doesn't allow
decompressing few chunks of data. I tried a few different sizes. But
any size less
than compressed file size results in following error.

Error: inflate() returned -5

There may be some way to modify zlib implementation to allow that but I am not a
compression expert to make such changes.

Let us know what other alternative approaches you prefer.

1. Save $filesize to kernel_comp_size after loading kernel binary and
pass it around in C code.
or
2.  Just use an environment variable other than filesize that users can set it.

I personally prefer 1st method as that reduce manual steps.
>
> David,
> I think we misunderstood Tom's feedback. Tom is asking to decompress
> initial chunk of
> compressed data to so that we have access to kernel Image header.
> Uncompressed image
> size can be retrieved from header and set it as the compressed data
> size which should do the trick.
>
> I think the word "header" was the source of confusion. Thanks for the
> suggestion. I will try this at my end.
>
>
> > - gzip stores the uncompressed size at the end of the file, but that's valid
> > only for compressed files up to 4G in size. This might not be valid if
> > compressed with something else (e.g. pigz).
> > - XZ: https://tukaani.org/xz/xz-file-format-1.0.4.txt
> >  xz could have multiple streams, each stream could have multiple blocks.
> >  the uncompressed size is stored in block header (which is optional and
> > indicated by a flag if field is valid). The format also states that the only
> > reliable way to get uncompressed size is to uncompress. Quote:
> >
> >         It should be noted that the only reliable way to determine
> >         the real uncompressed size is to uncompress the Block,
> >         because the Block Header and Index fields may contain
> >         (intentionally or unintentionally) invalid information.
> >
> > Also the index is also stored at the end of the file.
> > - LZO: https://www.kernel.org/doc/Documentation/lzo.txt
> > I don't see anyone mentioning uncompressed size anywhere.
> > I think kernel uses pure LZO stream without any container (?).
> > No official spec exist, this is reverse engineered by reading kernel code.
> > - bzip2: here also an official spec doesn't exist. Reverse engineered
> > is here: https://github.com/dsnet/compress/blob/master/doc/bzip2-format.pdf
> > It doesn't store such information.
> >
> > TL;DR figuring out uncompressed size (especially for multiple containers)
> > seems to be complicated, and you need to uncompress if you want to be
> > sure about the uncompressed size. That's the message I get the
> > more I google about it.
> >
> > So we either pick a proper variable name (works now, but cannot be $filesize) or
> > we internally save $filesize after loading kernel binary and pass it
> > around in C code.
> >
> > > 2. Given that the Image header tells us the uncompressed size of the
> > > file (image_size field), we know how much space we need in the end.
> > > 3. Uncompress to where it needs to go.
> > >
> > > --
> > > Tom
>
>
>
> --
> Regards,
> Atish



-- 
Regards,
Atish

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[Tom Rini]

On Fri, Feb 28, 2020 at 05:15:53PM -0800, Atish Patra wrote:
> On Thu, Feb 20, 2020 at 2:25 PM Atish Patra <atishp@atishpatra.org> wrote:
> >
> > On Thu, Feb 20, 2020 at 1:14 PM David Abdurachmanov
> > <david.abdurachmanov@gmail.com> wrote:
> > >
> > > On Tue, Feb 18, 2020 at 10:38 PM Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Sun, Feb 16, 2020 at 04:48:22PM -0800, Atish Patra wrote:
> > > > > On Fri, Feb 14, 2020 at 8:43 AM Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Thu, Feb 13, 2020 at 11:32:52PM +0200, David Abdurachmanov wrote:
> > > > > > > On Thu, Feb 13, 2020 at 6:17 PM Tom Rini <trini@konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Wed, Feb 05, 2020 at 12:01:38AM +0000, Atish Patra wrote:
> > > > > > > > > On Fri, 2019-11-22 at 18:19 -0800, Atish Patra wrote:
> > > > > > > > > > On Wed, 2019-11-13 at 11:47 -0800, Atish Patra wrote:
> > > > > > > > > > > On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> > > > > > > > > > > > On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com>
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > > Add compressed Image parsing support so that booti can parse
> > > > > > > > > > > > > both
> > > > > > > > > > > > > flat and compressed Image to boot Linux. Currently, it is
> > > > > > > > > > > > > difficult
> > > > > > > > > > > > > to calculate a safe address for every board where the
> > > > > > > > > > > > > compressed
> > > > > > > > > > > > > image can be decompressed. It is also not possible to figure
> > > > > > > > > > > > > out
> > > > > > > > > > > > > the
> > > > > > > > > > > > > size of the compressed file as well. Thus, user need to set two
> > > > > > > > > > > > > additional environment variables kernel_comp_addr_r and
> > > > > > > > > > > > > filesize
> > > > > > > > > > > > > to
> > > > > > > > > > > > > make this work.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Following compression methods are supported for now.
> > > > > > > > > > > > > lzma, lzo, bzip2, gzip.
> > > > > > > > > > > > >
> > > > > > > > > > > > > lz4 support is not added as ARM64 kernel generates a lz4
> > > > > > > > > > > > > compressed
> > > > > > > > > > > > > image with legacy header which U-Boot doesn't know how to parse
> > > > > > > > > > > > > and
> > > > > > > > > > > > > decompress.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Tested on HiFive Unleashed and Qemu for RISC-V.
> > > > > > > > > > > > > Tested on Qemu for ARM64.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > > > > > > > > > > > ---
> > > > > > > > > > > > > I could not test this patch on any ARM64 boards due to lack of
> > > > > > > > > > > > > access to any ARM64 board. If anybody can test it on ARM64,
> > > > > > > > > > > > > that
> > > > > > > > > > > > > would be great.
> > > > > > > > > > > > > ---
> > > > > > > > > > > > >  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
> > > > > > > > > > > > >  doc/README.distro          | 12 +++++++++
> > > > > > > > > > > > >  doc/board/sifive/fu540.rst | 55
> > > > > > > > > > > > > ++++++++++++++++++++++++++++++++++++++
> > > > > > > > > > > > >  3 files changed, 106 insertions(+), 1 deletion(-)
> > > > > > > > > > > > >
> > > > > > > > > > > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > > > > > > > > > > index c36b0235df8c..cd8670a9a8db 100644
> > > > > > > > > > > > > --- a/cmd/booti.c
> > > > > > > > > > > > > +++ b/cmd/booti.c
> > > > > > > > > > > > > @@ -13,6 +13,7 @@
> > > > > > > > > > > > >  #include <linux/kernel.h>
> > > > > > > > > > > > >  #include <linux/sizes.h>
> > > > > > > > > > > > >
> > > > > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > > > > > > > > > >  /*
> > > > > > > > > > > > >   * Image booting support
> > > > > > > > > > > > >   */
> > > > > > > > > > > > > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > > > > > > flag, int argc,
> > > > > > > > > > > > >         ulong ld;
> > > > > > > > > > > > >         ulong relocated_addr;
> > > > > > > > > > > > >         ulong image_size;
> > > > > > > > > > > > > +       uint8_t *temp;
> > > > > > > > > > > > > +       ulong dest;
> > > > > > > > > > > > > +       ulong dest_end;
> > > > > > > > > > > > > +       unsigned long comp_len;
> > > > > > > > > > > > > +       unsigned long decomp_len;
> > > > > > > > > > > > > +       int ctype;
> > > > > > > > > > > > >
> > > > > > > > > > > > >         ret = do_bootm_states(cmdtp, flag, argc, argv,
> > > > > > > > > > > > > BOOTM_STATE_START,
> > > > > > > > > > > > >                               images, 1);
> > > > > > > > > > > > > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > > > > > > flag, int argc,
> > > > > > > > > > > > >                 debug("*  kernel: cmdline image address =
> > > > > > > > > > > > > 0x%08lx\n", ld);
> > > > > > > > > > > > >         }
> > > > > > > > > > > > >
> > > > > > > > > > > > > +       temp = map_sysmem(ld, 0);
> > > > > > > > > > > > > +       ctype = image_decomp_type(temp, 2);
> > > > > > > > > > > > > +       if (ctype > 0) {
> > > > > > > > > > > > > +               dest = env_get_ulong("kernel_comp_addr_r", 16,
> > > > > > > > > > > > > 0);
> > > > > > > > > > > > > +               comp_len = env_get_ulong("filesize", 16, 0);
> > > > > > > > > > > > > +               if (!dest || !comp_len) {
> > > > > > > > > > > > > +                       puts("kernel_comp_addr_r or filesize is
> > > > > > > > > > > > > not
> > > > > > > > > > > > > provided!\n");
> > > > > > > > > > > > > +                       return -EINVAL;
> > > > > > > > > > > > > +               }
> > > > > > > > > > > > > +               if (dest < gd->ram_base || dest > gd->ram_top)
> > > > > > > > > > > > > {
> > > > > > > > > > > > > +                       puts("kernel_comp_addr_r is outside of
> > > > > > > > > > > > > DRAM
> > > > > > > > > > > > > range!\n");
> > > > > > > > > > > > > +                       return -EINVAL;
> > > > > > > > > > > > > +               }
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +               debug("kernel image compression type %d size =
> > > > > > > > > > > > > 0x%08lx address = 0x%08lx\n",
> > > > > > > > > > > > > +                       ctype, comp_len, (ulong)dest);
> > > > > > > > > > > > > +               decomp_len = comp_len * 10;
> > > > > > > > > > > > > +               ret = image_decomp(ctype, 0, ld,
> > > > > > > > > > > > > IH_TYPE_KERNEL,
> > > > > > > > > > > > > +                                (void *)dest, (void *)ld,
> > > > > > > > > > > > > comp_len,
> > > > > > > > > > > > > +                                decomp_len, &dest_end);
> > > > > > > > > > > > > +               if (ret)
> > > > > > > > > > > > > +                       return ret;
> > > > > > > > > > > > > +               /* dest_end contains the uncompressed Image
> > > > > > > > > > > > > size
> > > > > > > > > > > > > */
> > > > > > > > > > > > > +               memmove((void *) ld, (void *)dest, dest_end);
> > > > > > > > > > > > > +       }
> > > > > > > > > > > > > +       unmap_sysmem((void *)ld);
> > > > > > > > > > > > > +
> > > > > > > > > > > > >         ret = booti_setup(ld, &relocated_addr, &image_size,
> > > > > > > > > > > > > false);
> > > > > > > > > > > > >         if (ret != 0)
> > > > > > > > > > > > >                 return 1;
> > > > > > > > > > > > > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag,
> > > > > > > > > > > > > int
> > > > > > > > > > > > > argc, char * const argv[])
> > > > > > > > > > > > >  #ifdef CONFIG_SYS_LONGHELP
> > > > > > > > > > > > >  static char booti_help_text[] =
> > > > > > > > > > > > >         "[addr [initrd[:size]] [fdt]]\n"
> > > > > > > > > > > > > -       "    - boot Linux 'Image' stored at 'addr'\n"
> > > > > > > > > > > > > +       "    - boot Linux flat or compressed 'Image' stored at
> > > > > > > > > > > > > 'addr'\n"
> > > > > > > > > > > > >         "\tThe argument 'initrd' is optional and specifies the
> > > > > > > > > > > > > address\n"
> > > > > > > > > > > > >         "\tof an initrd in memory. The optional parameter
> > > > > > > > > > > > > ':size'
> > > > > > > > > > > > > allows\n"
> > > > > > > > > > > > >         "\tspecifying the size of a RAW initrd.\n"
> > > > > > > > > > > > > +       "\tCurrently only booting from gz, bz2, lzma and lz4
> > > > > > > > > > > > > compression\n"
> > > > > > > > > > > > > +       "\ttypes are supported. In order to boot from any of
> > > > > > > > > > > > > these
> > > > > > > > > > > > > compressed\n"
> > > > > > > > > > > > > +       "\timages, user have to set kernel_comp_addr_r and
> > > > > > > > > > > > > filesize
> > > > > > > > > > > > > enviornment\n"
> > > > > > > > > > > > > +       "\tvariables beforehand.\n"
> > > > > > > > > > > > >  #if defined(CONFIG_OF_LIBFDT)
> > > > > > > > > > > > >         "\tSince booting a Linux kernel requires a flat device-
> > > > > > > > > > > > > tree, a\n"
> > > > > > > > > > > > >         "\tthird argument providing the address of the device-
> > > > > > > > > > > > > tree
> > > > > > > > > > > > > blob\n"
> > > > > > > > > > > > > diff --git a/doc/README.distro b/doc/README.distro
> > > > > > > > > > > > > index ab6e6f4e74be..67b49e1e4b6a 100644
> > > > > > > > > > > > > --- a/doc/README.distro
> > > > > > > > > > > > > +++ b/doc/README.distro
> > > > > > > > > > > > > @@ -246,6 +246,18 @@ kernel_addr_r:
> > > > > > > > > > > > >
> > > > > > > > > > > > >    A size of 16MB for the kernel is likely adequate.
> > > > > > > > > > > > >
> > > > > > > > > > > > > +kernel_comp_addr_r:
> > > > > > > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > > > > > > from
> > > > > > > > > > > > > a compressed
> > > > > > > > > > > > > +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It
> > > > > > > > > > > > > represents
> > > > > > > > > > > > > the location
> > > > > > > > > > > > > +  in RAM where the compressed Image will be decompressed
> > > > > > > > > > > > > temporarily. Once the
> > > > > > > > > > > > > +  decompression is complete, decompressed data will be moved
> > > > > > > > > > > > > kernel_addr_r for
> > > > > > > > > > > > > +  booting.
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +filesize:
> > > > > > > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > > > > > > from
> > > > > > > > > > > > > a compressed
> > > > > > > > > > > > > +  Image using booti command. It represents the size of the
> > > > > > > > > > > > > compressed file. The
> > > > > > > > > > > > > +  size has to at least the size of loaded image for
> > > > > > > > > > > > > decompression
> > > > > > > > > > > > > to succeed.
> > > > > > > > > > > > > +
> > > > > > > > > > > >
> > > > > > > > > > > > I am not sure I like using filesize here compared to
> > > > > > > > > > > > kernel_gz_size.
> > > > > > > > > > > > It's true that $filesize will be set once your load the binary,
> > > > > > > > > > > > e.g.
> > > > > > > > > > > > from mmc.
> > > > > > > > > > > > But in EXTLINUX/PXE situation $filesize could be wrong.
> > > > > > > > > > > >
> > > > > > > > > > > > The load happens in this order:
> > > > > > > > > > > > - initrd
> > > > > > > > > > > > - kernel
> > > > > > > > > > > > - fdt
> > > > > > > > > > > >
> > > > > > > > > > > > So if I specify FDT in my EXTLINUX/PXE config the $filesize will
> > > > > > > > > > > > be
> > > > > > > > > > > > wrong
> > > > > > > > > > > > while attempting to boot the kernel. Unless we save filesize of
> > > > > > > > > > > > kernel after
> > > > > > > > > > > > loading and set it again before calling do_booti() in pxe.c.
> > > > > > > > > > > >
> > > > > > > > > > > > Currently I set kernel_gz_size in board environment, which is set
> > > > > > > > > > > > to
> > > > > > > > > > > > max
> > > > > > > > > > > > allowed size.
> > > > > > > > > > > >
> > > > > > > > > > > > david
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Ahh okay. There are two ways to fix it.
> > > > > > > > > > >
> > > > > > > > > > > 1. Fix the pxe implementation but save the filesize to be used
> > > > > > > > > > > later.
> > > > > > > > > > >
> > > > > > > > > > > But some other user may fall into the same issue without realizing
> > > > > > > > > > > it
> > > > > > > > > > > if the user loads any other image after compressed kernel Image.
> > > > > > > > > > >
> > > > > > > > > > > We need clear documentation saying that, compressed kernel Image
> > > > > > > > > > > should
> > > > > > > > > > > be loaded last before executing booti or $filesize must be set
> > > > > > > > > > > manually
> > > > > > > > > > > before calling booti.
> > > > > > > > > > >
> > > > > > > > > > > 2. Just change it back to kernel_comp_size (compressed kernel image
> > > > > > > > > > > size) to avoid any confusion.
> > > > > > > > > > >
> > > > > > > > > > > Any preference ?
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Any thoughts ?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > I think this patch got lost during holidays :). Any suggestion on what
> > > > > > > > > should be the best approach to resolve the issue with pxe
> > > > > > > > > implementation ?
> > > > > > > >
> > > > > > > > I think that we really need to be careful when adding "automagic"
> > > > > > > > features like this.  Thinking harder about it all, we can't re-use any
> > > > > > > > existing variable as there's going to be some case of a setup breaking
> > > > > > > > in strange silent ways.
> > > > > > > >
> > > > > > > > Can we uncompress a single chunk / page / something of the compressed
> > > > > > > > image to get the header and thus know the uncompressed size?  We would
> > > > > > > > then know the compressed size is no larger than that and can progress
> > > > > > > > from there?
> > > > > > >
> > > > > > > After a quick google on various compression formats I get impression this
> > > > > > > is not a common thing to record in the headers. The kernels could be
> > > > > > > compressed with gzip, bzip2, lz4, lzma and lzo (I only checked Makefile for
> > > > > > >  riscv).
> > > > > >
> > > > > > Sorry I wasn't clear enough.  The contents in question here contain a
> > > > > > header at the start which does contain the uncompressed image size.
> > > > > >
> > > > >
> > > > > The objective of the patch was to support all the compressed image
> > > > > type that kernel supports.
> > > > > i.e. gz, bz2, lzma, lzo.
> > > > >
> > > > > uncompressed image size in compressed header in the following file formats:
> > > > >
> > > > > 1. gz: Uncompressed size is at the footer (http://www.zlib.org/rfc-gzip.html)
> > > > > 2. bz2: No information in the header
> > > > > 3. lzma: Present in the header
> > > > > (https://svn.python.org/projects/external/xz-5.0.3/doc/lzma-file-format.txt)
> > > > > 4. lzo: Present in the header (https://gist.github.com/jledet/1333896)
> > > > >
> > > > > To summarize, if we want to parse the compressed file to determine the
> > > > > uncompressed size,
> > > > > we have to add the support to generic compression library for all file
> > > > > formats rather than just doing it here.
> > > > > Additionally, not all format mandates the uncompressed size which
> > > > > makes it difficult.
> > > >
> > > > Here's my confusion, and perhaps dumb question.  What happens if we:
> > > > 1. Uncompress the first "chunk" of the data, which will let us at the
> > > > Image header.
> > >
> 
> Theoretically that should work. However, current zlib implementation
> doesn't allow
> decompressing few chunks of data. I tried a few different sizes. But
> any size less
> than compressed file size results in following error.
> 
> Error: inflate() returned -5
> 
> There may be some way to modify zlib implementation to allow that but I am not a
> compression expert to make such changes.

Thanks for looking in to this.

> Let us know what other alternative approaches you prefer.
> 
> 1. Save $filesize to kernel_comp_size after loading kernel binary and
> pass it around in C code.
> or
> 2.  Just use an environment variable other than filesize that users can set it.

I think we need to go with #2 here as I fear #1 will be error prone (how
do we know we just loaded the kernel?) over just adapting the existing
general logic to add 'setenv kernel_comp_size $filesize' after loading
the kernel image.  I'm open to reviewing #1 all the same however.
Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200302/7aad66d1/attachment.sig>

[U-Boot] [RFC/RFT PATCH v4 3/3] image: Add compressed Image parsing support in booti.

[Atish Patra]

On Mon, Mar 2, 2020 at 10:41 AM Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Feb 28, 2020 at 05:15:53PM -0800, Atish Patra wrote:
> > On Thu, Feb 20, 2020 at 2:25 PM Atish Patra <atishp@atishpatra.org> wrote:
> > >
> > > On Thu, Feb 20, 2020 at 1:14 PM David Abdurachmanov
> > > <david.abdurachmanov@gmail.com> wrote:
> > > >
> > > > On Tue, Feb 18, 2020 at 10:38 PM Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Sun, Feb 16, 2020 at 04:48:22PM -0800, Atish Patra wrote:
> > > > > > On Fri, Feb 14, 2020 at 8:43 AM Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Thu, Feb 13, 2020 at 11:32:52PM +0200, David Abdurachmanov wrote:
> > > > > > > > On Thu, Feb 13, 2020 at 6:17 PM Tom Rini <trini@konsulko.com> wrote:
> > > > > > > > >
> > > > > > > > > On Wed, Feb 05, 2020 at 12:01:38AM +0000, Atish Patra wrote:
> > > > > > > > > > On Fri, 2019-11-22 at 18:19 -0800, Atish Patra wrote:
> > > > > > > > > > > On Wed, 2019-11-13 at 11:47 -0800, Atish Patra wrote:
> > > > > > > > > > > > On Wed, 2019-11-13 at 15:36 +0200, David Abdurachmanov wrote:
> > > > > > > > > > > > > On Sat, Nov 9, 2019 at 2:14 AM Atish Patra <atish.patra@wdc.com>
> > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > Add compressed Image parsing support so that booti can parse
> > > > > > > > > > > > > > both
> > > > > > > > > > > > > > flat and compressed Image to boot Linux. Currently, it is
> > > > > > > > > > > > > > difficult
> > > > > > > > > > > > > > to calculate a safe address for every board where the
> > > > > > > > > > > > > > compressed
> > > > > > > > > > > > > > image can be decompressed. It is also not possible to figure
> > > > > > > > > > > > > > out
> > > > > > > > > > > > > > the
> > > > > > > > > > > > > > size of the compressed file as well. Thus, user need to set two
> > > > > > > > > > > > > > additional environment variables kernel_comp_addr_r and
> > > > > > > > > > > > > > filesize
> > > > > > > > > > > > > > to
> > > > > > > > > > > > > > make this work.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Following compression methods are supported for now.
> > > > > > > > > > > > > > lzma, lzo, bzip2, gzip.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > lz4 support is not added as ARM64 kernel generates a lz4
> > > > > > > > > > > > > > compressed
> > > > > > > > > > > > > > image with legacy header which U-Boot doesn't know how to parse
> > > > > > > > > > > > > > and
> > > > > > > > > > > > > > decompress.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Tested on HiFive Unleashed and Qemu for RISC-V.
> > > > > > > > > > > > > > Tested on Qemu for ARM64.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > > > > > > > > > > > > ---
> > > > > > > > > > > > > > I could not test this patch on any ARM64 boards due to lack of
> > > > > > > > > > > > > > access to any ARM64 board. If anybody can test it on ARM64,
> > > > > > > > > > > > > > that
> > > > > > > > > > > > > > would be great.
> > > > > > > > > > > > > > ---
> > > > > > > > > > > > > >  cmd/booti.c                | 40 ++++++++++++++++++++++++++-
> > > > > > > > > > > > > >  doc/README.distro          | 12 +++++++++
> > > > > > > > > > > > > >  doc/board/sifive/fu540.rst | 55
> > > > > > > > > > > > > > ++++++++++++++++++++++++++++++++++++++
> > > > > > > > > > > > > >  3 files changed, 106 insertions(+), 1 deletion(-)
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > diff --git a/cmd/booti.c b/cmd/booti.c
> > > > > > > > > > > > > > index c36b0235df8c..cd8670a9a8db 100644
> > > > > > > > > > > > > > --- a/cmd/booti.c
> > > > > > > > > > > > > > +++ b/cmd/booti.c
> > > > > > > > > > > > > > @@ -13,6 +13,7 @@
> > > > > > > > > > > > > >  #include <linux/kernel.h>
> > > > > > > > > > > > > >  #include <linux/sizes.h>
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > +DECLARE_GLOBAL_DATA_PTR;
> > > > > > > > > > > > > >  /*
> > > > > > > > > > > > > >   * Image booting support
> > > > > > > > > > > > > >   */
> > > > > > > > > > > > > > @@ -23,6 +24,12 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > > > > > > > flag, int argc,
> > > > > > > > > > > > > >         ulong ld;
> > > > > > > > > > > > > >         ulong relocated_addr;
> > > > > > > > > > > > > >         ulong image_size;
> > > > > > > > > > > > > > +       uint8_t *temp;
> > > > > > > > > > > > > > +       ulong dest;
> > > > > > > > > > > > > > +       ulong dest_end;
> > > > > > > > > > > > > > +       unsigned long comp_len;
> > > > > > > > > > > > > > +       unsigned long decomp_len;
> > > > > > > > > > > > > > +       int ctype;
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >         ret = do_bootm_states(cmdtp, flag, argc, argv,
> > > > > > > > > > > > > > BOOTM_STATE_START,
> > > > > > > > > > > > > >                               images, 1);
> > > > > > > > > > > > > > @@ -37,6 +44,33 @@ static int booti_start(cmd_tbl_t *cmdtp, int
> > > > > > > > > > > > > > flag, int argc,
> > > > > > > > > > > > > >                 debug("*  kernel: cmdline image address =
> > > > > > > > > > > > > > 0x%08lx\n", ld);
> > > > > > > > > > > > > >         }
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > +       temp = map_sysmem(ld, 0);
> > > > > > > > > > > > > > +       ctype = image_decomp_type(temp, 2);
> > > > > > > > > > > > > > +       if (ctype > 0) {
> > > > > > > > > > > > > > +               dest = env_get_ulong("kernel_comp_addr_r", 16,
> > > > > > > > > > > > > > 0);
> > > > > > > > > > > > > > +               comp_len = env_get_ulong("filesize", 16, 0);
> > > > > > > > > > > > > > +               if (!dest || !comp_len) {
> > > > > > > > > > > > > > +                       puts("kernel_comp_addr_r or filesize is
> > > > > > > > > > > > > > not
> > > > > > > > > > > > > > provided!\n");
> > > > > > > > > > > > > > +                       return -EINVAL;
> > > > > > > > > > > > > > +               }
> > > > > > > > > > > > > > +               if (dest < gd->ram_base || dest > gd->ram_top)
> > > > > > > > > > > > > > {
> > > > > > > > > > > > > > +                       puts("kernel_comp_addr_r is outside of
> > > > > > > > > > > > > > DRAM
> > > > > > > > > > > > > > range!\n");
> > > > > > > > > > > > > > +                       return -EINVAL;
> > > > > > > > > > > > > > +               }
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > +               debug("kernel image compression type %d size =
> > > > > > > > > > > > > > 0x%08lx address = 0x%08lx\n",
> > > > > > > > > > > > > > +                       ctype, comp_len, (ulong)dest);
> > > > > > > > > > > > > > +               decomp_len = comp_len * 10;
> > > > > > > > > > > > > > +               ret = image_decomp(ctype, 0, ld,
> > > > > > > > > > > > > > IH_TYPE_KERNEL,
> > > > > > > > > > > > > > +                                (void *)dest, (void *)ld,
> > > > > > > > > > > > > > comp_len,
> > > > > > > > > > > > > > +                                decomp_len, &dest_end);
> > > > > > > > > > > > > > +               if (ret)
> > > > > > > > > > > > > > +                       return ret;
> > > > > > > > > > > > > > +               /* dest_end contains the uncompressed Image
> > > > > > > > > > > > > > size
> > > > > > > > > > > > > > */
> > > > > > > > > > > > > > +               memmove((void *) ld, (void *)dest, dest_end);
> > > > > > > > > > > > > > +       }
> > > > > > > > > > > > > > +       unmap_sysmem((void *)ld);
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > >         ret = booti_setup(ld, &relocated_addr, &image_size,
> > > > > > > > > > > > > > false);
> > > > > > > > > > > > > >         if (ret != 0)
> > > > > > > > > > > > > >                 return 1;
> > > > > > > > > > > > > > @@ -96,10 +130,14 @@ int do_booti(cmd_tbl_t *cmdtp, int flag,
> > > > > > > > > > > > > > int
> > > > > > > > > > > > > > argc, char * const argv[])
> > > > > > > > > > > > > >  #ifdef CONFIG_SYS_LONGHELP
> > > > > > > > > > > > > >  static char booti_help_text[] =
> > > > > > > > > > > > > >         "[addr [initrd[:size]] [fdt]]\n"
> > > > > > > > > > > > > > -       "    - boot Linux 'Image' stored at 'addr'\n"
> > > > > > > > > > > > > > +       "    - boot Linux flat or compressed 'Image' stored at
> > > > > > > > > > > > > > 'addr'\n"
> > > > > > > > > > > > > >         "\tThe argument 'initrd' is optional and specifies the
> > > > > > > > > > > > > > address\n"
> > > > > > > > > > > > > >         "\tof an initrd in memory. The optional parameter
> > > > > > > > > > > > > > ':size'
> > > > > > > > > > > > > > allows\n"
> > > > > > > > > > > > > >         "\tspecifying the size of a RAW initrd.\n"
> > > > > > > > > > > > > > +       "\tCurrently only booting from gz, bz2, lzma and lz4
> > > > > > > > > > > > > > compression\n"
> > > > > > > > > > > > > > +       "\ttypes are supported. In order to boot from any of
> > > > > > > > > > > > > > these
> > > > > > > > > > > > > > compressed\n"
> > > > > > > > > > > > > > +       "\timages, user have to set kernel_comp_addr_r and
> > > > > > > > > > > > > > filesize
> > > > > > > > > > > > > > enviornment\n"
> > > > > > > > > > > > > > +       "\tvariables beforehand.\n"
> > > > > > > > > > > > > >  #if defined(CONFIG_OF_LIBFDT)
> > > > > > > > > > > > > >         "\tSince booting a Linux kernel requires a flat device-
> > > > > > > > > > > > > > tree, a\n"
> > > > > > > > > > > > > >         "\tthird argument providing the address of the device-
> > > > > > > > > > > > > > tree
> > > > > > > > > > > > > > blob\n"
> > > > > > > > > > > > > > diff --git a/doc/README.distro b/doc/README.distro
> > > > > > > > > > > > > > index ab6e6f4e74be..67b49e1e4b6a 100644
> > > > > > > > > > > > > > --- a/doc/README.distro
> > > > > > > > > > > > > > +++ b/doc/README.distro
> > > > > > > > > > > > > > @@ -246,6 +246,18 @@ kernel_addr_r:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >    A size of 16MB for the kernel is likely adequate.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > +kernel_comp_addr_r:
> > > > > > > > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > > > > > > > from
> > > > > > > > > > > > > > a compressed
> > > > > > > > > > > > > > +  Image(.gz, .bz2, .lzma, .lzo) using booti command. It
> > > > > > > > > > > > > > represents
> > > > > > > > > > > > > > the location
> > > > > > > > > > > > > > +  in RAM where the compressed Image will be decompressed
> > > > > > > > > > > > > > temporarily. Once the
> > > > > > > > > > > > > > +  decompression is complete, decompressed data will be moved
> > > > > > > > > > > > > > kernel_addr_r for
> > > > > > > > > > > > > > +  booting.
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > +filesize:
> > > > > > > > > > > > > > +  Optional. This is only required if user wants to boot Linux
> > > > > > > > > > > > > > from
> > > > > > > > > > > > > > a compressed
> > > > > > > > > > > > > > +  Image using booti command. It represents the size of the
> > > > > > > > > > > > > > compressed file. The
> > > > > > > > > > > > > > +  size has to at least the size of loaded image for
> > > > > > > > > > > > > > decompression
> > > > > > > > > > > > > > to succeed.
> > > > > > > > > > > > > > +
> > > > > > > > > > > > >
> > > > > > > > > > > > > I am not sure I like using filesize here compared to
> > > > > > > > > > > > > kernel_gz_size.
> > > > > > > > > > > > > It's true that $filesize will be set once your load the binary,
> > > > > > > > > > > > > e.g.
> > > > > > > > > > > > > from mmc.
> > > > > > > > > > > > > But in EXTLINUX/PXE situation $filesize could be wrong.
> > > > > > > > > > > > >
> > > > > > > > > > > > > The load happens in this order:
> > > > > > > > > > > > > - initrd
> > > > > > > > > > > > > - kernel
> > > > > > > > > > > > > - fdt
> > > > > > > > > > > > >
> > > > > > > > > > > > > So if I specify FDT in my EXTLINUX/PXE config the $filesize will
> > > > > > > > > > > > > be
> > > > > > > > > > > > > wrong
> > > > > > > > > > > > > while attempting to boot the kernel. Unless we save filesize of
> > > > > > > > > > > > > kernel after
> > > > > > > > > > > > > loading and set it again before calling do_booti() in pxe.c.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Currently I set kernel_gz_size in board environment, which is set
> > > > > > > > > > > > > to
> > > > > > > > > > > > > max
> > > > > > > > > > > > > allowed size.
> > > > > > > > > > > > >
> > > > > > > > > > > > > david
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > Ahh okay. There are two ways to fix it.
> > > > > > > > > > > >
> > > > > > > > > > > > 1. Fix the pxe implementation but save the filesize to be used
> > > > > > > > > > > > later.
> > > > > > > > > > > >
> > > > > > > > > > > > But some other user may fall into the same issue without realizing
> > > > > > > > > > > > it
> > > > > > > > > > > > if the user loads any other image after compressed kernel Image.
> > > > > > > > > > > >
> > > > > > > > > > > > We need clear documentation saying that, compressed kernel Image
> > > > > > > > > > > > should
> > > > > > > > > > > > be loaded last before executing booti or $filesize must be set
> > > > > > > > > > > > manually
> > > > > > > > > > > > before calling booti.
> > > > > > > > > > > >
> > > > > > > > > > > > 2. Just change it back to kernel_comp_size (compressed kernel image
> > > > > > > > > > > > size) to avoid any confusion.
> > > > > > > > > > > >
> > > > > > > > > > > > Any preference ?
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Any thoughts ?
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > I think this patch got lost during holidays :). Any suggestion on what
> > > > > > > > > > should be the best approach to resolve the issue with pxe
> > > > > > > > > > implementation ?
> > > > > > > > >
> > > > > > > > > I think that we really need to be careful when adding "automagic"
> > > > > > > > > features like this.  Thinking harder about it all, we can't re-use any
> > > > > > > > > existing variable as there's going to be some case of a setup breaking
> > > > > > > > > in strange silent ways.
> > > > > > > > >
> > > > > > > > > Can we uncompress a single chunk / page / something of the compressed
> > > > > > > > > image to get the header and thus know the uncompressed size?  We would
> > > > > > > > > then know the compressed size is no larger than that and can progress
> > > > > > > > > from there?
> > > > > > > >
> > > > > > > > After a quick google on various compression formats I get impression this
> > > > > > > > is not a common thing to record in the headers. The kernels could be
> > > > > > > > compressed with gzip, bzip2, lz4, lzma and lzo (I only checked Makefile for
> > > > > > > >  riscv).
> > > > > > >
> > > > > > > Sorry I wasn't clear enough.  The contents in question here contain a
> > > > > > > header at the start which does contain the uncompressed image size.
> > > > > > >
> > > > > >
> > > > > > The objective of the patch was to support all the compressed image
> > > > > > type that kernel supports.
> > > > > > i.e. gz, bz2, lzma, lzo.
> > > > > >
> > > > > > uncompressed image size in compressed header in the following file formats:
> > > > > >
> > > > > > 1. gz: Uncompressed size is at the footer (http://www.zlib.org/rfc-gzip.html)
> > > > > > 2. bz2: No information in the header
> > > > > > 3. lzma: Present in the header
> > > > > > (https://svn.python.org/projects/external/xz-5.0.3/doc/lzma-file-format.txt)
> > > > > > 4. lzo: Present in the header (https://gist.github.com/jledet/1333896)
> > > > > >
> > > > > > To summarize, if we want to parse the compressed file to determine the
> > > > > > uncompressed size,
> > > > > > we have to add the support to generic compression library for all file
> > > > > > formats rather than just doing it here.
> > > > > > Additionally, not all format mandates the uncompressed size which
> > > > > > makes it difficult.
> > > > >
> > > > > Here's my confusion, and perhaps dumb question.  What happens if we:
> > > > > 1. Uncompress the first "chunk" of the data, which will let us at the
> > > > > Image header.
> > > >
> >
> > Theoretically that should work. However, current zlib implementation
> > doesn't allow
> > decompressing few chunks of data. I tried a few different sizes. But
> > any size less
> > than compressed file size results in following error.
> >
> > Error: inflate() returned -5
> >
> > There may be some way to modify zlib implementation to allow that but I am not a
> > compression expert to make such changes.
>
> Thanks for looking in to this.
>
> > Let us know what other alternative approaches you prefer.
> >
> > 1. Save $filesize to kernel_comp_size after loading kernel binary and
> > pass it around in C code.
> > or
> > 2.  Just use an environment variable other than filesize that users can set it.
>
> I think we need to go with #2 here as I fear #1 will be error prone (how
> do we know we just loaded the kernel?)

You are correct. The bigger issue is what if kernel is already loaded
at a predefined address
by previous stage loader. We will stick to kernel_comp_size for now. I
will update the patch.

over just adapting the existing
> general logic to add 'setenv kernel_comp_size $filesize' after loading
> the kernel image.  I'm open to reviewing #1 all the same however.

> Thanks!
>
> --
> Tom



-- 
Regards,
Atish