[PATCH 1/3] binman: Use low level compression commands in tests

[PATCH 1/3] binman: Use low level compression commands in tests

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Use the low level compression commands in the tests to detect failures
in the utilities to compress and decompress data.

The decompression in U-Boot expects plain compression data without any
header.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

---
This commit breaks the binman tests because the tests wrongly expects a
header in front of the data.

 tools/binman/ftest.py | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index fa1f421c05..b1f564ed7d 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -107,6 +107,8 @@ BASE_DTB_PROPS = ['offset', 'size', 'image-pos']
 # Extra properties expected to be in the device tree when allow-repack is used
 REPACK_DTB_PROPS = ['orig-offset', 'orig-size']
 
+# Tools
+LZ4 = bintool.Bintool.create('lz4')
 
 class TestFunctional(unittest.TestCase):
     """Functional tests for binman
@@ -1967,7 +1969,7 @@ class TestFunctional(unittest.TestCase):
             self._ResetDtbs()
 
     def _decompress(self, data):
-        return comp_util.decompress(data, 'lz4')
+        return LZ4.decompress(data)
 
     def testCompress(self):
         """Test compression of blobs"""
@@ -2856,7 +2858,8 @@ class TestFunctional(unittest.TestCase):
     def testExtractCbfsRaw(self):
         """Test extracting CBFS compressed data without decompressing it"""
         data = self._RunExtractCmd('section/cbfs/u-boot-dtb', decomp=False)
-        dtb = comp_util.decompress(data, 'lzma', with_header=False)
+        lzma_alone = bintool.Bintool.create('lzma_alone')
+        dtb = lzma_alone.decompress(data)
         self.assertEqual(EXTRACT_DTB_SIZE, len(dtb))
 
     def testExtractBadEntry(self):
@@ -4412,6 +4415,9 @@ class TestFunctional(unittest.TestCase):
             }
         self.assertEqual(expected, props)
 
+    def _compress(self, data):
+        return LZ4.compress(data)
+
     def testCompressExtra(self):
         """Test compression of a section with no fixed size"""
         self._CheckLz4()
@@ -4427,15 +4433,17 @@ class TestFunctional(unittest.TestCase):
         rest = base[len(U_BOOT_DATA):]
 
         # Check compressed data
-        section1 = self._decompress(rest)
-        expect1 = comp_util.compress(COMPRESS_DATA + U_BOOT_DATA, 'lz4')
-        self.assertEquals(expect1, rest[:len(expect1)])
+        expect1 = self._compress(COMPRESS_DATA + U_BOOT_DATA)
+        data1 = rest[:len(expect1)]
+        self.assertEquals(expect1, data1)
+        section1 = self._decompress(data1)
         self.assertEquals(COMPRESS_DATA + U_BOOT_DATA, section1)
         rest1 = rest[len(expect1):]
 
-        section2 = self._decompress(rest1)
-        expect2 = comp_util.compress(COMPRESS_DATA + COMPRESS_DATA, 'lz4')
-        self.assertEquals(expect2, rest1[:len(expect2)])
+        expect2 = self._compress(COMPRESS_DATA + COMPRESS_DATA)
+        data2 = rest1[:len(expect2)]
+        self.assertEquals(expect2, data2)
+        section2 = self._decompress(data2)
         self.assertEquals(COMPRESS_DATA + COMPRESS_DATA, section2)
         rest2 = rest1[len(expect2):]
 
-- 
2.30.2

[PATCH 2/3] binman: Remove header from compressed data

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Remove header from compressed data because this is uncommon, not
supported by U-Boot and incompatible with external compressed artifacts.

The header was introduced as part of commit eb0f4a4cb402 ("binman:
Support replacing data in a cbfs") to allow device tree entries to be
larger that the compressed contents. Regarding the commit "this is
necessary to cope with a compressed device tree being updated in such a
way that it shrinks after the entry size is already set (an obscure
case)". This case need to be fixed without influence the compressed data
by itself.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

---

 tools/binman/cbfs_util.py |  8 ++++----
 tools/binman/comp_util.py | 11 ++---------
 tools/binman/entry.py     |  2 +-
 3 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py
index 9cad03886f..a1836f4ad3 100644
--- a/tools/binman/cbfs_util.py
+++ b/tools/binman/cbfs_util.py
@@ -241,9 +241,9 @@ class CbfsFile(object):
         """Handle decompressing data if necessary"""
         indata = self.data
         if self.compress == COMPRESS_LZ4:
-            data = comp_util.decompress(indata, 'lz4', with_header=False)
+            data = comp_util.decompress(indata, 'lz4')
         elif self.compress == COMPRESS_LZMA:
-            data = comp_util.decompress(indata, 'lzma', with_header=False)
+            data = comp_util.decompress(indata, 'lzma')
         else:
             data = indata
         self.memlen = len(data)
@@ -362,9 +362,9 @@ class CbfsFile(object):
         elif self.ftype == TYPE_RAW:
             orig_data = data
             if self.compress == COMPRESS_LZ4:
-                data = comp_util.compress(orig_data, 'lz4', with_header=False)
+                data = comp_util.compress(orig_data, 'lz4')
             elif self.compress == COMPRESS_LZMA:
-                data = comp_util.compress(orig_data, 'lzma', with_header=False)
+                data = comp_util.compress(orig_data, 'lzma')
             self.memlen = len(orig_data)
             self.data_len = len(data)
             attr = struct.pack(ATTR_COMPRESSION_FORMAT,
diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py
index dc76adab35..269bbf7975 100644
--- a/tools/binman/comp_util.py
+++ b/tools/binman/comp_util.py
@@ -3,7 +3,6 @@
 #
 """Utilities to compress and decompress data"""
 
-import struct
 import tempfile
 
 from binman import bintool
@@ -16,7 +15,7 @@ LZMA_ALONE = bintool.Bintool.create('lzma_alone')
 HAVE_LZMA_ALONE = LZMA_ALONE.is_present()
 
 
-def compress(indata, algo, with_header=True):
+def compress(indata, algo):
     """Compress some data using a given algorithm
 
     Note that for lzma this uses an old version of the algorithm, not that
