Re: [U-Boot] [RFC v3 09/10] efi_loader: rework bootmgr/bootefi using load_image API
On 16.04.19 18:20, Heinrich Schuchardt wrote:
> On 4/16/19 12:56 PM, Heinrich Schuchardt wrote:
>> On 4/16/19 6:24 AM, AKASHI Takahiro wrote:
>>> In the current implementation, bootefi command and EFI boot manager
>>> don't use load_image API, instead, use more primitive and internal
>>> functions. This will introduce duplicated code and potentially
>>> unknown bugs as well as inconsistent behaviours.
>>>
>>> With this patch, do_efibootmgr() and do_boot_efi() are completely
>>> overhauled and re-implemented using load_image API.
>>>
>>> Signed-off-by: AKASHI Takahiro
>>> ---
>>> cmd/bootefi.c | 197
>>> +++---
>>> include/efi_loader.h | 5 +-
>>> lib/efi_loader/efi_bootmgr.c | 43
>>> lib/efi_loader/efi_boottime.c | 2 +
>>> 4 files changed, 134 insertions(+), 113 deletions(-)
>>>
>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>>> index f14966961b23..97d49a53a44b 100644
>>> --- a/cmd/bootefi.c
>>> +++ b/cmd/bootefi.c
>>> @@ -222,88 +222,39 @@ static efi_status_t efi_install_fdt(const char
>>> *fdt_opt)
>>> /**
>>> * do_bootefi_exec() - execute EFI binary
>>> *
>>> - * @efi: address of the binary
>>> - * @device_path: path of the device from which the binary was
>>> loaded
>>> - * @image_path: device path of the binary
>>> + * @handle: handle of loaded image
>>> * Return: status code
>>> *
>>> * Load the EFI binary into a newly assigned memory unwinding the
>>> relocation
>>> * information, install the loaded image protocol, and call the
>>> binary.
>>> */
>>> -static efi_status_t do_bootefi_exec(void *efi,
>>> - struct efi_device_path *device_path,
>>> - struct efi_device_path *image_path)
>>> +static efi_status_t do_bootefi_exec(efi_handle_t handle)
>>> {
>>> - efi_handle_t mem_handle = NULL;
>>> - struct efi_device_path *memdp = NULL;
>>> - efi_status_t ret;
>>> - struct efi_loaded_image_obj *image_obj = NULL;
>>> struct efi_loaded_image *loaded_image_info = NULL;
>>> -
>>> - /*
>>> - * Special case for efi payload not loaded from disk, such as
>>> - * 'bootefi hello' or for example payload loaded directly into
>>> - * memory via JTAG, etc:
>>> - */
>>> - if (!device_path && !image_path) {
>>> - printf("WARNING: using memory device/image path, this may
>>> confuse some payloads!\n");
>>> - /* actual addresses filled in after efi_load_pe() */
>>> - memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0);
>>> - device_path = image_path = memdp;
>>> - /*
>>> - * Grub expects that the device path of the loaded image is
>>> - * installed on a handle.
>>> - */
>>> - ret = efi_create_handle(&mem_handle);
>>> - if (ret != EFI_SUCCESS)
>>> - return ret; /* TODO: leaks device_path */
>>> - ret = efi_add_protocol(mem_handle, &efi_guid_device_path,
>>> - device_path);
>>> - if (ret != EFI_SUCCESS)
>>> - goto err_add_protocol;
>>> - } else {
>>> - assert(device_path && image_path);
>>> - }
>>> -
>>> - ret = efi_setup_loaded_image(device_path, image_path, &image_obj,
>>> - &loaded_image_info);
>>> - if (ret)
>>> - goto err_prepare;
>>> + efi_status_t ret;
>>>
>>> /* Transfer environment variable as load options */
>>> - set_load_options(loaded_image_info, "bootargs");
>>
>> In set_load_options() an error could occur (out of memory). So I think
>> it should return a status.
>>
>>> -
>>> - /* Load the EFI payload */
>>> - ret = efi_load_pe(image_obj, efi, loaded_image_info);
>>> + ret = EFI_CALL(systab.boottime->open_protocol(
>>> + handle,
>>> + &efi_guid_loaded_image,
>>> + (void **)&loaded_image_info,
>>> + NULL, NULL,
>>> + EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL));
>>
>> Shouldn't we move this to set_load_options()?
>>
>>> if (ret != EFI_SUCCESS)
>>> - goto err_prepare;
>>> -
>>> - if (memdp) {
>>> - struct efi_device_path_memory *mdp = (void *)memdp;
>>> - mdp->memory_type = loaded_image_info->image_code_type;
>>> - mdp->start_address = (uintptr_t)loaded_image_info->image_base;
>>> - mdp->end_address = mdp->start_address +
>>> - loaded_image_info->image_size;
>>> - }
>>> + goto err;
>>> + set_load_options(loaded_image_info, "bootargs");
>>>
>>> /* we don't support much: */
>>>
>>> env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported",
>>>
>>> "{ro,boot}(blob)");
>>
>> Shouldn't this move to efi_setup.c? Probably we should also set
>> OsIndications. I would prefer using efi_set_variable(). I think moving
>> this could be done in a preparatory patch.
>>
>>>
>>> /* Call our payload! */
>>> -
Re: [U-Boot] [RFC v3 09/10] efi_loader: rework bootmgr/bootefi using load_image API
Correction:
On Thu, Apr 18, 2019 at 09:13:52AM +0900, AKASHI Takahiro wrote:
> On Tue, Apr 16, 2019 at 12:56:32PM +0200, Heinrich Schuchardt wrote:
> > On 4/16/19 6:24 AM, AKASHI Takahiro wrote:
> > >In the current implementation, bootefi command and EFI boot manager
> > >don't use load_image API, instead, use more primitive and internal
> > >functions. This will introduce duplicated code and potentially
> > >unknown bugs as well as inconsistent behaviours.
> > >
> > >With this patch, do_efibootmgr() and do_boot_efi() are completely
> > >overhauled and re-implemented using load_image API.
> > >
> > >Signed-off-by: AKASHI Takahiro
> > >---
> > > cmd/bootefi.c | 197 +++---
> > > include/efi_loader.h | 5 +-
> > > lib/efi_loader/efi_bootmgr.c | 43
> > > lib/efi_loader/efi_boottime.c | 2 +
> > > 4 files changed, 134 insertions(+), 113 deletions(-)
> > >
> > >diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > >index f14966961b23..97d49a53a44b 100644
> > >--- a/cmd/bootefi.c
> > >+++ b/cmd/bootefi.c
> > >@@ -222,88 +222,39 @@ static efi_status_t efi_install_fdt(const char
> > >*fdt_opt)
> > > /**
> > > * do_bootefi_exec() - execute EFI binary
> > > *
> > >- * @efi: address of the binary
> > >- * @device_path: path of the device from which the binary was loaded
> > >- * @image_path: device path of the binary
> > >+ * @handle: handle of loaded image
> > > * Return: status code
> > > *
> > > * Load the EFI binary into a newly assigned memory unwinding the
> > > relocation
> > > * information, install the loaded image protocol, and call the binary.
> > > */
> > >-static efi_status_t do_bootefi_exec(void *efi,
> > >- struct efi_device_path *device_path,
> > >- struct efi_device_path *image_path)
> > >+static efi_status_t do_bootefi_exec(efi_handle_t handle)
> > > {
> > >- efi_handle_t mem_handle = NULL;
> > >- struct efi_device_path *memdp = NULL;
> > >- efi_status_t ret;
> > >- struct efi_loaded_image_obj *image_obj = NULL;
> > > struct efi_loaded_image *loaded_image_info = NULL;
> > >-
> > >- /*
> > >- * Special case for efi payload not loaded from disk, such as
> > >- * 'bootefi hello' or for example payload loaded directly into
> > >- * memory via JTAG, etc:
> > >- */
> > >- if (!device_path && !image_path) {
> > >- printf("WARNING: using memory device/image path, this may
> > >confuse some payloads!\n");
> > >- /* actual addresses filled in after efi_load_pe() */
> > >- memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0);
> > >- device_path = image_path = memdp;
> > >- /*
> > >- * Grub expects that the device path of the loaded image is
> > >- * installed on a handle.
> > >- */
> > >- ret = efi_create_handle(&mem_handle);
> > >- if (ret != EFI_SUCCESS)
> > >- return ret; /* TODO: leaks device_path */
> > >- ret = efi_add_protocol(mem_handle, &efi_guid_device_path,
> > >- device_path);
> > >- if (ret != EFI_SUCCESS)
> > >- goto err_add_protocol;
> > >- } else {
> > >- assert(device_path && image_path);
> > >- }
> > >-
> > >- ret = efi_setup_loaded_image(device_path, image_path, &image_obj,
> > >- &loaded_image_info);
> > >- if (ret)
> > >- goto err_prepare;
> > >+ efi_status_t ret;
> > >
> > > /* Transfer environment variable as load options */
> > >- set_load_options(loaded_image_info, "bootargs");
> >
> > In set_load_options() an error could occur (out of memory). So I think
> > it should return a status.
>
> I didn't change anything in set_load_options(), but I will follow
> your comment.
>
> > >-
> > >- /* Load the EFI payload */
> > >- ret = efi_load_pe(image_obj, efi, loaded_image_info);
> > >+ ret = EFI_CALL(systab.boottime->open_protocol(
> > >+ handle,
> > >+ &efi_guid_loaded_image,
> > >+ (void **)&loaded_image_info,
> > >+ NULL, NULL,
> > >+ EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL));
> >
> > Shouldn't we move this to set_load_options()?
>
> No. This line has nothing to do with "load options."
Wrong, I will follow your comment.
This change, however, uncovers one issue: Who is responsible
for freeing loaded_image_info->load_option, particularly,
when efi_exit()/unload_image() is fully implemented?
In such a case, we have no chance to access this handle,
hence loaded_image_info, after returning from efi_start_image().
Thanks,
-Takahiro Akashi
> > > if (ret != EFI_SUCCESS)
> > >- goto err_prepare;
> > >-
> > >- if (memdp) {
> > >- struct efi_device_path_memory *mdp = (void *)m
Re: [U-Boot] [RFC v3 09/10] efi_loader: rework bootmgr/bootefi using load_image API
On Tue, Apr 16, 2019 at 12:56:32PM +0200, Heinrich Schuchardt wrote:
> On 4/16/19 6:24 AM, AKASHI Takahiro wrote:
> >In the current implementation, bootefi command and EFI boot manager
> >don't use load_image API, instead, use more primitive and internal
> >functions. This will introduce duplicated code and potentially
> >unknown bugs as well as inconsistent behaviours.
> >
> >With this patch, do_efibootmgr() and do_boot_efi() are completely
> >overhauled and re-implemented using load_image API.
> >
> >Signed-off-by: AKASHI Takahiro
> >---
> > cmd/bootefi.c | 197 +++---
> > include/efi_loader.h | 5 +-
> > lib/efi_loader/efi_bootmgr.c | 43
> > lib/efi_loader/efi_boottime.c | 2 +
> > 4 files changed, 134 insertions(+), 113 deletions(-)
> >
> >diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> >index f14966961b23..97d49a53a44b 100644
> >--- a/cmd/bootefi.c
> >+++ b/cmd/bootefi.c
> >@@ -222,88 +222,39 @@ static efi_status_t efi_install_fdt(const char
> >*fdt_opt)
> > /**
> > * do_bootefi_exec() - execute EFI binary
> > *
> >- * @efi:address of the binary
> >- * @device_path:path of the device from which the binary was loaded
> >- * @image_path: device path of the binary
> >+ * @handle: handle of loaded image
> > * Return: status code
> > *
> > * Load the EFI binary into a newly assigned memory unwinding the
> > relocation
> > * information, install the loaded image protocol, and call the binary.
> > */
> >-static efi_status_t do_bootefi_exec(void *efi,
> >-struct efi_device_path *device_path,
> >-struct efi_device_path *image_path)
> >+static efi_status_t do_bootefi_exec(efi_handle_t handle)
> > {
> >-efi_handle_t mem_handle = NULL;
> >-struct efi_device_path *memdp = NULL;
> >-efi_status_t ret;
> >-struct efi_loaded_image_obj *image_obj = NULL;
> > struct efi_loaded_image *loaded_image_info = NULL;
> >-
> >-/*
> >- * Special case for efi payload not loaded from disk, such as
> >- * 'bootefi hello' or for example payload loaded directly into
> >- * memory via JTAG, etc:
> >- */
> >-if (!device_path && !image_path) {
> >-printf("WARNING: using memory device/image path, this may
> >confuse some payloads!\n");
> >-/* actual addresses filled in after efi_load_pe() */
> >-memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0);
> >-device_path = image_path = memdp;
> >-/*
> >- * Grub expects that the device path of the loaded image is
> >- * installed on a handle.
> >- */
> >-ret = efi_create_handle(&mem_handle);
> >-if (ret != EFI_SUCCESS)
> >-return ret; /* TODO: leaks device_path */
> >-ret = efi_add_protocol(mem_handle, &efi_guid_device_path,
> >- device_path);
> >-if (ret != EFI_SUCCESS)
> >-goto err_add_protocol;
> >-} else {
> >-assert(device_path && image_path);
> >-}
> >-
> >-ret = efi_setup_loaded_image(device_path, image_path, &image_obj,
> >- &loaded_image_info);
> >-if (ret)
> >-goto err_prepare;
> >+efi_status_t ret;
> >
> > /* Transfer environment variable as load options */
> >-set_load_options(loaded_image_info, "bootargs");
>
> In set_load_options() an error could occur (out of memory). So I think
> it should return a status.
I didn't change anything in set_load_options(), but I will follow
your comment.
> >-
> >-/* Load the EFI payload */
> >-ret = efi_load_pe(image_obj, efi, loaded_image_info);
> >+ret = EFI_CALL(systab.boottime->open_protocol(
> >+handle,
> >+&efi_guid_loaded_image,
> >+(void **)&loaded_image_info,
> >+NULL, NULL,
> >+EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL));
>
> Shouldn't we move this to set_load_options()?
No. This line has nothing to do with "load options."
> > if (ret != EFI_SUCCESS)
> >-goto err_prepare;
> >-
> >-if (memdp) {
> >-struct efi_device_path_memory *mdp = (void *)memdp;
> >-mdp->memory_type = loaded_image_info->image_code_type;
> >-mdp->start_address = (uintptr_t)loaded_image_info->image_base;
> >-mdp->end_address = mdp->start_address +
> >-loaded_image_info->image_size;
> >-}
> >+goto err;
> >+set_load_options(loaded_image_info, "bootargs");
> >
> > /* we don't support much: */
> >
> > env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported",
> > "{ro,boot}(blob)");
>
>
Re: [U-Boot] [RFC v3 09/10] efi_loader: rework bootmgr/bootefi using load_image API
On 4/16/19 12:56 PM, Heinrich Schuchardt wrote:
On 4/16/19 6:24 AM, AKASHI Takahiro wrote:
In the current implementation, bootefi command and EFI boot manager
don't use load_image API, instead, use more primitive and internal
functions. This will introduce duplicated code and potentially
unknown bugs as well as inconsistent behaviours.
With this patch, do_efibootmgr() and do_boot_efi() are completely
overhauled and re-implemented using load_image API.
Signed-off-by: AKASHI Takahiro
---
cmd/bootefi.c | 197 +++---
include/efi_loader.h | 5 +-
lib/efi_loader/efi_bootmgr.c | 43
lib/efi_loader/efi_boottime.c | 2 +
4 files changed, 134 insertions(+), 113 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index f14966961b23..97d49a53a44b 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -222,88 +222,39 @@ static efi_status_t efi_install_fdt(const char
*fdt_opt)
/**
* do_bootefi_exec() - execute EFI binary
*
- * @efi: address of the binary
- * @device_path: path of the device from which the binary was loaded
- * @image_path: device path of the binary
+ * @handle: handle of loaded image
* Return: status code
*
* Load the EFI binary into a newly assigned memory unwinding the
relocation
* information, install the loaded image protocol, and call the binary.
*/
-static efi_status_t do_bootefi_exec(void *efi,
- struct efi_device_path *device_path,
- struct efi_device_path *image_path)
+static efi_status_t do_bootefi_exec(efi_handle_t handle)
{
- efi_handle_t mem_handle = NULL;
- struct efi_device_path *memdp = NULL;
- efi_status_t ret;
- struct efi_loaded_image_obj *image_obj = NULL;
struct efi_loaded_image *loaded_image_info = NULL;
-
- /*
- * Special case for efi payload not loaded from disk, such as
- * 'bootefi hello' or for example payload loaded directly into
- * memory via JTAG, etc:
- */
- if (!device_path && !image_path) {
- printf("WARNING: using memory device/image path, this may
confuse some payloads!\n");
- /* actual addresses filled in after efi_load_pe() */
- memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0);
- device_path = image_path = memdp;
- /*
- * Grub expects that the device path of the loaded image is
- * installed on a handle.
- */
- ret = efi_create_handle(&mem_handle);
- if (ret != EFI_SUCCESS)
- return ret; /* TODO: leaks device_path */
- ret = efi_add_protocol(mem_handle, &efi_guid_device_path,
- device_path);
- if (ret != EFI_SUCCESS)
- goto err_add_protocol;
- } else {
- assert(device_path && image_path);
- }
-
- ret = efi_setup_loaded_image(device_path, image_path, &image_obj,
- &loaded_image_info);
- if (ret)
- goto err_prepare;
+ efi_status_t ret;
/* Transfer environment variable as load options */
- set_load_options(loaded_image_info, "bootargs");
In set_load_options() an error could occur (out of memory). So I think
it should return a status.
-
- /* Load the EFI payload */
- ret = efi_load_pe(image_obj, efi, loaded_image_info);
+ ret = EFI_CALL(systab.boottime->open_protocol(
+ handle,
+ &efi_guid_loaded_image,
+ (void **)&loaded_image_info,
+ NULL, NULL,
+ EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL));
Shouldn't we move this to set_load_options()?
if (ret != EFI_SUCCESS)
- goto err_prepare;
-
- if (memdp) {
- struct efi_device_path_memory *mdp = (void *)memdp;
- mdp->memory_type = loaded_image_info->image_code_type;
- mdp->start_address = (uintptr_t)loaded_image_info->image_base;
- mdp->end_address = mdp->start_address +
- loaded_image_info->image_size;
- }
+ goto err;
+ set_load_options(loaded_image_info, "bootargs");
/* we don't support much: */
env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported",
"{ro,boot}(blob)");
Shouldn't this move to efi_setup.c? Probably we should also set
OsIndications. I would prefer using efi_set_variable(). I think moving
this could be done in a preparatory patch.
/* Call our payload! */
- debug("%s: Jumping to 0x%p\n", __func__, image_obj->entry);
- ret = EFI_CALL(efi_start_image(&image_obj->header, NULL, NULL));
+ ret = EFI_CALL(efi_start_image(handle, NULL, NULL));
-err_prepare:
- /* image has returned, loaded-image obj goes *poof*: */
efi_restore_gd();
free(loaded_image_info->load_options);
- efi_delete_handle(&image_obj->header);
-
-err_add_protocol:
- if (mem_handle)
- efi_delete_handle(mem_handle);
+err:
return ret;
}
@@ -317,8 +268,7 @
Re: [U-Boot] [RFC v3 09/10] efi_loader: rework bootmgr/bootefi using load_image API
On 4/16/19 6:24 AM, AKASHI Takahiro wrote:
In the current implementation, bootefi command and EFI boot manager
don't use load_image API, instead, use more primitive and internal
functions. This will introduce duplicated code and potentially
unknown bugs as well as inconsistent behaviours.
With this patch, do_efibootmgr() and do_boot_efi() are completely
overhauled and re-implemented using load_image API.
Signed-off-by: AKASHI Takahiro
---
cmd/bootefi.c | 197 +++---
include/efi_loader.h | 5 +-
lib/efi_loader/efi_bootmgr.c | 43
lib/efi_loader/efi_boottime.c | 2 +
4 files changed, 134 insertions(+), 113 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index f14966961b23..97d49a53a44b 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -222,88 +222,39 @@ static efi_status_t efi_install_fdt(const char *fdt_opt)
/**
* do_bootefi_exec() - execute EFI binary
*
- * @efi: address of the binary
- * @device_path: path of the device from which the binary was loaded
- * @image_path:device path of the binary
+ * @handle:handle of loaded image
* Return:status code
*
* Load the EFI binary into a newly assigned memory unwinding the relocation
* information, install the loaded image protocol, and call the binary.
*/
-static efi_status_t do_bootefi_exec(void *efi,
- struct efi_device_path *device_path,
- struct efi_device_path *image_path)
+static efi_status_t do_bootefi_exec(efi_handle_t handle)
{
- efi_handle_t mem_handle = NULL;
- struct efi_device_path *memdp = NULL;
- efi_status_t ret;
- struct efi_loaded_image_obj *image_obj = NULL;
struct efi_loaded_image *loaded_image_info = NULL;
-
- /*
-* Special case for efi payload not loaded from disk, such as
-* 'bootefi hello' or for example payload loaded directly into
-* memory via JTAG, etc:
-*/
- if (!device_path && !image_path) {
- printf("WARNING: using memory device/image path, this may confuse
some payloads!\n");
- /* actual addresses filled in after efi_load_pe() */
- memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0);
- device_path = image_path = memdp;
- /*
-* Grub expects that the device path of the loaded image is
-* installed on a handle.
-*/
- ret = efi_create_handle(&mem_handle);
- if (ret != EFI_SUCCESS)
- return ret; /* TODO: leaks device_path */
- ret = efi_add_protocol(mem_handle, &efi_guid_device_path,
- device_path);
- if (ret != EFI_SUCCESS)
- goto err_add_protocol;
- } else {
- assert(device_path && image_path);
- }
-
- ret = efi_setup_loaded_image(device_path, image_path, &image_obj,
-&loaded_image_info);
- if (ret)
- goto err_prepare;
+ efi_status_t ret;
/* Transfer environment variable as load options */
- set_load_options(loaded_image_info, "bootargs");
In set_load_options() an error could occur (out of memory). So I think
it should return a status.
-
- /* Load the EFI payload */
- ret = efi_load_pe(image_obj, efi, loaded_image_info);
+ ret = EFI_CALL(systab.boottime->open_protocol(
+ handle,
+ &efi_guid_loaded_image,
+ (void **)&loaded_image_info,
+ NULL, NULL,
+ EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL));
Shouldn't we move this to set_load_options()?
if (ret != EFI_SUCCESS)
- goto err_prepare;
-
- if (memdp) {
- struct efi_device_path_memory *mdp = (void *)memdp;
- mdp->memory_type = loaded_image_info->image_code_type;
- mdp->start_address = (uintptr_t)loaded_image_info->image_base;
- mdp->end_address = mdp->start_address +
- loaded_image_info->image_size;
- }
+ goto err;
+ set_load_options(loaded_image_info, "bootargs");
/* we don't support much: */
env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported",
"{ro,boot}(blob)");
Shouldn't this move to efi_setup.c? Probably we should also set
OsIndications. I would prefer using efi_set_variable(). I think moving
this could be done in a preparatory patch.
/* Call our payload! */
- debug("%s: Jumping to 0x%p\n", __func__, image_obj->entry);
- ret = EFI_CALL(efi_start_image(&image_obj->header,
[U-Boot] [RFC v3 09/10] efi_loader: rework bootmgr/bootefi using load_image API
In the current implementation, bootefi command and EFI boot manager
don't use load_image API, instead, use more primitive and internal
functions. This will introduce duplicated code and potentially
unknown bugs as well as inconsistent behaviours.
With this patch, do_efibootmgr() and do_boot_efi() are completely
overhauled and re-implemented using load_image API.
Signed-off-by: AKASHI Takahiro
---
cmd/bootefi.c | 197 +++---
include/efi_loader.h | 5 +-
lib/efi_loader/efi_bootmgr.c | 43
lib/efi_loader/efi_boottime.c | 2 +
4 files changed, 134 insertions(+), 113 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index f14966961b23..97d49a53a44b 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -222,88 +222,39 @@ static efi_status_t efi_install_fdt(const char *fdt_opt)
/**
* do_bootefi_exec() - execute EFI binary
*
- * @efi: address of the binary
- * @device_path: path of the device from which the binary was loaded
- * @image_path:device path of the binary
+ * @handle:handle of loaded image
* Return: status code
*
* Load the EFI binary into a newly assigned memory unwinding the relocation
* information, install the loaded image protocol, and call the binary.
*/
-static efi_status_t do_bootefi_exec(void *efi,
- struct efi_device_path *device_path,
- struct efi_device_path *image_path)
+static efi_status_t do_bootefi_exec(efi_handle_t handle)
{
- efi_handle_t mem_handle = NULL;
- struct efi_device_path *memdp = NULL;
- efi_status_t ret;
- struct efi_loaded_image_obj *image_obj = NULL;
struct efi_loaded_image *loaded_image_info = NULL;
-
- /*
-* Special case for efi payload not loaded from disk, such as
-* 'bootefi hello' or for example payload loaded directly into
-* memory via JTAG, etc:
-*/
- if (!device_path && !image_path) {
- printf("WARNING: using memory device/image path, this may
confuse some payloads!\n");
- /* actual addresses filled in after efi_load_pe() */
- memdp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE, 0, 0);
- device_path = image_path = memdp;
- /*
-* Grub expects that the device path of the loaded image is
-* installed on a handle.
-*/
- ret = efi_create_handle(&mem_handle);
- if (ret != EFI_SUCCESS)
- return ret; /* TODO: leaks device_path */
- ret = efi_add_protocol(mem_handle, &efi_guid_device_path,
- device_path);
- if (ret != EFI_SUCCESS)
- goto err_add_protocol;
- } else {
- assert(device_path && image_path);
- }
-
- ret = efi_setup_loaded_image(device_path, image_path, &image_obj,
-&loaded_image_info);
- if (ret)
- goto err_prepare;
+ efi_status_t ret;
/* Transfer environment variable as load options */
- set_load_options(loaded_image_info, "bootargs");
-
- /* Load the EFI payload */
- ret = efi_load_pe(image_obj, efi, loaded_image_info);
+ ret = EFI_CALL(systab.boottime->open_protocol(
+ handle,
+ &efi_guid_loaded_image,
+ (void **)&loaded_image_info,
+ NULL, NULL,
+ EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL));
if (ret != EFI_SUCCESS)
- goto err_prepare;
-
- if (memdp) {
- struct efi_device_path_memory *mdp = (void *)memdp;
- mdp->memory_type = loaded_image_info->image_code_type;
- mdp->start_address = (uintptr_t)loaded_image_info->image_base;
- mdp->end_address = mdp->start_address +
- loaded_image_info->image_size;
- }
+ goto err;
+ set_load_options(loaded_image_info, "bootargs");
/* we don't support much: */
env_set("efi_8be4df61-93ca-11d2-aa0d-00e098032b8c_OsIndicationsSupported",
"{ro,boot}(blob)");
/* Call our payload! */
- debug("%s: Jumping to 0x%p\n", __func__, image_obj->entry);
- ret = EFI_CALL(efi_start_image(&image_obj->header, NULL, NULL));
+ ret = EFI_CALL(efi_start_image(handle, NULL, NULL));
-err_prepare:
- /* image has returned, loaded-image obj goes *poof*: */
efi_restore_gd();
free(loaded_image_info->load_options);
- efi_delete_handle(&image_obj->header);
-
-err_add_protocol:
- if (mem_handle)
- efi_delete_handle(mem_handle);
+err:
return
