Re: [PATCH 09/10] efi/libstub: annotate firmware routines as __efiapi

[Brian Gerst]

On Sat, Dec 14, 2019 at 1:00 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
>
> Annotate all the firmware routines (boot services, runtime services and
> protocol methods) called in the boot context as __efiapi, and make
> it expand to __attribute__((ms_abi)) on 64-bit x86. This allows us
> to use the compiler to generate the calls into firmware that use the
> MS calling convention instead of the SysV one.
>
> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> ---
>  arch/x86/Kconfig                      |   1 +
>  arch/x86/boot/compressed/eboot.h      |   4 +-
>  drivers/firmware/efi/libstub/random.c |   8 +-
>  include/linux/efi.h                   | 130 +++++++++++---------
>  4 files changed, 79 insertions(+), 64 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 5e8949953660..8ba81036a7ef 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1993,6 +1993,7 @@ config EFI
>  config EFI_STUB
>         bool "EFI stub support"
>         depends on EFI && !X86_USE_3DNOW
> +       depends on $(cc-option,-mabi=ms)
>         select RELOCATABLE
>         ---help---
>            This kernel feature allows a bzImage to be loaded directly
> diff --git a/arch/x86/boot/compressed/eboot.h b/arch/x86/boot/compressed/eboot.h
> index 318531f128c2..512d2143a94f 100644
> --- a/arch/x86/boot/compressed/eboot.h
> +++ b/arch/x86/boot/compressed/eboot.h
> @@ -19,8 +19,8 @@ typedef struct {
>  } efi_uga_draw_protocol_32_t;
>
>  typedef struct efi_uga_draw_protocol {
> -       efi_status_t (*get_mode)(struct efi_uga_draw_protocol *,
> -                                u32*, u32*, u32*, u32*);
> +       efi_status_t (__efiapi *get_mode)(struct efi_uga_draw_protocol *,
> +                                         u32*, u32*, u32*, u32*);
>         void *set_mode;
>         void *blt;
>  } efi_uga_draw_protocol_t;
> diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c
> index 3b85883fb312..872d2de3eaaf 100644
> --- a/drivers/firmware/efi/libstub/random.c
> +++ b/drivers/firmware/efi/libstub/random.c
> @@ -17,10 +17,10 @@ typedef struct {
>  } efi_rng_protocol_32_t;
>
>  struct efi_rng_protocol {
> -       efi_status_t (*get_info)(struct efi_rng_protocol *,
> -                                unsigned long *, efi_guid_t *);
> -       efi_status_t (*get_rng)(struct efi_rng_protocol *,
> -                               efi_guid_t *, unsigned long, u8 *out);
> +       efi_status_t (__efiapi *get_info)(struct efi_rng_protocol *,
> +                                         unsigned long *, efi_guid_t *);
> +       efi_status_t (__efiapi *get_rng)(struct efi_rng_protocol *,
> +                                        efi_guid_t *, unsigned long, u8 *out);
>  };
>
>  efi_status_t efi_get_random_bytes(efi_system_table_t *sys_table_arg,
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 7f1a99bd2c05..2ba13d901055 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -48,6 +48,12 @@ typedef u16 efi_char16_t;            /* UNICODE character */
>  typedef u64 efi_physical_addr_t;
>  typedef void *efi_handle_t;
>
> +#ifdef CONFIG_X86_64
> +#define __efiapi __attribute__((ms_abi))
> +#else
> +#define __efiapi

I think it would be better to explicitly use the regparm(0) attribute
for 32-bit, even though that is currently the default.

--
Brian Gerst