[PATCH v9 07/14] binman: Support new op-tee binary format

[Simon Glass <sjg@chromium.org>]

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).

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

(no changes since v8)

Changes in v8:
- Fix 'anb', 'provide' and 'second'  typos
- Don't silence the return code from tee_os.ObtainContents()
- Rename is_optee_bin()
- Improve various comments

Changes in v7:
- Correct missing test coverage

Changes in v6:
- Update op-tee to support new v1 binary header

 tools/binman/entries.rst                     | 37 ++++++++-
 tools/binman/entry.py                        | 13 +++
 tools/binman/etype/fit.py                    | 72 +++++++++--------
 tools/binman/etype/section.py                |  9 +++
 tools/binman/etype/tee_os.py                 | 76 +++++++++++++++++-
 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, 352 insertions(+), 33 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 5b9eb8b82c7..f6cc8003853 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -1508,12 +1508,47 @@ 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 an 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 does NOT 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 provided (see :ref:`etype_fit`), or produce a single node if the
+OP-TEE binary v1 format is provided (see optee_doc_) .
+
+.. _optee_doc: https://optee.readthedocs.io/en/latest/architecture/core.html#partitioning-of-the-binary
+
 
 
 .. _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..fea3adcc68d 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -540,41 +540,35 @@ 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 (list): list of segments, each:
+                    int: Segment number (0 = first)
+                    int: Start address of segment in memory
+                    bytes: Contents of segment
+                entry_addr (int): entry address of ELF file
             """
-            # 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 +592,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 section containing the
+                    contents of this node
             """
             oper = self._get_operation(base_node, node)
             if oper == OP_GEN_FDT_NODES:
@@ -609,10 +605,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..5529727e833 100644
--- a/tools/binman/etype/tee_os.py
+++ b/tools/binman/etype/tee_os.py
@@ -4,19 +4,93 @@
 # 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 an 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 does NOT 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 provided (see :ref:`etype_fit`), or produce a single node if the
+    OP-TEE binary v1 format is provided (see optee_doc_) .
+
+    .. _optee_doc: https://optee.readthedocs.io/en/latest/architecture/core.html#partitioning-of-the-binary
     """
     def __init__(self, section, etype, node):
         super().__init__(section, etype, node, 'tee-os')
         self.external = True
+
+    @staticmethod
+    def is_optee_bin_v1(data):
+        return len(data) >= 8 and data[0:5] == b'OPTE\x01'
+
+    def ObtainContents(self, fake_size=0):
+        result = super().ObtainContents(fake_size)
+        if not self.missing:
+            # If using the flat binary (without the OP-TEE header), then it is
+            # just included as a blob. But if it is an ELF or usees the v1
+            # binary header, then the FIT implementation will call
+            # read_elf_segments() to get the segment information
+            if elf.is_valid(self.data):
+                self.mark_absent('uses Elf format which must be in a FIT')
+            elif self.is_optee_bin_v1(self.data):
+                # The FIT implementation will call read_elf_segments() to get
+                # the segment information
+                self.mark_absent('uses v1 format which must be in a FIT')
+        return result
+
+    def read_elf_segments(self):
+        data = self.GetData()
+        if self.is_optee_bin_v1(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 {
+					};
+				};
+			};
+		};
+	};
+};
-- 
2.39.0.314.g84b9a713c41-goog