All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)
@ 2021-11-24  4:08 Simon Glass
  2021-11-24  4:08 ` [PATCH 1/2] binman: Add a utility module for ATF FIP Simon Glass
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Simon Glass @ 2021-11-24  4:08 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Ilias Apalodimas, Simon Glass


This series adds support for the FIP format as used by ARM Trusted
Firmware (in particular TF-A).

This allows images to be created containing a FIP, which itself contains
various binaries. With this, image creation can be handled from an in-tree
image description instead of needing to perform a lot of manual steps or
custom scripts to build the FIP.


Simon Glass (2):
  binman: Add a utility module for ATF FIP
  binman: Add support for ATF FIP

 scripts/pylint.base                      |   9 +-
 tools/binman/entries.rst                 | 154 ++++++
 tools/binman/etype/atf_fip.py            | 273 ++++++++++
 tools/binman/fip_util.py                 | 653 +++++++++++++++++++++++
 tools/binman/fip_util_test.py            | 405 ++++++++++++++
 tools/binman/ftest.py                    | 217 ++++++++
 tools/binman/main.py                     |   4 +-
 tools/binman/test/203_fip.dts            |  21 +
 tools/binman/test/204_fip_other.dts      |  22 +
 tools/binman/test/205_fip_no_type.dts    |  15 +
 tools/binman/test/206_fip_uuid.dts       |  22 +
 tools/binman/test/207_fip_ls.dts         |  25 +
 tools/binman/test/208_fip_replace.dts    |  33 ++
 tools/binman/test/209_fip_missing.dts    |  19 +
 tools/binman/test/210_fip_size.dts       |  19 +
 tools/binman/test/211_fip_bad_align.dts  |  18 +
 tools/binman/test/212_fip_collection.dts |  24 +
 17 files changed, 1929 insertions(+), 4 deletions(-)
 create mode 100644 tools/binman/etype/atf_fip.py
 create mode 100755 tools/binman/fip_util.py
 create mode 100755 tools/binman/fip_util_test.py
 create mode 100644 tools/binman/test/203_fip.dts
 create mode 100644 tools/binman/test/204_fip_other.dts
 create mode 100644 tools/binman/test/205_fip_no_type.dts
 create mode 100644 tools/binman/test/206_fip_uuid.dts
 create mode 100644 tools/binman/test/207_fip_ls.dts
 create mode 100644 tools/binman/test/208_fip_replace.dts
 create mode 100644 tools/binman/test/209_fip_missing.dts
 create mode 100644 tools/binman/test/210_fip_size.dts
 create mode 100644 tools/binman/test/211_fip_bad_align.dts
 create mode 100644 tools/binman/test/212_fip_collection.dts

-- 
2.34.0.rc2.393.gf8c9666880-goog


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/2] binman: Add a utility module for ATF FIP
  2021-11-24  4:08 [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP) Simon Glass
@ 2021-11-24  4:08 ` Simon Glass
  2021-11-25 14:47   ` Ilias Apalodimas
  2021-12-15  0:33   ` Simon Glass
  2021-11-24  4:08 ` [PATCH 2/2] binman: Add support " Simon Glass
  2021-11-25  9:42 ` [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP) Ilias Apalodimas
  2 siblings, 2 replies; 18+ messages in thread
From: Simon Glass @ 2021-11-24  4:08 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Ilias Apalodimas, Simon Glass

Add support for this format which is used by ARM Trusted Firmware to find
firmware binaries to load.

FIP is like a simpler version of FMAP but uses a UUID instead of a name,
for each entry.

It supports reading a FIP, writing a FIP and parsing the ATF source code
to get a list of supported UUIDs.

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

 scripts/pylint.base           |   8 +-
 tools/binman/fip_util.py      | 653 ++++++++++++++++++++++++++++++++++
 tools/binman/fip_util_test.py | 405 +++++++++++++++++++++
 tools/binman/main.py          |   4 +-
 4 files changed, 1066 insertions(+), 4 deletions(-)
 create mode 100755 tools/binman/fip_util.py
 create mode 100755 tools/binman/fip_util_test.py

diff --git a/scripts/pylint.base b/scripts/pylint.base
index 4e82dd4a616..a35dbe34d2d 100644
--- a/scripts/pylint.base
+++ b/scripts/pylint.base
@@ -9,11 +9,13 @@ binman.elf_test 5.41
 binman.entry 2.39
 binman.entry_test 5.29
 binman.fdt_test 3.23
+binman.fip_util 9.86
+binman.fip_util_test 9.75
 binman.fmap_util 6.67
 binman.ftest 7.38
 binman.image 6.39
 binman.image_test 4.48
-binman.main 4.26
+binman.main 4.03
 binman.setup 5.00
 binman.state 3.30
 blob -1.94
@@ -33,7 +35,7 @@ buildman.main 1.43
 buildman.test 6.10
 buildman.toolchain 5.81
 capsule_defs 5.00
-cbfs -1.52
+cbfs -1.44
 collection 2.33
 concurrencytest 6.77
 conftest -3.84
@@ -107,7 +109,7 @@ powerpc_mpc85xx_bootpg_resetvec -10.00
 rkmux 6.76
 rmboard 7.76
 scp -6.00
-section 4.25
+section 4.31
 sqfs_common 8.12
 test 8.18
 test_000_version 7.50
diff --git a/tools/binman/fip_util.py b/tools/binman/fip_util.py
new file mode 100755
index 00000000000..5f7dbc04d56
--- /dev/null
+++ b/tools/binman/fip_util.py
@@ -0,0 +1,653 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2021 Google LLC
+# Written by Simon Glass <sjg@chromium.org>
+
+"""Support for ARM's Firmware Image Package (FIP) format
+
+FIP is a format similar to FMAP[1] but with fewer features and an obscure UUID
+instead of the region name.
+
+It consists of a header and a table of entries, each pointing to a place in the
+firmware image where something can be found.
+
+[1] https://chromium.googlesource.com/chromiumos/third_party/flashmap/+/refs/heads/master/lib/fmap.h
+
+If ATF updates, run this program to update the FIT_TYPE_LIST.
+
+ARM Trusted Firmware is available at:
+
+https://github.com/ARM-software/arm-trusted-firmware.git
+"""
+
+from argparse import ArgumentParser
+import collections
+import io
+import os
+import re
+import struct
+import sys
+from uuid import UUID
+
+OUR_FILE = os.path.realpath(__file__)
+OUR_PATH = os.path.dirname(OUR_FILE)
+
+# Bring in the patman and dtoc libraries (but don't override the first path
+# in PYTHONPATH)
+sys.path.insert(2, os.path.join(OUR_PATH, '..'))
+
+# pylint: disable=C0413
+from patman import command
+from patman import tools
+
+# The TOC header, at the start of the FIP
+HEADER_FORMAT = '<IIQ'
+HEADER_LEN = 0x10
+HEADER_MAGIC = 0xaA640001
+HEADER_SERIAL = 0x12345678
+
+# The entry header (a table of these comes after the TOC header)
+UUID_LEN = 16
+ENTRY_FORMAT = f'<{UUID_LEN}sQQQ'
+ENTRY_SIZE = 0x28
+
+HEADER_NAMES = (
+    'name',
+    'serial',
+    'flags',
+)
+
+ENTRY_NAMES = (
+    'uuid',
+    'offset',
+    'size',
+    'flags',
+)
+
+# Set to True to enable output from running fiptool for debugging
+VERBOSE = False
+
+# Use a class so we can convert the bytes, making the table more readable
+# pylint: disable=R0903
+class FipType:
+    """A FIP entry type that we understand"""
+    def __init__(self, name, desc, uuid_bytes):
+        """Create up a new type
+
+        Args:
+            name (str): Short name for the type
+            desc (str): Longer description for the type
+            uuid_bytes (bytes): List of 16 bytes for the UUID
+        """
+        self.name = name
+        self.desc = desc
+        self.uuid = bytes(uuid_bytes)
+
+# This is taken from tbbr_config.c in ARM Trusted Firmware
+FIP_TYPE_LIST = [
+    # ToC Entry UUIDs
+    FipType('scp-fwu-cfg', 'SCP Firmware Updater Configuration FWU SCP_BL2U',
+            [0x65, 0x92, 0x27, 0x03, 0x2f, 0x74, 0xe6, 0x44,
+             0x8d, 0xff, 0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10]),
+    FipType('ap-fwu-cfg', 'AP Firmware Updater Configuration BL2U',
+            [0x60, 0xb3, 0xeb, 0x37, 0xc1, 0xe5, 0xea, 0x41,
+             0x9d, 0xf3, 0x19, 0xed, 0xa1, 0x1f, 0x68, 0x01]),
+    FipType('fwu', 'Firmware Updater NS_BL2U',
+            [0x4f, 0x51, 0x1d, 0x11, 0x2b, 0xe5, 0x4e, 0x49,
+             0xb4, 0xc5, 0x83, 0xc2, 0xf7, 0x15, 0x84, 0x0a]),
+    FipType('fwu-cert', 'Non-Trusted Firmware Updater certificate',
+            [0x71, 0x40, 0x8a, 0xb2, 0x18, 0xd6, 0x87, 0x4c,
+             0x8b, 0x2e, 0xc6, 0xdc, 0xcd, 0x50, 0xf0, 0x96]),
+    FipType('tb-fw', 'Trusted Boot Firmware BL2',
+            [0x5f, 0xf9, 0xec, 0x0b, 0x4d, 0x22, 0x3e, 0x4d,
+             0xa5, 0x44, 0xc3, 0x9d, 0x81, 0xc7, 0x3f, 0x0a]),
+    FipType('scp-fw', 'SCP Firmware SCP_BL2',
+            [0x97, 0x66, 0xfd, 0x3d, 0x89, 0xbe, 0xe8, 0x49,
+             0xae, 0x5d, 0x78, 0xa1, 0x40, 0x60, 0x82, 0x13]),
+    FipType('soc-fw', 'EL3 Runtime Firmware BL31',
+            [0x47, 0xd4, 0x08, 0x6d, 0x4c, 0xfe, 0x98, 0x46,
+             0x9b, 0x95, 0x29, 0x50, 0xcb, 0xbd, 0x5a, 0x00]),
+    FipType('tos-fw', 'Secure Payload BL32 (Trusted OS)',
+            [0x05, 0xd0, 0xe1, 0x89, 0x53, 0xdc, 0x13, 0x47,
+             0x8d, 0x2b, 0x50, 0x0a, 0x4b, 0x7a, 0x3e, 0x38]),
+    FipType('tos-fw-extra1', 'Secure Payload BL32 Extra1 (Trusted OS Extra1)',
+            [0x0b, 0x70, 0xc2, 0x9b, 0x2a, 0x5a, 0x78, 0x40,
+             0x9f, 0x65, 0x0a, 0x56, 0x82, 0x73, 0x82, 0x88]),
+    FipType('tos-fw-extra2', 'Secure Payload BL32 Extra2 (Trusted OS Extra2)',
+            [0x8e, 0xa8, 0x7b, 0xb1, 0xcf, 0xa2, 0x3f, 0x4d,
+             0x85, 0xfd, 0xe7, 0xbb, 0xa5, 0x02, 0x20, 0xd9]),
+    FipType('nt-fw', 'Non-Trusted Firmware BL33',
+            [0xd6, 0xd0, 0xee, 0xa7, 0xfc, 0xea, 0xd5, 0x4b,
+             0x97, 0x82, 0x99, 0x34, 0xf2, 0x34, 0xb6, 0xe4]),
+    FipType('rmm-fw', 'Realm Monitor Management Firmware',
+            [0x6c, 0x07, 0x62, 0xa6, 0x12, 0xf2, 0x4b, 0x56,
+             0x92, 0xcb, 0xba, 0x8f, 0x63, 0x36, 0x06, 0xd9]),
+    # Key certificates
+    FipType('rot-cert', 'Root Of Trust key certificate',
+            [0x86, 0x2d, 0x1d, 0x72, 0xf8, 0x60, 0xe4, 0x11,
+             0x92, 0x0b, 0x8b, 0xe7, 0x62, 0x16, 0x0f, 0x24]),
+    FipType('trusted-key-cert', 'Trusted key certificate',
+            [0x82, 0x7e, 0xe8, 0x90, 0xf8, 0x60, 0xe4, 0x11,
+             0xa1, 0xb4, 0x77, 0x7a, 0x21, 0xb4, 0xf9, 0x4c]),
+    FipType('scp-fw-key-cert', 'SCP Firmware key certificate',
+            [0x02, 0x42, 0x21, 0xa1, 0xf8, 0x60, 0xe4, 0x11,
+             0x8d, 0x9b, 0xf3, 0x3c, 0x0e, 0x15, 0xa0, 0x14]),
+    FipType('soc-fw-key-cert', 'SoC Firmware key certificate',
+            [0x8a, 0xb8, 0xbe, 0xcc, 0xf9, 0x60, 0xe4, 0x11,
+             0x9a, 0xd0, 0xeb, 0x48, 0x22, 0xd8, 0xdc, 0xf8]),
+    FipType('tos-fw-key-cert', 'Trusted OS Firmware key certificate',
+            [0x94, 0x77, 0xd6, 0x03, 0xfb, 0x60, 0xe4, 0x11,
+             0x85, 0xdd, 0xb7, 0x10, 0x5b, 0x8c, 0xee, 0x04]),
+    FipType('nt-fw-key-cert', 'Non-Trusted Firmware key certificate',
+            [0x8a, 0xd5, 0x83, 0x2a, 0xfb, 0x60, 0xe4, 0x11,
+             0x8a, 0xaf, 0xdf, 0x30, 0xbb, 0xc4, 0x98, 0x59]),
+    # Content certificates
+    FipType('tb-fw-cert', 'Trusted Boot Firmware BL2 certificate',
+            [0xd6, 0xe2, 0x69, 0xea, 0x5d, 0x63, 0xe4, 0x11,
+             0x8d, 0x8c, 0x9f, 0xba, 0xbe, 0x99, 0x56, 0xa5]),
+    FipType('scp-fw-cert', 'SCP Firmware content certificate',
+            [0x44, 0xbe, 0x6f, 0x04, 0x5e, 0x63, 0xe4, 0x11,
+             0xb2, 0x8b, 0x73, 0xd8, 0xea, 0xae, 0x96, 0x56]),
+    FipType('soc-fw-cert', 'SoC Firmware content certificate',
+            [0xe2, 0xb2, 0x0c, 0x20, 0x5e, 0x63, 0xe4, 0x11,
+             0x9c, 0xe8, 0xab, 0xcc, 0xf9, 0x2b, 0xb6, 0x66]),
+    FipType('tos-fw-cert', 'Trusted OS Firmware content certificate',
+            [0xa4, 0x9f, 0x44, 0x11, 0x5e, 0x63, 0xe4, 0x11,
+             0x87, 0x28, 0x3f, 0x05, 0x72, 0x2a, 0xf3, 0x3d]),
+    FipType('nt-fw-cert', 'Non-Trusted Firmware content certificate',
+            [0x8e, 0xc4, 0xc1, 0xf3, 0x5d, 0x63, 0xe4, 0x11,
+             0xa7, 0xa9, 0x87, 0xee, 0x40, 0xb2, 0x3f, 0xa7]),
+    FipType('sip-sp-cert', 'SiP owned Secure Partition content certificate',
+            [0x77, 0x6d, 0xfd, 0x44, 0x86, 0x97, 0x4c, 0x3b,
+             0x91, 0xeb, 0xc1, 0x3e, 0x02, 0x5a, 0x2a, 0x6f]),
+    FipType('plat-sp-cert', 'Platform owned Secure Partition content certificate',
+            [0xdd, 0xcb, 0xbf, 0x4a, 0xca, 0xd6, 0x11, 0xea,
+             0x87, 0xd0, 0x02, 0x42, 0xac, 0x13, 0x00, 0x03]),
+    # Dynamic configs
+    FipType('hw-config', 'HW_CONFIG',
+            [0x08, 0xb8, 0xf1, 0xd9, 0xc9, 0xcf, 0x93, 0x49,
+             0xa9, 0x62, 0x6f, 0xbc, 0x6b, 0x72, 0x65, 0xcc]),
+    FipType('tb-fw-config', 'TB_FW_CONFIG',
+            [0x6c, 0x04, 0x58, 0xff, 0xaf, 0x6b, 0x7d, 0x4f,
+             0x82, 0xed, 0xaa, 0x27, 0xbc, 0x69, 0xbf, 0xd2]),
+    FipType('soc-fw-config', 'SOC_FW_CONFIG',
+            [0x99, 0x79, 0x81, 0x4b, 0x03, 0x76, 0xfb, 0x46,
+             0x8c, 0x8e, 0x8d, 0x26, 0x7f, 0x78, 0x59, 0xe0]),
+    FipType('tos-fw-config', 'TOS_FW_CONFIG',
+            [0x26, 0x25, 0x7c, 0x1a, 0xdb, 0xc6, 0x7f, 0x47,
+             0x8d, 0x96, 0xc4, 0xc4, 0xb0, 0x24, 0x80, 0x21]),
+    FipType('nt-fw-config', 'NT_FW_CONFIG',
+            [0x28, 0xda, 0x98, 0x15, 0x93, 0xe8, 0x7e, 0x44,
+             0xac, 0x66, 0x1a, 0xaf, 0x80, 0x15, 0x50, 0xf9]),
+    FipType('fw-config', 'FW_CONFIG',
+            [0x58, 0x07, 0xe1, 0x6a, 0x84, 0x59, 0x47, 0xbe,
+             0x8e, 0xd5, 0x64, 0x8e, 0x8d, 0xdd, 0xab, 0x0e]),
+    ] # end
+
+FIP_TYPES = {ftype.name: ftype for ftype in FIP_TYPE_LIST}
+
+
+def get_type_uuid(fip_type_or_uuid):
+    """get_type_uuid() - Convert a type or uuid into both
+
+    This always returns a UUID, but may not return a type since it does not do
+    the reverse lookup.
+
+    Args:
+        fip_type_or_uuid (str or bytes): Either a string containing the name of
+            an entry (e.g. 'soc-fw') or a bytes(16) containing the UUID
+
+    Returns:
+        tuple:
+            str: fip type (None if not known)
+            bytes(16): uuid
+
+    Raises:
+        ValueError: An unknown type was requested
+    """
+    if isinstance(fip_type_or_uuid, str):
+        fip_type = fip_type_or_uuid
+        lookup = FIP_TYPES.get(fip_type)
+        if not lookup:
+            raise ValueError(f"Unknown FIP entry type '{fip_type}'")
+        uuid = lookup.uuid
+    else:
+        fip_type = None
+        uuid = fip_type_or_uuid
+    return fip_type, uuid
+
+
+# pylint: disable=R0903
+class FipHeader:
+    """Class to represent a FIP header"""
+    def __init__(self, name, serial, flags):
+        """Set up a new header object
+
+        Args:
+            name (str): Name, i.e. HEADER_MAGIC
+            serial (str): Serial value, i.e. HEADER_SERIAL
+            flags (int64): Flags value
+        """
+        self.name = name
+        self.serial = serial
+        self.flags = flags
+
+
+# pylint: disable=R0903
+class FipEntry:
+    """Class to represent a single FIP entry
+
+    This is used to hold the information about an entry, including its contents.
+    Use the get_data() method to obtain the raw output for writing to the FIP
+    file.
+    """
+    def __init__(self, uuid, offset, size, flags):
+        self.uuid = uuid
+        self.offset = offset
+        self.size = size
+        self.flags = flags
+        self.fip_type = None
+        self.data = None
+        self.valid = uuid != tools.GetBytes(0, UUID_LEN)
+        if self.valid:
+            # Look up the friendly name
+            matches = {val for (key, val) in FIP_TYPES.items()
+                       if val.uuid == uuid}
+            if len(matches) == 1:
+                self.fip_type = matches.pop().name
+
+    @classmethod
+    def from_type(cls, fip_type_or_uuid, data, flags):
+        """Create a FipEntry from a type name
+
+        Args:
+            cls (class): This class
+            fip_type_or_uuid (str or bytes): Name of the type to create, or
+                bytes(16) uuid
+            data (bytes): Contents of entry
+            flags (int64): Flags value
+
+        Returns:
+            FipEntry: Created 241
+        """
+        fip_type, uuid = get_type_uuid(fip_type_or_uuid)
+        fent = FipEntry(uuid, None, len(data), flags)
+        fent.fip_type = fip_type
+        fent.data = data
+        return fent
+
+
+def decode_fip(data):
+    """Decode a FIP into a header and list of FIP entries
+
+    Args:
+        data (bytes): Data block containing the FMAP
+
+    Returns:
+        Tuple:
+            header: FipHeader object
+            List of FipArea objects
+    """
+    fields = list(struct.unpack(HEADER_FORMAT, data[:HEADER_LEN]))
+    header = FipHeader(*fields)
+    fents = []
+    pos = HEADER_LEN
+    while True:
+        fields = list(struct.unpack(ENTRY_FORMAT, data[pos:pos + ENTRY_SIZE]))
+        fent = FipEntry(*fields)
+        if not fent.valid:
+            break
+        fent.data = data[fent.offset:fent.offset + fent.size]
+        fents.append(fent)
+        pos += ENTRY_SIZE
+    return header, fents
+
+
+class FipWriter:
+    """Class to handle writing a ARM Trusted Firmware's Firmware Image Package
+
+    Usage is something like:
+
+        fip = FipWriter(size)
+        fip.add_entry('scp-fwu-cfg', tools.ReadFile('something.bin'))
+        ...
+        data = cbw.get_data()
+
+    Attributes:
+    """
+    def __init__(self, flags, align):
+        self._fip_entries = []
+        self._flags = flags
+        self._align = align
+
+    def add_entry(self, fip_type, data, flags):
+        """Add a new entry to the FIP
+
+        Args:
+            fip_type (str): Type to add, e.g. 'tos-fw-config'
+            data (bytes): Contents of entry
+            flags (int64): Entry flags
+
+        Returns:
+            FipEntry: entry that was added
+        """
+        fent = FipEntry.from_type(fip_type, data, flags)
+        self._fip_entries.append(fent)
+        return fent
+
+    def get_data(self):
+        """Obtain the full contents of the FIP
+
+        Thhis builds the FIP with headers and all required FIP entries.
+
+        Returns:
+            bytes: data resulting from building the FIP
+        """
+        buf = io.BytesIO()
+        hdr = struct.pack(HEADER_FORMAT, HEADER_MAGIC, HEADER_SERIAL,
+                          self._flags)
+        buf.write(hdr)
+
+        # Calculate the position fo the first entry
+        offset = len(hdr)
+        offset += len(self._fip_entries) * ENTRY_SIZE
+        offset += ENTRY_SIZE   # terminating entry
+
+        for fent in self._fip_entries:
+            offset = tools.Align(offset, self._align)
+            fent.offset = offset
+            offset += fent.size
+
+        # Write out the TOC
+        for fent in self._fip_entries:
+            hdr = struct.pack(ENTRY_FORMAT, fent.uuid, fent.offset, fent.size,
+                              fent.flags)
+            buf.write(hdr)
+
+        # Write out the entries
+        for fent in self._fip_entries:
+            buf.seek(fent.offset)
+            buf.write(fent.data)
+
+        return buf.getvalue()
+
+
+class FipReader():
+    """Class to handle reading a Firmware Image Package (FIP)
+
+    Usage is something like:
+        fip = fip_util.FipReader(data)
+        fent = fip.get_entry('fwu')
+        self.WriteFile('ufwu.bin', fent.data)
+        blob = fip.get_entry(
+            bytes([0xe3, 0xb7, 0x8d, 0x9e, 0x4a, 0x64, 0x11, 0xec,
+                   0xb4, 0x5c, 0xfb, 0xa2, 0xb9, 0xb4, 0x97, 0x88]))
+        self.WriteFile('blob.bin', blob.data)
+    """
+    def __init__(self, data, read=True):
+        """Set up a new FitReader
+
+        Args:
+            data (bytes): data to read
+            read (bool): True to read the data now
+        """
+        self.fents = collections.OrderedDict()
+        self.data = data
+        if read:
+            self.read()
+
+    def read(self):
+        """Read all the files in the FIP and add them to self.files"""
+        self.header, self.fents = decode_fip(self.data)
+
+    def get_entry(self, fip_type_or_uuid):
+        """get_entry() - Find an entry by type or UUID
+
+        Args:
+            fip_type_or_uuid (str or bytes): Name of the type to create, or
+                    bytes(16) uuid
+
+        Returns:
+            FipEntry: if found
+
+        Raises:
+            ValueError: entry type not found
+        """
+        fip_type, uuid = get_type_uuid(fip_type_or_uuid)
+        for fent in self.fents:
+            if fent.uuid == uuid:
+                return fent
+        label = fip_type
+        if not label:
+            label = UUID(bytes=uuid)
+        raise ValueError(f"Cannot find FIP entry '{label}'")
+
+
+def parse_macros(srcdir):
+    """parse_macros: Parse the firmware_image_package.h file
+
+    Args:
+        srcdir (str): 'arm-trusted-firmware' source directory
+
+    Returns:
+        dict:
+            key: UUID macro name, e.g. 'UUID_TRUSTED_FWU_CERT'
+            value: list:
+                file comment, e.g. 'ToC Entry UUIDs'
+                macro name, e.g. 'UUID_TRUSTED_FWU_CERT'
+                uuid as bytes(16)
+
+    Raises:
+        ValueError: a line cannot be parsed
+    """
+    re_uuid = re.compile('0x[0-9a-fA-F]{2}')
+    re_comment = re.compile(r'^/\* (.*) \*/$')
+    fname = os.path.join(srcdir, 'include/tools_share/firmware_image_package.h')
+    data = tools.ReadFile(fname, binary=False)
+    macros = collections.OrderedDict()
+    comment = None
+    for linenum, line in enumerate(data.splitlines()):
+        if line.startswith('/*'):
+            mat = re_comment.match(line)
+            if mat:
+                comment = mat.group(1)
+        else:
+            # Example: #define UUID_TOS_FW_CONFIG \
+            if 'UUID' in line:
+                macro = line.split()[1]
+            elif '{{' in line:
+                mat = re_uuid.findall(line)
+                if not mat or len(mat) != 16:
+                    raise ValueError(
+                        f'{fname}: Cannot parse UUID line {linenum + 1}: Got matches: {mat}')
+
+                uuid = bytes([int(val, 16) for val in mat])
+                macros[macro] = comment, macro, uuid
+    if not macros:
+        raise ValueError(f'{fname}: Cannot parse file')
+    return macros
+
+
+def parse_names(srcdir):
+    """parse_names: Parse the tbbr_config.c file
+
+    Args:
+        srcdir (str): 'arm-trusted-firmware' source directory
+
+    Returns:
+        tuple: dict of entries:
+            key: UUID macro, e.g. 'UUID_NON_TRUSTED_FIRMWARE_BL33'
+            tuple: entry information
+                Description of entry, e.g. 'Non-Trusted Firmware BL33'
+                UUID macro, e.g. 'UUID_NON_TRUSTED_FIRMWARE_BL33'
+                Name of entry, e.g. 'nt-fw'
+
+    Raises:
+        ValueError: the file cannot be parsed
+    """
+    # Extract the .name, .uuid and .cmdline_name values
+    re_data = re.compile(r'\.name = "([^"]*)",\s*\.uuid = (UUID_\w*),\s*\.cmdline_name = "([^"]+)"',
+                         re.S)
+    fname = os.path.join(srcdir, 'tools/fiptool/tbbr_config.c')
+    data = tools.ReadFile(fname, binary=False)
+
+    # Example entry:
+    #   {
+    #       .name = "Secure Payload BL32 Extra2 (Trusted OS Extra2)",
+    #       .uuid = UUID_SECURE_PAYLOAD_BL32_EXTRA2,
+    #       .cmdline_name = "tos-fw-extra2"
+    #   },
+    mat = re_data.findall(data)
+    if not mat:
+        raise ValueError(f'{fname}: Cannot parse file')
+    names = {uuid: (desc, uuid, name) for desc, uuid, name in mat}
+    return names
+
+
+def create_code_output(macros, names):
+    """create_code_output() - Create the new version of this Python file
+
+    Args:
+        macros (dict):
+            key (str): UUID macro name, e.g. 'UUID_TRUSTED_FWU_CERT'
+            value: list:
+                file comment, e.g. 'ToC Entry UUIDs'
+                macro name, e.g. 'UUID_TRUSTED_FWU_CERT'
+                uuid as bytes(16)
+
+        names (dict): list of entries, each
+            tuple: entry information
+                Description of entry, e.g. 'Non-Trusted Firmware BL33'
+                UUID macro, e.g. 'UUID_NON_TRUSTED_FIRMWARE_BL33'
+                Name of entry, e.g. 'nt-fw'
+
+    Returns:
+        str: Table of FipType() entries
+    """
+    def _to_hex_list(data):
+        """Convert bytes into C code
+
+        Args:
+            bytes to convert
+
+        Returns:
+            str: in the format '0x12, 0x34, 0x56...'
+        """
+        # Use 0x instead of %# since the latter ignores the 0 modifier in
+        # Python 3.8.10
+        return ', '.join(['0x%02x' % byte for byte in data])
+
+    out = ''
+    last_comment = None
+    for comment, macro, uuid in macros.values():
+        name_entry = names.get(macro)
+        if not name_entry:
+            print(f"Warning: UUID '{macro}' is not mentioned in tbbr_config.c file")
+            continue
+        desc, _, name = name_entry
+        if last_comment != comment:
+            out += f'    # {comment}\n'
+            last_comment = comment
+        out += """    FipType('%s', '%s',
+            [%s,
+             %s]),
+""" % (name, desc, _to_hex_list(uuid[:8]), _to_hex_list(uuid[8:]))
+    return out
+
+
+def parse_atf_source(srcdir, dstfile, oldfile):
+    """parse_atf_source(): Parse the ATF source tree and update this file
+
+    Args:
+        srcdir (str): Path to 'arm-trusted-firmware' directory. Get this from:
+            https://github.com/ARM-software/arm-trusted-firmware.git
+        dstfile (str): File to write new code to, if an update is needed
+        oldfile (str): Python source file to compare against
+
+    Raises:
+        ValueError: srcdir readme.rst is missing or the first line does not
+            match what is expected
+    """
+    # We expect a readme file
+    readme_fname = os.path.join(srcdir, 'readme.rst')
+    if not os.path.exists(readme_fname):
+        raise ValueError(
+            f"Expected file '{readme_fname}' - try using -s to specify the "
+            'arm-trusted-firmware directory')
+    readme = tools.ReadFile(readme_fname, binary=False)
+    first_line = 'Trusted Firmware-A'
+    if readme.splitlines()[0] != first_line:
+        raise ValueError(f"'{readme_fname}' does not start with '{first_line}'")
+    macros = parse_macros(srcdir)
+    names = parse_names(srcdir)
+    output = create_code_output(macros, names)
+    orig = tools.ReadFile(oldfile, binary=False)
+    re_fip_list = re.compile(r'(.*FIP_TYPE_LIST = \[).*?(    ] # end.*)', re.S)
+    mat = re_fip_list.match(orig)
+    new_code = mat.group(1) + '\n' + output + mat.group(2) if mat else output
+    if new_code == orig:
+        print(f"Existing code in '{oldfile}' is up-to-date")
+    else:
+        tools.WriteFile(dstfile, new_code, binary=False)
+        print(f'Needs update, try:\n\tmeld {dstfile} {oldfile}')
+
+
+def main(argv, oldfile):
+    """Main program for this tool
+
+    Args:
+        argv (list): List of str command-line arguments
+        oldfile (str): Python source file to compare against
+
+    Returns:
+        int: 0 (exit code)
+    """
+    parser = ArgumentParser(epilog='''Creates an updated version of this code,
+with a table of FIP-entry types parsed from the arm-trusted-firmware source
+directory''')
+    parser.add_argument(
+        '-D', '--debug', action='store_true',
+        help='Enabling debugging (provides a full traceback on error)')
+    parser.add_argument(
+        '-o', '--outfile', type=str, default='fip_util.py.out',
+        help='Output file to write new fip_util.py file to')
+    parser.add_argument(
+        '-s', '--src', type=str, default='.',
+        help='Directory containing the arm-trusted-firmware source')
+    args = parser.parse_args(argv)
+
+    if not args.debug:
+        sys.tracebacklimit = 0
+
+    parse_atf_source(args.src, args.outfile, oldfile)
+    return 0
+
+
+def fiptool(fname, *fip_args):
+    """Run fiptool with provided arguments
+
+    If the tool fails then this function raises an exception and prints out the
+    output and stderr.
+
+    Args:
+        fname (str): Filename of FIP
+        *fip_args: List of arguments to pass to fiptool
+
+    Returns:
+        CommandResult: object containing the results
+
+    Raises:
+        ValueError: the tool failed to run
+    """
+    args = ['fiptool', fname] + list(fip_args)
+    result = command.RunPipe([args], capture=not VERBOSE,
+                             capture_stderr=not VERBOSE, raise_on_error=False)
+    if result.return_code:
+        print(result.stderr, file=sys.stderr)
+        raise ValueError("Failed to run (error %d): '%s'" %
+                         (result.return_code, ' '.join(args)))
+    return result
+
+
+if __name__ == "__main__":
+    sys.exit(main(sys.argv[1:], OUR_FILE))  # pragma: no cover
diff --git a/tools/binman/fip_util_test.py b/tools/binman/fip_util_test.py
new file mode 100755
index 00000000000..4839b298762
--- /dev/null
+++ b/tools/binman/fip_util_test.py
@@ -0,0 +1,405 @@
+#!/usr/bin/env python3
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2021 Google LLC
+# Written by Simon Glass <sjg@chromium.org>
+
+"""Tests for fip_util
+
+This tests a few features of fip_util which are not covered by binman's ftest.py
+"""
+
+import os
+import shutil
+import sys
+import tempfile
+import unittest
+
+# Bring in the patman and dtoc libraries (but don't override the first path
+# in PYTHONPATH)
+OUR_PATH = os.path.dirname(os.path.realpath(__file__))
+sys.path.insert(2, os.path.join(OUR_PATH, '..'))
+
+# pylint: disable=C0413
+from patman import test_util
+from patman import tools
+import fip_util
+
+HAVE_FIPTOOL = True
+try:
+    tools.Run('which', 'fiptool')
+except ValueError:
+    HAVE_FIPTOOL = False
+
+# pylint: disable=R0902,R0904
+class TestFip(unittest.TestCase):
+    """Test of fip_util classes"""
+    #pylint: disable=W0212
+    def setUp(self):
+        # Create a temporary directory for test files
+        self._indir = tempfile.mkdtemp(prefix='fip_util.')
+        tools.SetInputDirs([self._indir])
+
+        # Set up a temporary output directory, used by the tools library when
+        # compressing files
+        tools.PrepareOutputDir(None)
+
+        self.src_file = os.path.join(self._indir, 'orig.py')
+        self.outname = tools.GetOutputFilename('out.py')
+        self.args = ['-D', '-s', self._indir, '-o', self.outname]
+        self.readme = os.path.join(self._indir, 'readme.rst')
+        self.macro_dir = os.path.join(self._indir, 'include/tools_share')
+        self.macro_fname = os.path.join(self.macro_dir,
+                                        'firmware_image_package.h')
+        self.name_dir = os.path.join(self._indir, 'tools/fiptool')
+        self.name_fname = os.path.join(self.name_dir, 'tbbr_config.c')
+
+    macro_contents = '''
+
+/* ToC Entry UUIDs */
+#define UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U \\
+	{{0x65, 0x92, 0x27, 0x03}, {0x2f, 0x74}, {0xe6, 0x44}, 0x8d, 0xff, {0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10} }
+#define UUID_TRUSTED_UPDATE_FIRMWARE_BL2U \\
+	{{0x60, 0xb3, 0xeb, 0x37}, {0xc1, 0xe5}, {0xea, 0x41}, 0x9d, 0xf3, {0x19, 0xed, 0xa1, 0x1f, 0x68, 0x01} }
+
+'''
+
+    name_contents = '''
+
+toc_entry_t toc_entries[] = {
+	{
+		.name = "SCP Firmware Updater Configuration FWU SCP_BL2U",
+		.uuid = UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U,
+		.cmdline_name = "scp-fwu-cfg"
+	},
+	{
+		.name = "AP Firmware Updater Configuration BL2U",
+		.uuid = UUID_TRUSTED_UPDATE_FIRMWARE_BL2U,
+		.cmdline_name = "ap-fwu-cfg"
+	},
+'''
+
+    def setup_readme(self):
+        """Set up the readme.txt file"""
+        tools.WriteFile(self.readme, 'Trusted Firmware-A\n==================',
+                        binary=False)
+
+    def setup_macro(self, data=macro_contents):
+        """Set up the tbbr_config.c file"""
+        os.makedirs(self.macro_dir)
+        tools.WriteFile(self.macro_fname, data, binary=False)
+
+    def setup_name(self, data=name_contents):
+        """Set up the firmware_image_package.h file"""
+        os.makedirs(self.name_dir)
+        tools.WriteFile(self.name_fname, data, binary=False)
+
+    def tearDown(self):
+        """Remove the temporary input directory and its contents"""
+        if self._indir:
+            shutil.rmtree(self._indir)
+        self._indir = None
+        tools.FinaliseOutputDir()
+
+    def test_no_readme(self):
+        """Test handling of a missing readme.rst"""
+        with self.assertRaises(Exception) as err:
+            fip_util.main(self.args, self.src_file)
+        self.assertIn('Expected file', str(err.exception))
+
+    def test_invalid_readme(self):
+        """Test that an invalid readme.rst is detected"""
+        tools.WriteFile(self.readme, 'blah', binary=False)
+        with self.assertRaises(Exception) as err:
+            fip_util.main(self.args, self.src_file)
+        self.assertIn('does not start with', str(err.exception))
+
+    def test_no_fip_h(self):
+        """Check handling of missing firmware_image_package.h"""
+        self.setup_readme()
+        with self.assertRaises(Exception) as err:
+            fip_util.main(self.args, self.src_file)
+        self.assertIn('No such file or directory', str(err.exception))
+
+    def test_invalid_fip_h(self):
+        """Check failure to parse firmware_image_package.h"""
+        self.setup_readme()
+        self.setup_macro('blah')
+        with self.assertRaises(Exception) as err:
+            fip_util.main(self.args, self.src_file)
+        self.assertIn('Cannot parse file', str(err.exception))
+
+    def test_parse_fip_h(self):
+        """Check parsing of firmware_image_package.h"""
+        self.setup_readme()
+        # Check parsing the header file
+        self.setup_macro()
+        macros = fip_util.parse_macros(self._indir)
+        expected_macros = {
+            'UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U':
+                ('ToC Entry UUIDs', 'UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U',
+                 bytes([0x65, 0x92, 0x27, 0x03, 0x2f, 0x74, 0xe6, 0x44,
+                        0x8d, 0xff, 0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10])),
+            'UUID_TRUSTED_UPDATE_FIRMWARE_BL2U':
+                ('ToC Entry UUIDs', 'UUID_TRUSTED_UPDATE_FIRMWARE_BL2U',
+                 bytes([0x60, 0xb3, 0xeb, 0x37, 0xc1, 0xe5, 0xea, 0x41,
+                        0x9d, 0xf3, 0x19, 0xed, 0xa1, 0x1f, 0x68, 0x01])),
+            }
+        self.assertEqual(expected_macros, macros)
+
+    def test_missing_tbbr_c(self):
+        """Check handlinh of missing tbbr_config.c"""
+        self.setup_readme()
+        self.setup_macro()
+
+        # Still need the .c file
+        with self.assertRaises(Exception) as err:
+            fip_util.main(self.args, self.src_file)
+        self.assertIn('tbbr_config.c', str(err.exception))
+
+    def test_invalid_tbbr_c(self):
+        """Check failure to parse tbbr_config.c"""
+        self.setup_readme()
+        self.setup_macro()
+        # Check invalid format for C file
+        self.setup_name('blah')
+        with self.assertRaises(Exception) as err:
+            fip_util.main(self.args, self.src_file)
+        self.assertIn('Cannot parse file', str(err.exception))
+
+    def test_inconsistent_tbbr_c(self):
+        """Check tbbr_config.c in a format we don't expect"""
+        self.setup_readme()
+        # This is missing a hex value
+        self.setup_macro('''
+
+/* ToC Entry UUIDs */
+#define UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U \\
+	{{0x65, 0x92, 0x27,}, {0x2f, 0x74}, {0xe6, 0x44}, 0x8d, 0xff, {0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10} }
+#define UUID_TRUSTED_UPDATE_FIRMWARE_BL2U \\
+	{{0x60, 0xb3, 0xeb, 0x37}, {0xc1, 0xe5}, {0xea, 0x41}, 0x9d, 0xf3, {0x19, 0xed, 0xa1, 0x1f, 0x68, 0x01} }
+
+''')
+        # Check invalid format for C file
+        self.setup_name('blah')
+        with self.assertRaises(Exception) as err:
+            fip_util.main(self.args, self.src_file)
+        self.assertIn('Cannot parse UUID line 5', str(err.exception))
+
+    def test_parse_tbbr_c(self):
+        """Check parsing tbbr_config.c"""
+        self.setup_readme()
+        self.setup_macro()
+        self.setup_name()
+
+        names = fip_util.parse_names(self._indir)
+
+        expected_names = {
+            'UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U': (
+                'SCP Firmware Updater Configuration FWU SCP_BL2U',
+                'UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U',
+                'scp-fwu-cfg'),
+            'UUID_TRUSTED_UPDATE_FIRMWARE_BL2U': (
+                'AP Firmware Updater Configuration BL2U',
+                'UUID_TRUSTED_UPDATE_FIRMWARE_BL2U',
+                'ap-fwu-cfg'),
+            }
+        self.assertEqual(expected_names, names)
+
+    def test_uuid_not_in_tbbr_config_c(self):
+        """Check handling a UUID in the header file that's not in the .c file"""
+        self.setup_readme()
+        self.setup_macro(self.macro_contents + '''
+#define UUID_TRUSTED_OS_FW_KEY_CERT \\
+	{{0x94,  0x77, 0xd6, 0x03}, {0xfb, 0x60}, {0xe4, 0x11}, 0x85, 0xdd, {0xb7, 0x10, 0x5b, 0x8c, 0xee, 0x04} }
+
+''')
+        self.setup_name()
+
+        macros = fip_util.parse_macros(self._indir)
+        names = fip_util.parse_names(self._indir)
+        with test_util.capture_sys_output() as (stdout, _):
+            fip_util.create_code_output(macros, names)
+        self.assertIn(
+            "UUID 'UUID_TRUSTED_OS_FW_KEY_CERT' is not mentioned in tbbr_config.c file",
+            stdout.getvalue())
+
+    def test_changes(self):
+        """Check handling of a source file that does/doesn't need changes"""
+        self.setup_readme()
+        self.setup_macro()
+        self.setup_name()
+
+        # Check generating the file when changes are needed
+        tools.WriteFile(self.src_file, '''
+
+# This is taken from tbbr_config.c in ARM Trusted Firmware
+FIP_TYPE_LIST = [
+    # ToC Entry UUIDs
+    FipType('scp-fwu-cfg', 'SCP Firmware Updater Configuration FWU SCP_BL2U',
+            [0x65, 0x92, 0x27, 0x03, 0x2f, 0x74, 0xe6, 0x44,
+             0x8d, 0xff, 0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10]),
+    ] # end
+blah de blah
+                        ''', binary=False)
+        with test_util.capture_sys_output() as (stdout, _):
+            fip_util.main(self.args, self.src_file)
+        self.assertIn('Needs update', stdout.getvalue())
+
+        # Check generating the file when no changes are needed
+        tools.WriteFile(self.src_file, '''
+# This is taken from tbbr_config.c in ARM Trusted Firmware
+FIP_TYPE_LIST = [
+    # ToC Entry UUIDs
+    FipType('scp-fwu-cfg', 'SCP Firmware Updater Configuration FWU SCP_BL2U',
+            [0x65, 0x92, 0x27, 0x03, 0x2f, 0x74, 0xe6, 0x44,
+             0x8d, 0xff, 0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10]),
+    FipType('ap-fwu-cfg', 'AP Firmware Updater Configuration BL2U',
+            [0x60, 0xb3, 0xeb, 0x37, 0xc1, 0xe5, 0xea, 0x41,
+             0x9d, 0xf3, 0x19, 0xed, 0xa1, 0x1f, 0x68, 0x01]),
+    ] # end
+blah blah''', binary=False)
+        with test_util.capture_sys_output() as (stdout, _):
+            fip_util.main(self.args, self.src_file)
+        self.assertIn('is up-to-date', stdout.getvalue())
+
+    def test_no_debug(self):
+        """Test running without the -D flag"""
+        self.setup_readme()
+        self.setup_macro()
+        self.setup_name()
+
+        args = self.args.copy()
+        args.remove('-D')
+        tools.WriteFile(self.src_file, '', binary=False)
+        with test_util.capture_sys_output():
+            fip_util.main(args, self.src_file)
+
+    @unittest.skipIf(not HAVE_FIPTOOL, 'No fiptool available')
+    def test_fiptool_list(self):
+        """Create a FIP and check that fiptool can read it"""
+        fwu = b'my data'
+        tb_fw = b'some more data'
+        fip = fip_util.FipWriter(0x123, 0x10)
+        fip.add_entry('fwu', fwu, 0x456)
+        fip.add_entry('tb-fw', tb_fw, 0)
+        fip.add_entry(bytes(range(16)), tb_fw, 0)
+        data = fip.get_data()
+        fname = tools.GetOutputFilename('data.fip')
+        tools.WriteFile(fname, data)
+        result = fip_util.fiptool('info', fname)
+        self.assertEqual(
+            '''Firmware Updater NS_BL2U: offset=0xB0, size=0x7, cmdline="--fwu"
+Trusted Boot Firmware BL2: offset=0xC0, size=0xE, cmdline="--tb-fw"
+00010203-0405-0607-0809-0A0B0C0D0E0F: offset=0xD0, size=0xE, cmdline="--blob"
+''',
+            result.stdout)
+
+    fwu_data = b'my data'
+    tb_fw_data = b'some more data'
+    other_fw_data = b'even more'
+
+    def create_fiptool_image(self):
+        """Create an image with fiptool which we can use for testing
+
+        Returns:
+            FipReader: reader for the image
+        """
+        fwu = os.path.join(self._indir, 'fwu')
+        tools.WriteFile(fwu, self.fwu_data)
+
+        tb_fw = os.path.join(self._indir, 'tb_fw')
+        tools.WriteFile(tb_fw, self.tb_fw_data)
+
+        other_fw = os.path.join(self._indir, 'other_fw')
+        tools.WriteFile(other_fw, self.other_fw_data)
+
+        fname = tools.GetOutputFilename('data.fip')
+        uuid = 'e3b78d9e-4a64-11ec-b45c-fba2b9b49788'
+        fip_util.fiptool('create', '--align', '8', '--plat-toc-flags', '0x123',
+                         '--fwu', fwu,
+                         '--tb-fw', tb_fw,
+                         '--blob', f'uuid={uuid},file={other_fw}',
+                          fname)
+
+        return fip_util.FipReader(tools.ReadFile(fname))
+
+    @unittest.skipIf(not HAVE_FIPTOOL, 'No fiptool available')
+    def test_fiptool_create(self):
+        """Create a FIP with fiptool and check that fip_util can read it"""
+        reader = self.create_fiptool_image()
+
+        header = reader.header
+        fents = reader.fents
+
+        self.assertEqual(0x123 << 32, header.flags)
+        self.assertEqual(fip_util.HEADER_MAGIC, header.name)
+        self.assertEqual(fip_util.HEADER_SERIAL, header.serial)
+
+        self.assertEqual(3, len(fents))
+        fent = fents[0]
+        self.assertEqual(
+            bytes([0x4f, 0x51, 0x1d, 0x11, 0x2b, 0xe5, 0x4e, 0x49,
+                   0xb4, 0xc5, 0x83, 0xc2, 0xf7, 0x15, 0x84, 0x0a]), fent.uuid)
+        self.assertEqual(0xb0, fent.offset)
+        self.assertEqual(len(self.fwu_data), fent.size)
+        self.assertEqual(0, fent.flags)
+        self.assertEqual(self.fwu_data, fent.data)
+
+        fent = fents[1]
+        self.assertEqual(
+            bytes([0x5f, 0xf9, 0xec, 0x0b, 0x4d, 0x22, 0x3e, 0x4d,
+             0xa5, 0x44, 0xc3, 0x9d, 0x81, 0xc7, 0x3f, 0x0a]), fent.uuid)
+        self.assertEqual(0xb8, fent.offset)
+        self.assertEqual(len(self.tb_fw_data), fent.size)
+        self.assertEqual(0, fent.flags)
+        self.assertEqual(self.tb_fw_data, fent.data)
+
+        fent = fents[2]
+        self.assertEqual(
+            bytes([0xe3, 0xb7, 0x8d, 0x9e, 0x4a, 0x64, 0x11, 0xec,
+                   0xb4, 0x5c, 0xfb, 0xa2, 0xb9, 0xb4, 0x97, 0x88]), fent.uuid)
+        self.assertEqual(0xc8, fent.offset)
+        self.assertEqual(len(self.other_fw_data), fent.size)
+        self.assertEqual(0, fent.flags)
+        self.assertEqual(self.other_fw_data, fent.data)
+
+    @unittest.skipIf(not HAVE_FIPTOOL, 'No fiptool available')
+    def test_reader_get_entry(self):
+        """Test get_entry() by name and UUID"""
+        reader = self.create_fiptool_image()
+        fents = reader.fents
+        fent = reader.get_entry('fwu')
+        self.assertEqual(fent, fents[0])
+
+        fent = reader.get_entry(
+            bytes([0x5f, 0xf9, 0xec, 0x0b, 0x4d, 0x22, 0x3e, 0x4d,
+                   0xa5, 0x44, 0xc3, 0x9d, 0x81, 0xc7, 0x3f, 0x0a]))
+        self.assertEqual(fent, fents[1])
+
+        # Try finding entries that don't exist
+        with self.assertRaises(Exception) as err:
+            fent = reader.get_entry('scp-fwu-cfg')
+        self.assertIn("Cannot find FIP entry 'scp-fwu-cfg'", str(err.exception))
+
+        with self.assertRaises(Exception) as err:
+            fent = reader.get_entry(bytes(list(range(16))))
+        self.assertIn(
+            "Cannot find FIP entry '00010203-0405-0607-0809-0a0b0c0d0e0f'",
+            str(err.exception))
+
+        with self.assertRaises(Exception) as err:
+            fent = reader.get_entry('blah')
+        self.assertIn("Unknown FIP entry type 'blah'", str(err.exception))
+
+    @unittest.skipIf(not HAVE_FIPTOOL, 'No fiptool available')
+    def test_fiptool_errors(self):
+        """Check some error reporting from fiptool"""
+        with self.assertRaises(Exception) as err:
+            with test_util.capture_sys_output():
+                fip_util.fiptool('create', '--fred')
+        self.assertIn("Failed to run (error 1): 'fiptool create --fred'",
+                      str(err.exception))
+
+
+if __name__ == '__main__':
+    unittest.main()
diff --git a/tools/binman/main.py b/tools/binman/main.py
index 8c1e478d54c..1a639f43e9e 100755
--- a/tools/binman/main.py
+++ b/tools/binman/main.py
@@ -59,6 +59,7 @@ def RunTests(debug, verbosity, processes, test_preserve_dirs, args, toolpath):
     from binman import elf_test
     from binman import entry_test
     from binman import fdt_test
