From: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
Move the compressed data header handling into the dtb blob entry class and make it optional. The header is uncommon, not supported by U-Boot and incompatible with external compressed artifacts.
If needed the header could be enabled with the following attribute beside the compress attribute: prepend = "length";
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 any compressed data by itself.
Signed-off-by: Stefan Herbrechtsmeier stefan.herbrechtsmeier@weidmueller.com
---
Changes in v2: - Reworked to make the feature optional.
tools/binman/cbfs_util.py | 8 ++--- tools/binman/comp_util.py | 11 ++---- tools/binman/entries.rst | 4 +++ tools/binman/entry.py | 2 +- tools/binman/etype/blob_dtb.py | 21 ++++++++++++ tools/binman/ftest.py | 34 ++++++++++++++++--- .../test/235_compress_prepend_length_dtb.dts | 17 ++++++++++ 7 files changed, 78 insertions(+), 19 deletions(-) create mode 100644 tools/binman/test/235_compress_prepend_length_dtb.dts
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/entries.rst b/tools/binman/entries.rst index ae4305c99e..8e722426d3 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -208,6 +208,10 @@ This is a blob containing a device tree. The contents of the blob are obtained from the list of available device-tree files, managed by the 'state' module.
+Additional Properties / Entry arguments: + - prepend: Header type to use: + none: No header + length: 32-bit length header
Entry: blob-ext: Externally built binary blob 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': diff --git a/tools/binman/etype/blob_dtb.py b/tools/binman/etype/blob_dtb.py index 4159e3032a..652b8abd8f 100644 --- a/tools/binman/etype/blob_dtb.py +++ b/tools/binman/etype/blob_dtb.py @@ -7,6 +7,8 @@
from binman.entry import Entry from binman.etype.blob import Entry_blob +from dtoc import fdt_util +import struct
# This is imported if needed state = None @@ -17,6 +19,9 @@ class Entry_blob_dtb(Entry_blob): This is a blob containing a device tree. The contents of the blob are obtained from the list of available device-tree files, managed by the 'state' module. + + Additional attributes: + prepend: Header used (e.g. 'length'), 'none' if none """ def __init__(self, section, etype, node): # Put this here to allow entry-docs and help to work without libfdt @@ -25,6 +30,12 @@ class Entry_blob_dtb(Entry_blob):
super().__init__(section, etype, node)
+ self.prepend = 'none' + + def ReadNode(self): + super().ReadNode() + self.prepend = fdt_util.GetString(self._node, 'prepend', 'none') + def ObtainContents(self): """Get the device-tree from the list held by the 'state' module""" self._filename = self.GetDefaultFilename() @@ -35,6 +46,9 @@ class Entry_blob_dtb(Entry_blob): """Re-read the DTB contents so that we get any calculated properties""" _, indata = state.GetFdtContents(self.GetFdtEtype()) data = self.CompressData(indata) + if self.prepend == 'length': + hdr = struct.pack('<I', len(data)) + data = hdr + data return self.ProcessContentsUpdate(data)
def GetFdtEtype(self): @@ -50,6 +64,13 @@ class Entry_blob_dtb(Entry_blob): fname = self.GetDefaultFilename() return {self.GetFdtEtype(): [self, fname]}
+ def ReadData(self, decomp=True, alt_format=None): + data = super().ReadData(decomp, alt_format) + if self.prepend == 'length': + data_len = struct.unpack('<I', data[:4])[0] + data = data[4:4 + data_len] + return data + def WriteData(self, data, decomp=True): ok = super().WriteData(data, decomp)
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index da9aa9e679..057b4e28b7 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -2536,6 +2536,28 @@ class TestFunctional(unittest.TestCase): } self.assertEqual(expected, props)
+ def testCompressPrependLengthDtb(self): + """Test that compress of device-tree files with length header is + supported + """ + data = self.data = self._DoReadFileRealDtb('235_compress_prepend_length_dtb.dts') + self.assertEqual(U_BOOT_DATA, data[:len(U_BOOT_DATA)]) + dtb_data = data[len(U_BOOT_DATA):] + comp_data_len = struct.unpack('<I', dtb_data[:4])[0] + comp_data = dtb_data[4:4 + comp_data_len] + orig = self._decompress(comp_data) + dtb = fdt.Fdt.FromData(orig) + dtb.Scan() + props = self._GetPropTree(dtb, ['size', 'uncomp-size']) + expected = { + 'u-boot:size': len(U_BOOT_DATA), + 'u-boot-dtb:uncomp-size': len(orig), + 'u-boot-dtb:size': len(dtb_data), + 'size': len(data), + } + self.assertEqual(expected, props) + + def testCbfsUpdateFdt(self): """Test that we can update the device tree with CBFS offset/size info""" self._CheckLz4() @@ -2856,7 +2878,7 @@ 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) + dtb = comp_util.decompress(data, 'lzma') self.assertEqual(EXTRACT_DTB_SIZE, len(dtb))
def testExtractBadEntry(self): @@ -4427,15 +4449,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)]) + data1 = rest[:len(expect1)] + section1 = self._decompress(data1) + self.assertEquals(expect1, 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)]) + data2 = rest1[:len(expect2)] + section2 = self._decompress(data2) + self.assertEquals(expect2, data2) self.assertEquals(COMPRESS_DATA + COMPRESS_DATA, section2) rest2 = rest1[len(expect2):]
diff --git a/tools/binman/test/235_compress_prepend_length_dtb.dts b/tools/binman/test/235_compress_prepend_length_dtb.dts new file mode 100644 index 0000000000..a5abc60477 --- /dev/null +++ b/tools/binman/test/235_compress_prepend_length_dtb.dts @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot { + }; + u-boot-dtb { + compress = "lz4"; + prepend = "length"; + }; + }; +};