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

[Simon Glass <sjg@chromium.org>]

Hi Alper,

On Thu, 3 Mar 2022 at 14:17, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
>
> 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;
>   };

Yes that needs fixing, but I'll leave that case for now. It needs its own test.

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

I wish we could get the kevin patches applied. What on earth is going
on there? I haven't been able to test any of this on kevin.



[..]

Regards,
Simon