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

Alper Nebi Yasak
2022-03-03T21:19:14Z

On 24/02/2022 02:00, 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.
>
> Also add checks that the fit,xxx directives are valid.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Rewrite this to use the new FIT entry-type implementation
> - Rename op-tee to tee-os
>
> tools/binman/entries.rst | 146 ++++++++++++
> tools/binman/etype/fit.py | 230 ++++++++++++++++++-
> tools/binman/ftest.py | 147 ++++++++++++
> tools/binman/test/226_fit_split_elf.dts | 67 ++++++
> tools/binman/test/227_fit_bad_dir.dts | 9 +
> tools/binman/test/228_fit_bad_dir_config.dts | 9 +
> 6 files changed, 598 insertions(+), 10 deletions(-)
> create mode 100644 tools/binman/test/226_fit_split_elf.dts
> create mode 100644 tools/binman/test/227_fit_bad_dir.dts
> create mode 100644 tools/binman/test/228_fit_bad_dir_config.dts
>
> [...]
>
> diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> index 30b20a07a2..63d552ed19 100644> > [...]
>
> @@ -154,6 +159,149 @@ class Entry_fit(Entry_section):
>
> 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

segmnet -> segment, missed it the first time around.

> +
> + 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)
>
> [...]
>
> @@ -363,11 +514,20 @@ class Entry_fit(Entry_section):
> fname = tools.get_input_filename(fdt_fname + '.dtb')
> with fsw.add_node(node_name):
> for pname, prop in node.props.items():
> - val = prop.bytes.replace(
> - b'NAME', tools.to_bytes(fdt_fname))
> - val = val.replace(
> - b'SEQ', tools.to_bytes(str(seq + 1)))
> - fsw.property(pname, val)
> + if pname == 'fit,loadables':
> + val = '\0'.join(self._loadables) + '\0'
> + fsw.property('loadables', val.encode('utf-8'))
> + elif pname == 'fit,operation':
> + pass
> + elif pname.startswith('fit,'):
> + self._raise(base_node, node,
> + f"Unknown directive '{pname}'")
> + else:
> + val = prop.bytes.replace(
> + b'NAME', tools.to_bytes(fdt_fname))
> + val = val.replace(
> + b'SEQ', tools.to_bytes(str(seq + 1)))
> + fsw.property(pname, val)

I get two distinct "loadables" properties with this config fragment,
and fdtget and mkimage -l only considers the first one:

@config-SEQ {
description = "NAME.dtb";
fdt = "fdt-SEQ";
firmware = "atf-1";
loadables = "u-boot";
fit,loadables;
};

$ fdtget simple-bin.fit.fit /configurations/config-1 -p
description
fdt
firmware
loadables
loadables

$ fdtget simple-bin.fit.fit /configurations/config-1 loadables
u-boot

$ mkimage -l simple-bin.fit.fit
FIT description: FIT image for U-Boot with bl31 (TF-A)
Created: Thu Feb 24 17:36:36 2022
Image 0 (u-boot)
Description: U-Boot (64-bit)
Created: Thu Feb 24 17:36:36 2022
Type: Standalone Program
Compression: uncompressed
Data Size: 727192 Bytes = 710.15 KiB = 0.69 MiB
Architecture: AArch64
Load Address: 0x00200000
Entry Point: unavailable
Image 1 (atf-1)
Description: ARM Trusted Firmware
Created: Thu Feb 24 17:36:36 2022
Type: Firmware
Compression: uncompressed
Data Size: 139342 Bytes = 136.08 KiB = 0.13 MiB
Architecture: AArch64
OS: ARM Trusted Firmware
Load Address: 0x00040000
Image 2 (atf-2)
Description: ARM Trusted Firmware
Created: Thu Feb 24 17:36:36 2022
Type: Firmware
Compression: uncompressed
Data Size: 8024 Bytes = 7.84 KiB = 0.01 MiB
Architecture: AArch64
OS: ARM Trusted Firmware
Load Address: 0xff3b0000
Image 3 (atf-3)
Description: ARM Trusted Firmware
Created: Thu Feb 24 17:36:36 2022
Type: Firmware
Compression: uncompressed
Data Size: 8192 Bytes = 8.00 KiB = 0.01 MiB
Architecture: AArch64
OS: ARM Trusted Firmware
Load Address: 0xff8c0000
Image 4 (fdt-1)
Description: fdt-rk3399-gru-bob
Created: Thu Feb 24 17:36:36 2022
Type: Flat Device Tree
Compression: uncompressed
Data Size: 69392 Bytes = 67.77 KiB = 0.07 MiB
Architecture: Unknown Architecture
Default Configuration: 'config-1'
Configuration 0 (config-1)
Description: rk3399-gru-bob.dtb
Kernel: unavailable
Firmware: atf-1
FDT: fdt-1
Loadables: u-boot

I didn't test if my chromebook_kevin boots in this double-loadables
configuration, though.

>
> # Add data for 'images' nodes (but not 'config')
> if depth == 1 and in_images:
> @@ -380,7 +540,43 @@ class Entry_fit(Entry_section):
> else:
> self.Raise("Generator node requires 'fit,fdt-list' property")
>
> - def _gen_node(base_node, node, depth, in_images):
> + def _gen_split_elf(base_node, node, elf_data, missing):
> + """Add nodes for the ELF file, one per group of contiguous segments
> +
> + Args:
> + node (Node): Template node from the binman definition

node -> base_node

> + node (Node): Node to replace (in the FIT being built)
> + data (bytes): ELF-format data to process (may be empty)
> + missing (bool): True if any of the data is missing
> +
> + """
> + # If any pieces are missing, skip this. The missing entries will
> + # show an error
> + if not missing:
> + try:
> + segments, entry = elf.read_loadable_segments(elf_data)
> + except ValueError as exc:
> + self._raise(base_node, node,
> + f'Failed to read ELF file: {str(exc)}')
> + for (seq, start, data) in segments:
> + node_name = node.name[1:].replace('SEQ', str(seq + 1))
> + with fsw.add_node(node_name):
> + loadables.append(node_name)
> + for pname, prop in node.props.items():
> + if not pname.startswith('fit,'):
> + fsw.property(pname, prop.bytes)
> + elif pname == 'fit,load':
> + fsw.property_u32('load', start)
> + elif pname == 'fit,entry':
> + if not seq:

I'd prefer seq == 0 as it's a number.

> + fsw.property_u32('entry', entry)
> + elif pname == 'fit,data':
> + fsw.property('data', bytes(data))
> + elif pname != 'fit,operation':
> + self._raise(base_node, node,
> + f"Unknown directive '{pname}'")
> +
> + def _gen_node(base_node, node, depth, in_images, entry):
> """Generate nodes from a template
>
> This creates one node for each member of self._fdts using the
> …