All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] image: Add support for ZSTD decompression
@ 2020-04-25 17:37 Robert Marko
  2020-05-01 14:56 ` Tom Rini
  2020-07-08  3:02 ` Tom Rini
  0 siblings, 2 replies; 13+ messages in thread
From: Robert Marko @ 2020-04-25 17:37 UTC (permalink / raw)
  To: u-boot

This patch adds support for ZSTD decompression of FIT images.

Signed-off-by: Robert Marko <robert.marko@sartura.hr>
Cc: Luka Perkov <luka.perkov@sartura.hr>
---
 common/image.c  | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/image.h |  1 +
 2 files changed, 53 insertions(+)

diff --git a/common/image.c b/common/image.c
index 94873cb6ed..70ba0f4328 100644
--- a/common/image.c
+++ b/common/image.c
@@ -42,6 +42,7 @@
 #include <lzma/LzmaTypes.h>
 #include <lzma/LzmaDec.h>
 #include <lzma/LzmaTools.h>
+#include <linux/zstd.h>
 
 #ifdef CONFIG_CMD_BDI
 extern int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
@@ -193,6 +194,7 @@ static const table_entry_t uimage_comp[] = {
 	{	IH_COMP_LZMA,	"lzma",		"lzma compressed",	},
 	{	IH_COMP_LZO,	"lzo",		"lzo compressed",	},
 	{	IH_COMP_LZ4,	"lz4",		"lz4 compressed",	},
+	{	IH_COMP_ZSTD,	"zstd",		"zstd compressed",	},
 	{	-1,		"",		"",			},
 };
 
@@ -480,6 +482,56 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
 		break;
 	}
 #endif /* CONFIG_LZ4 */
+#ifdef CONFIG_ZSTD
+	case IH_COMP_ZSTD: {
+		size_t size = unc_len;
+		ZSTD_DStream *dstream;
+		ZSTD_inBuffer in_buf;
+		ZSTD_outBuffer out_buf;
+		void *workspace;
+		size_t wsize;
+
+		wsize = ZSTD_DStreamWorkspaceBound(image_len);
+		workspace = malloc(wsize);
+		if (!workspace) {
+			debug("%s: cannot allocate workspace of size %zu\n", __func__,
+			      wsize);
+			return -1;
+		}
+
+		dstream = ZSTD_initDStream(image_len, workspace, wsize);
+		if (!dstream) {
+			printf("%s: ZSTD_initDStream failed\n", __func__);
+			return ZSTD_getErrorCode(ret);
+		}
+
+		in_buf.src = image_buf;
+		in_buf.pos = 0;
+		in_buf.size = image_len;
+
+		out_buf.dst = load_buf;
+		out_buf.pos = 0;
+		out_buf.size = size;
+
+		while (1) {
+			size_t ret;
+
+			ret = ZSTD_decompressStream(dstream, &out_buf, &in_buf);
+			if (ZSTD_isError(ret)) {
+				printf("%s: ZSTD_decompressStream error %d\n", __func__,
+				       ZSTD_getErrorCode(ret));
+				return ZSTD_getErrorCode(ret);
+			}
+
+			if (in_buf.pos >= image_len || !ret)
+				break;
+		}
+
+		image_len = out_buf.pos;
+
+		break;
+	}
+#endif /* CONFIG_ZSTD */
 	default:
 		printf("Unimplemented compression type %d\n", comp);
 		return -ENOSYS;
diff --git a/include/image.h b/include/image.h
index 3ffc0fdd68..8c79f6df9e 100644
--- a/include/image.h
+++ b/include/image.h
@@ -308,6 +308,7 @@ enum {
 	IH_COMP_LZMA,			/* lzma  Compression Used	*/
 	IH_COMP_LZO,			/* lzo   Compression Used	*/
 	IH_COMP_LZ4,			/* lz4   Compression Used	*/
+	IH_COMP_ZSTD,			/* zstd   Compression Used	*/
 
 	IH_COMP_COUNT,
 };
-- 
2.26.2

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

* [PATCH] image: Add support for ZSTD decompression
  2020-04-25 17:37 [PATCH] image: Add support for ZSTD decompression Robert Marko
