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

Heinrich,

On Sat, Apr 09, 2022 at 01:16:01PM +0200, Heinrich Schuchardt wrote:

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'

I wanted any changes related to lib/efi_driver to be separated from the rest of the patch series, but

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

If you want, we can fix a bisect issue either by a) "squash" patch#19 ("efi_driver: align with efi_disk-dm integration") into patch#14 ("efi_loader: disk: a helper function to create"), or b) squash patch#18 ("efi_loader: disk: not create BLK device for BLK (IF_TYPE_EFI_LOADER) devices"), in addition to patch#19, into patch#14

You can do (a) by yourself without any problem as patch#19 is the only change against lib/efi_driver/efi_block_device.c in this series. If you like (b), I will post a new version as changing the order of patch#18 will create some conflict.

-Takahiro Akashi

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; }