block: uImage.FIT filesystem image mapper

uImage.FIT is the default image format of Das U-Boot bootloader which is used on the great majority of embedded Linux devices.

Using uImage.FIT to also store the root filesystem besides kernel and dtb has several obvious advantages which are hard to obtain in any other way:
* single image accross different storage types (mmcblk, mtdblock, ubiblock)
* dynamically sized 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 branding

Add uImage.FIT partition parser, this time implemented as a tiny block driver which can be built as a module. Being an early RFC, module unloading by now has only been implemented rudimentary without removing partitions added by the driver on unload. If the overall approach looks acceptable, this can of course be improved.

For this to work, the image has to be created with external data and sub-images aligned to the system's memory page boundaries, ie.
mkimage -E -B 0x1000 -p 0x1000 ...

Booting such images has been supported by Das U-Boot since v2018.01.

A previous version of this partition parser is in production use on some OpenWrt devices, eg. the BananaPi R64 where using the FIT parser allows booting the very same image from eMMC, SD Card or SPI-NAND/UBI and also using it as a firmware-upgrade image at the same time.
The Ubiquiti UniFi 6 LR access point served as a reference board with SPI-NOR flash and use of the partition parser on top of a mtdblock device.

As Das U-Boot by now also passes down the selected configuration node name via device tree this allows the partition parser (or userspace process via sysfs) to identify the selected image configuration.

Device Tree schema for that has also been merged already[1].

In most cases this partition parser can be used without relying on the bootloader to pass-down the configuration node name. The default configuration node is used then in that case.

A check of the chosen/u-boot,version node is used to make sure that Das U-Boot has been used to boot the system and the user (or the bootloader) is required to specify the lower block device to be used by the parser. In case any of the two is missing, this driver won't start.

This RFC presents an alternative implementation of the previously posted patch series "partition parser for U-Boot's uImage.FIT"[1].

[1]: https://github.com/devicetree-org/dt-schema/commit/a24d97d43491e55d4def006213213a6c4045b646
[2]: https://patchwork.kernel.org/project/linux-block/list/?series=695709

Daniel Golle (4):
init: move block device helpers from init/do_mounts.c
block: add new flag to add partitions read-only
blkdev: add function to add named read-only partitions
block: add uImage.FIT block partition driver

 MAINTAINERS             |   6 +
 block/bdev.c            | 166 ++++++++++++++++++
 block/blk.h             |   1 +
 block/partitions/core.c |  37 ++++
 drivers/block/Kconfig   |   6 +
 drivers/block/Makefile  |   2 +
 drivers/block/fitblk.c  | 364 ++++++++++++++++++++++++++++++++++++++++
 include/linux/blkdev.h  |  22 +++
 init/do_mounts.c        | 166 +-----------------
 9 files changed, 606 insertions(+), 164 deletions(-)
 create mode 100644 drivers/block/fitblk.c

2.38.1

In init/do_mounts.c there are helper functions devt_from_partuuid, devt_from_partlabel and devt_from_devname. In order to make these functions available to other users, move them to block/bdev.c.

---
 block/bdev.c           | 166 +++++++++++++++++++++++++++++++++++++++++
 include/linux/blkdev.h |  15 ++++
 init/do_mounts.c       | 166 +----------------------------------------
 3 files changed, 183 insertions(+), 164 deletions(-)

index d699ecdb3260..92e3e5c81337 100644
@@ -1092,3 +1092,169 @@ void bdev_statx_dioalign(struct inode *inode, struct kstat *stat)
 
 	blkdev_put_no_open(bdev);
 }
