On Thu, 9 Nov 2023 at 17:04, Ilias Apalodimas
<ilias.apalodi...@linaro.org> wrote:
>
> Kojima-san
>
> On Wed, 8 Nov 2023 at 13:08, Masahisa Kojima <masahisa.koj...@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.koj...@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.apalodi...@linaro.org>
Thanks,
Masahisa Kojima
