[PATCH v5 07/12] efi_loader: disk: a helper function to create efi_disk objects from udevice

Neil Armstrong narmstrong at baylibre.com
Wed May 18 11:23:32 CEST 2022


On 19/04/2022 03:05, 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 at linaro.org>
> ---
>   include/efi_loader.h              |   4 +-
>   lib/efi_driver/efi_block_device.c |  34 ++---
>   lib/efi_loader/Kconfig            |   3 +
>   lib/efi_loader/efi_disk.c         | 201 +++++++++++++++++++-----------
>   lib/efi_loader/efi_setup.c        |   4 +-
>   5 files changed, 143 insertions(+), 103 deletions(-)
> 
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 5bb8fb2acd04..ba79a9afb404 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -525,8 +525,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_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c
> index 04cb3ef0d4e5..5baa6f87a375 100644
> --- a/lib/efi_driver/efi_block_device.c
> +++ b/lib/efi_driver/efi_block_device.c
> @@ -35,6 +35,7 @@
>   #include <malloc.h>
>   #include <dm/device-internal.h>
>   #include <dm/root.h>
> +#include <dm/tag.h>
>   
>   /*
>    * EFI attributes of the udevice handled by this driver.
> @@ -106,25 +107,6 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>   	return blkcnt;
>   }
>   
> -/**
> - * Create partions for the block device.
> - *
> - * @handle:	EFI handle of the block device
> - * @dev:	udevice of the block device
> - * Return:	number of partitions created
> - */
> -static int efi_bl_bind_partitions(efi_handle_t handle, struct udevice *dev)
> -{
> -	struct blk_desc *desc;
> -	const char *if_typename;
> -
> -	desc = dev_get_uclass_plat(dev);
> -	if_typename = blk_get_if_type_name(desc->if_type);
> -
> -	return efi_disk_create_partitions(handle, desc, if_typename,
> -					  desc->devnum, dev->name);
> -}
> -
>   /**
>    * Create a block device for a handle
>    *
> @@ -139,7 +121,6 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
>   	char *name;
>   	struct efi_object *obj = efi_search_obj(handle);
>   	struct efi_block_io *io = interface;
> -	int disks;
>   	struct efi_blk_plat *plat;
>   
>   	EFI_PRINT("%s: handle %p, interface %p\n", __func__, handle, io);
> @@ -173,15 +154,20 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
>   	plat->handle = handle;
>   	plat->io = interface;
>   
> +	/*
> +	 * FIXME: necessary because we won't do almost nothing in
> +	 * efi_disk_create() when called from device_probe().
> +	 */
> +	ret = dev_tag_set_ptr(bdev, DM_TAG_EFI, handle);
> +	if (ret)
> +		/* FIXME: cleanup for bdev */
> +		return ret;
> +
>   	ret = device_probe(bdev);
>   	if (ret)
>   		return ret;
>   	EFI_PRINT("%s: block device '%s' created\n", __func__, bdev->name);
>   
> -	/* Create handles for the partions of the block device */
> -	disks = efi_bl_bind_partitions(handle, bdev);
> -	EFI_PRINT("Found %d partitions\n", disks);
> -
>   	return 0;
>   }
>   
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index bc518d7a413b..6b245f50a726 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
>   	imply PARTITION_UUIDS
>   	select HAVE_BLOCK_DEVICE
> @@ -40,6 +42,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 c905c12abc2f..d4a0edb458b8 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 at 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;
>   }

This change as commit a9bf024b2933bba0e23038892970a18b72dfaeb4 in master makes Amlogic S905X board libretech-cc to crash:
Found U-Boot script /boot.scr
449 bytes read in 2 ms (218.8 KiB/s)
## Executing script at 08000000
36500032 bytes read in 1554 ms (22.4 MiB/s)
42873252 bytes read in 1829 ms (22.4 MiB/s)
29092 bytes read in 4 ms (6.9 MiB/s)
## Booting kernel from Legacy Image at 13000000 ...
    Image Name:
    Image Type:   AArch64 Linux Kernel Image (uncompressed)
    Data Size:    36499968 Bytes = 34.8 MiB
    Load Address: 13000000
    Entry Point:  13000000
    Verifying Checksum ... OK
## Loading init Ramdisk from Legacy Image at 06000000 ...
    Image Name:
    Image Type:   AArch64 Linux RAMDisk Image (uncompressed)
    Data Size:    42873188 Bytes = 40.9 MiB
    Load Address: 00000000
    Entry Point:  00000000
    Verifying Checksum ... OK
## Flattened Device Tree blob at 09000000
    Booting using the fdt blob at 0x9000000
    Loading Kernel Image
    Loading Ramdisk to 7965b000, end 7bf3e164 ... OK
    Loading Device Tree to 0000000079650000, end 000000007965a1a3 ... OK
