On 01/24/2018 12:04 AM, Alexander Graf wrote:

On 23.01.18 22:35, Heinrich Schuchardt wrote:

...

On 01/19/2018 09:16 PM, xypron.glpk@gmx.de wrote:

...

From: Heinrich Schuchardt xypron.glpk@gmx.de

The calling convention for the entry point of an EFI image is always 'asmlinkage'.

Signed-off-by: Heinrich Schuchardt xypron.glpk@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?

Because EFI supported started as payload support.

...

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

Not quite. I guess we basically want to have EFI_LOADER select EFI_STUB_64BIT on x86_64. So maybe something like the patch below?

Alex

diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 51213c0293..c579b76b4b 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -126,8 +126,8 @@ static void *copy_fdt(void *fdt)

static efi_status_t efi_do_enter( efi_handle_t image_handle, struct efi_system_table *st,

• 	asmlinkage ulong (*entry)(efi_handle_t image_handle,
  

• 				  struct efi_system_table *st))
  

• 	EFIAPI ulong (*entry)(efi_handle_t image_handle,
  

• 			      struct efi_system_table *st))
  

{ efi_status_t ret = EFI_LOAD_ERROR;

@@ -138,7 +138,7 @@ static efi_status_t efi_do_enter( }

#ifdef CONFIG_ARM64 -static efi_status_t efi_run_in_el2(asmlinkage ulong (*entry)( +static efi_status_t efi_run_in_el2(EFIAPI ulong (*entry)(

We should use efi_status_t and not ulong here (and below). It dislike if the same property gets different names.

	efi_handle_t image_handle, struct efi_system_table *st),
	efi_handle_t image_handle, struct efi_system_table *st)

{ @@ -163,7 +163,7 @@ static efi_status_t do_bootefi_exec(void *efi, void *fdt, ulong ret;

ulong (*entry)(efi_handle_t image_handle, struct efi_system_table *st)

• asmlinkage;
  

• EFIAPI;
  

  ulong fdt_pages, fdt_size, fdt_start, fdt_end; const efi_guid_t fdt_guid = EFI_FDT_GUID; bootm_headers_t img = { 0 };

diff --git a/include/efi.h b/include/efi.h index 2f0be9c86c..29f6930f53 100644 --- a/include/efi.h +++ b/include/efi.h @@ -19,7 +19,7 @@ #include <linux/string.h> #include <linux/types.h>

-#ifdef CONFIG_EFI_STUB_64BIT +#if CONFIG_EFI_STUB_64BIT || (!defined(CONFIG_EFI_STUB) && defined (__x86_64__)) /* EFI uses the Microsoft ABI which is not the default for GCC */ #define EFIAPI __attribute__((ms_abi)) #else diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index d2b6327119..827c267b60 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -1,6 +1,10 @@ config EFI_LOADER bool "Support running EFI Applications in U-Boot" depends on (ARM || X86) && OF_LIBFDT

• # We need EFI_STUB_64BIT to be set on x86_64 with EFI_STUB
• depends on !EFI_STUB || !X86_64 || EFI_STUB_64BIT
• # We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
• depends on !EFI_STUB || !X86 || X86_64 || EFI_STUB_32BIT

Wouldn't it be better to let EFI_API depend on CONFIG_X86_64?

Why shouldn't EFI_LOADER and EFI_STUB be built together?

Regards

Heinrich

default y help Select this option if you want to run EFI applications (like grub2) diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 39d8511fe3..5b78740bff 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1534,8 +1534,8 @@ static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, unsigned long *exit_data_size, s16 **exit_data) {

• asmlinkage ulong (*entry)(efi_handle_t image_handle,

• 		  struct efi_system_table *st);
  

• EFIAPI ulong (*entry)(efi_handle_t image_handle,

• 	      struct efi_system_table *st);
  

  struct efi_loaded_image *info = image_handle; efi_status_t ret;