Re: [PATCH 20/24] binman: Support splitting an ELF file into multiple nodes

On 08/02/2022 21:50, Simon Glass wrote:

Some boards need to load an ELF file using the 'loadables' property, but the file has segments at different memory addresses. This means that it cannot be supplied as a flat binary.

Allow generating a separate node in the FIT for each segment in the ELF, with a different load address for each.

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

tools/binman/entries.rst | 146 +++++++++++ tools/binman/etype/fit.py | 259 ++++++++++++++++++- tools/binman/ftest.py | 116 +++++++++ tools/binman/test/221_fit_split_elf.dts | 67 +++++ tools/binman/test/222_fit_bad_dir.dts | 9 + tools/binman/test/223_fit_bad_dir_config.dts | 9 + 6 files changed, 594 insertions(+), 12 deletions(-) create mode 100644 tools/binman/test/221_fit_split_elf.dts create mode 100644 tools/binman/test/222_fit_bad_dir.dts create mode 100644 tools/binman/test/223_fit_bad_dir_config.dts

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index d483169712..079fed1a9c 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -612,6 +612,9 @@ gen-fdt-nodes Generate FDT nodes as above. This is the default if there is no `fit,operation` property.

+split-elf

• Split an ELF file into a separate node for each segment.

Generating nodes from an FDT list (gen-fdt-nodes)

@@ -655,6 +658,149 @@ for each of your two files.
Note that if no devicetree files are provided (with '-a of-list' as above)
then no nodes will be generated.

+Generating nodes from an ELF file (split-elf)
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+This uses the node as a template to generate multiple nodes. The following
+special properties are available:
+
+split-elf
+    Split an ELF file into a separate node for each segment. This uses the
+    node as a template to generate multiple nodes. The following special
+    properties are available:
+
+    fit,load
+        Generates a `load = <...>` property with the load address of the
+        segmnet
+
+    fit,entry
+        Generates a `entry = <...>` property with the entry address of the
+        ELF. This is only produced for the first entry
+
+    fit,data
+        Generates a `data = <...>` property with the contents of the segment

I think all these should be done by default. I don't see the point of not setting the properties, or setting them manually to values that will be the same for multiple nodes.

• fit,loadables

•    Generates a `loadable = <...>` property with a list of the generated
  

•    nodes (including all nodes if this operation is used multiple times)
  

I think this could also be the default behaviour like the above, and even for configs which have a "loadables" property we could append to it by default.

Either way, if the "loadables" property exists this needs to append to it instead of entirely replacing it. (Needs to append to "u-boot", see comment at example generated config below.)

+Here is an example showing ATF, TEE and a device tree all combined::

This looks like a better place to write about the template node design so I'll try here instead of the previous patch. I've always thought they were ugly but I couldn't think of a good enough design when they were introduced. Maybe I have now.

