[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