+    from binman import fip_util_test
     from binman import ftest
     from binman import image_test
     import doctest
@@ -72,7 +73,8 @@ def RunTests(debug, verbosity, processes, test_preserve_dirs, args, toolpath):
         result, debug, verbosity, test_preserve_dirs, processes, test_name,
         toolpath,
         [entry_test.TestEntry, ftest.TestFunctional, fdt_test.TestFdt,
-         elf_test.TestElf, image_test.TestImage, cbfs_util_test.TestCbfs])
+         elf_test.TestElf, image_test.TestImage, cbfs_util_test.TestCbfs,
+         fip_util_test.TestFip])
 
     return test_util.ReportResult('binman', test_name, result)
 
-- 
2.34.0.rc2.393.gf8c9666880-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/2] binman: Add support for ATF FIP
  2021-11-24  4:08 [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP) Simon Glass
  2021-11-24  4:08 ` [PATCH 1/2] binman: Add a utility module for ATF FIP Simon Glass
@ 2021-11-24  4:08 ` Simon Glass
  2021-11-25 14:47   ` Ilias Apalodimas
  2021-12-15  0:33   ` Simon Glass
  2021-11-25  9:42 ` [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP) Ilias Apalodimas
  2 siblings, 2 replies; 18+ messages in thread
From: Simon Glass @ 2021-11-24  4:08 UTC (permalink / raw)
  To: U-Boot Mailing List; +Cc: Ilias Apalodimas, Simon Glass

This format is used in firmware binaries so we may as well supported it.

With this patch binman supports creating, listing and updating FIPs, as
well as extracting files from one, provided that an FDTMAP is also present
somewhere in the image.

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

 scripts/pylint.base                      |   1 +
 tools/binman/entries.rst                 | 154 +++++++++++++
 tools/binman/etype/atf_fip.py            | 273 +++++++++++++++++++++++
 tools/binman/ftest.py                    | 217 ++++++++++++++++++
 tools/binman/test/203_fip.dts            |  21 ++
 tools/binman/test/204_fip_other.dts      |  22 ++
 tools/binman/test/205_fip_no_type.dts    |  15 ++
 tools/binman/test/206_fip_uuid.dts       |  22 ++
 tools/binman/test/207_fip_ls.dts         |  25 +++
 tools/binman/test/208_fip_replace.dts    |  33 +++
 tools/binman/test/209_fip_missing.dts    |  19 ++
 tools/binman/test/210_fip_size.dts       |  19 ++
 tools/binman/test/211_fip_bad_align.dts  |  18 ++
 tools/binman/test/212_fip_collection.dts |  24 ++
 14 files changed, 863 insertions(+)
 create mode 100644 tools/binman/etype/atf_fip.py
 create mode 100644 tools/binman/test/203_fip.dts
 create mode 100644 tools/binman/test/204_fip_other.dts
 create mode 100644 tools/binman/test/205_fip_no_type.dts
 create mode 100644 tools/binman/test/206_fip_uuid.dts
 create mode 100644 tools/binman/test/207_fip_ls.dts
 create mode 100644 tools/binman/test/208_fip_replace.dts
 create mode 100644 tools/binman/test/209_fip_missing.dts
 create mode 100644 tools/binman/test/210_fip_size.dts
 create mode 100644 tools/binman/test/211_fip_bad_align.dts
 create mode 100644 tools/binman/test/212_fip_collection.dts

diff --git a/scripts/pylint.base b/scripts/pylint.base
index a35dbe34d2d..3d891edf261 100644
--- a/scripts/pylint.base
+++ b/scripts/pylint.base
@@ -1,5 +1,6 @@
 _testing 0.83
 atf_bl31 -6.00
+atf_fip 0.44
 binman.cbfs_util 7.70
 binman.cbfs_util_test 9.19
 binman.cmdline 9.06
diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index 748277c1cde..84f828a6352 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -25,6 +25,160 @@ about ATF.
 
 
 
+Entry: atf-fip: ARM Trusted Firmware's Firmware Image Package (FIP)
+-------------------------------------------------------------------
+
+A FIP_ provides a way to group binaries in a firmware image, used by ARM's
+Trusted Firmware A (TF-A) code. It is a simple format consisting of a
+table of contents with information about the type, offset and size of the
+binaries in the FIP. It is quite similar to FMAP, with the major difference
+that it uses UUIDs to indicate the type of each entry.
+
+Note: It is recommended to always add an fdtmap to every image, as well as
+any FIPs so that binman and other tools can access the entire image
+correctly.
+
+The UUIDs correspond to useful names in `fiptool`, provided by ATF to
+operate on FIPs. Binman uses these names to make it easier to understand
+what is going on, although it is possible to provide a UUID if needed.
+
+The contents of the FIP are defined by subnodes of the atf-fip entry, e.g.::
+
+    atf-fip {
+        soc-fw {
+            filename = "bl31.bin";
+        };
+
+        scp-fwu-cfg {
+            filename = "bl2u.bin";
+        };
+
+        u-boot {
+            fip-type = "nt-fw";
+        };
+    };
+
+This describes a FIP with three entries: soc-fw, scp-fwu-cfg and nt-fw.
+You can use normal (non-external) binaries like U-Boot simply by adding a
+FIP type, with the `fip-type` property, as above.
+
+Since FIP exists to bring blobs together, Binman assumes that all FIP
+entries are external binaries. If a binary may not exist, you can use the
+`--allow-missing` flag to Binman, in which case the image is still created,
+even though it will not actually work.
+
+The size of the FIP depends on the size of the binaries. There is currently
+no way to specify a fixed size. If the `atf-fip` node has a `size` entry,
+this affects the space taken up by the `atf-fip` entry, but the FIP itself
+does not expand to use that space.
+
+Some other FIP features are available with Binman. The header and the
+entries have 64-bit flag works. The flag flags do not seem to be defined
+anywhere, but you can use `fip-hdr-flags` and fip-flags` to set the values
+of the header and entries respectively.
+
+FIP entries can be aligned to a particular power-of-two boundary. Use
+fip-align for this.
+
+Binman only understands the entry types that are included in its
+implementation. It is possible to specify a 16-byte UUID instead, using the
+fip-uuid property. In this case Binman doesn't know what its type is, so
+just uses the UUID. See the `u-boot` node in this example::
+
+    binman {
+        atf-fip {
+            fip-hdr-flags = /bits/ 64 <0x123>;
+            fip-align = <16>;
+            soc-fw {
+                fip-flags = /bits/ 64 <0x456>;
+                filename = "bl31.bin";
+            };
+
+            scp-fwu-cfg {
+                filename = "bl2u.bin";
+            };
+
+            u-boot {
+                fip-uuid = [fc 65 13 92 4a 5b 11 ec
+                            94 35 ff 2d 1c fc 79 9c];
+            };
+        };
+        fdtmap {
+        };
+    };
+
+Binman allows reading and updating FIP entries after the image is created,
+provided that an FDPMAP is present too. Updates which change the size of a
+FIP entry will cause it to be expanded or contracted as needed.
+
+Properties for top-level atf-fip node
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+fip-hdr-flags (64 bits)
+    Sets the flags for the FIP header.
+
+Properties for subnodes
+~~~~~~~~~~~~~~~~~~~~~~~
+
+fip-type (str)
+    FIP type to use for this entry. This is needed if the entry
+    name is not a valid type. Value types are defined in `fip_util.py`.
+    The FIP type defines the UUID that is used (they map 1:1).
+
+fip-uuid (16 bytes)
+    If there is no FIP-type name defined, or it is not supported by Binman,
+    this property sets the UUID. It should be a 16-byte value, following the
+    hex digits of the UUID.
+
+fip-flags (64 bits)
+    Set the flags for a FIP entry. Use in one of the subnodes of the
+    7atf-fip entry.
+
+fip-align
+    Set the alignment for a FIP entry, FIP entries can be aligned to a
+    particular power-of-two boundary. The default is 1.
+
+Adding new FIP-entry types
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+When new FIP entries are defined by TF-A they appear in the
+`TF-A source tree`_. You can use `fip_util.py` to update Binman to support
+new types, then `send a patch`_ to the U-Boot mailing list. There are two
+source files that the tool examples:
+
+- `include/tools_share/firmware_image_package.h` has the UUIDs
+- `tools/fiptool/tbbr_config.c` has the name and descripion for each UUID
+
+To run the tool::
+
+    $ tools/binman/fip_util.py  -s /path/to/arm-trusted-firmware
+    Warning: UUID 'UUID_NON_TRUSTED_WORLD_KEY_CERT' is not mentioned in tbbr_config.c file
+    Existing code in 'tools/binman/fip_util.py' is up-to-date
+
+If it shows there is an update, it writes a new version of `fip_util.py`
+to `fip_util.py.out`. You can change the output file using the `-i` flag.
+If you have a problem, use `-D` to enable traceback debugging.
+
+FIP commentary
+~~~~~~~~~~~~~~
+
+As a side effect of use of UUIDs, FIP does not support multiple
+entries of the same type, such as might be used to store fonts or graphics
+icons, for example. For verified boot it could be used for each part of the
+image (e.g. separate FIPs for A and B) but cannot describe the whole
+firmware image. As with FMAP there is no hierarchy defined, although FMAP
+works around this by having 'section' areas which encompass others. A
+similar workaround would be possible with FIP but is not currently defined.
+
+It is recommended to always add an fdtmap to every image, as well as any
+FIPs so that binman and other tools can access the entire image correctly.
+
+.. _FIP: https://trustedfirmware-a.readthedocs.io/en/latest/design/firmware-design.html#firmware-image-package-fip
+.. _`TF-A source tree`: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git
+.. _`send a patch`: https://www.denx.de/wiki/U-Boot/Patches
+
+
+
 Entry: blob: Arbitrary binary blob
 ----------------------------------
 
