[PATCH] imxtract: specify max gunzip size

[PATCH] imxtract: specify max gunzip size

[Nikita Shubin]

From: Nikita Shubin <n.shubin@yadro.com>

Specify max gunzip size from config to override SYS_XIMG_LEN
default value wich is 0x800000.

Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
---
 cmd/Kconfig | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index d93731f2af6..06e200802d1 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -481,6 +481,13 @@ config CMD_XXD
 	help
 	  Print file as hexdump to standard output
 
+config SYS_XIMG_LEN
+	hex "imxtract max gunzip size"
+	default 0x800000
+	depends on CMD_XIMG
+	help
+	  Specify max gunzip size for imxtract tool.
+
 config CMD_SPL
 	bool "spl export - Export boot information for Falcon boot"
 	depends on SPL
-- 
2.37.4

Re: [PATCH] imxtract: specify max gunzip size

[Simon Glass]

Hi Nikita,

On Thu, 15 Dec 2022 at 23:03, Nikita Shubin <nikita.shubin@maquefel.me> wrote:
>
> From: Nikita Shubin <n.shubin@yadro.com>
>
> Specify max gunzip size from config to override SYS_XIMG_LEN
> default value wich is 0x800000.
>
> Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
> ---
>  cmd/Kconfig | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index d93731f2af6..06e200802d1 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -481,6 +481,13 @@ config CMD_XXD
>         help
>           Print file as hexdump to standard output
>
> +config SYS_XIMG_LEN
> +       hex "imxtract max gunzip size"
> +       default 0x800000
> +       depends on CMD_XIMG
> +       help
> +         Specify max gunzip size for imxtract tool.

Can you expand what this means? Is it the compress or decompressed
size? What is the impact of having it too big or too small? Help for
Kconfig options should be at least 4 lines of text. If you use patman
you should see a warning about that.

> +
>  config CMD_SPL
>         bool "spl export - Export boot information for Falcon boot"
>         depends on SPL
> --
> 2.37.4
>

Regards,
Simon

[PATCH v2] imxtract: specify max gunzip size

[Nikita Shubin]

From: Nikita Shubin <n.shubin@yadro.com>

Specify max gunzip size from config to override SYS_XIMG_LEN
default value wich is 0x800000. In case we have a large portion of FIT
image, for example gzipped kernel with decompressed size large than
0x800000 we should enlarge imxract area, otherwise extracting it will
fail.

It used to be a hardcoded define in cmd/ximg.c and we are moving it to
config.

Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
---
Hello Simon!

> Can you expand what this means? 

Yes you are right - i should be more verbose, please check a v2 version
of this patch.

Also i thought it's a good idea to drop hardcoded define in cmd/ximg.c
completely.

v1 -> v2:

- more verbosity in help
- drop define from cmd/ximg.c, take it from config
---
 cmd/Kconfig | 10 ++++++++++
 cmd/ximg.c  |  5 -----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index d93731f2af6..516dfd102f6 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -476,6 +476,16 @@ config CMD_XIMG
 	help
 	  Extract a part of a multi-image.
 
+config SYS_XIMG_LEN
+        hex "imxtract max gunzip size"
+        default 0x800000
+        depends on CMD_XIMG && GZIP
+        help
+	  This provides the size of the commad-line argument area
+	  used by imxtract for extracting pieces of FIT image.
+	  It should be large enough to fit uncompressed size of
+	  FIT piece we are extracting.
+
 config CMD_XXD
 	bool "xxd"
 	help
diff --git a/cmd/ximg.c b/cmd/ximg.c
index 1c40fd27a05..0a41b4b3bb2 100644
--- a/cmd/ximg.c
+++ b/cmd/ximg.c
@@ -27,11 +27,6 @@
 #include <asm/cache.h>
 #include <asm/io.h>
 
-#ifndef CONFIG_SYS_XIMG_LEN
-/* use 8MByte as default max gunzip size */
-#define CONFIG_SYS_XIMG_LEN	0x800000
-#endif
-
 static int
 do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 {
-- 
2.37.4

Re: [PATCH v2] imxtract: specify max gunzip size

[Simon Glass]

On Mon, 19 Dec 2022 at 01:05, Nikita Shubin <nikita.shubin@maquefel.me> wrote:
>
> From: Nikita Shubin <n.shubin@yadro.com>
>
> Specify max gunzip size from config to override SYS_XIMG_LEN
> default value wich is 0x800000. In case we have a large portion of FIT
> image, for example gzipped kernel with decompressed size large than
> 0x800000 we should enlarge imxract area, otherwise extracting it will
> fail.
>
> It used to be a hardcoded define in cmd/ximg.c and we are moving it to
> config.
>
> Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
> ---
> Hello Simon!
>
> > Can you expand what this means?
>
> Yes you are right - i should be more verbose, please check a v2 version
> of this patch.
>
> Also i thought it's a good idea to drop hardcoded define in cmd/ximg.c
> completely.
>
> v1 -> v2:
>
> - more verbosity in help
> - drop define from cmd/ximg.c, take it from config
> ---
>  cmd/Kconfig | 10 ++++++++++
>  cmd/ximg.c  |  5 -----
>  2 files changed, 10 insertions(+), 5 deletions(-)

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


>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index d93731f2af6..516dfd102f6 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -476,6 +476,16 @@ config CMD_XIMG
>         help
>           Extract a part of a multi-image.
>
> +config SYS_XIMG_LEN
> +        hex "imxtract max gunzip size"
> +        default 0x800000
> +        depends on CMD_XIMG && GZIP
> +        help
> +         This provides the size of the commad-line argument area
> +         used by imxtract for extracting pieces of FIT image.
> +         It should be large enough to fit uncompressed size of
> +         FIT piece we are extracting.
> +
>  config CMD_XXD
>         bool "xxd"
>         help
> diff --git a/cmd/ximg.c b/cmd/ximg.c
> index 1c40fd27a05..0a41b4b3bb2 100644
> --- a/cmd/ximg.c
> +++ b/cmd/ximg.c
> @@ -27,11 +27,6 @@
>  #include <asm/cache.h>
>  #include <asm/io.h>
>
> -#ifndef CONFIG_SYS_XIMG_LEN
> -/* use 8MByte as default max gunzip size */
> -#define CONFIG_SYS_XIMG_LEN    0x800000
> -#endif
> -
>  static int
>  do_imgextract(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>  {
> --
> 2.37.4
>

Re: [PATCH v2] imxtract: specify max gunzip size

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

On Mon, Dec 19, 2022 at 11:05:27AM +0300, Nikita Shubin wrote:

> From: Nikita Shubin <n.shubin@yadro.com>
> 
> Specify max gunzip size from config to override SYS_XIMG_LEN
> default value wich is 0x800000. In case we have a large portion of FIT
> image, for example gzipped kernel with decompressed size large than
> 0x800000 we should enlarge imxract area, otherwise extracting it will
> fail.
> 
> It used to be a hardcoded define in cmd/ximg.c and we are moving it to
> config.
> 
> Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]