Re: [PATCH 1/2] efi_loader: auto-generate boot option for each blkio device

[Heinrich Schuchardt]

On 12/25/23 12:00, Heinrich Schuchardt wrote:

On 12/20/23 09:41, Masahisa Kojima wrote:

Current efibootmgr auto-generates the boot option for all
disks and partitions installing EFI_SIMPLE_FILE_SYSTEM_PROTOCOL,
while EDK II reference implementation auto-generates the boot option
for all devices installing  EFI_BLOCK_IO_PROTOCOL with
eliminating logical partitions.

This commit modifies the efibootmgr to get aligned to EDK II.

Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org>
---
  lib/efi_loader/efi_bootmgr.c | 94 +++++++++++++++++++++++++++---------
  1 file changed, 71 insertions(+), 23 deletions(-)

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 56d97f2382..636f01e2eb 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -560,6 +560,50 @@ err:
      return ret;
  }

+/**
+ * try_load_from_media() - load file from media
+ *
+ * @fp:        file path
+ * @handle:    pointer to handle for newly installed image
+ *
+ * If @fp contains a file name, load the file.
+ * If @fp does not have a file name, search the architecture-specific
+ * default file and load it.
+ * TODO: If the FilePathList[0] device does not support
+ * EFI_SIMPLE_FILE_SYSTEM_PROTOCOL but supports EFI_BLOCK_IO_PROTOCOL,
+ * call EFI_BOOT_SERVICES.ConnectController()
+ * TODO: FilePathList[0] device supports EFI_SIMPLE_FILE_SYSTEM_PROTOCOL
+ * not based on EFI_BLOCK_IO_PROTOCOL
+ *
+ * Return:    status code
+ */

+static efi_status_t try_load_from_media(struct efi_device_path *file_path,

+                    efi_handle_t *handle)
+{
+    efi_handle_t h;
+    efi_status_t ret = EFI_SUCCESS;
+    struct efi_device_path *rem, *dp = NULL;
+    struct efi_device_path *final_dp = file_path;
+
+    h = efi_dp_find_obj(file_path, &efi_block_io_guid, &rem);
+    if (h) {
+        if (rem->type == DEVICE_PATH_TYPE_END) {
+            /* no file name present, try default file */
+            ret = check_disk_has_default_file(h->dev, &dp);
+            if (ret != EFI_SUCCESS)
+                return ret;
+
+            final_dp = dp;
+        }
+    }
+

+    ret = EFI_CALL(efi_load_image(true, efi_root, final_dp, NULL, 0, handle));

+
+    efi_free_pool(dp);
+
+    return ret;
+}
+
  /**
   * try_load_entry() - try to load image for boot option
   *

@@ -594,7 +638,6 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,

      }

      if (lo.attributes & LOAD_OPTION_ACTIVE) {
-        struct efi_device_path *file_path;
          u32 attributes;

          log_debug("trying to load \"%ls\" from %pD\n", lo.label,

@@ -611,10 +654,7 @@ static efi_status_t try_load_entry(u16 n, efi_handle_t *handle,

              else
                  ret = EFI_LOAD_ERROR;

This line does not exist in efi/next. Am I missing a prerequisite patch?

My fault.

Best regards

Heinrich

          } else {
-            file_path = expand_media_path(lo.file_path);
-            ret = EFI_CALL(efi_load_image(true, efi_root, file_path,
-                              NULL, 0, handle));
-            efi_free_pool(file_path);
+            ret = try_load_from_media(lo.file_path, handle);
          }
          if (ret != EFI_SUCCESS) {
              log_warning("Loading %ls '%ls' failed\n",
@@ -748,19 +788,19 @@ error:

   * efi_bootmgr_enumerate_boot_option() - enumerate the possible bootable media

   *
   * @opt:        pointer to the media boot option structure
- * @volume_handles:    pointer to the efi handles
+ * @handles:        pointer to the efi handles
   * @count:        number of efi handle
   * Return:        status code
   */

  static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boot_option *opt,