diff --git a/tools/binman/etype/atf_fip.py b/tools/binman/etype/atf_fip.py
new file mode 100644
index 00000000000..f9039e14c0a
--- /dev/null
+++ b/tools/binman/etype/atf_fip.py
@@ -0,0 +1,273 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2019 Google LLC
+# Written by Simon Glass <sjg@chromium.org>
+#
+# Entry-type module for the ARM Trusted Firmware's Firmware Image Package (FIP)
+# format
+
+from collections import OrderedDict
+
+from binman.entry import Entry
+from binman.etype.section import Entry_section
+from binman.fip_util import FIP_TYPES, FipReader, FipWriter, UUID_LEN
+from dtoc import fdt_util
+from patman import tools
+
+class Entry_atf_fip(Entry_section):
+    """ARM Trusted Firmware's Firmware Image Package (FIP)
+
+    A FIP_ provides a way to group binaries in a firmware image, used by ARM's
+    Trusted Firmware A (TF-A) code. It is a simple format consisting of a
+    table of contents with information about the type, offset and size of the
+    binaries in the FIP. It is quite similar to FMAP, with the major difference
+    that it uses UUIDs to indicate the type of each entry.
+
+    Note: It is recommended to always add an fdtmap to every image, as well as
+    any FIPs so that binman and other tools can access the entire image
+    correctly.
+
+    The UUIDs correspond to useful names in `fiptool`, provided by ATF to
+    operate on FIPs. Binman uses these names to make it easier to understand
+    what is going on, although it is possible to provide a UUID if needed.
+
+    The contents of the FIP are defined by subnodes of the atf-fip entry, e.g.::
+
+        atf-fip {
+            soc-fw {
+                filename = "bl31.bin";
+            };
+
+            scp-fwu-cfg {
+                filename = "bl2u.bin";
+            };
+
+            u-boot {
+                fip-type = "nt-fw";
+            };
+        };
+
+    This describes a FIP with three entries: soc-fw, scp-fwu-cfg and nt-fw.
+    You can use normal (non-external) binaries like U-Boot simply by adding a
+    FIP type, with the `fip-type` property, as above.
+
+    Since FIP exists to bring blobs together, Binman assumes that all FIP
+    entries are external binaries. If a binary may not exist, you can use the
+    `--allow-missing` flag to Binman, in which case the image is still created,
+    even though it will not actually work.
+
+    The size of the FIP depends on the size of the binaries. There is currently
+    no way to specify a fixed size. If the `atf-fip` node has a `size` entry,
+    this affects the space taken up by the `atf-fip` entry, but the FIP itself
+    does not expand to use that space.
+
+    Some other FIP features are available with Binman. The header and the
+    entries have 64-bit flag works. The flag flags do not seem to be defined
+    anywhere, but you can use `fip-hdr-flags` and fip-flags` to set the values
+    of the header and entries respectively.
+
+    FIP entries can be aligned to a particular power-of-two boundary. Use
+    fip-align for this.
+
+    Binman only understands the entry types that are included in its
+    implementation. It is possible to specify a 16-byte UUID instead, using the
+    fip-uuid property. In this case Binman doesn't know what its type is, so
+    just uses the UUID. See the `u-boot` node in this example::
+
+        binman {
+            atf-fip {
+                fip-hdr-flags = /bits/ 64 <0x123>;
+                fip-align = <16>;
+                soc-fw {
+                    fip-flags = /bits/ 64 <0x456>;
+                    filename = "bl31.bin";
+                };
+
+                scp-fwu-cfg {
+                    filename = "bl2u.bin";
+                };
+
+                u-boot {
+                    fip-uuid = [fc 65 13 92 4a 5b 11 ec
+                                94 35 ff 2d 1c fc 79 9c];
+                };
+            };
+            fdtmap {
+            };
+        };
+
+    Binman allows reading and updating FIP entries after the image is created,
+    provided that an FDPMAP is present too. Updates which change the size of a
+    FIP entry will cause it to be expanded or contracted as needed.
+
+    Properties for top-level atf-fip node
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+    fip-hdr-flags (64 bits)
+        Sets the flags for the FIP header.
+
+    Properties for subnodes
+    ~~~~~~~~~~~~~~~~~~~~~~~
+
+    fip-type (str)
+        FIP type to use for this entry. This is needed if the entry
+        name is not a valid type. Value types are defined in `fip_util.py`.
+        The FIP type defines the UUID that is used (they map 1:1).
+
+    fip-uuid (16 bytes)
+        If there is no FIP-type name defined, or it is not supported by Binman,
+        this property sets the UUID. It should be a 16-byte value, following the
+        hex digits of the UUID.
+
+    fip-flags (64 bits)
+        Set the flags for a FIP entry. Use in one of the subnodes of the
+        7atf-fip entry.
+
+    fip-align
+        Set the alignment for a FIP entry, FIP entries can be aligned to a
+        particular power-of-two boundary. The default is 1.
+
+    Adding new FIP-entry types
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+    When new FIP entries are defined by TF-A they appear in the
+    `TF-A source tree`_. You can use `fip_util.py` to update Binman to support
+    new types, then `send a patch`_ to the U-Boot mailing list. There are two
+    source files that the tool examples:
+
+    - `include/tools_share/firmware_image_package.h` has the UUIDs
+    - `tools/fiptool/tbbr_config.c` has the name and descripion for each UUID
+
+    To run the tool::
+
+        $ tools/binman/fip_util.py  -s /path/to/arm-trusted-firmware
+        Warning: UUID 'UUID_NON_TRUSTED_WORLD_KEY_CERT' is not mentioned in tbbr_config.c file
+        Existing code in 'tools/binman/fip_util.py' is up-to-date
+
+    If it shows there is an update, it writes a new version of `fip_util.py`
+    to `fip_util.py.out`. You can change the output file using the `-i` flag.
+    If you have a problem, use `-D` to enable traceback debugging.
+
+    FIP commentary
+    ~~~~~~~~~~~~~~
+
+    As a side effect of use of UUIDs, FIP does not support multiple
+    entries of the same type, such as might be used to store fonts or graphics
+    icons, for example. For verified boot it could be used for each part of the
+    image (e.g. separate FIPs for A and B) but cannot describe the whole
+    firmware image. As with FMAP there is no hierarchy defined, although FMAP
+    works around this by having 'section' areas which encompass others. A
+    similar workaround would be possible with FIP but is not currently defined.
+
+    It is recommended to always add an fdtmap to every image, as well as any
+    FIPs so that binman and other tools can access the entire image correctly.
+
+    .. _FIP: https://trustedfirmware-a.readthedocs.io/en/latest/design/firmware-design.html#firmware-image-package-fip
+    .. _`TF-A source tree`: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git
+    .. _`send a patch`: https://www.denx.de/wiki/U-Boot/Patches
+    """
+    def __init__(self, section, etype, node):
+        # Put this here to allow entry-docs and help to work without libfdt
+        global state
+        from binman import state
+
+        super().__init__(section, etype, node)
+        self.align_default = None
+        self._entries = OrderedDict()
+        self.reader = None
+
+    def ReadNode(self):
+        """Read properties from the atf-fip node"""
+        super().ReadNode()
+        self._pad_byte = fdt_util.GetInt(self._node, 'pad-byte', 0)
+        self._fip_flags = fdt_util.GetInt64(self._node, 'fip-hdr-flags', 0)
+        self._fip_align = fdt_util.GetInt(self._node, 'fip-align', 1)
+        if tools.NotPowerOfTwo(self._fip_align):
+            raise ValueError("Node '%s': FIP alignment %s must be a power of two" %
+                             (self._node.path, self._fip_align))
+        self.ReadEntries()
+
+    def ReadEntries(self):
+        """Read the subnodes to find out what should go in this FIP"""
+        for node in self._node.subnodes:
+            fip_type = None
+            etype = None
+            if node.name in FIP_TYPES:
+                fip_type = node.name
+                etype = 'blob-ext'
+
+            entry = Entry.Create(self, node, etype)
+            entry._fip_uuid = fdt_util.GetBytes(node, 'fip-uuid', UUID_LEN)
+            if not fip_type and not entry._fip_uuid:
+                fip_type = fdt_util.GetString(node, 'fip-type')
+                if not fip_type:
+                    self.Raise("Must provide a fip-type (node name '%s' is not a known FIP type)" %
+                               node.name)
+
+            entry._fip_type = fip_type
+            entry._fip_flags = fdt_util.GetInt64(node, 'fip-flags', 0)
+            entry.ReadNode()
+            entry._fip_name = node.name
+            self._entries[entry._fip_name] = entry
+
+    def BuildSectionData(self, required):
+        """Override this function to create a custom format for the entries
+
+        Arguments:
+            required (bool): True if the data must be valid, False if it may
+                be missing (entry.GetData() returns None
+
+        Returns:
+            bytes: Data obtained, or None if None
+        """
+        fip = FipWriter(self._fip_flags, self._fip_align)
+        for entry in self._entries.values():
+            # First get the input data and put it in an entry. If not available,
+            # try later.
+            entry_data = entry.GetData(required)
+            if not required and entry_data is None:
+                return None
+            fent = fip.add_entry(entry._fip_type or entry._fip_uuid, entry_data,
+                                 entry._fip_flags)
+            if fent:
+                entry._fip_entry = fent
+        data = fip.get_data()
+        return data
+
+    def SetImagePos(self, image_pos):
+        """Override this function to set all the entry properties from FIP
+
+        We can only do this once image_pos is known
+
+        Args:
+            image_pos: Position of this entry in the image
+        """
+        super().SetImagePos(image_pos)
+
+        # Now update the entries with info from the FIP entries
+        for entry in self._entries.values():
+            fent = entry._fip_entry
+            entry.size = fent.size
+            entry.offset = fent.offset
+            entry.image_pos = self.image_pos + entry.offset
+
+    def ReadChildData(self, child, decomp=True):
+        if not self.reader:
+            self.fip_data = super().ReadData(True)
+            self.reader = FipReader(self.fip_data)
+        reader = self.reader
+
+        # It is tricky to obtain the data from a FIP entry since it is indexed
+        # by its UUID.
+        fent = reader.get_entry(child._fip_type or child._fip_uuid)
+        return fent.data
+
+        # Note:
+        # It is also possible to extract it using the offsets directly, but this
+        # seems less FIP_friendly:
+        # return self.fip_data[child.offset:child.offset + child.size]
+
+    def WriteChildData(self, child):
+        # Recreate the data structure, leaving the data for this child alone,
+        # so that child.data is used to pack into the FIP.
+        self.ObtainContents(skip_entry=child)
+        return True
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 0f4330b6807..d8ddbc81c1f 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -23,6 +23,7 @@ from binman import cmdline
 from binman import control
 from binman import elf
 from binman import elf_test
+from binman import fip_util
 from binman import fmap_util
 from binman import state
 from dtoc import fdt
@@ -76,6 +77,7 @@ FSP_M_DATA            = b'fsp_m'
 FSP_S_DATA            = b'fsp_s'
 FSP_T_DATA            = b'fsp_t'
 ATF_BL31_DATA         = b'bl31'
+ATF_BL2U_DATA         = b'bl2u'
 OPENSBI_DATA          = b'opensbi'
 SCP_DATA              = b'scp'
 TEST_FDT1_DATA        = b'fdt1'
@@ -179,6 +181,7 @@ class TestFunctional(unittest.TestCase):
         TestFunctional._MakeInputFile('compress', COMPRESS_DATA)
         TestFunctional._MakeInputFile('compress_big', COMPRESS_DATA_BIG)
         TestFunctional._MakeInputFile('bl31.bin', ATF_BL31_DATA)
+        TestFunctional._MakeInputFile('bl2u.bin', ATF_BL2U_DATA)
         TestFunctional._MakeInputFile('fw_dynamic.bin', OPENSBI_DATA)
         TestFunctional._MakeInputFile('scp.bin', SCP_DATA)
 
@@ -4681,6 +4684,220 @@ class TestFunctional(unittest.TestCase):
                         binary=False)
         self.assertEqual(version, state.GetVersion(self._indir))
 
