[PATCH v8 06/13] binman: Support new op-tee binary format
Simon Glass
sjg at chromium.org
Thu Dec 22 21:23:06 CET 2022
(dropping the two bounces from cc)
On Thu, 22 Dec 2022 at 13:18, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Jerome,
>
> On Thu, 22 Dec 2022 at 08:36, Jerome Forissier
> <jerome.forissier at linaro.org> wrote:
> >
> >
> >
> > On 12/22/22 00:07, Simon Glass wrote:
> > > OP-TEE has a format with a binary header that can be used instead of the
> > > ELF file. With newer versions of OP-TEE this may be required on some
> > > platforms.
> > >
> > > Add support for this in binman. First, add a method to obtain the ELF
> > > sections from an entry, then use that in the FIT support. We then end up
> > > with the ability to support both types of OP-TEE files, depending on which
> > > one is passed in with the entry argument (TEE=xxx in the U-Boot build).
> >
> > So, with:
> >
> > BL31=/path/to/bl31.elf TEE=/path/to/tee.bin make -C u-boot \
> > CROSS_COMPILE=aarch64-linux-gnu- CC=aarch64-linux-gnu-gcc \
> > HOSTCC=gcc BINMAN_DEBUG=1 BINMAN_VERBOSE=4
> >
> >
> > ...I get:
> >
> > Entry '/binman/simple-bin/fit/images/@tee-SEQ/tee-os' marked absent: uses v1 format which must be in a FIT
> >
> >
> > More complete log at https://pastebin.com/UZzZeicQ
>
> Thanks.
>
> Is this file in the v1 binary format (with the custom header), or is
> is a plain binary file? At present only the former is supported, as I
> thought that it can only be the elf or the v1 binary format these
> days? Actually can you please send me the tee.bin ?
>
> Regards,
> Simon
>
>
> >
> > Thanks,
> > --
> > Jerome
> >
> > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > ---
> > >
> > > (no changes since v7)
> > >
> > > Changes in v7:
> > > - Correct missing test coverage
> > >
> > > Changes in v6:
> > > - Update op-tee to support new v1 binary header
> > >
> > > tools/binman/entries.rst | 35 ++++++++-
> > > tools/binman/entry.py | 13 +++
> > > tools/binman/etype/fit.py | 69 +++++++++-------
> > > tools/binman/etype/section.py | 9 +++
> > > tools/binman/etype/tee_os.py | 68 +++++++++++++++-
> > > tools/binman/ftest.py | 83 ++++++++++++++++++++
> > > tools/binman/test/263_tee_os_opt.dts | 22 ++++++
> > > tools/binman/test/264_tee_os_opt_fit.dts | 33 ++++++++
> > > tools/binman/test/265_tee_os_opt_fit_bad.dts | 40 ++++++++++
> > > 9 files changed, 340 insertions(+), 32 deletions(-)
> > > create mode 100644 tools/binman/test/263_tee_os_opt.dts
> > > create mode 100644 tools/binman/test/264_tee_os_opt_fit.dts
> > > create mode 100644 tools/binman/test/265_tee_os_opt_fit_bad.dts
> > >
> > > diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
> > > index b2ce7960d3b..a3e4493a44f 100644
> > > --- a/tools/binman/entries.rst
> > > +++ b/tools/binman/entries.rst
> > > @@ -1508,12 +1508,45 @@ Entry: tee-os: Entry containing an OP-TEE Trusted OS (TEE) blob
> > >
> > > Properties / Entry arguments:
> > > - tee-os-path: Filename of file to read into entry. This is typically
> > > - called tee-pager.bin
> > > + called tee.bin or tee.elf
> > >
> > > This entry holds the run-time firmware, typically started by U-Boot SPL.
> > > See the U-Boot README for your architecture or board for how to use it. See
> > > https://github.com/OP-TEE/optee_os for more information about OP-TEE.
> > >
> > > +Note that if the file is in ELF format, it must go in a FIT. In that case,
> > > +this entry will mark itself as absent, providing the data only through the
> > > +read_elf_segments() method.
> > > +
> > > +Marking this entry as absent means that it if is used in the wrong context
> > > +it can be automatically dropped. Thus it is possible to add anb OP-TEE entry
> > > +like this::
> > > +
> > > + binman {
> > > + tee-os {
> > > + };
> > > + };
> > > +
> > > +and pass either an ELF or plain binary in with -a tee-os-path <filename>
> > > +and have binman do the right thing:
> > > +
> > > + - include the entry if tee.bin is provided and it doesn't have the v1
> > > + header
> > > + - drop it otherwise
> > > +
> > > +When used within a FIT, we can do::
> > > +
> > > + binman {
> > > + fit {
> > > + tee-os {
> > > + };
> > > + };
> > > + };
> > > +
> > > +which will split the ELF into separate nodes for each segment, if an ELF
> > > +file is provide (see Flat Image Tree / FIT), or produce a single node if
> > > +the binary v1 format is provided.
> > > +
> > >
> > >
> > > .. _etype_text:
> > > diff --git a/tools/binman/entry.py b/tools/binman/entry.py
> > > index 637aece3705..de51d295891 100644
> > > --- a/tools/binman/entry.py
> > > +++ b/tools/binman/entry.py
> > > @@ -1290,3 +1290,16 @@ features to produce new behaviours.
> > > def mark_absent(self, msg):
> > > tout.info("Entry '%s' marked absent: %s" % (self._node.path, msg))
> > > self.absent = True
> > > +
> > > + def read_elf_segments(self):
> > > + """Read segments from an entry that can generate an ELF file
> > > +
> > > + Returns:
> > > + tuple:
> > > + list of segments, each:
> > > + int: Segment number (0 = first)
> > > + int: Start address of segment in memory
> > > + bytes: Contents of segment
> > > + int: entry address of ELF file
> > > + """
> > > + return None
> > > diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> > > index 8ad4f3a8a83..21c769a1cbe 100644
> > > --- a/tools/binman/etype/fit.py
> > > +++ b/tools/binman/etype/fit.py
> > > @@ -540,41 +540,34 @@ class Entry_fit(Entry_section):
> > > else:
> > > self.Raise("Generator node requires 'fit,fdt-list' property")
> > >
> > > - def _gen_split_elf(base_node, node, elf_data, missing):
> > > + def _gen_split_elf(base_node, node, segments, entry_addr):
> > > """Add nodes for the ELF file, one per group of contiguous segments
> > >
> > > Args:
> > > base_node (Node): Template node from the binman definition
> > > node (Node): Node to replace (in the FIT being built)
> > > - data (bytes): ELF-format data to process (may be empty)
> > > - missing (bool): True if any of the data is missing
> > >
> > > + segments, entry_addr
> > > +
> > > + data (bytes): ELF-format data to process (may be empty)
> > > """
> > > - # If any pieces are missing, skip this. The missing entries will
> > > - # show an error
> > > - if not missing:
> > > - try:
> > > - segments, entry = elf.read_loadable_segments(elf_data)
> > > - except ValueError as exc:
> > > - self._raise_subnode(node,
> > > - f'Failed to read ELF file: {str(exc)}')
> > > - for (seq, start, data) in segments:
> > > - node_name = node.name[1:].replace('SEQ', str(seq + 1))
> > > - with fsw.add_node(node_name):
> > > - loadables.append(node_name)
> > > - for pname, prop in node.props.items():
> > > - if not pname.startswith('fit,'):
> > > - fsw.property(pname, prop.bytes)
> > > - elif pname == 'fit,load':
> > > - fsw.property_u32('load', start)
> > > - elif pname == 'fit,entry':
> > > - if seq == 0:
> > > - fsw.property_u32('entry', entry)
> > > - elif pname == 'fit,data':
> > > - fsw.property('data', bytes(data))
> > > - elif pname != 'fit,operation':
> > > - self._raise_subnode(
> > > - node, f"Unknown directive '{pname}'")
> > > + for (seq, start, data) in segments:
> > > + node_name = node.name[1:].replace('SEQ', str(seq + 1))
> > > + with fsw.add_node(node_name):
> > > + loadables.append(node_name)
> > > + for pname, prop in node.props.items():
> > > + if not pname.startswith('fit,'):
> > > + fsw.property(pname, prop.bytes)
> > > + elif pname == 'fit,load':
> > > + fsw.property_u32('load', start)
> > > + elif pname == 'fit,entry':
> > > + if seq == 0:
> > > + fsw.property_u32('entry', entry_addr)
> > > + elif pname == 'fit,data':
> > > + fsw.property('data', bytes(data))
> > > + elif pname != 'fit,operation':
> > > + self._raise_subnode(
> > > + node, f"Unknown directive '{pname}'")
> > >
> > > def _gen_node(base_node, node, depth, in_images, entry):
> > > """Generate nodes from a template
> > > @@ -598,6 +591,8 @@ class Entry_fit(Entry_section):
> > > depth (int): Current node depth (0 is the base 'fit' node)
> > > in_images (bool): True if this is inside the 'images' node, so
> > > that 'data' properties should be generated
> > > + entry (entry_Section): Entry for the second containing the
> > > + contents of this node
> > > """
> > > oper = self._get_operation(base_node, node)
> > > if oper == OP_GEN_FDT_NODES:
> > > @@ -609,10 +604,24 @@ class Entry_fit(Entry_section):
> > > missing_list = []
> > > entry.ObtainContents()
> > > entry.Pack(0)
> > > - data = entry.GetData()
> > > entry.CheckMissing(missing_list)
> > >
> > > - _gen_split_elf(base_node, node, data, bool(missing_list))
> > > + # If any pieces are missing, skip this. The missing entries will
> > > + # show an error
> > > + if not missing_list:
> > > + segs = entry.read_elf_segments()
> > > + if segs:
> > > + segments, entry_addr = segs
> > > + else:
> > > + elf_data = entry.GetData()
> > > + try:
> > > + segments, entry_addr = (
> > > + elf.read_loadable_segments(elf_data))
> > > + except ValueError as exc:
> > > + self._raise_subnode(
> > > + node, f'Failed to read ELF file: {str(exc)}')
> > > +
> > > + _gen_split_elf(base_node, node, segments, entry_addr)
> > >
> > > def _add_node(base_node, depth, node):
> > > """Add nodes to the output FIT
> > > diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
> > > index dcb7a062047..57bfee0b286 100644
> > > --- a/tools/binman/etype/section.py
> > > +++ b/tools/binman/etype/section.py
> > > @@ -948,3 +948,12 @@ class Entry_section(Entry):
> > > super().AddBintools(btools)
> > > for entry in self._entries.values():
> > > entry.AddBintools(btools)
> > > +
> > > + def read_elf_segments(self):
> > > + entries = self.GetEntries()
> > > +
> > > + # If the section only has one entry, see if it can provide ELF segments
> > > + if len(entries) == 1:
> > > + for entry in entries.values():
> > > + return entry.read_elf_segments()
> > > + return None
> > > diff --git a/tools/binman/etype/tee_os.py b/tools/binman/etype/tee_os.py
> > > index 6ce4b672de4..687acd4689f 100644
> > > --- a/tools/binman/etype/tee_os.py
> > > +++ b/tools/binman/etype/tee_os.py
> > > @@ -4,19 +4,85 @@
> > > # Entry-type module for OP-TEE Trusted OS firmware blob
> > > #
> > >
> > > +import struct
> > > +
> > > from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg
> > > +from binman import elf
> > >
> > > class Entry_tee_os(Entry_blob_named_by_arg):
> > > """Entry containing an OP-TEE Trusted OS (TEE) blob
> > >
> > > Properties / Entry arguments:
> > > - tee-os-path: Filename of file to read into entry. This is typically
> > > - called tee-pager.bin
> > > + called tee.bin or tee.elf
> > >
> > > This entry holds the run-time firmware, typically started by U-Boot SPL.
> > > See the U-Boot README for your architecture or board for how to use it. See
> > > https://github.com/OP-TEE/optee_os for more information about OP-TEE.
> > > +
> > > + Note that if the file is in ELF format, it must go in a FIT. In that case,
> > > + this entry will mark itself as absent, providing the data only through the
> > > + read_elf_segments() method.
> > > +
> > > + Marking this entry as absent means that it if is used in the wrong context
> > > + it can be automatically dropped. Thus it is possible to add anb OP-TEE entry
> > > + like this::
> > > +
> > > + binman {
> > > + tee-os {
> > > + };
> > > + };
> > > +
> > > + and pass either an ELF or plain binary in with -a tee-os-path <filename>
> > > + and have binman do the right thing:
> > > +
> > > + - include the entry if tee.bin is provided and it doesn't have the v1
> > > + header
> > > + - drop it otherwise
> > > +
> > > + When used within a FIT, we can do::
> > > +
> > > + binman {
> > > + fit {
> > > + tee-os {
> > > + };
> > > + };
> > > + };
> > > +
> > > + which will split the ELF into separate nodes for each segment, if an ELF
> > > + file is provide (see Flat Image Tree / FIT), or produce a single node if
> > > + the binary v1 format is provided.
> > > """
> > > def __init__(self, section, etype, node):
> > > super().__init__(section, etype, node, 'tee-os')
> > > self.external = True
> > > +
> > > + @staticmethod
> > > + def is_optee_bin(data):
> > > + return len(data) >= 8 and data[0:5] == b'OPTE\x01'
> > > +
> > > + def ObtainContents(self, fake_size=0):
> > > + super().ObtainContents(fake_size)
> > > + if not self.missing:
> > > + if elf.is_valid(self.data):
> > > + self.mark_absent('uses Elf format which must be in a FIT')
> > > + elif self.is_optee_bin(self.data):
> > > + self.mark_absent('uses v1 format which must be in a FIT')
> > > + return True
> > > +
> > > + def read_elf_segments(self):
> > > + data = self.GetData()
> > > + if self.is_optee_bin(data):
> > > + # OP-TEE v1 format (tee.bin)
> > > + init_sz, start_hi, start_lo, _, paged_sz = (
> > > + struct.unpack_from('<5I', data, 0x8))
> > > + if paged_sz != 0:
> > > + self.Raise("OP-TEE paged mode not supported")
> > > + e_entry = (start_hi << 32) + start_lo
> > > + p_addr = e_entry
> > > + p_data = data[0x1c:]
> > > + if len(p_data) != init_sz:
> > > + self.Raise("Invalid OP-TEE file: size mismatch (expected %#x, have %#x)" %
> > > + (init_sz, len(p_data)))
> > > + return [[0, p_addr, p_data]], e_entry
> > > + return None
> > > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> > > index f47a745f1e1..f893050e706 100644
> > > --- a/tools/binman/ftest.py
> > > +++ b/tools/binman/ftest.py
> > > @@ -112,6 +112,8 @@ REPACK_DTB_PROPS = ['orig-offset', 'orig-size']
> > > # Supported compression bintools
> > > COMP_BINTOOLS = ['bzip2', 'gzip', 'lz4', 'lzma_alone', 'lzop', 'xz', 'zstd']
> > >
> > > +TEE_ADDR = 0x5678
> > > +
> > > class TestFunctional(unittest.TestCase):
> > > """Functional tests for binman
> > >
> > > @@ -219,6 +221,9 @@ class TestFunctional(unittest.TestCase):
> > > TestFunctional._MakeInputFile('tee.elf',
> > > tools.read_file(cls.ElfTestFile('elf_sections')))
> > >
> > > + # Newer OP_TEE file in v1 binary format
> > > + cls.make_tee_bin('tee.bin')
> > > +
> > > cls.comp_bintools = {}
> > > for name in COMP_BINTOOLS:
> > > cls.comp_bintools[name] = bintool.Bintool.create(name)
> > > @@ -644,6 +649,14 @@ class TestFunctional(unittest.TestCase):
> > > def ElfTestFile(cls, fname):
> > > return os.path.join(cls._elf_testdir, fname)
> > >
> > > + @classmethod
> > > + def make_tee_bin(cls, fname, paged_sz=0, extra_data=b''):
> > > + init_sz, start_hi, start_lo, dummy = (len(U_BOOT_DATA), 0, TEE_ADDR, 0)
> > > + data = b'OPTE\x01xxx' + struct.pack('<5I', init_sz, start_hi, start_lo,
> > > + dummy, paged_sz) + U_BOOT_DATA
> > > + data += extra_data
> > > + TestFunctional._MakeInputFile(fname, data)
> > > +
> > > def AssertInList(self, grep_list, target):
> > > """Assert that at least one of a list of things is in a target
> > >
> > > @@ -6095,6 +6108,76 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
> > > data = self._DoReadFile('262_absent.dts')
> > > self.assertEqual(U_BOOT_DATA + U_BOOT_IMG_DATA, data)
> > >
> > > + def testPackTeeOsOptional(self):
> > > + """Test that an image with an optional TEE binary can be created"""
> > > + entry_args = {
> > > + 'tee-os-path': 'tee.elf',
> > > + }
> > > + data = self._DoReadFileDtb('263_tee_os_opt.dts',
> > > + entry_args=entry_args)[0]
> > > + self.assertEqual(U_BOOT_DATA + U_BOOT_IMG_DATA, data)
> > > +
> > > + def checkFitTee(self, dts, tee_fname):
> > > + """Check that a tee-os entry works and returns data
> > > +
> > > + Args:
> > > + dts (str): Device tree filename to use
> > > + tee_fname (str): filename containing tee-os
> > > +
> > > + Returns:
> > > + bytes: Image contents
> > > + """
> > > + if not elf.ELF_TOOLS:
> > > + self.skipTest('Python elftools not available')
> > > + entry_args = {
> > > + 'of-list': 'test-fdt1 test-fdt2',
> > > + 'default-dt': 'test-fdt2',
> > > + 'tee-os-path': tee_fname,
> > > + }
> > > + test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR)
> > > + data = self._DoReadFileDtb(dts, entry_args=entry_args,
> > > + extra_indirs=[test_subdir])[0]
> > > + return data
> > > +
> > > + def testFitTeeOsOptionalFit(self):
> > > + """Test an image with a FIT with an optional OP-TEE binary"""
> > > + data = self.checkFitTee('264_tee_os_opt_fit.dts', 'tee.bin')
> > > +
> > > + # There should be only one node, holding the data set up in SetUpClass()
> > > + # for tee.bin
> > > + dtb = fdt.Fdt.FromData(data)
> > > + dtb.Scan()
> > > + node = dtb.GetNode('/images/tee-1')
> > > + self.assertEqual(TEE_ADDR,
> > > + fdt_util.fdt32_to_cpu(node.props['load'].value))
> > > + self.assertEqual(TEE_ADDR,
> > > + fdt_util.fdt32_to_cpu(node.props['entry'].value))
> > > + self.assertEqual(U_BOOT_DATA, node.props['data'].bytes)
> > > +
> > > + def testFitTeeOsOptionalFitBad(self):
> > > + """Test an image with a FIT with an optional OP-TEE binary"""
> > > + with self.assertRaises(ValueError) as exc:
> > > + self.checkFitTee('265_tee_os_opt_fit_bad.dts', 'tee.bin')
> > > + self.assertIn(
> > > + "Node '/binman/fit': subnode 'images/@tee-SEQ': Failed to read ELF file: Magic number does not match",
> > > + str(exc.exception))
> > > +
> > > + def testFitTeeOsBad(self):
> > > + """Test an OP-TEE binary with wrong formats"""
> > > + self.make_tee_bin('tee.bad1', 123)
> > > + with self.assertRaises(ValueError) as exc:
> > > + self.checkFitTee('264_tee_os_opt_fit.dts', 'tee.bad1')
> > > + self.assertIn(
> > > + "Node '/binman/fit/images/@tee-SEQ/tee-os': OP-TEE paged mode not supported",
> > > + str(exc.exception))
> > > +
> > > + self.make_tee_bin('tee.bad2', 0, b'extra data')
> > > + with self.assertRaises(ValueError) as exc:
> > > + self.checkFitTee('264_tee_os_opt_fit.dts', 'tee.bad2')
> > > + self.assertIn(
> > > + "Node '/binman/fit/images/@tee-SEQ/tee-os': Invalid OP-TEE file: size mismatch (expected 0x4, have 0xe)",
> > > + str(exc.exception))
> > > +
> > >
> > > if __name__ == "__main__":
> > > unittest.main()
> > > diff --git a/tools/binman/test/263_tee_os_opt.dts b/tools/binman/test/263_tee_os_opt.dts
> > > new file mode 100644
> > > index 00000000000..2e4ec24ac2c
> > > --- /dev/null
> > > +++ b/tools/binman/test/263_tee_os_opt.dts
> > > @@ -0,0 +1,22 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +
> > > +/dts-v1/;
> > > +
> > > +/ {
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> > > +
> > > + binman {
> > > + u-boot {
> > > + };
> > > + tee-os {
> > > + /*
> > > + * this results in nothing being added since only the
> > > + * .bin format is supported by this etype, unless it is
> > > + * part of a FIT
> > > + */
> > > + };
> > > + u-boot-img {
> > > + };
> > > + };
> > > +};
> > > diff --git a/tools/binman/test/264_tee_os_opt_fit.dts b/tools/binman/test/264_tee_os_opt_fit.dts
> > > new file mode 100644
> > > index 00000000000..ae44b433edf
> > > --- /dev/null
> > > +++ b/tools/binman/test/264_tee_os_opt_fit.dts
> > > @@ -0,0 +1,33 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +
> > > +/dts-v1/;
> > > +
> > > +/ {
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> > > +
> > > + binman {
> > > + fit {
> > > + description = "test-desc";
> > > + #address-cells = <1>;
> > > + fit,fdt-list = "of-list";
> > > +
> > > + images {
> > > + @tee-SEQ {
> > > + fit,operation = "split-elf";
> > > + description = "TEE";
> > > + type = "tee";
> > > + arch = "arm64";
> > > + os = "tee";
> > > + compression = "none";
> > > + fit,load;
> > > + fit,entry;
> > > + fit,data;
> > > +
> > > + tee-os {
> > > + };
> > > + };
> > > + };
> > > + };
> > > + };
> > > +};
> > > diff --git a/tools/binman/test/265_tee_os_opt_fit_bad.dts b/tools/binman/test/265_tee_os_opt_fit_bad.dts
> > > new file mode 100644
> > > index 00000000000..7fa363cc199
> > > --- /dev/null
> > > +++ b/tools/binman/test/265_tee_os_opt_fit_bad.dts
> > > @@ -0,0 +1,40 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +
> > > +/dts-v1/;
> > > +
> > > +/ {
> > > + #address-cells = <1>;
> > > + #size-cells = <1>;
> > > +
> > > + binman {
> > > + fit {
> > > + description = "test-desc";
> > > + #address-cells = <1>;
> > > + fit,fdt-list = "of-list";
> > > +
> > > + images {
> > > + @tee-SEQ {
> > > + fit,operation = "split-elf";
> > > + description = "TEE";
> > > + type = "tee";
> > > + arch = "arm64";
> > > + os = "tee";
> > > + compression = "none";
> > > + fit,load;
> > > + fit,entry;
> > > + fit,data;
> > > +
> > > + tee-os {
> > > + };
> > > +
> > > + /*
> > > + * mess up the ELF data by adding
> > > + * another bit of data at the end
> > > + */
> > > + u-boot {
> > > + };
> > > + };
> > > + };
> > > + };
> > > + };
> > > +};
More information about the U-Boot
mailing list