Message ID | YjkIr8wmz1XEOVNh@makrotopia.org |
---|---|
State | Superseded |
Delegated to: | Tom Rini |
Headers | show |
Series | image-fdt: save name of FIT configuration in '/chosen' node | expand |
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. > > Signed-off-by: Daniel Golle <daniel@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? Best regards Heinrich > /* Update ethernet nodes */ > fdt_fixup_ethernet(blob); > #if CONFIG_IS_ENABLED(CMD_PSTORE)
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@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. Cheers Daniel
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@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
On Tue, Mar 22, 2022 at 10:09:38PM +0100, Heinrich Schuchardt wrote: > 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@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? No, it is part of the FIT image which lives on a block device (which is typically either a partition on a mmc block device, a ubiblock device or a mtdblock device, depending on the storage type used). An additional partition parser (metioned above) then breaks out the configured filesystem parts into sub-partitions in a generic way, so the kernel can mount the rootfs (and later on maybe other additional filesystems e.g. with firmware localization or customization). > Should U-Boot check a signature of the squashfs? Yes, as failing to mount it will result in an unusable system, U-Boot should validate the rootfs before launching the kernel (and dual-boot into a recovery system in case anything in the FIT image got damaged). > Should U-Boot already load it into RAM? No, that's unneeded. The rootfs can be large and serving the compressed image from RAM would result in permanently allocating the space needed for that. (think: total FIT size ~40MiB, total RAM 256MiB; 40MiB additional allocation is about 15% of all the RAM there is, and that's too much) > Why can't you pack the squashfs into an initrd.img file and do without > any modification? Because this is not about systems running off a ramdisk or even having an initrd at all. But that also doesn't matter too much at this point: Even if we would solve selecting the (large) rootfs sub-image with a (small) initrd userspace process, also that userspace process would need to know which boot configuration has been selected, so it could select the configured 'filesystem' sub-image from FIT (which is stored on some permanent storage like raw NOR or NAND, or MMC). Thank you for reviewing! Cheers Daniel
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); + /* Update ethernet nodes */ fdt_fixup_ethernet(blob); #if CONFIG_IS_ENABLED(CMD_PSTORE)
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. Signed-off-by: Daniel Golle <daniel@makrotopia.org> --- boot/image-fdt.c | 6 ++++++ 1 file changed, 6 insertions(+)