+    def testFip(self):
+        """Basic test of generation of an ARM Firmware Image Package (FIP)"""
+        data = self._DoReadFile('203_fip.dts')
+        hdr, fents = fip_util.decode_fip(data)
+        self.assertEqual(fip_util.HEADER_MAGIC, hdr.name)
+        self.assertEqual(fip_util.HEADER_SERIAL, hdr.serial)
+        self.assertEqual(0x123, hdr.flags)
+
+        self.assertEqual(2, len(fents))
+
+        fent = fents[0]
+        self.assertEqual(
+            bytes([0x47,  0xd4, 0x08, 0x6d, 0x4c, 0xfe, 0x98, 0x46,
+                  0x9b, 0x95, 0x29, 0x50, 0xcb, 0xbd, 0x5a, 0x0]), fent.uuid)
+        self.assertEqual('soc-fw', fent.fip_type)
+        self.assertEqual(0x88, fent.offset)
+        self.assertEqual(len(ATF_BL31_DATA), fent.size)
+        self.assertEqual(0x123456789abcdef, fent.flags)
+        self.assertEqual(ATF_BL31_DATA, fent.data)
+        self.assertEqual(True, fent.valid)
+
+        fent = fents[1]
+        self.assertEqual(
+            bytes([0x65, 0x92, 0x27, 0x03, 0x2f, 0x74, 0xe6, 0x44,
+             0x8d, 0xff, 0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10]), fent.uuid)
+        self.assertEqual('scp-fwu-cfg', fent.fip_type)
+        self.assertEqual(0x8c, fent.offset)
+        self.assertEqual(len(ATF_BL31_DATA), fent.size)
+        self.assertEqual(0, fent.flags)
+        self.assertEqual(ATF_BL2U_DATA, fent.data)
+        self.assertEqual(True, fent.valid)
+
+    def testFipOther(self):
+        """Basic FIP with something that isn't a external blob"""
+        data = self._DoReadFile('204_fip_other.dts')
+        hdr, fents = fip_util.decode_fip(data)
+
+        self.assertEqual(2, len(fents))
+        fent = fents[1]
+        self.assertEqual('rot-cert', fent.fip_type)
+        self.assertEqual(b'aa', fent.data)
+
+    def testFipOther(self):
+        """Basic FIP with something that isn't a external blob"""
+        data = self._DoReadFile('204_fip_other.dts')
+        hdr, fents = fip_util.decode_fip(data)
+
+        self.assertEqual(2, len(fents))
+        fent = fents[1]
+        self.assertEqual('rot-cert', fent.fip_type)
+        self.assertEqual(b'aa', fent.data)
+
+    def testFipNoType(self):
+        """FIP with an entry of an unknown type"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFile('205_fip_no_type.dts')
+        self.assertIn("Must provide a fip-type (node name 'u-boot' is not a known FIP type)",
+                      str(e.exception))
+
+    def testFipUuid(self):
+        """Basic FIP with a manual uuid"""
+        data = self._DoReadFile('206_fip_uuid.dts')
+        hdr, fents = fip_util.decode_fip(data)
+
+        self.assertEqual(2, len(fents))
+        fent = fents[1]
+        self.assertEqual(None, fent.fip_type)
+        self.assertEqual(
+            bytes([0xfc, 0x65, 0x13, 0x92, 0x4a, 0x5b, 0x11, 0xec,
+                   0x94, 0x35, 0xff, 0x2d, 0x1c, 0xfc, 0x79, 0x9c]),
+            fent.uuid)
+        self.assertEqual(U_BOOT_DATA, fent.data)
+
+    def testFipLs(self):
+        """Test listing a FIP"""
+        data = self._DoReadFileRealDtb('207_fip_ls.dts')
+        hdr, fents = fip_util.decode_fip(data)
+
+        try:
+            tmpdir, updated_fname = self._SetupImageInTmpdir()
+            with test_util.capture_sys_output() as (stdout, stderr):
+                self._DoBinman('ls', '-i', updated_fname)
+        finally:
+            shutil.rmtree(tmpdir)
+        lines = stdout.getvalue().splitlines()
+        expected = [
+'Name          Image-pos  Size  Entry-type  Offset  Uncomp-size',
+'----------------------------------------------------------------',
+'main-section          0   2d3  section          0',
+'  atf-fip             0    90  atf-fip          0',
+'    soc-fw           88     4  blob-ext        88',
+'    u-boot           8c     4  u-boot          8c',
+'  fdtmap             90   243  fdtmap          90',
+]
+        self.assertEqual(expected, lines)
+
+        image = control.images['image']
+        entries = image.GetEntries()
+        fdtmap = entries['fdtmap']
+
+        fdtmap_data = data[fdtmap.image_pos:fdtmap.image_pos + fdtmap.size]
+        magic = fdtmap_data[:8]
+        self.assertEqual(b'_FDTMAP_', magic)
+        self.assertEqual(tools.GetBytes(0, 8), fdtmap_data[8:16])
+
+        fdt_data = fdtmap_data[16:]
+        dtb = fdt.Fdt.FromData(fdt_data)
+        dtb.Scan()
+        props = self._GetPropTree(dtb, BASE_DTB_PROPS, prefix='/')
+        self.assertEqual({
+            'atf-fip/soc-fw:image-pos': 136,
+            'atf-fip/soc-fw:offset': 136,
+            'atf-fip/soc-fw:size': 4,
+            'atf-fip/u-boot:image-pos': 140,
+            'atf-fip/u-boot:offset': 140,
+            'atf-fip/u-boot:size': 4,
+            'atf-fip:image-pos': 0,
+            'atf-fip:offset': 0,
+            'atf-fip:size': 144,
+            'image-pos': 0,
+            'offset': 0,
+            'fdtmap:image-pos': fdtmap.image_pos,
+            'fdtmap:offset': fdtmap.offset,
+            'fdtmap:size': len(fdtmap_data),
+            'size': len(data),
+        }, props)
+
+    def testFipExtractOneEntry(self):
+        """Test extracting a single entry fron an FIP"""
+        self._DoReadFileRealDtb('207_fip_ls.dts')
+        image_fname = tools.GetOutputFilename('image.bin')
+        fname = os.path.join(self._indir, 'output.extact')
+        control.ExtractEntries(image_fname, fname, None, ['atf-fip/u-boot'])
+        data = tools.ReadFile(fname)
+        self.assertEqual(U_BOOT_DATA, data)
+
+    def testFipReplace(self):
+        """Test replacing a single file in a FIP"""
+        expected = U_BOOT_DATA + tools.GetBytes(0x78, 50)
+        data = self._DoReadFileRealDtb('208_fip_replace.dts')
+        updated_fname = tools.GetOutputFilename('image-updated.bin')
+        tools.WriteFile(updated_fname, data)
+        entry_name = 'atf-fip/u-boot'
+        control.WriteEntry(updated_fname, entry_name, expected,
+                           allow_resize=True)
+        actual = control.ReadEntry(updated_fname, entry_name)
+        self.assertEqual(expected, actual)
+
+        new_data = tools.ReadFile(updated_fname)
+        hdr, fents = fip_util.decode_fip(new_data)
+
+        self.assertEqual(2, len(fents))
+
+        # Check that the FIP entry is updated
+        fent = fents[1]
+        self.assertEqual(0x8c, fent.offset)
+        self.assertEqual(len(expected), fent.size)
+        self.assertEqual(0, fent.flags)
+        self.assertEqual(expected, fent.data)
+        self.assertEqual(True, fent.valid)
+
+    def testFipMissing(self):
+        with test_util.capture_sys_output() as (stdout, stderr):
+            self._DoTestFile('209_fip_missing.dts', allow_missing=True)
+        err = stderr.getvalue()
+        self.assertRegex(err, "Image 'main-section'.*missing.*: rmm-fw")
+
+    def testFipSize(self):
+        """Test a FIP with a size property"""
+        data = self._DoReadFile('210_fip_size.dts')
+        self.assertEqual(0x100 + len(U_BOOT_DATA), len(data))
+        hdr, fents = fip_util.decode_fip(data)
+        self.assertEqual(fip_util.HEADER_MAGIC, hdr.name)
+        self.assertEqual(fip_util.HEADER_SERIAL, hdr.serial)
+
+        self.assertEqual(1, len(fents))
+
+        fent = fents[0]
+        self.assertEqual('soc-fw', fent.fip_type)
+        self.assertEqual(0x60, fent.offset)
+        self.assertEqual(len(ATF_BL31_DATA), fent.size)
+        self.assertEqual(ATF_BL31_DATA, fent.data)
+        self.assertEqual(True, fent.valid)
+
+        rest = data[0x60 + len(ATF_BL31_DATA):0x100]
+        self.assertEqual(tools.GetBytes(0xff, len(rest)), rest)
+
+    def testFipBadAlign(self):
+        """Test that an invalid alignment value in a FIP is detected"""
+        with self.assertRaises(ValueError) as e:
+            self._DoTestFile('211_fip_bad_align.dts')
+        self.assertIn(
+            "Node \'/binman/atf-fip\': FIP alignment 31 must be a power of two",
+            str(e.exception))
+
+    def testFipCollection(self):
+        """Test using a FIP in a collection"""
+        data = self._DoReadFile('212_fip_collection.dts')
+        entry1 = control.images['image'].GetEntries()['collection']
+        data1 = data[:entry1.size]
+        hdr1, fents2 = fip_util.decode_fip(data1)
+
+        entry2 = control.images['image'].GetEntries()['atf-fip']
+        data2 = data[entry2.offset:entry2.offset + entry2.size]
+        hdr1, fents2 = fip_util.decode_fip(data2)
+
+        # The 'collection' entry should have U-Boot included at the end
+        self.assertEqual(entry1.size - len(U_BOOT_DATA), entry2.size)
+        self.assertEqual(data1, data2 + U_BOOT_DATA)
+        self.assertEqual(U_BOOT_DATA, data1[-4:])
+
+        # There should be a U-Boot after the final FIP
+        self.assertEqual(U_BOOT_DATA, data[-4:])
+
 
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/203_fip.dts b/tools/binman/test/203_fip.dts
new file mode 100644
index 00000000000..08973373240
--- /dev/null
+++ b/tools/binman/test/203_fip.dts
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		atf-fip {
+			fip-hdr-flags = /bits/ 64 <0x123>;
+			soc-fw {
+				fip-flags = /bits/ 64 <0x123456789abcdef>;
+				filename = "bl31.bin";
+			};
+
+			scp-fwu-cfg {
+				filename = "bl2u.bin";
+			};
+		};
+	};
+};
diff --git a/tools/binman/test/204_fip_other.dts b/tools/binman/test/204_fip_other.dts
new file mode 100644
index 00000000000..65039410986
--- /dev/null
+++ b/tools/binman/test/204_fip_other.dts
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		atf-fip {
+			fip-hdr-flags = /bits/ 64 <0x123>;
+			soc-fw {
+				fip-flags = /bits/ 64 <0x123456789abcdef>;
+				filename = "bl31.bin";
+			};
+
+			_testing {
+				fip-type = "rot-cert";
+				return-contents-later;
+			};
+		};
+	};
+};
diff --git a/tools/binman/test/205_fip_no_type.dts b/tools/binman/test/205_fip_no_type.dts
new file mode 100644
index 00000000000..23c8c3bc37e
--- /dev/null
+++ b/tools/binman/test/205_fip_no_type.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		atf-fip {
+			fip-hdr-flags = /bits/ 64 <0x123>;
+			u-boot {
+			};
+		};
+	};
+};
diff --git a/tools/binman/test/206_fip_uuid.dts b/tools/binman/test/206_fip_uuid.dts
new file mode 100644
index 00000000000..c9bd44f9c31
--- /dev/null
+++ b/tools/binman/test/206_fip_uuid.dts
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		atf-fip {
+			fip-hdr-flags = /bits/ 64 <0x123>;
+			soc-fw {
+				fip-flags = /bits/ 64 <0x123456789abcdef>;
+				filename = "bl31.bin";
+			};
+
+			u-boot {
+				fip-uuid = [fc 65 13 92 4a 5b 11 ec
+					    94 35 ff 2d 1c fc 79 9c];
+			};
+		};
+	};
+};
diff --git a/tools/binman/test/207_fip_ls.dts b/tools/binman/test/207_fip_ls.dts
new file mode 100644
index 00000000000..630fca15024
--- /dev/null
+++ b/tools/binman/test/207_fip_ls.dts
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		atf-fip {
+			fip-hdr-flags = /bits/ 64 <0x123>;
+			soc-fw {
+				fip-flags = /bits/ 64 <0x123456789abcdef>;
+				filename = "bl31.bin";
+			};
+
+			u-boot {
+				fip-uuid = [fc 65 13 92 4a 5b 11 ec
+					    94 35 ff 2d 1c fc 79 9c];
+			};
+		};
+
+		fdtmap {
+		};
+	};
+};
diff --git a/tools/binman/test/208_fip_replace.dts b/tools/binman/test/208_fip_replace.dts
new file mode 100644
index 00000000000..432c12474df
--- /dev/null
+++ b/tools/binman/test/208_fip_replace.dts
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		allow-repack;
+		atf-fip {
+			fip-hdr-flags = /bits/ 64 <0x123>;
+			soc-fw {
+				fip-flags = /bits/ 64 <0x123456789abcdef>;
+				filename = "bl31.bin";
+			};
+
+			u-boot {
+				fip-uuid = [fc 65 13 92 4a 5b 11 ec
+					    94 35 ff 2d 1c fc 79 9c];
+			};
+
+		};
+
+		u-boot {
+		};
+
+		u-boot-dtb {
+		};
+
+		fdtmap {
+		};
+	};
+};
diff --git a/tools/binman/test/209_fip_missing.dts b/tools/binman/test/209_fip_missing.dts
new file mode 100644
index 00000000000..43bb600d047
--- /dev/null
+++ b/tools/binman/test/209_fip_missing.dts
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		atf-fip {
+			soc-fw {
+				filename = "bl31.bin";
+			};
+
+			rmm-fw {
+				filename = "rmm.bin";
+			};
+		};
+	};
+};
diff --git a/tools/binman/test/210_fip_size.dts b/tools/binman/test/210_fip_size.dts
new file mode 100644
index 00000000000..9dfee796459
--- /dev/null
+++ b/tools/binman/test/210_fip_size.dts
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		atf-fip {
+			size = <0x100>;
+			pad-byte = <0xff>;
+			soc-fw {
+				filename = "bl31.bin";
+			};
+		};
+		u-boot {
+		};
+	};
+};
diff --git a/tools/binman/test/211_fip_bad_align.dts b/tools/binman/test/211_fip_bad_align.dts
new file mode 100644
index 00000000000..a0901496d80
--- /dev/null
+++ b/tools/binman/test/211_fip_bad_align.dts
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		atf-fip {
+			fip-align = <31>;
+			size = <0x100>;
+			pad-byte = <0xff>;
+			soc-fw {
+				filename = "bl31.bin";
+			};
+		};
+	};
+};
diff --git a/tools/binman/test/212_fip_collection.dts b/tools/binman/test/212_fip_collection.dts
new file mode 100644
index 00000000000..332c023af87
--- /dev/null
+++ b/tools/binman/test/212_fip_collection.dts
@@ -0,0 +1,24 @@
+// SPDX-License-Identifier: GPL-2.0+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		collection {
+			content = <&fip &u_boot>;
+		};
+		fip: atf-fip {
+			soc-fw {
+				filename = "bl31.bin";
+			};
+
+			scp-fwu-cfg {
+				filename = "bl2u.bin";
+			};
+		};
+		u_boot: u-boot {
+		};
+	};
+};
-- 
2.34.0.rc2.393.gf8c9666880-goog


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)
  2021-11-24  4:08 [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP) Simon Glass
  2021-11-24  4:08 ` [PATCH 1/2] binman: Add a utility module for ATF FIP Simon Glass
  2021-11-24  4:08 ` [PATCH 2/2] binman: Add support " Simon Glass
@ 2021-11-25  9:42 ` Ilias Apalodimas
  2021-11-25 14:47   ` Ilias Apalodimas
  2 siblings, 1 reply; 18+ messages in thread
From: Ilias Apalodimas @ 2021-11-25  9:42 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

Hi Simon,


On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org> wrote:
>
>
> This series adds support for the FIP format as used by ARM Trusted
> Firmware (in particular TF-A).
>
> This allows images to be created containing a FIP, which itself contains
> various binaries. With this, image creation can be handled from an in-tree
> image description instead of needing to perform a lot of manual steps or
> custom scripts to build the FIP.
>
>
> Simon Glass (2):
>   binman: Add a utility module for ATF FIP
>   binman: Add support for ATF FIP
>
>  scripts/pylint.base                      |   9 +-
>  tools/binman/entries.rst                 | 154 ++++++
>  tools/binman/etype/atf_fip.py            | 273 ++++++++++
>  tools/binman/fip_util.py                 | 653 +++++++++++++++++++++++
>  tools/binman/fip_util_test.py            | 405 ++++++++++++++
>  tools/binman/ftest.py                    | 217 ++++++++
>  tools/binman/main.py                     |   4 +-
>  tools/binman/test/203_fip.dts            |  21 +
>  tools/binman/test/204_fip_other.dts      |  22 +
>  tools/binman/test/205_fip_no_type.dts    |  15 +
>  tools/binman/test/206_fip_uuid.dts       |  22 +
>  tools/binman/test/207_fip_ls.dts         |  25 +
>  tools/binman/test/208_fip_replace.dts    |  33 ++
>  tools/binman/test/209_fip_missing.dts    |  19 +
>  tools/binman/test/210_fip_size.dts       |  19 +
>  tools/binman/test/211_fip_bad_align.dts  |  18 +
>  tools/binman/test/212_fip_collection.dts |  24 +
>  17 files changed, 1929 insertions(+), 4 deletions(-)
>  create mode 100644 tools/binman/etype/atf_fip.py
>  create mode 100755 tools/binman/fip_util.py
>  create mode 100755 tools/binman/fip_util_test.py
>  create mode 100644 tools/binman/test/203_fip.dts
>  create mode 100644 tools/binman/test/204_fip_other.dts
>  create mode 100644 tools/binman/test/205_fip_no_type.dts
>  create mode 100644 tools/binman/test/206_fip_uuid.dts
>  create mode 100644 tools/binman/test/207_fip_ls.dts
>  create mode 100644 tools/binman/test/208_fip_replace.dts
>  create mode 100644 tools/binman/test/209_fip_missing.dts
>  create mode 100644 tools/binman/test/210_fip_size.dts
>  create mode 100644 tools/binman/test/211_fip_bad_align.dts
>  create mode 100644 tools/binman/test/212_fip_collection.dts
>
> --
> 2.34.0.rc2.393.gf8c9666880-goog
>

My python is mediocre at best.  I'll try having a look, but CC'ing
TF-A developers would be a good idea.

Thanks
/Ilias

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)
  2021-11-25  9:42 ` [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP) Ilias Apalodimas
@ 2021-11-25 14:47   ` Ilias Apalodimas
  2021-11-25 15:11     ` François Ozog
  0 siblings, 1 reply; 18+ messages in thread
From: Ilias Apalodimas @ 2021-11-25 14:47 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List, Sandrine Bailleux

+cc Sandrine

On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Simon,
>
>
> On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org> wrote:
> >
> >
> > This series adds support for the FIP format as used by ARM Trusted
> > Firmware (in particular TF-A).
> >
> > This allows images to be created containing a FIP, which itself contains
> > various binaries. With this, image creation can be handled from an in-tree
> > image description instead of needing to perform a lot of manual steps or
> > custom scripts to build the FIP.
> >
> >
> > Simon Glass (2):
> >   binman: Add a utility module for ATF FIP
> >   binman: Add support for ATF FIP
> >
> >  scripts/pylint.base                      |   9 +-
> >  tools/binman/entries.rst                 | 154 ++++++
> >  tools/binman/etype/atf_fip.py            | 273 ++++++++++
> >  tools/binman/fip_util.py                 | 653 +++++++++++++++++++++++
> >  tools/binman/fip_util_test.py            | 405 ++++++++++++++
> >  tools/binman/ftest.py                    | 217 ++++++++
> >  tools/binman/main.py                     |   4 +-
> >  tools/binman/test/203_fip.dts            |  21 +
> >  tools/binman/test/204_fip_other.dts      |  22 +
> >  tools/binman/test/205_fip_no_type.dts    |  15 +
> >  tools/binman/test/206_fip_uuid.dts       |  22 +
> >  tools/binman/test/207_fip_ls.dts         |  25 +
> >  tools/binman/test/208_fip_replace.dts    |  33 ++
> >  tools/binman/test/209_fip_missing.dts    |  19 +
> >  tools/binman/test/210_fip_size.dts       |  19 +
> >  tools/binman/test/211_fip_bad_align.dts  |  18 +
> >  tools/binman/test/212_fip_collection.dts |  24 +
> >  17 files changed, 1929 insertions(+), 4 deletions(-)
> >  create mode 100644 tools/binman/etype/atf_fip.py
> >  create mode 100755 tools/binman/fip_util.py
> >  create mode 100755 tools/binman/fip_util_test.py
> >  create mode 100644 tools/binman/test/203_fip.dts
> >  create mode 100644 tools/binman/test/204_fip_other.dts
> >  create mode 100644 tools/binman/test/205_fip_no_type.dts
> >  create mode 100644 tools/binman/test/206_fip_uuid.dts
> >  create mode 100644 tools/binman/test/207_fip_ls.dts
> >  create mode 100644 tools/binman/test/208_fip_replace.dts
> >  create mode 100644 tools/binman/test/209_fip_missing.dts
> >  create mode 100644 tools/binman/test/210_fip_size.dts
> >  create mode 100644 tools/binman/test/211_fip_bad_align.dts
> >  create mode 100644 tools/binman/test/212_fip_collection.dts
> >
> > --
> > 2.34.0.rc2.393.gf8c9666880-goog
> >
>
> My python is mediocre at best.  I'll try having a look, but CC'ing
> TF-A developers would be a good idea.
>
> Thanks
> /Ilias

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] binman: Add a utility module for ATF FIP
  2021-11-24  4:08 ` [PATCH 1/2] binman: Add a utility module for ATF FIP Simon Glass
@ 2021-11-25 14:47   ` Ilias Apalodimas
  2021-12-15  0:33   ` Simon Glass
  1 sibling, 0 replies; 18+ messages in thread
From: Ilias Apalodimas @ 2021-11-25 14:47 UTC (permalink / raw)
  To: Simon Glass, Sandrine Bailleux; +Cc: U-Boot Mailing List

+cc Sandrine

On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org> wrote:
>
> Add support for this format which is used by ARM Trusted Firmware to find
> firmware binaries to load.
>
> FIP is like a simpler version of FMAP but uses a UUID instead of a name,
> for each entry.
>
> It supports reading a FIP, writing a FIP and parsing the ATF source code
> to get a list of supported UUIDs.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  scripts/pylint.base           |   8 +-
>  tools/binman/fip_util.py      | 653 ++++++++++++++++++++++++++++++++++
>  tools/binman/fip_util_test.py | 405 +++++++++++++++++++++
>  tools/binman/main.py          |   4 +-
>  4 files changed, 1066 insertions(+), 4 deletions(-)
>  create mode 100755 tools/binman/fip_util.py
>  create mode 100755 tools/binman/fip_util_test.py
>
> diff --git a/scripts/pylint.base b/scripts/pylint.base
> index 4e82dd4a616..a35dbe34d2d 100644
> --- a/scripts/pylint.base
> +++ b/scripts/pylint.base
> @@ -9,11 +9,13 @@ binman.elf_test 5.41
>  binman.entry 2.39
>  binman.entry_test 5.29
>  binman.fdt_test 3.23
> +binman.fip_util 9.86
> +binman.fip_util_test 9.75
>  binman.fmap_util 6.67
>  binman.ftest 7.38
>  binman.image 6.39
>  binman.image_test 4.48
> -binman.main 4.26
> +binman.main 4.03
>  binman.setup 5.00
>  binman.state 3.30
>  blob -1.94
> @@ -33,7 +35,7 @@ buildman.main 1.43
>  buildman.test 6.10
>  buildman.toolchain 5.81
>  capsule_defs 5.00
> -cbfs -1.52
> +cbfs -1.44
>  collection 2.33
>  concurrencytest 6.77
>  conftest -3.84
> @@ -107,7 +109,7 @@ powerpc_mpc85xx_bootpg_resetvec -10.00
>  rkmux 6.76
>  rmboard 7.76
>  scp -6.00
> -section 4.25
> +section 4.31
>  sqfs_common 8.12
>  test 8.18
>  test_000_version 7.50
> diff --git a/tools/binman/fip_util.py b/tools/binman/fip_util.py
> new file mode 100755
> index 00000000000..5f7dbc04d56
> --- /dev/null
> +++ b/tools/binman/fip_util.py
> @@ -0,0 +1,653 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright 2021 Google LLC
> +# Written by Simon Glass <sjg@chromium.org>
> +
> +"""Support for ARM's Firmware Image Package (FIP) format
> +
> +FIP is a format similar to FMAP[1] but with fewer features and an obscure UUID
> +instead of the region name.
> +
> +It consists of a header and a table of entries, each pointing to a place in the
> +firmware image where something can be found.
> +
> +[1] https://chromium.googlesource.com/chromiumos/third_party/flashmap/+/refs/heads/master/lib/fmap.h
> +
> +If ATF updates, run this program to update the FIT_TYPE_LIST.
> +
> +ARM Trusted Firmware is available at:
> +
> +https://github.com/ARM-software/arm-trusted-firmware.git
> +"""
> +
> +from argparse import ArgumentParser
> +import collections
> +import io
> +import os
> +import re
> +import struct
> +import sys
> +from uuid import UUID
> +
> +OUR_FILE = os.path.realpath(__file__)
> +OUR_PATH = os.path.dirname(OUR_FILE)
> +
> +# Bring in the patman and dtoc libraries (but don't override the first path
> +# in PYTHONPATH)
> +sys.path.insert(2, os.path.join(OUR_PATH, '..'))
> +
> +# pylint: disable=C0413
> +from patman import command
> +from patman import tools
> +
> +# The TOC header, at the start of the FIP
> +HEADER_FORMAT = '<IIQ'
> +HEADER_LEN = 0x10
> +HEADER_MAGIC = 0xaA640001
> +HEADER_SERIAL = 0x12345678
> +
> +# The entry header (a table of these comes after the TOC header)
> +UUID_LEN = 16
> +ENTRY_FORMAT = f'<{UUID_LEN}sQQQ'
> +ENTRY_SIZE = 0x28
> +
> +HEADER_NAMES = (
> +    'name',
> +    'serial',
> +    'flags',
> +)
> +
> +ENTRY_NAMES = (
> +    'uuid',
> +    'offset',
> +    'size',
> +    'flags',
> +)
> +
> +# Set to True to enable output from running fiptool for debugging
> +VERBOSE = False
> +
> +# Use a class so we can convert the bytes, making the table more readable
> +# pylint: disable=R0903
> +class FipType:
> +    """A FIP entry type that we understand"""
> +    def __init__(self, name, desc, uuid_bytes):
> +        """Create up a new type
> +
> +        Args:
> +            name (str): Short name for the type
> +            desc (str): Longer description for the type
> +            uuid_bytes (bytes): List of 16 bytes for the UUID
> +        """
> +        self.name = name
> +        self.desc = desc
> +        self.uuid = bytes(uuid_bytes)
> +
> +# This is taken from tbbr_config.c in ARM Trusted Firmware
> +FIP_TYPE_LIST = [
> +    # ToC Entry UUIDs
> +    FipType('scp-fwu-cfg', 'SCP Firmware Updater Configuration FWU SCP_BL2U',
> +            [0x65, 0x92, 0x27, 0x03, 0x2f, 0x74, 0xe6, 0x44,
> +             0x8d, 0xff, 0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10]),
> +    FipType('ap-fwu-cfg', 'AP Firmware Updater Configuration BL2U',
> +            [0x60, 0xb3, 0xeb, 0x37, 0xc1, 0xe5, 0xea, 0x41,
> +             0x9d, 0xf3, 0x19, 0xed, 0xa1, 0x1f, 0x68, 0x01]),
> +    FipType('fwu', 'Firmware Updater NS_BL2U',
> +            [0x4f, 0x51, 0x1d, 0x11, 0x2b, 0xe5, 0x4e, 0x49,
> +             0xb4, 0xc5, 0x83, 0xc2, 0xf7, 0x15, 0x84, 0x0a]),
> +    FipType('fwu-cert', 'Non-Trusted Firmware Updater certificate',
> +            [0x71, 0x40, 0x8a, 0xb2, 0x18, 0xd6, 0x87, 0x4c,
> +             0x8b, 0x2e, 0xc6, 0xdc, 0xcd, 0x50, 0xf0, 0x96]),
> +    FipType('tb-fw', 'Trusted Boot Firmware BL2',
> +            [0x5f, 0xf9, 0xec, 0x0b, 0x4d, 0x22, 0x3e, 0x4d,
> +             0xa5, 0x44, 0xc3, 0x9d, 0x81, 0xc7, 0x3f, 0x0a]),
> +    FipType('scp-fw', 'SCP Firmware SCP_BL2',
> +            [0x97, 0x66, 0xfd, 0x3d, 0x89, 0xbe, 0xe8, 0x49,
> +             0xae, 0x5d, 0x78, 0xa1, 0x40, 0x60, 0x82, 0x13]),
> +    FipType('soc-fw', 'EL3 Runtime Firmware BL31',
> +            [0x47, 0xd4, 0x08, 0x6d, 0x4c, 0xfe, 0x98, 0x46,
> +             0x9b, 0x95, 0x29, 0x50, 0xcb, 0xbd, 0x5a, 0x00]),
> +    FipType('tos-fw', 'Secure Payload BL32 (Trusted OS)',
> +            [0x05, 0xd0, 0xe1, 0x89, 0x53, 0xdc, 0x13, 0x47,
> +             0x8d, 0x2b, 0x50, 0x0a, 0x4b, 0x7a, 0x3e, 0x38]),
> +    FipType('tos-fw-extra1', 'Secure Payload BL32 Extra1 (Trusted OS Extra1)',
> +            [0x0b, 0x70, 0xc2, 0x9b, 0x2a, 0x5a, 0x78, 0x40,
> +             0x9f, 0x65, 0x0a, 0x56, 0x82, 0x73, 0x82, 0x88]),
> +    FipType('tos-fw-extra2', 'Secure Payload BL32 Extra2 (Trusted OS Extra2)',
> +            [0x8e, 0xa8, 0x7b, 0xb1, 0xcf, 0xa2, 0x3f, 0x4d,
> +             0x85, 0xfd, 0xe7, 0xbb, 0xa5, 0x02, 0x20, 0xd9]),
> +    FipType('nt-fw', 'Non-Trusted Firmware BL33',
> +            [0xd6, 0xd0, 0xee, 0xa7, 0xfc, 0xea, 0xd5, 0x4b,
> +             0x97, 0x82, 0x99, 0x34, 0xf2, 0x34, 0xb6, 0xe4]),
> +    FipType('rmm-fw', 'Realm Monitor Management Firmware',
> +            [0x6c, 0x07, 0x62, 0xa6, 0x12, 0xf2, 0x4b, 0x56,
> +             0x92, 0xcb, 0xba, 0x8f, 0x63, 0x36, 0x06, 0xd9]),
> +    # Key certificates
> +    FipType('rot-cert', 'Root Of Trust key certificate',
> +            [0x86, 0x2d, 0x1d, 0x72, 0xf8, 0x60, 0xe4, 0x11,
> +             0x92, 0x0b, 0x8b, 0xe7, 0x62, 0x16, 0x0f, 0x24]),
> +    FipType('trusted-key-cert', 'Trusted key certificate',
> +            [0x82, 0x7e, 0xe8, 0x90, 0xf8, 0x60, 0xe4, 0x11,
> +             0xa1, 0xb4, 0x77, 0x7a, 0x21, 0xb4, 0xf9, 0x4c]),
> +    FipType('scp-fw-key-cert', 'SCP Firmware key certificate',
> +            [0x02, 0x42, 0x21, 0xa1, 0xf8, 0x60, 0xe4, 0x11,
> +             0x8d, 0x9b, 0xf3, 0x3c, 0x0e, 0x15, 0xa0, 0x14]),
> +    FipType('soc-fw-key-cert', 'SoC Firmware key certificate',
> +            [0x8a, 0xb8, 0xbe, 0xcc, 0xf9, 0x60, 0xe4, 0x11,
> +             0x9a, 0xd0, 0xeb, 0x48, 0x22, 0xd8, 0xdc, 0xf8]),
> +    FipType('tos-fw-key-cert', 'Trusted OS Firmware key certificate',
> +            [0x94, 0x77, 0xd6, 0x03, 0xfb, 0x60, 0xe4, 0x11,
> +             0x85, 0xdd, 0xb7, 0x10, 0x5b, 0x8c, 0xee, 0x04]),
> +    FipType('nt-fw-key-cert', 'Non-Trusted Firmware key certificate',
> +            [0x8a, 0xd5, 0x83, 0x2a, 0xfb, 0x60, 0xe4, 0x11,
> +             0x8a, 0xaf, 0xdf, 0x30, 0xbb, 0xc4, 0x98, 0x59]),
> +    # Content certificates
> +    FipType('tb-fw-cert', 'Trusted Boot Firmware BL2 certificate',
> +            [0xd6, 0xe2, 0x69, 0xea, 0x5d, 0x63, 0xe4, 0x11,
> +             0x8d, 0x8c, 0x9f, 0xba, 0xbe, 0x99, 0x56, 0xa5]),
> +    FipType('scp-fw-cert', 'SCP Firmware content certificate',
> +            [0x44, 0xbe, 0x6f, 0x04, 0x5e, 0x63, 0xe4, 0x11,
> +             0xb2, 0x8b, 0x73, 0xd8, 0xea, 0xae, 0x96, 0x56]),
> +    FipType('soc-fw-cert', 'SoC Firmware content certificate',
> +            [0xe2, 0xb2, 0x0c, 0x20, 0x5e, 0x63, 0xe4, 0x11,
> +             0x9c, 0xe8, 0xab, 0xcc, 0xf9, 0x2b, 0xb6, 0x66]),
> +    FipType('tos-fw-cert', 'Trusted OS Firmware content certificate',
> +            [0xa4, 0x9f, 0x44, 0x11, 0x5e, 0x63, 0xe4, 0x11,
> +             0x87, 0x28, 0x3f, 0x05, 0x72, 0x2a, 0xf3, 0x3d]),
> +    FipType('nt-fw-cert', 'Non-Trusted Firmware content certificate',
> +            [0x8e, 0xc4, 0xc1, 0xf3, 0x5d, 0x63, 0xe4, 0x11,
> +             0xa7, 0xa9, 0x87, 0xee, 0x40, 0xb2, 0x3f, 0xa7]),
> +    FipType('sip-sp-cert', 'SiP owned Secure Partition content certificate',
> +            [0x77, 0x6d, 0xfd, 0x44, 0x86, 0x97, 0x4c, 0x3b,
> +             0x91, 0xeb, 0xc1, 0x3e, 0x02, 0x5a, 0x2a, 0x6f]),
> +    FipType('plat-sp-cert', 'Platform owned Secure Partition content certificate',
> +            [0xdd, 0xcb, 0xbf, 0x4a, 0xca, 0xd6, 0x11, 0xea,
> +             0x87, 0xd0, 0x02, 0x42, 0xac, 0x13, 0x00, 0x03]),
> +    # Dynamic configs
> +    FipType('hw-config', 'HW_CONFIG',
> +            [0x08, 0xb8, 0xf1, 0xd9, 0xc9, 0xcf, 0x93, 0x49,
> +             0xa9, 0x62, 0x6f, 0xbc, 0x6b, 0x72, 0x65, 0xcc]),
> +    FipType('tb-fw-config', 'TB_FW_CONFIG',
> +            [0x6c, 0x04, 0x58, 0xff, 0xaf, 0x6b, 0x7d, 0x4f,
> +             0x82, 0xed, 0xaa, 0x27, 0xbc, 0x69, 0xbf, 0xd2]),
> +    FipType('soc-fw-config', 'SOC_FW_CONFIG',
> +            [0x99, 0x79, 0x81, 0x4b, 0x03, 0x76, 0xfb, 0x46,
> +             0x8c, 0x8e, 0x8d, 0x26, 0x7f, 0x78, 0x59, 0xe0]),
> +    FipType('tos-fw-config', 'TOS_FW_CONFIG',
> +            [0x26, 0x25, 0x7c, 0x1a, 0xdb, 0xc6, 0x7f, 0x47,
> +             0x8d, 0x96, 0xc4, 0xc4, 0xb0, 0x24, 0x80, 0x21]),
> +    FipType('nt-fw-config', 'NT_FW_CONFIG',
> +            [0x28, 0xda, 0x98, 0x15, 0x93, 0xe8, 0x7e, 0x44,
> +             0xac, 0x66, 0x1a, 0xaf, 0x80, 0x15, 0x50, 0xf9]),
> +    FipType('fw-config', 'FW_CONFIG',
> +            [0x58, 0x07, 0xe1, 0x6a, 0x84, 0x59, 0x47, 0xbe,
> +             0x8e, 0xd5, 0x64, 0x8e, 0x8d, 0xdd, 0xab, 0x0e]),
> +    ] # end
> +
> +FIP_TYPES = {ftype.name: ftype for ftype in FIP_TYPE_LIST}
> +
> +
> +def get_type_uuid(fip_type_or_uuid):
> +    """get_type_uuid() - Convert a type or uuid into both
> +
> +    This always returns a UUID, but may not return a type since it does not do
> +    the reverse lookup.
> +
> +    Args:
> +        fip_type_or_uuid (str or bytes): Either a string containing the name of
> +            an entry (e.g. 'soc-fw') or a bytes(16) containing the UUID
> +
> +    Returns:
> +        tuple:
> +            str: fip type (None if not known)
> +            bytes(16): uuid
> +
> +    Raises:
> +        ValueError: An unknown type was requested
> +    """
> +    if isinstance(fip_type_or_uuid, str):
> +        fip_type = fip_type_or_uuid
> +        lookup = FIP_TYPES.get(fip_type)
> +        if not lookup:
> +            raise ValueError(f"Unknown FIP entry type '{fip_type}'")
> +        uuid = lookup.uuid
> +    else:
> +        fip_type = None
> +        uuid = fip_type_or_uuid
> +    return fip_type, uuid
> +
> +
> +# pylint: disable=R0903
> +class FipHeader:
> +    """Class to represent a FIP header"""
> +    def __init__(self, name, serial, flags):
> +        """Set up a new header object
> +
> +        Args:
> +            name (str): Name, i.e. HEADER_MAGIC
> +            serial (str): Serial value, i.e. HEADER_SERIAL
> +            flags (int64): Flags value
> +        """
> +        self.name = name
> +        self.serial = serial
> +        self.flags = flags
> +
> +
> +# pylint: disable=R0903
> +class FipEntry:
> +    """Class to represent a single FIP entry
> +
> +    This is used to hold the information about an entry, including its contents.
> +    Use the get_data() method to obtain the raw output for writing to the FIP
> +    file.
> +    """
> +    def __init__(self, uuid, offset, size, flags):
> +        self.uuid = uuid
> +        self.offset = offset
> +        self.size = size
> +        self.flags = flags
> +        self.fip_type = None
> +        self.data = None
> +        self.valid = uuid != tools.GetBytes(0, UUID_LEN)
> +        if self.valid:
> +            # Look up the friendly name
> +            matches = {val for (key, val) in FIP_TYPES.items()
> +                       if val.uuid == uuid}
> +            if len(matches) == 1:
> +                self.fip_type = matches.pop().name
> +
> +    @classmethod
> +    def from_type(cls, fip_type_or_uuid, data, flags):
> +        """Create a FipEntry from a type name
> +
> +        Args:
> +            cls (class): This class
> +            fip_type_or_uuid (str or bytes): Name of the type to create, or
> +                bytes(16) uuid
> +            data (bytes): Contents of entry
> +            flags (int64): Flags value
> +
> +        Returns:
> +            FipEntry: Created 241
> +        """
> +        fip_type, uuid = get_type_uuid(fip_type_or_uuid)
> +        fent = FipEntry(uuid, None, len(data), flags)
> +        fent.fip_type = fip_type
> +        fent.data = data
> +        return fent
> +
> +
> +def decode_fip(data):
> +    """Decode a FIP into a header and list of FIP entries
> +
> +    Args:
> +        data (bytes): Data block containing the FMAP
> +
> +    Returns:
> +        Tuple:
> +            header: FipHeader object
> +            List of FipArea objects
> +    """
> +    fields = list(struct.unpack(HEADER_FORMAT, data[:HEADER_LEN]))
> +    header = FipHeader(*fields)
> +    fents = []
> +    pos = HEADER_LEN
> +    while True:
> +        fields = list(struct.unpack(ENTRY_FORMAT, data[pos:pos + ENTRY_SIZE]))
> +        fent = FipEntry(*fields)
> +        if not fent.valid:
> +            break
> +        fent.data = data[fent.offset:fent.offset + fent.size]
> +        fents.append(fent)
> +        pos += ENTRY_SIZE
> +    return header, fents
> +
> +
> +class FipWriter:
> +    """Class to handle writing a ARM Trusted Firmware's Firmware Image Package
> +
> +    Usage is something like:
> +
> +        fip = FipWriter(size)
> +        fip.add_entry('scp-fwu-cfg', tools.ReadFile('something.bin'))
> +        ...
> +        data = cbw.get_data()
> +
> +    Attributes:
> +    """
> +    def __init__(self, flags, align):
> +        self._fip_entries = []
> +        self._flags = flags
> +        self._align = align
> +
> +    def add_entry(self, fip_type, data, flags):
> +        """Add a new entry to the FIP
> +
> +        Args:
> +            fip_type (str): Type to add, e.g. 'tos-fw-config'
> +            data (bytes): Contents of entry
> +            flags (int64): Entry flags
> +
> +        Returns:
> +            FipEntry: entry that was added
> +        """
> +        fent = FipEntry.from_type(fip_type, data, flags)
> +        self._fip_entries.append(fent)
> +        return fent
> +
> +    def get_data(self):
> +        """Obtain the full contents of the FIP
> +
> +        Thhis builds the FIP with headers and all required FIP entries.
> +
> +        Returns:
> +            bytes: data resulting from building the FIP
> +        """
> +        buf = io.BytesIO()
> +        hdr = struct.pack(HEADER_FORMAT, HEADER_MAGIC, HEADER_SERIAL,
> +                          self._flags)
> +        buf.write(hdr)
> +
> +        # Calculate the position fo the first entry
> +        offset = len(hdr)
> +        offset += len(self._fip_entries) * ENTRY_SIZE
> +        offset += ENTRY_SIZE   # terminating entry
> +
> +        for fent in self._fip_entries:
> +            offset = tools.Align(offset, self._align)
> +            fent.offset = offset
> +            offset += fent.size
> +
> +        # Write out the TOC
> +        for fent in self._fip_entries:
> +            hdr = struct.pack(ENTRY_FORMAT, fent.uuid, fent.offset, fent.size,
> +                              fent.flags)
> +            buf.write(hdr)
> +
> +        # Write out the entries
> +        for fent in self._fip_entries:
> +            buf.seek(fent.offset)
> +            buf.write(fent.data)
> +
> +        return buf.getvalue()
> +
> +
> +class FipReader():
> +    """Class to handle reading a Firmware Image Package (FIP)
> +
> +    Usage is something like:
> +        fip = fip_util.FipReader(data)
> +        fent = fip.get_entry('fwu')
> +        self.WriteFile('ufwu.bin', fent.data)
> +        blob = fip.get_entry(
> +            bytes([0xe3, 0xb7, 0x8d, 0x9e, 0x4a, 0x64, 0x11, 0xec,
> +                   0xb4, 0x5c, 0xfb, 0xa2, 0xb9, 0xb4, 0x97, 0x88]))
> +        self.WriteFile('blob.bin', blob.data)
> +    """
> +    def __init__(self, data, read=True):
> +        """Set up a new FitReader
> +
> +        Args:
> +            data (bytes): data to read
> +            read (bool): True to read the data now
> +        """
> +        self.fents = collections.OrderedDict()
> +        self.data = data
> +        if read:
> +            self.read()
> +
> +    def read(self):
> +        """Read all the files in the FIP and add them to self.files"""
> +        self.header, self.fents = decode_fip(self.data)
> +
> +    def get_entry(self, fip_type_or_uuid):
> +        """get_entry() - Find an entry by type or UUID
> +
> +        Args:
> +            fip_type_or_uuid (str or bytes): Name of the type to create, or
> +                    bytes(16) uuid
> +
> +        Returns:
> +            FipEntry: if found
> +
> +        Raises:
> +            ValueError: entry type not found
> +        """
> +        fip_type, uuid = get_type_uuid(fip_type_or_uuid)
> +        for fent in self.fents:
> +            if fent.uuid == uuid:
> +                return fent
> +        label = fip_type
> +        if not label:
> +            label = UUID(bytes=uuid)
> +        raise ValueError(f"Cannot find FIP entry '{label}'")
> +
> +
> +def parse_macros(srcdir):
> +    """parse_macros: Parse the firmware_image_package.h file
> +
> +    Args:
> +        srcdir (str): 'arm-trusted-firmware' source directory
> +
> +    Returns:
> +        dict:
> +            key: UUID macro name, e.g. 'UUID_TRUSTED_FWU_CERT'
> +            value: list:
> +                file comment, e.g. 'ToC Entry UUIDs'
> +                macro name, e.g. 'UUID_TRUSTED_FWU_CERT'
> +                uuid as bytes(16)
> +
> +    Raises:
> +        ValueError: a line cannot be parsed
> +    """
> +    re_uuid = re.compile('0x[0-9a-fA-F]{2}')
> +    re_comment = re.compile(r'^/\* (.*) \*/$')
> +    fname = os.path.join(srcdir, 'include/tools_share/firmware_image_package.h')
> +    data = tools.ReadFile(fname, binary=False)
> +    macros = collections.OrderedDict()
> +    comment = None
> +    for linenum, line in enumerate(data.splitlines()):
> +        if line.startswith('/*'):
> +            mat = re_comment.match(line)
> +            if mat:
> +                comment = mat.group(1)
> +        else:
> +            # Example: #define UUID_TOS_FW_CONFIG \
> +            if 'UUID' in line:
> +                macro = line.split()[1]
> +            elif '{{' in line:
> +                mat = re_uuid.findall(line)
> +                if not mat or len(mat) != 16:
> +                    raise ValueError(
> +                        f'{fname}: Cannot parse UUID line {linenum + 1}: Got matches: {mat}')
> +
> +                uuid = bytes([int(val, 16) for val in mat])
> +                macros[macro] = comment, macro, uuid
> +    if not macros:
> +        raise ValueError(f'{fname}: Cannot parse file')
> +    return macros
> +
> +
> +def parse_names(srcdir):
> +    """parse_names: Parse the tbbr_config.c file
> +
> +    Args:
> +        srcdir (str): 'arm-trusted-firmware' source directory
> +
> +    Returns:
> +        tuple: dict of entries:
> +            key: UUID macro, e.g. 'UUID_NON_TRUSTED_FIRMWARE_BL33'
> +            tuple: entry information
> +                Description of entry, e.g. 'Non-Trusted Firmware BL33'
> +                UUID macro, e.g. 'UUID_NON_TRUSTED_FIRMWARE_BL33'
> +                Name of entry, e.g. 'nt-fw'
> +
> +    Raises:
> +        ValueError: the file cannot be parsed
> +    """
> +    # Extract the .name, .uuid and .cmdline_name values
> +    re_data = re.compile(r'\.name = "([^"]*)",\s*\.uuid = (UUID_\w*),\s*\.cmdline_name = "([^"]+)"',
> +                         re.S)
> +    fname = os.path.join(srcdir, 'tools/fiptool/tbbr_config.c')
> +    data = tools.ReadFile(fname, binary=False)
> +
> +    # Example entry:
> +    #   {
> +    #       .name = "Secure Payload BL32 Extra2 (Trusted OS Extra2)",
> +    #       .uuid = UUID_SECURE_PAYLOAD_BL32_EXTRA2,
> +    #       .cmdline_name = "tos-fw-extra2"
> +    #   },
> +    mat = re_data.findall(data)
> +    if not mat:
> +        raise ValueError(f'{fname}: Cannot parse file')
> +    names = {uuid: (desc, uuid, name) for desc, uuid, name in mat}
> +    return names
> +
> +
> +def create_code_output(macros, names):
> +    """create_code_output() - Create the new version of this Python file
> +
> +    Args:
> +        macros (dict):
> +            key (str): UUID macro name, e.g. 'UUID_TRUSTED_FWU_CERT'
> +            value: list:
> +                file comment, e.g. 'ToC Entry UUIDs'
> +                macro name, e.g. 'UUID_TRUSTED_FWU_CERT'
> +                uuid as bytes(16)
> +
> +        names (dict): list of entries, each
> +            tuple: entry information
> +                Description of entry, e.g. 'Non-Trusted Firmware BL33'
> +                UUID macro, e.g. 'UUID_NON_TRUSTED_FIRMWARE_BL33'
> +                Name of entry, e.g. 'nt-fw'
> +
> +    Returns:
> +        str: Table of FipType() entries
> +    """
> +    def _to_hex_list(data):
> +        """Convert bytes into C code
> +
> +        Args:
> +            bytes to convert
> +
> +        Returns:
> +            str: in the format '0x12, 0x34, 0x56...'
> +        """
> +        # Use 0x instead of %# since the latter ignores the 0 modifier in
> +        # Python 3.8.10
> +        return ', '.join(['0x%02x' % byte for byte in data])
> +
> +    out = ''
> +    last_comment = None
> +    for comment, macro, uuid in macros.values():
> +        name_entry = names.get(macro)
> +        if not name_entry:
> +            print(f"Warning: UUID '{macro}' is not mentioned in tbbr_config.c file")
> +            continue
> +        desc, _, name = name_entry
> +        if last_comment != comment:
> +            out += f'    # {comment}\n'
> +            last_comment = comment
> +        out += """    FipType('%s', '%s',
> +            [%s,
> +             %s]),
> +""" % (name, desc, _to_hex_list(uuid[:8]), _to_hex_list(uuid[8:]))
> +    return out
> +
> +
> +def parse_atf_source(srcdir, dstfile, oldfile):
> +    """parse_atf_source(): Parse the ATF source tree and update this file
> +
> +    Args:
> +        srcdir (str): Path to 'arm-trusted-firmware' directory. Get this from:
> +            https://github.com/ARM-software/arm-trusted-firmware.git
> +        dstfile (str): File to write new code to, if an update is needed
> +        oldfile (str): Python source file to compare against
> +
> +    Raises:
> +        ValueError: srcdir readme.rst is missing or the first line does not
> +            match what is expected
> +    """
> +    # We expect a readme file
> +    readme_fname = os.path.join(srcdir, 'readme.rst')
> +    if not os.path.exists(readme_fname):
> +        raise ValueError(
> +            f"Expected file '{readme_fname}' - try using -s to specify the "
> +            'arm-trusted-firmware directory')
> +    readme = tools.ReadFile(readme_fname, binary=False)
> +    first_line = 'Trusted Firmware-A'
> +    if readme.splitlines()[0] != first_line:
> +        raise ValueError(f"'{readme_fname}' does not start with '{first_line}'")
> +    macros = parse_macros(srcdir)
> +    names = parse_names(srcdir)
> +    output = create_code_output(macros, names)
> +    orig = tools.ReadFile(oldfile, binary=False)
> +    re_fip_list = re.compile(r'(.*FIP_TYPE_LIST = \[).*?(    ] # end.*)', re.S)
> +    mat = re_fip_list.match(orig)
> +    new_code = mat.group(1) + '\n' + output + mat.group(2) if mat else output
> +    if new_code == orig:
> +        print(f"Existing code in '{oldfile}' is up-to-date")
> +    else:
> +        tools.WriteFile(dstfile, new_code, binary=False)
> +        print(f'Needs update, try:\n\tmeld {dstfile} {oldfile}')
> +
> +
> +def main(argv, oldfile):
> +    """Main program for this tool
> +
> +    Args:
> +        argv (list): List of str command-line arguments
> +        oldfile (str): Python source file to compare against
> +
> +    Returns:
> +        int: 0 (exit code)
> +    """
> +    parser = ArgumentParser(epilog='''Creates an updated version of this code,
> +with a table of FIP-entry types parsed from the arm-trusted-firmware source
> +directory''')
> +    parser.add_argument(
> +        '-D', '--debug', action='store_true',
> +        help='Enabling debugging (provides a full traceback on error)')
> +    parser.add_argument(
> +        '-o', '--outfile', type=str, default='fip_util.py.out',
> +        help='Output file to write new fip_util.py file to')
> +    parser.add_argument(
> +        '-s', '--src', type=str, default='.',
> +        help='Directory containing the arm-trusted-firmware source')
> +    args = parser.parse_args(argv)
> +
> +    if not args.debug:
> +        sys.tracebacklimit = 0
> +
> +    parse_atf_source(args.src, args.outfile, oldfile)
> +    return 0
> +
> +
> +def fiptool(fname, *fip_args):
> +    """Run fiptool with provided arguments
> +
> +    If the tool fails then this function raises an exception and prints out the
> +    output and stderr.
> +
> +    Args:
> +        fname (str): Filename of FIP
> +        *fip_args: List of arguments to pass to fiptool
> +
> +    Returns:
> +        CommandResult: object containing the results
> +
> +    Raises:
> +        ValueError: the tool failed to run
> +    """
> +    args = ['fiptool', fname] + list(fip_args)
> +    result = command.RunPipe([args], capture=not VERBOSE,
> +                             capture_stderr=not VERBOSE, raise_on_error=False)
> +    if result.return_code:
> +        print(result.stderr, file=sys.stderr)
> +        raise ValueError("Failed to run (error %d): '%s'" %
> +                         (result.return_code, ' '.join(args)))
> +    return result
> +
> +
> +if __name__ == "__main__":
> +    sys.exit(main(sys.argv[1:], OUR_FILE))  # pragma: no cover
> diff --git a/tools/binman/fip_util_test.py b/tools/binman/fip_util_test.py
> new file mode 100755
> index 00000000000..4839b298762
> --- /dev/null
> +++ b/tools/binman/fip_util_test.py
> @@ -0,0 +1,405 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright 2021 Google LLC
> +# Written by Simon Glass <sjg@chromium.org>
> +
> +"""Tests for fip_util
> +
> +This tests a few features of fip_util which are not covered by binman's ftest.py
> +"""
> +
> +import os
> +import shutil
> +import sys
> +import tempfile
> +import unittest
> +
> +# Bring in the patman and dtoc libraries (but don't override the first path
> +# in PYTHONPATH)
> +OUR_PATH = os.path.dirname(os.path.realpath(__file__))
> +sys.path.insert(2, os.path.join(OUR_PATH, '..'))
> +
> +# pylint: disable=C0413
> +from patman import test_util
> +from patman import tools
> +import fip_util
> +
> +HAVE_FIPTOOL = True
> +try:
> +    tools.Run('which', 'fiptool')
> +except ValueError:
> +    HAVE_FIPTOOL = False
> +
> +# pylint: disable=R0902,R0904
> +class TestFip(unittest.TestCase):
> +    """Test of fip_util classes"""
> +    #pylint: disable=W0212
> +    def setUp(self):
> +        # Create a temporary directory for test files
> +        self._indir = tempfile.mkdtemp(prefix='fip_util.')
> +        tools.SetInputDirs([self._indir])
> +
> +        # Set up a temporary output directory, used by the tools library when
> +        # compressing files
> +        tools.PrepareOutputDir(None)
> +
> +        self.src_file = os.path.join(self._indir, 'orig.py')
> +        self.outname = tools.GetOutputFilename('out.py')
> +        self.args = ['-D', '-s', self._indir, '-o', self.outname]
> +        self.readme = os.path.join(self._indir, 'readme.rst')
> +        self.macro_dir = os.path.join(self._indir, 'include/tools_share')
> +        self.macro_fname = os.path.join(self.macro_dir,
> +                                        'firmware_image_package.h')
> +        self.name_dir = os.path.join(self._indir, 'tools/fiptool')
> +        self.name_fname = os.path.join(self.name_dir, 'tbbr_config.c')
> +
> +    macro_contents = '''
> +
> +/* ToC Entry UUIDs */
> +#define UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U \\
> +       {{0x65, 0x92, 0x27, 0x03}, {0x2f, 0x74}, {0xe6, 0x44}, 0x8d, 0xff, {0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10} }
> +#define UUID_TRUSTED_UPDATE_FIRMWARE_BL2U \\
> +       {{0x60, 0xb3, 0xeb, 0x37}, {0xc1, 0xe5}, {0xea, 0x41}, 0x9d, 0xf3, {0x19, 0xed, 0xa1, 0x1f, 0x68, 0x01} }
> +
> +'''
> +
> +    name_contents = '''
> +
> +toc_entry_t toc_entries[] = {
> +       {
> +               .name = "SCP Firmware Updater Configuration FWU SCP_BL2U",
> +               .uuid = UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U,
> +               .cmdline_name = "scp-fwu-cfg"
> +       },
> +       {
> +               .name = "AP Firmware Updater Configuration BL2U",
> +               .uuid = UUID_TRUSTED_UPDATE_FIRMWARE_BL2U,
> +               .cmdline_name = "ap-fwu-cfg"
> +       },
> +'''
> +
> +    def setup_readme(self):
> +        """Set up the readme.txt file"""
> +        tools.WriteFile(self.readme, 'Trusted Firmware-A\n==================',
> +                        binary=False)
> +
> +    def setup_macro(self, data=macro_contents):
> +        """Set up the tbbr_config.c file"""
> +        os.makedirs(self.macro_dir)
> +        tools.WriteFile(self.macro_fname, data, binary=False)
> +
> +    def setup_name(self, data=name_contents):
> +        """Set up the firmware_image_package.h file"""
> +        os.makedirs(self.name_dir)
> +        tools.WriteFile(self.name_fname, data, binary=False)
> +
> +    def tearDown(self):
> +        """Remove the temporary input directory and its contents"""
> +        if self._indir:
> +            shutil.rmtree(self._indir)
> +        self._indir = None
> +        tools.FinaliseOutputDir()
> +
> +    def test_no_readme(self):
> +        """Test handling of a missing readme.rst"""
> +        with self.assertRaises(Exception) as err:
> +            fip_util.main(self.args, self.src_file)
> +        self.assertIn('Expected file', str(err.exception))
> +
> +    def test_invalid_readme(self):
> +        """Test that an invalid readme.rst is detected"""
> +        tools.WriteFile(self.readme, 'blah', binary=False)
> +        with self.assertRaises(Exception) as err:
> +            fip_util.main(self.args, self.src_file)
> +        self.assertIn('does not start with', str(err.exception))
> +
> +    def test_no_fip_h(self):
> +        """Check handling of missing firmware_image_package.h"""
> +        self.setup_readme()
> +        with self.assertRaises(Exception) as err:
> +            fip_util.main(self.args, self.src_file)
> +        self.assertIn('No such file or directory', str(err.exception))
> +
> +    def test_invalid_fip_h(self):
> +        """Check failure to parse firmware_image_package.h"""
> +        self.setup_readme()
> +        self.setup_macro('blah')
> +        with self.assertRaises(Exception) as err:
> +            fip_util.main(self.args, self.src_file)
> +        self.assertIn('Cannot parse file', str(err.exception))
> +
> +    def test_parse_fip_h(self):
> +        """Check parsing of firmware_image_package.h"""
> +        self.setup_readme()
> +        # Check parsing the header file
> +        self.setup_macro()
> +        macros = fip_util.parse_macros(self._indir)
> +        expected_macros = {
> +            'UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U':
> +                ('ToC Entry UUIDs', 'UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U',
> +                 bytes([0x65, 0x92, 0x27, 0x03, 0x2f, 0x74, 0xe6, 0x44,
> +                        0x8d, 0xff, 0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10])),
> +            'UUID_TRUSTED_UPDATE_FIRMWARE_BL2U':
> +                ('ToC Entry UUIDs', 'UUID_TRUSTED_UPDATE_FIRMWARE_BL2U',
> +                 bytes([0x60, 0xb3, 0xeb, 0x37, 0xc1, 0xe5, 0xea, 0x41,
> +                        0x9d, 0xf3, 0x19, 0xed, 0xa1, 0x1f, 0x68, 0x01])),
> +            }
> +        self.assertEqual(expected_macros, macros)
> +
> +    def test_missing_tbbr_c(self):
> +        """Check handlinh of missing tbbr_config.c"""
> +        self.setup_readme()
> +        self.setup_macro()
> +
> +        # Still need the .c file
> +        with self.assertRaises(Exception) as err:
> +            fip_util.main(self.args, self.src_file)
> +        self.assertIn('tbbr_config.c', str(err.exception))
> +
> +    def test_invalid_tbbr_c(self):
> +        """Check failure to parse tbbr_config.c"""
> +        self.setup_readme()
> +        self.setup_macro()
> +        # Check invalid format for C file
> +        self.setup_name('blah')
> +        with self.assertRaises(Exception) as err:
> +            fip_util.main(self.args, self.src_file)
> +        self.assertIn('Cannot parse file', str(err.exception))
> +
> +    def test_inconsistent_tbbr_c(self):
> +        """Check tbbr_config.c in a format we don't expect"""
> +        self.setup_readme()
> +        # This is missing a hex value
> +        self.setup_macro('''
> +
> +/* ToC Entry UUIDs */
> +#define UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U \\
> +       {{0x65, 0x92, 0x27,}, {0x2f, 0x74}, {0xe6, 0x44}, 0x8d, 0xff, {0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10} }
> +#define UUID_TRUSTED_UPDATE_FIRMWARE_BL2U \\
> +       {{0x60, 0xb3, 0xeb, 0x37}, {0xc1, 0xe5}, {0xea, 0x41}, 0x9d, 0xf3, {0x19, 0xed, 0xa1, 0x1f, 0x68, 0x01} }
> +
> +''')
> +        # Check invalid format for C file
> +        self.setup_name('blah')
> +        with self.assertRaises(Exception) as err:
> +            fip_util.main(self.args, self.src_file)
> +        self.assertIn('Cannot parse UUID line 5', str(err.exception))
> +
> +    def test_parse_tbbr_c(self):
> +        """Check parsing tbbr_config.c"""
> +        self.setup_readme()
> +        self.setup_macro()
> +        self.setup_name()
> +
> +        names = fip_util.parse_names(self._indir)
> +
> +        expected_names = {
> +            'UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U': (
> +                'SCP Firmware Updater Configuration FWU SCP_BL2U',
> +                'UUID_TRUSTED_UPDATE_FIRMWARE_SCP_BL2U',
> +                'scp-fwu-cfg'),
> +            'UUID_TRUSTED_UPDATE_FIRMWARE_BL2U': (
> +                'AP Firmware Updater Configuration BL2U',
> +                'UUID_TRUSTED_UPDATE_FIRMWARE_BL2U',
> +                'ap-fwu-cfg'),
> +            }
> +        self.assertEqual(expected_names, names)
> +
> +    def test_uuid_not_in_tbbr_config_c(self):
> +        """Check handling a UUID in the header file that's not in the .c file"""
> +        self.setup_readme()
> +        self.setup_macro(self.macro_contents + '''
> +#define UUID_TRUSTED_OS_FW_KEY_CERT \\
> +       {{0x94,  0x77, 0xd6, 0x03}, {0xfb, 0x60}, {0xe4, 0x11}, 0x85, 0xdd, {0xb7, 0x10, 0x5b, 0x8c, 0xee, 0x04} }
> +
> +''')
> +        self.setup_name()
> +
> +        macros = fip_util.parse_macros(self._indir)
> +        names = fip_util.parse_names(self._indir)
> +        with test_util.capture_sys_output() as (stdout, _):
> +            fip_util.create_code_output(macros, names)
> +        self.assertIn(
> +            "UUID 'UUID_TRUSTED_OS_FW_KEY_CERT' is not mentioned in tbbr_config.c file",
> +            stdout.getvalue())
> +
> +    def test_changes(self):
> +        """Check handling of a source file that does/doesn't need changes"""
> +        self.setup_readme()
> +        self.setup_macro()
> +        self.setup_name()
> +
> +        # Check generating the file when changes are needed
> +        tools.WriteFile(self.src_file, '''
> +
> +# This is taken from tbbr_config.c in ARM Trusted Firmware
> +FIP_TYPE_LIST = [
> +    # ToC Entry UUIDs
> +    FipType('scp-fwu-cfg', 'SCP Firmware Updater Configuration FWU SCP_BL2U',
> +            [0x65, 0x92, 0x27, 0x03, 0x2f, 0x74, 0xe6, 0x44,
> +             0x8d, 0xff, 0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10]),
> +    ] # end
> +blah de blah
> +                        ''', binary=False)
> +        with test_util.capture_sys_output() as (stdout, _):
> +            fip_util.main(self.args, self.src_file)
> +        self.assertIn('Needs update', stdout.getvalue())
> +
> +        # Check generating the file when no changes are needed
> +        tools.WriteFile(self.src_file, '''
> +# This is taken from tbbr_config.c in ARM Trusted Firmware
> +FIP_TYPE_LIST = [
> +    # ToC Entry UUIDs
> +    FipType('scp-fwu-cfg', 'SCP Firmware Updater Configuration FWU SCP_BL2U',
> +            [0x65, 0x92, 0x27, 0x03, 0x2f, 0x74, 0xe6, 0x44,
> +             0x8d, 0xff, 0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10]),
> +    FipType('ap-fwu-cfg', 'AP Firmware Updater Configuration BL2U',
> +            [0x60, 0xb3, 0xeb, 0x37, 0xc1, 0xe5, 0xea, 0x41,
> +             0x9d, 0xf3, 0x19, 0xed, 0xa1, 0x1f, 0x68, 0x01]),
> +    ] # end
> +blah blah''', binary=False)
> +        with test_util.capture_sys_output() as (stdout, _):
> +            fip_util.main(self.args, self.src_file)
> +        self.assertIn('is up-to-date', stdout.getvalue())
> +
> +    def test_no_debug(self):
> +        """Test running without the -D flag"""
> +        self.setup_readme()
> +        self.setup_macro()
> +        self.setup_name()
> +
> +        args = self.args.copy()
> +        args.remove('-D')
> +        tools.WriteFile(self.src_file, '', binary=False)
> +        with test_util.capture_sys_output():
> +            fip_util.main(args, self.src_file)
> +
> +    @unittest.skipIf(not HAVE_FIPTOOL, 'No fiptool available')
> +    def test_fiptool_list(self):
> +        """Create a FIP and check that fiptool can read it"""
> +        fwu = b'my data'
> +        tb_fw = b'some more data'
> +        fip = fip_util.FipWriter(0x123, 0x10)
> +        fip.add_entry('fwu', fwu, 0x456)
> +        fip.add_entry('tb-fw', tb_fw, 0)
> +        fip.add_entry(bytes(range(16)), tb_fw, 0)
> +        data = fip.get_data()
> +        fname = tools.GetOutputFilename('data.fip')
> +        tools.WriteFile(fname, data)
> +        result = fip_util.fiptool('info', fname)
> +        self.assertEqual(
> +            '''Firmware Updater NS_BL2U: offset=0xB0, size=0x7, cmdline="--fwu"
> +Trusted Boot Firmware BL2: offset=0xC0, size=0xE, cmdline="--tb-fw"
> +00010203-0405-0607-0809-0A0B0C0D0E0F: offset=0xD0, size=0xE, cmdline="--blob"
> +''',
> +            result.stdout)
> +
> +    fwu_data = b'my data'
> +    tb_fw_data = b'some more data'
> +    other_fw_data = b'even more'
> +
> +    def create_fiptool_image(self):
> +        """Create an image with fiptool which we can use for testing
> +
> +        Returns:
> +            FipReader: reader for the image
> +        """
> +        fwu = os.path.join(self._indir, 'fwu')
> +        tools.WriteFile(fwu, self.fwu_data)
> +
> +        tb_fw = os.path.join(self._indir, 'tb_fw')
> +        tools.WriteFile(tb_fw, self.tb_fw_data)
> +
> +        other_fw = os.path.join(self._indir, 'other_fw')
> +        tools.WriteFile(other_fw, self.other_fw_data)
> +
> +        fname = tools.GetOutputFilename('data.fip')
> +        uuid = 'e3b78d9e-4a64-11ec-b45c-fba2b9b49788'
> +        fip_util.fiptool('create', '--align', '8', '--plat-toc-flags', '0x123',
> +                         '--fwu', fwu,
> +                         '--tb-fw', tb_fw,
> +                         '--blob', f'uuid={uuid},file={other_fw}',
> +                          fname)
> +
> +        return fip_util.FipReader(tools.ReadFile(fname))
> +
> +    @unittest.skipIf(not HAVE_FIPTOOL, 'No fiptool available')
> +    def test_fiptool_create(self):
> +        """Create a FIP with fiptool and check that fip_util can read it"""
> +        reader = self.create_fiptool_image()
> +
> +        header = reader.header
> +        fents = reader.fents
> +
> +        self.assertEqual(0x123 << 32, header.flags)
> +        self.assertEqual(fip_util.HEADER_MAGIC, header.name)
> +        self.assertEqual(fip_util.HEADER_SERIAL, header.serial)
> +
> +        self.assertEqual(3, len(fents))
> +        fent = fents[0]
> +        self.assertEqual(
> +            bytes([0x4f, 0x51, 0x1d, 0x11, 0x2b, 0xe5, 0x4e, 0x49,
> +                   0xb4, 0xc5, 0x83, 0xc2, 0xf7, 0x15, 0x84, 0x0a]), fent.uuid)
> +        self.assertEqual(0xb0, fent.offset)
> +        self.assertEqual(len(self.fwu_data), fent.size)
> +        self.assertEqual(0, fent.flags)
> +        self.assertEqual(self.fwu_data, fent.data)
> +
> +        fent = fents[1]
> +        self.assertEqual(
> +            bytes([0x5f, 0xf9, 0xec, 0x0b, 0x4d, 0x22, 0x3e, 0x4d,
> +             0xa5, 0x44, 0xc3, 0x9d, 0x81, 0xc7, 0x3f, 0x0a]), fent.uuid)
> +        self.assertEqual(0xb8, fent.offset)
> +        self.assertEqual(len(self.tb_fw_data), fent.size)
> +        self.assertEqual(0, fent.flags)
> +        self.assertEqual(self.tb_fw_data, fent.data)
> +
> +        fent = fents[2]
> +        self.assertEqual(
> +            bytes([0xe3, 0xb7, 0x8d, 0x9e, 0x4a, 0x64, 0x11, 0xec,
> +                   0xb4, 0x5c, 0xfb, 0xa2, 0xb9, 0xb4, 0x97, 0x88]), fent.uuid)
> +        self.assertEqual(0xc8, fent.offset)
> +        self.assertEqual(len(self.other_fw_data), fent.size)
> +        self.assertEqual(0, fent.flags)
> +        self.assertEqual(self.other_fw_data, fent.data)
> +
> +    @unittest.skipIf(not HAVE_FIPTOOL, 'No fiptool available')
> +    def test_reader_get_entry(self):
> +        """Test get_entry() by name and UUID"""
> +        reader = self.create_fiptool_image()
> +        fents = reader.fents
> +        fent = reader.get_entry('fwu')
> +        self.assertEqual(fent, fents[0])
> +
> +        fent = reader.get_entry(
> +            bytes([0x5f, 0xf9, 0xec, 0x0b, 0x4d, 0x22, 0x3e, 0x4d,
> +                   0xa5, 0x44, 0xc3, 0x9d, 0x81, 0xc7, 0x3f, 0x0a]))
> +        self.assertEqual(fent, fents[1])
> +
> +        # Try finding entries that don't exist
> +        with self.assertRaises(Exception) as err:
> +            fent = reader.get_entry('scp-fwu-cfg')
> +        self.assertIn("Cannot find FIP entry 'scp-fwu-cfg'", str(err.exception))
> +
> +        with self.assertRaises(Exception) as err:
> +            fent = reader.get_entry(bytes(list(range(16))))
> +        self.assertIn(
> +            "Cannot find FIP entry '00010203-0405-0607-0809-0a0b0c0d0e0f'",
> +            str(err.exception))
> +
> +        with self.assertRaises(Exception) as err:
> +            fent = reader.get_entry('blah')
> +        self.assertIn("Unknown FIP entry type 'blah'", str(err.exception))
> +
> +    @unittest.skipIf(not HAVE_FIPTOOL, 'No fiptool available')
> +    def test_fiptool_errors(self):
> +        """Check some error reporting from fiptool"""
> +        with self.assertRaises(Exception) as err:
> +            with test_util.capture_sys_output():
> +                fip_util.fiptool('create', '--fred')
> +        self.assertIn("Failed to run (error 1): 'fiptool create --fred'",
> +                      str(err.exception))
> +
> +
> +if __name__ == '__main__':
> +    unittest.main()
> diff --git a/tools/binman/main.py b/tools/binman/main.py
> index 8c1e478d54c..1a639f43e9e 100755
> --- a/tools/binman/main.py
> +++ b/tools/binman/main.py
> @@ -59,6 +59,7 @@ def RunTests(debug, verbosity, processes, test_preserve_dirs, args, toolpath):
>      from binman import elf_test
>      from binman import entry_test
>      from binman import fdt_test
> +    from binman import fip_util_test
>      from binman import ftest
>      from binman import image_test
>      import doctest
> @@ -72,7 +73,8 @@ def RunTests(debug, verbosity, processes, test_preserve_dirs, args, toolpath):
>          result, debug, verbosity, test_preserve_dirs, processes, test_name,
>          toolpath,
>          [entry_test.TestEntry, ftest.TestFunctional, fdt_test.TestFdt,
> -         elf_test.TestElf, image_test.TestImage, cbfs_util_test.TestCbfs])
> +         elf_test.TestElf, image_test.TestImage, cbfs_util_test.TestCbfs,
> +         fip_util_test.TestFip])
>
>      return test_util.ReportResult('binman', test_name, result)
>
> --
> 2.34.0.rc2.393.gf8c9666880-goog
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] binman: Add support for ATF FIP
  2021-11-24  4:08 ` [PATCH 2/2] binman: Add support " Simon Glass
@ 2021-11-25 14:47   ` Ilias Apalodimas
  2021-12-15  0:33   ` Simon Glass
  1 sibling, 0 replies; 18+ messages in thread
From: Ilias Apalodimas @ 2021-11-25 14:47 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List, Sandrine Bailleux

+CC Sandrine

On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org> wrote:
>
> This format is used in firmware binaries so we may as well supported it.
>
> With this patch binman supports creating, listing and updating FIPs, as
> well as extracting files from one, provided that an FDTMAP is also present
> somewhere in the image.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  scripts/pylint.base                      |   1 +
>  tools/binman/entries.rst                 | 154 +++++++++++++
>  tools/binman/etype/atf_fip.py            | 273 +++++++++++++++++++++++
>  tools/binman/ftest.py                    | 217 ++++++++++++++++++
>  tools/binman/test/203_fip.dts            |  21 ++
>  tools/binman/test/204_fip_other.dts      |  22 ++
>  tools/binman/test/205_fip_no_type.dts    |  15 ++
>  tools/binman/test/206_fip_uuid.dts       |  22 ++
>  tools/binman/test/207_fip_ls.dts         |  25 +++
>  tools/binman/test/208_fip_replace.dts    |  33 +++
>  tools/binman/test/209_fip_missing.dts    |  19 ++
>  tools/binman/test/210_fip_size.dts       |  19 ++
>  tools/binman/test/211_fip_bad_align.dts  |  18 ++
>  tools/binman/test/212_fip_collection.dts |  24 ++
>  14 files changed, 863 insertions(+)
>  create mode 100644 tools/binman/etype/atf_fip.py
>  create mode 100644 tools/binman/test/203_fip.dts
>  create mode 100644 tools/binman/test/204_fip_other.dts
>  create mode 100644 tools/binman/test/205_fip_no_type.dts
>  create mode 100644 tools/binman/test/206_fip_uuid.dts
>  create mode 100644 tools/binman/test/207_fip_ls.dts
>  create mode 100644 tools/binman/test/208_fip_replace.dts
>  create mode 100644 tools/binman/test/209_fip_missing.dts
>  create mode 100644 tools/binman/test/210_fip_size.dts
>  create mode 100644 tools/binman/test/211_fip_bad_align.dts
>  create mode 100644 tools/binman/test/212_fip_collection.dts
>
> diff --git a/scripts/pylint.base b/scripts/pylint.base
> index a35dbe34d2d..3d891edf261 100644
> --- a/scripts/pylint.base
> +++ b/scripts/pylint.base
> @@ -1,5 +1,6 @@
>  _testing 0.83
>  atf_bl31 -6.00
> +atf_fip 0.44
>  binman.cbfs_util 7.70
>  binman.cbfs_util_test 9.19
>  binman.cmdline 9.06
> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
> index 748277c1cde..84f828a6352 100644
> --- a/tools/binman/entries.rst
> +++ b/tools/binman/entries.rst
> @@ -25,6 +25,160 @@ about ATF.
>
>
>
> +Entry: atf-fip: ARM Trusted Firmware's Firmware Image Package (FIP)
> +-------------------------------------------------------------------
> +
> +A FIP_ provides a way to group binaries in a firmware image, used by ARM's
> +Trusted Firmware A (TF-A) code. It is a simple format consisting of a
> +table of contents with information about the type, offset and size of the
> +binaries in the FIP. It is quite similar to FMAP, with the major difference
> +that it uses UUIDs to indicate the type of each entry.
> +
> +Note: It is recommended to always add an fdtmap to every image, as well as
> +any FIPs so that binman and other tools can access the entire image
> +correctly.
> +
> +The UUIDs correspond to useful names in `fiptool`, provided by ATF to
> +operate on FIPs. Binman uses these names to make it easier to understand
> +what is going on, although it is possible to provide a UUID if needed.
> +
> +The contents of the FIP are defined by subnodes of the atf-fip entry, e.g.::
> +
> +    atf-fip {
> +        soc-fw {
> +            filename = "bl31.bin";
> +        };
> +
> +        scp-fwu-cfg {
> +            filename = "bl2u.bin";
> +        };
> +
> +        u-boot {
> +            fip-type = "nt-fw";
> +        };
> +    };
> +
> +This describes a FIP with three entries: soc-fw, scp-fwu-cfg and nt-fw.
> +You can use normal (non-external) binaries like U-Boot simply by adding a
> +FIP type, with the `fip-type` property, as above.
> +
> +Since FIP exists to bring blobs together, Binman assumes that all FIP
> +entries are external binaries. If a binary may not exist, you can use the
> +`--allow-missing` flag to Binman, in which case the image is still created,
> +even though it will not actually work.
> +
> +The size of the FIP depends on the size of the binaries. There is currently
> +no way to specify a fixed size. If the `atf-fip` node has a `size` entry,
> +this affects the space taken up by the `atf-fip` entry, but the FIP itself
> +does not expand to use that space.
> +
> +Some other FIP features are available with Binman. The header and the
> +entries have 64-bit flag works. The flag flags do not seem to be defined
> +anywhere, but you can use `fip-hdr-flags` and fip-flags` to set the values
> +of the header and entries respectively.
> +
> +FIP entries can be aligned to a particular power-of-two boundary. Use
> +fip-align for this.
> +
> +Binman only understands the entry types that are included in its
> +implementation. It is possible to specify a 16-byte UUID instead, using the
> +fip-uuid property. In this case Binman doesn't know what its type is, so
> +just uses the UUID. See the `u-boot` node in this example::
> +
> +    binman {
> +        atf-fip {
> +            fip-hdr-flags = /bits/ 64 <0x123>;
> +            fip-align = <16>;
> +            soc-fw {
> +                fip-flags = /bits/ 64 <0x456>;
> +                filename = "bl31.bin";
> +            };
> +
> +            scp-fwu-cfg {
> +                filename = "bl2u.bin";
> +            };
> +
> +            u-boot {
> +                fip-uuid = [fc 65 13 92 4a 5b 11 ec
> +                            94 35 ff 2d 1c fc 79 9c];
> +            };
> +        };
> +        fdtmap {
> +        };
> +    };
> +
> +Binman allows reading and updating FIP entries after the image is created,
> +provided that an FDPMAP is present too. Updates which change the size of a
> +FIP entry will cause it to be expanded or contracted as needed.
> +
> +Properties for top-level atf-fip node
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +fip-hdr-flags (64 bits)
> +    Sets the flags for the FIP header.
> +
> +Properties for subnodes
> +~~~~~~~~~~~~~~~~~~~~~~~
> +
> +fip-type (str)
> +    FIP type to use for this entry. This is needed if the entry
> +    name is not a valid type. Value types are defined in `fip_util.py`.
> +    The FIP type defines the UUID that is used (they map 1:1).
> +
> +fip-uuid (16 bytes)
> +    If there is no FIP-type name defined, or it is not supported by Binman,
> +    this property sets the UUID. It should be a 16-byte value, following the
> +    hex digits of the UUID.
> +
> +fip-flags (64 bits)
> +    Set the flags for a FIP entry. Use in one of the subnodes of the
> +    7atf-fip entry.
> +
> +fip-align
> +    Set the alignment for a FIP entry, FIP entries can be aligned to a
> +    particular power-of-two boundary. The default is 1.
> +
> +Adding new FIP-entry types
> +~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +When new FIP entries are defined by TF-A they appear in the
> +`TF-A source tree`_. You can use `fip_util.py` to update Binman to support
> +new types, then `send a patch`_ to the U-Boot mailing list. There are two
> +source files that the tool examples:
> +
> +- `include/tools_share/firmware_image_package.h` has the UUIDs
> +- `tools/fiptool/tbbr_config.c` has the name and descripion for each UUID
> +
> +To run the tool::
> +
> +    $ tools/binman/fip_util.py  -s /path/to/arm-trusted-firmware
> +    Warning: UUID 'UUID_NON_TRUSTED_WORLD_KEY_CERT' is not mentioned in tbbr_config.c file
> +    Existing code in 'tools/binman/fip_util.py' is up-to-date
> +
> +If it shows there is an update, it writes a new version of `fip_util.py`
> +to `fip_util.py.out`. You can change the output file using the `-i` flag.
> +If you have a problem, use `-D` to enable traceback debugging.
> +
> +FIP commentary
> +~~~~~~~~~~~~~~
> +
> +As a side effect of use of UUIDs, FIP does not support multiple
> +entries of the same type, such as might be used to store fonts or graphics
> +icons, for example. For verified boot it could be used for each part of the
> +image (e.g. separate FIPs for A and B) but cannot describe the whole
> +firmware image. As with FMAP there is no hierarchy defined, although FMAP
> +works around this by having 'section' areas which encompass others. A
> +similar workaround would be possible with FIP but is not currently defined.
> +
> +It is recommended to always add an fdtmap to every image, as well as any
> +FIPs so that binman and other tools can access the entire image correctly.
> +
> +.. _FIP: https://trustedfirmware-a.readthedocs.io/en/latest/design/firmware-design.html#firmware-image-package-fip
> +.. _`TF-A source tree`: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git
> +.. _`send a patch`: https://www.denx.de/wiki/U-Boot/Patches
> +
> +
> +
>  Entry: blob: Arbitrary binary blob
>  ----------------------------------
>
> diff --git a/tools/binman/etype/atf_fip.py b/tools/binman/etype/atf_fip.py
> new file mode 100644
> index 00000000000..f9039e14c0a
> --- /dev/null
> +++ b/tools/binman/etype/atf_fip.py
> @@ -0,0 +1,273 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright 2019 Google LLC
> +# Written by Simon Glass <sjg@chromium.org>
> +#
> +# Entry-type module for the ARM Trusted Firmware's Firmware Image Package (FIP)
> +# format
> +
> +from collections import OrderedDict
> +
> +from binman.entry import Entry
> +from binman.etype.section import Entry_section
> +from binman.fip_util import FIP_TYPES, FipReader, FipWriter, UUID_LEN
> +from dtoc import fdt_util
> +from patman import tools
> +
> +class Entry_atf_fip(Entry_section):
> +    """ARM Trusted Firmware's Firmware Image Package (FIP)
> +
> +    A FIP_ provides a way to group binaries in a firmware image, used by ARM's
> +    Trusted Firmware A (TF-A) code. It is a simple format consisting of a
> +    table of contents with information about the type, offset and size of the
> +    binaries in the FIP. It is quite similar to FMAP, with the major difference
> +    that it uses UUIDs to indicate the type of each entry.
> +
> +    Note: It is recommended to always add an fdtmap to every image, as well as
> +    any FIPs so that binman and other tools can access the entire image
> +    correctly.
> +
> +    The UUIDs correspond to useful names in `fiptool`, provided by ATF to
> +    operate on FIPs. Binman uses these names to make it easier to understand
> +    what is going on, although it is possible to provide a UUID if needed.
> +
> +    The contents of the FIP are defined by subnodes of the atf-fip entry, e.g.::
> +
> +        atf-fip {
> +            soc-fw {
> +                filename = "bl31.bin";
> +            };
> +
> +            scp-fwu-cfg {
> +                filename = "bl2u.bin";
> +            };
> +
> +            u-boot {
> +                fip-type = "nt-fw";
> +            };
> +        };
> +
> +    This describes a FIP with three entries: soc-fw, scp-fwu-cfg and nt-fw.
> +    You can use normal (non-external) binaries like U-Boot simply by adding a
> +    FIP type, with the `fip-type` property, as above.
> +
> +    Since FIP exists to bring blobs together, Binman assumes that all FIP
> +    entries are external binaries. If a binary may not exist, you can use the
> +    `--allow-missing` flag to Binman, in which case the image is still created,
> +    even though it will not actually work.
> +
> +    The size of the FIP depends on the size of the binaries. There is currently
> +    no way to specify a fixed size. If the `atf-fip` node has a `size` entry,
> +    this affects the space taken up by the `atf-fip` entry, but the FIP itself
> +    does not expand to use that space.
> +
> +    Some other FIP features are available with Binman. The header and the
> +    entries have 64-bit flag works. The flag flags do not seem to be defined
> +    anywhere, but you can use `fip-hdr-flags` and fip-flags` to set the values
> +    of the header and entries respectively.
> +
> +    FIP entries can be aligned to a particular power-of-two boundary. Use
> +    fip-align for this.
> +
> +    Binman only understands the entry types that are included in its
> +    implementation. It is possible to specify a 16-byte UUID instead, using the
> +    fip-uuid property. In this case Binman doesn't know what its type is, so
> +    just uses the UUID. See the `u-boot` node in this example::
> +
> +        binman {
> +            atf-fip {
> +                fip-hdr-flags = /bits/ 64 <0x123>;
> +                fip-align = <16>;
> +                soc-fw {
> +                    fip-flags = /bits/ 64 <0x456>;
> +                    filename = "bl31.bin";
> +                };
> +
> +                scp-fwu-cfg {
> +                    filename = "bl2u.bin";
> +                };
> +
> +                u-boot {
> +                    fip-uuid = [fc 65 13 92 4a 5b 11 ec
> +                                94 35 ff 2d 1c fc 79 9c];
> +                };
> +            };
> +            fdtmap {
> +            };
> +        };
> +
> +    Binman allows reading and updating FIP entries after the image is created,
> +    provided that an FDPMAP is present too. Updates which change the size of a
> +    FIP entry will cause it to be expanded or contracted as needed.
> +
> +    Properties for top-level atf-fip node
> +    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +    fip-hdr-flags (64 bits)
> +        Sets the flags for the FIP header.
> +
> +    Properties for subnodes
> +    ~~~~~~~~~~~~~~~~~~~~~~~
> +
> +    fip-type (str)
> +        FIP type to use for this entry. This is needed if the entry
> +        name is not a valid type. Value types are defined in `fip_util.py`.
> +        The FIP type defines the UUID that is used (they map 1:1).
> +
> +    fip-uuid (16 bytes)
> +        If there is no FIP-type name defined, or it is not supported by Binman,
> +        this property sets the UUID. It should be a 16-byte value, following the
> +        hex digits of the UUID.
> +
> +    fip-flags (64 bits)
> +        Set the flags for a FIP entry. Use in one of the subnodes of the
> +        7atf-fip entry.
> +
> +    fip-align
> +        Set the alignment for a FIP entry, FIP entries can be aligned to a
> +        particular power-of-two boundary. The default is 1.
> +
> +    Adding new FIP-entry types
> +    ~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> +    When new FIP entries are defined by TF-A they appear in the
> +    `TF-A source tree`_. You can use `fip_util.py` to update Binman to support
> +    new types, then `send a patch`_ to the U-Boot mailing list. There are two
> +    source files that the tool examples:
> +
> +    - `include/tools_share/firmware_image_package.h` has the UUIDs
> +    - `tools/fiptool/tbbr_config.c` has the name and descripion for each UUID
> +
> +    To run the tool::
> +
> +        $ tools/binman/fip_util.py  -s /path/to/arm-trusted-firmware
> +        Warning: UUID 'UUID_NON_TRUSTED_WORLD_KEY_CERT' is not mentioned in tbbr_config.c file
> +        Existing code in 'tools/binman/fip_util.py' is up-to-date
> +
> +    If it shows there is an update, it writes a new version of `fip_util.py`
> +    to `fip_util.py.out`. You can change the output file using the `-i` flag.
> +    If you have a problem, use `-D` to enable traceback debugging.
> +
> +    FIP commentary
> +    ~~~~~~~~~~~~~~
> +
> +    As a side effect of use of UUIDs, FIP does not support multiple
> +    entries of the same type, such as might be used to store fonts or graphics
> +    icons, for example. For verified boot it could be used for each part of the
> +    image (e.g. separate FIPs for A and B) but cannot describe the whole
> +    firmware image. As with FMAP there is no hierarchy defined, although FMAP
> +    works around this by having 'section' areas which encompass others. A
> +    similar workaround would be possible with FIP but is not currently defined.
> +
> +    It is recommended to always add an fdtmap to every image, as well as any
> +    FIPs so that binman and other tools can access the entire image correctly.
> +
> +    .. _FIP: https://trustedfirmware-a.readthedocs.io/en/latest/design/firmware-design.html#firmware-image-package-fip
> +    .. _`TF-A source tree`: https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git
> +    .. _`send a patch`: https://www.denx.de/wiki/U-Boot/Patches
> +    """
> +    def __init__(self, section, etype, node):
> +        # Put this here to allow entry-docs and help to work without libfdt
> +        global state
> +        from binman import state
> +
> +        super().__init__(section, etype, node)
> +        self.align_default = None
> +        self._entries = OrderedDict()
> +        self.reader = None
> +
> +    def ReadNode(self):
> +        """Read properties from the atf-fip node"""
> +        super().ReadNode()
> +        self._pad_byte = fdt_util.GetInt(self._node, 'pad-byte', 0)
> +        self._fip_flags = fdt_util.GetInt64(self._node, 'fip-hdr-flags', 0)
> +        self._fip_align = fdt_util.GetInt(self._node, 'fip-align', 1)
> +        if tools.NotPowerOfTwo(self._fip_align):
> +            raise ValueError("Node '%s': FIP alignment %s must be a power of two" %
> +                             (self._node.path, self._fip_align))
> +        self.ReadEntries()
> +
> +    def ReadEntries(self):
> +        """Read the subnodes to find out what should go in this FIP"""
> +        for node in self._node.subnodes:
> +            fip_type = None
> +            etype = None
> +            if node.name in FIP_TYPES:
> +                fip_type = node.name
> +                etype = 'blob-ext'
> +
> +            entry = Entry.Create(self, node, etype)
> +            entry._fip_uuid = fdt_util.GetBytes(node, 'fip-uuid', UUID_LEN)
> +            if not fip_type and not entry._fip_uuid:
> +                fip_type = fdt_util.GetString(node, 'fip-type')
> +                if not fip_type:
> +                    self.Raise("Must provide a fip-type (node name '%s' is not a known FIP type)" %
> +                               node.name)
> +
> +            entry._fip_type = fip_type
> +            entry._fip_flags = fdt_util.GetInt64(node, 'fip-flags', 0)
> +            entry.ReadNode()
> +            entry._fip_name = node.name
> +            self._entries[entry._fip_name] = entry
> +
> +    def BuildSectionData(self, required):
> +        """Override this function to create a custom format for the entries
> +
> +        Arguments:
> +            required (bool): True if the data must be valid, False if it may
> +                be missing (entry.GetData() returns None
> +
> +        Returns:
> +            bytes: Data obtained, or None if None
> +        """
> +        fip = FipWriter(self._fip_flags, self._fip_align)
> +        for entry in self._entries.values():
> +            # First get the input data and put it in an entry. If not available,
> +            # try later.
> +            entry_data = entry.GetData(required)
> +            if not required and entry_data is None:
> +                return None
> +            fent = fip.add_entry(entry._fip_type or entry._fip_uuid, entry_data,
> +                                 entry._fip_flags)
> +            if fent:
> +                entry._fip_entry = fent
> +        data = fip.get_data()
> +        return data
> +
> +    def SetImagePos(self, image_pos):
> +        """Override this function to set all the entry properties from FIP
> +
> +        We can only do this once image_pos is known
> +
> +        Args:
> +            image_pos: Position of this entry in the image
> +        """
> +        super().SetImagePos(image_pos)
> +
> +        # Now update the entries with info from the FIP entries
> +        for entry in self._entries.values():
> +            fent = entry._fip_entry
> +            entry.size = fent.size
> +            entry.offset = fent.offset
> +            entry.image_pos = self.image_pos + entry.offset
> +
> +    def ReadChildData(self, child, decomp=True):
> +        if not self.reader:
> +            self.fip_data = super().ReadData(True)
> +            self.reader = FipReader(self.fip_data)
> +        reader = self.reader
> +
> +        # It is tricky to obtain the data from a FIP entry since it is indexed
> +        # by its UUID.
> +        fent = reader.get_entry(child._fip_type or child._fip_uuid)
> +        return fent.data
> +
> +        # Note:
> +        # It is also possible to extract it using the offsets directly, but this
> +        # seems less FIP_friendly:
> +        # return self.fip_data[child.offset:child.offset + child.size]
> +
> +    def WriteChildData(self, child):
> +        # Recreate the data structure, leaving the data for this child alone,
> +        # so that child.data is used to pack into the FIP.
> +        self.ObtainContents(skip_entry=child)
> +        return True
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index 0f4330b6807..d8ddbc81c1f 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -23,6 +23,7 @@ from binman import cmdline
>  from binman import control
>  from binman import elf
>  from binman import elf_test
> +from binman import fip_util
>  from binman import fmap_util
>  from binman import state
>  from dtoc import fdt
> @@ -76,6 +77,7 @@ FSP_M_DATA            = b'fsp_m'
>  FSP_S_DATA            = b'fsp_s'
>  FSP_T_DATA            = b'fsp_t'
>  ATF_BL31_DATA         = b'bl31'
> +ATF_BL2U_DATA         = b'bl2u'
>  OPENSBI_DATA          = b'opensbi'
>  SCP_DATA              = b'scp'
>  TEST_FDT1_DATA        = b'fdt1'
> @@ -179,6 +181,7 @@ class TestFunctional(unittest.TestCase):
>          TestFunctional._MakeInputFile('compress', COMPRESS_DATA)
>          TestFunctional._MakeInputFile('compress_big', COMPRESS_DATA_BIG)
>          TestFunctional._MakeInputFile('bl31.bin', ATF_BL31_DATA)
> +        TestFunctional._MakeInputFile('bl2u.bin', ATF_BL2U_DATA)
>          TestFunctional._MakeInputFile('fw_dynamic.bin', OPENSBI_DATA)
>          TestFunctional._MakeInputFile('scp.bin', SCP_DATA)
>
> @@ -4681,6 +4684,220 @@ class TestFunctional(unittest.TestCase):
>                          binary=False)
>          self.assertEqual(version, state.GetVersion(self._indir))
>
> +    def testFip(self):
> +        """Basic test of generation of an ARM Firmware Image Package (FIP)"""
> +        data = self._DoReadFile('203_fip.dts')
> +        hdr, fents = fip_util.decode_fip(data)
> +        self.assertEqual(fip_util.HEADER_MAGIC, hdr.name)
> +        self.assertEqual(fip_util.HEADER_SERIAL, hdr.serial)
> +        self.assertEqual(0x123, hdr.flags)
> +
> +        self.assertEqual(2, len(fents))
> +
> +        fent = fents[0]
> +        self.assertEqual(
> +            bytes([0x47,  0xd4, 0x08, 0x6d, 0x4c, 0xfe, 0x98, 0x46,
> +                  0x9b, 0x95, 0x29, 0x50, 0xcb, 0xbd, 0x5a, 0x0]), fent.uuid)
> +        self.assertEqual('soc-fw', fent.fip_type)
> +        self.assertEqual(0x88, fent.offset)
> +        self.assertEqual(len(ATF_BL31_DATA), fent.size)
> +        self.assertEqual(0x123456789abcdef, fent.flags)
> +        self.assertEqual(ATF_BL31_DATA, fent.data)
> +        self.assertEqual(True, fent.valid)
> +
> +        fent = fents[1]
> +        self.assertEqual(
> +            bytes([0x65, 0x92, 0x27, 0x03, 0x2f, 0x74, 0xe6, 0x44,
> +             0x8d, 0xff, 0x57, 0x9a, 0xc1, 0xff, 0x06, 0x10]), fent.uuid)
> +        self.assertEqual('scp-fwu-cfg', fent.fip_type)
> +        self.assertEqual(0x8c, fent.offset)
> +        self.assertEqual(len(ATF_BL31_DATA), fent.size)
> +        self.assertEqual(0, fent.flags)
> +        self.assertEqual(ATF_BL2U_DATA, fent.data)
> +        self.assertEqual(True, fent.valid)
> +
> +    def testFipOther(self):
> +        """Basic FIP with something that isn't a external blob"""
> +        data = self._DoReadFile('204_fip_other.dts')
> +        hdr, fents = fip_util.decode_fip(data)
> +
> +        self.assertEqual(2, len(fents))
> +        fent = fents[1]
> +        self.assertEqual('rot-cert', fent.fip_type)
> +        self.assertEqual(b'aa', fent.data)
> +
> +    def testFipOther(self):
> +        """Basic FIP with something that isn't a external blob"""
> +        data = self._DoReadFile('204_fip_other.dts')
> +        hdr, fents = fip_util.decode_fip(data)
> +
> +        self.assertEqual(2, len(fents))
> +        fent = fents[1]
> +        self.assertEqual('rot-cert', fent.fip_type)
> +        self.assertEqual(b'aa', fent.data)
> +
> +    def testFipNoType(self):
> +        """FIP with an entry of an unknown type"""
> +        with self.assertRaises(ValueError) as e:
> +            self._DoReadFile('205_fip_no_type.dts')
> +        self.assertIn("Must provide a fip-type (node name 'u-boot' is not a known FIP type)",
> +                      str(e.exception))
> +
> +    def testFipUuid(self):
> +        """Basic FIP with a manual uuid"""
> +        data = self._DoReadFile('206_fip_uuid.dts')
> +        hdr, fents = fip_util.decode_fip(data)
> +
> +        self.assertEqual(2, len(fents))
> +        fent = fents[1]
> +        self.assertEqual(None, fent.fip_type)
> +        self.assertEqual(
> +            bytes([0xfc, 0x65, 0x13, 0x92, 0x4a, 0x5b, 0x11, 0xec,
> +                   0x94, 0x35, 0xff, 0x2d, 0x1c, 0xfc, 0x79, 0x9c]),
> +            fent.uuid)
> +        self.assertEqual(U_BOOT_DATA, fent.data)
> +
> +    def testFipLs(self):
> +        """Test listing a FIP"""
> +        data = self._DoReadFileRealDtb('207_fip_ls.dts')
> +        hdr, fents = fip_util.decode_fip(data)
> +
> +        try:
> +            tmpdir, updated_fname = self._SetupImageInTmpdir()
> +            with test_util.capture_sys_output() as (stdout, stderr):
> +                self._DoBinman('ls', '-i', updated_fname)
> +        finally:
> +            shutil.rmtree(tmpdir)
> +        lines = stdout.getvalue().splitlines()
> +        expected = [
> +'Name          Image-pos  Size  Entry-type  Offset  Uncomp-size',
> +'----------------------------------------------------------------',
> +'main-section          0   2d3  section          0',
> +'  atf-fip             0    90  atf-fip          0',
> +'    soc-fw           88     4  blob-ext        88',
> +'    u-boot           8c     4  u-boot          8c',
> +'  fdtmap             90   243  fdtmap          90',
> +]
> +        self.assertEqual(expected, lines)
> +
> +        image = control.images['image']
> +        entries = image.GetEntries()
> +        fdtmap = entries['fdtmap']
> +
> +        fdtmap_data = data[fdtmap.image_pos:fdtmap.image_pos + fdtmap.size]
> +        magic = fdtmap_data[:8]
> +        self.assertEqual(b'_FDTMAP_', magic)
> +        self.assertEqual(tools.GetBytes(0, 8), fdtmap_data[8:16])
> +
> +        fdt_data = fdtmap_data[16:]
> +        dtb = fdt.Fdt.FromData(fdt_data)
> +        dtb.Scan()
> +        props = self._GetPropTree(dtb, BASE_DTB_PROPS, prefix='/')
> +        self.assertEqual({
> +            'atf-fip/soc-fw:image-pos': 136,
> +            'atf-fip/soc-fw:offset': 136,
> +            'atf-fip/soc-fw:size': 4,
> +            'atf-fip/u-boot:image-pos': 140,
> +            'atf-fip/u-boot:offset': 140,
> +            'atf-fip/u-boot:size': 4,
> +            'atf-fip:image-pos': 0,
> +            'atf-fip:offset': 0,
> +            'atf-fip:size': 144,
> +            'image-pos': 0,
> +            'offset': 0,
> +            'fdtmap:image-pos': fdtmap.image_pos,
> +            'fdtmap:offset': fdtmap.offset,
> +            'fdtmap:size': len(fdtmap_data),
> +            'size': len(data),
> +        }, props)
> +
> +    def testFipExtractOneEntry(self):
> +        """Test extracting a single entry fron an FIP"""
> +        self._DoReadFileRealDtb('207_fip_ls.dts')
> +        image_fname = tools.GetOutputFilename('image.bin')
> +        fname = os.path.join(self._indir, 'output.extact')
> +        control.ExtractEntries(image_fname, fname, None, ['atf-fip/u-boot'])
> +        data = tools.ReadFile(fname)
> +        self.assertEqual(U_BOOT_DATA, data)
> +
> +    def testFipReplace(self):
> +        """Test replacing a single file in a FIP"""
> +        expected = U_BOOT_DATA + tools.GetBytes(0x78, 50)
> +        data = self._DoReadFileRealDtb('208_fip_replace.dts')
> +        updated_fname = tools.GetOutputFilename('image-updated.bin')
> +        tools.WriteFile(updated_fname, data)
> +        entry_name = 'atf-fip/u-boot'
> +        control.WriteEntry(updated_fname, entry_name, expected,
> +                           allow_resize=True)
> +        actual = control.ReadEntry(updated_fname, entry_name)
> +        self.assertEqual(expected, actual)
> +
> +        new_data = tools.ReadFile(updated_fname)
> +        hdr, fents = fip_util.decode_fip(new_data)
> +
> +        self.assertEqual(2, len(fents))
> +
> +        # Check that the FIP entry is updated
> +        fent = fents[1]
> +        self.assertEqual(0x8c, fent.offset)
> +        self.assertEqual(len(expected), fent.size)
> +        self.assertEqual(0, fent.flags)
> +        self.assertEqual(expected, fent.data)
> +        self.assertEqual(True, fent.valid)
> +
> +    def testFipMissing(self):
> +        with test_util.capture_sys_output() as (stdout, stderr):
> +            self._DoTestFile('209_fip_missing.dts', allow_missing=True)
> +        err = stderr.getvalue()
> +        self.assertRegex(err, "Image 'main-section'.*missing.*: rmm-fw")
> +
> +    def testFipSize(self):
> +        """Test a FIP with a size property"""
> +        data = self._DoReadFile('210_fip_size.dts')
> +        self.assertEqual(0x100 + len(U_BOOT_DATA), len(data))
> +        hdr, fents = fip_util.decode_fip(data)
> +        self.assertEqual(fip_util.HEADER_MAGIC, hdr.name)
> +        self.assertEqual(fip_util.HEADER_SERIAL, hdr.serial)
> +
> +        self.assertEqual(1, len(fents))
> +
> +        fent = fents[0]
> +        self.assertEqual('soc-fw', fent.fip_type)
> +        self.assertEqual(0x60, fent.offset)
> +        self.assertEqual(len(ATF_BL31_DATA), fent.size)
> +        self.assertEqual(ATF_BL31_DATA, fent.data)
> +        self.assertEqual(True, fent.valid)
> +
> +        rest = data[0x60 + len(ATF_BL31_DATA):0x100]
> +        self.assertEqual(tools.GetBytes(0xff, len(rest)), rest)
> +
> +    def testFipBadAlign(self):
> +        """Test that an invalid alignment value in a FIP is detected"""
> +        with self.assertRaises(ValueError) as e:
> +            self._DoTestFile('211_fip_bad_align.dts')
> +        self.assertIn(
> +            "Node \'/binman/atf-fip\': FIP alignment 31 must be a power of two",
> +            str(e.exception))
> +
> +    def testFipCollection(self):
> +        """Test using a FIP in a collection"""
> +        data = self._DoReadFile('212_fip_collection.dts')
> +        entry1 = control.images['image'].GetEntries()['collection']
> +        data1 = data[:entry1.size]
> +        hdr1, fents2 = fip_util.decode_fip(data1)
> +
> +        entry2 = control.images['image'].GetEntries()['atf-fip']
> +        data2 = data[entry2.offset:entry2.offset + entry2.size]
> +        hdr1, fents2 = fip_util.decode_fip(data2)
> +
> +        # The 'collection' entry should have U-Boot included at the end
> +        self.assertEqual(entry1.size - len(U_BOOT_DATA), entry2.size)
> +        self.assertEqual(data1, data2 + U_BOOT_DATA)
> +        self.assertEqual(U_BOOT_DATA, data1[-4:])
> +
> +        # There should be a U-Boot after the final FIP
> +        self.assertEqual(U_BOOT_DATA, data[-4:])
> +
>
>  if __name__ == "__main__":
>      unittest.main()
> diff --git a/tools/binman/test/203_fip.dts b/tools/binman/test/203_fip.dts
> new file mode 100644
> index 00000000000..08973373240
> --- /dev/null
> +++ b/tools/binman/test/203_fip.dts
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               atf-fip {
> +                       fip-hdr-flags = /bits/ 64 <0x123>;
> +                       soc-fw {
> +                               fip-flags = /bits/ 64 <0x123456789abcdef>;
> +                               filename = "bl31.bin";
> +                       };
> +
> +                       scp-fwu-cfg {
> +                               filename = "bl2u.bin";
> +                       };
> +               };
> +       };
> +};
> diff --git a/tools/binman/test/204_fip_other.dts b/tools/binman/test/204_fip_other.dts
> new file mode 100644
> index 00000000000..65039410986
> --- /dev/null
> +++ b/tools/binman/test/204_fip_other.dts
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               atf-fip {
> +                       fip-hdr-flags = /bits/ 64 <0x123>;
> +                       soc-fw {
> +                               fip-flags = /bits/ 64 <0x123456789abcdef>;
> +                               filename = "bl31.bin";
> +                       };
> +
> +                       _testing {
> +                               fip-type = "rot-cert";
> +                               return-contents-later;
> +                       };
> +               };
> +       };
> +};
> diff --git a/tools/binman/test/205_fip_no_type.dts b/tools/binman/test/205_fip_no_type.dts
> new file mode 100644
> index 00000000000..23c8c3bc37e
> --- /dev/null
> +++ b/tools/binman/test/205_fip_no_type.dts
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               atf-fip {
> +                       fip-hdr-flags = /bits/ 64 <0x123>;
> +                       u-boot {
> +                       };
> +               };
> +       };
> +};
> diff --git a/tools/binman/test/206_fip_uuid.dts b/tools/binman/test/206_fip_uuid.dts
> new file mode 100644
> index 00000000000..c9bd44f9c31
> --- /dev/null
> +++ b/tools/binman/test/206_fip_uuid.dts
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               atf-fip {
> +                       fip-hdr-flags = /bits/ 64 <0x123>;
> +                       soc-fw {
> +                               fip-flags = /bits/ 64 <0x123456789abcdef>;
> +                               filename = "bl31.bin";
> +                       };
> +
> +                       u-boot {
> +                               fip-uuid = [fc 65 13 92 4a 5b 11 ec
> +                                           94 35 ff 2d 1c fc 79 9c];
> +                       };
> +               };
> +       };
> +};
> diff --git a/tools/binman/test/207_fip_ls.dts b/tools/binman/test/207_fip_ls.dts
> new file mode 100644
> index 00000000000..630fca15024
> --- /dev/null
> +++ b/tools/binman/test/207_fip_ls.dts
> @@ -0,0 +1,25 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               atf-fip {
> +                       fip-hdr-flags = /bits/ 64 <0x123>;
> +                       soc-fw {
> +                               fip-flags = /bits/ 64 <0x123456789abcdef>;
> +                               filename = "bl31.bin";
> +                       };
> +
> +                       u-boot {
> +                               fip-uuid = [fc 65 13 92 4a 5b 11 ec
> +                                           94 35 ff 2d 1c fc 79 9c];
> +                       };
> +               };
> +
> +               fdtmap {
> +               };
> +       };
> +};
> diff --git a/tools/binman/test/208_fip_replace.dts b/tools/binman/test/208_fip_replace.dts
> new file mode 100644
> index 00000000000..432c12474df
> --- /dev/null
> +++ b/tools/binman/test/208_fip_replace.dts
> @@ -0,0 +1,33 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               allow-repack;
> +               atf-fip {
> +                       fip-hdr-flags = /bits/ 64 <0x123>;
> +                       soc-fw {
> +                               fip-flags = /bits/ 64 <0x123456789abcdef>;
> +                               filename = "bl31.bin";
> +                       };
> +
> +                       u-boot {
> +                               fip-uuid = [fc 65 13 92 4a 5b 11 ec
> +                                           94 35 ff 2d 1c fc 79 9c];
> +                       };
> +
> +               };
> +
> +               u-boot {
> +               };
> +
> +               u-boot-dtb {
> +               };
> +
> +               fdtmap {
> +               };
> +       };
> +};
> diff --git a/tools/binman/test/209_fip_missing.dts b/tools/binman/test/209_fip_missing.dts
> new file mode 100644
> index 00000000000..43bb600d047
> --- /dev/null
> +++ b/tools/binman/test/209_fip_missing.dts
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               atf-fip {
> +                       soc-fw {
> +                               filename = "bl31.bin";
> +                       };
> +
> +                       rmm-fw {
> +                               filename = "rmm.bin";
> +                       };
> +               };
> +       };
> +};
> diff --git a/tools/binman/test/210_fip_size.dts b/tools/binman/test/210_fip_size.dts
> new file mode 100644
> index 00000000000..9dfee796459
> --- /dev/null
> +++ b/tools/binman/test/210_fip_size.dts
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               atf-fip {
> +                       size = <0x100>;
> +                       pad-byte = <0xff>;
> +                       soc-fw {
> +                               filename = "bl31.bin";
> +                       };
> +               };
> +               u-boot {
> +               };
> +       };
> +};
> diff --git a/tools/binman/test/211_fip_bad_align.dts b/tools/binman/test/211_fip_bad_align.dts
> new file mode 100644
> index 00000000000..a0901496d80
> --- /dev/null
> +++ b/tools/binman/test/211_fip_bad_align.dts
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               atf-fip {
> +                       fip-align = <31>;
> +                       size = <0x100>;
> +                       pad-byte = <0xff>;
> +                       soc-fw {
> +                               filename = "bl31.bin";
> +                       };
> +               };
> +       };
> +};
> diff --git a/tools/binman/test/212_fip_collection.dts b/tools/binman/test/212_fip_collection.dts
> new file mode 100644
> index 00000000000..332c023af87
> --- /dev/null
> +++ b/tools/binman/test/212_fip_collection.dts
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               collection {
> +                       content = <&fip &u_boot>;
> +               };
> +               fip: atf-fip {
> +                       soc-fw {
> +                               filename = "bl31.bin";
> +                       };
> +
> +                       scp-fwu-cfg {
> +                               filename = "bl2u.bin";
> +                       };
> +               };
> +               u_boot: u-boot {
> +               };
> +       };
> +};
> --
> 2.34.0.rc2.393.gf8c9666880-goog
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)
  2021-11-25 14:47   ` Ilias Apalodimas
@ 2021-11-25 15:11     ` François Ozog
  2021-11-25 16:49       ` Simon Glass
  0 siblings, 1 reply; 18+ messages in thread
From: François Ozog @ 2021-11-25 15:11 UTC (permalink / raw)
  To: Ilias Apalodimas; +Cc: Simon Glass, U-Boot Mailing List, Sandrine Bailleux

Hi Simon,




On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas <ilias.apalodimas@linaro.org>
wrote:

> +cc Sandrine
>
> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Simon,
> >
> >
> > On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org> wrote:
> > >
> > >
> > > This series adds support for the FIP format as used by ARM Trusted
> > > Firmware (in particular TF-A).
> > >
>
I will use a question you use often "what problem are you trying to
solve?". If FIP format is used it means that TF-A/BL2 is going to parse it
and verify the hashes/signatures according to TF-A scheme.

The packager will embed in a FIP components like Secure Monitor, Secure
hypervisor, Secure partitions code and manifests.

All in all, U-Boot will be representing a small percentage of the
functionality offered by secure firmware  as a whole and it feels odd to
make another implementation that is more "accessory" rather than critical
for the U-Boot community. It may be a good idea but I wish you could
explain it.

> > This allows images to be created containing a FIP, which itself contains
> > > various binaries. With this, image creation can be handled from an
> in-tree
> > > image description instead of needing to perform a lot of manual steps
> or
> > > custom scripts to build the FIP.
> > >
>
That's not my experience of building a fip.  Packaging even Linux as a BL33
(instead of U-Boot) is very simple.

> > >
> > > Simon Glass (2):
> > >   binman: Add a utility module for ATF FIP
> > >   binman: Add support for ATF FIP
> > >
> > >  scripts/pylint.base                      |   9 +-
> > >  tools/binman/entries.rst                 | 154 ++++++
> > >  tools/binman/etype/atf_fip.py            | 273 ++++++++++
> > >  tools/binman/fip_util.py                 | 653 +++++++++++++++++++++++
> > >  tools/binman/fip_util_test.py            | 405 ++++++++++++++
> > >  tools/binman/ftest.py                    | 217 ++++++++
> > >  tools/binman/main.py                     |   4 +-
> > >  tools/binman/test/203_fip.dts            |  21 +
> > >  tools/binman/test/204_fip_other.dts      |  22 +
> > >  tools/binman/test/205_fip_no_type.dts    |  15 +
> > >  tools/binman/test/206_fip_uuid.dts       |  22 +
> > >  tools/binman/test/207_fip_ls.dts         |  25 +
> > >  tools/binman/test/208_fip_replace.dts    |  33 ++
> > >  tools/binman/test/209_fip_missing.dts    |  19 +
> > >  tools/binman/test/210_fip_size.dts       |  19 +
> > >  tools/binman/test/211_fip_bad_align.dts  |  18 +
> > >  tools/binman/test/212_fip_collection.dts |  24 +
> > >  17 files changed, 1929 insertions(+), 4 deletions(-)
> > >  create mode 100644 tools/binman/etype/atf_fip.py
> > >  create mode 100755 tools/binman/fip_util.py
> > >  create mode 100755 tools/binman/fip_util_test.py
> > >  create mode 100644 tools/binman/test/203_fip.dts
> > >  create mode 100644 tools/binman/test/204_fip_other.dts
> > >  create mode 100644 tools/binman/test/205_fip_no_type.dts
> > >  create mode 100644 tools/binman/test/206_fip_uuid.dts
> > >  create mode 100644 tools/binman/test/207_fip_ls.dts
> > >  create mode 100644 tools/binman/test/208_fip_replace.dts
> > >  create mode 100644 tools/binman/test/209_fip_missing.dts
> > >  create mode 100644 tools/binman/test/210_fip_size.dts
> > >  create mode 100644 tools/binman/test/211_fip_bad_align.dts
> > >  create mode 100644 tools/binman/test/212_fip_collection.dts
> > >
> > > --
> > > 2.34.0.rc2.393.gf8c9666880-goog
> > >
> >
> > My python is mediocre at best.  I'll try having a look, but CC'ing
> > TF-A developers would be a good idea.
> >
> > Thanks
> > /Ilias
>


-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)
  2021-11-25 15:11     ` François Ozog
