From bd13255a91af04ab431972ab145e665147802c30 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 14 Oct 2023 14:40:26 -0600 Subject: binman: Don't add compression attribute for uncompressed files cbfsutil changed to skip adding a compression attribute if there is no compression. Adjust the binman implementation to do the same. This mirrors commit 105cdf5625 in coreboot. Signed-off-by: Simon Glass Reviewed-by: Neha Malcom Francis --- tools/binman/cbfs_util.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'tools/binman/cbfs_util.py') diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index fc56b40b753..92d2add2514 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -333,7 +333,8 @@ class CbfsFile(object): if self.ftype == TYPE_STAGE: pass elif self.ftype == TYPE_RAW: - hdr_len += ATTR_COMPRESSION_LEN + if self.compress: + hdr_len += ATTR_COMPRESSION_LEN elif self.ftype == TYPE_EMPTY: pass else: @@ -369,9 +370,11 @@ class CbfsFile(object): data = self.comp_bintool.compress(orig_data) self.memlen = len(orig_data) self.data_len = len(data) - attr = struct.pack(ATTR_COMPRESSION_FORMAT, - FILE_ATTR_TAG_COMPRESSION, ATTR_COMPRESSION_LEN, - self.compress, self.memlen) + if self.compress: + attr = struct.pack(ATTR_COMPRESSION_FORMAT, + FILE_ATTR_TAG_COMPRESSION, + ATTR_COMPRESSION_LEN, self.compress, + self.memlen) elif self.ftype == TYPE_EMPTY: data = tools.get_bytes(self.erase_byte, self.size) else: @@ -405,7 +408,7 @@ class CbfsFile(object): if expected_len != actual_len: # pragma: no cover # Test coverage of this is not available since this should never # happen. It probably indicates that get_header_len() is broken. - raise ValueError("Internal error: CBFS file '%s': Expected headers of %#x bytes, got %#d" % + raise ValueError("Internal error: CBFS file '%s': Expected headers of %#x bytes, got %#x" % (self.name, expected_len, actual_len)) return hdr + name + attr + pad + content + data, hdr_len -- cgit v1.2.3 From e9199a74e03290826a985c608d60d921a0deffc0 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 14 Oct 2023 14:40:27 -0600 Subject: binman: Ensure attributes always come last in the metadata cbfsutil changed to write zero bytes instead of 0xff when a small padding must be added. Adjust the binman implementation to do the same. Drop the code which looks for an unused attribute tag, since it is not used. A future patch moves the attributes to the end of the header in any case, so no data will follow the attributes. This mirrors commit f0cc7adb2f in coreboot. Signed-off-by: Simon Glass --- tools/binman/cbfs_util.py | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) (limited to 'tools/binman/cbfs_util.py') diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 92d2add2514..9ac38d8e4c1 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -54,10 +54,6 @@ ATTR_COMPRESSION_FORMAT = '>IIII' ATTR_COMPRESSION_LEN = 0x10 # Attribute tags -# Depending on how the header was initialised, it may be backed with 0x00 or -# 0xff. Support both. -FILE_ATTR_TAG_UNUSED = 0 -FILE_ATTR_TAG_UNUSED2 = 0xffffffff FILE_ATTR_TAG_COMPRESSION = 0x42435a4c FILE_ATTR_TAG_HASH = 0x68736148 FILE_ATTR_TAG_POSITION = 0x42435350 # PSCB @@ -394,6 +390,8 @@ class CbfsFile(object): raise ValueError("Internal error: CBFS file '%s': Requested offset %#x but current output position is %#x" % (self.name, self.cbfs_offset, offset)) pad = tools.get_bytes(pad_byte, pad_len) + if attr_pos: + attr_pos += pad_len hdr_len += pad_len # This is the offset of the start of the file's data, @@ -410,7 +408,7 @@ class CbfsFile(object): # happen. It probably indicates that get_header_len() is broken. raise ValueError("Internal error: CBFS file '%s': Expected headers of %#x bytes, got %#x" % (self.name, expected_len, actual_len)) - return hdr + name + attr + pad + content + data, hdr_len + return hdr + name + pad + attr + content + data, hdr_len class CbfsWriter(object): @@ -456,6 +454,9 @@ class CbfsWriter(object): self._arch = arch self._bootblock_size = 0 self._erase_byte = 0xff + + # Small padding to align a file uses 0 + self._small_pad_byte = 0 self._align = ENTRY_ALIGN self._add_fileheader = False if self._arch == ARCHITECTURE_X86: @@ -477,7 +478,7 @@ class CbfsWriter(object): self._bootblock_size, self._align) self._hdr_at_start = True - def _skip_to(self, fd, offset): + def _skip_to(self, fd, offset, pad_byte): """Write out pad bytes until a given offset Args: @@ -487,16 +488,16 @@ class CbfsWriter(object): if fd.tell() > offset: raise ValueError('No space for data before offset %#x (current offset %#x)' % (offset, fd.tell())) - fd.write(tools.get_bytes(self._erase_byte, offset - fd.tell())) + fd.write(tools.get_bytes(pad_byte, offset - fd.tell())) - def _pad_to(self, fd, offset): + def _pad_to(self, fd, offset, pad_byte): """Write out pad bytes and/or an empty file until a given offset Args: fd: File objext to write to offset: Offset to write to """ - self._align_to(fd, self._align) + self._align_to(fd, self._align, pad_byte) upto = fd.tell() if upto > offset: raise ValueError('No space for data before pad offset %#x (current offset %#x)' % @@ -505,9 +506,9 @@ class CbfsWriter(object): if todo: cbf = CbfsFile.empty(todo, self._erase_byte) fd.write(cbf.get_data_and_offset()[0]) - self._skip_to(fd, offset) + self._skip_to(fd, offset, pad_byte) - def _align_to(self, fd, align): + def _align_to(self, fd, align, pad_byte): """Write out pad bytes until a given alignment is reached This only aligns if the resulting output would not reach the end of the @@ -521,7 +522,7 @@ class CbfsWriter(object): """ offset = align_int(fd.tell(), align) if offset < self._size: - self._skip_to(fd, offset) + self._skip_to(fd, offset, pad_byte) def add_file_stage(self, name, data, cbfs_offset=None): """Add a new stage file to the CBFS @@ -571,7 +572,7 @@ class CbfsWriter(object): raise ValueError('No space for header at offset %#x (current offset %#x)' % (self._header_offset, fd.tell())) if not add_fileheader: - self._pad_to(fd, self._header_offset) + self._pad_to(fd, self._header_offset, self._erase_byte) hdr = struct.pack(HEADER_FORMAT, HEADER_MAGIC, HEADER_VERSION2, self._size, self._bootblock_size, self._align, self._contents_offset, self._arch, 0xffffffff) @@ -583,7 +584,7 @@ class CbfsWriter(object): fd.write(name) self._header_offset = fd.tell() fd.write(hdr) - self._align_to(fd, self._align) + self._align_to(fd, self._align, self._erase_byte) else: fd.write(hdr) @@ -600,24 +601,26 @@ class CbfsWriter(object): # THe header can go at the start in some cases if self._hdr_at_start: self._write_header(fd, add_fileheader=self._add_fileheader) - self._skip_to(fd, self._contents_offset) + self._skip_to(fd, self._contents_offset, self._erase_byte) # Write out each file for cbf in self._files.values(): # Place the file at its requested place, if any offset = cbf.calc_start_offset() if offset is not None: - self._pad_to(fd, align_int_down(offset, self._align)) + self._pad_to(fd, align_int_down(offset, self._align), + self._erase_byte) pos = fd.tell() - data, data_offset = cbf.get_data_and_offset(pos, self._erase_byte) + data, data_offset = cbf.get_data_and_offset(pos, + self._small_pad_byte) fd.write(data) - self._align_to(fd, self._align) + self._align_to(fd, self._align, self._erase_byte) cbf.calced_cbfs_offset = pos + data_offset if not self._hdr_at_start: self._write_header(fd, add_fileheader=self._add_fileheader) # Pad to the end and write a pointer to the CBFS master header - self._pad_to(fd, self._base_address or self._size - 4) + self._pad_to(fd, self._base_address or self._size - 4, self._erase_byte) rel_offset = self._header_offset - self._size fd.write(struct.pack(' Date: Sat, 14 Oct 2023 14:40:28 -0600 Subject: binman: Replace FILENAME_ALIGN 16 with ATTRIBUTE_ALIGN 4 cbfsutil changed to 4-byte alignment for filenames instead of 16. Adjust the binman implementation to do the same. This mirrors commit 5779ca718c in coreboot. Signed-off-by: Simon Glass --- tools/binman/cbfs_util.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'tools/binman/cbfs_util.py') diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 9ac38d8e4c1..076768ae839 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -42,7 +42,7 @@ HEADER_VERSION2 = 0x31313132 FILE_HEADER_FORMAT = b'>8sIIII' FILE_HEADER_LEN = 0x18 FILE_MAGIC = b'LARCHIVE' -FILENAME_ALIGN = 16 # Filename lengths are aligned to this +ATTRIBUTE_ALIGN = 4 # All attribute sizes must be divisible by this # A stage header containing information about 'stage' files # Yes this is correct: this header is in litte-endian format @@ -186,7 +186,7 @@ def _pack_string(instr): String with required padding (at least one 0x00 byte) at the end """ val = tools.to_bytes(instr) - pad_len = align_int(len(val) + 1, FILENAME_ALIGN) + pad_len = align_int(len(val) + 1, ATTRIBUTE_ALIGN) return val + tools.get_bytes(0, pad_len - len(val)) @@ -300,7 +300,7 @@ class CbfsFile(object): CbfsFile object containing the file information """ cfile = CbfsFile('', TYPE_EMPTY, b'', None) - cfile.size = space_to_use - FILE_HEADER_LEN - FILENAME_ALIGN + cfile.size = space_to_use - FILE_HEADER_LEN - ATTRIBUTE_ALIGN cfile.erase_byte = erase_byte return cfile @@ -859,8 +859,8 @@ class CbfsReader(object): """ val = b'' while True: - data = fd.read(FILENAME_ALIGN) - if len(data) < FILENAME_ALIGN: + data = fd.read(ATTRIBUTE_ALIGN) + if len(data) < ATTRIBUTE_ALIGN: return None pos = data.find(b'\0') if pos == -1: -- cgit v1.2.3 From fe35c2f011006b6753df53946ec6a206213f3a34 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 14 Oct 2023 14:40:29 -0600 Subject: binman: Rename TYPE_STAGE to TYPE_LEGACY_STAGE In preparation for changing how stages are stored, rename the existing stage tag. Signed-off-by: Simon Glass --- tools/binman/cbfs_util.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'tools/binman/cbfs_util.py') diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 076768ae839..9ca32f7309f 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -96,7 +96,7 @@ ARCH_NAMES = { # File types. Only supported ones are included here TYPE_CBFSHEADER = 0x02 # Master header, HEADER_FORMAT -TYPE_STAGE = 0x10 # Stage, holding an executable, see STAGE_FORMAT +TYPE_LEGACY_STAGE = 0x10 # Stage, holding an executable TYPE_RAW = 0x50 # Raw file, possibly compressed TYPE_EMPTY = 0xffffffff # Empty data @@ -265,7 +265,7 @@ class CbfsFile(object): Returns: CbfsFile object containing the file information """ - cfile = CbfsFile(name, TYPE_STAGE, data, cbfs_offset) + cfile = CbfsFile(name, TYPE_LEGACY_STAGE, data, cbfs_offset) cfile.base_address = base_address return cfile @@ -326,7 +326,7 @@ class CbfsFile(object): """ name = _pack_string(self.name) hdr_len = len(name) + FILE_HEADER_LEN - if self.ftype == TYPE_STAGE: + if self.ftype == TYPE_LEGACY_STAGE: pass elif self.ftype == TYPE_RAW: if self.compress: @@ -354,7 +354,7 @@ class CbfsFile(object): attr = b'' pad = b'' data = self.data - if self.ftype == TYPE_STAGE: + if self.ftype == TYPE_LEGACY_STAGE: elf_data = elf.DecodeElf(data, self.base_address) content = struct.pack(STAGE_FORMAT, self.compress, elf_data.entry, elf_data.load, @@ -639,7 +639,7 @@ class CbfsReader(object): files: Ordered list of CbfsFile objects align: Alignment to use for files, typically ENTRT_ALIGN stage_base_address: Base address to use when mapping ELF files into the - CBFS for TYPE_STAGE files. If this is larger than the code address + CBFS for TYPE_LEGACY_STAGE files. If this is larger than the code address of the ELF file, then data at the start of the ELF file will not appear in the CBFS. Currently there are no tests for behaviour as documentation is sparse @@ -750,7 +750,7 @@ class CbfsReader(object): fd.seek(cbfs_offset, io.SEEK_SET) if ftype == TYPE_CBFSHEADER: self._read_header(fd) - elif ftype == TYPE_STAGE: + elif ftype == TYPE_LEGACY_STAGE: data = fd.read(STAGE_LEN) cfile = CbfsFile.stage(self.stage_base_address, name, b'', cbfs_offset) -- cgit v1.2.3 From ce0e9e3990e051b2d79f89771e6253fc095d1ca1 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Sat, 14 Oct 2023 14:40:30 -0600 Subject: binman: Move stage header into a CBFS attribute cbfsutil completely changed the way that stages are formatted in CBFS. Adjust the binman implementation to do the same. This mirrors commit 81dc20e744 in coreboot. Signed-off-by: Simon Glass --- tools/binman/cbfs_util.py | 64 +++++++++++++++++++++++++++++------------------ 1 file changed, 39 insertions(+), 25 deletions(-) (limited to 'tools/binman/cbfs_util.py') diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py index 9ca32f7309f..671cafa34c0 100644 --- a/tools/binman/cbfs_util.py +++ b/tools/binman/cbfs_util.py @@ -44,10 +44,10 @@ FILE_HEADER_LEN = 0x18 FILE_MAGIC = b'LARCHIVE' ATTRIBUTE_ALIGN = 4 # All attribute sizes must be divisible by this -# A stage header containing information about 'stage' files +# A stage-header attribute containing information about 'stage' files # Yes this is correct: this header is in litte-endian format -STAGE_FORMAT = '