[PATCH 12/12] binman: Support simple templates
Jan Kiszka
jan.kiszka at siemens.com
Fri Jul 7 15:56:32 CEST 2023
On 07.07.23 14:42, Simon Glass wrote:
> Hi Jan,
>
> On Fri, 7 Jul 2023 at 11:05, Jan Kiszka <jan.kiszka at siemens.com> wrote:
>>
>> On 05.07.23 00:14, Simon Glass wrote:
>>> Hi Jan,
>>>
>>> On Mon, 3 Jul 2023 at 20:34, Jan Kiszka <jan.kiszka at siemens.com> wrote:
>>>>
>>>> Hi Simon,
>>>>
>>>> On 28.06.23 13:41, Simon Glass wrote:
>>>>> Collections can used to collect the contents of other entries into a
>>>>> single entry, but they result in a single entry, with the original entries
>>>>> 'left behind' in their old place.
>>>>>
>>>>> It is useful to be able to specific a set of entries ones and have it used
>>>>> in multiple images, or parts of an image.
>>>>>
>>>>> Implement this mechanism.
>>>>>
>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>>> ---
>>>>>
>>>>> tools/binman/binman.rst | 80 ++++++++++++++++++++++++
>>>>> tools/binman/control.py | 9 +++
>>>>> tools/binman/etype/section.py | 3 +-
>>>>> tools/binman/ftest.py | 8 +++
>>>>> tools/binman/test/286_entry_template.dts | 42 +++++++++++++
>>>>> 5 files changed, 141 insertions(+), 1 deletion(-)
>>>>> create mode 100644 tools/binman/test/286_entry_template.dts
>>>>>
>>>>> diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
>>>>> index a4b31fe5397b..9be979ae1c5b 100644
>>>>> --- a/tools/binman/binman.rst
>>>>> +++ b/tools/binman/binman.rst
>>>>> @@ -727,6 +727,13 @@ optional:
>>>>> Note that missing, optional blobs do not produce a non-zero exit code from
>>>>> binman, although it does show a warning about the missing external blob.
>>>>>
>>>>> +insert-template:
>>>>> + This is not strictly speaking an entry property, since it is processed early
>>>>> + in Binman before the entries are read. It is a list of phandles of nodes to
>>>>> + include in the current (target) node. For each node, its subnodes and their
>>>>> + properties are brought into the target node. See Templates_ below for
>>>>> + more information.
>>>>> +
>>>>> The attributes supported for images and sections are described below. Several
>>>>> are similar to those for entries.
>>>>>
>>>>> @@ -1172,6 +1179,79 @@ If you are having trouble figuring out what is going on, you can use
>>>>> arch/arm/dts/u-boot.dtsi ... found: "arch/arm/dts/juno-r2-u-boot.dtsi"
>>>>>
>>>>>
>>>>> +Templates
>>>>> +=========
>>>>> +
>>>>> +Sometimes multiple images need to be created which have all have a common
>>>>> +part. For example, a board may generate SPI and eMMC images which both include
>>>>> +a FIT. Since the FIT includes many entries, it is tedious to repeat them twice
>>>>> +in the image description.
>>>>> +
>>>>> +Templates provide a simple way to handle this::
>>>>> +
>>>>> + binman {
>>>>> + multiple-images;
>>>>> + common_part: template-1 {
>>>>> + fit {
>>>>> + ... lots of entries in here
>>>>> + };
>>>>> +
>>>>> + text {
>>>>> + text = "base image";
>>>>> + };
>>>>> + };
>>>>> +
>>>>> + spi-image {
>>>>> + filename = "image-spi.bin";
>>>>> + insert-template = <&fit>;
>>>>> +
>>>>> + /* things specific to SPI follow */
>>>>> + header {
>>>>> + ];
>>>>> +
>>>>> + text {
>>>>> + text = "SPI image";
>>>>> + };
>>>>> + };
>>>>> +
>>>>> + mmc-image {
>>>>> + filename = "image-mmc.bin";
>>>>> + insert-template = <&fit>;
>>>>> +
>>>>> + /* things specific to MMC follow */
>>>>> + header {
>>>>> + ];
>>>>> +
>>>>> + text {
>>>>> + text = "MMC image";
>>>>> + };
>>>>> + };
>>>>> + };
>>>>> +
>>>>> +The template node name must start with 'template', so it is not considered to be
>>>>> +an image itself.
>>>>> +
>>>>> +The mechanism is very simple. For each phandle in the 'insert-templates'
>>>>> +property, the source node is looked up. Then the subnodes of that source node
>>>>> +are copied into the target node, i.e. the one containing the `insert-template`
>>>>> +property.
>>>>> +
>>>>> +If the target node has a node with the same name as a template, its properties
>>>>> +override corresponding properties in the template. This allows the template to
>>>>> +be uses as a base, with the node providing updates to the properties as needed.
>>>>> +The overriding happens recursively.
>>>>> +
>>>>> +At present there is an unpleasant limitation on this feature: it works by
>>>>> +appending the template nodes after any existing subnodes to the target node.
>>>>> +This means that if the target node includes any subnodes, these appear in order
>>>>> +before the template node. In the above example, 'header' becomes the first
>>>>> +subnode of each image, followed by `fit` and `text`. If this is not what is
>>>>> +desired, there is no way to adjust it.
>>>>> +
>>>>> +Note: The above limitation will likely be removed in future, so that the
>>>>> +template subnodes appear before the target subnodes.
>>>>> +
>>>>> +
>>>>> Updating an ELF file
>>>>> ====================
>>>>>
>>>>> diff --git a/tools/binman/control.py b/tools/binman/control.py
>>>>> index 68597c4e7792..e7faca78e9aa 100644
>>>>> --- a/tools/binman/control.py
>>>>> +++ b/tools/binman/control.py
>>>>> @@ -22,6 +22,7 @@ from binman import bintool
>>>>> from binman import cbfs_util
>>>>> from binman import elf
>>>>> from binman import entry
>>>>> +from dtoc import fdt_util
>>>>> from u_boot_pylib import command
>>>>> from u_boot_pylib import tools
>>>>> from u_boot_pylib import tout
>>>>> @@ -478,6 +479,12 @@ def SignEntries(image_fname, input_fname, privatekey_fname, algo, entry_paths,
>>>>>
>>>>> AfterReplace(image, allow_resize=True, write_map=write_map)
>>>>>
>>>>> +def _ProcessTemplates(parent):
>>>>> + for node in parent.subnodes:
>>>>> + tmpl = fdt_util.GetPhandleList(node, 'insert-template')
>>>>> + if tmpl:
>>>>> + node.copy_subnodes_from_phandles(tmpl)
>>>>> +
>>>>> def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded):
>>>>> """Prepare the images to be processed and select the device tree
>>>>>
>>>>> @@ -520,6 +527,8 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded):
>>>>> raise ValueError("Device tree '%s' does not have a 'binman' "
>>>>> "node" % dtb_fname)
>>>>>
>>>>> + _ProcessTemplates(node)
>>>>> +
>>>>> images = _ReadImageDesc(node, use_expanded)
>>>>>
>>>>> if select_images:
>>>>> diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
>>>>> index d56cc11d1023..adac2ff7fa87 100644
>>>>> --- a/tools/binman/etype/section.py
>>>>> +++ b/tools/binman/etype/section.py
>>>>> @@ -179,7 +179,8 @@ class Entry_section(Entry):
>>>>> Returns:
>>>>> bool: True if the node is a special one, else False
>>>>> """
>>>>> - return node.name.startswith('hash') or node.name.startswith('signature')
>>>>> + start_list = ('hash', 'signature', 'template')
>>>>> + return any(node.name.startswith(name) for name in start_list)
>>>>>
>>>>> def ReadNode(self):
>>>>> """Read properties from the section node"""
>>>>> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
>>>>> index 4db54c69682c..9d9e47ce26b0 100644
>>>>> --- a/tools/binman/ftest.py
>>>>> +++ b/tools/binman/ftest.py
>>>>> @@ -6763,6 +6763,14 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
>>>>> data = self._DoReadFileDtb('285_spl_expand.dts',
>>>>> use_expanded=True, entry_args=entry_args)[0]
>>>>>
>>>>> + def testEntryTemplate(self):
>>>>> + """Test using a template"""
>>>>> + TestFunctional._MakeInputFile('vga2.bin', b'#' + VGA_DATA)
>>>>> + data = self._DoReadFile('286_entry_template.dts')
>>>>> + first = U_BOOT_DTB_DATA + U_BOOT_DATA + VGA_DATA
>>>>> + second = U_BOOT_DTB_DATA + b'#' + VGA_DATA + U_BOOT_DATA
>>>>> + self.assertEqual(U_BOOT_IMG_DATA + first + second, data)
>>>>> +
>>>>>
>>>>> if __name__ == "__main__":
>>>>> unittest.main()
>>>>> diff --git a/tools/binman/test/286_entry_template.dts b/tools/binman/test/286_entry_template.dts
>>>>> new file mode 100644
>>>>> index 000000000000..6980dbfafcc6
>>>>> --- /dev/null
>>>>> +++ b/tools/binman/test/286_entry_template.dts
>>>>> @@ -0,0 +1,42 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>>> +
>>>>> +/dts-v1/;
>>>>> +
>>>>> +/ {
>>>>> + #address-cells = <1>;
>>>>> + #size-cells = <1>;
>>>>> +
>>>>> + binman {
>>>>> + u-boot-img {
>>>>> + };
>>>>> +
>>>>> + common_part: template {
>>>>> + u-boot {
>>>>> + };
>>>>> +
>>>>> + intel-vga {
>>>>> + filename = "vga.bin";
>>>>> + };
>>>>> + };
>>>>> +
>>>>> + first {
>>>>> + type = "section";
>>>>> + insert-template = <&common_part>;
>>>>> +
>>>>> + u-boot-dtb {
>>>>> + };
>>>>> + };
>>>>> +
>>>>> + second {
>>>>> + type = "section";
>>>>> + insert-template = <&common_part>;
>>>>> +
>>>>> + u-boot-dtb {
>>>>> + };
>>>>> +
>>>>> + intel-vga {
>>>>> + filename = "vga2.bin";
>>>>> + };
>>>>> + };
>>>>> + };
>>>>> +};
>>>>
>>>> I tried to apply this on the IOT2050 use case, but it failed in two cases:
>>>>
>>>>
>>>> 1. We need a template where nodes can be manipulated afterwards, thus we
>>>> need anchors into those nodes. Naturally, those anchors need to get
>>>> unique names so that something like this is possible:
>>>>
>>>> common: template {
>>>> anchor: some-node {
>>>> value-1 = <1>;
>>>> };
>>>> };
>>>>
>>>> user-a {
>>>> insert-template = <&common>;
>>>> };
>>>>
>>>> user-b {
>>>> insert-template = <&common>;
>>>> };
>>>>
>>>> &anchor-a {
>>>> value-2 = <2>;
>>>> };
>>>>
>>>> &anchor-b {
>>>> value-2 = <3>;
>>>> };
>>>
>>> Rather than using a phandle, can you not use the node name in the
>>> template? For example:
>>>
>>> user-a {
>>> insert-template = <&common>;
>>> some-node {
>>> value2 = <2>;
>>> }
>>>
>>> user-b {
>>> insert-template = <&common>;
>>> some-node {
>>> value2 = <3>;
>>> }
>>>
>>
>> I don't think this is working already. This is what I tried:
>>
>> /dts-v1/;
>> / {
>> binman: binman {
>> multiple-images;
>>
>> my_template: template {
>> blob-ext at 0 {
>> filename = "my-blob.bin";
>> offset = <0>;
>> };
>> blob-ext at 100 {
>> filename = "/dev/null";
>> offset = <0x100>;
>> };
>> };
>>
>> my-image {
>> filename = "my-image.bin";
>> insert-template = <&my_template>;
>> blob-ext at 100 {
>> filename = "my-blob2.bin";
>> };
>> };
>> };
>> };
>>
>> First, I had to apply that null filename workaround, and the overridden
>> filename is taken for the final image (if I leave out blob-ext at 0), but
>> this is at least ugly.
>>
>> However, the file above as-is does not build:
>>
>> binman: Node '/binman/my-image/blob-ext at 0': Offset 0x0 (0) overlaps with
>> previous entry '/binman/my-image/blob-ext at 100' ending at 0x107 (263)
>>
>>
>> Probably for the same reason, leaving fit,fdt-list-val out in a template
>> also generates an error during build. Putting an empty string there and
>> overriding it later does not work.
>
> I think it is just that it doesn't support templating of
> multiple-images nodes property. I sent a patch for this [1]
>
> There may be other cases too, but if you have a failure, please see if
> you can adjust that test (287) to fail for you.
>
Hmm, already the test pattern from [1] still gives me
binman: Node '/binman/image/blob-ext at 0': Offset 0x0 (0) overlaps with
previous entry '/binman/image/blob-ext at 8' ending at 0xf (15)
I've applied this series and then [1] on top.
Jan
> Regards,
> Simon
>
> [1] https://patchwork.ozlabs.org/project/uboot/patch/20230707124024.1668724-1-sjg@chromium.org/
--
Siemens AG, Technology
Competence Center Embedded Linux
More information about the U-Boot
mailing list