[PATCH v2 5/6] binman: Add support for prepending loadables with split-elf
Jonas Karlman
jonas at kwiboo.se
Fri Jan 20 21:46:59 CET 2023
Hi Simon,
On 2023-01-20 20:19, Simon Glass wrote:
> 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 */
This looks reasonable, I do however wonder if it will be more flexible
if we can skip the fit,firmware-next prop altogether and just handle it
with a fit,firmware prop alone, if we treat it like a string list.
fit,firmware: List of possible entries, the first existing entry is used
for the 'firmware' property.
fit,loadables: Adds 'loadables' property with a list of all remaining existing
entries in 'fit,firmware' and remaining generated loadables.
That way we do not create a dependency between the images and configurations
and make it possible to generate configs with different 'firmware' like in
the test dts.
Example for known entries 'atf-1', 'atf-2' and 'u-boot':
fit,firmware = "u-boot"; firmware = "u-boot";
fit,loadables; loadables = "atf-1", "atf-2";
fit,firmware = "atf-1", "u-boot"; firmware = "atf-1";
fit,loadables; loadables = "u-boot", "atf-2";
fit,firmware = "fw-1", "u-boot"; firmware = "u-boot";
fit,loadables; loadables = "atf-1", "atf-2";
Regards,
Jonas
>
> 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