[U-Boot] [PATCH] efi_loader: execute image's unload function

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."
Signed-off-by: AKASHI Takahiro takahiro.akashi@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); + 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; 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); + } + efiobj = efi_search_obj(image_handle); if (efiobj) list_del(&efiobj->link);

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?
Signed-off-by: AKASHI Takahiro takahiro.akashi@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=c982874e930d5d673501cd94df07...
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);
}
efiobj = efi_search_obj(image_handle); if (efiobj) list_del(&efiobj->link);

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.akashi@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=c982874e930d5d673501cd94df07...
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);
}
efiobj = efi_search_obj(image_handle); if (efiobj) list_del(&efiobj->link);

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.akashi@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=c982874e930d5d673501cd94df07...
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);

Heinrich,
On Fri, Sep 28, 2018 at 01:24:42PM +0900, 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.
Does my patch[1] fix your case? [1] https://lists.denx.de/pipermail/u-boot/2018-October/342501.html Or do you see another issue here?
-Takahiro Akashi
I will address your comments below later.
Thanks, -Takahiro Akashi
Signed-off-by: AKASHI Takahiro takahiro.akashi@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=c982874e930d5d673501cd94df07...
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);
}
efiobj = efi_search_obj(image_handle); if (efiobj) list_del(&efiobj->link);
participants (2)
-
AKASHI Takahiro
-
Heinrich Schuchardt