@ 2020-05-01 14:56 ` Tom Rini
  2020-05-01 15:15   ` Robert Marko
  2020-07-08  3:02 ` Tom Rini
  1 sibling, 1 reply; 13+ messages in thread
From: Tom Rini @ 2020-05-01 14:56 UTC (permalink / raw)
  To: u-boot

On Sat, Apr 25, 2020 at 07:37:21PM +0200, Robert Marko wrote:

> This patch adds support for ZSTD decompression of FIT images.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>
> ---
>  common/image.c  | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/image.h |  1 +
>  2 files changed, 53 insertions(+)
> 
> diff --git a/common/image.c b/common/image.c
> index 94873cb6ed..70ba0f4328 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -42,6 +42,7 @@
>  #include <lzma/LzmaTypes.h>
>  #include <lzma/LzmaDec.h>
>  #include <lzma/LzmaTools.h>
> +#include <linux/zstd.h>
>  
>  #ifdef CONFIG_CMD_BDI
>  extern int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
> @@ -193,6 +194,7 @@ static const table_entry_t uimage_comp[] = {
>  	{	IH_COMP_LZMA,	"lzma",		"lzma compressed",	},
>  	{	IH_COMP_LZO,	"lzo",		"lzo compressed",	},
>  	{	IH_COMP_LZ4,	"lz4",		"lz4 compressed",	},
> +	{	IH_COMP_ZSTD,	"zstd",		"zstd compressed",	},
>  	{	-1,		"",		"",			},
>  };
>  
> @@ -480,6 +482,56 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
>  		break;
>  	}
>  #endif /* CONFIG_LZ4 */
> +#ifdef CONFIG_ZSTD

We need to add SPL_ZSTD as a symbol to lib/Kconfig and then use
CONFIG_IS_ENABLED() tests here to avoid growth in SPL.  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/20200501/3ab0b783/attachment.sig>

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

* [PATCH] image: Add support for ZSTD decompression
  2020-05-01 14:56 ` Tom Rini
@ 2020-05-01 15:15   ` Robert Marko
  2020-05-01 16:42     ` Tom Rini
  0 siblings, 1 reply; 13+ messages in thread
From: Robert Marko @ 2020-05-01 15:15 UTC (permalink / raw)
  To: u-boot

On Fri, May 1, 2020 at 4:56 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Sat, Apr 25, 2020 at 07:37:21PM +0200, Robert Marko wrote:
>
> > This patch adds support for ZSTD decompression of FIT images.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > Cc: Luka Perkov <luka.perkov@sartura.hr>
> > ---
> >  common/image.c  | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/image.h |  1 +
> >  2 files changed, 53 insertions(+)
> >
> > diff --git a/common/image.c b/common/image.c
> > index 94873cb6ed..70ba0f4328 100644
> > --- a/common/image.c
> > +++ b/common/image.c
> > @@ -42,6 +42,7 @@
> >  #include <lzma/LzmaTypes.h>
> >  #include <lzma/LzmaDec.h>
> >  #include <lzma/LzmaTools.h>
> > +#include <linux/zstd.h>
> >
> >  #ifdef CONFIG_CMD_BDI
> >  extern int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
> > @@ -193,6 +194,7 @@ static const table_entry_t uimage_comp[] = {
> >       {       IH_COMP_LZMA,   "lzma",         "lzma compressed",      },
> >       {       IH_COMP_LZO,    "lzo",          "lzo compressed",       },
> >       {       IH_COMP_LZ4,    "lz4",          "lz4 compressed",       },
> > +     {       IH_COMP_ZSTD,   "zstd",         "zstd compressed",      },
> >       {       -1,             "",             "",                     },
> >  };
> >
> > @@ -480,6 +482,56 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
> >               break;
> >       }
> >  #endif /* CONFIG_LZ4 */
> > +#ifdef CONFIG_ZSTD
>
> We need to add SPL_ZSTD as a symbol to lib/Kconfig and then use
> CONFIG_IS_ENABLED() tests here to avoid growth in SPL.  Thanks!
Hi,
is that something that I need to do or?

Thanks
>
> --
> Tom

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