• fit {

•    description = "test-desc";
  

•    #address-cells = <1>;
  

•    fit,fdt-list = "of-list";
  

•    images {
  

•        u-boot {
  

•            description = "U-Boot (64-bit)";
  

•            type = "standalone";
  

•            os = "U-Boot";
  

•            arch = "arm64";
  

•            compression = "none";
  

•            load = <CONFIG_SYS_TEXT_BASE>;
  

•            u-boot-nodtb {
  

•            };
  

•        };
  

•        @fdt-SEQ {
  

•            description = "fdt-NAME.dtb";
  

•            type = "flat_dt";
  

•            compression = "none";
  

•        };
  

Instead of putting these into the images subnode, we could have images-level subnodes for the operations. For example, instead of the above:

images@gen-fdt-nodes { fdt-list = "of-list";

fdt { type = "flat_dt"; compression = "none"; }; };

We can remove the -SEQ if we always append a sequence number, and we can set "description" to "NAME.dtb" when it's missing, or do the replacement when it's given. We can go further and use "%s", "%(name)s" or "{name}" instead of "NAME" for python-ish formatting and likewise for the sequence number.

•        @atf-SEQ {
  

•            fit,operation = "split-elf";
  

•            description = "ARM Trusted Firmware";
  

•            type = "firmware";
  

•            arch = "arm64";
  

•            os = "arm-trusted-firmware";
  

•            compression = "none";
  

•            fit,load;
  

•            fit,entry;
  

•            fit,data;
  

•            atf-bl31 {
  

•            };
  

•        };
  

•        @tee-SEQ {
  

•            fit,operation = "split-elf";
  

•            description = "TEE";
  

•            type = "tee";
  

•            arch = "arm64";
  

•            os = "tee";
  

•            compression = "none";
  

•            fit,load;
  

•            fit,entry;
  

•            fit,data;
  

•            op-tee {
  

•            };
  

•        };
  

•    };
  

Similarly, instead of the above two:

images@split-elf { atf { description = "ARM Trusted Firmware"; type = "firmware"; arch = "arm64"; os = "arm-trusted-firmware"; compression = "none";

atf-bl31 { }; };

tee { description = "TEE"; type = "tee"; arch = "arm64"; os = "tee"; compression = "none";

op-tee { }; }; };

The load, entry, data properties could be set automatically without explicit mention.

•    configurations {
  

•        default = "@config-DEFAULT-SEQ";
  

•        @config-SEQ {
  

•            description = "conf-NAME.dtb";
  

•            fdt = "fdt-SEQ";
  

•            firmware = "u-boot";
  

•            fit,loadables;
  

•        };
  

•    };
  

And instead of the above:

configurations@gen-fdt-nodes { fdt-list = "of-list";

config { fdt = "fdt"; firmware = "u-boot"; }; };

Again, we can automatically add a sequence number to "config" node names and "fdt" property value, and set/replace a description property.

We can collect all loadables from the images@split-elf nodes and create or append to the "loadables" properties. Or we could specify "atf" and "fdt" in the loadables and they could be expanded with sequence numbers.

And if the "default" property of "configurations" node (or even the whole node) is missing, we can make configurations@gen-fdt-nodes set it to the default-dt.

(Remains to be seen if I'll be annoyed enough to implement this myself.)

• };

+If ATF-BL31 is available, this generates a node for each segment in the +ELF file, for example::

• images {

•    atf-1 {
  

•        data = <...contents of first segment...>;
  

•        data-offset = <0x00000000>;
  

•        entry = <0x00040000>;
  

•        load = <0x00040000>;
  

•        compression = "none";
  

•        os = "arm-trusted-firmware";
  

•        arch = "arm64";
  

•        type = "firmware";
  

•        description = "ARM Trusted Firmware";
  

•    };
  

•    atf-2 {
  

•        data = <...contents of second segment...>;
  

•        load = <0xff3b0000>;
  

•        compression = "none";
  

•        os = "arm-trusted-firmware";
  

•        arch = "arm64";
  

•        type = "firmware";
  

•        description = "ARM Trusted Firmware";
  

•    };
  

• };

+The same applies for OP-TEE if that is available.

+If each binary is not available, the relevant template node (@atf-SEQ or +@tee-SEQ) is removed from the output.

I'm running into the fake-file thing with op-tee and getting a "Magic number does not match" error on consecutive builds as the leftover fake op-tee isn't an ELF...

+This also generates a `config-xxx` node for each device tree in `of-list`. +Note that the U-Boot build system uses `-a of-list=$(CONFIG_OF_LIST)` +so you can use `CONFIG_OF_LIST` to define that list. In this example it is +set up for `firefly-rk3399` with a single device tree and the default set +with `-a default-dt=$(CONFIG_DEFAULT_DEVICE_TREE)`, so the resulting output +is::

• configurations {

•    default = "config-1";
  

•    config-1 {
  

•        loadables = "atf-1", "atf-2", "atf-3", "tee-1", "tee-2";
  

•        description = "rk3399-firefly.dtb";
  

•        fdt = "fdt-1";
  

•        firmware = "u-boot";
  

•    };
  

• };

These loadables/firmware values didn't work on my chromebook_kevin. The Rockchip generator gets me:

firmware = "atf-1"; loadables = "u-boot", "atf-2", "atf-3";

instead of the ones above (I didn't pass an op-tee file), and these work when I hardcode them in the binman definition.

+U-Boot SPL can then load the firmware (U-Boot proper) and all the loadables +(ATF and TEE), then proceed with the boot.

Entry: fmap: An entry which contains an Fmap section diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py index 6210deeef7..b2a037c742 100644 --- a/tools/binman/etype/fit.py +++ b/tools/binman/etype/fit.py @@ -6,17 +6,20 @@ #

from collections import defaultdict, OrderedDict +import io import libfdt

from binman.entry import Entry, EntryArg +from binman import elf from dtoc import fdt_util from dtoc.fdt import Fdt from patman import tools

# Supported operations, with the fit,operation property -OP_GEN_FDT_NODES = range(1) +OP_GEN_FDT_NODES, OP_SPLIT_ELF = range(2) OPERATIONS = { 'gen-fdt-nodes': OP_GEN_FDT_NODES,

• 'split-elf': OP_SPLIT_ELF, }

class Entry_fit(Entry): @@ -111,6 +114,9 @@ class Entry_fit(Entry): Generate FDT nodes as above. This is the default if there is no `fit,operation` property.

• split-elf

•    Split an ELF file into a separate node for each segment.
  

• Generating nodes from an FDT list (gen-fdt-nodes)

@@ -153,6 +159,149 @@ class Entry_fit(Entry):

 Note that if no devicetree files are provided (with '-a of-list' as above)
 then no nodes will be generated.

• Generating nodes from an ELF file (split-elf)
• This uses the node as a template to generate multiple nodes. The following
• special properties are available:
• split-elf

•    Split an ELF file into a separate node for each segment. This uses the
  

•    node as a template to generate multiple nodes. The following special
  

•    properties are available:
  

•    fit,load
  

•        Generates a `load = <...>` property with the load address of the
  

•        segmnet
  

•    fit,entry
  

•        Generates a `entry = <...>` property with the entry address of the
  

•        ELF. This is only produced for the first entry
  

•    fit,data
  

•        Generates a `data = <...>` property with the contents of the segment
  

•    fit,loadables
  

•        Generates a `loadable = <...>` property with a list of the generated
  

•        nodes (including all nodes if this operation is used multiple times)
  

• Here is an example showing ATF, TEE and a device tree all combined::

•    fit {
  

•        description = "test-desc";
  

•        #address-cells = <1>;
  

•        fit,fdt-list = "of-list";
  

•        images {
  

•            u-boot {
  

•                description = "U-Boot (64-bit)";
  

•                type = "standalone";
  

•                os = "U-Boot";
  

•                arch = "arm64";
  

•                compression = "none";
  

•                load = <CONFIG_SYS_TEXT_BASE>;
  

•                u-boot-nodtb {
  

•                };
  

•            };
  

•            @fdt-SEQ {
  

•                description = "fdt-NAME.dtb";
  

•                type = "flat_dt";
  

•                compression = "none";
  

•            };
  

•            @atf-SEQ {
  

•                fit,operation = "split-elf";
  

•                description = "ARM Trusted Firmware";
  

•                type = "firmware";
  

•                arch = "arm64";
  

•                os = "arm-trusted-firmware";
  

•                compression = "none";
  

•                fit,load;
  

•                fit,entry;
  

•                fit,data;
  

•                atf-bl31 {
  

•                };
  

•            };
  

•            @tee-SEQ {
  

•                fit,operation = "split-elf";
  

•                description = "TEE";
  

•                type = "tee";
  

•                arch = "arm64";
  

•                os = "tee";
  

•                compression = "none";
  

•                fit,load;
  

•                fit,entry;
  

•                fit,data;
  

•                op-tee {
  

•                };
  

•            };
  

•        };
  

•        configurations {
  

•            default = "@config-DEFAULT-SEQ";
  

•            @config-SEQ {
  

•                description = "conf-NAME.dtb";
  

•                fdt = "fdt-SEQ";
  

•                firmware = "u-boot";
  

•                fit,loadables;
  

•            };
  

•        };
  

•    };
  

• If ATF-BL31 is available, this generates a node for each segment in the
• ELF file, for example::

•    images {
  

•        atf-1 {
  

•            data = <...contents of first segment...>;
  

•            data-offset = <0x00000000>;
  

•            entry = <0x00040000>;
  

•            load = <0x00040000>;
  

•            compression = "none";
  

•            os = "arm-trusted-firmware";
  

•            arch = "arm64";
  

•            type = "firmware";
  

•            description = "ARM Trusted Firmware";
  

•        };
  

•        atf-2 {
  

•            data = <...contents of second segment...>;
  

•            load = <0xff3b0000>;
  

•            compression = "none";
  

•            os = "arm-trusted-firmware";
  

•            arch = "arm64";
  

•            type = "firmware";
  

•            description = "ARM Trusted Firmware";
  

•        };
  

•    };
  

• The same applies for OP-TEE if that is available.
• If each binary is not available, the relevant template node (@atf-SEQ or
• @tee-SEQ) is removed from the output.
• This also generates a `config-xxx` node for each device tree in `of-list`.
• Note that the U-Boot build system uses `-a of-list=$(CONFIG_OF_LIST)`
• so you can use `CONFIG_OF_LIST` to define that list. In this example it is
• set up for `firefly-rk3399` with a single device tree and the default set
• with `-a default-dt=$(CONFIG_DEFAULT_DEVICE_TREE)`, so the resulting output
• is::

•    configurations {
  

•        default = "config-1";
  

•        config-1 {
  

•            loadables = "atf-1", "atf-2", "atf-3", "tee-1", "tee-2";
  

•            description = "rk3399-firefly.dtb";
  

•            fdt = "fdt-1";
  

•            firmware = "u-boot";
  

•        };
  

•    };
  

• U-Boot SPL can then load the firmware (U-Boot proper) and all the loadables
• (ATF and TEE), then proceed with the boot. """ def __init__(self, section, etype, node): """

@@ -182,6 +331,12 @@ class Entry_fit(Entry): str)])[0] self.mkimage = None