+
+struct uuidcmp {
+	const char *uuid;
+	int len;
+};
+
+/**
+ * match_dev_by_uuid - callback for finding a partition using its uuid
+ * @dev:	device passed in by the caller
+ * @data:	opaque pointer to the desired struct uuidcmp to match
+ *
+ * Returns 1 if the device matches, and 0 otherwise.
+ */
+static int match_dev_by_uuid(struct device *dev, const void *data)
+{
+	struct block_device *bdev = dev_to_bdev(dev);
+	const struct uuidcmp *cmp = data;
+
+	if (!bdev->bd_meta_info ||
+	    strncasecmp(cmp->uuid, bdev->bd_meta_info->uuid, cmp->len))
+		return 0;
+	return 1;
+}
+
+/**
+ * devt_from_partuuid - looks up the dev_t of a partition by its UUID
+ * @uuid_str:	char array containing ascii UUID
+ *
+ * The function will return the first partition which contains a matching
+ * UUID value in its partition_meta_info struct.  This does not search
+ * by filesystem UUIDs.
+ *
+ * If @uuid_str is followed by a "/PARTNROFF=%d", then the number will be
+ * extracted and used as an offset from the partition identified by the UUID.
+ *
+ * Returns the matching dev_t on success or 0 on failure.
+ */
+dev_t devt_from_partuuid(const char *uuid_str, int *root_wait)
+{
+	struct uuidcmp cmp;
+	struct device *dev = NULL;
+	dev_t devt = 0;
+	int offset = 0;
+	char *slash;
+
+	cmp.uuid = uuid_str;
+
+	slash = strchr(uuid_str, '/');
+	/* Check for optional partition number offset attributes. */
+	if (slash) {
+		char c = 0;
+
+		/* Explicitly fail on poor PARTUUID syntax. */
+		if (sscanf(slash + 1, "PARTNROFF=%d%c", &offset, &c) != 1)
+			goto clear_root_wait;
+		cmp.len = slash - uuid_str;
+	} else {
+		cmp.len = strlen(uuid_str);
+	}
+
+	if (!cmp.len)
+		goto clear_root_wait;
+
+	dev = class_find_device(&block_class, NULL, &cmp, &match_dev_by_uuid);
+	if (!dev)
+		return 0;
+
+	if (offset) {
+		/*
+		 * Attempt to find the requested partition by adding an offset
+		 * to the partition number found by UUID.
+		 */
+		devt = part_devt(dev_to_disk(dev),
+				 dev_to_bdev(dev)->bd_partno + offset);
+	} else {
+		devt = dev->devt;
+	}
+
+	put_device(dev);
+	return devt;
+
+clear_root_wait:
+	pr_err("VFS: PARTUUID= is invalid.\n"
+	       "Expected PARTUUID=<valid-uuid-id>[/PARTNROFF=%%d]\n");
+	if (root_wait && *root_wait) {
+		pr_err("Disabling rootwait; root= is invalid.\n");
+		*root_wait = 0;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(devt_from_partuuid);
+
+/**
+ * match_dev_by_label - callback for finding a partition using its label
+ * @dev:	device passed in by the caller
+ * @data:	opaque pointer to the label to match
+ *
+ * Returns 1 if the device matches, and 0 otherwise.
+ */
+static int match_dev_by_label(struct device *dev, const void *data)
+{
+	struct block_device *bdev = dev_to_bdev(dev);
+	const char *label = data;
+
+	if (!bdev->bd_meta_info || strcmp(label, bdev->bd_meta_info->volname))
+		return 0;
+	return 1;
+}
+
+dev_t devt_from_partlabel(const char *label)
+{
+	struct device *dev;
+	dev_t devt = 0;
+
+	dev = class_find_device(&block_class, NULL, label, &match_dev_by_label);
+	if (dev) {
+		devt = dev->devt;
+		put_device(dev);
+	}
+
+	return devt;
+}
+EXPORT_SYMBOL_GPL(devt_from_partlabel);
+
+dev_t devt_from_devname(const char *name)
+{
+	dev_t devt = 0;
+	int part;
+	char s[32];
+	char *p;
+
+	if (strlen(name) > 31)
+		return 0;
+	strcpy(s, name);
+	for (p = s; *p; p++) {
+		if (*p == '/')
+			*p = '!';
+	}
+
+	devt = blk_lookup_devt(s, 0);
+	if (devt)
+		return devt;
+
+	/*
+	 * Try non-existent, but valid partition, which may only exist after
+	 * opening the device, like partitioned md devices.
+	 */
+	while (p > s && isdigit(p[-1]))
+		p--;
+	if (p == s || !*p || *p == '0')
+		return 0;
+
+	/* try disk name without <part number> */
+	part = simple_strtoul(p, NULL, 10);
+	*p = '\0';
+	devt = blk_lookup_devt(s, part);
+	if (devt)
+		return devt;
+
+	/* try disk name without p<part number> */
+	if (p < s + 2 || !isdigit(p[-2]) || p[-1] != 'p')
+		return 0;
+	p[-1] = '\0';
+	return blk_lookup_devt(s, part);
+}
+EXPORT_SYMBOL_GPL(devt_from_devname);

index fb27dfaaaf85..b45cdcdccc6d 100644
@@ -1494,6 +1494,9 @@ int truncate_bdev_range(struct block_device *bdev, fmode_t mode, loff_t lstart,
 		loff_t lend);
 
 #ifdef CONFIG_BLOCK
+dev_t devt_from_partuuid(const char *uuid_str, int *root_wait);
+dev_t devt_from_partlabel(const char *label);
+dev_t devt_from_devname(const char *name);
 void invalidate_bdev(struct block_device *bdev);
 int sync_blockdev(struct block_device *bdev);
 int sync_blockdev_range(struct block_device *bdev, loff_t lstart, loff_t lend);

@@ -1502,6 +1505,18 @@ void sync_bdevs(bool wait);
 void bdev_statx_dioalign(struct inode *inode, struct kstat *stat);
 void printk_all_partitions(void);
 #else
+static inline dev_t devt_from_partuuid(const char *uuid_str, int *root_wait)
+{
+	return MKDEV(0, 0);
+}
+static inline dev_t devt_from_partlabel(const char *label);
+{
+	return MKDEV(0, 0);
+}
+static inline dev_t devt_from_devname(const char *name);
+{
+	return MKDEV(0, 0);
+}
 static inline void invalidate_bdev(struct block_device *bdev)
 {
 }

2.38.1

Yes, they should be moved and I have an old patch to do this a bit smarter. But that doesn't mean anyone else should call them.

Hi Christoph,

I have followed your advise and implemented a "real" block driver stacking on top of an existing block device to expose uImage.FIT filesystem subimages as block devices. It mangles any bio submitted to it and re-submits the bio with changed bi_bdev and sector offset added to bi_iter.bi_sector for all list elements.

That works, but has slightly less utility value than the partition parser approach as in this way I cannot easily populate the PARTNAME uevent which can later help userspace to identify a device by the FIT subimage name -- I'd have to either invent a new bus_type or device_type, both seem inappropriate and have unwanted side effects. Or am I missing something and there is a way to use add_uevent_var() for a disk_type device?

In exchange, instead of using the PARTNAME= uevent var, I can now freely name the device node by setting the disk_name string in struct gendisk. While this doesn't offer the same amount of freedom, it is sufficient for our use-case.

However, I don't see a way to avoid using (or duplicating) devt_from_devname() to select the lower device somehow without having to probe and parse *all* block devices present (which is, from what I understood, what you want to avoid, and I agree that it is more safe to not do that...)

Can you or anyone give some advise on how this should be done?

As block partitions are not present in device tree, using a cmdline argument (with similar semantics to 'root=') specifying the lower block device seems unavoidable.

Also note that due to libfdt not exporting symbols, the driver currently cannot be built as a module. Hence, I would either need to export most of libfdt API or only allow the driver to be built-into the kernel. I don't see a problem with having it always built-in.

Yet another (imho not terrible) problem is removal of the lower device. Many of the supported SBC use a micro SD card to boot, which can be removed by the user while the system is running (which is generally not a good idea, but anyway). For partitions this is handled automatically by blk_drop_partitions() called directly from genhd.c.
I'm currently playing with doing something similar using the bus device removal notification, but it doesn't seem to work for all cases, e.g. mmcblk device do not seem to have the ->bus pointer populated at all (ie. disk_to_dev(disk)->bus == NULL for mmcblk devices).

Any ideas regarding that?

Thank you for your advise!

Daniel

You're not exposing a partition here - this is an image format that sits in a partition and we should not pretend that is a partition.

Just set the block driver up from an initramfs, like we do for all modern stackable drivers.

I have WIP patches that allow the claimer of a block device get resize and removal notification. It's not going to land for 6.2, but I hope I have it ready in time for the next merge window.

Hi Christoph,

It doesn't need to be literally the PARTNAME uevent, just any way to communicate the names of mapped subimages to userspace.

My understanding by now is that there is no way around introducing a new device_type and then mitigate the unwanted side effects by follow-up changes, ie. make it possible to use that new device_type when specifying the rootfs= cmdline variable (currently only disks and partitions are considered there).

Or give up on the idea that uImage.FIT subimages mapped by the new driver can be identified by userspace by poking uevent from sysfs and just rely on convention and ordering.

Instead of using a kernel cmdline parameter we could also have the bootloader embed that information as string in the 'chosen' section in the device tree blob, right next to the cmdline. However, as there is no representation of block partitions in device tree, also in that case the lower device will have to be referenced by a string somehow, ie. devt_from_devname() or the like will be needed.

Needing an initramfs, even if it boils down to just one statically compile executable, is a massive bloat and complication when building embedded device firmware and none of the over 1580 devices currently supported by OpenWrt need an intermediate initramfs to mount their on-flash squashfs rootfs (some, however, already use this uImage.FIT partition parser, and not needing a downstream patch for that would be nice).

uImage.FIT typically contains the complete firmware used on an embedded device, ie. at least a Linux kernel, device tree blob and a filesystem. The main use of this whole uImage.FIT-parsing-in-Linux approach I'm trying to get across here is to expose one or more 'filesystem'-type subimages of such an image as block devices, also so that one of them can directly be mounted as rootfs by the kernel.

This *replaces* the use of 'ramdisk' type sub-images which need to remain allocated at runtime, while using a squashfs 'filesystem' type sub-image allows freeing the filesystem cache if ram is becomes scarce.

As both, storage and memory, are often very limited on small embedded devices, OpenWrt has always been using a squashfs as rootfs with a storage-type specific filesytem used as r/w overlay on top. Up to now, the rootfs is often stored in platform-specific ways, ie. an additional partition on block devices, MTD partition on NOR flash or UBI volume on NAND flash.

Carrying the read-only squashfs filesystem inside the uImage.FIT structure has the advantage of being agnostic regarding the storage-type (NOR/mtdblockX, NAND/ubiblockX, MMC/mmcblkXpY) and allows the bootloader to validate the filesystem hash before starting the kernel, ie. ensuring integrity of the firmware as-a-whole which includes the root filesystem.

I'm looking forward to integrate that in the uImage.FIT block driver I've been working on. In the meantime, should I already post my current draft so we can start discussing if that solution could be acceptable?

Best regards

Daniel

To be honest I'm still really confused by the patch set. What is the problem with simply using an initramfs, which is the way to deal with any non-trivial init issue for about 20 years now, predated by the somewhat more awkware initrd...

You can always build the initrams into the kernel image, I don't see how that is bloat.

What is the point of the uImage.FIT? Why can't you use a sane format? Or at least not use it on top of partitions, which is just braindead. Who actually came up with this amazingly convoluted and annoying scheme and why?

The thing is that there isn't anything extraordinarily complex here, just dynamically partitioning a block device based on a well-known format.

Using initramfs implies that we would need a 2nd copy of the standard C library and libfdt, both alone will already occupy more than just a single 64kB block of flash. I understand that from the point of view of classic x86 servers or even embedded devices with eMMC this seems negligible. However, keep in mind that a huge number of existing devices and also new designs of embedded devices often boot from just a few megabytes of NOR flash, and there every byte counts.

It is the format used by Das U-Boot, which is by far the most common bootloader found on small embedded devices running Linux. Is is already used by Das U-Boot to validate and load kernel, devicetree, initramfs, ... to RAM before launching Linux. I've included a link to the documentation[1] which gives insights regarding the motivation to create such a format.

Define "sane format". :) And tell that to the people over at Das U-Boot please. I'm just trying to make best use of the status-quo which is that uImage.FIT is the de-facto standard to boot Linux on small embedded devices.

In fact, that's only one out of three possible uses in which parsing the contained sub-image boundaries can be useful:
* On devices with NOR flash uImage.FIT is stored in an MTD partition, hence the uImage.FIT partition parser (or small stackable block driver) would then operate on top of /dev/mtdblockX.

* On devices with NAND flash uImage.FIT is stored in a UBI volume, hence in this case /dev/ubiblockX needs to be processed.

* On devices with block storage (like eMMC or SD card) the image is stored on a (GPT) partition (/dev/mmcblkXpY).

Generally speaking, when it comes to storing the rootfs (typically squashfs), this can be solved in two ways:
a) keep the rootfs inside the uImage.FIT structure (to be then dealt with by this patch series)