* [PATCH] image: Add support for ZSTD decompression
  2020-05-01 15:15   ` Robert Marko
@ 2020-05-01 16:42     ` Tom Rini
  2020-05-03 10:24       ` Robert Marko
  0 siblings, 1 reply; 13+ messages in thread
From: Tom Rini @ 2020-05-01 16:42 UTC (permalink / raw)
  To: u-boot

On Fri, May 01, 2020 at 05:15:41PM +0200, Robert Marko wrote:
> On Fri, May 1, 2020 at 4:56 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sat, Apr 25, 2020 at 07:37:21PM +0200, Robert Marko wrote:
> >
> > > This patch adds support for ZSTD decompression of FIT images.
> > >
> > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > > Cc: Luka Perkov <luka.perkov@sartura.hr>
> > > ---
> > >  common/image.c  | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
> > >  include/image.h |  1 +
> > >  2 files changed, 53 insertions(+)
> > >
> > > diff --git a/common/image.c b/common/image.c
> > > index 94873cb6ed..70ba0f4328 100644
> > > --- a/common/image.c
> > > +++ b/common/image.c
> > > @@ -42,6 +42,7 @@
> > >  #include <lzma/LzmaTypes.h>
> > >  #include <lzma/LzmaDec.h>
> > >  #include <lzma/LzmaTools.h>
> > > +#include <linux/zstd.h>
> > >
> > >  #ifdef CONFIG_CMD_BDI
> > >  extern int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]);
> > > @@ -193,6 +194,7 @@ static const table_entry_t uimage_comp[] = {
> > >       {       IH_COMP_LZMA,   "lzma",         "lzma compressed",      },
> > >       {       IH_COMP_LZO,    "lzo",          "lzo compressed",       },
> > >       {       IH_COMP_LZ4,    "lz4",          "lz4 compressed",       },
> > > +     {       IH_COMP_ZSTD,   "zstd",         "zstd compressed",      },
> > >       {       -1,             "",             "",                     },
> > >  };
> > >
> > > @@ -480,6 +482,56 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
> > >               break;
> > >       }
> > >  #endif /* CONFIG_LZ4 */
> > > +#ifdef CONFIG_ZSTD
> >
> > We need to add SPL_ZSTD as a symbol to lib/Kconfig and then use
> > CONFIG_IS_ENABLED() tests here to avoid growth in SPL.  Thanks!
> Hi,
> is that something that I need to do or?

Yes.  You need to add the symbol, and then the code you're adding needs
to make use of '#if CONFIG_IS_ENABLED(ZSTD)' rather than '#ifdef
CONFIG_ZSTD'.  Sorry for not being clear enough.

-- 
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/20200501/ddc2e77b/attachment.sig>

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

* [PATCH] image: Add support for ZSTD decompression
  2020-05-01 16:42     ` Tom Rini
@ 2020-05-03 10:24       ` Robert Marko
  2020-05-04 13:03         ` Tom Rini
  0 siblings, 1 reply; 13+ messages in thread
From: Robert Marko @ 2020-05-03 10:24 UTC (permalink / raw)
  To: u-boot

Hi,

I checked and SPL_ZSTD symbol already exists.
But trying to use #if CONFIG_IS_ENABLED(ZSTD) inside
of the switch case will fail with the preprocessor error:
In file included from tools/common/image.c:1:

> ./tools/../common/image.c: In function ?image_decomp?:
> ./tools/../common/image.c:510:22: error: missing binary operator before
> token "("
>   510 | #if CONFIG_IS_ENABLED(ZSTD)


Outside of the switch_case it works fine


On Fri, May 1, 2020 at 6:42 PM Tom Rini <trini@konsulko.com> wrote:

> On Fri, May 01, 2020 at 05:15:41PM +0200, Robert Marko wrote:
> > On Fri, May 1, 2020 at 4:56 PM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sat, Apr 25, 2020 at 07:37:21PM +0200, Robert Marko wrote:
> > >
> > > > This patch adds support for ZSTD decompression of FIT images.
> > > >
> > > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > > > Cc: Luka Perkov <luka.perkov@sartura.hr>
> > > > ---
> > > >  common/image.c  | 52
> +++++++++++++++++++++++++++++++++++++++++++++++++
> > > >  include/image.h |  1 +
> > > >  2 files changed, 53 insertions(+)
> > > >
> > > > diff --git a/common/image.c b/common/image.c
> > > > index 94873cb6ed..70ba0f4328 100644
> > > > --- a/common/image.c
> > > > +++ b/common/image.c
> > > > @@ -42,6 +42,7 @@
> > > >  #include <lzma/LzmaTypes.h>
> > > >  #include <lzma/LzmaDec.h>
> > > >  #include <lzma/LzmaTools.h>
> > > > +#include <linux/zstd.h>
> > > >
> > > >  #ifdef CONFIG_CMD_BDI
> > > >  extern int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *
> const argv[]);
> > > > @@ -193,6 +194,7 @@ static const table_entry_t uimage_comp[] = {
> > > >       {       IH_COMP_LZMA,   "lzma",         "lzma compressed",
>   },
> > > >       {       IH_COMP_LZO,    "lzo",          "lzo compressed",
>  },
> > > >       {       IH_COMP_LZ4,    "lz4",          "lz4 compressed",
>  },
> > > > +     {       IH_COMP_ZSTD,   "zstd",         "zstd compressed",
>   },
> > > >       {       -1,             "",             "",
>  },
> > > >  };
> > > >
> > > > @@ -480,6 +482,56 @@ int image_decomp(int comp, ulong load, ulong
> image_start, int type,
> > > >               break;
> > > >       }
> > > >  #endif /* CONFIG_LZ4 */
> > > > +#ifdef CONFIG_ZSTD
> > >
> > > We need to add SPL_ZSTD as a symbol to lib/Kconfig and then use
> > > CONFIG_IS_ENABLED() tests here to avoid growth in SPL.  Thanks!
> > Hi,
> > is that something that I need to do or?
>
> Yes.  You need to add the symbol, and then the code you're adding needs
> to make use of '#if CONFIG_IS_ENABLED(ZSTD)' rather than '#ifdef
> CONFIG_ZSTD'.  Sorry for not being clear enough.
>
> --
> Tom
>

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

