Re: [U-Boot] efi_loader: execute image's unload function
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
> > > ---
> > > 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);
> > > + }
> > > +
> > > efiobj = efi_search_obj(image_handle);
> > > if (efiobj)
> > > list_del(>link);
> > >
> >
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] efi_loader: execute image's unload function
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
>>> ---
>>> 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(>link);
>>>
>>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] efi_loader: execute image's unload function
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
> > ---
> > 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);
> > + }
> > +
> > efiobj = efi_search_obj(image_handle);
> > if (efiobj)
> > list_del(>link);
> >
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] efi_loader: execute image's unload function
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
> ---
> 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);
> + }
> +
> efiobj = efi_search_obj(image_handle);
> if (efiobj)
> list_del(>link);
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