@ 2021-11-25 16:49       ` Simon Glass
  2021-11-25 17:01         ` François Ozog
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-11-25 16:49 UTC (permalink / raw)
  To: François Ozog
  Cc: Ilias Apalodimas, U-Boot Mailing List, Sandrine Bailleux

Hi François,

On Thu, 25 Nov 2021 at 08:11, François Ozog <francois.ozog@linaro.org> wrote:
>
> Hi Simon,
>
>
>
>
> On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
>>
>> +cc Sandrine
>>
>> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
>> <ilias.apalodimas@linaro.org> wrote:
>> >
>> > Hi Simon,
>> >
>> >
>> > On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org> wrote:
>> > >
>> > >
>> > > This series adds support for the FIP format as used by ARM Trusted
>> > > Firmware (in particular TF-A).
>> > >
>
> I will use a question you use often "what problem are you trying to solve?". If FIP format is used it means that TF-A/BL2 is going to parse it and verify the hashes/signatures according to TF-A scheme.
>
> The packager will embed in a FIP components like Secure Monitor, Secure hypervisor, Secure partitions code and manifests.
>
> All in all, U-Boot will be representing a small percentage of the functionality offered by secure firmware  as a whole and it feels odd to make another implementation that is more "accessory" rather than critical for the U-Boot community. It may be a good idea but I wish you could explain it.

