On 18.01.18 19:28, Heinrich Schuchardt wrote: > 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.g...@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 */
Could you make that slightly more verbose? Which EFI_CALL? Why do we need to execute the return path? Keep in mind that people 2 years down the road should be able to grasp what the code does :). Alex _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot