binman: Add support for fixed-offset files in CBFS

A feature of CBFS is that it allows files to be positioned at particular
offset (as with binman in general). This is useful to support
execute-in-place (XIP) code, since this may not be relocatable.

Add a new cbfs-offset property to control this.

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

6 files changed, 276 insertions, 34 deletions

diff --git a/tools/binman/README.entries b/tools/binman/README.entries
index 2e6aea1e84..3241befc7f 100644
--- a/tools/binman/README.entries
+++ b/tools/binman/README.entries
@@ -100,6 +100,7 @@ with the second subnode below:
             filename = "u-boot.dtb";
             cbfs-type = "raw";
             cbfs-compress = "lz4";
+            cbfs-offset = <0x100000>;
         };
     };
 
@@ -158,6 +159,15 @@ cbfs-type:
         to add a flat binary with a load/start address, similar to the
         'add-flat-binary' option in cbfstool.
 
+cbfs-offset:
+    This is the offset of the file's data within the CBFS. It is used to
+    specify where the file should be placed in cases where a fixed position
+    is needed. Typical uses are for code which is not relocatable and must
+    execute in-place from a particular address. This works because SPI flash
+    is generally mapped into memory on x86 devices. The file header is
+    placed before this offset so that the data start lines up exactly with
+    the chosen offset. If this property is not provided, then the file is
+    placed in the next available spot.
 
 The current implementation supports only a subset of CBFS features. It does
 not support other file types (e.g. payload), adding multiple files (like the
@@ -334,6 +344,34 @@ See README.x86 for information about x86 binary blobs.
 
 
 
+Entry: intel-ifwi: Entry containing an Intel Integrated Firmware Image (IFWI) file
+----------------------------------------------------------------------------------
+
+Properties / Entry arguments:
+    - filename: Filename of file to read into entry. This is either the
+        IFWI file itself, or a file that can be converted into one using a
+        tool
+    - convert-fit: If present this indicates that the ifwitool should be
+        used to convert the provided file into a IFWI.
+
+This file contains code and data used by the SoC that is required to make
+it work. It includes U-Boot TPL, microcode, things related to the CSE
+(Converged Security Engine, the microcontroller that loads all the firmware)
+and other items beyond the wit of man.
+
+A typical filename is 'ifwi.bin' for an IFWI file, or 'fitimage.bin' for a
+file that will be converted to an IFWI.
+
+The position of this entry is generally set by the intel-descriptor entry.
+
+The contents of the IFWI are specified by the subnodes of the IFWI node.
+Each subnode describes an entry which is placed into the IFWFI with a given
+sub-partition (and optional entry name).
+
+See README.x86 for information about x86 binary blobs.
+
+
+
 Entry: intel-me: Entry containing an Intel Management Engine (ME) file
 ----------------------------------------------------------------------
 
diff --git a/tools/binman/cbfs_util.py b/tools/binman/cbfs_util.py
index ec4a2e5a8c..1cdbcb2339 100644
--- a/tools/binman/cbfs_util.py
+++ b/tools/binman/cbfs_util.py
@@ -12,7 +12,7 @@ it is necessary to rely on the C structures and source code (mostly cbfstool)
 to fully understand it.
 
 Currently supported: raw and stage types with compression, padding empty areas
-    with empty files
+    with empty files, fixed-offset files
 """
 
 from __future__ import print_function
@@ -190,6 +190,8 @@ class CbfsFile(object):
     Properties:
         name: Name of file
         offset: Offset of file data from start of file header
+        cbfs_offset: Offset of file data in bytes from start of CBFS, or None to
+            place this file anyway
         data: Contents of file, uncompressed
         data_len: Length of (possibly compressed) data in bytes
         ftype: File type (TYPE_...)
@@ -203,9 +205,10 @@ class CbfsFile(object):
             contents (used for empty files)
         size: Size of the file in bytes (used for empty files)
     """
-    def __init__(self, name, ftype, data, compress=COMPRESS_NONE):
+    def __init__(self, name, ftype, data, cbfs_offset, compress=COMPRESS_NONE):
         self.name = name
         self.offset = None
+        self.cbfs_offset = cbfs_offset
         self.data = data
         self.ftype = ftype
         self.compress = compress
@@ -231,7 +234,7 @@ class CbfsFile(object):
         self.data_len = len(indata)
 
     @classmethod
-    def stage(cls, base_address, name, data):
+    def stage(cls, base_address, name, data, cbfs_offset):
         """Create a new stage file
 
         Args:
@@ -239,28 +242,32 @@ class CbfsFile(object):
             name: String file name to put in CBFS (does not need to correspond
                 to the name that the file originally came from)
             data: Contents of file
+            cbfs_offset: Offset of file data in bytes from start of CBFS, or
+                None to place this file anyway
 
         Returns:
             CbfsFile object containing the file information
         """
-        cfile = CbfsFile(name, TYPE_STAGE, data)
+        cfile = CbfsFile(name, TYPE_STAGE, data, cbfs_offset)
         cfile.base_address = base_address
         return cfile
 
     @classmethod
-    def raw(cls, name, data, compress):
+    def raw(cls, name, data, cbfs_offset, compress):
         """Create a new raw file
 
         Args:
             name: String file name to put in CBFS (does not need to correspond
                 to the name that the file originally came from)
             data: Contents of file
+            cbfs_offset: Offset of file data in bytes from start of CBFS, or
+                None to place this file anyway
             compress: Compression algorithm to use (COMPRESS_...)
 
         Returns:
             CbfsFile object containing the file information
         """
-        return CbfsFile(name, TYPE_RAW, data, compress)
+        return CbfsFile(name, TYPE_RAW, data, cbfs_offset, compress)
 
     @classmethod
     def empty(cls, space_to_use, erase_byte):
@@ -275,12 +282,44 @@ class CbfsFile(object):
         Returns:
             CbfsFile object containing the file information
         """
-        cfile = CbfsFile('', TYPE_EMPTY, b'')
+        cfile = CbfsFile('', TYPE_EMPTY, b'', None)
         cfile.size = space_to_use - FILE_HEADER_LEN - FILENAME_ALIGN
         cfile.erase_byte = erase_byte
         return cfile
 
-    def get_data(self):
+    def calc_start_offset(self):
+        """Check if this file needs to start at a particular offset in CBFS
+
+        Returns:
+            None if the file can be placed anywhere, or
+            the largest offset where the file could start (integer)
+        """
+        if self.cbfs_offset is None:
+            return None
+        return self.cbfs_offset - self.get_header_len()
+
+    def get_header_len(self):
+        """Get the length of headers required for a file
+
+        This is the minimum length required before the actual data for this file
+        could start. It might start later if there is padding.
+
+        Returns:
+            Total length of all non-data fields, in bytes
+        """
+        name = _pack_string(self.name)
+        hdr_len = len(name) + FILE_HEADER_LEN
+        if self.ftype == TYPE_STAGE:
+            pass
+        elif self.ftype == TYPE_RAW:
+            hdr_len += ATTR_COMPRESSION_LEN
+        elif self.ftype == TYPE_EMPTY:
+            pass
+        else:
+            raise ValueError('Unknown file type %#x\n' % self.ftype)
+        return hdr_len
+
+    def get_data(self, offset=None, pad_byte=None):
         """Obtain the contents of the file, in CBFS format
 
         Returns:
@@ -292,6 +331,7 @@ class CbfsFile(object):
         attr_pos = 0
         content = b''
         attr = b''
+        pad = b''
         data = self.data
         if self.ftype == TYPE_STAGE:
             elf_data = elf.DecodeElf(data, self.base_address)
@@ -315,10 +355,33 @@ class CbfsFile(object):
         if attr:
             attr_pos = hdr_len
             hdr_len += len(attr)
-        hdr = struct.pack(FILE_HEADER_FORMAT, FILE_MAGIC,
-                          len(content) + len(data),
+        if self.cbfs_offset is not None:
+            pad_len = self.cbfs_offset - offset - hdr_len
+            if pad_len < 0:  # pragma: no cover
+                # Test coverage of this is not available since this should never
+                # happen. It indicates that get_header_len() provided an
+                # incorrect value (too small) so that we decided that we could
+                # put this file at the requested place, but in fact a previous
+                # file extends far enough into the CBFS that this is not
+                # possible.
+                raise ValueError("Internal error: CBFS file '%s': Requested offset %#x but current output position is %#x" %
+                                 (self.name, self.cbfs_offset, offset))
+            pad = tools.GetBytes(pad_byte, pad_len)
+            hdr_len += pad_len
+        self.offset = len(content) + len(data)
+        hdr = struct.pack(FILE_HEADER_FORMAT, FILE_MAGIC, self.offset,
                           self.ftype, attr_pos, hdr_len)
-        return hdr + name + attr + content + data
+
+        # Do a sanity check of the get_header_len() function, to ensure that it
+        # stays in lockstep with this function
+        expected_len = self.get_header_len()
+        actual_len = len(hdr + name + attr)
+        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" %
+                             (self.name, expected_len, actual_len))
+        return hdr + name + attr + pad + content + data
 
 
 class CbfsWriter(object):
@@ -431,34 +494,39 @@ class CbfsWriter(object):
         if offset < self._size:
             self._skip_to(fd, offset)
 
-    def add_file_stage(self, name, data):
+    def add_file_stage(self, name, data, cbfs_offset=None):
         """Add a new stage file to the CBFS
 
         Args:
             name: String file name to put in CBFS (does not need to correspond
                 to the name that the file originally came from)
             data: Contents of file
+            cbfs_offset: Offset of this file's data within the CBFS, in bytes,
+                or None to place this file anywhere
 
         Returns:
             CbfsFile object created
         """
-        cfile = CbfsFile.stage(self._base_address, name, data)
+        cfile = CbfsFile.stage(self._base_address, name, data, cbfs_offset)
         self._files[name] = cfile
         return cfile
 
-    def add_file_raw(self, name, data, compress=COMPRESS_NONE):
+    def add_file_raw(self, name, data, cbfs_offset=None,
+                     compress=COMPRESS_NONE):
         """Create a new raw file
 
         Args:
             name: String file name to put in CBFS (does not need to correspond
                 to the name that the file originally came from)
             data: Contents of file
+            cbfs_offset: Offset of this file's data within the CBFS, in bytes,
+                or None to place this file anywhere
             compress: Compression algorithm to use (COMPRESS_...)
 
         Returns:
             CbfsFile object created
         """
-        cfile = CbfsFile.raw(name, data, compress)
+        cfile = CbfsFile.raw(name, data, cbfs_offset, compress)
         self._files[name] = cfile
         return cfile
 
@@ -507,7 +575,11 @@ class CbfsWriter(object):
 
         # Write out each file
         for cbf in self._files.values():
-            fd.write(cbf.get_data())
+            # 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))
+            fd.write(cbf.get_data(fd.tell(), self._erase_byte))
             self._align_to(fd, self._align)
         if not self._hdr_at_start:
             self._write_header(fd, add_fileheader=self._add_fileheader)
@@ -639,25 +711,27 @@ class CbfsReader(object):
 
         # Create the correct CbfsFile object depending on the type
         cfile = None
-        fd.seek(file_pos + offset, io.SEEK_SET)
+        cbfs_offset = file_pos + offset
+        fd.seek(cbfs_offset, io.SEEK_SET)
         if ftype == TYPE_CBFSHEADER:
             self._read_header(fd)
         elif ftype == TYPE_STAGE:
             data = fd.read(STAGE_LEN)
-            cfile = CbfsFile.stage(self.stage_base_address, name, b'')
+            cfile = CbfsFile.stage(self.stage_base_address, name, b'',
+                                   cbfs_offset)
             (cfile.compress, cfile.entry, cfile.load, cfile.data_len,
              cfile.memlen) = struct.unpack(STAGE_FORMAT, data)
             cfile.data = fd.read(cfile.data_len)
         elif ftype == TYPE_RAW:
             data = fd.read(size)
-            cfile = CbfsFile.raw(name, data, compress)
+            cfile = CbfsFile.raw(name, data, cbfs_offset, compress)
             cfile.decompress()
             if DEBUG:
                 print('data', data)
         elif ftype == TYPE_EMPTY:
             # Just read the data and discard it, since it is only padding
             fd.read(size)
-            cfile = CbfsFile('', TYPE_EMPTY, b'')
+            cfile = CbfsFile('', TYPE_EMPTY, b'', cbfs_offset)
         else:
             raise ValueError('Unknown type %#x when reading\n' % ftype)
         if cfile:
@@ -674,7 +748,8 @@ class CbfsReader(object):
         """Read attributes from the file
 
         CBFS files can have attributes which are things that cannot fit into the
-        header. The only attribute currently supported is compression.
+        header. The only attributes currently supported are compression and the
+        unused tag.
 
         Args:
             fd: File to read from
@@ -703,6 +778,8 @@ class CbfsReader(object):
                 # We don't currently use this information
                 atag, alen, compress, _decomp_size = struct.unpack(
                     ATTR_COMPRESSION_FORMAT, data)
+            elif atag == FILE_ATTR_TAG_UNUSED2:
+                break
             else:
                 print('Unknown attribute tag %x' % atag)
             attr_size -= len(data)
@@ -760,7 +837,7 @@ class CbfsReader(object):
         return val.decode('utf-8')
 
 
-def cbfstool(fname, *cbfs_args):
+def cbfstool(fname, *cbfs_args, **kwargs):
     """Run cbfstool with provided arguments
 
     If the tool fails then this function raises an exception and prints out the
@@ -773,7 +850,9 @@ def cbfstool(fname, *cbfs_args):
     Returns:
         CommandResult object containing the results
     """
-    args = ('cbfstool', fname) + cbfs_args
+    args = ['cbfstool', fname] + list(cbfs_args)
+    if kwargs.get('base') is not None:
+        args += ['-b', '%#x' % kwargs['base']]
     result = command.RunPipe([args], capture=not VERBOSE,
                              capture_stderr=not VERBOSE, raise_on_error=False)
     if result.return_code:
diff --git a/tools/binman/cbfs_util_test.py b/tools/binman/cbfs_util_test.py
index 9bb6a29822..0fe4aa494e 100755
--- a/tools/binman/cbfs_util_test.py
+++ b/tools/binman/cbfs_util_test.py
@@ -105,7 +105,7 @@ class TestCbfs(unittest.TestCase):
         return cbfs
 
     def _check_uboot(self, cbfs, ftype=cbfs_util.TYPE_RAW, offset=0x38,
-                     data=U_BOOT_DATA):
+                     data=U_BOOT_DATA, cbfs_offset=None):
         """Check that the U-Boot file is as expected
 
         Args:
@@ -113,6 +113,7 @@ class TestCbfs(unittest.TestCase):
             ftype: Expected file type
             offset: Expected offset of file
             data: Expected data in file
+            cbfs_offset: Expected CBFS offset for file's data
 
         Returns:
             CbfsFile object containing the file
@@ -121,24 +122,30 @@ class TestCbfs(unittest.TestCase):
         cfile = cbfs.files['u-boot']
         self.assertEqual('u-boot', cfile.name)
         self.assertEqual(offset, cfile.offset)
+        if cbfs_offset is not None:
+            self.assertEqual(cbfs_offset, cfile.cbfs_offset)
         self.assertEqual(data, cfile.data)
         self.assertEqual(ftype, cfile.ftype)
         self.assertEqual(cbfs_util.COMPRESS_NONE, cfile.compress)
         self.assertEqual(len(data), cfile.memlen)
         return cfile
 
-    def _check_dtb(self, cbfs, offset=0x38, data=U_BOOT_DTB_DATA):
+    def _check_dtb(self, cbfs, offset=0x38, data=U_BOOT_DTB_DATA,
+                   cbfs_offset=None):
         """Check that the U-Boot dtb file is as expected
 
         Args:
             cbfs: CbfsReader object to check
             offset: Expected offset of file
             data: Expected data in file
+            cbfs_offset: Expected CBFS offset for file's data
         """
         self.assertIn('u-boot-dtb', cbfs.files)
         cfile = cbfs.files['u-boot-dtb']
         self.assertEqual('u-boot-dtb', cfile.name)
         self.assertEqual(offset, cfile.offset)
+        if cbfs_offset is not None:
+            self.assertEqual(cbfs_offset, cfile.cbfs_offset)
         self.assertEqual(U_BOOT_DTB_DATA, cfile.data)
         self.assertEqual(cbfs_util.TYPE_RAW, cfile.ftype)
         self.assertEqual(cbfs_util.COMPRESS_NONE, cfile.compress)
@@ -157,13 +164,14 @@ class TestCbfs(unittest.TestCase):
         self._check_uboot(cbfs)
         self._check_dtb(cbfs)
 
-    def _get_expected_cbfs(self, size, arch='x86', compress=None):
+    def _get_expected_cbfs(self, size, arch='x86', compress=None, base=None):
         """Get the file created by cbfstool for a particular scenario
 
         Args:
             size: Size of the CBFS in bytes
             arch: Architecture of the CBFS, as a string
             compress: Compression to use, e.g. cbfs_util.COMPRESS_LZMA
+            base: Base address of file, or None to put it anywhere
 
         Returns:
             Resulting CBFS file, or None if cbfstool is not available
@@ -172,14 +180,18 @@ class TestCbfs(unittest.TestCase):
             return None
         cbfs_fname = os.path.join(self._indir, 'test.cbfs')
         cbfs_util.cbfstool(cbfs_fname, 'create', '-m', arch, '-s', '%#x' % size)
+        if base:
+            base = [(1 << 32) - size + b for b in base]
         cbfs_util.cbfstool(cbfs_fname, 'add', '-n', 'u-boot', '-t', 'raw',
                            '-c', compress and compress[0] or 'none',
                            '-f', tools.GetInputFilename(
-                               compress and 'compress' or 'u-boot.bin'))
+                               compress and 'compress' or 'u-boot.bin'),
+                           base=base[0] if base else None)
         cbfs_util.cbfstool(cbfs_fname, 'add', '-n', 'u-boot-dtb', '-t', 'raw',
                            '-c', compress and compress[1] or 'none',
                            '-f', tools.GetInputFilename(
-                               compress and 'compress' or 'u-boot.dtb'))
+                               compress and 'compress' or 'u-boot.dtb'),
+                           base=base[1] if base else None)
         return cbfs_fname
 
     def _compare_expected_cbfs(self, data, cbfstool_fname):
@@ -407,7 +419,7 @@ class TestCbfs(unittest.TestCase):
             self.skipTest('lz4 --no-frame-crc not available')
         size = 0x140
         cbw = CbfsWriter(size)
-        cbw.add_file_raw('u-boot', COMPRESS_DATA,
+        cbw.add_file_raw('u-boot', COMPRESS_DATA, None,
                          compress=cbfs_util.COMPRESS_LZ4)
         data = cbw.get_data()
 
@@ -431,7 +443,7 @@ class TestCbfs(unittest.TestCase):
             self.skipTest('lz4 --no-frame-crc not available')
         size = 0x140
         cbw = CbfsWriter(size)
-        cbw.add_file_raw('u-boot', COMPRESS_DATA,
+        cbw.add_file_raw('u-boot', COMPRESS_DATA, None,
                          compress=cbfs_util.COMPRESS_LZ4)
         data = cbw.get_data()
 
@@ -517,9 +529,9 @@ class TestCbfs(unittest.TestCase):
             self.skipTest('lz4 --no-frame-crc not available')
         size = 0x140
         cbw = CbfsWriter(size)
-        cbw.add_file_raw('u-boot', COMPRESS_DATA,
+        cbw.add_file_raw('u-boot', COMPRESS_DATA, None,
                          compress=cbfs_util.COMPRESS_LZ4)
-        cbw.add_file_raw('u-boot-dtb', COMPRESS_DATA,
+        cbw.add_file_raw('u-boot-dtb', COMPRESS_DATA, None,
                          compress=cbfs_util.COMPRESS_LZMA)
         data = cbw.get_data()
 
@@ -556,6 +568,58 @@ class TestCbfs(unittest.TestCase):
         cbfs_fname = self._get_expected_cbfs(size=size)
         self._compare_expected_cbfs(data, cbfs_fname)
 
+    def test_cbfs_offset(self):
+        """Test a CBFS with files at particular offsets"""
+        size = 0x200
+        cbw = CbfsWriter(size)
+        cbw.add_file_raw('u-boot', U_BOOT_DATA, 0x40)
+        cbw.add_file_raw('u-boot-dtb', U_BOOT_DTB_DATA, 0x140)
+
+        data = cbw.get_data()
+        cbfs = self._check_hdr(data, size)
+        self._check_uboot(cbfs, ftype=cbfs_util.TYPE_RAW, offset=0x40,
+                          cbfs_offset=0x40)
+        self._check_dtb(cbfs, offset=0x40, cbfs_offset=0x140)
+
+        cbfs_fname = self._get_expected_cbfs(size=size, base=(0x40, 0x140))
+        self._compare_expected_cbfs(data, cbfs_fname)
+
+    def test_cbfs_invalid_file_type_header(self):
+        """Check handling of an invalid file type when outputting a header"""
+        size = 0xb0
+        cbw = CbfsWriter(size)
+        cfile = cbw.add_file_raw('u-boot', U_BOOT_DATA, 0)
+
+        # Change the type manually before generating the CBFS, and make sure
+        # that the generator complains
+        cfile.ftype = 0xff
+        with self.assertRaises(ValueError) as e:
+            cbw.get_data()
+        self.assertIn('Unknown file type 0xff', str(e.exception))
+
+    def test_cbfs_offset_conflict(self):
+        """Test a CBFS with files that want to overlap"""
+        size = 0x200
+        cbw = CbfsWriter(size)
+        cbw.add_file_raw('u-boot', U_BOOT_DATA, 0x40)
+        cbw.add_file_raw('u-boot-dtb', U_BOOT_DTB_DATA, 0x80)
+
+        with self.assertRaises(ValueError) as e:
+            cbw.get_data()
+        self.assertIn('No space for data before pad offset', str(e.exception))
+
+    def test_cbfs_check_offset(self):
+        """Test that we can discover the offset of a file after writing it"""
+        size = 0xb0
+        cbw = CbfsWriter(size)
+        cbw.add_file_raw('u-boot', U_BOOT_DATA)
+        cbw.add_file_raw('u-boot-dtb', U_BOOT_DTB_DATA)
+        data = cbw.get_data()
+
+        cbfs = cbfs_util.CbfsReader(data)
+        self.assertEqual(0x38, cbfs.files['u-boot'].cbfs_offset)
+        self.assertEqual(0x78, cbfs.files['u-boot-dtb'].cbfs_offset)
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/tools/binman/etype/cbfs.py b/tools/binman/etype/cbfs.py
index 513df217bc..49baa6a4f6 100644
--- a/tools/binman/etype/cbfs.py
+++ b/tools/binman/etype/cbfs.py
@@ -52,6 +52,7 @@ class Entry_cbfs(Entry):
                 filename = "u-boot.dtb";
                 cbfs-type = "raw";
                 cbfs-compress = "lz4";
+                cbfs-offset = <0x100000>;
             };
         };
 
@@ -110,6 +111,15 @@ class Entry_cbfs(Entry):
             to add a flat binary with a load/start address, similar to the
             'add-flat-binary' option in cbfstool.
 
+    cbfs-offset:
+        This is the offset of the file's data within the CBFS. It is used to
+        specify where the file should be placed in cases where a fixed position
+        is needed. Typical uses are for code which is not relocatable and must
+        execute in-place from a particular address. This works because SPI flash
+        is generally mapped into memory on x86 devices. The file header is
+        placed before this offset so that the data start lines up exactly with
+        the chosen offset. If this property is not provided, then the file is
+        placed in the next available spot.
 
     The current implementation supports only a subset of CBFS features. It does
     not support other file types (e.g. payload), adding multiple files (like the
@@ -172,9 +182,10 @@ class Entry_cbfs(Entry):
                 return False
             data = entry.GetData()
             if entry._type == 'raw':
-                cbfs.add_file_raw(entry._cbfs_name, data, entry._cbfs_compress)
+                cbfs.add_file_raw(entry._cbfs_name, data, entry._cbfs_offset,
+                                  entry._cbfs_compress)
             elif entry._type == 'stage':
-                cbfs.add_file_stage(entry._cbfs_name, data)
+                cbfs.add_file_stage(entry._cbfs_name, data, entry._cbfs_offset)
         data = cbfs.get_data()
         self.SetContents(data)
         return True
@@ -186,6 +197,7 @@ class Entry_cbfs(Entry):
             entry._cbfs_name = fdt_util.GetString(node, 'cbfs-name', entry.name)
             entry._type = fdt_util.GetString(node, 'cbfs-type')
             compress = fdt_util.GetString(node, 'cbfs-compress', 'none')
+            entry._cbfs_offset = fdt_util.GetInt(node, 'cbfs-offset')
             entry._cbfs_compress = cbfs_util.find_compress(compress)
             if entry._cbfs_compress is None:
                 self.Raise("Invalid compression in '%s': '%s'" %
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 1355c4f55d..5bde8aa30a 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -2012,5 +2012,28 @@ class TestFunctional(unittest.TestCase):
         self.assertIn('Could not complete processing of contents',
                       str(e.exception))
 
+    def testCbfsOffset(self):
+        """Test a CBFS with files at particular offsets
+
+        Like all CFBS tests, this is just checking the logic that calls
+        cbfs_util. See cbfs_util_test for fully tests (e.g. test_cbfs_offset()).
+        """
+        data = self._DoReadFile('114_cbfs_offset.dts')
+        size = 0x200
+
+        cbfs = cbfs_util.CbfsReader(data)
+        self.assertEqual(size, cbfs.rom_size)
+
+        self.assertIn('u-boot', cbfs.files)
+        cfile = cbfs.files['u-boot']
+        self.assertEqual(U_BOOT_DATA, cfile.data)
+        self.assertEqual(0x40, cfile.cbfs_offset)
+
+        self.assertIn('u-boot-dtb', cbfs.files)
+        cfile2 = cbfs.files['u-boot-dtb']
+        self.assertEqual(U_BOOT_DTB_DATA, cfile2.data)
+        self.assertEqual(0x140, cfile2.cbfs_offset)
+
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/114_cbfs_offset.dts b/tools/binman/test/114_cbfs_offset.dts
new file mode 100644
index 0000000000..7aa9d9d4bf
--- /dev/null
+++ b/tools/binman/test/114_cbfs_offset.dts
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		sort-by-offset;
+		end-at-4gb;
+		size = <0x200>;
+		cbfs {
+			size = <0x200>;
+			offset = <0xfffffe00>;
+			u-boot {
+				cbfs-offset = <0x40>;
+				cbfs-type = "raw";
+			};
+			u-boot-dtb {
+				cbfs-offset = <0x140>;
+				cbfs-type = "raw";
+			};
+		};
+	};
+};