Re: [PATCH v8 06/13] binman: Support new op-tee binary format

(dropping the two bounces from cc)

On Thu, 22 Dec 2022 at 13:18, Simon Glass sjg@chromium.org wrote:

Hi Jerome,

On Thu, 22 Dec 2022 at 08:36, Jerome Forissier jerome.forissier@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@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 {
  

•                                 };
  

•                         };
  

•                 };
  

•         };
  

• };
  

+};