[U-Boot] [PATCH v4 13/18] efi_loader: fix StartImage bootservice

[Heinrich Schuchardt]

On 01/19/2018 09:16 PM, xypron.glpk at gmx.de wrote:
> From: Heinrich Schuchardt <xypron.glpk at gmx.de>
> 
> The calling convention for the entry point of an EFI image
> is always 'asmlinkage'.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
> v4
> 	rebase according to https://github.com/agraf/efi_next
> v3
> 	Use efi_handle_t as type of the image handle.
> v2
> 	no change
> ---
> 
>  lib/efi_loader/efi_boottime.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 7c61dfb3a7..324abe4d48 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1530,7 +1530,8 @@ static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
>  					   unsigned long *exit_data_size,
>  					   s16 **exit_data)
>  {
> -	ulong (*entry)(efi_handle_t image_handle, struct efi_system_table *st);
> +	asmlinkage ulong (*entry)(efi_handle_t image_handle,
> +				  struct efi_system_table *st);

Alex,

could you once again carefully review this change.

Have a look at the definition of EFIAPI in include/efi.h

In cmd/bootefi.c we assume the entry point is asmlinkage.
In lib/efi_loader/helloworld.c we assume it is EFIAPI.

The definition of EFIAPI depends on CONFIG_EFI_STUB_64BIT, which is only
defined in configs/qemu-x86_efi_payload64_defconfig.

Why should the definition of EFIAPI depend on whether we are consuming
or offering the API? Shouldn't EFIAPI have the same definition when
compiling qemu-x86_64_defconfig?

Am I right that the entry point should in cmd/bootefi.c, helloworld.c,
efi_start_image() should always be defined as

    EFIAPI efi_status_t (*entry)(efi_handle_t image_handle

and further when compiling for x86_64 we should always define EFIAPI as

    __attribute__((ms_abi))

and on other systems as

    asmlinkage

Best regards

Heinrich

>  	struct efi_loaded_image *info = image_handle;
>  	efi_status_t ret;
>  
>