[U-Boot] [PATCH v2 1/2] efi_loader: correctly call images

[Heinrich Schuchardt]

On 01/18/2018 11:05 AM, Alexander Graf wrote:
> 
> 
> On 18.01.18 10:52, Heinrich Schuchardt wrote:
>>
>>
>> On 01/18/2018 10:24 AM, Alexander Graf wrote:
>>>
>>>
>>> On 18.01.18 08:24, Heinrich Schuchardt wrote:
>>>> Avoid a failed assertion when an EFI app calls an EFI app.
>>>>
>>>> Avoid that the indent level increases when calling 'bootefi hello'
>>>> repeatedly.
>>>>
>>>> Avoid negative indent level when an EFI app calls an EFI app that
>>>> calls an EFI app (e.g. iPXE loads grub which starts the kernel).
>>>>
>>>> Return the status code of a loaded image that returns without
>>>> calling the Exit boot service.
>>>>
>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>>>> ---
>>>>   lib/efi_loader/efi_boottime.c | 21 ++++++++++++++-------
>>>>   1 file changed, 14 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/lib/efi_loader/efi_boottime.c
>>>> b/lib/efi_loader/efi_boottime.c
>>>> index 2c5499e0c8..538cc55d20 100644
>>>> --- a/lib/efi_loader/efi_boottime.c
>>>> +++ b/lib/efi_loader/efi_boottime.c
>>>> @@ -1537,6 +1537,7 @@ static efi_status_t EFIAPI
>>>> efi_start_image(efi_handle_t image_handle,
>>>>       asmlinkage ulong (*entry)(efi_handle_t image_handle,
>>>>                     struct efi_system_table *st);
>>>>       struct efi_loaded_image *info = image_handle;
>>>> +    efi_status_t ret;
>>>>         EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size,
>>>> exit_data);
>>>>       entry = info->reserved;
>>>> @@ -1546,17 +1547,23 @@ static efi_status_t EFIAPI
>>>> efi_start_image(efi_handle_t image_handle,
>>>>       /* call the image! */
>>>>       if (setjmp(&info->exit_jmp)) {
>>>>           /* We returned from the child image */
>>>> +#ifdef CONFIG_ARM
>>>> +        /* efi_exit() called efi_restore_gd() */
>>>> +        gd = app_gd;
>>>> +#endif
>>>> +        /* Execute the return part of EFI_CALL */
>>>> +        assert(__efi_entry_check());
>>>> +        debug("%sEFI: %lu returned by started image\n",
>>>> +              __efi_nesting_dec(),
>>>
>>> I don't understand why you need to decrease the nesting level here after
>>> the other rework. You're now calling EFI_ENTRY/EFI_EXIT in all normal
>>> paths when going in/out of an application, no?
>>
>> bootefi -> level 0
>> ** EFI application running at level 0
>> LoadImage EFI_ENTRY -> level 1
>> LoadImage EFI_EXIT -> level 0
>> ** EFI application running at  level 0
> 
> -- base level at 0
> 
>> StartImage EFI_ENTRY -> level 1
> 
> This is decreased in EFI_EXIT of StartImage
> 
>> StartImage EFI_CALL -> level 2
> 
> This is the one that needs manual decrease then?
> 
>> Exit EFI_ENTRY -> level 3
> 
> Gets decreased right below in Exit again
> 
>> Exit EFI_EXIT -> level 2
>> longjmp -> level 2
>> __efi_nesting_dec() -> level 1
>> StartImage EFI_EXIT -> level 0
> 
> --- base level again
> 
> 
> So I guess the problem is that we never get into the second half of
> EFI_CALL when ->exit() gets called because of the longjmp.
> 
> Can you please add a comment explaining that rationale with a hint to
> EFI_CALL and that all we do is execute the lower half of it manually
> again because it got interrupted by the longjmp?
> 

I already wrote:
/* Execute the return part of EFI_CALL */

Regards

Heinrich

> 
> Alex
>