•    # List of generated split-elf nodes, each a None
  

•    self._loadables = []
  

•    # /configurations/xxx node used as a template
  

•    self._config_node = None
  

• def ReadNode(self): self.ReadEntries() super().ReadNode()

@@ -251,19 +406,21 @@ class Entry_fit(Entry): in_images: True if this is inside the 'images' node, so that 'data' properties should be generated """

•        if depth == 1 and not in_images:
  

•            self._config_node = subnode
         if self._fdts:
             # Generate nodes for each FDT
             for seq, fdt_fname in enumerate(self._fdts):
  

•                node_name = subnode.name[1:].replace('SEQ',
  

•                                                     str(seq + 1))
  

•                node_name = subnode.name[1:].replace('SEQ', str(seq + 1))
                 fname = tools.GetInputFilename(fdt_fname + '.dtb')
                 with fsw.add_node(node_name):
                     for pname, prop in subnode.props.items():
  

•                        val = prop.bytes.replace(
  

•                            b'NAME', tools.ToBytes(fdt_fname))
  

•                        val = val.replace(
  

•                            b'SEQ', tools.ToBytes(str(seq + 1)))
  

•                        fsw.property(pname, val)
  

•                        if not pname.startswith('fit,'):
  

•                            val = prop.bytes.replace(
  

•                                b'NAME', tools.ToBytes(fdt_fname))
  

•                            val = val.replace(
  

•                                b'SEQ', tools.ToBytes(str(seq + 1)))
  

•                            fsw.property(pname, val)
  
                     # Add data for 'images' nodes (but not 'config')
                     if depth == 1 and in_images:
  

@@ -277,7 +434,18 @@ class Entry_fit(Entry): else: self.Raise("Generator node requires 'fit,fdt-list' property")

•    def _scan_node(subnode, depth, in_images):
  

•    def _scan_split_elf(subnode, rel_path):
  

•        #data, input_fname, uniq = self.collect_contents_to_file(
  

•        entry = Entry.Create(self.section, subnode, etype='section')
  

•        entry.ReadNode()
  

•        self._fit_sections[rel_path] = entry
  

I did rebase this on top of dm-pull-8feb22 and my other two patches, and this series started failing on CheckEntries() complaining about the template node's entry's content not fitting inside the FIT section. I didn't really track it down but I guess it's due to adding it to self._entries here or somewhere else.

•        # Add this as a dummy node so we know the required position in the
  

•        # output FIT. It is replaced later in _BuildInput().
  

•        with fsw.add_node(subnode.name):
  

•            pass
  

•    def _scan_node(subnode, depth, in_images, rel_path):
         """Generate nodes from a template
  
         This creates one node for each member of self._fdts using the
  