Here is a talk about Binman, its goals and how it works.

https://www.youtube.com/watch?v=L84ujgUXBOQ

Think of Binman as a separate tool that brings everything together. It
has grown out of U-Boot, largely because U-Boot is the main firmware
in most cases. Getting U-Boot to start up and boot successfully is the
goal of this packaging process. There are lots of instructions in the
tree and elsewhere about how to build an image comprising U-Boot and
various binary blobs. Binman aims to provide documentation for that
process and an image description that can be used to build an image
reliably.

We are slowly converting these text instructions into an in-tree image
description.

I believe that Binman can help bring order to the chaos that is
otherwise only going to grow, with lots of shell scripts, manual
instructions, obscure binary tools, etc. Binman brings everything
together and makes it clear what is needing/missing to build an image.

>
>> > > This allows images to be created containing a FIP, which itself contains
>> > > various binaries. With this, image creation can be handled from an in-tree
>> > > image description instead of needing to perform a lot of manual steps or
>> > > custom scripts to build the FIP.
>> > >
>
> That's not my experience of building a fip.  Packaging even Linux as a BL33 (instead of U-Boot) is very simple.

But not automatic. With Binman you don't need to worry about the
packaging. It is done for you. You just need to find all the binary
blobs that are needed.  This ability is quite important as firmware is
fragmenting and getting very complicated these days.