b) store the rootfs on an additional partition or volume:
- additional GPT partition on block devices
- additional MTD partition on devices with NOR flash
- additional UBI volume on devices with NAND flash

Now lets look at the consequences of each approach:

I.
It is often a requirement that the bootloader shall decide about the integrity of the firmware before starting it (ie. verify hashes of kernel, dtb, **and rootfs**). In case of approch b) this will have to be implemented in a custom, platform-specific way; many vendors do things like this, resulting in huge variety of different, customs ways which let the bootloader validate the rootfs. As a huge majority of devices actually use Das U-Boot which already supports validating uImage.FIT content, this becomes a no-brainer when using approach a).

II.
Updating the firmware in case of approach a) being used is very simple:
Just write the whole image to a storage volume. In case of approach b) an archive or tarball has to be used, allowing kernel.uImage and rootfs to each be written to different storage volumes. And for the special case of NOR flash, developers came up with a complete out-of-tree subsystem handling only the splitting of MTD partitions in the various ways used by vendor-modified bootloaders [2].

III.
Approach a) allows the very same uImage.FIT binary to be read by the bootloader, used as firmware-upgrade format within Linux userland and also serve as a recovery image format which can be loaded e.g. via TFTP and stored by the bootloader. Approach b) will require different formats for firmware upgrades (eg. a tarball) and storage on-disk, depending on the platform and type of storage media. And when it comes to bootloader-level recovery via TFTP, one would have to implement extraction of that tarball **by the bootloader** or use yet another format.