* [PATCH] image: Add support for ZSTD decompression
  2020-05-03 10:24       ` Robert Marko
@ 2020-05-04 13:03         ` Tom Rini
  2020-05-05 21:19           ` Robert Marko
  0 siblings, 1 reply; 13+ messages in thread
From: Tom Rini @ 2020-05-04 13:03 UTC (permalink / raw)
  To: u-boot

On Sun, May 03, 2020 at 12:24:14PM +0200, Robert Marko wrote:
> Hi,
> 
> I checked and SPL_ZSTD symbol already exists.
> But trying to use #if CONFIG_IS_ENABLED(ZSTD) inside
> of the switch case will fail with the preprocessor error:
> In file included from tools/common/image.c:1:

Ah right, oops.

> > ./tools/../common/image.c: In function ?image_decomp?:
> > ./tools/../common/image.c:510:22: error: missing binary operator before
> > token "("
> >   510 | #if CONFIG_IS_ENABLED(ZSTD)
> 
> 
> Outside of the switch_case it works fine

Sounds like <linux/kconfig.h> needs an explicit #include then.

> 
> 
> On Fri, May 1, 2020 at 6:42 PM Tom Rini <trini@konsulko.com> wrote:
> 
> > On Fri, May 01, 2020 at 05:15:41PM +0200, Robert Marko wrote:
> > > On Fri, May 1, 2020 at 4:56 PM Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Sat, Apr 25, 2020 at 07:37:21PM +0200, Robert Marko wrote:
> > > >
> > > > > This patch adds support for ZSTD decompression of FIT images.
> > > > >
> > > > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > > > > Cc: Luka Perkov <luka.perkov@sartura.hr>
> > > > > ---
> > > > >  common/image.c  | 52
> > +++++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  include/image.h |  1 +
> > > > >  2 files changed, 53 insertions(+)
> > > > >
> > > > > diff --git a/common/image.c b/common/image.c
> > > > > index 94873cb6ed..70ba0f4328 100644
> > > > > --- a/common/image.c
> > > > > +++ b/common/image.c
> > > > > @@ -42,6 +42,7 @@
> > > > >  #include <lzma/LzmaTypes.h>
> > > > >  #include <lzma/LzmaDec.h>
> > > > >  #include <lzma/LzmaTools.h>
> > > > > +#include <linux/zstd.h>
> > > > >
> > > > >  #ifdef CONFIG_CMD_BDI
> > > > >  extern int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *
> > const argv[]);
> > > > > @@ -193,6 +194,7 @@ static const table_entry_t uimage_comp[] = {
> > > > >       {       IH_COMP_LZMA,   "lzma",         "lzma compressed",
> >   },
> > > > >       {       IH_COMP_LZO,    "lzo",          "lzo compressed",
> >  },
> > > > >       {       IH_COMP_LZ4,    "lz4",          "lz4 compressed",
> >  },
> > > > > +     {       IH_COMP_ZSTD,   "zstd",         "zstd compressed",
> >   },
> > > > >       {       -1,             "",             "",
> >  },
> > > > >  };
> > > > >
> > > > > @@ -480,6 +482,56 @@ int image_decomp(int comp, ulong load, ulong
> > image_start, int type,
> > > > >               break;
> > > > >       }
> > > > >  #endif /* CONFIG_LZ4 */
> > > > > +#ifdef CONFIG_ZSTD
> > > >
> > > > We need to add SPL_ZSTD as a symbol to lib/Kconfig and then use
> > > > CONFIG_IS_ENABLED() tests here to avoid growth in SPL.  Thanks!
> > > Hi,
> > > is that something that I need to do or?
> >
> > Yes.  You need to add the symbol, and then the code you're adding needs
> > to make use of '#if CONFIG_IS_ENABLED(ZSTD)' rather than '#ifdef
> > CONFIG_ZSTD'.  Sorry for not being clear enough.
> >
> > --
> > Tom
> >

-- 
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/20200504/e2eea154/attachment.sig>

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

* [PATCH] image: Add support for ZSTD decompression
  2020-05-04 13:03         ` Tom Rini
@ 2020-05-05 21:19           ` Robert Marko
  2020-05-20 11:38             ` Robert Marko
  0 siblings, 1 reply; 13+ messages in thread
From: Robert Marko @ 2020-05-05 21:19 UTC (permalink / raw)
  To: u-boot

On Mon, May 4, 2020 at 3:04 PM Tom Rini <trini@konsulko.com> wrote:

