* [PATCH 1/8] image: Fix script execution from FIT images with external data
2023-02-01 18:10 [PATCH 0/8] blk: blkmap: Composable virtual block devices Tobias Waldekranz
@ 2023-02-01 18:10 ` Tobias Waldekranz
2023-02-01 20:20 ` Simon Glass
2023-02-01 18:10 ` [PATCH 2/8] cmd: blk: Allow generic read/write operations to work in sandbox Tobias Waldekranz
` (7 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Tobias Waldekranz @ 2023-02-01 18:10 UTC (permalink / raw)
To: sjg, xypron.glpk, ilias.apalodimas; +Cc: u-boot
Update the script loading code to recognize when script data is stored
externally from the FIT metadata (i.e., built with `mkimage -E`).
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
boot/image-board.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/boot/image-board.c b/boot/image-board.c
index e5d71a3d54..74b2ad3580 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -1111,7 +1111,8 @@ fallback:
}
/* get script subimage data address and length */
- if (fit_image_get_data(fit_hdr, noffset, &fit_data, &fit_len)) {
+ if (fit_image_get_data_and_size(fit_hdr, noffset,
+ &fit_data, &fit_len)) {
puts("Could not find script subimage data\n");
return 1;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 2/8] cmd: blk: Allow generic read/write operations to work in sandbox
2023-02-01 18:10 [PATCH 0/8] blk: blkmap: Composable virtual block devices Tobias Waldekranz
2023-02-01 18:10 ` [PATCH 1/8] image: Fix script execution from FIT images with external data Tobias Waldekranz
@ 2023-02-01 18:10 ` Tobias Waldekranz
2023-02-01 20:20 ` Simon Glass
2023-02-01 18:10 ` [PATCH 3/8] blk: blkmap: Add basic infrastructure Tobias Waldekranz
` (6 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Tobias Waldekranz @ 2023-02-01 18:10 UTC (permalink / raw)
To: sjg, xypron.glpk, ilias.apalodimas; +Cc: u-boot
Ensure that the memory destination/source addresses of block
read/write operations are mapped in before access. Currently, this is
only needed on sandbox builds.
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
cmd/blk_common.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/cmd/blk_common.c b/cmd/blk_common.c
index 75a072caf5..9f9d4327a9 100644
--- a/cmd/blk_common.c
+++ b/cmd/blk_common.c
@@ -11,6 +11,7 @@
#include <common.h>
#include <blk.h>
#include <command.h>
+#include <mapmem.h>
int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
int *cur_devnump)
@@ -63,31 +64,37 @@ int blk_common_cmd(int argc, char *const argv[], enum uclass_id uclass_id,
default: /* at least 4 args */
if (strcmp(argv[1], "read") == 0) {
- ulong addr = hextoul(argv[2], NULL);
+ phys_addr_t paddr = hextoul(argv[2], NULL);
lbaint_t blk = hextoul(argv[3], NULL);
ulong cnt = hextoul(argv[4], NULL);
+ void *vaddr;
ulong n;
printf("\n%s read: device %d block # "LBAFU", count %lu ... ",
if_name, *cur_devnump, blk, cnt);
+ vaddr = map_sysmem(paddr, 512 * cnt);
n = blk_read_devnum(uclass_id, *cur_devnump, blk, cnt,
- (ulong *)addr);
+ vaddr);
+ unmap_sysmem(vaddr);
printf("%ld blocks read: %s\n", n,
n == cnt ? "OK" : "ERROR");
return n == cnt ? 0 : 1;
} else if (strcmp(argv[1], "write") == 0) {
- ulong addr = hextoul(argv[2], NULL);
+ phys_addr_t paddr = hextoul(argv[2], NULL);
lbaint_t blk = hextoul(argv[3], NULL);
ulong cnt = hextoul(argv[4], NULL);
+ void *vaddr;
ulong n;
printf("\n%s write: device %d block # "LBAFU", count %lu ... ",
if_name, *cur_devnump, blk, cnt);
+ vaddr = map_sysmem(paddr, 512 * cnt);
n = blk_write_devnum(uclass_id, *cur_devnump, blk, cnt,
- (ulong *)addr);
+ vaddr);
+ unmap_sysmem(vaddr);
printf("%ld blocks written: %s\n", n,
n == cnt ? "OK" : "ERROR");
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [PATCH 3/8] blk: blkmap: Add basic infrastructure
2023-02-01 18:10 [PATCH 0/8] blk: blkmap: Composable virtual block devices Tobias Waldekranz
2023-02-01 18:10 ` [PATCH 1/8] image: Fix script execution from FIT images with external data Tobias Waldekranz
2023-02-01 18:10 ` [PATCH 2/8] cmd: blk: Allow generic read/write operations to work in sandbox Tobias Waldekranz
@ 2023-02-01 18:10 ` Tobias Waldekranz
2023-02-01 20:20 ` Simon Glass
2023-02-01 18:10 ` [PATCH 4/8] blk: blkmap: Add memory mapping support Tobias Waldekranz
` (5 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Tobias Waldekranz @ 2023-02-01 18:10 UTC (permalink / raw)
To: sjg, xypron.glpk, ilias.apalodimas; +Cc: u-boot
blkmaps are loosely modeled on Linux's device mapper subsystem. The
basic idea is that you can create virtual block devices whose blocks
can be backed by a plethora of sources that are user configurable.
This change just adds the basic infrastructure for creating and
removing blkmap devices. Subsequent changes will extend this to add
support for actual mappings.
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
MAINTAINERS | 6 +
disk/part.c | 1 +
drivers/block/Kconfig | 18 ++
drivers/block/Makefile | 1 +
drivers/block/blk-uclass.c | 1 +
drivers/block/blkmap.c | 275 +++++++++++++++++++++++++++++++
include/blkmap.h | 15 ++
include/dm/uclass-id.h | 1 +
include/efi_loader.h | 4 +
lib/efi_loader/efi_device_path.c | 30 ++++
10 files changed, 352 insertions(+)
create mode 100644 drivers/block/blkmap.c
create mode 100644 include/blkmap.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 3e8e193ecc..28a34231bf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -786,6 +786,12 @@ M: Alper Nebi Yasak <alpernebiyasak@gmail.com>
S: Maintained
F: tools/binman/
+BLKMAP
+M: Tobias Waldekranz <tobias@waldekranz.com>
+S: Maintained
+F: drivers/block/blkmap.c
+F: include/blkmap.h
+
BOOTDEVICE
M: Simon Glass <sjg@chromium.org>
S: Maintained
diff --git a/disk/part.c b/disk/part.c
index d449635254..35300df590 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -140,6 +140,7 @@ void dev_print(struct blk_desc *dev_desc)
case UCLASS_NVME:
case UCLASS_PVBLOCK:
case UCLASS_HOST:
+ case UCLASS_BLKMAP:
printf ("Vendor: %s Rev: %s Prod: %s\n",
dev_desc->vendor,
dev_desc->revision,
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index e95da48bdc..5a1aeb3d2b 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -67,6 +67,24 @@ config BLOCK_CACHE
it will prevent repeated reads from directory structures and other
filesystem data structures.
+config BLKMAP
+ bool "Composable virtual block devices (blkmap)"
+ depends on BLK
+ help
+ Create virtual block devices that are backed by various sources,
+ e.g. RAM, or parts of an existing block device. Though much more
+ rudimentary, it borrows a lot of ideas from Linux's device mapper
+ subsystem.
+
+ Example use-cases:
+ - Treat a region of RAM as a block device, i.e. a RAM disk. This let's
+ you extract files from filesystem images stored in RAM (perhaps as a
+ result of a TFTP transfer).
+ - Create a virtual partition on an existing device. This let's you
+ access filesystems that aren't stored at an exact partition
+ boundary. A common example is a filesystem image embedded in an FIT
+ image.
+
config SPL_BLOCK_CACHE
bool "Use block device cache in SPL"
depends on SPL_BLK
diff --git a/drivers/block/Makefile b/drivers/block/Makefile
index f12447d78d..a161d145fd 100644
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_IDE) += ide.o
endif
obj-$(CONFIG_SANDBOX) += sandbox.o host-uclass.o host_dev.o
obj-$(CONFIG_$(SPL_TPL_)BLOCK_CACHE) += blkcache.o
+obj-$(CONFIG_BLKMAP) += blkmap.o
obj-$(CONFIG_EFI_MEDIA) += efi-media-uclass.o
obj-$(CONFIG_EFI_MEDIA_SANDBOX) += sb_efi_media.o
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index c69fc4d518..cb73faaeda 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -32,6 +32,7 @@ static struct {
{ UCLASS_EFI_LOADER, "efiloader" },
{ UCLASS_VIRTIO, "virtio" },
{ UCLASS_PVBLOCK, "pvblock" },
+ { UCLASS_BLKMAP, "blkmap" },
};
static enum uclass_id uclass_name_to_iftype(const char *uclass_idname)
diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
new file mode 100644
index 0000000000..a6ba07404c
--- /dev/null
+++ b/drivers/block/blkmap.c
@@ -0,0 +1,275 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2023 Addiva Elektronik
+ * Author: Tobias Waldekranz <tobias@waldekranz.com>
+ */
+
+#include <common.h>
+#include <blk.h>
+#include <blkmap.h>
+#include <dm.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+#include <dm/root.h>
+#include <malloc.h>
+#include <part.h>
+
+struct blkmap;
+
+struct blkmap_slice {
+ struct list_head node;
+
+ lbaint_t blknr;
+ lbaint_t blkcnt;
+
+ ulong (*read)(struct blkmap *bm, struct blkmap_slice *bms,
+ lbaint_t blknr, lbaint_t blkcnt, void *buffer);
+ ulong (*write)(struct blkmap *bm, struct blkmap_slice *bms,
+ lbaint_t blknr, lbaint_t blkcnt, const void *buffer);
+ void (*destroy)(struct blkmap *bm, struct blkmap_slice *bms);
+};
+
+struct blkmap {
+ struct udevice *dev;
+ struct list_head slices;
+};
+
+static bool blkmap_slice_contains(struct blkmap_slice *bms, lbaint_t blknr)
+{
+ return (blknr >= bms->blknr) && (blknr < (bms->blknr + bms->blkcnt));
+}
+
+static bool blkmap_slice_available(struct blkmap *bm, struct blkmap_slice *new)
+{
+ struct blkmap_slice *bms;
+ lbaint_t first, last;
+
+ first = new->blknr;
+ last = new->blknr + new->blkcnt - 1;
+
+ list_for_each_entry(bms, &bm->slices, node) {
+ if (blkmap_slice_contains(bms, first) ||
+ blkmap_slice_contains(bms, last) ||
+ blkmap_slice_contains(new, bms->blknr) ||
+ blkmap_slice_contains(new, bms->blknr + bms->blkcnt - 1))
+ return false;
+ }
+
+ return true;
+}
+
+static struct blkmap *blkmap_from_devnum(int devnum)
+{
+ struct udevice *dev;
+ int err;
+
+ err = blk_find_device(UCLASS_BLKMAP, devnum, &dev);
+
+ return err ? NULL : dev_get_priv(dev);
+}
+
+static int blkmap_add(struct blkmap *bm, struct blkmap_slice *new)
+{
+ struct blk_desc *bd = dev_get_uclass_plat(bm->dev);
+ struct list_head *insert = &bm->slices;
+ struct blkmap_slice *bms;
+
+ if (!blkmap_slice_available(bm, new))
+ return -EBUSY;
+
+ list_for_each_entry(bms, &bm->slices, node) {
+ if (bms->blknr < new->blknr)
+ continue;
+
+ insert = &bms->node;
+ break;
+ }
+
+ list_add_tail(&new->node, insert);
+
+ /* Disk might have grown, update the size */
+ bms = list_last_entry(&bm->slices, struct blkmap_slice, node);
+ bd->lba = bms->blknr + bms->blkcnt;
+ return 0;
+}
+
+static struct udevice *blkmap_root(void)
+{
+ static struct udevice *dev;
+ int err;
+
+ if (dev)
+ return dev;
+
+ err = device_bind_driver(dm_root(), "blkmap_root", "blkmap", &dev);
+ if (err)
+ return NULL;
+
+ err = device_probe(dev);
+ if (err) {
+ device_unbind(dev);
+ return NULL;
+ }
+
+ return dev;
+}
+
+int blkmap_create(int devnum)
+{
+ struct udevice *root;
+ struct blk_desc *bd;
+ struct blkmap *bm;
+ int err;
+
+ if (devnum >= 0 && blkmap_from_devnum(devnum))
+ return -EBUSY;
+
+ root = blkmap_root();
+ if (!root)
+ return -ENODEV;
+
+ bm = calloc(1, sizeof(*bm));
+ if (!bm)
+ return -ENOMEM;
+
+ err = blk_create_devicef(root, "blkmap_blk", "blk", UCLASS_BLKMAP,
+ devnum, 512, 0, &bm->dev);
+ if (err)
+ goto err_free;
+
+ bd = dev_get_uclass_plat(bm->dev);
+
+ /* EFI core isn't keen on zero-sized disks, so we lie. This is
+ * updated with the correct size once the user adds a
+ * mapping.
+ */
+ bd->lba = 1;
+
+ dev_set_priv(bm->dev, bm);
+ INIT_LIST_HEAD(&bm->slices);
+
+ err = blk_probe_or_unbind(bm->dev);
+ if (err)
+ goto err_remove;
+
+ return bd->devnum;
+
+err_remove:
+ device_remove(bm->dev, DM_REMOVE_NORMAL);
+err_free:
+ free(bm);
+ return err;
+}
+
+int blkmap_destroy(int devnum)
+{
+ struct blkmap_slice *bms, *tmp;
+ struct blkmap *bm;
+ int err;
+
+ bm = blkmap_from_devnum(devnum);
+ if (!bm)
+ return -ENODEV;
+
+ err = device_remove(bm->dev, DM_REMOVE_NORMAL);
+ if (err)
+ return err;
+
+ err = device_unbind(bm->dev);
+ if (err)
+ return err;
+
+ list_for_each_entry_safe(bms, tmp, &bm->slices, node) {
+ list_del(&bms->node);
+ free(bms);
+ }
+
+ free(bm);
+ return 0;
+}
+
+static ulong blkmap_read_slice(struct blkmap *bm, struct blkmap_slice *bms,
+ lbaint_t blknr, lbaint_t blkcnt, void *buffer)
+{
+ lbaint_t nr, cnt;
+
+ nr = blknr - bms->blknr;
+ cnt = (blkcnt < bms->blkcnt) ? blkcnt : bms->blkcnt;
+ return bms->read(bm, bms, nr, cnt, buffer);
+}
+
+static ulong blkmap_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
+ void *buffer)
+{
+ struct blk_desc *bd = dev_get_uclass_plat(dev);
+ struct blkmap *bm = dev_get_priv(dev);
+ struct blkmap_slice *bms;
+ lbaint_t cnt, total = 0;
+
+ list_for_each_entry(bms, &bm->slices, node) {
+ if (!blkmap_slice_contains(bms, blknr))
+ continue;
+
+ cnt = blkmap_read_slice(bm, bms, blknr, blkcnt, buffer);
+ blknr += cnt;
+ blkcnt -= cnt;
+ buffer += cnt << bd->log2blksz;
+ total += cnt;
+ }
+
+ return total;
+}
+
+static ulong blkmap_write_slice(struct blkmap *bm, struct blkmap_slice *bms,
+ lbaint_t blknr, lbaint_t blkcnt,
+ const void *buffer)
+{
+ lbaint_t nr, cnt;
+
+ nr = blknr - bms->blknr;
+ cnt = (blkcnt < bms->blkcnt) ? blkcnt : bms->blkcnt;
+ return bms->write(bm, bms, nr, cnt, buffer);
+}
+
+static ulong blkmap_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
+ const void *buffer)
+{
+ struct blk_desc *bd = dev_get_uclass_plat(dev);
+ struct blkmap *bm = dev_get_priv(dev);
+ struct blkmap_slice *bms;
+ lbaint_t cnt, total = 0;
+
+ list_for_each_entry(bms, &bm->slices, node) {
+ if (!blkmap_slice_contains(bms, blknr))
+ continue;
+
+ cnt = blkmap_write_slice(bm, bms, blknr, blkcnt, buffer);
+ blknr += cnt;
+ blkcnt -= cnt;
+ buffer += cnt << bd->log2blksz;
+ total += cnt;
+ }
+
+ return total;
+}
+
+static const struct blk_ops blkmap_ops = {
+ .read = blkmap_read,
+ .write = blkmap_write,
+};
+
+U_BOOT_DRIVER(blkmap_blk) = {
+ .name = "blkmap_blk",
+ .id = UCLASS_BLK,
+ .ops = &blkmap_ops,
+};
+
+U_BOOT_DRIVER(blkmap_root) = {
+ .name = "blkmap_root",
+ .id = UCLASS_BLKMAP,
+};
+
+UCLASS_DRIVER(blkmap) = {
+ .id = UCLASS_BLKMAP,
+ .name = "blkmap",
+};
diff --git a/include/blkmap.h b/include/blkmap.h
new file mode 100644
index 0000000000..37c0c31c3f
--- /dev/null
+++ b/include/blkmap.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (c) 2023 Addiva Elektronik
+ * Author: Tobias Waldekranz <tobias@waldekranz.com>
+ */
+
+#ifndef _BLKMAP_H
+#define _BLKMAP_H
+
+#include <stdbool.h>
+
+int blkmap_create(int devnum);
+int blkmap_destroy(int devnum);
+
+#endif /* _BLKMAP_H */
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 33e43c20db..576237b954 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -37,6 +37,7 @@ enum uclass_id {
UCLASS_AUDIO_CODEC, /* Audio codec with control and data path */
UCLASS_AXI, /* AXI bus */
UCLASS_BLK, /* Block device */
+ UCLASS_BLKMAP, /* Composable virtual block device */
UCLASS_BOOTCOUNT, /* Bootcount backing store */
UCLASS_BOOTDEV, /* Boot device for locating an OS to boot */
UCLASS_BOOTMETH, /* Bootmethod for booting an OS */
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 4560b0d04c..59687f44de 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -134,6 +134,10 @@ static inline efi_status_t efi_launch_capsules(void)
#define U_BOOT_GUID \
EFI_GUID(0xe61d73b9, 0xa384, 0x4acc, \
0xae, 0xab, 0x82, 0xe8, 0x28, 0xf3, 0x62, 0x8b)
+/* GUID used as root for blkmap devices */
+#define U_BOOT_BLKMAP_DEV_GUID \
+ EFI_GUID(0x4cad859d, 0xd644, 0x42ff, \
+ 0x87, 0x0b, 0xc0, 0x2e, 0xac, 0x05, 0x58, 0x63)
/* GUID used as host device on sandbox */
#define U_BOOT_HOST_DEV_GUID \
EFI_GUID(0xbbe4e671, 0x5773, 0x4ea1, \
diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index 3b267b713e..4b4c96bc2e 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -21,6 +21,9 @@
#include <asm-generic/unaligned.h>
#include <linux/compat.h> /* U16_MAX */
+#ifdef CONFIG_BLKMAP
+const efi_guid_t efi_guid_blkmap_dev = U_BOOT_BLKMAP_DEV_GUID;
+#endif
#ifdef CONFIG_SANDBOX
const efi_guid_t efi_guid_host_dev = U_BOOT_HOST_DEV_GUID;
#endif
@@ -573,6 +576,16 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev)
*/
return dp_size(dev->parent)
+ sizeof(struct efi_device_path_vendor) + 1;
+#endif
+#ifdef CONFIG_BLKMAP
+ case UCLASS_BLKMAP:
+ /*
+ * blkmap devices will be represented as a vendor
+ * device node with an extra byte for the device
+ * number.
+ */
+ return dp_size(dev->parent)
+ + sizeof(struct efi_device_path_vendor) + 1;
#endif
default:
return dp_size(dev->parent);
@@ -631,6 +644,23 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev)
#endif
case UCLASS_BLK:
switch (dev->parent->uclass->uc_drv->id) {
+#ifdef CONFIG_BLKMAP
+ case UCLASS_BLKMAP: {
+ struct efi_device_path_vendor *dp;
+ struct blk_desc *desc = dev_get_uclass_plat(dev);
+
+ dp_fill(buf, dev->parent);
+ dp = buf;
+ ++dp;
+ dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
+ dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
+ dp->dp.length = sizeof(*dp) + 1;
+ memcpy(&dp->guid, &efi_guid_blkmap_dev,
+ sizeof(efi_guid_t));
+ dp->vendor_data[0] = desc->devnum;
+ return &dp->vendor_data[1];
+ }
+#endif
#ifdef CONFIG_SANDBOX
case UCLASS_HOST: {
/* stop traversing parents at this point: */
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 3/8] blk: blkmap: Add basic infrastructure
2023-02-01 18:10 ` [PATCH 3/8] blk: blkmap: Add basic infrastructure Tobias Waldekranz
@ 2023-02-01 20:20 ` Simon Glass
2023-02-03 9:38 ` Tobias Waldekranz
0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2023-02-01 20:20 UTC (permalink / raw)
To: Tobias Waldekranz; +Cc: xypron.glpk, ilias.apalodimas, u-boot
Hi Tobias,
On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>
> blkmaps are loosely modeled on Linux's device mapper subsystem. The
> basic idea is that you can create virtual block devices whose blocks
> can be backed by a plethora of sources that are user configurable.
>
> This change just adds the basic infrastructure for creating and
> removing blkmap devices. Subsequent changes will extend this to add
> support for actual mappings.
>
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
> MAINTAINERS | 6 +
> disk/part.c | 1 +
> drivers/block/Kconfig | 18 ++
> drivers/block/Makefile | 1 +
> drivers/block/blk-uclass.c | 1 +
> drivers/block/blkmap.c | 275 +++++++++++++++++++++++++++++++
> include/blkmap.h | 15 ++
> include/dm/uclass-id.h | 1 +
> include/efi_loader.h | 4 +
> lib/efi_loader/efi_device_path.c | 30 ++++
> 10 files changed, 352 insertions(+)
> create mode 100644 drivers/block/blkmap.c
> create mode 100644 include/blkmap.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3e8e193ecc..28a34231bf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -786,6 +786,12 @@ M: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> S: Maintained
> F: tools/binman/
>
> +BLKMAP
> +M: Tobias Waldekranz <tobias@waldekranz.com>
> +S: Maintained
> +F: drivers/block/blkmap.c
> +F: include/blkmap.h
> +
> BOOTDEVICE
> M: Simon Glass <sjg@chromium.org>
> S: Maintained
> diff --git a/disk/part.c b/disk/part.c
> index d449635254..35300df590 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -140,6 +140,7 @@ void dev_print(struct blk_desc *dev_desc)
> case UCLASS_NVME:
> case UCLASS_PVBLOCK:
> case UCLASS_HOST:
> + case UCLASS_BLKMAP:
> printf ("Vendor: %s Rev: %s Prod: %s\n",
> dev_desc->vendor,
> dev_desc->revision,
> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> index e95da48bdc..5a1aeb3d2b 100644
> --- a/drivers/block/Kconfig
> +++ b/drivers/block/Kconfig
> @@ -67,6 +67,24 @@ config BLOCK_CACHE
> it will prevent repeated reads from directory structures and other
> filesystem data structures.
>
> +config BLKMAP
> + bool "Composable virtual block devices (blkmap)"
> + depends on BLK
> + help
> + Create virtual block devices that are backed by various sources,
> + e.g. RAM, or parts of an existing block device. Though much more
> + rudimentary, it borrows a lot of ideas from Linux's device mapper
> + subsystem.
> +
> + Example use-cases:
> + - Treat a region of RAM as a block device, i.e. a RAM disk. This let's
> + you extract files from filesystem images stored in RAM (perhaps as a
> + result of a TFTP transfer).
> + - Create a virtual partition on an existing device. This let's you
> + access filesystems that aren't stored at an exact partition
> + boundary. A common example is a filesystem image embedded in an FIT
> + image.
> +
> config SPL_BLOCK_CACHE
> bool "Use block device cache in SPL"
> depends on SPL_BLK
> diff --git a/drivers/block/Makefile b/drivers/block/Makefile
> index f12447d78d..a161d145fd 100644
> --- a/drivers/block/Makefile
> +++ b/drivers/block/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_IDE) += ide.o
> endif
> obj-$(CONFIG_SANDBOX) += sandbox.o host-uclass.o host_dev.o
> obj-$(CONFIG_$(SPL_TPL_)BLOCK_CACHE) += blkcache.o
> +obj-$(CONFIG_BLKMAP) += blkmap.o
>
> obj-$(CONFIG_EFI_MEDIA) += efi-media-uclass.o
> obj-$(CONFIG_EFI_MEDIA_SANDBOX) += sb_efi_media.o
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index c69fc4d518..cb73faaeda 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -32,6 +32,7 @@ static struct {
> { UCLASS_EFI_LOADER, "efiloader" },
> { UCLASS_VIRTIO, "virtio" },
> { UCLASS_PVBLOCK, "pvblock" },
> + { UCLASS_BLKMAP, "blkmap" },
> };
>
> static enum uclass_id uclass_name_to_iftype(const char *uclass_idname)
> diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
> new file mode 100644
> index 0000000000..a6ba07404c
> --- /dev/null
> +++ b/drivers/block/blkmap.c
> @@ -0,0 +1,275 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2023 Addiva Elektronik
> + * Author: Tobias Waldekranz <tobias@waldekranz.com>
> + */
> +
> +#include <common.h>
> +#include <blk.h>
> +#include <blkmap.h>
> +#include <dm.h>
> +#include <dm/device-internal.h>
> +#include <dm/lists.h>
> +#include <dm/root.h>
The three above should go at the end:
https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files
> +#include <malloc.h>
> +#include <part.h>
> +
> +struct blkmap;
> +
> +struct blkmap_slice {
> + struct list_head node;
> +
> + lbaint_t blknr;
> + lbaint_t blkcnt;
> +
> + ulong (*read)(struct blkmap *bm, struct blkmap_slice *bms,
> + lbaint_t blknr, lbaint_t blkcnt, void *buffer);
> + ulong (*write)(struct blkmap *bm, struct blkmap_slice *bms,
> + lbaint_t blknr, lbaint_t blkcnt, const void *buffer);
> + void (*destroy)(struct blkmap *bm, struct blkmap_slice *bms);
> +};
Please comment these fully in Sphinx style
> +
> +struct blkmap {
> + struct udevice *dev;
> + struct list_head slices;
> +};
> +
> +static bool blkmap_slice_contains(struct blkmap_slice *bms, lbaint_t blknr)
> +{
> + return (blknr >= bms->blknr) && (blknr < (bms->blknr + bms->blkcnt));
lots of brackets!
> +}
> +
> +static bool blkmap_slice_available(struct blkmap *bm, struct blkmap_slice *new)
> +{
> + struct blkmap_slice *bms;
> + lbaint_t first, last;
> +
> + first = new->blknr;
> + last = new->blknr + new->blkcnt - 1;
> +
> + list_for_each_entry(bms, &bm->slices, node) {
> + if (blkmap_slice_contains(bms, first) ||
> + blkmap_slice_contains(bms, last) ||
> + blkmap_slice_contains(new, bms->blknr) ||
> + blkmap_slice_contains(new, bms->blknr + bms->blkcnt - 1))
> + return false;
> + }
> +
> + return true;
> +}
> +
> +static struct blkmap *blkmap_from_devnum(int devnum)
I don't really like the use of devnum everywhere. Can we instead limit
the devnum stuff to the cmdline implementation, and use a struct
udevice everywhere else?
The devum can be allocated when the UCLASS_BLK device is created.
> +{
> + struct udevice *dev;
> + int err;
> +
> + err = blk_find_device(UCLASS_BLKMAP, devnum, &dev);
> +
> + return err ? NULL : dev_get_priv(dev);
> +}
> +
> +static int blkmap_add(struct blkmap *bm, struct blkmap_slice *new)
> +{
> + struct blk_desc *bd = dev_get_uclass_plat(bm->dev);
> + struct list_head *insert = &bm->slices;
> + struct blkmap_slice *bms;
> +
> + if (!blkmap_slice_available(bm, new))
> + return -EBUSY;
> +
> + list_for_each_entry(bms, &bm->slices, node) {
> + if (bms->blknr < new->blknr)
> + continue;
> +
> + insert = &bms->node;
> + break;
> + }
> +
> + list_add_tail(&new->node, insert);
> +
> + /* Disk might have grown, update the size */
> + bms = list_last_entry(&bm->slices, struct blkmap_slice, node);
> + bd->lba = bms->blknr + bms->blkcnt;
> + return 0;
> +}
> +
> +static struct udevice *blkmap_root(void)
This needs to be created as part of DM. See how host_create_device()
works. It attaches something to the uclass and then creates child
devices from there. It also operations (struct host_ops) but you don't
need to do that.
Note that the host commands support either an label or a devnum, which
I think is useful, so you might copy that?
> +{
> + static struct udevice *dev;
> + int err;
> +
> + if (dev)
> + return dev;
> +
> + err = device_bind_driver(dm_root(), "blkmap_root", "blkmap", &dev);
> + if (err)
> + return NULL;
> +
> + err = device_probe(dev);
> + if (err) {
> + device_unbind(dev);
> + return NULL;
> + }
Should not be needed as probing children will cause this to be probed.
So this function just becomes
uclass_first_device(UCLASS_BLKDEV, &
> +
> + return dev;
> +}
> +
> +int blkmap_create(int devnum)
Again, please drop the use of devnum and use devices. Here you could
use a label, perhaps?
> +{
> + struct udevice *root;
Please don't use that name , as we use it for the DM root device. How
about bm_parent? It isn't actually a tree of devices, so root doesn't
sound right to me anyway.
> + struct blk_desc *bd;
> + struct blkmap *bm;
> + int err;
> +
> + if (devnum >= 0 && blkmap_from_devnum(devnum))
> + return -EBUSY;
> +
> + root = blkmap_root();
> + if (!root)
> + return -ENODEV;
> +
> + bm = calloc(1, sizeof(*bm));
Can this be attached to the device as private data, so avoiding the malloc()?
> + if (!bm)
> + return -ENOMEM;
> +
> + err = blk_create_devicef(root, "blkmap_blk", "blk", UCLASS_BLKMAP,
> + devnum, 512, 0, &bm->dev);
> + if (err)
> + goto err_free;
> +
> + bd = dev_get_uclass_plat(bm->dev);
> +
> + /* EFI core isn't keen on zero-sized disks, so we lie. This is
> + * updated with the correct size once the user adds a
> + * mapping.
> + */
> + bd->lba = 1;
if (CONFIG_IS_ENABLED(EFI_LOADER))
?
> +
> + dev_set_priv(bm->dev, bm);
> + INIT_LIST_HEAD(&bm->slices);
> +
> + err = blk_probe_or_unbind(bm->dev);
> + if (err)
> + goto err_remove;
> +
> + return bd->devnum;
> +
> +err_remove:
> + device_remove(bm->dev, DM_REMOVE_NORMAL);
> +err_free:
> + free(bm);
> + return err;
> +}
> +
> +int blkmap_destroy(int devnum)
label
> +{
> + struct blkmap_slice *bms, *tmp;
> + struct blkmap *bm;
> + int err;
> +
> + bm = blkmap_from_devnum(devnum);
> + if (!bm)
> + return -ENODEV;
> +
> + err = device_remove(bm->dev, DM_REMOVE_NORMAL);
> + if (err)
> + return err;
> +
> + err = device_unbind(bm->dev);
> + if (err)
> + return err;
> +
> + list_for_each_entry_safe(bms, tmp, &bm->slices, node) {
> + list_del(&bms->node);
> + free(bms);
> + }
> +
> + free(bm);
> + return 0;
> +}
> +
> +static ulong blkmap_read_slice(struct blkmap *bm, struct blkmap_slice *bms,
> + lbaint_t blknr, lbaint_t blkcnt, void *buffer)
> +{
> + lbaint_t nr, cnt;
> +
> + nr = blknr - bms->blknr;
> + cnt = (blkcnt < bms->blkcnt) ? blkcnt : bms->blkcnt;
> + return bms->read(bm, bms, nr, cnt, buffer);
> +}
> +
> +static ulong blkmap_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
> + void *buffer)
> +{
> + struct blk_desc *bd = dev_get_uclass_plat(dev);
> + struct blkmap *bm = dev_get_priv(dev);
> + struct blkmap_slice *bms;
> + lbaint_t cnt, total = 0;
> +
> + list_for_each_entry(bms, &bm->slices, node) {
> + if (!blkmap_slice_contains(bms, blknr))
> + continue;
> +
> + cnt = blkmap_read_slice(bm, bms, blknr, blkcnt, buffer);
> + blknr += cnt;
> + blkcnt -= cnt;
> + buffer += cnt << bd->log2blksz;
> + total += cnt;
> + }
> +
> + return total;
> +}
> +
> +static ulong blkmap_write_slice(struct blkmap *bm, struct blkmap_slice *bms,
> + lbaint_t blknr, lbaint_t blkcnt,
> + const void *buffer)
> +{
> + lbaint_t nr, cnt;
> +
> + nr = blknr - bms->blknr;
> + cnt = (blkcnt < bms->blkcnt) ? blkcnt : bms->blkcnt;
> + return bms->write(bm, bms, nr, cnt, buffer);
> +}
> +
> +static ulong blkmap_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
> + const void *buffer)
> +{
> + struct blk_desc *bd = dev_get_uclass_plat(dev);
> + struct blkmap *bm = dev_get_priv(dev);
> + struct blkmap_slice *bms;
> + lbaint_t cnt, total = 0;
> +
> + list_for_each_entry(bms, &bm->slices, node) {
> + if (!blkmap_slice_contains(bms, blknr))
> + continue;
> +
> + cnt = blkmap_write_slice(bm, bms, blknr, blkcnt, buffer);
> + blknr += cnt;
> + blkcnt -= cnt;
> + buffer += cnt << bd->log2blksz;
> + total += cnt;
> + }
> +
> + return total;
> +}
> +
> +static const struct blk_ops blkmap_ops = {
> + .read = blkmap_read,
> + .write = blkmap_write,
> +};
> +
> +U_BOOT_DRIVER(blkmap_blk) = {
> + .name = "blkmap_blk",
> + .id = UCLASS_BLK,
> + .ops = &blkmap_ops,
> +};
> +
> +U_BOOT_DRIVER(blkmap_root) = {
> + .name = "blkmap_root",
> + .id = UCLASS_BLKMAP,
> +};
> +
> +UCLASS_DRIVER(blkmap) = {
> + .id = UCLASS_BLKMAP,
> + .name = "blkmap",
> +};
> diff --git a/include/blkmap.h b/include/blkmap.h
> new file mode 100644
> index 0000000000..37c0c31c3f
> --- /dev/null
> +++ b/include/blkmap.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */
> +/*
> + * Copyright (c) 2023 Addiva Elektronik
> + * Author: Tobias Waldekranz <tobias@waldekranz.com>
> + */
> +
> +#ifndef _BLKMAP_H
> +#define _BLKMAP_H
> +
> +#include <stdbool.h>
> +
> +int blkmap_create(int devnum);
> +int blkmap_destroy(int devnum);
full comments for exported functions
> +
> +#endif /* _BLKMAP_H */
> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
> index 33e43c20db..576237b954 100644
> --- a/include/dm/uclass-id.h
> +++ b/include/dm/uclass-id.h
> @@ -37,6 +37,7 @@ enum uclass_id {
> UCLASS_AUDIO_CODEC, /* Audio codec with control and data path */
> UCLASS_AXI, /* AXI bus */
> UCLASS_BLK, /* Block device */
> + UCLASS_BLKMAP, /* Composable virtual block device */
> UCLASS_BOOTCOUNT, /* Bootcount backing store */
> UCLASS_BOOTDEV, /* Boot device for locating an OS to boot */
> UCLASS_BOOTMETH, /* Bootmethod for booting an OS */
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 4560b0d04c..59687f44de 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -134,6 +134,10 @@ static inline efi_status_t efi_launch_capsules(void)
> #define U_BOOT_GUID \
> EFI_GUID(0xe61d73b9, 0xa384, 0x4acc, \
> 0xae, 0xab, 0x82, 0xe8, 0x28, 0xf3, 0x62, 0x8b)
> +/* GUID used as root for blkmap devices */
> +#define U_BOOT_BLKMAP_DEV_GUID \
> + EFI_GUID(0x4cad859d, 0xd644, 0x42ff, \
> + 0x87, 0x0b, 0xc0, 0x2e, 0xac, 0x05, 0x58, 0x63)
> /* GUID used as host device on sandbox */
> #define U_BOOT_HOST_DEV_GUID \
> EFI_GUID(0xbbe4e671, 0x5773, 0x4ea1, \
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index 3b267b713e..4b4c96bc2e 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
Please put this EFI stuff in a separate patch.
> @@ -21,6 +21,9 @@
> #include <asm-generic/unaligned.h>
> #include <linux/compat.h> /* U16_MAX */
>
> +#ifdef CONFIG_BLKMAP
> +const efi_guid_t efi_guid_blkmap_dev = U_BOOT_BLKMAP_DEV_GUID;
> +#endif
> #ifdef CONFIG_SANDBOX
> const efi_guid_t efi_guid_host_dev = U_BOOT_HOST_DEV_GUID;
> #endif
> @@ -573,6 +576,16 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev)
> */
> return dp_size(dev->parent)
> + sizeof(struct efi_device_path_vendor) + 1;
> +#endif
> +#ifdef CONFIG_BLKMAP
> + case UCLASS_BLKMAP:
> + /*
> + * blkmap devices will be represented as a vendor
> + * device node with an extra byte for the device
> + * number.
> + */
> + return dp_size(dev->parent)
> + + sizeof(struct efi_device_path_vendor) + 1;
> #endif
> default:
> return dp_size(dev->parent);
> @@ -631,6 +644,23 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev)
> #endif
> case UCLASS_BLK:
> switch (dev->parent->uclass->uc_drv->id) {
> +#ifdef CONFIG_BLKMAP
> + case UCLASS_BLKMAP: {
> + struct efi_device_path_vendor *dp;
> + struct blk_desc *desc = dev_get_uclass_plat(dev);
> +
> + dp_fill(buf, dev->parent);
> + dp = buf;
> + ++dp;
> + dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
> + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
> + dp->dp.length = sizeof(*dp) + 1;
> + memcpy(&dp->guid, &efi_guid_blkmap_dev,
> + sizeof(efi_guid_t));
> + dp->vendor_data[0] = desc->devnum;
> + return &dp->vendor_data[1];
> + }
> +#endif
> #ifdef CONFIG_SANDBOX
> case UCLASS_HOST: {
> /* stop traversing parents at this point: */
> --
> 2.34.1
>
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/8] blk: blkmap: Add basic infrastructure
2023-02-01 20:20 ` Simon Glass
@ 2023-02-03 9:38 ` Tobias Waldekranz
2023-02-04 0:20 ` Simon Glass
0 siblings, 1 reply; 25+ messages in thread
From: Tobias Waldekranz @ 2023-02-03 9:38 UTC (permalink / raw)
To: Simon Glass; +Cc: xypron.glpk, ilias.apalodimas, u-boot
On ons, feb 01, 2023 at 13:20, Simon Glass <sjg@chromium.org> wrote:
> Hi Tobias,
Hi Simon,
Thanks for the review!
> On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>>
>> blkmaps are loosely modeled on Linux's device mapper subsystem. The
>> basic idea is that you can create virtual block devices whose blocks
>> can be backed by a plethora of sources that are user configurable.
>>
>> This change just adds the basic infrastructure for creating and
>> removing blkmap devices. Subsequent changes will extend this to add
>> support for actual mappings.
>>
>> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
>> ---
>> MAINTAINERS | 6 +
>> disk/part.c | 1 +
>> drivers/block/Kconfig | 18 ++
>> drivers/block/Makefile | 1 +
>> drivers/block/blk-uclass.c | 1 +
>> drivers/block/blkmap.c | 275 +++++++++++++++++++++++++++++++
>> include/blkmap.h | 15 ++
>> include/dm/uclass-id.h | 1 +
>> include/efi_loader.h | 4 +
>> lib/efi_loader/efi_device_path.c | 30 ++++
>> 10 files changed, 352 insertions(+)
>> create mode 100644 drivers/block/blkmap.c
>> create mode 100644 include/blkmap.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 3e8e193ecc..28a34231bf 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -786,6 +786,12 @@ M: Alper Nebi Yasak <alpernebiyasak@gmail.com>
>> S: Maintained
>> F: tools/binman/
>>
>> +BLKMAP
>> +M: Tobias Waldekranz <tobias@waldekranz.com>
>> +S: Maintained
>> +F: drivers/block/blkmap.c
>> +F: include/blkmap.h
>> +
>> BOOTDEVICE
>> M: Simon Glass <sjg@chromium.org>
>> S: Maintained
>> diff --git a/disk/part.c b/disk/part.c
>> index d449635254..35300df590 100644
>> --- a/disk/part.c
>> +++ b/disk/part.c
>> @@ -140,6 +140,7 @@ void dev_print(struct blk_desc *dev_desc)
>> case UCLASS_NVME:
>> case UCLASS_PVBLOCK:
>> case UCLASS_HOST:
>> + case UCLASS_BLKMAP:
>> printf ("Vendor: %s Rev: %s Prod: %s\n",
>> dev_desc->vendor,
>> dev_desc->revision,
>> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
>> index e95da48bdc..5a1aeb3d2b 100644
>> --- a/drivers/block/Kconfig
>> +++ b/drivers/block/Kconfig
>> @@ -67,6 +67,24 @@ config BLOCK_CACHE
>> it will prevent repeated reads from directory structures and other
>> filesystem data structures.
>>
>> +config BLKMAP
>> + bool "Composable virtual block devices (blkmap)"
>> + depends on BLK
>> + help
>> + Create virtual block devices that are backed by various sources,
>> + e.g. RAM, or parts of an existing block device. Though much more
>> + rudimentary, it borrows a lot of ideas from Linux's device mapper
>> + subsystem.
>> +
>> + Example use-cases:
>> + - Treat a region of RAM as a block device, i.e. a RAM disk. This let's
>> + you extract files from filesystem images stored in RAM (perhaps as a
>> + result of a TFTP transfer).
>> + - Create a virtual partition on an existing device. This let's you
>> + access filesystems that aren't stored at an exact partition
>> + boundary. A common example is a filesystem image embedded in an FIT
>> + image.
>> +
>> config SPL_BLOCK_CACHE
>> bool "Use block device cache in SPL"
>> depends on SPL_BLK
>> diff --git a/drivers/block/Makefile b/drivers/block/Makefile
>> index f12447d78d..a161d145fd 100644
>> --- a/drivers/block/Makefile
>> +++ b/drivers/block/Makefile
>> @@ -14,6 +14,7 @@ obj-$(CONFIG_IDE) += ide.o
>> endif
>> obj-$(CONFIG_SANDBOX) += sandbox.o host-uclass.o host_dev.o
>> obj-$(CONFIG_$(SPL_TPL_)BLOCK_CACHE) += blkcache.o
>> +obj-$(CONFIG_BLKMAP) += blkmap.o
>>
>> obj-$(CONFIG_EFI_MEDIA) += efi-media-uclass.o
>> obj-$(CONFIG_EFI_MEDIA_SANDBOX) += sb_efi_media.o
>> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
>> index c69fc4d518..cb73faaeda 100644
>> --- a/drivers/block/blk-uclass.c
>> +++ b/drivers/block/blk-uclass.c
>> @@ -32,6 +32,7 @@ static struct {
>> { UCLASS_EFI_LOADER, "efiloader" },
>> { UCLASS_VIRTIO, "virtio" },
>> { UCLASS_PVBLOCK, "pvblock" },
>> + { UCLASS_BLKMAP, "blkmap" },
>> };
>>
>> static enum uclass_id uclass_name_to_iftype(const char *uclass_idname)
>> diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
>> new file mode 100644
>> index 0000000000..a6ba07404c
>> --- /dev/null
>> +++ b/drivers/block/blkmap.c
>> @@ -0,0 +1,275 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright (c) 2023 Addiva Elektronik
>> + * Author: Tobias Waldekranz <tobias@waldekranz.com>
>> + */
>> +
>> +#include <common.h>
>> +#include <blk.h>
>> +#include <blkmap.h>
>> +#include <dm.h>
>> +#include <dm/device-internal.h>
>> +#include <dm/lists.h>
>> +#include <dm/root.h>
>
> The three above should go at the end:
>
> https://u-boot.readthedocs.io/en/latest/develop/codingstyle.html#include-files
I should've read more carefully, sorry. Fixing.
>> +#include <malloc.h>
>> +#include <part.h>
>> +
>> +struct blkmap;
>> +
>> +struct blkmap_slice {
>> + struct list_head node;
>> +
>> + lbaint_t blknr;
>> + lbaint_t blkcnt;
>> +
>> + ulong (*read)(struct blkmap *bm, struct blkmap_slice *bms,
>> + lbaint_t blknr, lbaint_t blkcnt, void *buffer);
>> + ulong (*write)(struct blkmap *bm, struct blkmap_slice *bms,
>> + lbaint_t blknr, lbaint_t blkcnt, const void *buffer);
>> + void (*destroy)(struct blkmap *bm, struct blkmap_slice *bms);
>> +};
>
> Please comment these fully in Sphinx style
Will do.
>> +
>> +struct blkmap {
>> + struct udevice *dev;
>> + struct list_head slices;
>> +};
>> +
>> +static bool blkmap_slice_contains(struct blkmap_slice *bms, lbaint_t blknr)
>> +{
>> + return (blknr >= bms->blknr) && (blknr < (bms->blknr + bms->blkcnt));
>
> lots of brackets!
>
>> +}
>> +
>> +static bool blkmap_slice_available(struct blkmap *bm, struct blkmap_slice *new)
>> +{
>> + struct blkmap_slice *bms;
>> + lbaint_t first, last;
>> +
>> + first = new->blknr;
>> + last = new->blknr + new->blkcnt - 1;
>> +
>> + list_for_each_entry(bms, &bm->slices, node) {
>> + if (blkmap_slice_contains(bms, first) ||
>> + blkmap_slice_contains(bms, last) ||
>> + blkmap_slice_contains(new, bms->blknr) ||
>> + blkmap_slice_contains(new, bms->blknr + bms->blkcnt - 1))
>> + return false;
>> + }
>> +
>> + return true;
>> +}
>> +
>> +static struct blkmap *blkmap_from_devnum(int devnum)
>
> I don't really like the use of devnum everywhere. Can we instead limit
> the devnum stuff to the cmdline implementation, and use a struct
> udevice everywhere else?
Sure, I'll change it.
> The devum can be allocated when the UCLASS_BLK device is created.
>
>> +{
>> + struct udevice *dev;
>> + int err;
>> +
>> + err = blk_find_device(UCLASS_BLKMAP, devnum, &dev);
>> +
>> + return err ? NULL : dev_get_priv(dev);
>> +}
>> +
>> +static int blkmap_add(struct blkmap *bm, struct blkmap_slice *new)
>> +{
>> + struct blk_desc *bd = dev_get_uclass_plat(bm->dev);
>> + struct list_head *insert = &bm->slices;
>> + struct blkmap_slice *bms;
>> +
>> + if (!blkmap_slice_available(bm, new))
>> + return -EBUSY;
>> +
>> + list_for_each_entry(bms, &bm->slices, node) {
>> + if (bms->blknr < new->blknr)
>> + continue;
>> +
>> + insert = &bms->node;
>> + break;
>> + }
>> +
>> + list_add_tail(&new->node, insert);
>> +
>> + /* Disk might have grown, update the size */
>> + bms = list_last_entry(&bm->slices, struct blkmap_slice, node);
>> + bd->lba = bms->blknr + bms->blkcnt;
>> + return 0;
>> +}
>> +
>> +static struct udevice *blkmap_root(void)
>
> This needs to be created as part of DM. See how host_create_device()
> works. It attaches something to the uclass and then creates child
> devices from there. It also operations (struct host_ops) but you don't
> need to do that.
>
> Note that the host commands support either an label or a devnum, which
> I think is useful, so you might copy that?
>
I took a look at the hostfs implementation. I agree that labels are much
nicer than bare integers. However, for block maps the idea is to fit in
to the existing filesystem infrastructure. Addressing block devices
using the "<interface> <dev>[:<part>]" pattern seems very well
established...
>> +{
>> + static struct udevice *dev;
>> + int err;
>> +
>> + if (dev)
>> + return dev;
>> +
>> + err = device_bind_driver(dm_root(), "blkmap_root", "blkmap", &dev);
>> + if (err)
>> + return NULL;
>> +
>> + err = device_probe(dev);
>> + if (err) {
>> + device_unbind(dev);
>> + return NULL;
>> + }
>
> Should not be needed as probing children will cause this to be probed.
>
> So this function just becomes
>
> uclass_first_device(UCLASS_BLKDEV, &
>
>> +
>> + return dev;
>> +}
>> +
>> +int blkmap_create(int devnum)
>
> Again, please drop the use of devnum and use devices. Here you could
> use a label, perhaps?
...which is why I don't think a label is going to fly here. Let's say I
create a new ramdisk with a label instead, e.g.:
blkmap create rd
blkmap map rd 0 0x100 mem ${loadaddr}
How do I know which <dev> to supply to, e.g.:
ls blkmap <dev> /boot
It seems like labels are a hostfs-specific feature, or am I missing
something?
>> +{
>> + struct udevice *root;
>
> Please don't use that name , as we use it for the DM root device. How
> about bm_parent? It isn't actually a tree of devices, so root doesn't
> sound right to me anyway.
Alright, I'll change it.
>> + struct blk_desc *bd;
>> + struct blkmap *bm;
>> + int err;
>> +
>> + if (devnum >= 0 && blkmap_from_devnum(devnum))
>> + return -EBUSY;
>> +
>> + root = blkmap_root();
>> + if (!root)
>> + return -ENODEV;
>> +
>> + bm = calloc(1, sizeof(*bm));
>
> Can this be attached to the device as private data, so avoiding the malloc()?
Maybe, I'm not familiar enough with the U-Boot internals.
As it is now, all blkmaps are children of a single "blkmap_root"
device. I chose that approach so that devnums would be allocated from a
single pool.
AFAIK, that would mean having to store it in the "blkmap_blk" device,
but I thought that its private data was owned by the block subsystem?
>> + if (!bm)
>> + return -ENOMEM;
>> +
>> + err = blk_create_devicef(root, "blkmap_blk", "blk", UCLASS_BLKMAP,
>> + devnum, 512, 0, &bm->dev);
>> + if (err)
>> + goto err_free;
>> +
>> + bd = dev_get_uclass_plat(bm->dev);
>> +
>> + /* EFI core isn't keen on zero-sized disks, so we lie. This is
>> + * updated with the correct size once the user adds a
>> + * mapping.
>> + */
>> + bd->lba = 1;
>
> if (CONFIG_IS_ENABLED(EFI_LOADER))
>
> ?
Right.
>> +
>> + dev_set_priv(bm->dev, bm);
>> + INIT_LIST_HEAD(&bm->slices);
>> +
>> + err = blk_probe_or_unbind(bm->dev);
>> + if (err)
>> + goto err_remove;
>> +
>> + return bd->devnum;
>> +
>> +err_remove:
>> + device_remove(bm->dev, DM_REMOVE_NORMAL);
>> +err_free:
>> + free(bm);
>> + return err;
>> +}
>> +
>> +int blkmap_destroy(int devnum)
>
> label
>
>> +{
>> + struct blkmap_slice *bms, *tmp;
>> + struct blkmap *bm;
>> + int err;
>> +
>> + bm = blkmap_from_devnum(devnum);
>> + if (!bm)
>> + return -ENODEV;
>> +
>> + err = device_remove(bm->dev, DM_REMOVE_NORMAL);
>> + if (err)
>> + return err;
>> +
>> + err = device_unbind(bm->dev);
>> + if (err)
>> + return err;
>> +
>> + list_for_each_entry_safe(bms, tmp, &bm->slices, node) {
>> + list_del(&bms->node);
>> + free(bms);
>> + }
>> +
>> + free(bm);
>> + return 0;
>> +}
>> +
>> +static ulong blkmap_read_slice(struct blkmap *bm, struct blkmap_slice *bms,
>> + lbaint_t blknr, lbaint_t blkcnt, void *buffer)
>> +{
>> + lbaint_t nr, cnt;
>> +
>> + nr = blknr - bms->blknr;
>> + cnt = (blkcnt < bms->blkcnt) ? blkcnt : bms->blkcnt;
>> + return bms->read(bm, bms, nr, cnt, buffer);
>> +}
>> +
>> +static ulong blkmap_read(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>> + void *buffer)
>> +{
>> + struct blk_desc *bd = dev_get_uclass_plat(dev);
>> + struct blkmap *bm = dev_get_priv(dev);
>> + struct blkmap_slice *bms;
>> + lbaint_t cnt, total = 0;
>> +
>> + list_for_each_entry(bms, &bm->slices, node) {
>> + if (!blkmap_slice_contains(bms, blknr))
>> + continue;
>> +
>> + cnt = blkmap_read_slice(bm, bms, blknr, blkcnt, buffer);
>> + blknr += cnt;
>> + blkcnt -= cnt;
>> + buffer += cnt << bd->log2blksz;
>> + total += cnt;
>> + }
>> +
>> + return total;
>> +}
>> +
>> +static ulong blkmap_write_slice(struct blkmap *bm, struct blkmap_slice *bms,
>> + lbaint_t blknr, lbaint_t blkcnt,
>> + const void *buffer)
>> +{
>> + lbaint_t nr, cnt;
>> +
>> + nr = blknr - bms->blknr;
>> + cnt = (blkcnt < bms->blkcnt) ? blkcnt : bms->blkcnt;
>> + return bms->write(bm, bms, nr, cnt, buffer);
>> +}
>> +
>> +static ulong blkmap_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>> + const void *buffer)
>> +{
>> + struct blk_desc *bd = dev_get_uclass_plat(dev);
>> + struct blkmap *bm = dev_get_priv(dev);
>> + struct blkmap_slice *bms;
>> + lbaint_t cnt, total = 0;
>> +
>> + list_for_each_entry(bms, &bm->slices, node) {
>> + if (!blkmap_slice_contains(bms, blknr))
>> + continue;
>> +
>> + cnt = blkmap_write_slice(bm, bms, blknr, blkcnt, buffer);
>> + blknr += cnt;
>> + blkcnt -= cnt;
>> + buffer += cnt << bd->log2blksz;
>> + total += cnt;
>> + }
>> +
>> + return total;
>> +}
>> +
>> +static const struct blk_ops blkmap_ops = {
>> + .read = blkmap_read,
>> + .write = blkmap_write,
>> +};
>> +
>> +U_BOOT_DRIVER(blkmap_blk) = {
>> + .name = "blkmap_blk",
>> + .id = UCLASS_BLK,
>> + .ops = &blkmap_ops,
>> +};
>> +
>> +U_BOOT_DRIVER(blkmap_root) = {
>> + .name = "blkmap_root",
>> + .id = UCLASS_BLKMAP,
>> +};
>> +
>> +UCLASS_DRIVER(blkmap) = {
>> + .id = UCLASS_BLKMAP,
>> + .name = "blkmap",
>> +};
>> diff --git a/include/blkmap.h b/include/blkmap.h
>> new file mode 100644
>> index 0000000000..37c0c31c3f
>> --- /dev/null
>> +++ b/include/blkmap.h
>> @@ -0,0 +1,15 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ */
>> +/*
>> + * Copyright (c) 2023 Addiva Elektronik
>> + * Author: Tobias Waldekranz <tobias@waldekranz.com>
>> + */
>> +
>> +#ifndef _BLKMAP_H
>> +#define _BLKMAP_H
>> +
>> +#include <stdbool.h>
>> +
>> +int blkmap_create(int devnum);
>> +int blkmap_destroy(int devnum);
>
> full comments for exported functions
Fixing.
>> +
>> +#endif /* _BLKMAP_H */
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>> index 33e43c20db..576237b954 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -37,6 +37,7 @@ enum uclass_id {
>> UCLASS_AUDIO_CODEC, /* Audio codec with control and data path */
>> UCLASS_AXI, /* AXI bus */
>> UCLASS_BLK, /* Block device */
>> + UCLASS_BLKMAP, /* Composable virtual block device */
>> UCLASS_BOOTCOUNT, /* Bootcount backing store */
>> UCLASS_BOOTDEV, /* Boot device for locating an OS to boot */
>> UCLASS_BOOTMETH, /* Bootmethod for booting an OS */
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index 4560b0d04c..59687f44de 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -134,6 +134,10 @@ static inline efi_status_t efi_launch_capsules(void)
>> #define U_BOOT_GUID \
>> EFI_GUID(0xe61d73b9, 0xa384, 0x4acc, \
>> 0xae, 0xab, 0x82, 0xe8, 0x28, 0xf3, 0x62, 0x8b)
>> +/* GUID used as root for blkmap devices */
>> +#define U_BOOT_BLKMAP_DEV_GUID \
>> + EFI_GUID(0x4cad859d, 0xd644, 0x42ff, \
>> + 0x87, 0x0b, 0xc0, 0x2e, 0xac, 0x05, 0x58, 0x63)
>> /* GUID used as host device on sandbox */
>> #define U_BOOT_HOST_DEV_GUID \
>> EFI_GUID(0xbbe4e671, 0x5773, 0x4ea1, \
>> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
>> index 3b267b713e..4b4c96bc2e 100644
>> --- a/lib/efi_loader/efi_device_path.c
>> +++ b/lib/efi_loader/efi_device_path.c
>
> Please put this EFI stuff in a separate patch.
Will do.
>> @@ -21,6 +21,9 @@
>> #include <asm-generic/unaligned.h>
>> #include <linux/compat.h> /* U16_MAX */
>>
>> +#ifdef CONFIG_BLKMAP
>> +const efi_guid_t efi_guid_blkmap_dev = U_BOOT_BLKMAP_DEV_GUID;
>> +#endif
>> #ifdef CONFIG_SANDBOX
>> const efi_guid_t efi_guid_host_dev = U_BOOT_HOST_DEV_GUID;
>> #endif
>> @@ -573,6 +576,16 @@ __maybe_unused static unsigned int dp_size(struct udevice *dev)
>> */
>> return dp_size(dev->parent)
>> + sizeof(struct efi_device_path_vendor) + 1;
>> +#endif
>> +#ifdef CONFIG_BLKMAP
>> + case UCLASS_BLKMAP:
>> + /*
>> + * blkmap devices will be represented as a vendor
>> + * device node with an extra byte for the device
>> + * number.
>> + */
>> + return dp_size(dev->parent)
>> + + sizeof(struct efi_device_path_vendor) + 1;
>> #endif
>> default:
>> return dp_size(dev->parent);
>> @@ -631,6 +644,23 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev)
>> #endif
>> case UCLASS_BLK:
>> switch (dev->parent->uclass->uc_drv->id) {
>> +#ifdef CONFIG_BLKMAP
>> + case UCLASS_BLKMAP: {
>> + struct efi_device_path_vendor *dp;
>> + struct blk_desc *desc = dev_get_uclass_plat(dev);
>> +
>> + dp_fill(buf, dev->parent);
>> + dp = buf;
>> + ++dp;
>> + dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
>> + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
>> + dp->dp.length = sizeof(*dp) + 1;
>> + memcpy(&dp->guid, &efi_guid_blkmap_dev,
>> + sizeof(efi_guid_t));
>> + dp->vendor_data[0] = desc->devnum;
>> + return &dp->vendor_data[1];
>> + }
>> +#endif
>> #ifdef CONFIG_SANDBOX
>> case UCLASS_HOST: {
>> /* stop traversing parents at this point: */
>> --
>> 2.34.1
>>
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/8] blk: blkmap: Add basic infrastructure
2023-02-03 9:38 ` Tobias Waldekranz
@ 2023-02-04 0:20 ` Simon Glass
2023-02-06 8:30 ` Tobias Waldekranz
0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2023-02-04 0:20 UTC (permalink / raw)
To: Tobias Waldekranz; +Cc: xypron.glpk, ilias.apalodimas, u-boot
Hi Tobias,
On Fri, 3 Feb 2023 at 02:38, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>
> On ons, feb 01, 2023 at 13:20, Simon Glass <sjg@chromium.org> wrote:
> > Hi Tobias,
>
> Hi Simon,
>
> Thanks for the review!
>
> > On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias@waldekranz.com> wrote:
> >>
> >> blkmaps are loosely modeled on Linux's device mapper subsystem. The
> >> basic idea is that you can create virtual block devices whose blocks
> >> can be backed by a plethora of sources that are user configurable.
> >>
> >> This change just adds the basic infrastructure for creating and
> >> removing blkmap devices. Subsequent changes will extend this to add
> >> support for actual mappings.
> >>
> >> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> >> ---
> >> MAINTAINERS | 6 +
> >> disk/part.c | 1 +
> >> drivers/block/Kconfig | 18 ++
> >> drivers/block/Makefile | 1 +
> >> drivers/block/blk-uclass.c | 1 +
> >> drivers/block/blkmap.c | 275 +++++++++++++++++++++++++++++++
> >> include/blkmap.h | 15 ++
> >> include/dm/uclass-id.h | 1 +
> >> include/efi_loader.h | 4 +
> >> lib/efi_loader/efi_device_path.c | 30 ++++
> >> 10 files changed, 352 insertions(+)
> >> create mode 100644 drivers/block/blkmap.c
> >> create mode 100644 include/blkmap.h
> >>
[..]
> > This needs to be created as part of DM. See how host_create_device()
> > works. It attaches something to the uclass and then creates child
> > devices from there. It also operations (struct host_ops) but you don't
> > need to do that.
> >
> > Note that the host commands support either an label or a devnum, which
> > I think is useful, so you might copy that?
> >
>
> I took a look at the hostfs implementation. I agree that labels are much
> nicer than bare integers. However, for block maps the idea is to fit in
> to the existing filesystem infrastructure. Addressing block devices
> using the "<interface> <dev>[:<part>]" pattern seems very well
> established...
You can still do that, so long as the labels are "0" and "1", etc. But
it lets us move to a more flexible system in future.
>
> >> +{
> >> + static struct udevice *dev;
> >> + int err;
> >> +
> >> + if (dev)
> >> + return dev;
> >> +
> >> + err = device_bind_driver(dm_root(), "blkmap_root", "blkmap", &dev);
> >> + if (err)
> >> + return NULL;
> >> +
> >> + err = device_probe(dev);
> >> + if (err) {
> >> + device_unbind(dev);
> >> + return NULL;
> >> + }
> >
> > Should not be needed as probing children will cause this to be probed.
> >
> > So this function just becomes
> >
> > uclass_first_device(UCLASS_BLKDEV, &
> >
> >> +
> >> + return dev;
> >> +}
> >> +
> >> +int blkmap_create(int devnum)
> >
> > Again, please drop the use of devnum and use devices. Here you could
> > use a label, perhaps?
>
> ...which is why I don't think a label is going to fly here. Let's say I
> create a new ramdisk with a label instead, e.g.:
>
> blkmap create rd
> blkmap map rd 0 0x100 mem ${loadaddr}
>
> How do I know which <dev> to supply to, e.g.:
>
> ls blkmap <dev> /boot
>
> It seems like labels are a hostfs-specific feature, or am I missing
> something?
We have the same problem with hostfs, since we have not implemented
labels in block devices. For now you must use integer labels. But we
will get there.
>
> >> +{
> >> + struct udevice *root;
> >
> > Please don't use that name , as we use it for the DM root device. How
> > about bm_parent? It isn't actually a tree of devices, so root doesn't
> > sound right to me anyway.
>
> Alright, I'll change it.
>
> >> + struct blk_desc *bd;
> >> + struct blkmap *bm;
> >> + int err;
> >> +
> >> + if (devnum >= 0 && blkmap_from_devnum(devnum))
> >> + return -EBUSY;
> >> +
> >> + root = blkmap_root();
> >> + if (!root)
> >> + return -ENODEV;
> >> +
> >> + bm = calloc(1, sizeof(*bm));
> >
> > Can this be attached to the device as private data, so avoiding the malloc()?
>
> Maybe, I'm not familiar enough with the U-Boot internals.
>
> As it is now, all blkmaps are children of a single "blkmap_root"
> device. I chose that approach so that devnums would be allocated from a
> single pool.
Well, driver model handles this for you (see dev_seq()). You have a
single uclass so can attach your 'overall' blkmap data to that. Then
each device can have its own private data attached.
The only requirement is that BLK devices have a parent (so we know the
media type). I had understood that you had one blkmap for each block
map. If that is true, then you don't need to have a parent one as
well. You can use the BLKMAP uclass to hold any overall data.
>
> AFAIK, that would mean having to store it in the "blkmap_blk" device,
> but I thought that its private data was owned by the block subsystem?
[..]
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/8] blk: blkmap: Add basic infrastructure
2023-02-04 0:20 ` Simon Glass
@ 2023-02-06 8:30 ` Tobias Waldekranz
2023-02-07 4:02 ` Simon Glass
0 siblings, 1 reply; 25+ messages in thread
From: Tobias Waldekranz @ 2023-02-06 8:30 UTC (permalink / raw)
To: Simon Glass; +Cc: xypron.glpk, ilias.apalodimas, u-boot
On fre, feb 03, 2023 at 17:20, Simon Glass <sjg@chromium.org> wrote:
> Hi Tobias,
>
> On Fri, 3 Feb 2023 at 02:38, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>>
>> On ons, feb 01, 2023 at 13:20, Simon Glass <sjg@chromium.org> wrote:
>> > Hi Tobias,
>>
>> Hi Simon,
>>
>> Thanks for the review!
>>
>> > On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>> >>
>> >> blkmaps are loosely modeled on Linux's device mapper subsystem. The
>> >> basic idea is that you can create virtual block devices whose blocks
>> >> can be backed by a plethora of sources that are user configurable.
>> >>
>> >> This change just adds the basic infrastructure for creating and
>> >> removing blkmap devices. Subsequent changes will extend this to add
>> >> support for actual mappings.
>> >>
>> >> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
>> >> ---
>> >> MAINTAINERS | 6 +
>> >> disk/part.c | 1 +
>> >> drivers/block/Kconfig | 18 ++
>> >> drivers/block/Makefile | 1 +
>> >> drivers/block/blk-uclass.c | 1 +
>> >> drivers/block/blkmap.c | 275 +++++++++++++++++++++++++++++++
>> >> include/blkmap.h | 15 ++
>> >> include/dm/uclass-id.h | 1 +
>> >> include/efi_loader.h | 4 +
>> >> lib/efi_loader/efi_device_path.c | 30 ++++
>> >> 10 files changed, 352 insertions(+)
>> >> create mode 100644 drivers/block/blkmap.c
>> >> create mode 100644 include/blkmap.h
>> >>
>
> [..]
>
>> > This needs to be created as part of DM. See how host_create_device()
>> > works. It attaches something to the uclass and then creates child
>> > devices from there. It also operations (struct host_ops) but you don't
>> > need to do that.
>> >
>> > Note that the host commands support either an label or a devnum, which
>> > I think is useful, so you might copy that?
>> >
>>
>> I took a look at the hostfs implementation. I agree that labels are much
>> nicer than bare integers. However, for block maps the idea is to fit in
>> to the existing filesystem infrastructure. Addressing block devices
>> using the "<interface> <dev>[:<part>]" pattern seems very well
>> established...
>
> You can still do that, so long as the labels are "0" and "1", etc. But
> it lets us move to a more flexible system in future.
>
>>
>> >> +{
>> >> + static struct udevice *dev;
>> >> + int err;
>> >> +
>> >> + if (dev)
>> >> + return dev;
>> >> +
>> >> + err = device_bind_driver(dm_root(), "blkmap_root", "blkmap", &dev);
>> >> + if (err)
>> >> + return NULL;
>> >> +
>> >> + err = device_probe(dev);
>> >> + if (err) {
>> >> + device_unbind(dev);
>> >> + return NULL;
>> >> + }
>> >
>> > Should not be needed as probing children will cause this to be probed.
>> >
>> > So this function just becomes
>> >
>> > uclass_first_device(UCLASS_BLKDEV, &
>> >
>> >> +
>> >> + return dev;
>> >> +}
>> >> +
>> >> +int blkmap_create(int devnum)
>> >
>> > Again, please drop the use of devnum and use devices. Here you could
>> > use a label, perhaps?
>>
>> ...which is why I don't think a label is going to fly here. Let's say I
>> create a new ramdisk with a label instead, e.g.:
>>
>> blkmap create rd
>> blkmap map rd 0 0x100 mem ${loadaddr}
>>
>> How do I know which <dev> to supply to, e.g.:
>>
>> ls blkmap <dev> /boot
>>
>> It seems like labels are a hostfs-specific feature, or am I missing
>> something?
>
> We have the same problem with hostfs, since we have not implemented
> labels in block devices. For now you must use integer labels. But we
> will get there.
But there is no connection to the devnum that is allocated internally by
U-Boot. Here's an experiment I just ran:
I created two squashfs images containing a single directory:
zero.squashfs:
i_am_zero
one.squashfs:
i_am_one
Then I added a binding to them:
=> host bind 1 one.squashfs
=> host bind 0 zero.squashfs
When accessing them, we see that the existing filesystem utilities work
on the internally generated devnums, ignoring the labels:
=> ls host 1
i_am_zero/
0 file(s), 1 dir(s)
=> ls host 0
i_am_one/
0 file(s), 1 dir(s)
=>
Doesn't it therefore make more sense to stick with the established
abstraction?
>>
>> >> +{
>> >> + struct udevice *root;
>> >
>> > Please don't use that name , as we use it for the DM root device. How
>> > about bm_parent? It isn't actually a tree of devices, so root doesn't
>> > sound right to me anyway.
>>
>> Alright, I'll change it.
>>
>> >> + struct blk_desc *bd;
>> >> + struct blkmap *bm;
>> >> + int err;
>> >> +
>> >> + if (devnum >= 0 && blkmap_from_devnum(devnum))
>> >> + return -EBUSY;
>> >> +
>> >> + root = blkmap_root();
>> >> + if (!root)
>> >> + return -ENODEV;
>> >> +
>> >> + bm = calloc(1, sizeof(*bm));
>> >
>> > Can this be attached to the device as private data, so avoiding the malloc()?
>>
>> Maybe, I'm not familiar enough with the U-Boot internals.
>>
>> As it is now, all blkmaps are children of a single "blkmap_root"
>> device. I chose that approach so that devnums would be allocated from a
>> single pool.
>
> Well, driver model handles this for you (see dev_seq()). You have a
> single uclass so can attach your 'overall' blkmap data to that. Then
> each device can have its own private data attached.
>
> The only requirement is that BLK devices have a parent (so we know the
> media type). I had understood that you had one blkmap for each block
> map. If that is true, then you don't need to have a parent one as
> well. You can use the BLKMAP uclass to hold any overall data.
>
>>
>> AFAIK, that would mean having to store it in the "blkmap_blk" device,
>> but I thought that its private data was owned by the block subsystem?
>
> [..]
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/8] blk: blkmap: Add basic infrastructure
2023-02-06 8:30 ` Tobias Waldekranz
@ 2023-02-07 4:02 ` Simon Glass
2023-02-07 8:31 ` Tobias Waldekranz
0 siblings, 1 reply; 25+ messages in thread
From: Simon Glass @ 2023-02-07 4:02 UTC (permalink / raw)
To: Tobias Waldekranz; +Cc: xypron.glpk, ilias.apalodimas, u-boot
Hi Tobias,
On Mon, 6 Feb 2023 at 01:30, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>
> On fre, feb 03, 2023 at 17:20, Simon Glass <sjg@chromium.org> wrote:
> > Hi Tobias,
> >
> > On Fri, 3 Feb 2023 at 02:38, Tobias Waldekranz <tobias@waldekranz.com> wrote:
> >>
> >> On ons, feb 01, 2023 at 13:20, Simon Glass <sjg@chromium.org> wrote:
> >> > Hi Tobias,
> >>
> >> Hi Simon,
> >>
> >> Thanks for the review!
> >>
> >> > On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias@waldekranz.com> wrote:
> >> >>
> >> >> blkmaps are loosely modeled on Linux's device mapper subsystem. The
> >> >> basic idea is that you can create virtual block devices whose blocks
> >> >> can be backed by a plethora of sources that are user configurable.
> >> >>
> >> >> This change just adds the basic infrastructure for creating and
> >> >> removing blkmap devices. Subsequent changes will extend this to add
> >> >> support for actual mappings.
> >> >>
> >> >> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> >> >> ---
> >> >> MAINTAINERS | 6 +
> >> >> disk/part.c | 1 +
> >> >> drivers/block/Kconfig | 18 ++
> >> >> drivers/block/Makefile | 1 +
> >> >> drivers/block/blk-uclass.c | 1 +
> >> >> drivers/block/blkmap.c | 275 +++++++++++++++++++++++++++++++
> >> >> include/blkmap.h | 15 ++
> >> >> include/dm/uclass-id.h | 1 +
> >> >> include/efi_loader.h | 4 +
> >> >> lib/efi_loader/efi_device_path.c | 30 ++++
> >> >> 10 files changed, 352 insertions(+)
> >> >> create mode 100644 drivers/block/blkmap.c
> >> >> create mode 100644 include/blkmap.h
> >> >>
> >
> > [..]
> >
> >> > This needs to be created as part of DM. See how host_create_device()
> >> > works. It attaches something to the uclass and then creates child
> >> > devices from there. It also operations (struct host_ops) but you don't
> >> > need to do that.
> >> >
> >> > Note that the host commands support either an label or a devnum, which
> >> > I think is useful, so you might copy that?
> >> >
> >>
> >> I took a look at the hostfs implementation. I agree that labels are much
> >> nicer than bare integers. However, for block maps the idea is to fit in
> >> to the existing filesystem infrastructure. Addressing block devices
> >> using the "<interface> <dev>[:<part>]" pattern seems very well
> >> established...
> >
> > You can still do that, so long as the labels are "0" and "1", etc. But
> > it lets us move to a more flexible system in future.
> >
> >>
> >> >> +{
> >> >> + static struct udevice *dev;
> >> >> + int err;
> >> >> +
> >> >> + if (dev)
> >> >> + return dev;
> >> >> +
> >> >> + err = device_bind_driver(dm_root(), "blkmap_root", "blkmap", &dev);
> >> >> + if (err)
> >> >> + return NULL;
> >> >> +
> >> >> + err = device_probe(dev);
> >> >> + if (err) {
> >> >> + device_unbind(dev);
> >> >> + return NULL;
> >> >> + }
> >> >
> >> > Should not be needed as probing children will cause this to be probed.
> >> >
> >> > So this function just becomes
> >> >
> >> > uclass_first_device(UCLASS_BLKDEV, &
> >> >
> >> >> +
> >> >> + return dev;
> >> >> +}
> >> >> +
> >> >> +int blkmap_create(int devnum)
> >> >
> >> > Again, please drop the use of devnum and use devices. Here you could
> >> > use a label, perhaps?
> >>
> >> ...which is why I don't think a label is going to fly here. Let's say I
> >> create a new ramdisk with a label instead, e.g.:
> >>
> >> blkmap create rd
> >> blkmap map rd 0 0x100 mem ${loadaddr}
> >>
> >> How do I know which <dev> to supply to, e.g.:
> >>
> >> ls blkmap <dev> /boot
> >>
> >> It seems like labels are a hostfs-specific feature, or am I missing
> >> something?
> >
> > We have the same problem with hostfs, since we have not implemented
> > labels in block devices. For now you must use integer labels. But we
> > will get there.
>
> But there is no connection to the devnum that is allocated internally by
> U-Boot. Here's an experiment I just ran:
>
> I created two squashfs images containing a single directory:
>
> zero.squashfs:
> i_am_zero
>
> one.squashfs:
> i_am_one
>
> Then I added a binding to them:
>
> => host bind 1 one.squashfs
> => host bind 0 zero.squashfs
>
> When accessing them, we see that the existing filesystem utilities work
> on the internally generated devnums, ignoring the labels:
>
> => ls host 1
> i_am_zero/
>
> 0 file(s), 1 dir(s)
>
> => ls host 0
> i_am_one/
>
> 0 file(s), 1 dir(s)
>
> =>
>
> Doesn't it therefore make more sense to stick with the established
> abstraction?
It is pretty clear that this is a bit silly though :-)
I mean, right now, it would be easier to stick with numbered devices.
But we want to be able to support named devices (e.g. using struct
udevice->name). A good way to be forward compatible is to support a
label today.
When we do get to it, the less code that has piled up and needs
converting, the more likely it is to happen.
Sure, you have the problem as above, but mostly people are only going
to use one of them, so it doesn't matter.
We could also have a way of obtaining a number from a label, if you
want to go that far. But I suggest just telling people to use labels
like "1" and "0" which should work.
[.]
Regards,
SImon
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/8] blk: blkmap: Add basic infrastructure
2023-02-07 4:02 ` Simon Glass
@ 2023-02-07 8:31 ` Tobias Waldekranz
2023-02-07 13:38 ` Simon Glass
0 siblings, 1 reply; 25+ messages in thread
From: Tobias Waldekranz @ 2023-02-07 8:31 UTC (permalink / raw)
To: Simon Glass; +Cc: xypron.glpk, ilias.apalodimas, u-boot
On mån, feb 06, 2023 at 21:02, Simon Glass <sjg@chromium.org> wrote:
> Hi Tobias,
>
> On Mon, 6 Feb 2023 at 01:30, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>>
>> On fre, feb 03, 2023 at 17:20, Simon Glass <sjg@chromium.org> wrote:
>> > Hi Tobias,
>> >
>> > On Fri, 3 Feb 2023 at 02:38, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>> >>
>> >> On ons, feb 01, 2023 at 13:20, Simon Glass <sjg@chromium.org> wrote:
>> >> > Hi Tobias,
>> >>
>> >> Hi Simon,
>> >>
>> >> Thanks for the review!
>> >>
>> >> > On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>> >> >>
>> >> >> blkmaps are loosely modeled on Linux's device mapper subsystem. The
>> >> >> basic idea is that you can create virtual block devices whose blocks
>> >> >> can be backed by a plethora of sources that are user configurable.
>> >> >>
>> >> >> This change just adds the basic infrastructure for creating and
>> >> >> removing blkmap devices. Subsequent changes will extend this to add
>> >> >> support for actual mappings.
>> >> >>
>> >> >> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
>> >> >> ---
>> >> >> MAINTAINERS | 6 +
>> >> >> disk/part.c | 1 +
>> >> >> drivers/block/Kconfig | 18 ++
>> >> >> drivers/block/Makefile | 1 +
>> >> >> drivers/block/blk-uclass.c | 1 +
>> >> >> drivers/block/blkmap.c | 275 +++++++++++++++++++++++++++++++
>> >> >> include/blkmap.h | 15 ++
>> >> >> include/dm/uclass-id.h | 1 +
>> >> >> include/efi_loader.h | 4 +
>> >> >> lib/efi_loader/efi_device_path.c | 30 ++++
>> >> >> 10 files changed, 352 insertions(+)
>> >> >> create mode 100644 drivers/block/blkmap.c
>> >> >> create mode 100644 include/blkmap.h
>> >> >>
>> >
>> > [..]
>> >
>> >> > This needs to be created as part of DM. See how host_create_device()
>> >> > works. It attaches something to the uclass and then creates child
>> >> > devices from there. It also operations (struct host_ops) but you don't
>> >> > need to do that.
>> >> >
>> >> > Note that the host commands support either an label or a devnum, which
>> >> > I think is useful, so you might copy that?
>> >> >
>> >>
>> >> I took a look at the hostfs implementation. I agree that labels are much
>> >> nicer than bare integers. However, for block maps the idea is to fit in
>> >> to the existing filesystem infrastructure. Addressing block devices
>> >> using the "<interface> <dev>[:<part>]" pattern seems very well
>> >> established...
>> >
>> > You can still do that, so long as the labels are "0" and "1", etc. But
>> > it lets us move to a more flexible system in future.
>> >
>> >>
>> >> >> +{
>> >> >> + static struct udevice *dev;
>> >> >> + int err;
>> >> >> +
>> >> >> + if (dev)
>> >> >> + return dev;
>> >> >> +
>> >> >> + err = device_bind_driver(dm_root(), "blkmap_root", "blkmap", &dev);
>> >> >> + if (err)
>> >> >> + return NULL;
>> >> >> +
>> >> >> + err = device_probe(dev);
>> >> >> + if (err) {
>> >> >> + device_unbind(dev);
>> >> >> + return NULL;
>> >> >> + }
>> >> >
>> >> > Should not be needed as probing children will cause this to be probed.
>> >> >
>> >> > So this function just becomes
>> >> >
>> >> > uclass_first_device(UCLASS_BLKDEV, &
>> >> >
>> >> >> +
>> >> >> + return dev;
>> >> >> +}
>> >> >> +
>> >> >> +int blkmap_create(int devnum)
>> >> >
>> >> > Again, please drop the use of devnum and use devices. Here you could
>> >> > use a label, perhaps?
>> >>
>> >> ...which is why I don't think a label is going to fly here. Let's say I
>> >> create a new ramdisk with a label instead, e.g.:
>> >>
>> >> blkmap create rd
>> >> blkmap map rd 0 0x100 mem ${loadaddr}
>> >>
>> >> How do I know which <dev> to supply to, e.g.:
>> >>
>> >> ls blkmap <dev> /boot
>> >>
>> >> It seems like labels are a hostfs-specific feature, or am I missing
>> >> something?
>> >
>> > We have the same problem with hostfs, since we have not implemented
>> > labels in block devices. For now you must use integer labels. But we
>> > will get there.
>>
>> But there is no connection to the devnum that is allocated internally by
>> U-Boot. Here's an experiment I just ran:
>>
>> I created two squashfs images containing a single directory:
>>
>> zero.squashfs:
>> i_am_zero
>>
>> one.squashfs:
>> i_am_one
>>
>> Then I added a binding to them:
>>
>> => host bind 1 one.squashfs
>> => host bind 0 zero.squashfs
>>
>> When accessing them, we see that the existing filesystem utilities work
>> on the internally generated devnums, ignoring the labels:
>>
>> => ls host 1
>> i_am_zero/
>>
>> 0 file(s), 1 dir(s)
>>
>> => ls host 0
>> i_am_one/
>>
>> 0 file(s), 1 dir(s)
>>
>> =>
>>
>> Doesn't it therefore make more sense to stick with the established
>> abstraction?
>
> It is pretty clear that this is a bit silly though :-)
As in "this specific example will never be used in practice"? Sure :)
My point was just that the approach to stick to integer labels is
brittle, since there is no connection between the label and the devnum
used by existing commands.
Here's an even simpler example that might actually trip up a user:
=> host bind 1 one.squashfs
=> ls host 1
** Bad device specification host 1 **
Couldn't find partition host 1
=> ls host 0
i_am_one/
0 file(s), 1 dir(s)
=>
> I mean, right now, it would be easier to stick with numbered devices.
> But we want to be able to support named devices (e.g. using struct
> udevice->name). A good way to be forward compatible is to support a
> label today.
>
> When we do get to it, the less code that has piled up and needs
> converting, the more likely it is to happen.
I completely understand, and agree with, the direction you want to take
this.
> Sure, you have the problem as above, but mostly people are only going
> to use one of them, so it doesn't matter.
>
> We could also have a way of obtaining a number from a label, if you
> want to go that far. But I suggest just telling people to use labels
> like "1" and "0" which should work.
Alright, well, if that is acceptable then I'll do it that way. For my
own piece of mind, I think I'll also add some way of safely doing the
reverse mapping for any label. Does the following look ok?
blkmap get <label> dev <var>
This way you could extend it with other attributes in the future
(e.g. size).
^ permalink raw reply [flat|nested] 25+ messages in thread* Re: [PATCH 3/8] blk: blkmap: Add basic infrastructure
2023-02-07 8:31 ` Tobias Waldekranz
@ 2023-02-07 13:38 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2023-02-07 13:38 UTC (permalink / raw)
To: Tobias Waldekranz; +Cc: xypron.glpk, ilias.apalodimas, u-boot
Hi Tobias,
On Tue, 7 Feb 2023 at 01:31, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>
> On mån, feb 06, 2023 at 21:02, Simon Glass <sjg@chromium.org> wrote:
> > Hi Tobias,
> >
> > On Mon, 6 Feb 2023 at 01:30, Tobias Waldekranz <tobias@waldekranz.com> wrote:
> >>
> >> On fre, feb 03, 2023 at 17:20, Simon Glass <sjg@chromium.org> wrote:
> >> > Hi Tobias,
> >> >
> >> > On Fri, 3 Feb 2023 at 02:38, Tobias Waldekranz <tobias@waldekranz.com> wrote:
> >> >>
> >> >> On ons, feb 01, 2023 at 13:20, Simon Glass <sjg@chromium.org> wrote:
> >> >> > Hi Tobias,
> >> >>
> >> >> Hi Simon,
> >> >>
> >> >> Thanks for the review!
> >> >>
> >> >> > On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias@waldekranz.com> wrote:
> >> >> >>
> >> >> >> blkmaps are loosely modeled on Linux's device mapper subsystem. The
> >> >> >> basic idea is that you can create virtual block devices whose blocks
> >> >> >> can be backed by a plethora of sources that are user configurable.
> >> >> >>
> >> >> >> This change just adds the basic infrastructure for creating and
> >> >> >> removing blkmap devices. Subsequent changes will extend this to add
> >> >> >> support for actual mappings.
> >> >> >>
> >> >> >> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> >> >> >> ---
> >> >> >> MAINTAINERS | 6 +
> >> >> >> disk/part.c | 1 +
> >> >> >> drivers/block/Kconfig | 18 ++
> >> >> >> drivers/block/Makefile | 1 +
> >> >> >> drivers/block/blk-uclass.c | 1 +
> >> >> >> drivers/block/blkmap.c | 275 +++++++++++++++++++++++++++++++
> >> >> >> include/blkmap.h | 15 ++
> >> >> >> include/dm/uclass-id.h | 1 +
> >> >> >> include/efi_loader.h | 4 +
> >> >> >> lib/efi_loader/efi_device_path.c | 30 ++++
> >> >> >> 10 files changed, 352 insertions(+)
> >> >> >> create mode 100644 drivers/block/blkmap.c
> >> >> >> create mode 100644 include/blkmap.h
> >> >> >>
> >> >
> >> > [..]
> >> >
> >> >> > This needs to be created as part of DM. See how host_create_device()
> >> >> > works. It attaches something to the uclass and then creates child
> >> >> > devices from there. It also operations (struct host_ops) but you don't
> >> >> > need to do that.
> >> >> >
> >> >> > Note that the host commands support either an label or a devnum, which
> >> >> > I think is useful, so you might copy that?
> >> >> >
> >> >>
> >> >> I took a look at the hostfs implementation. I agree that labels are much
> >> >> nicer than bare integers. However, for block maps the idea is to fit in
> >> >> to the existing filesystem infrastructure. Addressing block devices
> >> >> using the "<interface> <dev>[:<part>]" pattern seems very well
> >> >> established...
> >> >
> >> > You can still do that, so long as the labels are "0" and "1", etc. But
> >> > it lets us move to a more flexible system in future.
> >> >
> >> >>
> >> >> >> +{
> >> >> >> + static struct udevice *dev;
> >> >> >> + int err;
> >> >> >> +
> >> >> >> + if (dev)
> >> >> >> + return dev;
> >> >> >> +
> >> >> >> + err = device_bind_driver(dm_root(), "blkmap_root", "blkmap", &dev);
> >> >> >> + if (err)
> >> >> >> + return NULL;
> >> >> >> +
> >> >> >> + err = device_probe(dev);
> >> >> >> + if (err) {
> >> >> >> + device_unbind(dev);
> >> >> >> + return NULL;
> >> >> >> + }
> >> >> >
> >> >> > Should not be needed as probing children will cause this to be probed.
> >> >> >
> >> >> > So this function just becomes
> >> >> >
> >> >> > uclass_first_device(UCLASS_BLKDEV, &
> >> >> >
> >> >> >> +
> >> >> >> + return dev;
> >> >> >> +}
> >> >> >> +
> >> >> >> +int blkmap_create(int devnum)
> >> >> >
> >> >> > Again, please drop the use of devnum and use devices. Here you could
> >> >> > use a label, perhaps?
> >> >>
> >> >> ...which is why I don't think a label is going to fly here. Let's say I
> >> >> create a new ramdisk with a label instead, e.g.:
> >> >>
> >> >> blkmap create rd
> >> >> blkmap map rd 0 0x100 mem ${loadaddr}
> >> >>
> >> >> How do I know which <dev> to supply to, e.g.:
> >> >>
> >> >> ls blkmap <dev> /boot
> >> >>
> >> >> It seems like labels are a hostfs-specific feature, or am I missing
> >> >> something?
> >> >
> >> > We have the same problem with hostfs, since we have not implemented
> >> > labels in block devices. For now you must use integer labels. But we
> >> > will get there.
> >>
> >> But there is no connection to the devnum that is allocated internally by
> >> U-Boot. Here's an experiment I just ran:
> >>
> >> I created two squashfs images containing a single directory:
> >>
> >> zero.squashfs:
> >> i_am_zero
> >>
> >> one.squashfs:
> >> i_am_one
> >>
> >> Then I added a binding to them:
> >>
> >> => host bind 1 one.squashfs
> >> => host bind 0 zero.squashfs
> >>
> >> When accessing them, we see that the existing filesystem utilities work
> >> on the internally generated devnums, ignoring the labels:
> >>
> >> => ls host 1
> >> i_am_zero/
> >>
> >> 0 file(s), 1 dir(s)
> >>
> >> => ls host 0
> >> i_am_one/
> >>
> >> 0 file(s), 1 dir(s)
> >>
> >> =>
> >>
> >> Doesn't it therefore make more sense to stick with the established
> >> abstraction?
> >
> > It is pretty clear that this is a bit silly though :-)
>
> As in "this specific example will never be used in practice"? Sure :)
>
> My point was just that the approach to stick to integer labels is
> brittle, since there is no connection between the label and the devnum
> used by existing commands.
>
> Here's an even simpler example that might actually trip up a user:
>
> => host bind 1 one.squashfs
> => ls host 1
> ** Bad device specification host 1 **
> Couldn't find partition host 1
> => ls host 0
> i_am_one/
>
> 0 file(s), 1 dir(s)
>
> =>
Yes, I get it. Perhaps this will spur us to look at device
naming...one of the impediments has been that we don't use CONFIG_BLK
in SPL on quite a few boards, so there are effectively two
implementations, one of which does not use driver model. So naming
doesn't exist in that case. It is hard to require driver model in SPL,
but perhaps at some point we can require it if block devices are
needed.
>
> > I mean, right now, it would be easier to stick with numbered devices.
> > But we want to be able to support named devices (e.g. using struct
> > udevice->name). A good way to be forward compatible is to support a
> > label today.
> >
> > When we do get to it, the less code that has piled up and needs
> > converting, the more likely it is to happen.
>
> I completely understand, and agree with, the direction you want to take
> this.
>
> > Sure, you have the problem as above, but mostly people are only going
> > to use one of them, so it doesn't matter.
> >
> > We could also have a way of obtaining a number from a label, if you
> > want to go that far. But I suggest just telling people to use labels
> > like "1" and "0" which should work.
>
> Alright, well, if that is acceptable then I'll do it that way. For my
> own piece of mind, I think I'll also add some way of safely doing the
> reverse mapping for any label. Does the following look ok?
>
> blkmap get <label> dev <var>
>
> This way you could extend it with other attributes in the future
> (e.g. size).
Yes that sounds great. Perhaps we can (at some point) extend that sort
of thing to block devices in general.
Regards,
SImon
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 4/8] blk: blkmap: Add memory mapping support
2023-02-01 18:10 [PATCH 0/8] blk: blkmap: Composable virtual block devices Tobias Waldekranz
` (2 preceding siblings ...)
2023-02-01 18:10 ` [PATCH 3/8] blk: blkmap: Add basic infrastructure Tobias Waldekranz
@ 2023-02-01 18:10 ` Tobias Waldekranz
2023-02-01 20:21 ` Simon Glass
2023-02-01 18:10 ` [PATCH 5/8] blk: blkmap: Add linear device " Tobias Waldekranz
` (4 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Tobias Waldekranz @ 2023-02-01 18:10 UTC (permalink / raw)
To: sjg, xypron.glpk, ilias.apalodimas; +Cc: u-boot
Allow a slice of RAM to be mapped to a blkmap. This means that RAM can
now be accessed as if it was a block device, meaning that existing
filesystem drivers can now be used to access ramdisks.
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
drivers/block/blkmap.c | 106 +++++++++++++++++++++++++++++++++++++++++
include/blkmap.h | 4 ++
2 files changed, 110 insertions(+)
diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
index a6ba07404c..c8c2dcac11 100644
--- a/drivers/block/blkmap.c
+++ b/drivers/block/blkmap.c
@@ -12,6 +12,7 @@
#include <dm/lists.h>
#include <dm/root.h>
#include <malloc.h>
+#include <mapmem.h>
#include <part.h>
struct blkmap;
@@ -93,6 +94,111 @@ static int blkmap_add(struct blkmap *bm, struct blkmap_slice *new)
return 0;
}
+struct blkmap_mem {
+ struct blkmap_slice slice;
+ void *addr;
+ bool remapped;
+};
+
+static ulong blkmap_mem_read(struct blkmap *bm, struct blkmap_slice *bms,
+ lbaint_t blknr, lbaint_t blkcnt, void *buffer)
+{
+ struct blkmap_mem *bmm = container_of(bms, struct blkmap_mem, slice);
+ struct blk_desc *bd = dev_get_uclass_plat(bm->dev);
+ char *src;
+
+ src = bmm->addr + (blknr << bd->log2blksz);
+ memcpy(buffer, src, blkcnt << bd->log2blksz);
+ return blkcnt;
+}
+
+static ulong blkmap_mem_write(struct blkmap *bm, struct blkmap_slice *bms,
+ lbaint_t blknr, lbaint_t blkcnt,
+ const void *buffer)
+{
+ struct blkmap_mem *bmm = container_of(bms, struct blkmap_mem, slice);
+ struct blk_desc *bd = dev_get_uclass_plat(bm->dev);
+ char *dst;
+
+ dst = bmm->addr + (blknr << bd->log2blksz);
+ memcpy(dst, buffer, blkcnt << bd->log2blksz);
+ return blkcnt;
+}
+
+static void blkmap_mem_destroy(struct blkmap *bm, struct blkmap_slice *bms)
+{
+ struct blkmap_mem *bmm = container_of(bms, struct blkmap_mem, slice);
+
+ if (bmm->remapped)
+ unmap_sysmem(bmm->addr);
+}
+
+int __blkmap_map_mem(int devnum, lbaint_t blknr, lbaint_t blkcnt, void *addr,
+ bool remapped)
+{
+ struct blkmap_mem *bmm;
+ struct blkmap *bm;
+ int err;
+
+ bm = blkmap_from_devnum(devnum);
+ if (!bm)
+ return -ENODEV;
+
+ bmm = malloc(sizeof(*bmm));
+ if (!bmm)
+ return -ENOMEM;
+
+ *bmm = (struct blkmap_mem) {
+ .slice = {
+ .blknr = blknr,
+ .blkcnt = blkcnt,
+
+ .read = blkmap_mem_read,
+ .write = blkmap_mem_write,
+ .destroy = blkmap_mem_destroy,
+ },
+
+ .addr = addr,
+ .remapped = remapped,
+ };
+
+ err = blkmap_add(bm, &bmm->slice);
+ if (err)
+ free(bmm);
+
+ return err;
+}
+
+int blkmap_map_mem(int devnum, lbaint_t blknr, lbaint_t blkcnt, void *addr)
+{
+ return __blkmap_map_mem(devnum, blknr, blkcnt, addr, false);
+}
+
+int blkmap_map_pmem(int devnum, lbaint_t blknr, lbaint_t blkcnt,
+ phys_addr_t paddr)
+{
+ struct blk_desc *bd;
+ struct blkmap *bm;
+ void *addr;
+ int err;
+
+ bm = blkmap_from_devnum(devnum);
+ if (!bm)
+ return -ENODEV;
+
+ bd = dev_get_uclass_plat(bm->dev);
+
+ addr = map_sysmem(paddr, blkcnt << bd->log2blksz);
+ if (!addr)
+ return -ENOMEM;
+
+ err = __blkmap_map_mem(devnum, blknr, blkcnt, addr, true);
+ if (err)
+ unmap_sysmem(addr);
+
+ return err;
+}
+
static struct udevice *blkmap_root(void)
{
static struct udevice *dev;
diff --git a/include/blkmap.h b/include/blkmap.h
index 37c0c31c3f..a93611ff62 100644
--- a/include/blkmap.h
+++ b/include/blkmap.h
@@ -9,6 +9,10 @@
#include <stdbool.h>
+int blkmap_map_mem(int devnum, lbaint_t blknr, lbaint_t blkcnt, void *addr);
+int blkmap_map_pmem(int devnum, lbaint_t blknr, lbaint_t blkcnt,
+ phys_addr_t paddr);
+
int blkmap_create(int devnum);
int blkmap_destroy(int devnum);
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 4/8] blk: blkmap: Add memory mapping support
2023-02-01 18:10 ` [PATCH 4/8] blk: blkmap: Add memory mapping support Tobias Waldekranz
@ 2023-02-01 20:21 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2023-02-01 20:21 UTC (permalink / raw)
To: Tobias Waldekranz; +Cc: xypron.glpk, ilias.apalodimas, u-boot
Hi Tobias,
On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>
> Allow a slice of RAM to be mapped to a blkmap. This means that RAM can
> now be accessed as if it was a block device, meaning that existing
> filesystem drivers can now be used to access ramdisks.
>
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
> drivers/block/blkmap.c | 106 +++++++++++++++++++++++++++++++++++++++++
> include/blkmap.h | 4 ++
> 2 files changed, 110 insertions(+)
>
> diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
> index a6ba07404c..c8c2dcac11 100644
> --- a/drivers/block/blkmap.c
> +++ b/drivers/block/blkmap.c
> @@ -12,6 +12,7 @@
> #include <dm/lists.h>
> #include <dm/root.h>
> #include <malloc.h>
> +#include <mapmem.h>
> #include <part.h>
>
> struct blkmap;
> @@ -93,6 +94,111 @@ static int blkmap_add(struct blkmap *bm, struct blkmap_slice *new)
> return 0;
> }
>
> +struct blkmap_mem {
> + struct blkmap_slice slice;
> + void *addr;
> + bool remapped;
> +};
> +
> +static ulong blkmap_mem_read(struct blkmap *bm, struct blkmap_slice *bms,
> + lbaint_t blknr, lbaint_t blkcnt, void *buffer)
> +{
> + struct blkmap_mem *bmm = container_of(bms, struct blkmap_mem, slice);
> + struct blk_desc *bd = dev_get_uclass_plat(bm->dev);
> + char *src;
> +
> + src = bmm->addr + (blknr << bd->log2blksz);
> + memcpy(buffer, src, blkcnt << bd->log2blksz);
> + return blkcnt;
> +}
> +
> +static ulong blkmap_mem_write(struct blkmap *bm, struct blkmap_slice *bms,
> + lbaint_t blknr, lbaint_t blkcnt,
> + const void *buffer)
> +{
> + struct blkmap_mem *bmm = container_of(bms, struct blkmap_mem, slice);
> + struct blk_desc *bd = dev_get_uclass_plat(bm->dev);
> + char *dst;
> +
> + dst = bmm->addr + (blknr << bd->log2blksz);
> + memcpy(dst, buffer, blkcnt << bd->log2blksz);
> + return blkcnt;
> +}
> +
> +static void blkmap_mem_destroy(struct blkmap *bm, struct blkmap_slice *bms)
> +{
> + struct blkmap_mem *bmm = container_of(bms, struct blkmap_mem, slice);
> +
> + if (bmm->remapped)
> + unmap_sysmem(bmm->addr);
> +}
> +
> +int __blkmap_map_mem(int devnum, lbaint_t blknr, lbaint_t blkcnt, void *addr,
> + bool remapped)
> +{
> + struct blkmap_mem *bmm;
> + struct blkmap *bm;
> + int err;
> +
> + bm = blkmap_from_devnum(devnum);
> + if (!bm)
> + return -ENODEV;
> +
> + bmm = malloc(sizeof(*bmm));
> + if (!bmm)
> + return -ENOMEM;
> +
> + *bmm = (struct blkmap_mem) {
> + .slice = {
> + .blknr = blknr,
> + .blkcnt = blkcnt,
> +
> + .read = blkmap_mem_read,
> + .write = blkmap_mem_write,
> + .destroy = blkmap_mem_destroy,
> + },
> +
> + .addr = addr,
> + .remapped = remapped,
> + };
> +
> + err = blkmap_add(bm, &bmm->slice);
> + if (err)
> + free(bmm);
> +
> + return err;
> +}
> +
> +int blkmap_map_mem(int devnum, lbaint_t blknr, lbaint_t blkcnt, void *addr)
> +{
> + return __blkmap_map_mem(devnum, blknr, blkcnt, addr, false);
> +}
> +
> +int blkmap_map_pmem(int devnum, lbaint_t blknr, lbaint_t blkcnt,
> + phys_addr_t paddr)
> +{
> + struct blk_desc *bd;
> + struct blkmap *bm;
> + void *addr;
> + int err;
> +
> + bm = blkmap_from_devnum(devnum);
> + if (!bm)
> + return -ENODEV;
> +
> + bd = dev_get_uclass_plat(bm->dev);
> +
> + addr = map_sysmem(paddr, blkcnt << bd->log2blksz);
> + if (!addr)
> + return -ENOMEM;
> +
> + err = __blkmap_map_mem(devnum, blknr, blkcnt, addr, true);
> + if (err)
> + unmap_sysmem(addr);
> +
> + return err;
> +}
> +
> static struct udevice *blkmap_root(void)
> {
> static struct udevice *dev;
> diff --git a/include/blkmap.h b/include/blkmap.h
> index 37c0c31c3f..a93611ff62 100644
> --- a/include/blkmap.h
> +++ b/include/blkmap.h
> @@ -9,6 +9,10 @@
>
> #include <stdbool.h>
>
> +int blkmap_map_mem(int devnum, lbaint_t blknr, lbaint_t blkcnt, void *addr);
> +int blkmap_map_pmem(int devnum, lbaint_t blknr, lbaint_t blkcnt,
> + phys_addr_t paddr);
Comments again.
> +
> int blkmap_create(int devnum);
> int blkmap_destroy(int devnum);
>
> --
> 2.34.1
>
Other than that and the devnum stuff, LGTM
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 5/8] blk: blkmap: Add linear device mapping support
2023-02-01 18:10 [PATCH 0/8] blk: blkmap: Composable virtual block devices Tobias Waldekranz
` (3 preceding siblings ...)
2023-02-01 18:10 ` [PATCH 4/8] blk: blkmap: Add memory mapping support Tobias Waldekranz
@ 2023-02-01 18:10 ` Tobias Waldekranz
2023-02-01 20:21 ` Simon Glass
2023-02-01 18:10 ` [PATCH 6/8] cmd: blkmap: Add blkmap command Tobias Waldekranz
` (3 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Tobias Waldekranz @ 2023-02-01 18:10 UTC (permalink / raw)
To: sjg, xypron.glpk, ilias.apalodimas; +Cc: u-boot
Allow a slice of an existing block device to be mapped to a
blkmap. This means that filesystems that are not stored at exact
partition boundaries can be accessed by remapping a slice of the
existing device to a blkmap device.
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
drivers/block/blkmap.c | 71 ++++++++++++++++++++++++++++++++++++++++++
include/blkmap.h | 2 ++
2 files changed, 73 insertions(+)
diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
index c8c2dcac11..14d2ec3f78 100644
--- a/drivers/block/blkmap.c
+++ b/drivers/block/blkmap.c
@@ -94,6 +94,77 @@ static int blkmap_add(struct blkmap *bm, struct blkmap_slice *new)
return 0;
}
+struct blkmap_linear {
+ struct blkmap_slice slice;
+
+ struct blk_desc *bd;
+ lbaint_t blknr;
+};
+
+static ulong blkmap_linear_read(struct blkmap *bm, struct blkmap_slice *bms,
+ lbaint_t blknr, lbaint_t blkcnt, void *buffer)
+{
+ struct blkmap_linear *bml = container_of(bms, struct blkmap_linear, slice);
+
+ return blk_dread(bml->bd, bml->blknr + blknr, blkcnt, buffer);
+}
+
+static ulong blkmap_linear_write(struct blkmap *bm, struct blkmap_slice *bms,
+ lbaint_t blknr, lbaint_t blkcnt,
+ const void *buffer)
+{
+ struct blkmap_linear *bml = container_of(bms, struct blkmap_linear, slice);
+
+ return blk_dwrite(bml->bd, bml->blknr + blknr, blkcnt, buffer);
+}
+
+int blkmap_map_linear(int devnum, lbaint_t blknr, lbaint_t blkcnt,
+ enum uclass_id lcls, int ldevnum, lbaint_t lblknr)
+{
+ struct blkmap_linear *linear;
+ struct blk_desc *bd, *lbd;
+ struct blkmap *bm;
+ int err;
+
+ bm = blkmap_from_devnum(devnum);
+ if (!bm)
+ return -ENODEV;
+
+ bd = dev_get_uclass_plat(bm->dev);
+ lbd = blk_get_devnum_by_uclass_id(lcls, ldevnum);
+ if (!lbd)
+ return -ENODEV;
+
+ if (lbd->blksz != bd->blksz)
+ /* We could support block size translation, but we
+ * don't yet.
+ */
+ return -EINVAL;
+
+ linear = malloc(sizeof(*linear));
+ if (!linear)
+ return -ENOMEM;
+
+ *linear = (struct blkmap_linear) {
+ .slice = {
+ .blknr = blknr,
+ .blkcnt = blkcnt,
+
+ .read = blkmap_linear_read,
+ .write = blkmap_linear_write,
+ },
+
+ .bd = lbd,
+ .blknr = lblknr,
+ };
+
+ err = blkmap_add(bm, &linear->slice);
+ if (err)
+ free(linear);
+
+ return err;
+}
+
struct blkmap_mem {
struct blkmap_slice slice;
void *addr;
diff --git a/include/blkmap.h b/include/blkmap.h
index a93611ff62..dca6e3fe6a 100644
--- a/include/blkmap.h
+++ b/include/blkmap.h
@@ -9,6 +9,8 @@
#include <stdbool.h>
+int blkmap_map_linear(int devnum, lbaint_t blknr, lbaint_t blkcnt,
+ enum uclass_id lcls, int ldevnum, lbaint_t lblknr);
int blkmap_map_mem(int devnum, lbaint_t blknr, lbaint_t blkcnt, void *addr);
int blkmap_map_pmem(int devnum, lbaint_t blknr, lbaint_t blkcnt,
phys_addr_t paddr);
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 5/8] blk: blkmap: Add linear device mapping support
2023-02-01 18:10 ` [PATCH 5/8] blk: blkmap: Add linear device " Tobias Waldekranz
@ 2023-02-01 20:21 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2023-02-01 20:21 UTC (permalink / raw)
To: Tobias Waldekranz; +Cc: xypron.glpk, ilias.apalodimas, u-boot
Hi Tobias,
On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>
> Allow a slice of an existing block device to be mapped to a
> blkmap. This means that filesystems that are not stored at exact
> partition boundaries can be accessed by remapping a slice of the
> existing device to a blkmap device.
>
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
> drivers/block/blkmap.c | 71 ++++++++++++++++++++++++++++++++++++++++++
> include/blkmap.h | 2 ++
> 2 files changed, 73 insertions(+)
>
> diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
> index c8c2dcac11..14d2ec3f78 100644
> --- a/drivers/block/blkmap.c
> +++ b/drivers/block/blkmap.c
> @@ -94,6 +94,77 @@ static int blkmap_add(struct blkmap *bm, struct blkmap_slice *new)
> return 0;
> }
>
> +struct blkmap_linear {
> + struct blkmap_slice slice;
> +
> + struct blk_desc *bd;
Please store the udevice * here. We are trying to avoid external use
of blk_desc.
> + lbaint_t blknr;
> +};
> +
> +static ulong blkmap_linear_read(struct blkmap *bm, struct blkmap_slice *bms,
> + lbaint_t blknr, lbaint_t blkcnt, void *buffer)
> +{
> + struct blkmap_linear *bml = container_of(bms, struct blkmap_linear, slice);
> +
> + return blk_dread(bml->bd, bml->blknr + blknr, blkcnt, buffer);
blk_read()
> +}
> +
> +static ulong blkmap_linear_write(struct blkmap *bm, struct blkmap_slice *bms,
> + lbaint_t blknr, lbaint_t blkcnt,
> + const void *buffer)
> +{
> + struct blkmap_linear *bml = container_of(bms, struct blkmap_linear, slice);
> +
> + return blk_dwrite(bml->bd, bml->blknr + blknr, blkcnt, buffer);
blk_write()
> +}
> +
> +int blkmap_map_linear(int devnum, lbaint_t blknr, lbaint_t blkcnt,
should take a struct udevice *, not a devnum
Finding should happen in the cmd.
> + enum uclass_id lcls, int ldevnum, lbaint_t lblknr)
> +{
> + struct blkmap_linear *linear;
> + struct blk_desc *bd, *lbd;
> + struct blkmap *bm;
> + int err;
> +
> + bm = blkmap_from_devnum(devnum);
> + if (!bm)
> + return -ENODEV;
> +
> + bd = dev_get_uclass_plat(bm->dev);
> + lbd = blk_get_devnum_by_uclass_id(lcls, ldevnum);
> + if (!lbd)
> + return -ENODEV;
> +
> + if (lbd->blksz != bd->blksz)
> + /* We could support block size translation, but we
> + * don't yet.
> + */
> + return -EINVAL;
> +
> + linear = malloc(sizeof(*linear));
> + if (!linear)
> + return -ENOMEM;
> +
> + *linear = (struct blkmap_linear) {
> + .slice = {
> + .blknr = blknr,
> + .blkcnt = blkcnt,
> +
> + .read = blkmap_linear_read,
> + .write = blkmap_linear_write,
> + },
> +
> + .bd = lbd,
> + .blknr = lblknr,
> + };
> +
> + err = blkmap_add(bm, &linear->slice);
> + if (err)
> + free(linear);
> +
> + return err;
> +}
> +
> struct blkmap_mem {
> struct blkmap_slice slice;
> void *addr;
> diff --git a/include/blkmap.h b/include/blkmap.h
> index a93611ff62..dca6e3fe6a 100644
> --- a/include/blkmap.h
> +++ b/include/blkmap.h
> @@ -9,6 +9,8 @@
>
> #include <stdbool.h>
>
> +int blkmap_map_linear(int devnum, lbaint_t blknr, lbaint_t blkcnt,
> + enum uclass_id lcls, int ldevnum, lbaint_t lblknr);
comments again
> int blkmap_map_mem(int devnum, lbaint_t blknr, lbaint_t blkcnt, void *addr);
> int blkmap_map_pmem(int devnum, lbaint_t blknr, lbaint_t blkcnt,
> phys_addr_t paddr);
> --
> 2.34.1
>
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 6/8] cmd: blkmap: Add blkmap command
2023-02-01 18:10 [PATCH 0/8] blk: blkmap: Composable virtual block devices Tobias Waldekranz
` (4 preceding siblings ...)
2023-02-01 18:10 ` [PATCH 5/8] blk: blkmap: Add linear device " Tobias Waldekranz
@ 2023-02-01 18:10 ` Tobias Waldekranz
2023-02-01 20:21 ` Simon Glass
2023-02-01 18:10 ` [PATCH 7/8] test: blkmap: Add test suite Tobias Waldekranz
` (2 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Tobias Waldekranz @ 2023-02-01 18:10 UTC (permalink / raw)
To: sjg, xypron.glpk, ilias.apalodimas; +Cc: u-boot
Add a frontend for the blkmap subsystem. In addition to the common
block device operations, this allows users to create and destroy
devices, and map in memory and slices of other block devices.
With that we support two primary use-cases:
- Being able to "distro boot" from a RAM disk. I.e., from an image
where the kernel is stored in /boot of some filesystem supported
by U-Boot.
- Accessing filesystems not located on exact partition boundaries,
e.g. when a filesystem image is wrapped in an FIT image and stored
in a disk partition.
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
MAINTAINERS | 1 +
cmd/Kconfig | 19 ++++++
cmd/Makefile | 1 +
cmd/blkmap.c | 181 +++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 202 insertions(+)
create mode 100644 cmd/blkmap.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 28a34231bf..83c0f90a53 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -789,6 +789,7 @@ F: tools/binman/
BLKMAP
M: Tobias Waldekranz <tobias@waldekranz.com>
S: Maintained
+F: cmd/blkmap.c
F: drivers/block/blkmap.c
F: include/blkmap.h
diff --git a/cmd/Kconfig b/cmd/Kconfig
index dc0446e02e..cd35b8318d 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1953,6 +1953,25 @@ config CMD_BLOCK_CACHE
during development, but also allows the cache to be disabled when
it might hurt performance (e.g. when using the ums command).
+config CMD_BLKMAP
+ bool "blkmap - Composable virtual block devices"
+ depends on BLKMAP
+ default y if BLKMAP
+ help
+ Create virtual block devices that are backed by various sources,
+ e.g. RAM, or parts of an existing block device. Though much more
+ rudimentary, it borrows a lot of ideas from Linux's device mapper
+ subsystem.
+
+ Example use-cases:
+ - Treat a region of RAM as a block device, i.e. a RAM disk. This let's
+ you extract files from filesystem images stored in RAM (perhaps as a
+ result of a TFTP transfer).
+ - Create a virtual partition on an existing device. This let's you
+ access filesystems that aren't stored at an exact partition
+ boundary. A common example is a filesystem image embedded in an FIT
+ image.
+
config CMD_BUTTON
bool "button"
depends on BUTTON
diff --git a/cmd/Makefile b/cmd/Makefile
index 7b6ff73186..1d51fddec1 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_CMD_BCB) += bcb.o
obj-$(CONFIG_CMD_BDI) += bdinfo.o
obj-$(CONFIG_CMD_BIND) += bind.o
obj-$(CONFIG_CMD_BINOP) += binop.o
+obj-$(CONFIG_CMD_BLKMAP) += blkmap.o
obj-$(CONFIG_CMD_BLOBLIST) += bloblist.o
obj-$(CONFIG_CMD_BLOCK_CACHE) += blkcache.o
obj-$(CONFIG_CMD_BMP) += bmp.o
diff --git a/cmd/blkmap.c b/cmd/blkmap.c
new file mode 100644
index 0000000000..f1d4a4bab0
--- /dev/null
+++ b/cmd/blkmap.c
@@ -0,0 +1,181 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2023 Addiva Elektronik
+ * Author: Tobias Waldekranz <tobias@waldekranz.com>
+ */
+
+#include <blk.h>
+#include <blkmap.h>
+#include <common.h>
+#include <command.h>
+#include <malloc.h>
+
+static int blkmap_curr_dev;
+
+struct map_ctx {
+ int devnum;
+ lbaint_t blknr, blkcnt;
+};
+
+typedef int (*map_parser_fn)(struct map_ctx *ctx, int argc, char *const argv[]);
+
+struct map_handler {
+ const char *name;
+ map_parser_fn fn;
+};
+
+int do_blkmap_map_linear(struct map_ctx *ctx, int argc, char *const argv[])
+{
+ struct blk_desc *lbd;
+ int err, ldevnum;
+ lbaint_t lblknr;
+
+ if (argc < 4)
+ return CMD_RET_USAGE;
+
+ ldevnum = dectoul(argv[2], NULL);
+ lblknr = dectoul(argv[3], NULL);
+
+ lbd = blk_get_devnum_by_uclass_idname(argv[1], ldevnum);
+ if (!lbd) {
+ printf("Found no device matching \"%s %d\"\n",
+ argv[1], ldevnum);
+ return CMD_RET_FAILURE;
+ }
+
+ err = blkmap_map_linear(ctx->devnum, ctx->blknr, ctx->blkcnt,
+ lbd->uclass_id, ldevnum, lblknr);
+ if (err) {
+ printf("Unable to map \"%s %d\" at block 0x" LBAF ": %d\n",
+ argv[1], ldevnum, ctx->blknr, err);
+
+ return CMD_RET_FAILURE;
+ }
+
+ printf("Block 0x" LBAF "+0x" LBAF " mapped to block 0x" LBAF " of \"%s %d\"\n",
+ ctx->blknr, ctx->blkcnt, lblknr, argv[1], ldevnum);
+ return CMD_RET_SUCCESS;
+}
+
+int do_blkmap_map_mem(struct map_ctx *ctx, int argc, char *const argv[])
+{
+ phys_addr_t addr;
+ int err;
+
+ if (argc < 2)
+ return CMD_RET_USAGE;
+
+ addr = hextoul(argv[1], NULL);
+
+ err = blkmap_map_pmem(ctx->devnum, ctx->blknr, ctx->blkcnt, addr);
+ if (err) {
+ printf("Unable to map %#llx at block 0x" LBAF ": %d\n",
+ (unsigned long long)addr, ctx->blknr, err);
+ return CMD_RET_FAILURE;
+ }
+
+ printf("Block 0x" LBAF "+0x" LBAF " mapped to %#llx\n",
+ ctx->blknr, ctx->blkcnt, (unsigned long long)addr);
+ return CMD_RET_SUCCESS;
+}
+
+struct map_handler map_handlers[] = {
+ { .name = "linear", .fn = do_blkmap_map_linear },
+ { .name = "mem", .fn = do_blkmap_map_mem },
+
+ { .name = NULL }
+};
+
+static int do_blkmap_map(struct cmd_tbl *cmdtp, int flag,
+ int argc, char *const argv[])
+{
+ struct map_handler *handler;
+ struct map_ctx ctx;
+
+ if (argc < 5)
+ return CMD_RET_USAGE;
+
+ ctx.devnum = dectoul(argv[1], NULL);
+ ctx.blknr = hextoul(argv[2], NULL);
+ ctx.blkcnt = hextoul(argv[3], NULL);
+ argc -= 4;
+ argv += 4;
+
+ for (handler = map_handlers; handler->name; handler++) {
+ if (!strcmp(handler->name, argv[0]))
+ return handler->fn(&ctx, argc, argv);
+ }
+
+ printf("Unknown map type \"%s\"\n", argv[0]);
+ return CMD_RET_USAGE;
+}
+
+static int do_blkmap_create(struct cmd_tbl *cmdtp, int flag,
+ int argc, char *const argv[])
+{
+ int devnum = -1;
+
+ if (argc == 2)
+ devnum = dectoul(argv[1], NULL);
+
+ devnum = blkmap_create(devnum);
+ if (devnum < 0) {
+ printf("Unable to create device: %d\n", devnum);
+ return CMD_RET_FAILURE;
+ }
+
+ printf("Created device %d\n", devnum);
+ return CMD_RET_SUCCESS;
+}
+
+static int do_blkmap_destroy(struct cmd_tbl *cmdtp, int flag,
+ int argc, char *const argv[])
+{
+ int err, devnum;
+
+ if (argc != 2)
+ return CMD_RET_USAGE;
+
+ devnum = dectoul(argv[1], NULL);
+
+ err = blkmap_destroy(devnum);
+ if (err) {
+ printf("Unable to destroy device %d: %d\n", devnum, err);
+ return CMD_RET_FAILURE;
+ }
+
+ printf("Destroyed device %d\n", devnum);
+ return CMD_RET_SUCCESS;
+}
+
+static int do_blkmap_common(struct cmd_tbl *cmdtp, int flag,
+ int argc, char *const argv[])
+{
+ /* The subcommand parsing pops the original argv[0] ("blkmap")
+ * which blk_common_cmd expects. Push it back again.
+ */
+ argc++;
+ argv--;
+
+ return blk_common_cmd(argc, argv, UCLASS_BLKMAP, &blkmap_curr_dev);
+}
+
+U_BOOT_CMD_WITH_SUBCMDS(
+ blkmap, "Composeable virtual block devices",
+ "info - list configured devices\n"
+ "blkmap part - list available partitions on current blkmap device\n"
+ "blkmap dev [<dev>] - show or set current blkmap device\n"
+ "blkmap read <addr> <blk#> <cnt>\n"
+ "blkmap write <addr> <blk#> <cnt>\n"
+ "blkmap create [<dev>] - create device\n"
+ "blkmap destroy <dev> - destroy device\n"
+ "blkmap map <dev> <blk#> <cnt> linear <interface> <dev> <blk#> - device mapping\n"
+ "blkmap map <dev> <blk#> <cnt> mem <addr> - memory mapping\n",
+ U_BOOT_SUBCMD_MKENT(info, 2, 1, do_blkmap_common),
+ U_BOOT_SUBCMD_MKENT(part, 2, 1, do_blkmap_common),
+ U_BOOT_SUBCMD_MKENT(dev, 4, 1, do_blkmap_common),
+ U_BOOT_SUBCMD_MKENT(read, 5, 1, do_blkmap_common),
+ U_BOOT_SUBCMD_MKENT(write, 5, 1, do_blkmap_common),
+ U_BOOT_SUBCMD_MKENT(create, 2, 1, do_blkmap_create),
+ U_BOOT_SUBCMD_MKENT(destroy, 2, 1, do_blkmap_destroy),
+ U_BOOT_SUBCMD_MKENT(map, 32, 1, do_blkmap_map));
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 6/8] cmd: blkmap: Add blkmap command
2023-02-01 18:10 ` [PATCH 6/8] cmd: blkmap: Add blkmap command Tobias Waldekranz
@ 2023-02-01 20:21 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2023-02-01 20:21 UTC (permalink / raw)
To: Tobias Waldekranz; +Cc: xypron.glpk, ilias.apalodimas, u-boot
Hi Tobias,
On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>
> Add a frontend for the blkmap subsystem. In addition to the common
> block device operations, this allows users to create and destroy
> devices, and map in memory and slices of other block devices.
>
> With that we support two primary use-cases:
>
> - Being able to "distro boot" from a RAM disk. I.e., from an image
> where the kernel is stored in /boot of some filesystem supported
> by U-Boot.
>
> - Accessing filesystems not located on exact partition boundaries,
> e.g. when a filesystem image is wrapped in an FIT image and stored
> in a disk partition.
>
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
> MAINTAINERS | 1 +
> cmd/Kconfig | 19 ++++++
> cmd/Makefile | 1 +
> cmd/blkmap.c | 181 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 202 insertions(+)
> create mode 100644 cmd/blkmap.c
>
Looks good, but please convert to using labels.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 7/8] test: blkmap: Add test suite
2023-02-01 18:10 [PATCH 0/8] blk: blkmap: Composable virtual block devices Tobias Waldekranz
` (5 preceding siblings ...)
2023-02-01 18:10 ` [PATCH 6/8] cmd: blkmap: Add blkmap command Tobias Waldekranz
@ 2023-02-01 18:10 ` Tobias Waldekranz
2023-02-01 20:21 ` Simon Glass
2023-02-01 18:10 ` [PATCH 8/8] doc: blkmap: Add introduction and examples Tobias Waldekranz
2023-02-01 20:21 ` [PATCH 0/8] blk: blkmap: Composable virtual block devices Simon Glass
8 siblings, 1 reply; 25+ messages in thread
From: Tobias Waldekranz @ 2023-02-01 18:10 UTC (permalink / raw)
To: sjg, xypron.glpk, ilias.apalodimas; +Cc: u-boot
Verify that:
- Block maps can be created and destroyed
- Mappings aren't allowed to overlap
- Multiple mappings can be attached and be read/written from/to
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
MAINTAINERS | 1 +
configs/sandbox_defconfig | 1 +
test/py/tests/test_blkmap.py | 164 +++++++++++++++++++++++++++++++++++
3 files changed, 166 insertions(+)
create mode 100644 test/py/tests/test_blkmap.py
diff --git a/MAINTAINERS b/MAINTAINERS
index 83c0f90a53..c420c8e1f9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -792,6 +792,7 @@ S: Maintained
F: cmd/blkmap.c
F: drivers/block/blkmap.c
F: include/blkmap.h
+F: test/py/tests/test_blkmap.py
BOOTDEVICE
M: Simon Glass <sjg@chromium.org>
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index 34c342b6f5..06021e4902 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -145,6 +145,7 @@ CONFIG_ADC=y
CONFIG_ADC_SANDBOX=y
CONFIG_AXI=y
CONFIG_AXI_SANDBOX=y
+CONFIG_BLKMAP=y
CONFIG_SYS_IDE_MAXBUS=1
CONFIG_SYS_ATA_BASE_ADDR=0x100
CONFIG_SYS_ATA_STRIDE=4
diff --git a/test/py/tests/test_blkmap.py b/test/py/tests/test_blkmap.py
new file mode 100644
index 0000000000..5a4c770c81
--- /dev/null
+++ b/test/py/tests/test_blkmap.py
@@ -0,0 +1,164 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Copyright (c) 2023 Addiva Elektronik
+# Author: Tobias Waldekranz <tobias@waldekranz.com>
+
+""" Unit test for blkmap command
+"""
+
+import pytest
+
+BLKSZ = 0x200
+
+MAPPING = [
+ ((0, 1), 3),
+ ((1, 3), 0),
+ ((4, 2), 6),
+ ((6, 2), 4),
+]
+
+ORDERED = 0x0000
+UNORDERED = 0x1000
+BUFFER = 0x2000
+
+def mkblob(base, mapping):
+ cmds = []
+
+ for ((blksrc, blkcnt), blkdst) in mapping:
+ for blknr in range(blkcnt):
+ cmds.append(f"mw.b 0x{base + (blkdst + blknr) * BLKSZ:x}" +
+ f" 0x{blksrc + blknr:x} 0x{BLKSZ:x}")
+ return cmds
+
+class Blkmap(object):
+ def __init__(self, console, num):
+ self.console, self.num = console, num
+
+ def __enter__(self):
+ r = self.console.run_command(f"blkmap create {self.num}")
+ assert(f"Created device {self.num}" in r)
+
+ r = self.console.run_command(f"blkmap dev {self.num}")
+ assert("is now current device" in r)
+
+ return self
+
+ def __exit__(self, typ, value, traceback):
+ r = self.console.run_command(f"blkmap destroy {self.num}")
+ assert(f"Destroyed device {self.num}" in r)
+
+ def map_mem(self, blknr, blkcnt, addr):
+ r = self.console.run_command(
+ f"blkmap map {self.num} {blknr:#x} {blkcnt:#x} mem {addr:#x}"
+ )
+ assert(" mapped to " in r)
+
+ def read(self, addr, blknr, blkcnt):
+ r = self.console.run_command(
+ f"blkmap read {addr:#x} {blknr:#x} {blkcnt:#x}"
+ )
+ assert(" OK" in r)
+
+ def write(self, addr, blknr, blkcnt):
+ r = self.console.run_command(
+ f"blkmap write {addr:#x} {blknr:#x} {blkcnt:#x}"
+ )
+ assert(" OK" in r)
+
+@pytest.mark.boardspec('sandbox')
+@pytest.mark.buildconfigspec('cmd_blkmap')
+def test_blkmap_creation(u_boot_console):
+ """ Verify that blkmaps can be created and destroyed
+
+ Args:
+ u_boot_console -- U-Boot console
+ """
+ with Blkmap(u_boot_console, 0):
+ # Can't have 2 blkmap 0's
+ with pytest.raises(AssertionError):
+ with Blkmap(u_boot_console, 0):
+ pass
+
+ # But blkmap 1 should be fine
+ with Blkmap(u_boot_console, 1):
+ pass
+
+ # Once blkmap 0 is destroyed, we should be able to create it again
+ with Blkmap(u_boot_console, 0):
+ pass
+
+@pytest.mark.boardspec('sandbox')
+@pytest.mark.buildconfigspec('cmd_blkmap')
+def test_blkmap_slicing(u_boot_console):
+ """ Verify that slices aren't allowed to overlap
+
+ Args:
+ u_boot_console -- U-Boot console
+ """
+ with Blkmap(u_boot_console, 0) as bm:
+ bm.map_mem(8, 8, 0)
+
+ # Can't overlap on the low end
+ with pytest.raises(AssertionError):
+ bm.map_mem(4, 5, 0)
+
+ # Can't be inside
+ with pytest.raises(AssertionError):
+ bm.map_mem(10, 2, 0)
+
+ # Can't overlap on the high end
+ with pytest.raises(AssertionError):
+ bm.map_mem(15, 4, 0)
+
+ # But we should be able to add slices right before and after
+ bm.map_mem( 4, 4, 0)
+ bm.map_mem(16, 4, 0)
+
+@pytest.mark.boardspec('sandbox')
+@pytest.mark.buildconfigspec('cmd_blkmap')
+def test_blkmap_mem_read(u_boot_console):
+ """ Test reading from a memory backed blkmap
+
+ Args:
+ u_boot_console -- U-Boot console
+ """
+
+ # Generate an ordered and an unordered pattern in memory
+ u_boot_console.run_command_list(mkblob(ORDERED, (((0, 8), 0),)))
+ u_boot_console.run_command_list(mkblob(UNORDERED, MAPPING))
+
+ with Blkmap(u_boot_console, 0) as bm:
+ # Create a blkmap that cancels out the disorder
+ for ((blksrc, blkcnt), blkdst) in MAPPING:
+ bm.map_mem(blksrc, blkcnt, UNORDERED + blkdst * BLKSZ)
+
+ # Read out the data via the blkmap device to another area
+ bm.read(BUFFER, 0, 8)
+
+ # And verify that it matches the ordered pattern
+ response = u_boot_console.run_command(f"cmp.b 0x{BUFFER:x} 0x{ORDERED:x} 0x1000")
+ assert("Total of 4096 byte(s) were the same" in response)
+
+@pytest.mark.boardspec('sandbox')
+@pytest.mark.buildconfigspec('cmd_blkmap')
+def test_blkmap_mem_write(u_boot_console):
+ """ Test writing to a memory backed blkmap
+
+ Args:
+ u_boot_console -- U-Boot console
+ """
+ # Generate an ordered and an unordered pattern in memory
+ u_boot_console.run_command_list(mkblob(ORDERED, (((0, 8), 0),)))
+ u_boot_console.run_command_list(mkblob(UNORDERED, MAPPING))
+
+ with Blkmap(u_boot_console, 0) as bm:
+ # Create a blkmap that mimics the disorder
+ for ((blksrc, blkcnt), blkdst) in MAPPING:
+ bm.map_mem(blksrc, blkcnt, BUFFER + blkdst * BLKSZ)
+
+ # Write the ordered data via the blkmap device to another area
+ bm.write(ORDERED, 0, 8)
+
+ # And verify that the result matches the unordered pattern
+ response = u_boot_console.run_command(f"cmp.b 0x{BUFFER:x} 0x{UNORDERED:x} 0x1000")
+ assert("Total of 4096 byte(s) were the same" in response)
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 7/8] test: blkmap: Add test suite
2023-02-01 18:10 ` [PATCH 7/8] test: blkmap: Add test suite Tobias Waldekranz
@ 2023-02-01 20:21 ` Simon Glass
0 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2023-02-01 20:21 UTC (permalink / raw)
To: Tobias Waldekranz; +Cc: xypron.glpk, ilias.apalodimas, u-boot
Hi Tobias,
On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>
> Verify that:
>
> - Block maps can be created and destroyed
> - Mappings aren't allowed to overlap
> - Multiple mappings can be attached and be read/written from/to
>
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
> MAINTAINERS | 1 +
> configs/sandbox_defconfig | 1 +
> test/py/tests/test_blkmap.py | 164 +++++++++++++++++++++++++++++++++++
> 3 files changed, 166 insertions(+)
> create mode 100644 test/py/tests/test_blkmap.py
https://u-boot.readthedocs.io/en/latest/develop/tests_writing.html
This should be done in C...see test/dm/acpi.c for an example of how to
check console output.
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 83c0f90a53..c420c8e1f9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -792,6 +792,7 @@ S: Maintained
> F: cmd/blkmap.c
> F: drivers/block/blkmap.c
> F: include/blkmap.h
> +F: test/py/tests/test_blkmap.py
>
> BOOTDEVICE
> M: Simon Glass <sjg@chromium.org>
> diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> index 34c342b6f5..06021e4902 100644
> --- a/configs/sandbox_defconfig
> +++ b/configs/sandbox_defconfig
> @@ -145,6 +145,7 @@ CONFIG_ADC=y
> CONFIG_ADC_SANDBOX=y
> CONFIG_AXI=y
> CONFIG_AXI_SANDBOX=y
> +CONFIG_BLKMAP=y
> CONFIG_SYS_IDE_MAXBUS=1
> CONFIG_SYS_ATA_BASE_ADDR=0x100
> CONFIG_SYS_ATA_STRIDE=4
> diff --git a/test/py/tests/test_blkmap.py b/test/py/tests/test_blkmap.py
> new file mode 100644
> index 0000000000..5a4c770c81
> --- /dev/null
> +++ b/test/py/tests/test_blkmap.py
> @@ -0,0 +1,164 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# Copyright (c) 2023 Addiva Elektronik
> +# Author: Tobias Waldekranz <tobias@waldekranz.com>
> +
> +""" Unit test for blkmap command
> +"""
> +
> +import pytest
> +
> +BLKSZ = 0x200
> +
> +MAPPING = [
> + ((0, 1), 3),
> + ((1, 3), 0),
> + ((4, 2), 6),
> + ((6, 2), 4),
> +]
> +
> +ORDERED = 0x0000
> +UNORDERED = 0x1000
> +BUFFER = 0x2000
> +
> +def mkblob(base, mapping):
> + cmds = []
> +
> + for ((blksrc, blkcnt), blkdst) in mapping:
> + for blknr in range(blkcnt):
> + cmds.append(f"mw.b 0x{base + (blkdst + blknr) * BLKSZ:x}" +
> + f" 0x{blksrc + blknr:x} 0x{BLKSZ:x}")
> + return cmds
> +
> +class Blkmap(object):
> + def __init__(self, console, num):
> + self.console, self.num = console, num
> +
> + def __enter__(self):
> + r = self.console.run_command(f"blkmap create {self.num}")
> + assert(f"Created device {self.num}" in r)
> +
> + r = self.console.run_command(f"blkmap dev {self.num}")
> + assert("is now current device" in r)
> +
> + return self
> +
> + def __exit__(self, typ, value, traceback):
> + r = self.console.run_command(f"blkmap destroy {self.num}")
> + assert(f"Destroyed device {self.num}" in r)
> +
> + def map_mem(self, blknr, blkcnt, addr):
> + r = self.console.run_command(
> + f"blkmap map {self.num} {blknr:#x} {blkcnt:#x} mem {addr:#x}"
> + )
> + assert(" mapped to " in r)
> +
> + def read(self, addr, blknr, blkcnt):
> + r = self.console.run_command(
> + f"blkmap read {addr:#x} {blknr:#x} {blkcnt:#x}"
> + )
> + assert(" OK" in r)
> +
> + def write(self, addr, blknr, blkcnt):
> + r = self.console.run_command(
> + f"blkmap write {addr:#x} {blknr:#x} {blkcnt:#x}"
> + )
> + assert(" OK" in r)
> +
> +@pytest.mark.boardspec('sandbox')
> +@pytest.mark.buildconfigspec('cmd_blkmap')
> +def test_blkmap_creation(u_boot_console):
> + """ Verify that blkmaps can be created and destroyed
> +
> + Args:
> + u_boot_console -- U-Boot console
> + """
> + with Blkmap(u_boot_console, 0):
> + # Can't have 2 blkmap 0's
> + with pytest.raises(AssertionError):
> + with Blkmap(u_boot_console, 0):
> + pass
> +
> + # But blkmap 1 should be fine
> + with Blkmap(u_boot_console, 1):
> + pass
> +
> + # Once blkmap 0 is destroyed, we should be able to create it again
> + with Blkmap(u_boot_console, 0):
> + pass
> +
> +@pytest.mark.boardspec('sandbox')
> +@pytest.mark.buildconfigspec('cmd_blkmap')
> +def test_blkmap_slicing(u_boot_console):
> + """ Verify that slices aren't allowed to overlap
> +
> + Args:
> + u_boot_console -- U-Boot console
> + """
> + with Blkmap(u_boot_console, 0) as bm:
> + bm.map_mem(8, 8, 0)
> +
> + # Can't overlap on the low end
> + with pytest.raises(AssertionError):
> + bm.map_mem(4, 5, 0)
> +
> + # Can't be inside
> + with pytest.raises(AssertionError):
> + bm.map_mem(10, 2, 0)
> +
> + # Can't overlap on the high end
> + with pytest.raises(AssertionError):
> + bm.map_mem(15, 4, 0)
> +
> + # But we should be able to add slices right before and after
> + bm.map_mem( 4, 4, 0)
> + bm.map_mem(16, 4, 0)
> +
> +@pytest.mark.boardspec('sandbox')
> +@pytest.mark.buildconfigspec('cmd_blkmap')
> +def test_blkmap_mem_read(u_boot_console):
> + """ Test reading from a memory backed blkmap
> +
> + Args:
> + u_boot_console -- U-Boot console
> + """
> +
> + # Generate an ordered and an unordered pattern in memory
> + u_boot_console.run_command_list(mkblob(ORDERED, (((0, 8), 0),)))
> + u_boot_console.run_command_list(mkblob(UNORDERED, MAPPING))
> +
> + with Blkmap(u_boot_console, 0) as bm:
> + # Create a blkmap that cancels out the disorder
> + for ((blksrc, blkcnt), blkdst) in MAPPING:
> + bm.map_mem(blksrc, blkcnt, UNORDERED + blkdst * BLKSZ)
> +
> + # Read out the data via the blkmap device to another area
> + bm.read(BUFFER, 0, 8)
> +
> + # And verify that it matches the ordered pattern
> + response = u_boot_console.run_command(f"cmp.b 0x{BUFFER:x} 0x{ORDERED:x} 0x1000")
> + assert("Total of 4096 byte(s) were the same" in response)
> +
> +@pytest.mark.boardspec('sandbox')
> +@pytest.mark.buildconfigspec('cmd_blkmap')
> +def test_blkmap_mem_write(u_boot_console):
> + """ Test writing to a memory backed blkmap
> +
> + Args:
> + u_boot_console -- U-Boot console
> + """
> + # Generate an ordered and an unordered pattern in memory
> + u_boot_console.run_command_list(mkblob(ORDERED, (((0, 8), 0),)))
> + u_boot_console.run_command_list(mkblob(UNORDERED, MAPPING))
> +
> + with Blkmap(u_boot_console, 0) as bm:
> + # Create a blkmap that mimics the disorder
> + for ((blksrc, blkcnt), blkdst) in MAPPING:
> + bm.map_mem(blksrc, blkcnt, BUFFER + blkdst * BLKSZ)
> +
> + # Write the ordered data via the blkmap device to another area
> + bm.write(ORDERED, 0, 8)
> +
> + # And verify that the result matches the unordered pattern
> + response = u_boot_console.run_command(f"cmp.b 0x{BUFFER:x} 0x{UNORDERED:x} 0x1000")
> + assert("Total of 4096 byte(s) were the same" in response)
> --
> 2.34.1
>
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH 8/8] doc: blkmap: Add introduction and examples
2023-02-01 18:10 [PATCH 0/8] blk: blkmap: Composable virtual block devices Tobias Waldekranz
` (6 preceding siblings ...)
2023-02-01 18:10 ` [PATCH 7/8] test: blkmap: Add test suite Tobias Waldekranz
@ 2023-02-01 18:10 ` Tobias Waldekranz
2023-02-01 20:21 ` Simon Glass
2023-02-01 21:14 ` Heinrich Schuchardt
2023-02-01 20:21 ` [PATCH 0/8] blk: blkmap: Composable virtual block devices Simon Glass
8 siblings, 2 replies; 25+ messages in thread
From: Tobias Waldekranz @ 2023-02-01 18:10 UTC (permalink / raw)
To: sjg, xypron.glpk, ilias.apalodimas; +Cc: u-boot
Explain block maps by going through two common use-cases.
Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
MAINTAINERS | 1 +
doc/usage/blkmap.rst | 109 +++++++++++++++++++++++++++++++++++++++++++
doc/usage/index.rst | 1 +
3 files changed, 111 insertions(+)
create mode 100644 doc/usage/blkmap.rst
diff --git a/MAINTAINERS b/MAINTAINERS
index c420c8e1f9..de0e41487d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -790,6 +790,7 @@ BLKMAP
M: Tobias Waldekranz <tobias@waldekranz.com>
S: Maintained
F: cmd/blkmap.c
+F: doc/usage/blkmap.rst
F: drivers/block/blkmap.c
F: include/blkmap.h
F: test/py/tests/test_blkmap.py
diff --git a/doc/usage/blkmap.rst b/doc/usage/blkmap.rst
new file mode 100644
index 0000000000..1cf6d97c1b
--- /dev/null
+++ b/doc/usage/blkmap.rst
@@ -0,0 +1,109 @@
+.. SPDX-License-Identifier: GPL-2.0+
+..
+.. Copyright (c) 2023 Addiva Elektronik
+.. Author: Tobias Waldekranz <tobias@waldekranz.com>
+
+Block Maps (blkmap)
+===================
+
+Block maps are a way of looking at various sources of data through the
+lens of a regular block device. It lets you treat devices that are not
+block devices, like RAM, as if they were. It also lets you export a
+slice of an existing block device, which does not have to correspond
+to a partition boundary, as a new block device.
+
+This is primarily useful because U-Boot's filesystem drivers only
+operate on block devices, so a block map lets you access filesystems
+wherever they might be located.
+
+The implementation is loosely modeled on Linux's "Device Mapper"
+subsystem, see `kernel documentation`_ for more information.
+
+.. _kernel documentation: https://docs.kernel.org/admin-guide/device-mapper/index.html
+
+
+Example: Netbooting an Ext4 Image
+---------------------------------
+
+Say that our system is using an Ext4 filesystem as its rootfs, where
+the kernel is stored in ``/boot``. This image is then typically stored
+in an eMMC partition. In this configuration, we can use something like
+``load mmc 0 ${kernel_addr_r} /boot/Image`` to load the kernel image
+into the expected location, and then boot the system. No problems.
+
+Now imagine that during development, or as a recovery mechanism, we
+want to boot the same type of image by downloading it over the
+network. Getting the image to the target is easy enough:
+
+::
+
+ dhcp ${ramdisk_addr_r} rootfs.ext4
+
+But now we are faced with a predicament: how to we extract the kernel
+image? Block maps to the rescue!
+
+We start by creating a new device:
+
+::
+
+ blkmap create 0
+
+Before setting up the mapping, we figure out the size of the
+downloaded file, in blocks:
+
+::
+
+ setexpr fileblks ${filesize} + 0x1ff
+ setexpr fileblks ${filesize} / 0x200
+
+Then we can add a mapping to the start of our device, backed by the
+memory at `${loadaddr}`:
+
+::
+
+ blkmap map 0 0 ${fileblks} mem ${fileaddr}
+
+Now we can access the filesystem via the virtual device:
+
+::
+
+ load blkmap 0 ${kernel_addr_r} /boot/Image
+
+
+Example: Accessing a filesystem inside an FIT image
+---------------------------------------------------
+
+In this example, an FIT image is stored in an eMMC partition. We would
+like to read the file ``/etc/version``, stored inside a Squashfs image
+in the FIT. Since the Squashfs image is not stored on a partition
+boundary, there is no way of accessing it via ``load mmc ...``.
+
+What we can to instead is to first figure out the offset and size of
+the filesystem:
+
+::
+
+ mmc dev 0
+ mmc read ${loadaddr} 0 0x100
+
+ fdt addr ${loadaddr}
+ fdt get value squashaddr /images/ramdisk data-position
+ fdt get value squashsize /images/ramdisk data-size
+
+ setexpr squashblk ${squashaddr} / 0x200
+ setexpr squashsize ${squashsize} + 0x1ff
+ setexpr squashsize ${squashsize} / 0x200
+
+Then we can create a block map that maps to that slice of the full
+partition:
+
+::
+
+ blkmap create 0
+ blkmap map 0 0 ${squashsize} linear mmc 0 ${squashblk}
+
+Now we can access the filesystem:
+
+::
+
+ load blkmap 0 ${loadaddr} /etc/version
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index 3804046835..856a3da28e 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -4,6 +4,7 @@ Use U-Boot
.. toctree::
:maxdepth: 1
+ blkmap
dfu
environment
fdt_overlays
--
2.34.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* Re: [PATCH 8/8] doc: blkmap: Add introduction and examples
2023-02-01 18:10 ` [PATCH 8/8] doc: blkmap: Add introduction and examples Tobias Waldekranz
@ 2023-02-01 20:21 ` Simon Glass
2023-02-01 21:14 ` Heinrich Schuchardt
1 sibling, 0 replies; 25+ messages in thread
From: Simon Glass @ 2023-02-01 20:21 UTC (permalink / raw)
To: Tobias Waldekranz; +Cc: xypron.glpk, ilias.apalodimas, u-boot
On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>
> Explain block maps by going through two common use-cases.
>
> Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
> ---
> MAINTAINERS | 1 +
> doc/usage/blkmap.rst | 109 +++++++++++++++++++++++++++++++++++++++++++
> doc/usage/index.rst | 1 +
> 3 files changed, 111 insertions(+)
> create mode 100644 doc/usage/blkmap.rst
Reviewed-by: Simon Glass <sjg@chromium.org>
Nice feature!
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 8/8] doc: blkmap: Add introduction and examples
2023-02-01 18:10 ` [PATCH 8/8] doc: blkmap: Add introduction and examples Tobias Waldekranz
2023-02-01 20:21 ` Simon Glass
@ 2023-02-01 21:14 ` Heinrich Schuchardt
1 sibling, 0 replies; 25+ messages in thread
From: Heinrich Schuchardt @ 2023-02-01 21:14 UTC (permalink / raw)
To: Tobias Waldekranz, sjg, ilias.apalodimas; +Cc: u-boot
Am 1. Februar 2023 19:10:16 MEZ schrieb Tobias Waldekranz <tobias@waldekranz.com>:
>Explain block maps by going through two common use-cases.
>
>Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
>---
> MAINTAINERS | 1 +
> doc/usage/blkmap.rst | 109 +++++++++++++++++++++++++++++++++++++++++++
> doc/usage/index.rst | 1 +
> 3 files changed, 111 insertions(+)
> create mode 100644 doc/usage/blkmap.rst
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index c420c8e1f9..de0e41487d 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -790,6 +790,7 @@ BLKMAP
> M: Tobias Waldekranz <tobias@waldekranz.com>
> S: Maintained
> F: cmd/blkmap.c
>+F: doc/usage/blkmap.rst
> F: drivers/block/blkmap.c
> F: include/blkmap.h
> F: test/py/tests/test_blkmap.py
>diff --git a/doc/usage/blkmap.rst b/doc/usage/blkmap.rst
>new file mode 100644
>index 0000000000..1cf6d97c1b
>--- /dev/null
>+++ b/doc/usage/blkmap.rst
>@@ -0,0 +1,109 @@
>+.. SPDX-License-Identifier: GPL-2.0+
>+..
>+.. Copyright (c) 2023 Addiva Elektronik
>+.. Author: Tobias Waldekranz <tobias@waldekranz.com>
>+
>+Block Maps (blkmap)
>+===================
>+
>+Block maps are a way of looking at various sources of data through the
>+lens of a regular block device. It lets you treat devices that are not
>+block devices, like RAM, as if they were. It also lets you export a
>+slice of an existing block device, which does not have to correspond
>+to a partition boundary, as a new block device.
>+
>+This is primarily useful because U-Boot's filesystem drivers only
>+operate on block devices, so a block map lets you access filesystems
>+wherever they might be located.
>+
>+The implementation is loosely modeled on Linux's "Device Mapper"
>+subsystem, see `kernel documentation`_ for more information.
>+
>+.. _kernel documentation: https://docs.kernel.org/admin-guide/device-mapper/index.html
>+
>+
>+Example: Netbooting an Ext4 Image
>+---------------------------------
>+
>+Say that our system is using an Ext4 filesystem as its rootfs, where
>+the kernel is stored in ``/boot``. This image is then typically stored
>+in an eMMC partition. In this configuration, we can use something like
>+``load mmc 0 ${kernel_addr_r} /boot/Image`` to load the kernel image
>+into the expected location, and then boot the system. No problems.
>+
>+Now imagine that during development, or as a recovery mechanism, we
>+want to boot the same type of image by downloading it over the
>+network. Getting the image to the target is easy enough:
>+
>+::
>+
>+ dhcp ${ramdisk_addr_r} rootfs.ext4
>+
>+But now we are faced with a predicament: how to we extract the kernel
>+image? Block maps to the rescue!
>+
>+We start by creating a new device:
>+
>+::
>+
>+ blkmap create 0
>+
>+Before setting up the mapping, we figure out the size of the
>+downloaded file, in blocks:
>+
>+::
>+
>+ setexpr fileblks ${filesize} + 0x1ff
>+ setexpr fileblks ${filesize} / 0x200
>+
>+Then we can add a mapping to the start of our device, backed by the
>+memory at `${loadaddr}`:
>+
>+::
>+
>+ blkmap map 0 0 ${fileblks} mem ${fileaddr}
This is way too complicated. Just accept the file size here.
How can we handle images assuming a different block size then?
>+
>+Now we can access the filesystem via the virtual device:
>+
>+::
>+
>+ load blkmap 0 ${kernel_addr_r} /boot/Image
>+
>+
>+Example: Accessing a filesystem inside an FIT image
>+---------------------------------------------------
>+
>+In this example, an FIT image is stored in an eMMC partition. We would
>+like to read the file ``/etc/version``, stored inside a Squashfs image
>+in the FIT. Since the Squashfs image is not stored on a partition
>+boundary, there is no way of accessing it via ``load mmc ...``.
>+
>+What we can to instead is to first figure out the offset and size of
>+the filesystem:
>+
>+::
>+
>+ mmc dev 0
>+ mmc read ${loadaddr} 0 0x100
>+
>+ fdt addr ${loadaddr}
>+ fdt get value squashaddr /images/ramdisk data-position
>+ fdt get value squashsize /images/ramdisk data-size
>+
>+ setexpr squashblk ${squashaddr} / 0x200
>+ setexpr squashsize ${squashsize} + 0x1ff
>+ setexpr squashsize ${squashsize} / 0x200
>+
>+Then we can create a block map that maps to that slice of the full
>+partition:
>+
>+::
>+
>+ blkmap create 0
>+ blkmap map 0 0 ${squashsize} linear mmc 0 ${squashblk}
We are the requirements on alignment?
Best regards
Heinrich
>+
>+Now we can access the filesystem:
>+
>+::
>+
>+ load blkmap 0 ${loadaddr} /etc/version
>diff --git a/doc/usage/index.rst b/doc/usage/index.rst
>index 3804046835..856a3da28e 100644
>--- a/doc/usage/index.rst
>+++ b/doc/usage/index.rst
>@@ -4,6 +4,7 @@ Use U-Boot
> .. toctree::
> :maxdepth: 1
>
>+ blkmap
> dfu
> environment
> fdt_overlays
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 0/8] blk: blkmap: Composable virtual block devices
2023-02-01 18:10 [PATCH 0/8] blk: blkmap: Composable virtual block devices Tobias Waldekranz
` (7 preceding siblings ...)
2023-02-01 18:10 ` [PATCH 8/8] doc: blkmap: Add introduction and examples Tobias Waldekranz
@ 2023-02-01 20:21 ` Simon Glass
8 siblings, 0 replies; 25+ messages in thread
From: Simon Glass @ 2023-02-01 20:21 UTC (permalink / raw)
To: Tobias Waldekranz; +Cc: xypron.glpk, ilias.apalodimas, u-boot
Hi Tobias,
On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias@waldekranz.com> wrote:
>
> Block maps are a way of looking at various sources of data through the
> lens of a regular block device. It lets you treat devices that are not
> block devices, like RAM, as if they were. It also lets you export a
> slice of an existing block device, which does not have to correspond to
> a partition boundary, as a new block device.
>
> This is primarily useful because U-Boot's filesystem drivers only
> operate on block devices, so a block map lets you access filesystems
> wherever they might be located.
>
> The implementation is loosely modeled on Linux's "Device Mapper"
> subsystem, see the kernel documentation [1] for more information.
>
> The primary use-cases are to access filesystem images stored in RAM, and
> within FIT images stored on disk. See doc/usage/blkmap.rst for more
> details.
>
> The architecture is pluggable, so adding other types of mappings should
> be quite easy.
>
> [1]: https://docs.kernel.org/admin-guide/device-mapper/index.html
>
> Tobias Waldekranz (8):
> image: Fix script execution from FIT images with external data
> cmd: blk: Allow generic read/write operations to work in sandbox
> blk: blkmap: Add basic infrastructure
> blk: blkmap: Add memory mapping support
> blk: blkmap: Add linear device mapping support
> cmd: blkmap: Add blkmap command
> test: blkmap: Add test suite
> doc: blkmap: Add introduction and examples
>
> MAINTAINERS | 9 +
> boot/image-board.c | 3 +-
> cmd/Kconfig | 19 ++
> cmd/Makefile | 1 +
> cmd/blk_common.c | 15 +-
> cmd/blkmap.c | 181 +++++++++++++
> configs/sandbox_defconfig | 1 +
> disk/part.c | 1 +
> doc/usage/blkmap.rst | 109 ++++++++
> doc/usage/index.rst | 1 +
> drivers/block/Kconfig | 18 ++
> drivers/block/Makefile | 1 +
> drivers/block/blk-uclass.c | 1 +
> drivers/block/blkmap.c | 452 +++++++++++++++++++++++++++++++
> include/blkmap.h | 21 ++
> include/dm/uclass-id.h | 1 +
> include/efi_loader.h | 4 +
> lib/efi_loader/efi_device_path.c | 30 ++
> test/py/tests/test_blkmap.py | 164 +++++++++++
> 19 files changed, 1027 insertions(+), 5 deletions(-)
> create mode 100644 cmd/blkmap.c
> create mode 100644 doc/usage/blkmap.rst
> create mode 100644 drivers/block/blkmap.c
> create mode 100644 include/blkmap.h
> create mode 100644 test/py/tests/test_blkmap.py
>
> --
> 2.34.1
>
Overall this looks good to me. I'll make suggestions on the individual patches.
The main thing to note is that device numbers are an internal thing
that I'd like to move away from, so labels are better.
Regards,
Simon
^ permalink raw reply [flat|nested] 25+ messages in thread