IV.
When it comes to devices allowing to boot from different media, approach a) allows using the exact same firmware image independently of the storage media used for booting. When using approach b), different firmware images have to be provided **for the same device**, depending on whether a block device, NAND/UBI or NOR/MTD is used for booting. The BananaPi BPi-R64 and more recent BananaPi BPi-R3 boards are good examples for boards which allow booting of different media, and are already using uImage.FIT as a unified format.

I hope this explains my motivation. Please ask should there by any doubts or if any of my explainations above are not clear.

This may answer your question:
[1]: https://source.denx.de/u-boot/u-boot/-/blob/master/doc/uImage.FIT/howto.txt

[2]: https://git.openwrt.org/?p=openwrt/openwrt.git;a=tree;f=target/linux/generic/files/drivers/mtd/mtdsplit

Yes, but a completely non-standard format that nests inside an partition.

Why do you need libfdt? And with a simple statically linked kpartx you won't pull in much of libc either.

So I've worked quite a bit on really small deeply embedded systems, and for those I wouldn't even think of using strange image formats or the rather wasteful GPT partition format.
There we wouldn't dare to use paritions or weird image formats, but

That doesn't explain why you'd want to use it. Nor how people came up with it.

And all the mtdblock / ubiblock is due to the lack of a native mtd/ubi backend for squashfs? Why can't we take the block layer out of the loop entirely?