> On Sun, May 03, 2020 at 12:24:14PM +0200, Robert Marko wrote:
> > Hi,
> >
> > I checked and SPL_ZSTD symbol already exists.
> > But trying to use #if CONFIG_IS_ENABLED(ZSTD) inside
> > of the switch case will fail with the preprocessor error:
> > In file included from tools/common/image.c:1:
>
> Ah right, oops.
>
> > > ./tools/../common/image.c: In function ?image_decomp?:
> > > ./tools/../common/image.c:510:22: error: missing binary operator before
> > > token "("
> > >   510 | #if CONFIG_IS_ENABLED(ZSTD)
> >
> >
> > Outside of the switch_case it works fine
>
> Sounds like <linux/kconfig.h> needs an explicit #include then.
>
Unfortunately, it does not help.
Preprocessor throws the same error

>
> >
> >
> > On Fri, May 1, 2020 at 6:42 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > > On Fri, May 01, 2020 at 05:15:41PM +0200, Robert Marko wrote:
> > > > On Fri, May 1, 2020 at 4:56 PM Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Sat, Apr 25, 2020 at 07:37:21PM +0200, Robert Marko wrote:
> > > > >
> > > > > > This patch adds support for ZSTD decompression of FIT images.
> > > > > >
> > > > > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > > > > > Cc: Luka Perkov <luka.perkov@sartura.hr>
> > > > > > ---
> > > > > >  common/image.c  | 52
> > > +++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > >  include/image.h |  1 +
> > > > > >  2 files changed, 53 insertions(+)
> > > > > >
> > > > > > diff --git a/common/image.c b/common/image.c
> > > > > > index 94873cb6ed..70ba0f4328 100644
> > > > > > --- a/common/image.c
> > > > > > +++ b/common/image.c
> > > > > > @@ -42,6 +42,7 @@
> > > > > >  #include <lzma/LzmaTypes.h>
> > > > > >  #include <lzma/LzmaDec.h>
> > > > > >  #include <lzma/LzmaTools.h>
> > > > > > +#include <linux/zstd.h>
> > > > > >
> > > > > >  #ifdef CONFIG_CMD_BDI
> > > > > >  extern int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char
> *
> > > const argv[]);
> > > > > > @@ -193,6 +194,7 @@ static const table_entry_t uimage_comp[] = {
> > > > > >       {       IH_COMP_LZMA,   "lzma",         "lzma compressed",
> > >   },
> > > > > >       {       IH_COMP_LZO,    "lzo",          "lzo compressed",
> > >  },
> > > > > >       {       IH_COMP_LZ4,    "lz4",          "lz4 compressed",
> > >  },
> > > > > > +     {       IH_COMP_ZSTD,   "zstd",         "zstd compressed",
> > >   },
> > > > > >       {       -1,             "",             "",
> > >  },
> > > > > >  };
> > > > > >
> > > > > > @@ -480,6 +482,56 @@ int image_decomp(int comp, ulong load, ulong
> > > image_start, int type,
> > > > > >               break;
> > > > > >       }
> > > > > >  #endif /* CONFIG_LZ4 */
> > > > > > +#ifdef CONFIG_ZSTD
> > > > >
> > > > > We need to add SPL_ZSTD as a symbol to lib/Kconfig and then use
> > > > > CONFIG_IS_ENABLED() tests here to avoid growth in SPL.  Thanks!
> > > > Hi,
> > > > is that something that I need to do or?
> > >
> > > Yes.  You need to add the symbol, and then the code you're adding needs
> > > to make use of '#if CONFIG_IS_ENABLED(ZSTD)' rather than '#ifdef
> > > CONFIG_ZSTD'.  Sorry for not being clear enough.
> > >
> > > --
> > > Tom
> > >
>
> --
> Tom
>

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