Binman runs twice...once in the U-Boot tree to do a build and again
later to repackage, put in a final fdtmap, add signatures and any
final pieces needed.

See this patch for an example of complicated build instructions with
Odroid-C2 (>10 binary blobs!) and how Binman can help (see the changes
in the .rst file):

https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-sjg@chromium.org/

Regards,
Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)
  2021-11-25 16:49       ` Simon Glass
@ 2021-11-25 17:01         ` François Ozog
  2021-11-25 18:16           ` Simon Glass
  2021-12-01 10:31           ` Sandrine Bailleux
  0 siblings, 2 replies; 18+ messages in thread
From: François Ozog @ 2021-11-25 17:01 UTC (permalink / raw)
  To: Simon Glass
  Cc: Ilias Apalodimas, U-Boot Mailing List, Sandrine Bailleux, Jan Kiszka

Hi Simon,

On Thu, 25 Nov 2021 at 17:49, Simon Glass <sjg@chromium.org> wrote:

> Hi François,
>
> On Thu, 25 Nov 2021 at 08:11, François Ozog <francois.ozog@linaro.org>
> wrote:
> >
> > Hi Simon,
> >
> >
> >
> >
> > On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas <
> ilias.apalodimas@linaro.org> wrote:
> >>
> >> +cc Sandrine
> >>
> >> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
> >> <ilias.apalodimas@linaro.org> wrote:
> >> >
> >> > Hi Simon,
> >> >
> >> >
> >> > On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org> wrote:
> >> > >
> >> > >
> >> > > This series adds support for the FIP format as used by ARM Trusted
> >> > > Firmware (in particular TF-A).
> >> > >
> >
> > I will use a question you use often "what problem are you trying to
> solve?". If FIP format is used it means that TF-A/BL2 is going to parse it
> and verify the hashes/signatures according to TF-A scheme.
> >
> > The packager will embed in a FIP components like Secure Monitor, Secure
> hypervisor, Secure partitions code and manifests.
> >
> > All in all, U-Boot will be representing a small percentage of the
> functionality offered by secure firmware  as a whole and it feels odd to
> make another implementation that is more "accessory" rather than critical
> for the U-Boot community. It may be a good idea but I wish you could
> explain it.
>
> Here is a talk about Binman, its goals and how it works.
>
> https://www.youtube.com/watch?v=L84ujgUXBOQ
>
> Think of Binman as a separate tool that brings everything together. It
> has grown out of U-Boot, largely because U-Boot is the main firmware
> in most cases. Getting U-Boot to start up and boot successfully is the
> goal of this packaging process. There are lots of instructions in the
> tree and elsewhere about how to build an image comprising U-Boot and
> various binary blobs. Binman aims to provide documentation for that
> process and an image description that can be used to build an image
> reliably.
>
> We are slowly converting these text instructions into an in-tree image
> description.
>
> I believe that Binman can help bring order to the chaos that is
> otherwise only going to grow, with lots of shell scripts, manual
> instructions, obscure binary tools, etc. Binman brings everything
> together and makes it clear what is needing/missing to build an image.
>
> >
> >> > > This allows images to be created containing a FIP, which itself
> contains
> >> > > various binaries. With this, image creation can be handled from an
> in-tree
> >> > > image description instead of needing to perform a lot of manual
> steps or
> >> > > custom scripts to build the FIP.
> >> > >
> >
> > That's not my experience of building a fip.  Packaging even Linux as a
> BL33 (instead of U-Boot) is very simple.
>
> But not automatic. With Binman you don't need to worry about the
> packaging. It is done for you. You just need to find all the binary
> blobs that are needed.  This ability is quite important as firmware is
> fragmenting and getting very complicated these days.
>
> Binman runs twice...once in the U-Boot tree to do a build and again
> later to repackage, put in a final fdtmap, add signatures and any
> final pieces needed.
>
> See this patch for an example of complicated build instructions with
> Odroid-C2 (>10 binary blobs!) and how Binman can help (see the changes
> in the .rst file):
>
>
> https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-sjg@chromium.org/
>
> That's indeed complicated! I can't comment whether this build process is
"canonical" as per TF-A standards so I'll let TF-A community comment.
Have you factored in the signature issues that Jan@Siemens has in the
overall process?


> Regards,
> Simon
>


-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)
  2021-11-25 17:01         ` François Ozog
@ 2021-11-25 18:16           ` Simon Glass
  2021-12-01 10:31           ` Sandrine Bailleux
  1 sibling, 0 replies; 18+ messages in thread
From: Simon Glass @ 2021-11-25 18:16 UTC (permalink / raw)
  To: François Ozog
  Cc: Ilias Apalodimas, U-Boot Mailing List, Sandrine Bailleux, Jan Kiszka

Hi François,

On Thu, 25 Nov 2021 at 10:01, François Ozog <francois.ozog@linaro.org> wrote:
>
> Hi Simon,
>
> On Thu, 25 Nov 2021 at 17:49, Simon Glass <sjg@chromium.org> wrote:
>>
>> Hi François,
>>
>> On Thu, 25 Nov 2021 at 08:11, François Ozog <francois.ozog@linaro.org> wrote:
>> >
>> > Hi Simon,
>> >
>> >
>> >
>> >
>> > On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote:
>> >>
>> >> +cc Sandrine
>> >>
>> >> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
>> >> <ilias.apalodimas@linaro.org> wrote:
>> >> >
>> >> > Hi Simon,
>> >> >
>> >> >
>> >> > On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org> wrote:
>> >> > >
>> >> > >
>> >> > > This series adds support for the FIP format as used by ARM Trusted
>> >> > > Firmware (in particular TF-A).
>> >> > >
>> >
>> > I will use a question you use often "what problem are you trying to solve?". If FIP format is used it means that TF-A/BL2 is going to parse it and verify the hashes/signatures according to TF-A scheme.
>> >
>> > The packager will embed in a FIP components like Secure Monitor, Secure hypervisor, Secure partitions code and manifests.
>> >
>> > All in all, U-Boot will be representing a small percentage of the functionality offered by secure firmware  as a whole and it feels odd to make another implementation that is more "accessory" rather than critical for the U-Boot community. It may be a good idea but I wish you could explain it.
>>
>> Here is a talk about Binman, its goals and how it works.
>>
>> https://www.youtube.com/watch?v=L84ujgUXBOQ
>>
>> Think of Binman as a separate tool that brings everything together. It
>> has grown out of U-Boot, largely because U-Boot is the main firmware
>> in most cases. Getting U-Boot to start up and boot successfully is the
>> goal of this packaging process. There are lots of instructions in the
>> tree and elsewhere about how to build an image comprising U-Boot and
>> various binary blobs. Binman aims to provide documentation for that
>> process and an image description that can be used to build an image
>> reliably.
>>
>> We are slowly converting these text instructions into an in-tree image
>> description.
>>
>> I believe that Binman can help bring order to the chaos that is
>> otherwise only going to grow, with lots of shell scripts, manual
>> instructions, obscure binary tools, etc. Binman brings everything
>> together and makes it clear what is needing/missing to build an image.
>>
>> >
>> >> > > This allows images to be created containing a FIP, which itself contains
>> >> > > various binaries. With this, image creation can be handled from an in-tree
>> >> > > image description instead of needing to perform a lot of manual steps or
>> >> > > custom scripts to build the FIP.
>> >> > >
>> >
>> > That's not my experience of building a fip.  Packaging even Linux as a BL33 (instead of U-Boot) is very simple.
>>
>> But not automatic. With Binman you don't need to worry about the
>> packaging. It is done for you. You just need to find all the binary
>> blobs that are needed.  This ability is quite important as firmware is
>> fragmenting and getting very complicated these days.
>>
>> Binman runs twice...once in the U-Boot tree to do a build and again
>> later to repackage, put in a final fdtmap, add signatures and any
>> final pieces needed.
>>
>> See this patch for an example of complicated build instructions with
>> Odroid-C2 (>10 binary blobs!) and how Binman can help (see the changes
>> in the .rst file):
>>
>> https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-sjg@chromium.org/
>>
> That's indeed complicated! I can't comment whether this build process is "canonical" as per TF-A standards so I'll let TF-A community comment.
> Have you factored in the signature issues that Jan@Siemens has in the overall process?

In Chrome OS we package up the material that needs to be signed and
send it to a signing server, then get back a key.

You can use 'binman extract' to get the signing data, although perhaps
we could add a special option for it.

You can use 'binman replace' to add the signature in when you get it
back. Again we could create a special path for this if needed.

Regards,
Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)
  2021-11-25 17:01         ` François Ozog
  2021-11-25 18:16           ` Simon Glass
@ 2021-12-01 10:31           ` Sandrine Bailleux
  2021-12-01 16:51             ` Simon Glass
  1 sibling, 1 reply; 18+ messages in thread
From: Sandrine Bailleux @ 2021-12-01 10:31 UTC (permalink / raw)
  To: François Ozog, Simon Glass
  Cc: Ilias Apalodimas, U-Boot Mailing List, Jan Kiszka

Hi everyone,

I am Sandrine Bailleux, from the Trusted Firmware-A project. Ilias
Apalodimas CC'ed me on this thread.

First of all, thanks for involving the TF-A developers in this thread
and my apologies for the delay in responding.

On 11/25/21 6:01 PM, François Ozog wrote:
> Hi Simon,
>
> On Thu, 25 Nov 2021 at 17:49, Simon Glass <sjg@chromium.org
> <mailto:sjg@chromium.org>> wrote:
>
>     Hi François,
>
>     On Thu, 25 Nov 2021 at 08:11, François Ozog
>     <francois.ozog@linaro.org <mailto:francois.ozog@linaro.org>> wrote:
>     >
>     > Hi Simon,
>     >
>     >
>     >
>     >
>     > On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas
>     <ilias.apalodimas@linaro.org <mailto:ilias.apalodimas@linaro.org>>
>     wrote:
>     >>
>     >> +cc Sandrine
>     >>
>     >> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
>     >> <ilias.apalodimas@linaro.org
>     <mailto:ilias.apalodimas@linaro.org>> wrote:
>     >> >
>     >> > Hi Simon,
>     >> >
>     >> >
>     >> > On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org
>     <mailto:sjg@chromium.org>> wrote:
>     >> > >
>     >> > >
>     >> > > This series adds support for the FIP format as used by ARM
>     Trusted
>     >> > > Firmware (in particular TF-A).
>     >> > >
>     >
>     > I will use a question you use often "what problem are you trying
>     to solve?". If FIP format is used it means that TF-A/BL2 is going to
>     parse it and verify the hashes/signatures according to TF-A scheme.
>     >
>     > The packager will embed in a FIP components like Secure Monitor,
>     Secure hypervisor, Secure partitions code and manifests.
>     >
>     > All in all, U-Boot will be representing a small percentage of the
>     functionality offered by secure firmware  as a whole and it feels
>     odd to make another implementation that is more "accessory" rather
>     than critical for the U-Boot community. It may be a good idea but I
>     wish you could explain it.
>
>     Here is a talk about Binman, its goals and how it works.
>
>     https://www.youtube.com/watch?v=L84ujgUXBOQ
>
>     Think of Binman as a separate tool that brings everything together. It
>     has grown out of U-Boot, largely because U-Boot is the main firmware
>     in most cases. Getting U-Boot to start up and boot successfully is the
>     goal of this packaging process. There are lots of instructions in the
>     tree and elsewhere about how to build an image comprising U-Boot and
>     various binary blobs. Binman aims to provide documentation for that
>     process and an image description that can be used to build an image
>     reliably.
>
>     We are slowly converting these text instructions into an in-tree image
>     description.
>
>     I believe that Binman can help bring order to the chaos that is
>     otherwise only going to grow, with lots of shell scripts, manual
>     instructions, obscure binary tools, etc. Binman brings everything
>     together and makes it clear what is needing/missing to build an image.
>
>     >
>     >> > > This allows images to be created containing a FIP, which
>     itself contains
>     >> > > various binaries. With this, image creation can be handled
>     from an in-tree
>     >> > > image description instead of needing to perform a lot of
>     manual steps or
>     >> > > custom scripts to build the FIP.
>     >> > >
>     >
>     > That's not my experience of building a fip.  Packaging even Linux
>     as a BL33 (instead of U-Boot) is very simple.
>
>     But not automatic. With Binman you don't need to worry about the
>     packaging. It is done for you. You just need to find all the binary
>     blobs that are needed.  This ability is quite important as firmware is
>     fragmenting and getting very complicated these days.
>
>     Binman runs twice...once in the U-Boot tree to do a build and again
>     later to repackage, put in a final fdtmap, add signatures and any
>     final pieces needed.
>
>     See this patch for an example of complicated build instructions with
>     Odroid-C2 (>10 binary blobs!) and how Binman can help (see the changes
>     in the .rst file):
>
>     https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-sjg@chromium.org/
>
> That's indeed complicated! I can't comment whether this build process is
> "canonical" as per TF-A standards so I'll let TF-A community comment.
> Have you factored in the signature issues that Jan@Siemens has in the
> overall process?

I am a bit confused by the ask here. What input would you like from TF-A
community? Are you asking for a code review or are you more interested
in getting an opinion about adding support for FIP files in binman?

Regardless, I had a brief look at the patches and I have some early
questions/comments.

In the first patch, the commit message mentions that the tool parses the
TF-A source code to get a list of supported UUIDs. However,
tools/binman/fip_util.py seems to embed a hard-coded list of these
UUIDs. I think I might be missing something there... Does it just mean
that the said list was generated using some other script that parsed the
TF-A code? Or does the tool really parse any TF-A code dynamically?

As you may know, we sometimes add new image types in TF-A so I am just
wondering how you intend to keep in sync with these changes and how
automated the process would be.

I second François' concerns about having 2 different implementations of
fiptool, even if you're trying to solve different (or bigger) problems
here. This could be confusing for users. Also, it is likely to generate
maintenance work for both TF-A and U-boot projects.

I am not saying the tool should stay within the TF-A project, though.
It's been in the back of our minds for some time that this tool should
have a life of its own, given that it packages more than just TF-A
binaries, but also the normal world bootloader, secure payload, ...
Also, I must admit that a python implementation looks better than a C
implementation. Rewriting the tool in a scripting language has also been
a goal of ours for a long time, although we never got round to do it.

Simon, you've mentioned that binman has grown out of U-Boot. How
independent is it from U-Boot right now? Are there lots of assumptions
about U-Boot environment in it? Or is it already a general firmware
image packager in your mind? I just want to explore the idea of
replacing fiptool by binman in the future. I am sure we're not there
yet, neither from TF-A perspective nor U-Boot, but I'd be keen on
understanding how far we are. Also, this would need discussion with the
broader TF-A community.

Regards,
Sandrine
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)
  2021-12-01 10:31           ` Sandrine Bailleux
@ 2021-12-01 16:51             ` Simon Glass
  2021-12-02  7:10               ` Sandrine Bailleux
  0 siblings, 1 reply; 18+ messages in thread
From: Simon Glass @ 2021-12-01 16:51 UTC (permalink / raw)
  To: Sandrine Bailleux
  Cc: François Ozog, Ilias Apalodimas, U-Boot Mailing List, Jan Kiszka

Hi Sandrine,

On Wed, 1 Dec 2021 at 03:32, Sandrine Bailleux
<sandrine.bailleux@arm.com> wrote:
>
> Hi everyone,
>
> I am Sandrine Bailleux, from the Trusted Firmware-A project. Ilias
> Apalodimas CC'ed me on this thread.
>
> First of all, thanks for involving the TF-A developers in this thread
> and my apologies for the delay in responding.

Thank you for your response.

>
> On 11/25/21 6:01 PM, François Ozog wrote:
> > Hi Simon,
> >
> > On Thu, 25 Nov 2021 at 17:49, Simon Glass <sjg@chromium.org
> > <mailto:sjg@chromium.org>> wrote:
> >
> >     Hi François,
> >
> >     On Thu, 25 Nov 2021 at 08:11, François Ozog
> >     <francois.ozog@linaro.org <mailto:francois.ozog@linaro.org>> wrote:
> >     >
> >     > Hi Simon,
> >     >
> >     >
> >     >
> >     >
> >     > On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas
> >     <ilias.apalodimas@linaro.org <mailto:ilias.apalodimas@linaro.org>>
> >     wrote:
> >     >>
> >     >> +cc Sandrine
> >     >>
> >     >> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
> >     >> <ilias.apalodimas@linaro.org
> >     <mailto:ilias.apalodimas@linaro.org>> wrote:
> >     >> >
> >     >> > Hi Simon,
> >     >> >
> >     >> >
> >     >> > On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org
> >     <mailto:sjg@chromium.org>> wrote:
> >     >> > >
> >     >> > >
> >     >> > > This series adds support for the FIP format as used by ARM
> >     Trusted
> >     >> > > Firmware (in particular TF-A).
> >     >> > >
> >     >
> >     > I will use a question you use often "what problem are you trying
> >     to solve?". If FIP format is used it means that TF-A/BL2 is going to
> >     parse it and verify the hashes/signatures according to TF-A scheme.
> >     >
> >     > The packager will embed in a FIP components like Secure Monitor,
> >     Secure hypervisor, Secure partitions code and manifests.
> >     >
> >     > All in all, U-Boot will be representing a small percentage of the
> >     functionality offered by secure firmware  as a whole and it feels
> >     odd to make another implementation that is more "accessory" rather
> >     than critical for the U-Boot community. It may be a good idea but I
> >     wish you could explain it.
> >
> >     Here is a talk about Binman, its goals and how it works.
> >
> >     https://www.youtube.com/watch?v=L84ujgUXBOQ
> >
> >     Think of Binman as a separate tool that brings everything together. It
> >     has grown out of U-Boot, largely because U-Boot is the main firmware
> >     in most cases. Getting U-Boot to start up and boot successfully is the
> >     goal of this packaging process. There are lots of instructions in the
> >     tree and elsewhere about how to build an image comprising U-Boot and
> >     various binary blobs. Binman aims to provide documentation for that
> >     process and an image description that can be used to build an image
> >     reliably.
> >
> >     We are slowly converting these text instructions into an in-tree image
> >     description.
> >
> >     I believe that Binman can help bring order to the chaos that is
> >     otherwise only going to grow, with lots of shell scripts, manual
> >     instructions, obscure binary tools, etc. Binman brings everything
> >     together and makes it clear what is needing/missing to build an image.
> >
> >     >
> >     >> > > This allows images to be created containing a FIP, which
> >     itself contains
> >     >> > > various binaries. With this, image creation can be handled
> >     from an in-tree
> >     >> > > image description instead of needing to perform a lot of
> >     manual steps or
> >     >> > > custom scripts to build the FIP.
> >     >> > >
> >     >
> >     > That's not my experience of building a fip.  Packaging even Linux
> >     as a BL33 (instead of U-Boot) is very simple.
> >
> >     But not automatic. With Binman you don't need to worry about the
> >     packaging. It is done for you. You just need to find all the binary
> >     blobs that are needed.  This ability is quite important as firmware is
> >     fragmenting and getting very complicated these days.
> >
> >     Binman runs twice...once in the U-Boot tree to do a build and again
> >     later to repackage, put in a final fdtmap, add signatures and any
> >     final pieces needed.
> >
> >     See this patch for an example of complicated build instructions with
> >     Odroid-C2 (>10 binary blobs!) and how Binman can help (see the changes
> >     in the .rst file):
> >
> >     https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-sjg@chromium.org/
> >
> > That's indeed complicated! I can't comment whether this build process is
> > "canonical" as per TF-A standards so I'll let TF-A community comment.
> > Have you factored in the signature issues that Jan@Siemens has in the
> > overall process?
>
> I am a bit confused by the ask here. What input would you like from TF-A
> community? Are you asking for a code review or are you more interested
> in getting an opinion about adding support for FIP files in binman?

The latter.

>
> Regardless, I had a brief look at the patches and I have some early
> questions/comments.
>
> In the first patch, the commit message mentions that the tool parses the
> TF-A source code to get a list of supported UUIDs. However,
> tools/binman/fip_util.py seems to embed a hard-coded list of these
> UUIDs. I think I might be missing something there... Does it just mean
> that the said list was generated using some other script that parsed the
> TF-A code? Or does the tool really parse any TF-A code dynamically?

A bit of both. The tool allows creating a new version of itself with
the updates parsed from the source code. For anything other than local
use, a patch must be submitted to do the updates.

To run the tool::

    $ tools/binman/fip_util.py  -s /path/to/arm-trusted-firmware
    Warning: UUID 'UUID_NON_TRUSTED_WORLD_KEY_CERT' is not mentioned
in tbbr_config.c file
    Existing code in 'tools/binman/fip_util.py' is up-to-date

If it shows there is an update, it writes a new version of `fip_util.py`
to `fip_util.py.out`. You can change the output file using the `-i` flag.
If you have a problem, use `-D` to enable traceback debugging.c

You can see that in the docs in this patch:

http://patchwork.ozlabs.org/project/uboot/patch/20211123210849.2.Idf2f2a46d26cdecb56b6e9f40472f62fd062e346@changeid/

> As you may know, we sometimes add new image types in TF-A so I am just
> wondering how you intend to keep in sync with these changes and how
> automated the process would be.

See above.

>
> I second François' concerns about having 2 different implementations of
> fiptool, even if you're trying to solve different (or bigger) problems
> here. This could be confusing for users. Also, it is likely to generate
> maintenance work for both TF-A and U-boot projects.
>
> I am not saying the tool should stay within the TF-A project, though.
> It's been in the back of our minds for some time that this tool should
> have a life of its own, given that it packages more than just TF-A
> binaries, but also the normal world bootloader, secure payload, ...
> Also, I must admit that a python implementation looks better than a C
> implementation. Rewriting the tool in a scripting language has also been
> a goal of ours for a long time, although we never got round to do it.
>
> Simon, you've mentioned that binman has grown out of U-Boot. How
> independent is it from U-Boot right now? Are there lots of assumptions
> about U-Boot environment in it? Or is it already a general firmware
> image packager in your mind? I just want to explore the idea of
> replacing fiptool by binman in the future. I am sure we're not there
> yet, neither from TF-A perspective nor U-Boot, but I'd be keen on
> understanding how far we are. Also, this would need discussion with the
> broader TF-A community.

Binman is a general-purpose packaging tool. It has some specific
features for U-Boot, Chrome OS and coreboot so far. I think it could
cover TF-A's needs also.

A key point is that Binman has two related purposes:
- building an initial image, perhaps just for development/CI purposes
(no signatures, some blobs missing, etc.)
- building a production/real image when everything is available

This is a concept that I very much struggle to get across, the
difference between building things and packaging them. I believe it is
becoming increasingly important to make this distinction, as firmware
fragments.

Some people will prefer to have C tools instead of Python, but if that
is not a concern, then I believe Binman could be a good solution for
TF-A. A few nice properties are that it is easy to extend and has 100%
test coverage.

I would be happy to help with what TF-A needs here.

One last point is that Binman can provide an 'fdtmap' which is a full
image description. This can provide insight into every binary in the
image, whether it is in a FIP, FIT, CBFS or whatever. Binman happens
to support generating an FMAP (which is vaguely similar to FIP), which
could serve as a model for generating table-of-contents data in other
useful formats.

There is a talk here that might help to explain the goals better:

https://www.youtube.com/watch?v=L84ujgUXBOQ

This patch shows converting lots of shell commands into a binman definition:

https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-sjg@chromium.org/

Regards,
Simon

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)
  2021-12-01 16:51             ` Simon Glass
@ 2021-12-02  7:10               ` Sandrine Bailleux
  2021-12-04 12:35                 ` François Ozog
  0 siblings, 1 reply; 18+ messages in thread
From: Sandrine Bailleux @ 2021-12-02  7:10 UTC (permalink / raw)
  To: Simon Glass
  Cc: François Ozog, Ilias Apalodimas, U-Boot Mailing List, Jan Kiszka

Hi Simon,

On 12/1/21 5:51 PM, Simon Glass wrote:
> Hi Sandrine,
>
> On Wed, 1 Dec 2021 at 03:32, Sandrine Bailleux
> <sandrine.bailleux@arm.com> wrote:
>>
>> Hi everyone,
>>
>> I am Sandrine Bailleux, from the Trusted Firmware-A project. Ilias
>> Apalodimas CC'ed me on this thread.
>>
>> First of all, thanks for involving the TF-A developers in this thread
>> and my apologies for the delay in responding.
>
> Thank you for your response.
>
>>
>> On 11/25/21 6:01 PM, François Ozog wrote:
>>> Hi Simon,
>>>
>>> On Thu, 25 Nov 2021 at 17:49, Simon Glass <sjg@chromium.org
>>> <mailto:sjg@chromium.org>> wrote:
>>>
>>>     Hi François,
>>>
>>>     On Thu, 25 Nov 2021 at 08:11, François Ozog
>>>     <francois.ozog@linaro.org <mailto:francois.ozog@linaro.org>> wrote:
>>>     >
>>>     > Hi Simon,
>>>     >
>>>     >
>>>     >
>>>     >
>>>     > On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas
>>>     <ilias.apalodimas@linaro.org <mailto:ilias.apalodimas@linaro.org>>
>>>     wrote:
>>>     >>
>>>     >> +cc Sandrine
>>>     >>
>>>     >> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
>>>     >> <ilias.apalodimas@linaro.org
>>>     <mailto:ilias.apalodimas@linaro.org>> wrote:
>>>     >> >
>>>     >> > Hi Simon,
>>>     >> >
>>>     >> >
>>>     >> > On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org
>>>     <mailto:sjg@chromium.org>> wrote:
>>>     >> > >
>>>     >> > >
>>>     >> > > This series adds support for the FIP format as used by ARM
>>>     Trusted
>>>     >> > > Firmware (in particular TF-A).
>>>     >> > >
>>>     >
>>>     > I will use a question you use often "what problem are you trying
>>>     to solve?". If FIP format is used it means that TF-A/BL2 is going to
>>>     parse it and verify the hashes/signatures according to TF-A scheme.
>>>     >
>>>     > The packager will embed in a FIP components like Secure Monitor,
>>>     Secure hypervisor, Secure partitions code and manifests.
>>>     >
>>>     > All in all, U-Boot will be representing a small percentage of the
>>>     functionality offered by secure firmware  as a whole and it feels
>>>     odd to make another implementation that is more "accessory" rather
>>>     than critical for the U-Boot community. It may be a good idea but I
>>>     wish you could explain it.
>>>
>>>     Here is a talk about Binman, its goals and how it works.
>>>
>>>     https://www.youtube.com/watch?v=L84ujgUXBOQ
>>>
>>>     Think of Binman as a separate tool that brings everything together. It
>>>     has grown out of U-Boot, largely because U-Boot is the main firmware
>>>     in most cases. Getting U-Boot to start up and boot successfully is the
>>>     goal of this packaging process. There are lots of instructions in the
>>>     tree and elsewhere about how to build an image comprising U-Boot and
>>>     various binary blobs. Binman aims to provide documentation for that
>>>     process and an image description that can be used to build an image
>>>     reliably.
>>>
>>>     We are slowly converting these text instructions into an in-tree image
>>>     description.
>>>
>>>     I believe that Binman can help bring order to the chaos that is
>>>     otherwise only going to grow, with lots of shell scripts, manual
>>>     instructions, obscure binary tools, etc. Binman brings everything
>>>     together and makes it clear what is needing/missing to build an image.
>>>
>>>     >
>>>     >> > > This allows images to be created containing a FIP, which
>>>     itself contains
>>>     >> > > various binaries. With this, image creation can be handled
>>>     from an in-tree
>>>     >> > > image description instead of needing to perform a lot of
>>>     manual steps or
>>>     >> > > custom scripts to build the FIP.
>>>     >> > >
>>>     >
>>>     > That's not my experience of building a fip.  Packaging even Linux
>>>     as a BL33 (instead of U-Boot) is very simple.
>>>
>>>     But not automatic. With Binman you don't need to worry about the
>>>     packaging. It is done for you. You just need to find all the binary
>>>     blobs that are needed.  This ability is quite important as firmware is
>>>     fragmenting and getting very complicated these days.
>>>
>>>     Binman runs twice...once in the U-Boot tree to do a build and again
>>>     later to repackage, put in a final fdtmap, add signatures and any
>>>     final pieces needed.
>>>
>>>     See this patch for an example of complicated build instructions with
>>>     Odroid-C2 (>10 binary blobs!) and how Binman can help (see the changes
>>>     in the .rst file):
>>>
>>>     https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-sjg@chromium.org/
>>>
>>> That's indeed complicated! I can't comment whether this build process is
>>> "canonical" as per TF-A standards so I'll let TF-A community comment.
>>> Have you factored in the signature issues that Jan@Siemens has in the
>>> overall process?
>>
>> I am a bit confused by the ask here. What input would you like from TF-A
>> community? Are you asking for a code review or are you more interested
>> in getting an opinion about adding support for FIP files in binman?
>
> The latter.
>
>>
>> Regardless, I had a brief look at the patches and I have some early
>> questions/comments.
>>
>> In the first patch, the commit message mentions that the tool parses the
>> TF-A source code to get a list of supported UUIDs. However,
>> tools/binman/fip_util.py seems to embed a hard-coded list of these
>> UUIDs. I think I might be missing something there... Does it just mean
>> that the said list was generated using some other script that parsed the
>> TF-A code? Or does the tool really parse any TF-A code dynamically?
>
> A bit of both. The tool allows creating a new version of itself with
> the updates parsed from the source code. For anything other than local
> use, a patch must be submitted to do the updates.
>
> To run the tool::
>
>     $ tools/binman/fip_util.py  -s /path/to/arm-trusted-firmware
>     Warning: UUID 'UUID_NON_TRUSTED_WORLD_KEY_CERT' is not mentioned
> in tbbr_config.c file
>     Existing code in 'tools/binman/fip_util.py' is up-to-date
>
> If it shows there is an update, it writes a new version of `fip_util.py`
> to `fip_util.py.out`. You can change the output file using the `-i` flag.
> If you have a problem, use `-D` to enable traceback debugging.c
>
> You can see that in the docs in this patch:
>
> http://patchwork.ozlabs.org/project/uboot/patch/20211123210849.2.Idf2f2a46d26cdecb56b6e9f40472f62fd062e346@changeid/
>
>> As you may know, we sometimes add new image types in TF-A so I am just
>> wondering how you intend to keep in sync with these changes and how
>> automated the process would be.
>
> See above.
>
>>
>> I second François' concerns about having 2 different implementations of
>> fiptool, even if you're trying to solve different (or bigger) problems
>> here. This could be confusing for users. Also, it is likely to generate
>> maintenance work for both TF-A and U-boot projects.
>>
>> I am not saying the tool should stay within the TF-A project, though.
>> It's been in the back of our minds for some time that this tool should
>> have a life of its own, given that it packages more than just TF-A
>> binaries, but also the normal world bootloader, secure payload, ...
>> Also, I must admit that a python implementation looks better than a C
>> implementation. Rewriting the tool in a scripting language has also been
>> a goal of ours for a long time, although we never got round to do it.
>>
>> Simon, you've mentioned that binman has grown out of U-Boot. How
>> independent is it from U-Boot right now? Are there lots of assumptions
>> about U-Boot environment in it? Or is it already a general firmware
>> image packager in your mind? I just want to explore the idea of
>> replacing fiptool by binman in the future. I am sure we're not there
>> yet, neither from TF-A perspective nor U-Boot, but I'd be keen on
>> understanding how far we are. Also, this would need discussion with the
>> broader TF-A community.
>
> Binman is a general-purpose packaging tool. It has some specific
> features for U-Boot, Chrome OS and coreboot so far. I think it could
> cover TF-A's needs also.
>
> A key point is that Binman has two related purposes:
> - building an initial image, perhaps just for development/CI purposes
> (no signatures, some blobs missing, etc.)
> - building a production/real image when everything is available
>
> This is a concept that I very much struggle to get across, the
> difference between building things and packaging them. I believe it is
> becoming increasingly important to make this distinction, as firmware
> fragments.
>
> Some people will prefer to have C tools instead of Python, but if that
> is not a concern, then I believe Binman could be a good solution for
> TF-A. A few nice properties are that it is easy to extend and has 100%
> test coverage.
>
> I would be happy to help with what TF-A needs here.
>
> One last point is that Binman can provide an 'fdtmap' which is a full
> image description. This can provide insight into every binary in the
> image, whether it is in a FIP, FIT, CBFS or whatever. Binman happens
> to support generating an FMAP (which is vaguely similar to FIP), which
> could serve as a model for generating table-of-contents data in other
> useful formats.
>
> There is a talk here that might help to explain the goals better:
>
> https://www.youtube.com/watch?v=L84ujgUXBOQ
>
> This patch shows converting lots of shell commands into a binman definition:
>
> https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-sjg@chromium.org/

Thanks for the information, this certainly looks interesting to me in
the context of TF-A! I'll have a closest look at the patches and
resources you've pointed me to. I'll also talk to my team, see what they
think and get back to you.

Regards,
Sandrine
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)
  2021-12-02  7:10               ` Sandrine Bailleux