@@ -291,10 +459,14 @@ class Entry_fit(Entry): depth: Current node depth (0 is the base 'fit' node) in_images: True if this is inside the 'images' node, so that 'data' properties should be generated

•            rel_path (str): Path of subnode relative to the toplevel 'fit'
  

•                node
         """
         oper = self._get_operation(subnode)
         if oper == OP_GEN_FDT_NODES:
             _scan_gen_fdt_nodes(subnode, depth, in_images)
  

•        elif oper == OP_SPLIT_ELF:
  

•            _scan_split_elf(subnode, rel_path)
  
     def _AddNode(base_node, depth, node):
         """Add a node to the FIT
  

@@ -334,7 +506,8 @@ class Entry_fit(Entry): # fsw.add_node() or _AddNode() for it. pass elif self.GetImage().generate and subnode.name.startswith('@'):

•                _scan_node(subnode, depth, in_images)
  

•                _scan_node(subnode, depth, in_images,
  

•                           f'{rel_path}/{subnode.name}')
             else:
                 with fsw.add_node(subnode.name):
                     _AddNode(base_node, depth + 1, subnode)
  

@@ -383,6 +556,45 @@ class Entry_fit(Entry):

     return True

• def _add_split_elf(self, orig_node, node, data, missing):

•    """Add nodes for the ELF file, one per group of contiguous segments
  

•    The existing placeholder node is replaced
  

•    Args:
  

•        orig_node (Node): Template node from the binman definition
  

•        node (Node): Node to replace (in the FIT being built)
  

•        data (bytes): ELF-format data to process (may be empty)
  

•    """
  