* [PATCH] image: Add support for ZSTD decompression
  2020-05-05 21:19           ` Robert Marko
@ 2020-05-20 11:38             ` Robert Marko
  2020-05-20 12:35               ` Tom Rini
  0 siblings, 1 reply; 13+ messages in thread
From: Robert Marko @ 2020-05-20 11:38 UTC (permalink / raw)
  To: u-boot

Tom,
I have tried various things but CONFIG_IS_ENABLED won't work inside of
switch case.
It works fine outside of if though.


On Tue, May 5, 2020 at 11:19 PM Robert Marko <robert.marko@sartura.hr> wrote:
>
>
>
> On Mon, May 4, 2020 at 3:04 PM Tom Rini <trini@konsulko.com> wrote:
>>
>> On Sun, May 03, 2020 at 12:24:14PM +0200, Robert Marko wrote:
>> > Hi,
>> >
>> > I checked and SPL_ZSTD symbol already exists.
>> > But trying to use #if CONFIG_IS_ENABLED(ZSTD) inside
>> > of the switch case will fail with the preprocessor error:
>> > In file included from tools/common/image.c:1:
>>
>> Ah right, oops.
>>
>> > > ./tools/../common/image.c: In function ?image_decomp?:
>> > > ./tools/../common/image.c:510:22: error: missing binary operator before
>> > > token "("
>> > >   510 | #if CONFIG_IS_ENABLED(ZSTD)
>> >
>> >
>> > Outside of the switch_case it works fine
>>
>> Sounds like <linux/kconfig.h> needs an explicit #include then.
>
> Unfortunately, it does not help.
> Preprocessor throws the same error
>>
>>
>> >
>> >
>> > On Fri, May 1, 2020 at 6:42 PM Tom Rini <trini@konsulko.com> wrote:
>> >
>> > > On Fri, May 01, 2020 at 05:15:41PM +0200, Robert Marko wrote:
>> > > > On Fri, May 1, 2020 at 4:56 PM Tom Rini <trini@konsulko.com> wrote:
>> > > > >
>> > > > > On Sat, Apr 25, 2020 at 07:37:21PM +0200, Robert Marko wrote:
>> > > > >
>> > > > > > This patch adds support for ZSTD decompression of FIT images.
>> > > > > >
>> > > > > > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
>> > > > > > Cc: Luka Perkov <luka.perkov@sartura.hr>
>> > > > > > ---
>> > > > > >  common/image.c  | 52
>> > > +++++++++++++++++++++++++++++++++++++++++++++++++
>> > > > > >  include/image.h |  1 +
>> > > > > >  2 files changed, 53 insertions(+)
>> > > > > >
>> > > > > > diff --git a/common/image.c b/common/image.c
>> > > > > > index 94873cb6ed..70ba0f4328 100644
>> > > > > > --- a/common/image.c
>> > > > > > +++ b/common/image.c
>> > > > > > @@ -42,6 +42,7 @@
>> > > > > >  #include <lzma/LzmaTypes.h>
>> > > > > >  #include <lzma/LzmaDec.h>
>> > > > > >  #include <lzma/LzmaTools.h>
>> > > > > > +#include <linux/zstd.h>
>> > > > > >
>> > > > > >  #ifdef CONFIG_CMD_BDI
>> > > > > >  extern int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *
>> > > const argv[]);
>> > > > > > @@ -193,6 +194,7 @@ static const table_entry_t uimage_comp[] = {
>> > > > > >       {       IH_COMP_LZMA,   "lzma",         "lzma compressed",
>> > >   },
>> > > > > >       {       IH_COMP_LZO,    "lzo",          "lzo compressed",
>> > >  },
>> > > > > >       {       IH_COMP_LZ4,    "lz4",          "lz4 compressed",
>> > >  },
>> > > > > > +     {       IH_COMP_ZSTD,   "zstd",         "zstd compressed",
>> > >   },
>> > > > > >       {       -1,             "",             "",
>> > >  },
>> > > > > >  };
>> > > > > >
>> > > > > > @@ -480,6 +482,56 @@ int image_decomp(int comp, ulong load, ulong
>> > > image_start, int type,
>> > > > > >               break;
>> > > > > >       }
>> > > > > >  #endif /* CONFIG_LZ4 */
>> > > > > > +#ifdef CONFIG_ZSTD
>> > > > >
>> > > > > We need to add SPL_ZSTD as a symbol to lib/Kconfig and then use
>> > > > > CONFIG_IS_ENABLED() tests here to avoid growth in SPL.  Thanks!
>> > > > Hi,
>> > > > is that something that I need to do or?
>> > >
>> > > Yes.  You need to add the symbol, and then the code you're adding needs
>> > > to make use of '#if CONFIG_IS_ENABLED(ZSTD)' rather than '#ifdef
>> > > CONFIG_ZSTD'.  Sorry for not being clear enough.
>> > >
>> > > --
>> > > Tom
>> > >
>>
>> --
>> Tom

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

* [PATCH] image: Add support for ZSTD decompression
  2020-05-20 11:38             ` Robert Marko
@ 2020-05-20 12:35               ` Tom Rini
  2020-06-08 19:01                 ` Robert Marko
  0 siblings, 1 reply; 13+ messages in thread
From: Tom Rini @ 2020-05-20 12:35 UTC (permalink / raw)
  To: u-boot

On Wed, May 20, 2020 at 01:38:01PM +0200, Robert Marko wrote:

> Tom,
> I have tried various things but CONFIG_IS_ENABLED won't work inside of
> switch case.
> It works fine outside of if though.

OK, thanks, I'll poke things more to make sure what I'm worried about
doesn't happen.

-- 
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/20200520/66e18932/attachment.sig>

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

