[PATCH v11 7/9] efi_loader: support boot from URI device path

[Masahisa Kojima]

On Thu, 9 Nov 2023 at 17:04, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Kojima-san
>
> On Wed, 8 Nov 2023 at 13:08, Masahisa Kojima <masahisa.kojima at linaro.org> wrote:
> >
> > This supports to boot from the URI device path.
> > When user selects the URI device path, bootmgr downloads
> > the file using wget into the address specified by loadaddr
> > env variable.
> > If the file is .iso or .img file, mount the image with blkmap
> > then try to boot with the default file(e.g. EFI/BOOT/BOOTAA64.EFI).
> > Since boot option indicating the default file is automatically
> > created when new disk is detected, system can boot by selecting
> > the automatically created blkmap boot option.
> > If the file is PE-COFF file, load and start the downloaded file.
> >
> > The buffer used to download the ISO image file must be
> > reserved to avoid the unintended access to the image and
> > expose the ramdisk to the OS.
> > For PE-COFF file case, this memory reservation is done
> > in LoadImage Boot Service.
> >
> > Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> > ---
> >  lib/efi_loader/Kconfig       |   9 +
> >  lib/efi_loader/efi_bootmgr.c | 376 +++++++++++++++++++++++++++++++++++
> >  2 files changed, 385 insertions(+)
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index d20aaab6db..5d99206dc3 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -479,4 +479,13 @@ config EFI_RISCV_BOOT_PROTOCOL
> >           replace the transfer via the device-tree. The latter is not
> >           possible on systems using ACPI.
> >
> > +config EFI_HTTP_BOOT
> > +       bool "EFI HTTP Boot support"
> > +       depends on CMD_DNS
> > +       depends on CMD_WGET
> > +       depends on BLKMAP
> > +       help
> > +         Enabling this option adds EFI HTTP Boot support. It allows to
> > +         directly boot from network.
> > +
> >  endif
>
> Since we depend on all these, I think it would be easier to 'select'
> instead of 'depends on', otherwise enabling EFI_HTTP_BOOT will be very
> tricky.

Yes, I agree.

>
>
>
>
> > + */
> > +static efi_status_t search_default_file(struct udevice *dev,
> > +                                       struct efi_device_path **loaded_dp)
> > +{
> > +       efi_status_t ret;
> > +       efi_handle_t handle;
> > +       u16 *default_file_name = NULL;
> > +       struct efi_file_handle *root, *f;
> > +       struct efi_device_path *dp = NULL, *fp = NULL;
> > +       struct efi_simple_file_system_protocol *file_system;
> > +       struct efi_device_path *device_path, *full_path = NULL;
> > +
> > +       if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle)) {
> > +               log_warning("DM_TAG_EFI not found\n");
> > +               return EFI_INVALID_PARAMETER;
> > +       }
> > +
> > +       ret = EFI_CALL(bs->open_protocol(handle, &efi_simple_file_system_protocol_guid,
> > +                                        (void **)&file_system, efi_root, NULL,
> > +                                        EFI_OPEN_PROTOCOL_GET_PROTOCOL));
> > +       if (ret != EFI_SUCCESS)
> > +               return ret;
> > +
> > +       ret = EFI_CALL(file_system->open_volume(file_system, &root));
> > +       if (ret != EFI_SUCCESS)
> > +               return ret;
> > +
> > +       ret = EFI_CALL(bs->open_protocol(handle, &efi_guid_device_path,
> > +                                        (void **)&device_path, efi_root, NULL,
> > +                                        EFI_OPEN_PROTOCOL_GET_PROTOCOL));
> > +       if (ret != EFI_SUCCESS)
> > +               return ret;
> > +
> > +       full_path = expand_media_path(device_path);
> > +       ret = efi_dp_split_file_path(full_path, &dp, &fp);
> > +       if (ret != EFI_SUCCESS)
> > +               goto err;
> > +
> > +       default_file_name = efi_dp_str(fp);
> > +       efi_free_pool(dp);
> > +       efi_free_pool(fp);
> > +       if (!default_file_name) {
> > +               ret = EFI_OUT_OF_RESOURCES;
> > +               goto err;
> > +       }
> > +
> > +       ret = EFI_CALL(root->open(root, &f, default_file_name,
> > +                                 EFI_FILE_MODE_READ, 0));
> > +       efi_free_pool(default_file_name);
> > +       if (ret != EFI_SUCCESS)
> > +               goto err;
> > +
> > +       EFI_CALL(f->close(f));\
>
> This closes the file correctly, but the volume is still open. Don't we need
> EFI_CALL(root->close(root) as well?

Thank you for pointing it out.
I will fix it.

>
> [...]
>
> With the above fixed
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>

Thanks,
Masahisa Kojima