@@ -41,12 +40,9 @@ def compress(indata, algo, with_header=True):
         data = LZMA_ALONE.compress(indata)
     else:
         raise ValueError("Unknown algorithm '%s'" % algo)
-    if with_header:
-        hdr = struct.pack('<I', len(data))
-        data = hdr + data
     return data
 
-def decompress(indata, algo, with_header=True):
+def decompress(indata, algo):
     """Decompress some data using a given algorithm
 
     Note that for lzma this uses an old version of the algorithm, not that
@@ -64,9 +60,6 @@ def decompress(indata, algo, with_header=True):
     """
     if algo == 'none':
         return indata
-    if with_header:
-        data_len = struct.unpack('<I', indata[:4])[0]
-        indata = indata[4:4 + data_len]
     if algo == 'lz4':
         data = LZ4.decompress(indata)
     elif algo == 'lzma':
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index a07a588864..8cbfd43af9 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -1069,7 +1069,7 @@ features to produce new behaviours.
             indata: Data to compress
 
         Returns:
-            Compressed data (first word is the compressed size)
+            Compressed data
         """
         self.uncomp_data = indata
         if self.compress != 'none':
-- 
2.30.2

[PATCH 3/3] binman: Add gzip bintool

[Stefan Herbrechtsmeier]

From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>

Add gzip bintool to binman to support on-the-fly compression of Linux
kernel images and FPGA bitstreams. The SPL basic fitImage implementation
supports only gzip decompression.

Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
---

 tools/binman/btool/gzip.py | 94 ++++++++++++++++++++++++++++++++++++++
 tools/binman/comp_util.py  | 17 +++++--
 tools/binman/ftest.py      | 18 ++++++++
 3 files changed, 124 insertions(+), 5 deletions(-)
 create mode 100644 tools/binman/btool/gzip.py

diff --git a/tools/binman/btool/gzip.py b/tools/binman/btool/gzip.py
new file mode 100644
index 0000000000..27fb654ed5
--- /dev/null
+++ b/tools/binman/btool/gzip.py
@@ -0,0 +1,94 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2022 Google LLC
+#
+"""Bintool implementation for gzip
+
+gzip allows compression and decompression of files.
+
+Documentation is available via::
+
+   man gzip
+"""
+
+import re
+import tempfile
+
+from binman import bintool
+from patman import tools
+
+# pylint: disable=C0103
+class Bintoolgzip(bintool.Bintool):
+    """Compression/decompression using the gzip algorithm
+
+    This bintool supports running `gzip` to compress and decompress data, as
+    used by binman.
+
+    It is also possible to fetch the tool, which uses `apt` to install it.
+
+    Documentation is available via::
+
+        man gzip
+    """
+    def __init__(self, name):
+        super().__init__(name, 'gzip compression')
+
+    def compress(self, indata):
+        """Compress data with gzip
+
+        Args:
+            indata (bytes): Data to compress
+
+        Returns:
+            bytes: Compressed data
+        """
+        with tempfile.NamedTemporaryFile(prefix='comp.tmp',
+                                         dir=tools.get_output_dir()) as tmp:
+            tools.write_file(tmp.name, indata)
+            args = ['-c', tmp.name]
+            return self.run_cmd(*args, binary=True)
+
+    def decompress(self, indata):
+        """Decompress data with gzip
+
+        Args:
+            indata (bytes): Data to decompress
+
+        Returns:
+            bytes: Decompressed data
+        """
+        with tempfile.NamedTemporaryFile(prefix='decomp.tmp',
+                                         dir=tools.get_output_dir()) as inf:
+            tools.write_file(inf.name, indata)
+            args = ['-cd', inf.name]
+            return self.run_cmd(*args, binary=True)
+
+    def fetch(self, method):
+        """Fetch handler for gzip
+
+        This installs the gzip package using the apt utility.
+
+        Args:
+            method (FETCH_...): Method to use
+
+        Returns:
+            True if the file was fetched and now installed, None if a method
+            other than FETCH_BIN was requested
+
+        Raises:
+            Valuerror: Fetching could not be completed
+        """
+        if method != bintool.FETCH_BIN:
+            return None
+        return self.apt_install('gzip')
+
+    def version(self):
+        """Version handler
+
+        Returns:
+            str: Version number of gzip
+        """
+        out = self.run_cmd('-V').strip()
+        if not out:
+            return super().version()
+        m_version = re.match(r'.*gzip ([0-9.]*).*', out)
+        return m_version.group(1) if m_version else out
diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py
index 269bbf7975..1d30f1613c 100644
--- a/tools/binman/comp_util.py
+++ b/tools/binman/comp_util.py
@@ -8,6 +8,9 @@ import tempfile
 from binman import bintool
 from patman import tools
 
+GZIP = bintool.Bintool.create('gzip')
+HAVE_GZIP = GZIP.is_present()
+
 LZ4 = bintool.Bintool.create('lz4')
 HAVE_LZ4 = LZ4.is_present()
 
@@ -21,19 +24,21 @@ def compress(indata, algo):
     Note that for lzma this uses an old version of the algorithm, not that
     provided by xz.
 
-    This requires 'lz4' and 'lzma_alone' tools. It also requires an output
+    This requires 'gzip', 'lz4' and 'lzma_alone' tools. It also requires an output
     directory to be previously set up, by calling PrepareOutputDir().
 
     Args:
         indata (bytes): Input data to compress
-        algo (str): Algorithm to use ('none', 'lz4' or 'lzma')
+        algo (str): Algorithm to use ('none', 'gzip', 'lz4' or 'lzma')
 
     Returns:
         bytes: Compressed data
     """
     if algo == 'none':
         return indata