None of this explains the silly nesting inside the GPT partition. It is not needed for the any use cases and the root probem here. Without that you could simply implement a parition format, with that you get into crazy nesting behavior. Note that it would have any benefit over just not doing this silly image.

Maybe someone just needs to go back and come up wit ha scheme that actually works and implement that in uboot as well.

The reason for this current discussion (see subject line) is exactly that you didn't like the newly introduced partition type GUID which then calls the newly introduced partition parser taking care of the uImage.FIT content of a partition.
Instead you suggested to implement it as a small stackable block driver, which I did (and will post as RFC in a moment from now).

This block driver (if built-into the kernel and relied upon to expose the block device used as root filesystem) will need to identify the lower device it should work on. And for that the helper functions such as devt_from_devname() need to be available for that driver.

uImage.FIT is based on FDT, so the easiest way to parse this format without reinventing the wheel is using libfdt.

Well, enough to have kpartx and libfdt covered. I still assume that's more than 64KiB of compressed bytecode in total.

Well, the reality as of today is that even the smallest of all embedded devices will need updated firmware at some point.

Using an image format which allows to dynamically partition a firmware image into kernel and rootfs parts is needed **especially** on devices with very little storage space -- it's there that we cannot afford to use hard-coded MTD partition sizes either limiting or overestimating the future kernel size.

And having a unified format which serves a whole range of devices does make that easier -- surely, on NOR flash, there would not be a GPT partition; mtdblock on top of an MTD partition is used in that case.

I didn't want to quote all the history here, but it is explained rather well in their documentation I had linked there.

A block representation is the common denominator of all the above. Sure, I could implement splitting MTD devices according to uImage.FIT and then add MTD support to squashfs. Then implement splitting of UBI volumes and add UBI support to squashfs.

However, simply using mtdblock on NOR and ubiblock on NAND seem well-suited and hence there wasn't ever a need to implement MTD or UBI support for any **read-only** filesystem. Afaik that is what mtdblock and ubiblock are intended to be used for.

