[PATCH] image-fdt: save name of FIT configuration in '/chosen' node

[Heinrich Schuchardt]

On 3/22/22 14:18, Daniel Golle wrote:
> Hi Heinrich,
>
> On Tue, Mar 22, 2022 at 09:29:54AM +0100, Heinrich Schuchardt wrote:
>> On 3/22/22 00:22, Daniel Golle wrote:
>>> It can be useful for the OS (Linux) to know which configuration has
>>> been chosen by U-Boot when launching a FIT image.
>>> Store the name of the FIT configuration node used in a new string
>>> attribute called 'bootconf' in the '/chosen' node in device tree.
>>
>> Please, point out where you want to use that information. I cannot see
>> that the Linux kernel will make any use of it.
>
> It can be useful when parsing FIT image in kernel, but can also be
> useful in userspace (via /sys/firmware/devicetree/base/chosen/bootconf).
> For OpenWrt I've written a uImage.FIT partition parser in order to
> allow a single type of firmware image containing kernel, dtb and rootfs
> to work accross all storage types (SPI-NOR/mtd, NAND/ubi, eMMC/block).
>
> I haven't yet submitted it upstream, exactly because some edges like
> selection of configuration would need to be discussed first.
>
> https://git.openwrt.org/?p=openwrt/openwrt.git;a=blob;f=target/linux/generic/files/block/partitions/fit.c;h=89b5fb3454f0b69cdcfe53958169fb2a6f9c48a9;hb=HEAD
>
> However, it's doing the job for a bunch of OpenWrt boards already,
> incl. some popular devices
>
> https://openwrt.org/toh/linksys/e8450
> (boots from SPI-NAND/ubi)
>
> https://openwrt.org/toh/sinovoip/bananapi_bpi-r64_v1.1
> (can boot from SD Card, eMMC or SPI-NAND/ubi)
>
> Using uImage.FIT to store the squashfs besides kernel and dtb has
> several obvious advantages which are hard to obtain in any other way:
>   * single image accross NOR/mtd, NAND/ubi and MMC/block devices
>   * dynamically sized sub-partitions for kernel and rootfs
>   * hash also for rootfs checked by U-Boot before launching kernel
>   * images may include additional filesystems e.g. for localization or
>     brandinge (hence the need to parse the boot configuration when
>     mapping the 'filesystem' subimages into block partitions).
>
>>
>>>
>>> Signed-off-by: Daniel Golle <daniel at makrotopia.org>
>>> ---
>>>    boot/image-fdt.c | 6 ++++++
>>>    1 file changed, 6 insertions(+)
>>>
>>> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
>>> index 692a9ad3e4..4017bc94a6 100644
>>> --- a/boot/image-fdt.c
>>> +++ b/boot/image-fdt.c
>>> @@ -601,6 +601,12 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob,
>>>    		goto err;
>>>    	}
>>>
>>> +	/* Store name of configuration node as bootconf in /chosen node */
>>> +	if (images->fit_uname_cfg)
>>> +		fdt_find_and_setprop(blob, "/chosen", "bootconf",
>>> +					images->fit_uname_cfg,
>>> +					strlen(images->fit_uname_cfg) + 1, 1);
>>> +
>>
>> We should not inject arbitrary properties into the device-tree. Where is
>> the property /chosen/bootconf defined in the Linux documentation or in
>> the device-tree specification?
>
>>From Linux Documenation (chosen.txt):
> "The chosen node does not represent a real device, but serves as a place
>   for passing data between firmware and the operating system, like boot
>   arguments."
>
>>From Device Tree specification (Release v0.3):
> "The /chosen node does not represent a real device in the system but
>   describes parameters chosen or specified by the system firmware at run
>   time."
>
> So to me it sounds like the right place for an additional attribute
> serving the above purpose. It could as well become an additional kernel
> cmdline argument in (ie. appended to 'bootargs' in '/chosen'), but that
> seemed more complex and harder to deal with.
>
> The name could be changed into something like 'denx,boot-configuration'
> if 'bootconf' is perceived to be too generic.

Is this squashfs meant to be served from RAM?
Should U-Boot check a signature of the squashfs?
Should U-Boot already load it into RAM?

Why can't you pack the squashfs into an initrd.img file and do without
any modification?

Best regards

Heinrich

>
> Cheers
>
> Daniel