[RFC PATCH v2 0/4] mtd: ubi: Enable accessing RO filesystems in UBI vols

[RFC PATCH v2 0/4] mtd: ubi: Enable accessing RO filesystems in UBI vols

[Sam Edwards]

Hi list,

This is the second version of my RFC patchset to treat static UBI volumes as DM
block devices, allowing users to access read-only filesystems (SquashFS, EROFS)
contained within such volumes.

This is a rebased and updated version, as requested by Heiko.

Previously, we have agreed on a syntax, which my downstream is now starting to
use:
=> ls ubi 0:rootfs /boot
=> ls ubi 0:2 /boot

This is still not yet ready for mainline inclusion, because the actual UBI DM
access is happening in disk/part.c, which is not "clean" with the way the
DM/legacy switching is currently plumbed. I'm still looking for guidance on how
to name/implement block functions for looking up a *subvolume* block device by
type+parentidx+{name,ID}.

Changes v1->v2:
- Rebased onto next post-2023.10's release.
- Fix NULL dereference caused by passing NULL to `blk_create_device`
- Parse UBI index/volume numbers with `dectoul` instead of `hextoul`, to match
  Linux's behavior of treating these numbers as decimal.
- Do not treat a valid decimal number as a volume name, even if the volume ID
  doesn't exist, to match Linux's behavior of always treating decimal numbers
  as volume IDs.

Cheers,
Sam

Sam Edwards (4):
  mtd: ubi: register UBI attachments as DM devices
  mtd: ubi: bind block device driver for static volumes
  disk: part: fall-through if "ubi" requested but ubifs not mounted
  HACK: enable access to `ubi 0:volname` block devices

 cmd/ubi.c                    |  11 +++
 disk/part.c                  |  69 +++++++++++--
 drivers/mtd/ubi/Makefile     |   1 +
 drivers/mtd/ubi/ubi-uclass.c | 185 +++++++++++++++++++++++++++++++++++
 include/dm/uclass-id.h       |   1 +
 include/ubi_uboot.h          |   5 +
 6 files changed, 264 insertions(+), 8 deletions(-)
 create mode 100644 drivers/mtd/ubi/ubi-uclass.c

-- 
2.41.0

[RFC PATCH v2 1/4] mtd: ubi: register UBI attachments as DM devices

[Sam Edwards]

This is in preparation for exposing static UBI volumes as block devices.

A UBI uclass and driver are introduced, and a "ubi0" virtual device
with the proper driver is created below whichever MTD device is
attached as the active UBI partition. This virtual device will soon
be the parent for the BLK devices that represent the static volumes.

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
 cmd/ubi.c                    | 11 ++++++
 drivers/mtd/ubi/Makefile     |  1 +
 drivers/mtd/ubi/ubi-uclass.c | 74 ++++++++++++++++++++++++++++++++++++
 include/dm/uclass-id.h       |  1 +
 include/ubi_uboot.h          |  5 +++
 5 files changed, 92 insertions(+)
 create mode 100644 drivers/mtd/ubi/ubi-uclass.c

diff --git a/cmd/ubi.c b/cmd/ubi.c
index 0a6a80bdd1..314c7f60f4 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -560,6 +560,13 @@ static int ubi_detach(void)
 		cmd_ubifs_umount();
 #endif
 
+#ifdef CONFIG_DM_MTD
+	/*
+	 * Clean up any UBI devices in DM
+	 */
+	ubi_dm_unbind_all();
+#endif
+
 	/*
 	 * Call ubi_exit() before re-initializing the UBI subsystem
 	 */
@@ -598,6 +605,10 @@ int ubi_part(char *part_name, const char *vid_header_offset)
 		return err;
 	}
 
+#ifdef CONFIG_DM_MTD
+	ubi_dm_bind(0);
+#endif
+
 	ubi = ubi_devices[0];
 
 	return 0;
diff --git a/drivers/mtd/ubi/Makefile b/drivers/mtd/ubi/Makefile
index 30d00fbdfe..375075f75e 100644
--- a/drivers/mtd/ubi/Makefile
+++ b/drivers/mtd/ubi/Makefile
@@ -7,3 +7,4 @@ obj-y += attach.o build.o vtbl.o vmt.o upd.o kapi.o eba.o io.o wl.o crc32.o
 obj-$(CONFIG_MTD_UBI_FASTMAP) += fastmap.o
 obj-y += misc.o
 obj-y += debug.o
