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

[Heinrich Schuchardt]

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
StartImage EFI_ENTRY -> level 1
StartImage EFI_CALL -> level 2
Exit EFI_ENTRY -> level 3
Exit EFI_EXIT -> level 2
longjmp -> level 2
__efi_nesting_dec() -> level 1
StartImage EFI_EXIT -> level 0
** EFI application running at level 0 again.
Exit EFI_ENTRY -> level 1
Exit EFI_EXIT -> level 0
Back to U-Boot

For testing indent levels enable DEBUG and
* repeatedly execute 'bootefi hello'
* setenv efi_selftest start image
   bootefi selftest
   (requires [PATCH v2 2/2] efi_selftest: test start image)

Best regards

Heinrich

> 
> 
> Alex
> 
>> +		      (unsigned long)((uintptr_t)info->exit_status &
>> +				      ~EFI_ERROR_MASK));
>>   		return EFI_EXIT(info->exit_status);
>>   	}
>>   
>> -	__efi_nesting_dec();
>> -	__efi_exit_check();
>> -	entry(image_handle, &systab);
>> -	__efi_entry_check();
>> -	__efi_nesting_inc();
>> +	ret = EFI_CALL(entry(image_handle, &systab));
>>   
>>   	/* Should usually never get here */
>> -	return EFI_EXIT(EFI_SUCCESS);
>> +	return EFI_EXIT(ret);
>>   }
>>   
>>   /*
>> @@ -1593,7 +1600,7 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle,
>>   		  exit_data_size, exit_data);
>>   
>>   	/* Make sure entry/exit counts for EFI world cross-overs match */
>> -	__efi_exit_check();
>> +	EFI_EXIT(exit_status);
>>   
>>   	/*
>>   	 * But longjmp out with the U-Boot gd, not the application's, as
>>
>