On Sat, May 04, 2019 at 10:36:36AM +0200, Heinrich Schuchardt wrote:
> Implement unloading of images in the Exit() boot services:
> 
> * unload images that are not yet started,
> * unload started applications,
> * unload drivers returning an error.
>
> Signed-off-by: Heinrich Schuchardt <[email protected]>
> ---
>  include/efi_loader.h              |  1 +
>  lib/efi_loader/efi_boottime.c     | 34 ++++++++++++++++++++++++++-----
>  lib/efi_loader/efi_image_loader.c |  2 ++
>  3 files changed, 32 insertions(+), 5 deletions(-)
> 
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index c2a449e5b6..d73c89ac26 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -237,6 +237,7 @@ struct efi_loaded_image_obj {
>       struct jmp_buf_data exit_jmp;
>       EFIAPI efi_status_t (*entry)(efi_handle_t image_handle,
>                                    struct efi_system_table *st);
> +     u16 image_type;
>  };
> 
>  /**
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index bbcd66caa6..51e0bb2fd5 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -13,6 +13,7 @@
>  #include <linux/libfdt_env.h>
>  #include <u-boot/crc.h>
>  #include <bootm.h>
> +#include <pe.h>
>  #include <watchdog.h>
> 
>  DECLARE_GLOBAL_DATA_PTR;
> @@ -2798,7 +2799,7 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t 
> image_handle,
>        *       image protocol.
>        */
>       efi_status_t ret;
> -     void *info;
> +     struct efi_loaded_image *loaded_image_protocol;
>       struct efi_loaded_image_obj *image_obj =
>               (struct efi_loaded_image_obj *)image_handle;
> 
> @@ -2806,13 +2807,33 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t 
> image_handle,
>                 exit_data_size, exit_data);
> 
>       /* Check parameters */
> -     if (image_handle != current_image)
> +     if (image_handle != current_image) {

Does this check make sense even for a driver?

> +             ret = EFI_INVALID_PARAMETER;
>               goto out;
> +     }
>       ret = EFI_CALL(efi_open_protocol(image_handle, &efi_guid_loaded_image,
> -                                      &info, NULL, NULL,
> +                                      (void **)&loaded_image_protocol,
> +                                      NULL, NULL,
>                                        EFI_OPEN_PROTOCOL_GET_PROTOCOL));
> -     if (ret != EFI_SUCCESS)
> +     if (ret != EFI_SUCCESS) {

Is this check necessary even when 'header.type' is introduced?

> +             ret = EFI_INVALID_PARAMETER;
>               goto out;
> +     }
> +
> +     /* Unloading of unstarted images */

'Unload' sounds odd when we call efi_delete_image(), doesn't it?

> +     switch (image_obj->header.type) {
> +     case EFI_OBJECT_TYPE_STARTED_IMAGE:
> +             break;
> +     case EFI_OBJECT_TYPE_LOADED_IMAGE:
> +             efi_delete_image(image_obj, loaded_image_protocol);
> +             ret = EFI_SUCCESS;
> +             goto out;
> +     default:
> +             /* Handle does not refer to loaded image */
> +             ret = EFI_INVALID_PARAMETER;
> +             goto out;
> +     }
> +     image_obj->header.type = EFI_OBJECT_TYPE_LOADED_IMAGE;
> 
>       /* Exit data is only foreseen in case of failure. */
>       if (exit_status != EFI_SUCCESS) {
> @@ -2822,6 +2843,9 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t 
> image_handle,
>               if (ret != EFI_SUCCESS)
>                       EFI_PRINT("%s: out of memory\n", __func__);
>       }
> +     if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION ||
> +         exit_status != EFI_SUCCESS)
> +             efi_delete_image(image_obj, loaded_image_protocol);

Why not merge those two efi_delete_image()?

-Takahiro Akashi


>       /* Make sure entry/exit counts for EFI world cross-overs match */
>       EFI_EXIT(exit_status);
> @@ -2837,7 +2861,7 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t 
> image_handle,
> 
>       panic("EFI application exited");
>  out:
> -     return EFI_EXIT(EFI_INVALID_PARAMETER);
> +     return EFI_EXIT(ret);
>  }
> 
>  /**
> diff --git a/lib/efi_loader/efi_image_loader.c 
> b/lib/efi_loader/efi_image_loader.c
> index f8092b6202..13541cfa7a 100644
> --- a/lib/efi_loader/efi_image_loader.c
> +++ b/lib/efi_loader/efi_image_loader.c
> @@ -273,6 +273,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj 
> *handle, void *efi,
>               IMAGE_OPTIONAL_HEADER64 *opt = &nt64->OptionalHeader;
>               image_base = opt->ImageBase;
>               efi_set_code_and_data_type(loaded_image_info, opt->Subsystem);
> +             handle->image_type = opt->Subsystem;
>               efi_reloc = efi_alloc(virt_size,
>                                     loaded_image_info->image_code_type);
>               if (!efi_reloc) {
> @@ -288,6 +289,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj 
> *handle, void *efi,
>               IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader;
>               image_base = opt->ImageBase;
>               efi_set_code_and_data_type(loaded_image_info, opt->Subsystem);
> +             handle->image_type = opt->Subsystem;
>               efi_reloc = efi_alloc(virt_size,
>                                     loaded_image_info->image_code_type);
>               if (!efi_reloc) {
> --
> 2.20.1
> 
_______________________________________________
U-Boot mailing list
[email protected]
https://lists.denx.de/listinfo/u-boot

Reply via email to