Hi Ilias,
On Thu, 28 Dec 2023 at 22:14, Ilias Apalodimas
<ilias.apalodi...@linaro.org> wrote:
>
> 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'?
opt[] array is a container of valid boot options with a block_io_protocol
to be auto-generated by efi bootmgr.
handles[] array contains all block_io_protocol efi handles.
With this patch, we ignore the logical partition, opt[] array does not contain
the boot options of logical partitions.
So 'num' instead of 'i' is correct here.
Thanks,
Masahisa Kojima
>
> > 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
