[PATCH v2 5/6] binman: Add support for prepending loadables with split-elf

Simon Glass sjg at chromium.org
Fri Jan 20 20:19:20 CET 2023


Hi Jonas,

On Fri, 20 Jan 2023 at 01:26, Jonas Karlman <jonas at kwiboo.se> wrote:
>
> In some cases it is desired for SPL to start TF-A instead of U-Boot
> proper. Add support to prepend a list of strings to the loadables list
> generated by the split-elf generator.
>
> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> ---
> v2:
> - New patch
>
>  tools/binman/entries.rst                |  5 +-
>  tools/binman/etype/fit.py               | 13 +++-
>  tools/binman/ftest.py                   | 37 +++++++++++
>  tools/binman/test/276_fit_loadables.dts | 87 +++++++++++++++++++++++++
>  4 files changed, 137 insertions(+), 5 deletions(-)
>  create mode 100644 tools/binman/test/276_fit_loadables.dts
>
> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
> index 78f95dae1a..0ffffd60f2 100644
> --- a/tools/binman/entries.rst
> +++ b/tools/binman/entries.rst
> @@ -724,6 +724,7 @@ split-elf
>      fit,loadables
>          Generates a `loadable = <...>` property with a list of the generated
>          nodes (including all nodes if this operation is used multiple times)
> +        Optional property value is prepended to the generated list value
>
>
>  Here is an example showing ATF, TEE and a device tree all combined::
> @@ -791,8 +792,8 @@ Here is an example showing ATF, TEE and a device tree all combined::
>              @config-SEQ {
>                  description = "conf-NAME.dtb";
>                  fdt = "fdt-SEQ";
> -                firmware = "u-boot";
> -                fit,loadables;
> +                firmware = "atf-1";
> +                fit,loadables = "u-boot";
>              };
>          };
>      };
> diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
> index bcb606f3f9..3c90b50b7e 100644
> --- a/tools/binman/etype/fit.py
> +++ b/tools/binman/etype/fit.py
> @@ -190,6 +190,7 @@ class Entry_fit(Entry_section):
>          fit,loadables
>              Generates a `loadable = <...>` property with a list of the generated
>              nodes (including all nodes if this operation is used multiple times)
> +            Optional property value is prepended to the generated list value
>
>
>      Here is an example showing ATF, TEE and a device tree all combined::
> @@ -257,8 +258,8 @@ class Entry_fit(Entry_section):
>                  @config-SEQ {
>                      description = "conf-NAME.dtb";
>                      fdt = "fdt-SEQ";
> -                    firmware = "u-boot";
> -                    fit,loadables;
> +                    firmware = "atf-1";
> +                    fit,loadables = "u-boot";
>                  };
>              };
>          };
> @@ -533,7 +534,13 @@ class Entry_fit(Entry_section):
>                      with fsw.add_node(node_name):
>                          for pname, prop in node.props.items():
>                              if pname == 'fit,loadables':
> -                                val = '\0'.join(self._loadables) + '\0'
> +                                if type(prop.value) is str:
> +                                    val = [prop.value]
> +                                elif type(prop.value) is list:
> +                                    val = prop.value
> +                                else:
> +                                    val = []
> +                                val = '\0'.join(val + self._loadables) + '\0'
>                                  fsw.property('loadables', val.encode('utf-8'))
>                              elif pname == 'fit,operation':
>                                  pass
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index cd27572571..053ae99bee 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -6337,6 +6337,43 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
>          }
>          self.assertEqual(expected, props)
>
> +    def testFitLoadables(self):
> +        """Test an image with an FIT with prepended loadables"""
> +        if not elf.ELF_TOOLS:
> +            self.skipTest('Python elftools not available')
> +        entry_args = {
> +            'of-list': 'test-fdt1',
> +            'default-dt': 'test-fdt1',
> +            'atf-bl31-path': 'bl31.elf',
> +            'tee-os-path': 'tee.elf',
> +        }
> +        test_subdir = os.path.join(self._indir, TEST_FDT_SUBDIR)
> +        data = self._DoReadFileDtb(
> +            '276_fit_loadables.dts',
> +            entry_args=entry_args,
> +            extra_indirs=[test_subdir])[0]
> +
> +        dtb = fdt.Fdt.FromData(data)
> +        dtb.Scan()
> +
> +        node = dtb.GetNode('/configurations/conf-uboot-1')
> +        self.assertEqual('u-boot', node.props['firmware'].value)
> +        self.assertEqual(
> +            ['atf-1', 'atf-2'],
> +            fdt_util.GetStringList(node, 'loadables'))
> +
> +        node = dtb.GetNode('/configurations/conf-atf-1')
> +        self.assertEqual('atf-1', node.props['firmware'].value)
> +        self.assertEqual(
> +            ['u-boot', 'atf-1', 'atf-2'],
> +            fdt_util.GetStringList(node, 'loadables'))
> +
> +        node = dtb.GetNode('/configurations/conf-tee-1')
> +        self.assertEqual('atf-1', node.props['firmware'].value)
> +        self.assertEqual(
> +            ['u-boot', 'tee', 'atf-1', 'atf-2'],
> +            fdt_util.GetStringList(node, 'loadables'))
> +
>
>  if __name__ == "__main__":
>      unittest.main()
> diff --git a/tools/binman/test/276_fit_loadables.dts b/tools/binman/test/276_fit_loadables.dts
> new file mode 100644
> index 0000000000..66dbc1fdf6
> --- /dev/null
> +++ b/tools/binman/test/276_fit_loadables.dts
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/dts-v1/;
> +
> +/ {
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       binman {
> +               fit {
> +                       description = "test desc";
> +                       #address-cells = <1>;
> +                       fit,fdt-list = "of-list";
> +
> +                       images {
> +                               u-boot {
> +                                       description = "test u-boot";
> +                                       type = "standalone";
> +                                       arch = "arm64";
> +                                       os = "u-boot";
> +                                       compression = "none";
> +                                       load = <0x00000000>;
> +                                       entry = <0x00000000>;
> +
> +                                       u-boot-nodtb {
> +                                       };
> +                               };
> +
> +                               tee {
> +                                       description = "TEE";
> +                                       type = "tee";
> +                                       arch = "arm64";
> +                                       os = "tee";
> +                                       compression = "none";
> +                                       load = <0x00200000>;
> +
> +                                       tee-os {
> +                                               optional;
> +                                       };
> +                               };
> +
> +                               @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 {
> +                                       };
> +                               };
> +
> +                               @fdt-SEQ {
> +                                       description = "test fdt";
> +                                       type = "flat_dt";
> +                                       compression = "none";
> +                               };
> +                       };
> +
> +                       configurations {
> +                               default = "@conf-uboot-DEFAULT-SEQ";
> +                               @conf-uboot-SEQ {
> +                                       description = "test config";
> +                                       fdt = "fdt-SEQ";
> +                                       firmware = "u-boot";
> +                                       fit,loadables;
> +                               };
> +                               @conf-atf-SEQ {
> +                                       description = "test config";
> +                                       fdt = "fdt-SEQ";
> +                                       firmware = "atf-1";
> +                                       fit,loadables = "u-boot";
> +                               };
> +                               @conf-tee-SEQ {
> +                                       description = "test config";
> +                                       fdt = "fdt-SEQ";
> +                                       firmware = "atf-1";
> +                                       fit,loadables = "u-boot", "tee";
> +                               };
> +                       };
> +               };
> +       };
> +};
> --
> 2.39.1
>

The problem with this is that aft-1 can be missing, in which case it
is still referenced in the 'firmware' property.

I suspect we need a way to say that the firmware should be something
other than U-Boot, if it exists.

How about:

atf-SEQ {
   fit,operation = "split-elf";
   fit,firmware-next;    /* bumps 'u-boot' out of the 'firmware' spot
if atf is present */
   description = "ARM Trusted Firmware";
   type = "firmware";
}

fit,firmware = "u-boot";   /* default value for 'firmware' if no atf */
fit,loadables;   /* ends up holding 'u-boot' too, if it is spilled by atf */

I also think the 'atf-1' should not appear in 'loadables' if it is in
'firmware'.

Regards,
Simon


More information about the U-Boot mailing list