* [PATCH] image: Add support for ZSTD decompression
  2020-05-20 12:35               ` Tom Rini
@ 2020-06-08 19:01                 ` Robert Marko
  2020-06-10  9:13                   ` Rasmus Villemoes
  0 siblings, 1 reply; 13+ messages in thread
From: Robert Marko @ 2020-06-08 19:01 UTC (permalink / raw)
  To: u-boot

On Wed, May 20, 2020 at 2:35 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, May 20, 2020 at 01:38:01PM +0200, Robert Marko wrote:
>
> > Tom,
> > I have tried various things but CONFIG_IS_ENABLED won't work inside of
> > switch case.
> > It works fine outside of if though.
>
> OK, thanks, I'll poke things more to make sure what I'm worried about
> doesn't happen.

Hi,
were you able to test the commit?

Regards,
Robert
>
> --
> Tom

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

* [PATCH] image: Add support for ZSTD decompression
  2020-06-08 19:01                 ` Robert Marko
@ 2020-06-10  9:13                   ` Rasmus Villemoes
  2020-06-11 20:10                     ` Tom Rini
  0 siblings, 1 reply; 13+ messages in thread
From: Rasmus Villemoes @ 2020-06-10  9:13 UTC (permalink / raw)
  To: u-boot

On 08/06/2020 21.01, Robert Marko wrote:
> On Wed, May 20, 2020 at 2:35 PM Tom Rini <trini@konsulko.com> wrote:
>>
>> On Wed, May 20, 2020 at 01:38:01PM +0200, Robert Marko wrote:
>>
>>> Tom,
>>> I have tried various things but CONFIG_IS_ENABLED won't work inside of
>>> switch case.
>>> It works fine outside of if though.
>>
>> OK, thanks, I'll poke things more to make sure what I'm worried about
>> doesn't happen.
> 
> Hi,
> were you able to test the commit?

So I took a look at this since I'm also interested in getting zstd
support in. And the problem is that when building host tools, the
IS_ENABLED / CONFIG_IS_ENABLED logic doesn't make much sense anyway -
one can of course include the kconfig.h header that defines those macros
to avoid the cpp error (one has to be careful, because the headers
included vary wildly depending on USE_HOSTCC), but I don't think it's
meaningful - the host tools should not depend on configuration for any
specific board.

Nor do they currently; the use of ifdef CONFIG_GZIP for example just
means that that part is ignored for the host tools - doing