-    if algo == 'lz4':
+    if algo == 'gzip':
+        data = GZIP.compress(indata)
+    elif algo == 'lz4':
         data = LZ4.compress(indata)
     # cbfstool uses a very old version of lzma
     elif algo == 'lzma':
@@ -48,12 +53,12 @@ def decompress(indata, algo):
     Note that for lzma this uses an old version of the algorithm, not that
     provided by xz.
 
-    This requires 'lz4' and 'lzma_alone' tools. It also requires an output
+    This requires 'gzip', 'lz4' and 'lzma_alone' tools. It also requires an output
     directory to be previously set up, by calling PrepareOutputDir().
 
     Args:
         indata (bytes): Input data to decompress
-        algo (str): Algorithm to use ('none', 'lz4' or 'lzma')
+        algo (str): Algorithm to use ('none', 'gzip', 'lz4' or 'lzma')
 
     Returns:
         (bytes) Compressed data
@@ -62,6 +67,8 @@ def decompress(indata, algo):
         return indata
     if algo == 'lz4':
         data = LZ4.decompress(indata)
+    elif algo == 'gzip':
+        data = GZIP.decompress(indata)
     elif algo == 'lzma':
         data = LZMA_ALONE.decompress(indata)
     else:
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index b1f564ed7d..5956ffa6e4 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -5226,6 +5226,24 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
             comp_util.decompress(b'1234', 'invalid')
         self.assertIn("Unknown algorithm 'invalid'", str(e.exception))
 
+    def testGzipCompress(self):
+        """Test gzip compress"""
+        if not comp_util.HAVE_GZIP:
+            self.skipTest('gzip not available')
+        data = comp_util.compress(COMPRESS_DATA, 'gzip')
+        gzip = bintool.Bintool.create('gzip')
+        orig = gzip.decompress(data)
+        self.assertEquals(COMPRESS_DATA, orig)
+
+    def testGzipDecompress(self):
+        """Test gzip decompress"""
+        if not comp_util.HAVE_GZIP:
+            self.skipTest('gzip not available')
+        gzip = bintool.Bintool.create('gzip')
+        data = gzip.compress(COMPRESS_DATA)
+        orig = comp_util.decompress(data, 'gzip')
+        self.assertEquals(COMPRESS_DATA, orig)
+
     def testBintoolDocs(self):
         """Test for creation of bintool documentation"""
         with test_util.capture_sys_output() as (stdout, stderr):
-- 
2.30.2

Re: [PATCH 2/3] binman: Remove header from compressed data

[Simon Glass]

Hi Stefan,