@ 2021-12-04 12:35                 ` François Ozog
  2021-12-05 20:00                   ` Simon Glass
  0 siblings, 1 reply; 18+ messages in thread
From: François Ozog @ 2021-12-04 12:35 UTC (permalink / raw)
  To: Sandrine Bailleux
  Cc: Ilias Apalodimas, Jan Kiszka, Simon Glass, U-Boot Mailing List

Hi Simon and Sandrine

Le jeu. 2 déc. 2021 à 08:10, Sandrine Bailleux <sandrine.bailleux@arm.com>
a écrit :

> Hi Simon,
>
> On 12/1/21 5:51 PM, Simon Glass wrote:
> > Hi Sandrine,
> >
> > On Wed, 1 Dec 2021 at 03:32, Sandrine Bailleux
> > <sandrine.bailleux@arm.com> wrote:
> >>
> >> Hi everyone,
> >>
> >> I am Sandrine Bailleux, from the Trusted Firmware-A project. Ilias
> >> Apalodimas CC'ed me on this thread.
> >>
> >> First of all, thanks for involving the TF-A developers in this thread
> >> and my apologies for the delay in responding.
> >
> > Thank you for your response.
> >
> >>
> >> On 11/25/21 6:01 PM, François Ozog wrote:
> >>> Hi Simon,
> >>>
> >>> On Thu, 25 Nov 2021 at 17:49, Simon Glass <sjg@chromium.org
> >>> <mailto:sjg@chromium.org>> wrote:
> >>>
> >>>     Hi François,
> >>>
> >>>     On Thu, 25 Nov 2021 at 08:11, François Ozog
> >>>     <francois.ozog@linaro.org <mailto:francois.ozog@linaro.org>>
> wrote:
> >>>     >
> >>>     > Hi Simon,
> >>>     >
> >>>     >
> >>>     >
> >>>     >
> >>>     > On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas
> >>>     <ilias.apalodimas@linaro.org <mailto:ilias.apalodimas@linaro.org>>
> >>>     wrote:
> >>>     >>
> >>>     >> +cc Sandrine
> >>>     >>
> >>>     >> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
> >>>     >> <ilias.apalodimas@linaro.org
> >>>     <mailto:ilias.apalodimas@linaro.org>> wrote:
> >>>     >> >
> >>>     >> > Hi Simon,
> >>>     >> >
> >>>     >> >
> >>>     >> > On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org
> >>>     <mailto:sjg@chromium.org>> wrote:
> >>>     >> > >
> >>>     >> > >
> >>>     >> > > This series adds support for the FIP format as used by ARM
> >>>     Trusted
> >>>     >> > > Firmware (in particular TF-A).
> >>>     >> > >
> >>>     >
> >>>     > I will use a question you use often "what problem are you trying
> >>>     to solve?". If FIP format is used it means that TF-A/BL2 is going
> to
> >>>     parse it and verify the hashes/signatures according to TF-A scheme.
> >>>     >
> >>>     > The packager will embed in a FIP components like Secure Monitor,
> >>>     Secure hypervisor, Secure partitions code and manifests.
> >>>     >
> >>>     > All in all, U-Boot will be representing a small percentage of the
> >>>     functionality offered by secure firmware  as a whole and it feels
> >>>     odd to make another implementation that is more "accessory" rather
> >>>     than critical for the U-Boot community. It may be a good idea but I
> >>>     wish you could explain it.
> >>>
> >>>     Here is a talk about Binman, its goals and how it works.
> >>>
> >>>     https://www.youtube.com/watch?v=L84ujgUXBOQ
> >>>
> >>>     Think of Binman as a separate tool that brings everything
> together. It
> >>>     has grown out of U-Boot, largely because U-Boot is the main
> firmware
> >>>     in most cases. Getting U-Boot to start up and boot successfully is
> the
> >>>     goal of this packaging process. There are lots of instructions in
> the
> >>>     tree and elsewhere about how to build an image comprising U-Boot
> and
> >>>     various binary blobs. Binman aims to provide documentation for that
> >>>     process and an image description that can be used to build an image
> >>>     reliably.
> >>>
> >>>     We are slowly converting these text instructions into an in-tree
> image
> >>>     description.
> >>>
> >>>     I believe that Binman can help bring order to the chaos that is
> >>>     otherwise only going to grow, with lots of shell scripts, manual
> >>>     instructions, obscure binary tools, etc. Binman brings everything
> >>>     together and makes it clear what is needing/missing to build an
> image.
> >>>
> >>>     >
> >>>     >> > > This allows images to be created containing a FIP, which
> >>>     itself contains
> >>>     >> > > various binaries. With this, image creation can be handled
> >>>     from an in-tree
> >>>     >> > > image description instead of needing to perform a lot of
> >>>     manual steps or
> >>>     >> > > custom scripts to build the FIP.
> >>>     >> > >
> >>>     >
> >>>     > That's not my experience of building a fip.  Packaging even Linux
> >>>     as a BL33 (instead of U-Boot) is very simple.
> >>>
> >>>     But not automatic. With Binman you don't need to worry about the
> >>>     packaging. It is done for you. You just need to find all the binary
> >>>     blobs that are needed.  This ability is quite important as
> firmware is
> >>>     fragmenting and getting very complicated these days.
> >>>
> >>>     Binman runs twice...once in the U-Boot tree to do a build and again
> >>>     later to repackage, put in a final fdtmap, add signatures and any
> >>>     final pieces needed.
> >>>
> >>>     See this patch for an example of complicated build instructions
> with
> >>>     Odroid-C2 (>10 binary blobs!) and how Binman can help (see the
> changes
> >>>     in the .rst file):
> >>>
> >>>
> https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-sjg@chromium.org/
> >>>
> >>> That's indeed complicated! I can't comment whether this build process
> is
> >>> "canonical" as per TF-A standards so I'll let TF-A community comment.
> >>> Have you factored in the signature issues that Jan@Siemens has in the
> >>> overall process?
> >>
> >> I am a bit confused by the ask here. What input would you like from TF-A
> >> community? Are you asking for a code review or are you more interested
> >> in getting an opinion about adding support for FIP files in binman?
> >
> > The latter.
> >
> >>
> >> Regardless, I had a brief look at the patches and I have some early
> >> questions/comments.
> >>
> >> In the first patch, the commit message mentions that the tool parses the
> >> TF-A source code to get a list of supported UUIDs. However,
> >> tools/binman/fip_util.py seems to embed a hard-coded list of these
> >> UUIDs. I think I might be missing something there... Does it just mean
> >> that the said list was generated using some other script that parsed the
> >> TF-A code? Or does the tool really parse any TF-A code dynamically?
> >
> > A bit of both. The tool allows creating a new version of itself with
> > the updates parsed from the source code. For anything other than local
> > use, a patch must be submitted to do the updates.
> >
> > To run the tool::
> >
> >     $ tools/binman/fip_util.py  -s /path/to/arm-trusted-firmware
> >     Warning: UUID 'UUID_NON_TRUSTED_WORLD_KEY_CERT' is not mentioned
> > in tbbr_config.c file
> >     Existing code in 'tools/binman/fip_util.py' is up-to-date
> >
> > If it shows there is an update, it writes a new version of `fip_util.py`
> > to `fip_util.py.out`. You can change the output file using the `-i` flag.
> > If you have a problem, use `-D` to enable traceback debugging.c
> >
> > You can see that in the docs in this patch:
> >
> >
> http://patchwork.ozlabs.org/project/uboot/patch/20211123210849.2.Idf2f2a46d26cdecb56b6e9f40472f62fd062e346@changeid/
> >
> >> As you may know, we sometimes add new image types in TF-A so I am just
> >> wondering how you intend to keep in sync with these changes and how
> >> automated the process would be.
> >
> > See above.
> >
> >>
> >> I second François' concerns about having 2 different implementations of
> >> fiptool, even if you're trying to solve different (or bigger) problems
> >> here. This could be confusing for users. Also, it is likely to generate
> >> maintenance work for both TF-A and U-boot projects.
> >>
> >> I am not saying the tool should stay within the TF-A project, though.
> >> It's been in the back of our minds for some time that this tool should
> >> have a life of its own, given that it packages more than just TF-A
> >> binaries, but also the normal world bootloader, secure payload, ...
> >> Also, I must admit that a python implementation looks better than a C
> >> implementation. Rewriting the tool in a scripting language has also been
> >> a goal of ours for a long time, although we never got round to do it.
> >>
> >> Simon, you've mentioned that binman has grown out of U-Boot. How
> >> independent is it from U-Boot right now? Are there lots of assumptions
> >> about U-Boot environment in it? Or is it already a general firmware
> >> image packager in your mind? I just want to explore the idea of
> >> replacing fiptool by binman in the future. I am sure we're not there
> >> yet, neither from TF-A perspective nor U-Boot, but I'd be keen on
> >> understanding how far we are. Also, this would need discussion with the
> >> broader TF-A community.
> >
> > Binman is a general-purpose packaging tool. It has some specific
> > features for U-Boot, Chrome OS and coreboot so far. I think it could
> > cover TF-A's needs also.
> >
> > A key point is that Binman has two related purposes:
> > - building an initial image, perhaps just for development/CI purposes
> > (no signatures, some blobs missing, etc.)
> > - building a production/real image when everything is available
> >
> > This is a concept that I very much struggle to get across, the
> > difference between building things and packaging them. I believe it is
> > becoming increasingly important to make this distinction, as firmware
> > fragments.
> >

indeed. Signature workflow needs quite a lot of attention. Companies in
Global Semi-conductor Alliance are targeting also traceability are the
hardware level (able to identify the wafer origin, who cut it into chips,
PCB mounter…) and signing software/firmware will need to fit in this
process. The workgroup dealing with this is just being created.

>
> > Some people will prefer to have C tools instead of Python, but if that
> > is not a concern, then I believe Binman could be a good solution for
> > TF-A. A few nice properties are that it is easy to extend and has 100%
> > test coverage.
> >
> > I would be happy to help with what TF-A needs here.
> >
> > One last point is that Binman can provide an 'fdtmap' which is a full
> > image description. This can provide insight into every binary in the
> > image, whether it is in a FIP, FIT, CBFS or whatever. Binman happens
> > to support generating an FMAP (which is vaguely similar to FIP), which
> > could serve as a model for generating table-of-contents data in other
> > useful formats.
> >

that looks quite exciting. Have you touched base with the LunuxBoot
community which may also have some needs related to packaging?

>
> > There is a talk here that might help to explain the goals better:
> >
> > https://www.youtube.com/watch?v=L84ujgUXBOQ
> >
> > This patch shows converting lots of shell commands into a binman
> definition:
> >
> >
> https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-sjg@chromium.org/
>
> Thanks for the information, this certainly looks interesting to me in
> the context of TF-A! I'll have a closest look at the patches and
> resources you've pointed me to. I'll also talk to my team, see what they
> think and get back to you.

Simon, Sandrine, I think we had a problem with uefi signing tool being in
edk2 repo in the past. Should there be a separate project for binman
(packaging and signing) outside any firmware project ? I ask this in the
context of the GSA thing where we may have to have a tool that fits in
various industrial processes. It may become quite a rich tool and may even
need certification (just thinking out loud here). So rather than making
multiple efforts in firmware projects, we could join forces in one
dedicated project.

>
>
> Regards,
> Sandrine
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>
-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.ozog@linaro.org | Skype: ffozog

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)
  2021-12-04 12:35                 ` François Ozog
@ 2021-12-05 20:00                   ` Simon Glass
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Glass @ 2021-12-05 20:00 UTC (permalink / raw)
  To: François Ozog
  Cc: Sandrine Bailleux, Ilias Apalodimas, Jan Kiszka, U-Boot Mailing List

Hi François,

On Sat, 4 Dec 2021 at 05:35, François Ozog <francois.ozog@linaro.org> wrote:
>
> Hi Simon and Sandrine
>
> Le jeu. 2 déc. 2021 à 08:10, Sandrine Bailleux <sandrine.bailleux@arm.com> a écrit :
>>
>> Hi Simon,
>>
>> On 12/1/21 5:51 PM, Simon Glass wrote:
>> > Hi Sandrine,
>> >
>> > On Wed, 1 Dec 2021 at 03:32, Sandrine Bailleux
>> > <sandrine.bailleux@arm.com> wrote:
>> >>
>> >> Hi everyone,
>> >>
>> >> I am Sandrine Bailleux, from the Trusted Firmware-A project. Ilias
>> >> Apalodimas CC'ed me on this thread.
>> >>
>> >> First of all, thanks for involving the TF-A developers in this thread
>> >> and my apologies for the delay in responding.
>> >
>> > Thank you for your response.
>> >
>> >>
>> >> On 11/25/21 6:01 PM, François Ozog wrote:
>> >>> Hi Simon,
>> >>>
>> >>> On Thu, 25 Nov 2021 at 17:49, Simon Glass <sjg@chromium.org
>> >>> <mailto:sjg@chromium.org>> wrote:
>> >>>
>> >>>     Hi François,
>> >>>
>> >>>     On Thu, 25 Nov 2021 at 08:11, François Ozog
>> >>>     <francois.ozog@linaro.org <mailto:francois.ozog@linaro.org>> wrote:
>> >>>     >
>> >>>     > Hi Simon,
>> >>>     >
>> >>>     >
>> >>>     >
>> >>>     >
>> >>>     > On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas
>> >>>     <ilias.apalodimas@linaro.org <mailto:ilias.apalodimas@linaro.org>>
>> >>>     wrote:
>> >>>     >>
>> >>>     >> +cc Sandrine
>> >>>     >>
>> >>>     >> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
>> >>>     >> <ilias.apalodimas@linaro.org
>> >>>     <mailto:ilias.apalodimas@linaro.org>> wrote:
>> >>>     >> >
>> >>>     >> > Hi Simon,
>> >>>     >> >
>> >>>     >> >
>> >>>     >> > On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org
>> >>>     <mailto:sjg@chromium.org>> wrote:
>> >>>     >> > >
>> >>>     >> > >
>> >>>     >> > > This series adds support for the FIP format as used by ARM
>> >>>     Trusted
>> >>>     >> > > Firmware (in particular TF-A).
>> >>>     >> > >
>> >>>     >
>> >>>     > I will use a question you use often "what problem are you trying
>> >>>     to solve?". If FIP format is used it means that TF-A/BL2 is going to
>> >>>     parse it and verify the hashes/signatures according to TF-A scheme.
>> >>>     >
>> >>>     > The packager will embed in a FIP components like Secure Monitor,
>> >>>     Secure hypervisor, Secure partitions code and manifests.
>> >>>     >
>> >>>     > All in all, U-Boot will be representing a small percentage of the
>> >>>     functionality offered by secure firmware  as a whole and it feels
>> >>>     odd to make another implementation that is more "accessory" rather
>> >>>     than critical for the U-Boot community. It may be a good idea but I
>> >>>     wish you could explain it.
>> >>>
>> >>>     Here is a talk about Binman, its goals and how it works.
>> >>>
>> >>>     https://www.youtube.com/watch?v=L84ujgUXBOQ
>> >>>
>> >>>     Think of Binman as a separate tool that brings everything together. It
>> >>>     has grown out of U-Boot, largely because U-Boot is the main firmware
>> >>>     in most cases. Getting U-Boot to start up and boot successfully is the
>> >>>     goal of this packaging process. There are lots of instructions in the
>> >>>     tree and elsewhere about how to build an image comprising U-Boot and
>> >>>     various binary blobs. Binman aims to provide documentation for that
>> >>>     process and an image description that can be used to build an image
>> >>>     reliably.
>> >>>
>> >>>     We are slowly converting these text instructions into an in-tree image
>> >>>     description.
>> >>>
>> >>>     I believe that Binman can help bring order to the chaos that is
>> >>>     otherwise only going to grow, with lots of shell scripts, manual
>> >>>     instructions, obscure binary tools, etc. Binman brings everything
>> >>>     together and makes it clear what is needing/missing to build an image.
>> >>>
>> >>>     >
>> >>>     >> > > This allows images to be created containing a FIP, which
>> >>>     itself contains
>> >>>     >> > > various binaries. With this, image creation can be handled
>> >>>     from an in-tree
>> >>>     >> > > image description instead of needing to perform a lot of
>> >>>     manual steps or
>> >>>     >> > > custom scripts to build the FIP.
>> >>>     >> > >
>> >>>     >
>> >>>     > That's not my experience of building a fip.  Packaging even Linux
>> >>>     as a BL33 (instead of U-Boot) is very simple.
>> >>>
>> >>>     But not automatic. With Binman you don't need to worry about the
>> >>>     packaging. It is done for you. You just need to find all the binary
>> >>>     blobs that are needed.  This ability is quite important as firmware is
>> >>>     fragmenting and getting very complicated these days.
>> >>>
>> >>>     Binman runs twice...once in the U-Boot tree to do a build and again
>> >>>     later to repackage, put in a final fdtmap, add signatures and any
>> >>>     final pieces needed.
>> >>>
>> >>>     See this patch for an example of complicated build instructions with
>> >>>     Odroid-C2 (>10 binary blobs!) and how Binman can help (see the changes
>> >>>     in the .rst file):
>> >>>
>> >>>     https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-sjg@chromium.org/
>> >>>
>> >>> That's indeed complicated! I can't comment whether this build process is
>> >>> "canonical" as per TF-A standards so I'll let TF-A community comment.
>> >>> Have you factored in the signature issues that Jan@Siemens has in the
>> >>> overall process?
>> >>
>> >> I am a bit confused by the ask here. What input would you like from TF-A
>> >> community? Are you asking for a code review or are you more interested
>> >> in getting an opinion about adding support for FIP files in binman?
>> >
>> > The latter.
>> >
>> >>
>> >> Regardless, I had a brief look at the patches and I have some early
>> >> questions/comments.
>> >>
>> >> In the first patch, the commit message mentions that the tool parses the
>> >> TF-A source code to get a list of supported UUIDs. However,
>> >> tools/binman/fip_util.py seems to embed a hard-coded list of these
>> >> UUIDs. I think I might be missing something there... Does it just mean
>> >> that the said list was generated using some other script that parsed the
>> >> TF-A code? Or does the tool really parse any TF-A code dynamically?
>> >
>> > A bit of both. The tool allows creating a new version of itself with
>> > the updates parsed from the source code. For anything other than local
>> > use, a patch must be submitted to do the updates.
>> >
>> > To run the tool::
>> >
>> >     $ tools/binman/fip_util.py  -s /path/to/arm-trusted-firmware
>> >     Warning: UUID 'UUID_NON_TRUSTED_WORLD_KEY_CERT' is not mentioned
>> > in tbbr_config.c file
>> >     Existing code in 'tools/binman/fip_util.py' is up-to-date
>> >
>> > If it shows there is an update, it writes a new version of `fip_util.py`
>> > to `fip_util.py.out`. You can change the output file using the `-i` flag.
>> > If you have a problem, use `-D` to enable traceback debugging.c
>> >
>> > You can see that in the docs in this patch:
>> >
>> > http://patchwork.ozlabs.org/project/uboot/patch/20211123210849.2.Idf2f2a46d26cdecb56b6e9f40472f62fd062e346@changeid/
>> >
>> >> As you may know, we sometimes add new image types in TF-A so I am just
>> >> wondering how you intend to keep in sync with these changes and how
>> >> automated the process would be.
>> >
>> > See above.
>> >
>> >>
>> >> I second François' concerns about having 2 different implementations of
>> >> fiptool, even if you're trying to solve different (or bigger) problems
>> >> here. This could be confusing for users. Also, it is likely to generate
>> >> maintenance work for both TF-A and U-boot projects.
>> >>
>> >> I am not saying the tool should stay within the TF-A project, though.
>> >> It's been in the back of our minds for some time that this tool should
>> >> have a life of its own, given that it packages more than just TF-A
>> >> binaries, but also the normal world bootloader, secure payload, ...
>> >> Also, I must admit that a python implementation looks better than a C
>> >> implementation. Rewriting the tool in a scripting language has also been
>> >> a goal of ours for a long time, although we never got round to do it.
>> >>
>> >> Simon, you've mentioned that binman has grown out of U-Boot. How
>> >> independent is it from U-Boot right now? Are there lots of assumptions
>> >> about U-Boot environment in it? Or is it already a general firmware
>> >> image packager in your mind? I just want to explore the idea of
>> >> replacing fiptool by binman in the future. I am sure we're not there
>> >> yet, neither from TF-A perspective nor U-Boot, but I'd be keen on
>> >> understanding how far we are. Also, this would need discussion with the
>> >> broader TF-A community.
>> >
>> > Binman is a general-purpose packaging tool. It has some specific
>> > features for U-Boot, Chrome OS and coreboot so far. I think it could
>> > cover TF-A's needs also.
>> >
>> > A key point is that Binman has two related purposes:
>> > - building an initial image, perhaps just for development/CI purposes
>> > (no signatures, some blobs missing, etc.)
>> > - building a production/real image when everything is available
>> >
>> > This is a concept that I very much struggle to get across, the
>> > difference between building things and packaging them. I believe it is
>> > becoming increasingly important to make this distinction, as firmware
>> > fragments.
>> >
>
> indeed. Signature workflow needs quite a lot of attention. Companies in Global Semi-conductor Alliance are targeting also traceability are the hardware level (able to identify the wafer origin, who cut it into chips, PCB mounter…) and signing software/firmware will need to fit in this process. The workgroup dealing with this is just being created.
>>
>>
>> > Some people will prefer to have C tools instead of Python, but if that
>> > is not a concern, then I believe Binman could be a good solution for
>> > TF-A. A few nice properties are that it is easy to extend and has 100%
>> > test coverage.
>> >
>> > I would be happy to help with what TF-A needs here.
>> >
>> > One last point is that Binman can provide an 'fdtmap' which is a full
>> > image description. This can provide insight into every binary in the
>> > image, whether it is in a FIP, FIT, CBFS or whatever. Binman happens
>> > to support generating an FMAP (which is vaguely similar to FIP), which
>> > could serve as a model for generating table-of-contents data in other
>> > useful formats.
>> >
>
> that looks quite exciting. Have you touched base with the LunuxBoot community which may also have some needs related to packaging?

No, my entire effort so far was a talk about 3 years ago.

>>
>>
>> > There is a talk here that might help to explain the goals better:
>> >
>> > https://www.youtube.com/watch?v=L84ujgUXBOQ
>> >
>> > This patch shows converting lots of shell commands into a binman definition:
>> >
>> > https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-sjg@chromium.org/
>>
>> Thanks for the information, this certainly looks interesting to me in
>> the context of TF-A! I'll have a closest look at the patches and
>> resources you've pointed me to. I'll also talk to my team, see what they
>> think and get back to you.
>
> Simon, Sandrine, I think we had a problem with uefi signing tool being in edk2 repo in the past. Should there be a separate project for binman (packaging and signing) outside any firmware project ? I ask this in the context of the GSA thing where we may have to have a tool that fits in various industrial processes. It may become quite a rich tool and may even need certification (just thinking out loud here). So rather than making multiple efforts in firmware projects, we could join forces in one dedicated project.

Maybe, people asked the same thing about patman years ago. But so far
only U-Boot has adopted it (and a bit of Chrome OS). Also, while
Binman supports some formats natively it does rely on external tools
for others, just to avoid duplication.

Re the tools that it uses, I'm hoping to add a feature which can tell
you how to get the ones that are missing, or get them automatically,
like buildman does.

I hope it can make packaging data-driven/testable, instead of
everything having untested scripts all over the place.

Regards,
Simon

>>
>>
>>
>> Regards,
>> Sandrine
>> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>
> --
> François-Frédéric Ozog | Director Business Development
> T: +33.67221.6485
> francois.ozog@linaro.org | Skype: ffozog
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/2] binman: Add support for ATF FIP
  2021-11-24  4:08 ` [PATCH 2/2] binman: Add support " Simon Glass
  2021-11-25 14:47   ` Ilias Apalodimas
@ 2021-12-15  0:33   ` Simon Glass
  1 sibling, 0 replies; 18+ messages in thread
From: Simon Glass @ 2021-12-15  0:33 UTC (permalink / raw)
  To: Ilias Apalodimas; +Cc: U-Boot Mailing List, Sandrine Bailleux, Simon Glass

+CC Sandrine

On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org> wrote:
>
> This format is used in firmware binaries so we may as well supported it.
>
> With this patch binman supports creating, listing and updating FIPs, as
> well as extracting files from one, provided that an FDTMAP is also present
> somewhere in the image.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  scripts/pylint.base                      |   1 +
>  tools/binman/entries.rst                 | 154 +++++++++++++
>  tools/binman/etype/atf_fip.py            | 273 +++++++++++++++++++++++
>  tools/binman/ftest.py                    | 217 ++++++++++++++++++
>  tools/binman/test/203_fip.dts            |  21 ++
>  tools/binman/test/204_fip_other.dts      |  22 ++
>  tools/binman/test/205_fip_no_type.dts    |  15 ++
>  tools/binman/test/206_fip_uuid.dts       |  22 ++
>  tools/binman/test/207_fip_ls.dts         |  25 +++
>  tools/binman/test/208_fip_replace.dts    |  33 +++
>  tools/binman/test/209_fip_missing.dts    |  19 ++
>  tools/binman/test/210_fip_size.dts       |  19 ++
>  tools/binman/test/211_fip_bad_align.dts  |  18 ++
>  tools/binman/test/212_fip_collection.dts |  24 ++
>  14 files changed, 863 insertions(+)
>  create mode 100644 tools/binman/etype/atf_fip.py
>  create mode 100644 tools/binman/test/203_fip.dts
>  create mode 100644 tools/binman/test/204_fip_other.dts
>  create mode 100644 tools/binman/test/205_fip_no_type.dts
>  create mode 100644 tools/binman/test/206_fip_uuid.dts
>  create mode 100644 tools/binman/test/207_fip_ls.dts
>  create mode 100644 tools/binman/test/208_fip_replace.dts
>  create mode 100644 tools/binman/test/209_fip_missing.dts
>  create mode 100644 tools/binman/test/210_fip_size.dts
>  create mode 100644 tools/binman/test/211_fip_bad_align.dts
>  create mode 100644 tools/binman/test/212_fip_collection.dts
>
Applied to u-boot-dm/next, thanks!

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/2] binman: Add a utility module for ATF FIP
  2021-11-24  4:08 ` [PATCH 1/2] binman: Add a utility module for ATF FIP Simon Glass
  2021-11-25 14:47   ` Ilias Apalodimas
@ 2021-12-15  0:33   ` Simon Glass
  1 sibling, 0 replies; 18+ messages in thread
From: Simon Glass @ 2021-12-15  0:33 UTC (permalink / raw)
  To: Ilias Apalodimas; +Cc: U-Boot Mailing List, Simon Glass, Sandrine Bailleux

+cc Sandrine

On Wed, 24 Nov 2021 at 06:09, Simon Glass <sjg@chromium.org> wrote:
>
> Add support for this format which is used by ARM Trusted Firmware to find
> firmware binaries to load.
>
> FIP is like a simpler version of FMAP but uses a UUID instead of a name,
> for each entry.
>
> It supports reading a FIP, writing a FIP and parsing the ATF source code
> to get a list of supported UUIDs.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  scripts/pylint.base           |   8 +-
>  tools/binman/fip_util.py      | 653 ++++++++++++++++++++++++++++++++++
>  tools/binman/fip_util_test.py | 405 +++++++++++++++++++++
>  tools/binman/main.py          |   4 +-
>  4 files changed, 1066 insertions(+), 4 deletions(-)
>  create mode 100755 tools/binman/fip_util.py
>  create mode 100755 tools/binman/fip_util_test.py
>
Applied to u-boot-dm/next, thanks!

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-12-15  0:34 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-24  4:08 [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP) Simon Glass
2021-11-24  4:08 ` [PATCH 1/2] binman: Add a utility module for ATF FIP Simon Glass
2021-11-25 14:47   ` Ilias Apalodimas
2021-12-15  0:33   ` Simon Glass
2021-11-24  4:08 ` [PATCH 2/2] binman: Add support " Simon Glass
2021-11-25 14:47   ` Ilias Apalodimas
2021-12-15  0:33   ` Simon Glass
2021-11-25  9:42 ` [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP) Ilias Apalodimas
2021-11-25 14:47   ` Ilias Apalodimas
2021-11-25 15:11     ` François Ozog
2021-11-25 16:49       ` Simon Glass
2021-11-25 17:01         ` François Ozog
2021-11-25 18:16           ` Simon Glass
2021-12-01 10:31           ` Sandrine Bailleux
2021-12-01 16:51             ` Simon Glass
2021-12-02  7:10               ` Sandrine Bailleux
2021-12-04 12:35                 ` François Ozog
2021-12-05 20:00                   ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.