Re: [PATCH v3 14/19] efi_loader: disk: a helper function to create efi_disk objects from udevice

Heinrich Schuchardt
2022-04-09T11:16:30Z

On 3/8/22 12:36, AKASHI Takahiro wrote:
> Add efi_disk_probe() function.
> This function creates an efi_disk object for a raw disk device (UCLASS_BLK)
> and additional objects for related partitions (UCLASS_PARTITION).
>
> So this function is expected to be called through driver model's "probe"
> interface every time one raw disk device is detected and activated.
> We assume that partition devices (UCLASS_PARTITION) have been created
> when this function is invoked.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

Merging all patches up to this one leads to:

/lib/efi_driver/efi_block_device.c:124: undefined reference to
`efi_disk_create_partitions'

As we want to be able to bisect code, please, ensure that after each
patch the code builds.

Best regards

Heinrich

> ---
> include/efi_loader.h | 4 +-
> lib/efi_loader/Kconfig | 3 +
> lib/efi_loader/efi_disk.c | 201 +++++++++++++++++++++++--------------
> lib/efi_loader/efi_setup.c | 4 +-
> 4 files changed, 133 insertions(+), 79 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index a9075c992687..24ec692c4d09 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -523,8 +523,8 @@ void efi_carve_out_dt_rsv(void *fdt);
> void efi_try_purge_kaslr_seed(void *fdt);
> /* Called by bootefi to make console interface available */
> efi_status_t efi_console_register(void);
> -/* Called by bootefi to make all disk storage accessible as EFI objects */
> -efi_status_t efi_disk_register(void);
> +/* Called by efi_init_obj_list() to initialize efi_disks */
> +efi_status_t efi_disk_init(void);
> /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
> efi_status_t efi_rng_register(void);
> /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index e5e35fe51f65..7ffd59759359 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -14,6 +14,8 @@ config EFI_LOADER
> depends on DM_ETH || !NET
> depends on !EFI_APP
> default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
> + select DM_EVENT
> + select EVENT_DYNAMIC
> select LIB_UUID
> select PARTITION_UUIDS
> select HAVE_BLOCK_DEVICE
> @@ -41,6 +43,7 @@ config CMD_BOOTEFI_BOOTMGR
>
> config EFI_SETUP_EARLY
> bool
> + default y
>
> choice
> prompt "Store for non-volatile UEFI variables"
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 45127d176869..bf9824b9b1a4 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -10,6 +10,9 @@
> #include <common.h>
> #include <blk.h>
> #include <dm.h>
> +#include <dm/device-internal.h>
> +#include <dm/tag.h>
> +#include <event.h>
> #include <efi_loader.h>
> #include <fs.h>
> #include <log.h>
> @@ -487,103 +490,153 @@ error:
> return ret;
> }
>
> -/**
> - * efi_disk_create_partitions() - create handles and protocols for partitions
> +/*
> + * Create a handle for a whole raw disk
> + *
> + * @dev uclass device (UCLASS_BLK)
> *
> - * Create handles and protocols for the partitions of a block device.
> + * Create an efi_disk object which is associated with @dev.
> + * The type of @dev must be UCLASS_BLK.
> *
> - * @parent: handle of the parent disk
> - * @desc: block device
> - * @if_typename: interface type
> - * @diskid: device number
> - * @pdevname: device name
> - * Return: number of partitions created
> + * @return 0 on success, -1 otherwise
> */
> -int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
> - const char *if_typename, int diskid,
> - const char *pdevname)
> +static int efi_disk_create_raw(struct udevice *dev)
> {
> - int disks = 0;
> - char devname[32] = { 0 }; /* dp->str is u16[32] long */
> - int part;
> - struct efi_device_path *dp = NULL;
> + struct efi_disk_obj *disk;
> + struct blk_desc *desc;
> + const char *if_typename;
> + int diskid;
> efi_status_t ret;
> - struct efi_handler *handler;
>
> - /* Get the device path of the parent */
> - ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
> - if (ret == EFI_SUCCESS)
> - dp = handler->protocol_interface;
> -
> - /* Add devices for each partition */
> - for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
> - struct disk_partition info;
> -
> - if (part_get_info(desc, part, &info))
> - continue;
> - snprintf(devname, sizeof(devname), "%s:%x", pdevname,
> - part);
> - ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid,
> - &info, part, NULL);
> - if (ret != EFI_SUCCESS) {
> - log_err("Adding partition %s failed\n", pdevname);
> - continue;
> - }
> - disks++;
> + desc = dev_get_uclass_plat(dev);
> + if_typename = blk_get_if_type_name(desc->if_type);
> + diskid = desc->devnum;
> +
> + ret = efi_disk_add_dev(NULL, NULL, if_typename, desc,
> + diskid, NULL, 0, &disk);
> + if (ret != EFI_SUCCESS) {
> + if (ret == EFI_NOT_READY)
> + log_notice("Disk %s not ready\n", dev->name);
> + else
> + log_err("Adding disk for %s failed\n", dev->name);
> +
> + return -1;
> + }
> + if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {
> + efi_free_pool(disk->dp);
> + efi_delete_handle(&disk->header);
> +
> + return -1;
> }
>
> - return disks;
> + return 0;
> }
>
> -/**
> - * efi_disk_register() - register block devices
> - *
> - * U-Boot doesn't have a list of all online disk devices. So when running our
> - * EFI payload, we scan through all of the potentially available ones and
> - * store them in our object pool.
> +/*
> + * Create a handle for a disk partition
> *
> - * This function is called in efi_init_obj_list().
> + * @dev uclass device (UCLASS_PARTITION)
> *
> - * TODO(sjg@chromium.org): Actually with CONFIG_BLK, U-Boot does have this.
> - * Consider converting the code to look up devices as needed. The EFI device
> - * could be a child of the UCLASS_BLK block device, perhaps.
> + * Create an efi_disk object which is associated with @dev.
> + * The type of @dev must be UCLASS_PARTITION.
> *
> - * Return: status code
> + * @return 0 on success, -1 otherwise
> */
> -efi_status_t efi_disk_register(void)
> +static int efi_disk_create_part(struct udevice *dev)
> {
> + efi_handle_t parent;
> + struct blk_desc *desc;
> + const char *if_typename;
> + struct disk_part *part_data;
> + struct disk_partition *info;
> + unsigned int part;
> + int diskid;
> + struct efi_handler *handler;
> + struct efi_device_path *dp_parent;
> struct efi_disk_obj *disk;
> - int disks = 0;
> efi_status_t ret;
> +
> + if (dev_tag_get_ptr(dev_get_parent(dev), DM_TAG_EFI, (void **)&parent))
> + return -1;
> +
> + desc = dev_get_uclass_plat(dev_get_parent(dev));
> + if_typename = blk_get_if_type_name(desc->if_type);
> + diskid = desc->devnum;
> +
> + part_data = dev_get_uclass_plat(dev);
> + part = part_data->partnum;
> + info = &part_data->gpt_part_info;
> +
> + ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
> + if (ret != EFI_SUCCESS)
> + return -1;
> + dp_parent = (struct efi_device_path *)handler->protocol_interface;
> +
> + ret = efi_disk_add_dev(parent, dp_parent, if_typename, desc, diskid,
> + info, part, &disk);
> + if (ret != EFI_SUCCESS) {
> + log_err("Adding partition for %s failed\n", dev->name);
> + return -1;
> + }
> + if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {
> + efi_free_pool(disk->dp);
> + efi_delete_handle(&disk->header);
> +
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * Create efi_disk objects for a block device
> + *
> + * @dev uclass device (UCLASS_BLK)
> + *
> + * Create efi_disk objects for partitions as well as a raw disk
> + * which is associated with @dev.
> + * The type of @dev must be UCLASS_BLK.
> + * This function is expected to be called at EV_PM_POST_PROBE.
> + *
> + * @return 0 on success, -1 otherwise
> + */
> +static int efi_disk_probe(void *ctx, struct event *event)
> +{
> struct udevice *dev;
> + enum uclass_id id;
> + struct udevice *child;
> + int ret;
>
> - for (uclass_first_device_check(UCLASS_BLK, &dev); dev;
> - uclass_next_device_check(&dev)) {
> - struct blk_desc *desc = dev_get_uclass_plat(dev);
> - const char *if_typename = blk_get_if_type_name(desc->if_type);
> + dev = event->data.dm.dev;
> + id = device_get_uclass_id(dev);
>
> - /* Add block device for the full device */
> - log_info("Scanning disk %s...\n", dev->name);
> - ret = efi_disk_add_dev(NULL, NULL, if_typename,
> - desc, desc->devnum, NULL, 0, &disk);
> - if (ret == EFI_NOT_READY) {
> - log_notice("Disk %s not ready\n", dev->name);
> - continue;
> - }
> - if (ret) {
> - log_err("ERROR: failure to add disk device %s, r = %lu\n",
> - dev->name, ret & ~EFI_ERROR_MASK);
> - continue;
> - }
> - disks++;
> + /* TODO: We won't support partitions in a partition */
> + if (id != UCLASS_BLK)
> + return 0;
> +
> + ret = efi_disk_create_raw(dev);
> + if (ret)
> + return -1;
>
> - /* Partitions show up as block devices in EFI */
> - disks += efi_disk_create_partitions(
> - &disk->header, desc, if_typename,
> - desc->devnum, dev->name);
> + device_foreach_child(child, dev) {
> + ret = efi_disk_create_part(child);
> + if (ret)
> + return -1;
> }
>
> - log_info("Found %d disks\n", disks);
> + return 0;
> +}
> +
> +efi_status_t efi_disk_init(void)
> +{
> + int ret;
> +
> + ret = event_register("efi_disk add", EVT_DM_POST_PROBE,
> + efi_disk_probe, NULL);
> + if (ret) {
> + log_err("Event registration for efi_disk add failed\n");
> + return EFI_OUT_OF_RESOURCES;
> + }
>
> return EFI_SUCCESS;
> }
> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> index de2f34bab537..250eeb2fcd6b 100644
> --- a/lib/efi_loader/efi_setup.c
> +++ b/lib/efi_loader/efi_setup.c
> @@ -198,9 +198,7 @@ static efi_status_t __efi_init_early(void)
> if (ret != EFI_SUCCESS)
> goto out;
>
> -#ifdef CONFIG_PARTITIONS
> - ret = efi_disk_register();
> -#endif
> + ret = efi_disk_init();
> out:
> return ret;
> }