On Tue, 2 Aug 2022 at 06:29, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> Remove header from compressed data because this is uncommon, not
> supported by U-Boot and incompatible with external compressed artifacts.
>
> The header was introduced as part of commit eb0f4a4cb402 ("binman:
> Support replacing data in a cbfs") to allow device tree entries to be
> larger that the compressed contents. Regarding the commit "this is
> necessary to cope with a compressed device tree being updated in such a
> way that it shrinks after the entry size is already set (an obscure
> case)". This case need to be fixed without influence the compressed data
> by itself.

I was not able to find a way around this due to the chicken-and egg
problem. Compressed data has an unpredictable size and adding an extra
uncompressed byte may increase or decrease the compressed size.

So my solution was to add the header.

It is optional though, so can we perhaps have a property in the
description to enable it?

Regards,
Simon


>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> ---
>
>  tools/binman/cbfs_util.py |  8 ++++----
>  tools/binman/comp_util.py | 11 ++---------
>  tools/binman/entry.py     |  2 +-
>  3 files changed, 7 insertions(+), 14 deletions(-)
>
> diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py
> index 9cad03886f..a1836f4ad3 100644
> --- a/tools/binman/cbfs_util.py
> +++ b/tools/binman/cbfs_util.py
> @@ -241,9 +241,9 @@ class CbfsFile(object):
>          """Handle decompressing data if necessary"""
>          indata = self.data
>          if self.compress == COMPRESS_LZ4:
> -            data = comp_util.decompress(indata, 'lz4', with_header=False)
> +            data = comp_util.decompress(indata, 'lz4')
>          elif self.compress == COMPRESS_LZMA:
> -            data = comp_util.decompress(indata, 'lzma', with_header=False)
> +            data = comp_util.decompress(indata, 'lzma')
>          else:
>              data = indata
>          self.memlen = len(data)
> @@ -362,9 +362,9 @@ class CbfsFile(object):
>          elif self.ftype == TYPE_RAW:
>              orig_data = data
>              if self.compress == COMPRESS_LZ4:
> -                data = comp_util.compress(orig_data, 'lz4', with_header=False)
> +                data = comp_util.compress(orig_data, 'lz4')
>              elif self.compress == COMPRESS_LZMA:
> -                data = comp_util.compress(orig_data, 'lzma', with_header=False)
> +                data = comp_util.compress(orig_data, 'lzma')
>              self.memlen = len(orig_data)
>              self.data_len = len(data)
>              attr = struct.pack(ATTR_COMPRESSION_FORMAT,
> diff --git a/tools/binman/comp_util.py b/tools/binman/comp_util.py
> index dc76adab35..269bbf7975 100644
> --- a/tools/binman/comp_util.py
> +++ b/tools/binman/comp_util.py
> @@ -3,7 +3,6 @@
>  #
>  """Utilities to compress and decompress data"""
>
> -import struct
>  import tempfile
>
>  from binman import bintool
> @@ -16,7 +15,7 @@ LZMA_ALONE = bintool.Bintool.create('lzma_alone')
>  HAVE_LZMA_ALONE = LZMA_ALONE.is_present()
>
>
> -def compress(indata, algo, with_header=True):
> +def compress(indata, algo):
>      """Compress some data using a given algorithm
>
>      Note that for lzma this uses an old version of the algorithm, not that
> @@ -41,12 +40,9 @@ def compress(indata, algo, with_header=True):
>          data = LZMA_ALONE.compress(indata)
>      else:
>          raise ValueError("Unknown algorithm '%s'" % algo)
> -    if with_header:
> -        hdr = struct.pack('<I', len(data))
> -        data = hdr + data
>      return data
>
> -def decompress(indata, algo, with_header=True):
> +def decompress(indata, algo):
>      """Decompress some data using a given algorithm
>
>      Note that for lzma this uses an old version of the algorithm, not that
> @@ -64,9 +60,6 @@ def decompress(indata, algo, with_header=True):
>      """
>      if algo == 'none':
>          return indata
> -    if with_header:
> -        data_len = struct.unpack('<I', indata[:4])[0]
> -        indata = indata[4:4 + data_len]
>      if algo == 'lz4':
>          data = LZ4.decompress(indata)
>      elif algo == 'lzma':
> diff --git a/tools/binman/entry.py b/tools/binman/entry.py
> index a07a588864..8cbfd43af9 100644
> --- a/tools/binman/entry.py
> +++ b/tools/binman/entry.py
> @@ -1069,7 +1069,7 @@ features to produce new behaviours.
>              indata: Data to compress
>
>          Returns:
> -            Compressed data (first word is the compressed size)
> +            Compressed data
>          """
>          self.uncomp_data = indata
>          if self.compress != 'none':
> --
> 2.30.2
>

Re: [PATCH 3/3] binman: Add gzip bintool

[Simon Glass]

On Tue, 2 Aug 2022 at 06:29, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>
> Add gzip bintool to binman to support on-the-fly compression of Linux
> kernel images and FPGA bitstreams. The SPL basic fitImage implementation
> supports only gzip decompression.
>
> Signed-off-by: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> ---
>
>  tools/binman/btool/gzip.py | 94 ++++++++++++++++++++++++++++++++++++++
>  tools/binman/comp_util.py  | 17 +++++--
>  tools/binman/ftest.py      | 18 ++++++++
>  3 files changed, 124 insertions(+), 5 deletions(-)
>  create mode 100644 tools/binman/btool/gzip.py

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

Re: [PATCH 2/3] binman: Remove header from compressed data

[Stefan Herbrechtsmeier]

Hi Simon,

Am 02.08.2022 um 14:41 schrieb Simon Glass:
> Hi Stefan,
> 
> On Tue, 2 Aug 2022 at 06:29, Stefan Herbrechtsmeier
> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>>
>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>
>> Remove header from compressed data because this is uncommon, not
>> supported by U-Boot and incompatible with external compressed artifacts.
>>
>> The header was introduced as part of commit eb0f4a4cb402 ("binman:
>> Support replacing data in a cbfs") to allow device tree entries to be
>> larger that the compressed contents. Regarding the commit "this is
>> necessary to cope with a compressed device tree being updated in such a
>> way that it shrinks after the entry size is already set (an obscure
>> case)". This case need to be fixed without influence the compressed data
>> by itself.
> 
> I was not able to find a way around this due to the chicken-and egg
> problem. Compressed data has an unpredictable size and adding an extra
> uncompressed byte may increase or decrease the compressed size.

Is it possible to use the `pad-after` attribute to record the unused 
space. In this case it is possible to calculate the size of the 
compressed data.

Do you have a test for this use case?

> So my solution was to add the header.

Is the header used outside of binman? I don't spot it in the decompress 
fitImage implementation.


> It is optional though, so can we perhaps have a property in the
> description to enable it?

Is this header needed and supported outside of binman?

At the moment the header is incompatible and not well documented. It 
took me some time to find out why my gzip compression via binman doesn't 
work as expected because I assume a compatibility between binman 
compress and fitImage decompress.

Regards
   Stefan

Re: [PATCH 2/3] binman: Remove header from compressed data

[Simon Glass]

Hi Stefan,

On Tue, 2 Aug 2022 at 07:45, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> Hi Simon,
>
> Am 02.08.2022 um 14:41 schrieb Simon Glass:
> > Hi Stefan,
> >
> > On Tue, 2 Aug 2022 at 06:29, Stefan Herbrechtsmeier
> > <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
> >>
> >> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> >>
> >> Remove header from compressed data because this is uncommon, not
> >> supported by U-Boot and incompatible with external compressed artifacts.
> >>
> >> The header was introduced as part of commit eb0f4a4cb402 ("binman:
> >> Support replacing data in a cbfs") to allow device tree entries to be
> >> larger that the compressed contents. Regarding the commit "this is
> >> necessary to cope with a compressed device tree being updated in such a
> >> way that it shrinks after the entry size is already set (an obscure
> >> case)". This case need to be fixed without influence the compressed data
> >> by itself.
> >
> > I was not able to find a way around this due to the chicken-and egg
> > problem. Compressed data has an unpredictable size and adding an extra
> > uncompressed byte may increase or decrease the compressed size.
>
> Is it possible to use the `pad-after` attribute to record the unused
> space. In this case it is possible to calculate the size of the
> compressed data.

Well if you update that attribute it can change the size of the DTB
which is the chicken-and-egg problem again. As far as I know, if
people set the size of the region to something a bit larger than
needed, the problem is avoided. But the image generation does need to
be deterministic.

>
> Do you have a test for this use case?

There are various ones, e.g. testCompressDtb()

>
> > So my solution was to add the header.
>
> Is the header used outside of binman? I don't spot it in the decompress
> fitImage implementation.

It is used in the Chromium OS verified boot implementation, but not elsewhere.

>
>
> > It is optional though, so can we perhaps have a property in the
> > description to enable it?
>
> Is this header needed and supported outside of binman?
>
> At the moment the header is incompatible and not well documented. It
> took me some time to find out why my gzip compression via binman doesn't
> work as expected because I assume a compatibility between binman
> compress and fitImage decompress.

Yes I understand that, however you can pass a parameter to not include
the size value.

It would also be possible to add a property to the image (top-level
section) that indicates whether this field is present, such property
to apply to the whole image. We could have it default to off, if you
like.

Regards,
Simon

Re: [PATCH 2/3] binman: Remove header from compressed data

[Stefan Herbrechtsmeier]

Hi Simon,

Am 03.08.2022 um 20:14 schrieb Simon Glass:
> Hi Stefan,
> 
> On Tue, 2 Aug 2022 at 07:45, Stefan Herbrechtsmeier
> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>>
>> Hi Simon,
>>
>> Am 02.08.2022 um 14:41 schrieb Simon Glass:
>>> Hi Stefan,
>>>
>>> On Tue, 2 Aug 2022 at 06:29, Stefan Herbrechtsmeier
>>> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>>>>
>>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>>>
>>>> Remove header from compressed data because this is uncommon, not
>>>> supported by U-Boot and incompatible with external compressed artifacts.
>>>>
>>>> The header was introduced as part of commit eb0f4a4cb402 ("binman:
>>>> Support replacing data in a cbfs") to allow device tree entries to be
>>>> larger that the compressed contents. Regarding the commit "this is
>>>> necessary to cope with a compressed device tree being updated in such a
>>>> way that it shrinks after the entry size is already set (an obscure
>>>> case)". This case need to be fixed without influence the compressed data
>>>> by itself.
>>>
>>> I was not able to find a way around this due to the chicken-and egg
>>> problem. Compressed data has an unpredictable size and adding an extra
>>> uncompressed byte may increase or decrease the compressed size.
>>
>> Is it possible to use the `pad-after` attribute to record the unused
>> space. In this case it is possible to calculate the size of the
>> compressed data.
> 
> Well if you update that attribute it can change the size of the DTB
> which is the chicken-and-egg problem again. As far as I know, if
> people set the size of the region to something a bit larger than
> needed, the problem is avoided. But the image generation does need to
> be deterministic.

Does this means the size is only needed for the creation of the fitImage 
and not for decompression in u-boot?

> 
>>
>> Do you have a test for this use case?
> 
> There are various ones, e.g. testCompressDtb()

Thanks. Now I understand the problem and why you call it a 
chicken-and-egg problem. It wasn't clear to me that the attributes are 
inside the DTB.

But I wonder how the decompression of the DTB works if the fitImage 
implementation doesn't support the header.

>>> So my solution was to add the header.
>>
>> Is the header used outside of binman? I don't spot it in the decompress
>> fitImage implementation.
> 
> It is used in the Chromium OS verified boot implementation, but not elsewhere.
> 
>>
>>
>>> It is optional though, so can we perhaps have a property in the
>>> description to enable it?
>>
>> Is this header needed and supported outside of binman?
>>
>> At the moment the header is incompatible and not well documented. It
>> took me some time to find out why my gzip compression via binman doesn't
>> work as expected because I assume a compatibility between binman
>> compress and fitImage decompress.
> 
> Yes I understand that, however you can pass a parameter to not include
> the size value.

Do we need the header outside of the DTB? Otherwise we could handle this 
special or we could add a special compression type.

> It would also be possible to add a property to the image (top-level
> section) that indicates whether this field is present, such property
> to apply to the whole image. We could have it default to off, if you
> like.

Do we really need the header outside of the DTB entry?

Regards
   Stefan

Re: [PATCH 2/3] binman: Remove header from compressed data

[Simon Glass]

Hi Stefan,

On Thu, 4 Aug 2022 at 01:50, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> Hi Simon,
>
> Am 03.08.2022 um 20:14 schrieb Simon Glass:
> > Hi Stefan,
> >
> > On Tue, 2 Aug 2022 at 07:45, Stefan Herbrechtsmeier
> > <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
> >>
> >> Hi Simon,
> >>
> >> Am 02.08.2022 um 14:41 schrieb Simon Glass:
> >>> Hi Stefan,
> >>>
> >>> On Tue, 2 Aug 2022 at 06:29, Stefan Herbrechtsmeier
> >>> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
> >>>>
> >>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> >>>>
> >>>> Remove header from compressed data because this is uncommon, not
> >>>> supported by U-Boot and incompatible with external compressed artifacts.
> >>>>
> >>>> The header was introduced as part of commit eb0f4a4cb402 ("binman:
> >>>> Support replacing data in a cbfs") to allow device tree entries to be
> >>>> larger that the compressed contents. Regarding the commit "this is
> >>>> necessary to cope with a compressed device tree being updated in such a
> >>>> way that it shrinks after the entry size is already set (an obscure
> >>>> case)". This case need to be fixed without influence the compressed data
> >>>> by itself.
> >>>
> >>> I was not able to find a way around this due to the chicken-and egg
> >>> problem. Compressed data has an unpredictable size and adding an extra
> >>> uncompressed byte may increase or decrease the compressed size.
> >>
> >> Is it possible to use the `pad-after` attribute to record the unused
> >> space. In this case it is possible to calculate the size of the
> >> compressed data.
> >
> > Well if you update that attribute it can change the size of the DTB
> > which is the chicken-and-egg problem again. As far as I know, if
> > people set the size of the region to something a bit larger than
> > needed, the problem is avoided. But the image generation does need to
> > be deterministic.
>
> Does this means the size is only needed for the creation of the fitImage
> and not for decompression in u-boot?

Possibly, but of course we cannot do that. As you say, U-Boot mainline
does not expect or support the header, at present.

>
> >
> >>
> >> Do you have a test for this use case?
> >
> > There are various ones, e.g. testCompressDtb()
>
> Thanks. Now I understand the problem and why you call it a
> chicken-and-egg problem. It wasn't clear to me that the attributes are
> inside the DTB.

OK good.

>
> But I wonder how the decompression of the DTB works if the fitImage
> implementation doesn't support the header.

It doesn't. Something needs to change here for compression to work.

>
> >>> So my solution was to add the header.
> >>
> >> Is the header used outside of binman? I don't spot it in the decompress
> >> fitImage implementation.
> >
> > It is used in the Chromium OS verified boot implementation, but not elsewhere.
> >
> >>
> >>
> >>> It is optional though, so can we perhaps have a property in the
> >>> description to enable it?
> >>
> >> Is this header needed and supported outside of binman?
> >>
> >> At the moment the header is incompatible and not well documented. It
> >> took me some time to find out why my gzip compression via binman doesn't
> >> work as expected because I assume a compatibility between binman
> >> compress and fitImage decompress.
> >
> > Yes I understand that, however you can pass a parameter to not include
> > the size value.
>
> Do we need the header outside of the DTB? Otherwise we could handle this
> special or we could add a special compression type.
>
> > It would also be possible to add a property to the image (top-level
> > section) that indicates whether this field is present, such property
> > to apply to the whole image. We could have it default to off, if you
> > like.
>
> Do we really need the header outside of the DTB entry?

That's an interesting question. It is possible that we only need it if
DTB is present and is compressed. It should be possible to check this
by adjusting the tests and checking for failures.

But I am not sure it is a good idea, since it is wildly inconsistent.
I do prefer things to be deterministic - i.e.  you specify what you
want and you get it. If binman starts adopting obscure conventions it
could be confusing.

Regards,
Simon

Re: [PATCH 2/3] binman: Remove header from compressed data

[Stefan Herbrechtsmeier]

Hi Simon,

Am 04.08.2022 um 15:57 schrieb Simon Glass:
> Hi Stefan,
> 
> On Thu, 4 Aug 2022 at 01:50, Stefan Herbrechtsmeier
> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>>
>> Hi Simon,
>>
>> Am 03.08.2022 um 20:14 schrieb Simon Glass:
>>> Hi Stefan,
>>>
>>> On Tue, 2 Aug 2022 at 07:45, Stefan Herbrechtsmeier
>>> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>>>>
>>>> Hi Simon,
>>>>
>>>> Am 02.08.2022 um 14:41 schrieb Simon Glass:
>>>>> Hi Stefan,
>>>>>
>>>>> On Tue, 2 Aug 2022 at 06:29, Stefan Herbrechtsmeier
>>>>> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>>>>>>
>>>>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>>>>>
>>>>>> Remove header from compressed data because this is uncommon, not
>>>>>> supported by U-Boot and incompatible with external compressed artifacts.
>>>>>>
>>>>>> The header was introduced as part of commit eb0f4a4cb402 ("binman:
>>>>>> Support replacing data in a cbfs") to allow device tree entries to be
>>>>>> larger that the compressed contents. Regarding the commit "this is
>>>>>> necessary to cope with a compressed device tree being updated in such a
>>>>>> way that it shrinks after the entry size is already set (an obscure
>>>>>> case)". This case need to be fixed without influence the compressed data
>>>>>> by itself.
>>>>>
>>>>> I was not able to find a way around this due to the chicken-and egg
>>>>> problem. Compressed data has an unpredictable size and adding an extra
>>>>> uncompressed byte may increase or decrease the compressed size.
>>>>
>>>> Is it possible to use the `pad-after` attribute to record the unused
>>>> space. In this case it is possible to calculate the size of the
>>>> compressed data.
>>>
>>> Well if you update that attribute it can change the size of the DTB
>>> which is the chicken-and-egg problem again. As far as I know, if
>>> people set the size of the region to something a bit larger than
>>> needed, the problem is avoided. But the image generation does need to
>>> be deterministic.
>>
>> Does this means the size is only needed for the creation of the fitImage
>> and not for decompression in u-boot?
> 
> Possibly, but of course we cannot do that. As you say, U-Boot mainline
> does not expect or support the header, at present.
> 
>>
>>>
>>>>
>>>> Do you have a test for this use case?
>>>
>>> There are various ones, e.g. testCompressDtb()
>>
>> Thanks. Now I understand the problem and why you call it a
>> chicken-and-egg problem. It wasn't clear to me that the attributes are
>> inside the DTB.
> 
> OK good.
> 
>>
>> But I wonder how the decompression of the DTB works if the fitImage
>> implementation doesn't support the header.
> 
> It doesn't. Something needs to change here for compression to work.
> 
>>
>>>>> So my solution was to add the header.
>>>>
>>>> Is the header used outside of binman? I don't spot it in the decompress
>>>> fitImage implementation.
>>>
>>> It is used in the Chromium OS verified boot implementation, but not elsewhere.
>>>
>>>>
>>>>
>>>>> It is optional though, so can we perhaps have a property in the
>>>>> description to enable it?
>>>>
>>>> Is this header needed and supported outside of binman?
>>>>
>>>> At the moment the header is incompatible and not well documented. It
>>>> took me some time to find out why my gzip compression via binman doesn't
>>>> work as expected because I assume a compatibility between binman
>>>> compress and fitImage decompress.
>>>
>>> Yes I understand that, however you can pass a parameter to not include
>>> the size value.
>>
>> Do we need the header outside of the DTB? Otherwise we could handle this
>> special or we could add a special compression type.
>>
>>> It would also be possible to add a property to the image (top-level
>>> section) that indicates whether this field is present, such property
>>> to apply to the whole image. We could have it default to off, if you
>>> like.
>>
>> Do we really need the header outside of the DTB entry?
> 
> That's an interesting question. It is possible that we only need it if
> DTB is present and is compressed. It should be possible to check this
> by adjusting the tests and checking for failures.
> 
> But I am not sure it is a good idea, since it is wildly inconsistent.
> I do prefer things to be deterministic - i.e.  you specify what you
> want and you get it. If binman starts adopting obscure conventions it
> could be confusing.

I add tests for gzip, lz4 and lzma_alone and all support padding at the 
end and don't need the header. Even the testCompressDtb works without 
the header. Furthermore I add bzip2, lzop, xz and zstd support to 
bintool and only zstd doesn't support padding. Do we really need the 
header or could we add an error if DTB is used together with zstd?

Should I commit bzip2, lzop, xz and zstd support to bintool?

Regards
   Stefan

Re: [PATCH 2/3] binman: Remove header from compressed data

[Simon Glass]

Hi Stefan,

On Fri, 5 Aug 2022 at 01:51, Stefan Herbrechtsmeier
<stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>
> Hi Simon,
>
> Am 04.08.2022 um 15:57 schrieb Simon Glass:
> > Hi Stefan,
> >
> > On Thu, 4 Aug 2022 at 01:50, Stefan Herbrechtsmeier
> > <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
> >>
> >> Hi Simon,
> >>
> >> Am 03.08.2022 um 20:14 schrieb Simon Glass:
> >>> Hi Stefan,
> >>>
> >>> On Tue, 2 Aug 2022 at 07:45, Stefan Herbrechtsmeier
> >>> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
> >>>>
> >>>> Hi Simon,
> >>>>
> >>>> Am 02.08.2022 um 14:41 schrieb Simon Glass:
> >>>>> Hi Stefan,
> >>>>>
> >>>>> On Tue, 2 Aug 2022 at 06:29, Stefan Herbrechtsmeier
> >>>>> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
> >>>>>>
> >>>>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
> >>>>>>
> >>>>>> Remove header from compressed data because this is uncommon, not
> >>>>>> supported by U-Boot and incompatible with external compressed artifacts.
> >>>>>>
> >>>>>> The header was introduced as part of commit eb0f4a4cb402 ("binman:
> >>>>>> Support replacing data in a cbfs") to allow device tree entries to be
> >>>>>> larger that the compressed contents. Regarding the commit "this is
> >>>>>> necessary to cope with a compressed device tree being updated in such a
> >>>>>> way that it shrinks after the entry size is already set (an obscure
> >>>>>> case)". This case need to be fixed without influence the compressed data
> >>>>>> by itself.
> >>>>>
> >>>>> I was not able to find a way around this due to the chicken-and egg
> >>>>> problem. Compressed data has an unpredictable size and adding an extra
> >>>>> uncompressed byte may increase or decrease the compressed size.
> >>>>
> >>>> Is it possible to use the `pad-after` attribute to record the unused
> >>>> space. In this case it is possible to calculate the size of the
> >>>> compressed data.
> >>>
> >>> Well if you update that attribute it can change the size of the DTB
> >>> which is the chicken-and-egg problem again. As far as I know, if
> >>> people set the size of the region to something a bit larger than
> >>> needed, the problem is avoided. But the image generation does need to
> >>> be deterministic.
> >>
> >> Does this means the size is only needed for the creation of the fitImage
> >> and not for decompression in u-boot?
> >
> > Possibly, but of course we cannot do that. As you say, U-Boot mainline
> > does not expect or support the header, at present.
> >
> >>
> >>>
> >>>>
> >>>> Do you have a test for this use case?
> >>>
> >>> There are various ones, e.g. testCompressDtb()
> >>
> >> Thanks. Now I understand the problem and why you call it a
> >> chicken-and-egg problem. It wasn't clear to me that the attributes are
> >> inside the DTB.
> >
> > OK good.
> >
> >>
> >> But I wonder how the decompression of the DTB works if the fitImage
> >> implementation doesn't support the header.
> >
> > It doesn't. Something needs to change here for compression to work.
> >
> >>
> >>>>> So my solution was to add the header.
> >>>>
> >>>> Is the header used outside of binman? I don't spot it in the decompress
> >>>> fitImage implementation.
> >>>
> >>> It is used in the Chromium OS verified boot implementation, but not elsewhere.
> >>>
> >>>>
> >>>>
> >>>>> It is optional though, so can we perhaps have a property in the
> >>>>> description to enable it?
> >>>>
> >>>> Is this header needed and supported outside of binman?
> >>>>
> >>>> At the moment the header is incompatible and not well documented. It
> >>>> took me some time to find out why my gzip compression via binman doesn't
> >>>> work as expected because I assume a compatibility between binman
> >>>> compress and fitImage decompress.
> >>>
> >>> Yes I understand that, however you can pass a parameter to not include
> >>> the size value.
> >>
> >> Do we need the header outside of the DTB? Otherwise we could handle this
> >> special or we could add a special compression type.
> >>
> >>> It would also be possible to add a property to the image (top-level
> >>> section) that indicates whether this field is present, such property
> >>> to apply to the whole image. We could have it default to off, if you
> >>> like.
> >>
> >> Do we really need the header outside of the DTB entry?
> >
> > That's an interesting question. It is possible that we only need it if
> > DTB is present and is compressed. It should be possible to check this
> > by adjusting the tests and checking for failures.
> >
> > But I am not sure it is a good idea, since it is wildly inconsistent.
> > I do prefer things to be deterministic - i.e.  you specify what you
> > want and you get it. If binman starts adopting obscure conventions it
> > could be confusing.
>
> I add tests for gzip, lz4 and lzma_alone and all support padding at the
> end and don't need the header. Even the testCompressDtb works without
> the header. Furthermore I add bzip2, lzop, xz and zstd support to
> bintool and only zstd doesn't support padding. Do we really need the
> header or could we add an error if DTB is used together with zstd?

OK great!

Yes, I tried pretty hard to avoid it, but could not make it work.
Would you like me to take a look at the situations that spark it? It
might be around replacing things, too.

I really tried to avoid banning things, since it is such a pain and
confusing for people. But since this is error (and a corner case) I
think it would be fine to require a particular property to enable the
advanced functionality.

>
> Should I commit bzip2, lzop, xz and zstd support to bintool?

Yes please.

Regards,
Simon

Re: [PATCH 2/3] binman: Remove header from compressed data

[Stefan Herbrechtsmeier]

Hi Simon,

Am 05.08.2022 um 18:48 schrieb Simon Glass:
> Hi Stefan,
> 
> On Fri, 5 Aug 2022 at 01:51, Stefan Herbrechtsmeier
> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>>
>> Hi Simon,
>>
>> Am 04.08.2022 um 15:57 schrieb Simon Glass:
>>> Hi Stefan,
>>>
>>> On Thu, 4 Aug 2022 at 01:50, Stefan Herbrechtsmeier
>>> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>>>>
>>>> Hi Simon,
>>>>
>>>> Am 03.08.2022 um 20:14 schrieb Simon Glass:
>>>>> Hi Stefan,
>>>>>
>>>>> On Tue, 2 Aug 2022 at 07:45, Stefan Herbrechtsmeier
>>>>> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>>>>>>
>>>>>> Hi Simon,
>>>>>>
>>>>>> Am 02.08.2022 um 14:41 schrieb Simon Glass:
>>>>>>> Hi Stefan,
>>>>>>>
>>>>>>> On Tue, 2 Aug 2022 at 06:29, Stefan Herbrechtsmeier
>>>>>>> <stefan.herbrechtsmeier-oss@weidmueller.com> wrote:
>>>>>>>>
>>>>>>>> From: Stefan Herbrechtsmeier <stefan.herbrechtsmeier@weidmueller.com>
>>>>>>>>
>>>>>>>> Remove header from compressed data because this is uncommon, not
>>>>>>>> supported by U-Boot and incompatible with external compressed artifacts.
>>>>>>>>
>>>>>>>> The header was introduced as part of commit eb0f4a4cb402 ("binman:
>>>>>>>> Support replacing data in a cbfs") to allow device tree entries to be
>>>>>>>> larger that the compressed contents. Regarding the commit "this is
>>>>>>>> necessary to cope with a compressed device tree being updated in such a
>>>>>>>> way that it shrinks after the entry size is already set (an obscure
>>>>>>>> case)". This case need to be fixed without influence the compressed data
>>>>>>>> by itself.
>>>>>>>
>>>>>>> I was not able to find a way around this due to the chicken-and egg
>>>>>>> problem. Compressed data has an unpredictable size and adding an extra
>>>>>>> uncompressed byte may increase or decrease the compressed size.
>>>>>>
>>>>>> Is it possible to use the `pad-after` attribute to record the unused
>>>>>> space. In this case it is possible to calculate the size of the
>>>>>> compressed data.
>>>>>
>>>>> Well if you update that attribute it can change the size of the DTB
>>>>> which is the chicken-and-egg problem again. As far as I know, if
>>>>> people set the size of the region to something a bit larger than
>>>>> needed, the problem is avoided. But the image generation does need to
>>>>> be deterministic.
>>>>
>>>> Does this means the size is only needed for the creation of the fitImage
>>>> and not for decompression in u-boot?
>>>
>>> Possibly, but of course we cannot do that. As you say, U-Boot mainline
>>> does not expect or support the header, at present.
>>>
>>>>
>>>>>
>>>>>>
>>>>>> Do you have a test for this use case?
>>>>>
>>>>> There are various ones, e.g. testCompressDtb()
>>>>
>>>> Thanks. Now I understand the problem and why you call it a
>>>> chicken-and-egg problem. It wasn't clear to me that the attributes are
>>>> inside the DTB.
>>>
>>> OK good.
>>>
>>>>
>>>> But I wonder how the decompression of the DTB works if the fitImage
>>>> implementation doesn't support the header.
>>>
>>> It doesn't. Something needs to change here for compression to work.
>>>
>>>>
>>>>>>> So my solution was to add the header.
>>>>>>
>>>>>> Is the header used outside of binman? I don't spot it in the decompress
>>>>>> fitImage implementation.
>>>>>
>>>>> It is used in the Chromium OS verified boot implementation, but not elsewhere.
>>>>>
>>>>>>
>>>>>>
>>>>>>> It is optional though, so can we perhaps have a property in the
>>>>>>> description to enable it?
>>>>>>
>>>>>> Is this header needed and supported outside of binman?
>>>>>>
>>>>>> At the moment the header is incompatible and not well documented. It
>>>>>> took me some time to find out why my gzip compression via binman doesn't
>>>>>> work as expected because I assume a compatibility between binman
>>>>>> compress and fitImage decompress.
>>>>>
>>>>> Yes I understand that, however you can pass a parameter to not include
>>>>> the size value.
>>>>
>>>> Do we need the header outside of the DTB? Otherwise we could handle this
>>>> special or we could add a special compression type.
>>>>
>>>>> It would also be possible to add a property to the image (top-level
>>>>> section) that indicates whether this field is present, such property
>>>>> to apply to the whole image. We could have it default to off, if you
>>>>> like.
>>>>
>>>> Do we really need the header outside of the DTB entry?
>>>
>>> That's an interesting question. It is possible that we only need it if
>>> DTB is present and is compressed. It should be possible to check this
>>> by adjusting the tests and checking for failures.
>>>
>>> But I am not sure it is a good idea, since it is wildly inconsistent.
>>> I do prefer things to be deterministic - i.e.  you specify what you
>>> want and you get it. If binman starts adopting obscure conventions it
>>> could be confusing.
>>
>> I add tests for gzip, lz4 and lzma_alone and all support padding at the
>> end and don't need the header. Even the testCompressDtb works without
>> the header. Furthermore I add bzip2, lzop, xz and zstd support to
>> bintool and only zstd doesn't support padding. Do we really need the
>> header or could we add an error if DTB is used together with zstd?
> 
> OK great!
> 
> Yes, I tried pretty hard to avoid it, but could not make it work.
> Would you like me to take a look at the situations that spark it? It
> might be around replacing things, too.
> 
> I really tried to avoid banning things, since it is such a pain and
> confusing for people. But since this is error (and a corner case) I
> think it would be fine to require a particular property to enable the
> advanced functionality.

I have send a new version. Would be nice if you could check if you need 
the new attribute to append the length header.

>> Should I commit bzip2, lzop, xz and zstd support to bintool?

I have add the tools to the series. Maybe the test environment need an 
update to include all compression tools.

Regards
   Stefan