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

Kojima-san

[...]

/**

• 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; } 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) {
  

I am a bit lost here. Why are we using 'num' instead of 'i'? The opt variable contains all media boot options with a block_io_protocol right? Aren't we going to copy the wrong load options in optional_data if we don't use 'i'?

	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;

}

[...]

Thanks /Ilias