--- a/common/image.c
+++ b/common/image.c
@@ -430,12 +430,12 @@ int image_decomp(int comp, ulong load, ulong
image_start, int type,
                else
                        ret = -ENOSPC;
                break;
-#ifdef CONFIG_GZIP
+//#ifdef CONFIG_GZIP
        case IH_COMP_GZIP: {
                ret = gunzip(load_buf, unc_len, image_buf, &image_len);
                break;
        }
-#endif /* CONFIG_GZIP */
+//#endif /* CONFIG_GZIP */
 #ifdef CONFIG_BZIP2
        case IH_COMP_BZIP2: {
                uint size = unc_len;

should be a no-op when CONFIG_GZIP=y, but it breaks the build of the
host tools.

That, in turn, means that the fit_check_sign tool (which AFAICT is the
only host tool that actually care about this - we really should build
host tools with the
-ffunction-sections,-fdata-sections,-Wl,--gc-sections flags) currently
"works" in the sense that it does check the signature, but it doesn't
really check if the image(s) can be decompressed using the vendored copy
of the decompression code; hidden in the output is

Unimplemented compression type 5

but for some reason that ends up being ok.

So, for now, I suggest simply surrounding all the CASE_IH_COMP_* (except
NONE) by a #ifndef USE_HOSTCC - I don't think that will change anything
at all for the existing host tools. Then you should be able to use the
proper CONFIG_IS_ENABLED(ZSTD) to guard the IH_COMP_ZSTD case. Also, it
would allow us to change the existing bare ifdefs to be SPL-aware, i.e.
change #ifdef CONFIG_GZIP to #if CONFIG_IS_ENABLED(GZIP) [assuming the
SPL_GZIP symbol actually exists].

Rasmus

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

* [PATCH] image: Add support for ZSTD decompression
  2020-06-10  9:13                   ` Rasmus Villemoes
@ 2020-06-11 20:10                     ` Tom Rini
  0 siblings, 0 replies; 13+ messages in thread
From: Tom Rini @ 2020-06-11 20:10 UTC (permalink / raw)
  To: u-boot

On Wed, Jun 10, 2020 at 11:13:37AM +0200, Rasmus Villemoes wrote:
> On 08/06/2020 21.01, Robert Marko wrote:
> > On Wed, May 20, 2020 at 2:35 PM Tom Rini <trini@konsulko.com> wrote:
> >>
> >> On Wed, May 20, 2020 at 01:38:01PM +0200, Robert Marko wrote:
> >>
> >>> Tom,
> >>> I have tried various things but CONFIG_IS_ENABLED won't work inside of
> >>> switch case.
> >>> It works fine outside of if though.
> >>
> >> OK, thanks, I'll poke things more to make sure what I'm worried about
> >> doesn't happen.
> > 
> > Hi,
> > were you able to test the commit?
> 
> So I took a look at this since I'm also interested in getting zstd
> support in. And the problem is that when building host tools, the
> IS_ENABLED / CONFIG_IS_ENABLED logic doesn't make much sense anyway -
> one can of course include the kconfig.h header that defines those macros
> to avoid the cpp error (one has to be careful, because the headers
> included vary wildly depending on USE_HOSTCC), but I don't think it's
> meaningful - the host tools should not depend on configuration for any
> specific board.
> 
> Nor do they currently; the use of ifdef CONFIG_GZIP for example just
> means that that part is ignored for the host tools - doing
> 
> --- a/common/image.c
> +++ b/common/image.c
> @@ -430,12 +430,12 @@ int image_decomp(int comp, ulong load, ulong
> image_start, int type,
>                 else
>                         ret = -ENOSPC;
>                 break;
> -#ifdef CONFIG_GZIP
> +//#ifdef CONFIG_GZIP
>         case IH_COMP_GZIP: {
>                 ret = gunzip(load_buf, unc_len, image_buf, &image_len);
>                 break;
>         }
> -#endif /* CONFIG_GZIP */
> +//#endif /* CONFIG_GZIP */
>  #ifdef CONFIG_BZIP2
>         case IH_COMP_BZIP2: {
>                 uint size = unc_len;
> 
> should be a no-op when CONFIG_GZIP=y, but it breaks the build of the
> host tools.
> 
> That, in turn, means that the fit_check_sign tool (which AFAICT is the
> only host tool that actually care about this - we really should build
> host tools with the
> -ffunction-sections,-fdata-sections,-Wl,--gc-sections flags) currently
> "works" in the sense that it does check the signature, but it doesn't
> really check if the image(s) can be decompressed using the vendored copy
> of the decompression code; hidden in the output is
> 
> Unimplemented compression type 5
> 
> but for some reason that ends up being ok.
> 
> So, for now, I suggest simply surrounding all the CASE_IH_COMP_* (except
> NONE) by a #ifndef USE_HOSTCC - I don't think that will change anything
> at all for the existing host tools. Then you should be able to use the
> proper CONFIG_IS_ENABLED(ZSTD) to guard the IH_COMP_ZSTD case. Also, it
> would allow us to change the existing bare ifdefs to be SPL-aware, i.e.
> change #ifdef CONFIG_GZIP to #if CONFIG_IS_ENABLED(GZIP) [assuming the
> SPL_GZIP symbol actually exists].

Alright, so first thanks for digging here.  Looking over the tools, for
verifying the signature we don't need to decompress the contents as we
are just checking the signature.  For dumpimage it's also OK to dump a
compressed file and leave it to regular tools to decompress.  Do you
want to make a patch to guard the existing section and CONFIG_IS_ENABLED
it or should I and Suggested-by you?  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/20200611/e87c666b/attachment.sig>

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

* [PATCH] image: Add support for ZSTD decompression
  2020-04-25 17:37 [PATCH] image: Add support for ZSTD decompression Robert Marko
  2020-05-01 14:56 ` Tom Rini
@ 2020-07-08  3:02 ` Tom Rini
  1 sibling, 0 replies; 13+ messages in thread
From: Tom Rini @ 2020-07-08  3:02 UTC (permalink / raw)
  To: u-boot

On Sat, Apr 25, 2020 at 07:37:21PM +0200, Robert Marko wrote:

> This patch adds support for ZSTD decompression of FIT images.
> 
> Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> Cc: Luka Perkov <luka.perkov@sartura.hr>

Applied to u-boot/master, 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/20200707/dbe07042/attachment.sig>

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

end of thread, other threads:[~2020-07-08  3:02 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-25 17:37 [PATCH] image: Add support for ZSTD decompression Robert Marko
2020-05-01 14:56 ` Tom Rini
2020-05-01 15:15   ` Robert Marko
2020-05-01 16:42     ` Tom Rini
2020-05-03 10:24       ` Robert Marko
2020-05-04 13:03         ` Tom Rini
2020-05-05 21:19           ` Robert Marko
2020-05-20 11:38             ` Robert Marko
2020-05-20 12:35               ` Tom Rini
2020-06-08 19:01                 ` Robert Marko
2020-06-10  9:13                   ` Rasmus Villemoes
2020-06-11 20:10                     ` Tom Rini
2020-07-08  3:02 ` Tom Rini

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