•    # If any pieces are missing, skip this. The missing entries will show
  

•    # an error
  

•    if not missing:
  

•        try:
  

•            segments, entry = elf.read_segments(data)
  

•        except ValueError as exc:
  

•            self.Raise(f'Failed to read ELF file for {orig_node.path}: {str(exc)}')
  

•        parent = node.parent
  

•        for (seq, start, data) in segments:
  

•            node_name = orig_node.name[1:].replace('SEQ', str(seq + 1))
  

•            subnode = parent.AddSubnode(node_name) #, before=node)
  

•            self._loadables.append(subnode)
  

•            for pname, prop in orig_node.props.items():
  

•                if not pname.startswith('fit,'):
  

•                    subnode.AddData(pname, prop.bytes)
  

•                elif pname == 'fit,load':
  

•                    subnode.AddInt('load', start)
  

•                elif pname == 'fit,entry':
  

•                    if not seq:
  

•                        subnode.AddInt('entry', entry)
  

•                elif pname == 'fit,data':
  

•                    subnode.AddData('data', data)
  

•                elif pname != 'fit,operation':
  

•                    self.Raise(
  

•                        f"Unknown directive in '{subnode.path}': '{pname}'")
  

•    # Delete the template node as it has served its purpose
  

•    node.Delete()
  

Also, with the fit-as-section patches I get an error in SetImagePos() due to a template subentry in self._entries getting None as its node, thought it's related to the deletion here but didn't confirm.

• def _BuildInput(self, fdt): """Finish the FIT by adding the 'data' properties to it

@@ -393,13 +605,36 @@ class Entry_fit(Entry): New fdt contents (bytes) """ for path, section in self._fit_sections.items():

•        node = fdt.GetNode(path)
         # Entry_section.ObtainContents() either returns True or
         # raises an exception.
         section.ObtainContents()
         section.Pack(0)
         data = section.GetData()
  

•        node.AddData('data', data)
  

•        missing_list = []
  

•        section.CheckMissing(missing_list)
  

•        node = fdt.GetNode(path)
  

•        oper = self._get_operation(section._node)
  

•        if oper == OP_GEN_FDT_NODES:
  

•            node.AddData('data', data)
  

•        elif oper == OP_SPLIT_ELF:
  

•            self._add_split_elf(section._node, node, data,
  

•                                bool(missing_list))
  

•    # Set up the 'firmware' and 'loadables' properties in all
  

•    # 'configurations' nodes, but only if we are generating FDTs. Note that
  

•    # self._config_node is set in _scan_gen_fdt_nodes()
  

•    node = fdt.GetNode('/configurations')
  

•    if self._config_node:
  

•        for subnode in node.subnodes:
  

•            for pname, prop in self._config_node.props.items():
  

•                if pname == 'fit,loadables':
  

•                    subnode.AddStringList(
  

•                        'loadables',
  

•                        [node.name for node in self._loadables])
  

As mentioned above, if "loadables" exists this should append to it.

•                elif pname.startswith('fit,'):
  

•                    self.Raise(
  

•                        f"Unknown directive in '{subnode.path}': '{pname}'")
  
     fdt.Sync(auto_resize=True)
     data = fdt.GetContents()
  

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 5a0dc70ed9..dbaf412e9d 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -197,6 +197,13 @@ class TestFunctional(unittest.TestCase):

     TestFunctional._MakeInputFile('env.txt', ENV_DATA)

•    # ELF file with two sections in different parts of memory, used for both
  

