On 09/28/2018 06:24 AM, AKASHI Takahiro wrote: > Heinrich, > > On Fri, Sep 28, 2018 at 04:35:57AM +0200, Heinrich Schuchardt wrote: >> On 08/09/2018 08:50 AM, AKASHI Takahiro wrote: >>> Currently, unload function in EFI_LOADED_IMAGE_PROTOCOL is never called >>> at UnloadImage Boot Service. This is not compliant to UEFI specification. >>> See chapter "9.1 EFI Loaded Image Protocol." >> >> With all the patches we got into U-Boot (+HII protocols, +HOB and DXE >> tables) we now can start the EFI shell. But I do not succeed to run the >> SCT. Could you, please, provide an overview what is needed. >> >> I use the sct-prepare target in >> https://github.com/xypron/u-boot-build/blob/qemu-arm64/Makefile to set >> up my disk image with the SCT. >> >> Are we missing further patches or is there something wrong with the >> image I generate? > > I have the same problem since I rebased my own branch to Alex's > efi-next this week. I'm trying to understand what's happening now. > > I will address your comments below later. > > Thanks, > -Takahiro Akashi > > >>> >>> Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org> >>> --- >>> include/efi_api.h | 4 +++- >>> lib/efi_loader/efi_boottime.c | 9 +++++++++ >>> 2 files changed, 12 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/efi_api.h b/include/efi_api.h >>> index a4343ae98e2..b2806fba3b8 100644 >>> --- a/include/efi_api.h >>> +++ b/include/efi_api.h >>> @@ -326,6 +326,8 @@ struct efi_system_table { >>> >>> #define EFI_LOADED_IMAGE_PROTOCOL_REVISION 0x1000 >>> >>> +typedef efi_status_t (EFIAPI *efi_image_unload_t)(efi_handle_t >>> *image_handle); >> >> In the rest of efi_api.h we never used typedefs for function prototypes. >> I would prefer to use the explicit type. >> >> In scripts/checkpatch.pl this will create a warning. >> >>> + >>> struct efi_loaded_image { >>> u32 revision; >>> void *parent_handle; >>> @@ -339,7 +341,7 @@ struct efi_loaded_image { >>> aligned_u64 image_size; >>> unsigned int image_code_type; >>> unsigned int image_data_type; >>> - unsigned long unload; >>> + efi_image_unload_t unload; >>> >>> /* Below are efi loader private fields */ >>> #ifdef CONFIG_EFI_LOADER >>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c >>> index 80061e10ebc..d6fcf7e0049 100644 >>> --- a/lib/efi_loader/efi_boottime.c >>> +++ b/lib/efi_loader/efi_boottime.c >>> @@ -1843,9 +1843,18 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t >>> image_handle, >>> */ >>> static efi_status_t EFIAPI efi_unload_image(efi_handle_t image_handle) >>> { >>> + struct efi_loaded_image *info = image_handle; >> >> Please, do not assume the the image_handle points to the >> EFI_LOADED_IMAGE_PROTOCOL. Anyway this is not valid anymore after >> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=c982874e930d5d673501cd94df07bcbd215d5883 >> >> Best regards >> >> Heinrich >> >>> struct efi_object *efiobj; >>> + efi_status_t ret; >>> >>> EFI_ENTRY("%p", image_handle); >>> + >>> + if (info->unload) { >>> + ret = info->unload(image_handle); >>> + if (ret != EFI_SUCCESS) >>> + return EFI_EXIT(ret); >>> + }
We have to check that image_handle is valid and refers to a loaded image before trying to call the unload function. The unload function shall only be called for a loaded image. When unloading we have to close all protocol interfaces opened by image_handle as agent. Best regards Heinrich >>> + >>> efiobj = efi_search_obj(image_handle); >>> if (efiobj) >>> list_del(&efiobj->link); >>> >> > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