"Synchronous Abort" handler, esr 0x96000004
elr: 0000000001065150 lr : 0000000001054b18 (reloc)
elr: 000000007dfb5150 lr : 000000007dfa4b18
x0 : d55d507f5dacfff5 x1 : 000000007dfbce48
x2 : 0000000000000010 x3 : 0000000000000000
x4 : 0000000000000000 x5 : d55d507f5dacfff5
x6 : 000000007bf55bb0 x7 : 0000000000000000
x8 : 0000000000000007 x9 : 0000000000000000
x10: 0000000000000178 x11: 000000007bf4211c
x12: 00000000000000a4 x13: 000000007bf420d8
x14: 0000000079650000 x15: 0000000000000020
x16: 000000007df83454 x17: 0000000000000000
x18: 000000007bf4ddb0 x19: 000000007af42040
x20: 000000007df50b20 x21: 000000007dfbce48
x22: 0000000000001000 x23: 000000007bf55b00
x24: 000000007dfdb910 x25: 0000000001000000
x26: 0000000001000000 x27: 0000000001000000
x28: 0000000000001000 x29: 000000007bf420d0

Code: eb02009f 54000061 52800000 14000006 (386468a3)
Resetting CPU ...

using libretech-cc_defconfig and the followin boot.scr:
setenv autoload no
setenv initrd_high 0xffffffff
setenv fdt_high 0xffffffff
load mmc 0:1 0x13000000 uImage
load mmc 0:1 0x6000000 ramdisk.cpio.gz.uboot
setenv initrd_size ${filesize}
load mmc 0:1 0x9000000 meson-gxl-s905x-libretech-cc.dtb
setenv bootargs 'console=ttyAML0,115200n8 root=/dev/ram0 console_msg_format=syslog earlycon ip=dhcp'
bootm 0x13000000 0x6000000 0x9000000

bisect log:
# bad: [c387e62614713d0cc9e3ed022b86c9f320b02853] Merge branch '2022-05-11-Kconfig-cleanups-etc'
# good: [e4b6ebd3de982ae7185dbf689a030e73fd06e0d2] Prepare v2022.04
git bisect start 'c387e62614' 'v2022.04'
# good: [4228023eff7e94500137ce9e30687c00ffb4dd4f] led: bcm6753: Drop duplicate OF "label" property parsing
git bisect good 4228023eff7e94500137ce9e30687c00ffb4dd4f
# bad: [3fbdd485fd87ce0d4e1b747aea3c433646f4ced2] ARM: dts: sam9x60: Add pit64b node
git bisect bad 3fbdd485fd87ce0d4e1b747aea3c433646f4ced2
# good: [e50f66e364be80e02dd0834b84b830f3aade82ea] Merge https://source.denx.de/u-boot/custodians/u-boot-marvell
git bisect good e50f66e364be80e02dd0834b84b830f3aade82ea
# good: [9de612ae4ded53f742f5f99929c06d0839471ced] cmd: adc: Add support for storing ADC result in env variable
git bisect good 9de612ae4ded53f742f5f99929c06d0839471ced
# bad: [bc9da9fb50ac3ba7603487a0366d4db60b984812] Merge branch '2022-04-23-binman-updates'
git bisect bad bc9da9fb50ac3ba7603487a0366d4db60b984812
# bad: [3c809dfed757c37b92ff543c8f1c941407bafc2e] efi_loader: disk: not create BLK device for BLK(IF_TYPE_EFI_LOADER) devices
git bisect bad 3c809dfed757c37b92ff543c8f1c941407bafc2e
# good: [38f255b96085d2e50558a16256c87ffd885f2f7e] efi_loader: disk: compile efi_disk when CONFIG_BLK
git bisect good 38f255b96085d2e50558a16256c87ffd885f2f7e
# good: [8ff50227befdc778eb2cb239b778ed1ed843bf33] test: dm: add tests for tag support
git bisect good 8ff50227befdc778eb2cb239b778ed1ed843bf33
# good: [bf76031d19fa82041ae5ddc17214ec71858b0097] dm: blk: add a device-probe hook for scanning disk partitions
git bisect good bf76031d19fa82041ae5ddc17214ec71858b0097
# bad: [a9bf024b2933bba0e23038892970a18b72dfaeb4] efi_loader: disk: a helper function to create efi_disk objects from udevice
git bisect bad a9bf024b2933bba0e23038892970a18b72dfaeb4
# good: [a57ad20d07e82f9ddbbdf981c8f8dd5368b225e4] efi_loader: split efi_init_obj_list() into two stages
git bisect good a57ad20d07e82f9ddbbdf981c8f8dd5368b225e4
# first bad commit: [a9bf024b2933bba0e23038892970a18b72dfaeb4] efi_loader: disk: a helper function to create efi_disk objects from udevice


Neil



More information about the U-Boot mailing list