So where would you store the uImage (which will have to exist even to just load kernel and DTB in U-Boot, even without containing the root filesystem) on devices with eMMC then?

I did consider block2mtd on eMMC and gluebi (which is deprecated) on UBI, but in order to mount the filesystem I'd then use mtdblock further down the road, and that'd actually be silly.

Are you suggesting to come up with an entirely new type of partition table only for that purpose? Which will require its own tools and implementation in both, U-Boot and Linux? What would be the benefit over just using GPT partitioning?

I'm not saying that this cannot be done (and I will do it if you convince me that it'd be needed).

In terms of "works" both are working very well, the uImage.FIT partition parser as well as the tiny stackable block driver you had asked me to write instead. I've tested both on multiple devices by now, the former is even used in production on one of the currently most popular devices running OpenWrt (according to update backend stats).

Thank you for your patience!

Daniel

Which is the exact nesting I'm complaining about. Why do you need to use your format inside a GPT partition table? What you're doing is bascially nesting a partition table format inside another one, which doesn't make any sense at all.

And devt_from_devname must not be used by more non-init code. It is bad it got exposed at all, but new users are not acceptable.

Implementing MTD and/or UBI support would allow you to build a kernel without CONFIG_BLOCK, which will save you a lot more than the 64k you were whining about above.

Straight on the block device, where else?

Why do you need another layer of partitioning instead of storing all your information either in the uImage, or in some other partition format of your choice?

See, if you have GPT, DOS or whatever partitions, you just use partitions and store all the bits your care about in them.
If you want a fancy not invented here syndrome image format you use that. But don't use both.

The GPT partition table is typically written only once to an eMMC- based device in factory. Firmware images (typically uImage.FIT) are stored in partitions because there are sometimes two of them (for A/B dual-boot, or recovery/production dual-boot).

As a working device firmware consists of kernel, dtb and rootfs, all these three things have to be written and used together, typically they also come together in one file for firmware upgrade (ie. rootfs appended to kernel, tarballs, or uImage.FIT containing all three of them).

As the size of kernel and rootfs cannot be determined accurately at the time the device is made, having individual GPT partitions for kernel and rootfs ends up to either being a limit to future groth of the kernel image or wastes space by overestimating the kernel size.