•    # ATF and OP_TEE
  

•    TestFunctional._MakeInputFile('bl31.elf',
  

•        tools.ReadFile(cls.ElfTestFile('elf_sections')))
  

•    TestFunctional._MakeInputFile('tee.elf',
  

•        tools.ReadFile(cls.ElfTestFile('elf_sections')))
  

•    cls.have_lz4 = comp_util.HAVE_LZ4
  
  

  @classmethod

@@ -5125,6 +5132,115 @@ fdt fdtmap Extract the devicetree blob from the fdtmap self.assertIn("Node '/binman/fit': Unknown operation 'unknown'", str(exc.exception))

• def testFitSplitElf(self):

•    """Test an image with an FIT with an split-elf operation"""
  

•    entry_args = {
  

•        'of-list': 'test-fdt1 test-fdt2',
  

•        'default-dt': 'test-fdt2',
  

•        'atf-bl31-path': 'bl31.elf',
  

•        'op-tee-path': 'tee.elf',
  

•    }
  

•    test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR)
  

•    data = self._DoReadFileDtb(
  

•        '221_fit_split_elf.dts',
  

•        entry_args=entry_args,
  

•        extra_indirs=[test_subdir])[0]
  

•    self.assertEqual(U_BOOT_NODTB_DATA, data[-len(U_BOOT_NODTB_DATA):])
  

•    fit_data = data[len(U_BOOT_DATA):-len(U_BOOT_NODTB_DATA)]
  

•    base_keys = {'description', 'type', 'arch', 'os', 'compression',
  

•                 'data', 'load'}
  

•    dtb = fdt.Fdt.FromData(fit_data)
  

•    dtb.Scan()
  

•    elf_data = tools.ReadFile(os.path.join(self._indir, 'bl31.elf'))
  

•    segments, entry = elf.read_segments(elf_data)
  

•    # We assume there are two segments
  

•    self.assertEquals(2, len(segments))
  

•    atf1 = dtb.GetNode('/images/atf-1')
  

•    _, start, data = segments[0]
  

•    self.assertEqual(base_keys | {'entry'}, atf1.props.keys())
  

•    self.assertEqual(entry,
  

•                     fdt_util.fdt32_to_cpu(atf1.props['entry'].value))
  

•    self.assertEqual(start,
  

•                     fdt_util.fdt32_to_cpu(atf1.props['load'].value))
  

•    self.assertEqual(data, atf1.props['data'].bytes)
  

•    atf2 = dtb.GetNode('/images/atf-2')
  

•    self.assertEqual(base_keys, atf2.props.keys())
  

•    _, start, data = segments[1]
  

•    self.assertEqual(start,
  

•                     fdt_util.fdt32_to_cpu(atf2.props['load'].value))
  

•    self.assertEqual(data, atf2.props['data'].bytes)
  

•    conf = dtb.GetNode('/configurations')
  

•    self.assertEqual({'default'}, conf.props.keys())
  

•    for subnode in conf.subnodes:
  

•        self.assertEqual({'description', 'fdt', 'loadables'},
  

•                         subnode.props.keys())
  

•        self.assertEqual(
  

•            ['atf-1', 'atf-2', 'tee-1', 'tee-2'],
  

•            fdt_util.GetStringList(subnode, 'loadables'))
  

• def _check_bad_fit(self, dts):

•    """Check a bad FIT
  

•    This runs with the given dts and returns the assertion raised
  

•    Args:
  

•        dts (str): dts filename to use
  

•    Returns:
  

•        str: Assertion string raised
  

•    """
  

•    entry_args = {
  

•        'of-list': 'test-fdt1 test-fdt2',
  

•        'default-dt': 'test-fdt2',
  

•        'atf-bl31-path': 'bl31.elf',
  

•        'op-tee-path': 'tee.elf',
  

•    }
  

•    test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR)
  

•    with self.assertRaises(ValueError) as exc:
  

•        self._DoReadFileDtb(dts, entry_args=entry_args,
  

•                            extra_indirs=[test_subdir])[0]
  

•    return str(exc.exception)
  

• def testFitSplitElfBadElf(self):

•    """Test a FIT split-elf operation with an invalid ELF file"""
  

•    TestFunctional._MakeInputFile('bad.elf', tools.GetBytes(100, 100))
  