+obj-$(CONFIG_DM_MTD) += ubi-uclass.o
diff --git a/drivers/mtd/ubi/ubi-uclass.c b/drivers/mtd/ubi/ubi-uclass.c
new file mode 100644
index 0000000000..f8971e793e
--- /dev/null
+++ b/drivers/mtd/ubi/ubi-uclass.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * ubi-uclass.c - UBI partition and volume block device uclass driver
+ *
+ * Copyright (C) 2023 Sam Edwards <CFSworks@gmail.com>
+ */
+
+#define LOG_CATEGORY UCLASS_UBI
+
+#include <common.h>
+#include <dm.h>
+#include <dm/device-internal.h>
+#include <ubi_uboot.h>
+
+int ubi_dm_bind(unsigned int index)
+{
+	struct udevice *dev;
+	int ret;
+	char name[16];
+	const char *name_dup;
+	struct ubi_device *ubi = ubi_devices[index];
+	const struct mtd_info *mtd = ubi->mtd;
+
+	/* MTD partitions are not in DM; navigate to the real MTD device */
+	if (mtd->parent)
+		mtd = mtd->parent;
+
+	snprintf(name, sizeof(name), "ubi%u", index);
+	name_dup = strdup(name);
+	ret = device_bind(mtd->dev, DM_DRIVER_GET(ubi), name_dup, ubi,
+			  ofnode_null(), &dev);
+	if (ret) {
+		free((void *)name_dup);
+		return ret;
+	}
+
+	device_set_name_alloced(dev);
+
+	return 0;
+}
+
+int ubi_dm_unbind_all(void)
+{
+	int ret;
+	struct uclass *uc;
+	struct udevice *dev;
+	struct udevice *next;
+
+	ret = uclass_get(UCLASS_UBI, &uc);
+	if (ret)
+		return ret;
+
+	uclass_foreach_dev_safe(dev, next, uc) {
+		ret = device_remove(dev, DM_REMOVE_NORMAL);
+		if (ret)
+			return ret;
+
+		ret = device_unbind(dev);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+U_BOOT_DRIVER(ubi) = {
+	.id		= UCLASS_UBI,
+	.name		= "ubi_dev",
+};
+
+UCLASS_DRIVER(ubi) = {
+	.id		= UCLASS_UBI,
+	.name		= "ubi",
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 0432c95c9e..ee11fbb38d 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -140,6 +140,7 @@ enum uclass_id {
 	UCLASS_THERMAL,		/* Thermal sensor */
 	UCLASS_TIMER,		/* Timer device */
 	UCLASS_TPM,		/* Trusted Platform Module TIS interface */
+	UCLASS_UBI,		/* Unsorted Block Images MTD partition */
 	UCLASS_UFS,		/* Universal Flash Storage */
 	UCLASS_USB,		/* USB bus */
 	UCLASS_USB_DEV_GENERIC,	/* USB generic device */
diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
index 6da348eb62..9d37848f03 100644
--- a/include/ubi_uboot.h
+++ b/include/ubi_uboot.h
@@ -52,6 +52,11 @@ extern int ubi_part(char *part_name, const char *vid_header_offset);
 extern int ubi_volume_write(char *volume, void *buf, size_t size);
 extern int ubi_volume_read(char *volume, char *buf, size_t size);
 
+#ifdef CONFIG_DM_MTD
+extern int ubi_dm_bind(unsigned int);
+extern int ubi_dm_unbind_all(void);
+#endif
+
 extern struct ubi_device *ubi_devices[];
 int cmd_ubifs_mount(char *vol_name);
 int cmd_ubifs_umount(void);
-- 
2.41.0

[RFC PATCH v2 2/4] mtd: ubi: bind block device driver for static volumes

[Sam Edwards]

This makes static UBI volumes readable as block devices, however
no mechanism for selecting these volume devices yet exists.

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
 drivers/mtd/ubi/ubi-uclass.c | 111 +++++++++++++++++++++++++++++++++++
 1 file changed, 111 insertions(+)

diff --git a/drivers/mtd/ubi/ubi-uclass.c b/drivers/mtd/ubi/ubi-uclass.c
index f8971e793e..b2c47bfe0c 100644
--- a/drivers/mtd/ubi/ubi-uclass.c
+++ b/drivers/mtd/ubi/ubi-uclass.c
@@ -8,10 +8,120 @@
 #define LOG_CATEGORY UCLASS_UBI
 
 #include <common.h>
+#include <blk.h>
 #include <dm.h>
 #include <dm/device-internal.h>
 #include <ubi_uboot.h>
 
+static ulong ubi_bread(struct udevice *dev, lbaint_t lba, lbaint_t blkcnt,
+		       void *dst)
+{
+	int err, lnum;
+	struct blk_desc *blk = dev_get_uclass_plat(dev);
+	struct ubi_device *ubi = dev_get_plat(dev->parent);
+	struct ubi_volume *vol = ubi->volumes[blk->devnum];
+	lbaint_t lba_per_peb = vol->usable_leb_size / blk->blksz;
+	lbaint_t lba_off, lba_len, total = 0;
+
+	while (blkcnt) {
+		lnum = lba / lba_per_peb;
+		lba_off = lba % lba_per_peb;
+		lba_len = lba_per_peb - lba_off;
+		if (lba_len > blkcnt)
+			lba_len = blkcnt;
+
+		err = ubi_eba_read_leb(ubi, vol, lnum, dst,
+				       lba_off << blk->log2blksz,
+				       lba_len << blk->log2blksz, 0);
+		if (err) {
+			pr_err("UBI read error %x\n", err);
+			break;
+		}
+
+		lba += lba_len;
+		blkcnt -= lba_len;
+		dst += lba_len << blk->log2blksz;
+		total += lba_len;
+	}
+
+	return total;
+}
+
+static const struct blk_ops ubi_block_ops = {
+	.read 	= ubi_bread,
+};
+
+U_BOOT_DRIVER(ubi_block) = {
+	.name	= "ubi_block",
+	.id	= UCLASS_BLK,
+	.ops	= &ubi_block_ops,
+};
+
+static bool is_power_of_two(unsigned int x)
+{
+	return (x & -x) == x;
+}
+
+static unsigned int choose_blksz_for_volume(const struct ubi_volume *vol)
+{
+	/*
+	 * U-Boot assumes a power-of-two blksz; however, UBI LEBs are
+	 * very often not suitably sized. To solve this, we divide the
+	 * LEBs into a whole number of LBAs per LEB, such that each LBA
+	 * addresses a power-of-two-sized block. To choose the blksz,
+	 * we either:
+	 * 1) Use the volume alignment, if it's a non-unity power of
+	 *    two. The LEB size is a multiple of this alignment, and it
+	 *    allows the user to force a particular blksz if needed for
+	 *    their use case.
+	 * 2) Otherwise, find the greatest power-of-two factor of the
+	 *    LEB size.
+	 */
+	if (vol->alignment > 1 && is_power_of_two(vol->alignment))
+		return vol->alignment;
+
+	unsigned int blksz = 1;
+	while ((vol->usable_leb_size & blksz) == 0)
+		blksz <<= 1;
+	return blksz;
+}
+
+static int ubi_post_bind(struct udevice *dev)
+{
+	int i;
+	int ret;
+	unsigned int blksz;
+	lbaint_t lba;
+	struct udevice *blkdev;
+	struct ubi_device *ubi = dev_get_plat(dev);
+
+	for (i = 0; i < ubi->vtbl_slots; i++) {
+		struct ubi_volume *vol = ubi->volumes[i];
+		if (!vol || vol->vol_id >= UBI_INTERNAL_VOL_START ||
+		    vol->vol_type != UBI_STATIC_VOLUME)
+			continue;
+
+		if (vol->updating || vol->upd_marker) {
+			pr_err("** UBI volume %d (\"%s\") midupdate; ignored\n",
+			       vol->vol_id, vol->name);
+			continue;
+		}
+
+		blksz = choose_blksz_for_volume(vol);
+		lba = DIV_ROUND_UP((unsigned long long)vol->used_bytes, blksz);
+
+		pr_debug("UBI volume %d (\"%s\"): %lu blocks, %d bytes each\n",
+			 vol->vol_id, vol->name, lba, blksz);
+
+		ret = blk_create_device(dev, "ubi_block", vol->name, UCLASS_UBI,
+					vol->vol_id, blksz, lba, &blkdev);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 int ubi_dm_bind(unsigned int index)
 {
 	struct udevice *dev;
@@ -71,4 +181,5 @@ U_BOOT_DRIVER(ubi) = {
 UCLASS_DRIVER(ubi) = {
 	.id		= UCLASS_UBI,
 	.name		= "ubi",
+	.post_bind	= ubi_post_bind,
 };
-- 
2.41.0

[RFC PATCH v2 3/4] disk: part: fall-through if "ubi" requested but ubifs not mounted

[Sam Edwards]

Since we're adding the ability to access static UBI volumes as block
devices, it is no longer an error to use the "ubi" ifname with UBIFS
unmounted.

Ideally, the access to UBIFS should instead be called "ubifs" but it
would break backwards compatibility to change this. Instead, use the
UBIFS mount status to disambiguate what the user means by "ubi"

There is no change in functionality in this patch: UBIFS access works
the same and an error still occurs when using "ubi" without UBIFS
mounted. The only difference is that now, the error message is a plain
"Bad device specification" and does not suggest using ubifsmount.

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
 disk/part.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/disk/part.c b/disk/part.c
index 72241b7b23..a4b6d265da 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -511,15 +511,13 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
 
 #if IS_ENABLED(CONFIG_CMD_UBIFS) && !IS_ENABLED(CONFIG_SPL_BUILD)
 	/*
-	 * Special-case ubi, ubi goes through a mtd, rather than through
-	 * a regular block device.
+	 * Special-case ubifs, which does not go through the block device layer
+	 * and also must be mounted ahead of time. U-Boot has historically
+	 * called this "ubi" too, even though *static* UBI volumes are
+	 * accessible as block devices. For backward compatibility, assume that
+	 * when UBIFS is mounted, the user intends "ubi" to mean "ubifs."
 	 */
-	if (!strcmp(ifname, "ubi")) {
-		if (!ubifs_is_mounted()) {
-			printf("UBIFS not mounted, use ubifsmount to mount volume first!\n");
-			return -EINVAL;
-		}
-
+	if (ubifs_is_mounted() && !strcmp(ifname, "ubi")) {
 		strcpy((char *)info->type, BOOT_PART_TYPE);
 		strcpy((char *)info->name, "UBI");
 		return 0;
-- 
2.41.0

[RFC PATCH v2 4/4] HACK: enable access to `ubi 0:volname` block devices

[Sam Edwards]

---
 disk/part.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/disk/part.c b/disk/part.c
index a4b6d265da..7c995f583c 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -14,6 +14,9 @@
 #include <malloc.h>
 #include <part.h>
 #include <ubifs_uboot.h>
+#include <dm.h>
+#include <dm/device-internal.h>
+#include <dm/uclass-internal.h>
 
 #undef	PART_DEBUG
 
@@ -524,6 +527,58 @@ int blk_get_device_part_str(const char *ifname, const char *dev_part_str,
 	}
 #endif
 
+#if IS_ENABLED(CONFIG_CMD_UBI) && !IS_ENABLED(CONFIG_SPL_BUILD)
+	/*
+	 * Also special-case UBI, which may use names to find the specific
+	 * volumes, so this deviates a bit from the typical devnum:partnum
+	 * syntax.
+	 */
+	if (!strcmp(ifname, "ubi")) {
+		dev = dectoul(dev_part_str, &ep);
+		if (*ep == ':') {
+			struct udevice *ubi_dev = NULL;
+			struct udevice *vol_dev = NULL;
+			part_str = &ep[1];
+
+			ret = uclass_find_device(UCLASS_UBI, dev, &ubi_dev);
+			if (!ubi_dev || ret) {
+				printf("** Cannot find UBI %x\n", dev);
+				return -EINVAL;
+			}
+
+			part = dectoul(part_str, &ep);
+			if (!*ep) {
+				struct udevice *tmp_dev;
+				device_foreach_child(tmp_dev, ubi_dev) {
+					struct blk_desc *desc = dev_get_uclass_plat(tmp_dev);
+					if (desc->devnum == part) {
+						vol_dev = tmp_dev;
+						break;
+					}
+				}
+			} else {
+				ret = device_find_child_by_name(ubi_dev, part_str, &vol_dev);
+			}
+
+			if (!vol_dev || ret) {
+				printf("** UBI volume %s not found\n", part_str);
+				return -EINVAL;
+			}
+
+			ret = device_probe(vol_dev);
+			if (ret)
+				return ret;
+
+			*desc = dev_get_uclass_plat(vol_dev);
+			part_get_info_whole_disk(*desc, info);
+			return 0;
+		}
+
+		printf("UBIFS not mounted, use ubifsmount to mount volume first!\n");
+		return -EINVAL;
+	}
+#endif
+
 	/* If no dev_part_str, use bootdevice environment variable */
 	if (CONFIG_IS_ENABLED(ENV_SUPPORT)) {
 		if (!dev_part_str || !strlen(dev_part_str) ||
-- 
2.41.0