Changing the GPT partitioning when updating the device to match the exact sizes is also not an option as a damage to the GPT would then present a single point of failure (backup GPT also wouldn't help much here), so for dual-boot to actually be meaningful, we shouldn't ever write to any parts of the disk/flash which affect more than one of the dual-boot options.

See the last paragraph of this message for good reasons why one would want to do exactly that.

I assume that implementing anything similar using blk_lookup_devt in the driver itself is then also not acceptable, right?

Yet another option would be to implement a way to acquire this information from device tree. Ie. have a reference to the disk device as well as an unsigned integer in the 'chosen' node which the bootloader can use to communicate this to the kernel. Example:

chosen {
	bootdev = <&mmc0 3>;
};

It's a bit more tricky for ubiblock or mtdblock devices because they don't have *any* representation in device tree at all at this point.

In case of an MTD partition (for mtdblockX) we would just reference the mtd partition in the same way.

To do this cleanly with NAND/UBI, I'd start with adding device-tree-based attaching of an MTD partition to UBI using a device-tree attribute 'compatible = "linux,ubi"' on the MTD partition. We could then have sub-nodes referencing specific UBI volumes, to select them for use with ubiblock but also for those nodes then being a valid reference for use with the to-be-introduced 'bootdev' attribute in 'chosen'.

Does that sound acceptable from your perspective?

Even devices with NOR flash may still want support for removable block devices like USB pendrives or SD cards... Many home-routers got only 8MiB of NOR flash and yet come with USB 2.0 ports intended for a pendrive which is then shared via Samba.

Also, heavily customzied per-device kernel builds would never scale up to support thousands of devices -- hence OpenWrt uses the exact same kernel build for many devices, which makes both, the build process and also debugging kernel bugs, much much easier (or even doable at all).

As the first few blocks are typically used for bootloader code and bootloader environment, we would then need to hard-code the offset(s) of the uImage.FIT on the block device. Imho this becomes messy and just using partitions seemed like a straight forward solution.

And what about dual-boot systems where you have more than one firmware image? Hard-code more offsets? For each device?

In a way, I was considering this by using blkdevparts= cmdline option instead of GPT, but

The reason is the different life-cycle of the device main partition table, bootloader, bootloader environment, ... on one hand and each firmware image on a dual boot system on the other hand. Hence there is more than just one uImage: typically bootloader, bootloader environment, firmware A (uImage.FIT) and firmware B. Relace "A" and "B" with "recovery" and "production", depending on the dual-boot style implemented.

Therefore re-writing the whole disk during firmware upgrades is not an option because it is risky, eg. in case of a sudden power failure we could end up with a hard-bricked system. So to me it makes sense that for a firmware upgrade, we write only to one partition and don't touch GPT or anything else on the device. So in case something goes wrong, the device will still boot, the bootloader will realize that the uImage.FIT in one partition is broken (uImage.FIT also comes with hashes to ensure image integrity) and it will load something else (from another partition) instead.

No. blk_lookup_devt is a workaround for the early init code where the file system is not available yet. The only reasonable use case is the early init code, but even that is better handled by an initramfs.

Device tree questions are really out of my knowledge and domain, the right people to talk to would be the device-tree and relevant driver maintainers.

Ok, so we're not _that_ resource contrained at all, because for the really small devices with just a few MB of ram, not having the block layer makes a huge difference.

Ok, so the whole DOS-World boot load nightmare has spread to these devices as well.

Is this for a fail save use case where on image is update while the other on is in use? I've usually seen people use two different NOR chips for that to have full error isolation.

Oh, so the firmware image doesn't even include the bootloader, but the bootload is on the same device? That just seems like a very odd setup, but I'll take your word for it being used.

I'm just really confused about this whole setup. Maybe it's just because what I've worked with, which is either really deeply embededd devices where the everything is one image, that you might in some case have twice, or a full blown UEFI-like setup where the boot loader is separate, but can simply load a kernel from the actual root file system.

I have to say even after all these round trip I'm still not sure what problem where really solving. In basically all other environments that have a powerful enough bootloader to read file systems your rootfs would simply store the kernel image and dtb if they are interdependent. Is the root cause that uboot doesn't support reading files from squashfs despite supporting half a dozen other file systems?

Hi Christoph,

Any news about that patchset? I'd happily review, test and use it ;)

Add flag ADDPART_FLAG_READONLY to allow partition parsers marking a partition to be set read-only.
This is needed for the uImage.FIT partition parser added by a follow-up
commit: we need to be sure the contents of uImage.FIT sub-images remain unaltered they are validated using a hash within the uImage.FIT structure which also serves as partition table.

---
 block/blk.h             | 1 +
 block/partitions/core.c | 3 +++
 2 files changed, 4 insertions(+)

index e85703ae81dd..05ac426350b2 100644
@@ -414,6 +414,7 @@ void blk_free_ext_minor(unsigned int minor);
 #define ADDPART_FLAG_NONE	0
 #define ADDPART_FLAG_RAID	1
 #define ADDPART_FLAG_WHOLEDISK	2
+#define ADDPART_FLAG_READONLY	4
 int bdev_add_partition(struct gendisk *disk, int partno, sector_t start,
 		sector_t length);
 int bdev_del_partition(struct gendisk *disk, int partno);

index b8112f52d388..355646b0707d 100644
@@ -398,6 +398,9 @@ static struct block_device *add_partition(struct gendisk *disk, int partno,
 			goto out_del;
 	}
 
+	if (flags & ADDPART_FLAG_READONLY)
+		bdev->bd_read_only = true;
+
 	/* everything is up and running, commence */
 	err = xa_insert(&disk->part_tbl, partno, bdev, GFP_KERNEL);
 	if (err)

2.38.1

Add function bdev_add_partition_ro() which can be used by drivers to register named read-only partitions on a disk device. Unlike the existing bdev_add_partition() function, there is also no check for overlapping partitions.
This new function is going to be used by the uImage.FIT parser.

---
 block/partitions/core.c | 34 ++++++++++++++++++++++++++++++++++
 include/linux/blkdev.h  |  7 +++++++
 2 files changed, 41 insertions(+)

index 355646b0707d..060a6585a387 100644
@@ -469,6 +469,40 @@ int bdev_add_partition(struct gendisk *disk, int partno, sector_t start,
 	return ret;
 }
 
+int bdev_add_partition_ro(struct gendisk *disk, int partno, sector_t start,
+			  sector_t length, const char *volname)
+{
+	struct block_device *part;
+	struct partition_meta_info *info;
+	int ret;
+
+	mutex_lock(&disk->open_mutex);
+	if (!disk_live(disk)) {
+		ret = -ENXIO;
+		goto out;
+	}
+
+	part = add_partition(disk, partno, start, length,
+			     ADDPART_FLAG_READONLY, NULL);
+	ret = PTR_ERR_OR_ZERO(part);
+	if (ret)
+		goto out;
+
+	if (volname) {
+		info = kzalloc(sizeof(struct partition_meta_info), GFP_KERNEL);
+		if (!info) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		strscpy(info->volname, volname, sizeof(info->volname));
+		part->bd_meta_info = info;
+	}
+out:
+	mutex_unlock(&disk->open_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(bdev_add_partition_ro);
+
 int bdev_del_partition(struct gendisk *disk, int partno)
 {
 	struct block_device *part = NULL;

index b45cdcdccc6d..6e468a2fc4ec 100644
@@ -1504,6 +1504,8 @@ int sync_blockdev_nowait(struct block_device *bdev);
 void sync_bdevs(bool wait);
 void bdev_statx_dioalign(struct inode *inode, struct kstat *stat);
 void printk_all_partitions(void);
+int bdev_add_partition_ro(struct gendisk *disk, int partno, sector_t start,
+			  sector_t length, const char *volname);
 #else
 static inline dev_t devt_from_partuuid(const char *uuid_str, int *root_wait)
 {

@@ -1537,6 +1539,11 @@ static inline void bdev_statx_dioalign(struct inode *inode, struct kstat *stat)
 static inline void printk_all_partitions(void)
 {
 }
+static inline int bdev_add_partition_ro(struct gendisk *disk, int partno, sector_t start,
+			  sector_t length, const char *volname)
+{
+	return 0;
+}
 #endif /* CONFIG_BLOCK */
 
 int fsync_bdev(struct block_device *bdev);

2.38.1

Err, no. No on has any business adding partitions to the block device except for the partition parser.

Well, there is a user-space ioctl for this as well[1], just that won't set the partition to read-only and also doesn't allow naming it...

[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/block/ioctl.c#n40

Add a small block driver which allows exposing filesystem sub-images contained in U-Boot uImage.FIT images as block partitions. The driver is intended for system using the U-Boot bootloader and requires the user to specify the lower block device to be specified as module parameter 'lower_dev'.

---
 MAINTAINERS            |   6 +
 drivers/block/Kconfig  |   6 +
 drivers/block/Makefile |   2 +
 drivers/block/fitblk.c | 364 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 378 insertions(+)
 create mode 100644 drivers/block/fitblk.c

index 08b67532e374..39402d6d0b9c 100644
@@ -21184,6 +21184,12 @@ F:	Documentation/filesystems/ubifs-authentication.rst
 F:	Documentation/filesystems/ubifs.rst
 F:	fs/ubifs/
 
+U-BOOT UIMAGE.FIT PARSER
+M:	Daniel Golle <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	drivers/block/fitblk.c
+
 UBLK USERSPACE BLOCK DRIVER
 M:	Ming Lei <[email protected]>
 L:	[email protected]

index a41145d52de9..c96c5b9964c7 100644
@@ -383,6 +383,12 @@ config VIRTIO_BLK
 	  This is the virtual block driver for virtio.  It can be used with
           QEMU based VMMs (like KVM or Xen).  Say Y or M.
 
+config UIMAGE_FIT_BLK
+	tristate "uImage.FIT block driver"
+	help
+	  This is driver allows using filesystems contained in uImage.FIT images
+	  by mapping them as block partitions.
+
 config BLK_DEV_RBD
 	tristate "Rados block device (RBD)"
 	depends on INET && BLOCK

index 101612cba303..60ab4fc1442d 100644
@@ -39,4 +39,6 @@ obj-$(CONFIG_BLK_DEV_NULL_BLK)	+= null_blk/
 
 obj-$(CONFIG_BLK_DEV_UBLK)			+= ublk_drv.o
 
+obj-$(CONFIG_UIMAGE_FIT_BLK)	+= fitblk.o
+
 swim_mod-y	:= swim.o swim_asm.o

2.38.1

Umm, this is is not a block device driver. A block device driver would take in bios and the upper devices it creates and then submit them to the lower devices.