-                              efi_handle_t *volume_handles,
-                              efi_status_t count)
+                              efi_handle_t *handles,
+                              efi_uintn_t *count)
  {
-    u32 i;
+    u32 i, num = 0;
      struct efi_handler *handler;
      efi_status_t ret = EFI_SUCCESS;

-    for (i = 0; i < count; i++) {
+    for (i = 0; i < *count; i++) {
          u16 *p;
          u16 dev_name[BOOTMENU_DEVICE_NAME_MAX];
          char *optional_data;

@@ -768,8 +808,15 @@ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo

          char buf[BOOTMENU_DEVICE_NAME_MAX];
          struct efi_device_path *device_path;
          struct efi_device_path *short_dp;
+        struct efi_block_io *blkio;
+

+        ret = efi_search_protocol(handles[i], &efi_block_io_guid, &handler);

+        blkio = handler->protocol_interface;

-        ret = efi_search_protocol(volume_handles[i], &efi_guid_device_path, &handler);

+        if (blkio->media->logical_partition)
+            continue;
+

+        ret = efi_search_protocol(handles[i], &efi_guid_device_path, &handler);

          if (ret != EFI_SUCCESS)
              continue;
          ret = efi_protocol_open(handler, (void **)&device_path,

@@ -777,7 +824,7 @@ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo

          if (ret != EFI_SUCCESS)
              continue;

-        ret = efi_disk_get_device_name(volume_handles[i], buf, BOOTMENU_DEVICE_NAME_MAX); +        ret = efi_disk_get_device_name(handles[i], buf, BOOTMENU_DEVICE_NAME_MAX);

          if (ret != EFI_SUCCESS)
              continue;

@@ -801,17 +848,20 @@ static efi_status_t efi_bootmgr_enumerate_boot_option(struct eficonfig_media_boo

           * to store guid, instead of realloc the load_option.
           */
          lo.optional_data = "1234567";
-        opt[i].size = efi_serialize_load_option(&lo, (u8 **)&opt[i].lo);
-        if (!opt[i].size) {

+        opt[num].size = efi_serialize_load_option(&lo, (u8 **)&opt[num].lo);

+        if (!opt[num].size) {
              ret = EFI_OUT_OF_RESOURCES;
              goto out;
          }
          /* set the guid */

-        optional_data = (char *)opt[i].lo + (opt[i].size - u16_strsize(u"1234567")); +        optional_data = (char *)opt[num].lo + (opt[num].size - u16_strsize(u"1234567"));           memcpy(optional_data, &efi_guid_bootmenu_auto_generated, sizeof(efi_guid_t));

+        num++;
      }

  out:
+    *count = num;
+
      return ret;
  }

@@ -1040,8 +1090,7 @@ efi_status_t efi_bootmgr_delete_boot_option(u16 boot_index)

  /**

   * efi_bootmgr_update_media_device_boot_option() - generate the media device boot option

   *

- * This function enumerates all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL

- * and generate the bootmenu entries.

+ * This function enumerates all BlockIo devices and add the boot option for it.

   * This function also provide the BOOT#### variable maintenance for
   * the media device entries.

   * - Automatically create the BOOT#### variable for the newly detected device, @@ -1057,13 +1106,13 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void)

      u32 i;
      efi_status_t ret;
      efi_uintn_t count;
-    efi_handle_t *volume_handles = NULL;
+    efi_handle_t *handles = NULL;
      struct eficonfig_media_boot_option *opt = NULL;

      ret = efi_locate_handle_buffer_int(BY_PROTOCOL,
-                       &efi_simple_file_system_protocol_guid,
+                       &efi_block_io_guid,
                         NULL, &count,
-                       (efi_handle_t **)&volume_handles);
+                       (efi_handle_t **)&handles);
      if (ret != EFI_SUCCESS)
          goto out;

@@ -1073,8 +1122,7 @@ efi_status_t efi_bootmgr_update_media_device_boot_option(void)

          goto out;
      }

-    /* enumerate all devices supporting EFI_SIMPLE_FILE_SYSTEM_PROTOCOL */

-    ret = efi_bootmgr_enumerate_boot_option(opt, volume_handles, count);
+    ret = efi_bootmgr_enumerate_boot_option(opt, handles, &count);
      if (ret != EFI_SUCCESS)
          goto out;

@@ -1122,7 +1170,7 @@ out:
              free(opt[i].lo);
      }
      free(opt);
-    efi_free_pool(volume_handles);
+    efi_free_pool(handles);

      if (ret == EFI_NOT_FOUND)
          return EFI_SUCCESS;