Hi Tobias, On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tob...@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 <tob...@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 <alpernebiya...@gmail.com> > S: Maintained > F: tools/binman/ > > +BLKMAP > +M: Tobias Waldekranz <tob...@waldekranz.com> > +S: Maintained > +F: drivers/block/blkmap.c > +F: include/blkmap.h > + > BOOTDEVICE > M: Simon Glass <s...@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 <tob...@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 <tob...@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