•    entry_args = {
  

•        'of-list': 'test-fdt1 test-fdt2',
  

•        'default-dt': 'test-fdt2',
  

•        'atf-bl31-path': 'bad.elf',
  

This test fails for me with the fit-as-section patches but adding 'op-tee-path' here like the ones above makes it succeed. Though, you might have meant to test a missing op-tee here in addition to the bad.elf.

Could use an independent test that checks a missing op-tee doesn't result in an error, as it should just omit the nodes in the FIT.

•    }
  

•    test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR)
  

•    with self.assertRaises(ValueError) as exc:
  

•        self._DoReadFileDtb(
  

•            '221_fit_split_elf.dts',
  

•            entry_args=entry_args,
  

•            extra_indirs=[test_subdir])[0]
  

•    self.assertIn(
  

•        "Node '/binman/fit': Failed to read ELF file for /binman/fit/images/@atf-SEQ: Magic number does not match",
  

•        str(exc.exception))
  

• def testFitSplitElfBadDirective(self):

•    """Test a FIT split-elf invalid fit,xxx directive in an image node"""
  

•    err = self._check_bad_fit('222_fit_bad_dir.dts')
  

•    self.assertIn(
  

•        "Node '/binman/fit': Unknown directive in '/images/atf-1': 'fit,something'",
  

•        err)
  

• def testFitSplitElfBadDirectiveConfig(self):

•    """Test a FIT split-elf with invalid fit,xxx directive in config"""
  

•    err = self._check_bad_fit('223_fit_bad_dir_config.dts')
  

•    self.assertEqual(
  

•        "Node '/binman/fit': Unknown directive in '/configurations/config-1': 'fit,config'",
  

•        err)
  

if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/221_fit_split_elf.dts b/tools/binman/test/221_fit_split_elf.dts new file mode 100644 index 0000000000..ec771bd116 --- /dev/null +++ b/tools/binman/test/221_fit_split_elf.dts @@ -0,0 +1,67 @@ +// SPDX-License-Identifier: GPL-2.0+

+/dts-v1/;

+/ {

• #address-cells = <1>;
• #size-cells = <1>;
• binman {

• u-boot {
  

• };
  

• fit {
  

• 	description = "test-desc";
  

• 	#address-cells = <1>;
  

• 	fit,fdt-list = "of-list";
  

• 	images {
  

• 		@fdt-SEQ {
  

• 			description = "fdt-NAME.dtb";
  

• 			type = "flat_dt";
  

• 			compression = "none";
  

• 		};
  

• 		atf: @atf-SEQ {
  

• 			fit,operation = "split-elf";
  

• 			description = "ARM Trusted Firmware";
  

• 			type = "firmware";
  

• 			arch = "arm64";
  

• 			os = "arm-trusted-firmware";
  

• 			compression = "none";
  

• 			fit,load;
  

• 			fit,entry;
  

• 			fit,data;
  

• 			atf-bl31 {
  

• 			};
  

• 		};
  

• 		@tee-SEQ {
  

• 			fit,operation = "split-elf";
  

• 			description = "TEE";
  

• 			type = "tee";
  

• 			arch = "arm64";
  

• 			os = "tee";
  

• 			compression = "none";
  

• 			fit,load;
  

• 			fit,entry;
  

• 			fit,data;
  

• 			op-tee {
  

• 			};
  

• 		};
  

• 	};
  

• 	configurations {
  

• 		default = "@config-DEFAULT-SEQ";
  

• 		config: @config-SEQ {
  

• 			description = "conf-NAME.dtb";
  

• 			fdt = "fdt-SEQ";
  

• 			fit,loadables;
  

• 		};
  

• 	};
  

• };
  

• u-boot-nodtb {
  

• };
  

• };

+}; diff --git a/tools/binman/test/222_fit_bad_dir.dts b/tools/binman/test/222_fit_bad_dir.dts new file mode 100644 index 0000000000..91733c74c4 --- /dev/null +++ b/tools/binman/test/222_fit_bad_dir.dts @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0+

+/dts-v1/;

+#include "221_fit_split_elf.dts"

+&atf {

• fit,something = "bad";

+}; diff --git a/tools/binman/test/223_fit_bad_dir_config.dts b/tools/binman/test/223_fit_bad_dir_config.dts new file mode 100644 index 0000000000..17dae0c5b6 --- /dev/null +++ b/tools/binman/test/223_fit_bad_dir_config.dts @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0+

+/dts-v1/;

+#include "221_fit_split_elf.dts"

+&config {

• fit,config = "bad";

+};