[PATCH 3/8] blk: blkmap: Add basic infrastructure
Simon Glass
sjg at chromium.org
Wed Feb 1 21:20:59 CET 2023
Hi Tobias,
On Wed, 1 Feb 2023 at 11:10, Tobias Waldekranz <tobias at 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 at 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 at gmail.com>
> S: Maintained
> F: tools/binman/
>
> +BLKMAP
> +M: Tobias Waldekranz <tobias at waldekranz.com>
> +S: Maintained
> +F: drivers/block/blkmap.c
> +F: include/blkmap.h
> +
> BOOTDEVICE
> M: Simon Glass <sjg at 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 at 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 at 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
More information about the U-Boot
mailing list