Opened 5 years ago

Closed 5 years ago

#10800 closed bug (fixed)

UEFI: assembly files need to take calling convention into account

Reported by: jessicah Owned by: axeld
Priority: normal Milestone: R1
Component: System/libroot.so Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

For UEFI, the calling conventions on x86_64 follow the Microsoft calling conventions, which are not compatible with Haiku's calling conventions.

As a result, existing functions written in assembly fail to run as expected. E.g. byteorder.S currently returns 0 for all of the byteswap routines, which are used elsewhere, such as the definitions for the B_BENDIAN_xyz macros, and the ntohs function.

References: Wikipedia, OSDev Wiki

Attachments (2)

Change History (13)

comment:1 Changed 5 years ago by jessicah

I've filed this, and hoping it can be a task for choupy to undertake.

comment:2 Changed 5 years ago by pdziepak

The best solution would be to use inline assembly in C code and let GCC take care of calling convention. Moreover, many of such asm functions should be inlined wherever possible since they usually contain only few instructions. Use of GCC builtins also should be considered as it would allow some compile time optimizations, e.g. when changing byte order of the value that is known at compile time.

comment:3 Changed 5 years ago by choupy

Hey,

I am not very much in favor of GCC inlining. I consider C files to be pretty "platform-agnostic", however adding ASM inline in those files breaks this. I am also not comfortable with the inlining syntax as it is really messy in my opinion.

Letting gcc take care of the calling convention is however very appealing... What is your take on this jessicah?

comment:4 Changed 5 years ago by pulkomandy

My vote goes for using the gcc builtins if available. They are listed here (at tne bottom of the page): http://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html

There's nothing for float and doubles, however.

comment:5 Changed 5 years ago by jessicah

I'm happy for the builtins to be used. I also see no problem with using inline assembly. If it means less duplicated code and/or conditional code, then that, in my view, is a win.

comment:6 Changed 5 years ago by tqh

When I looked into this I wanted MS calling conventions to be used for UEFI calls only. We set GNU_EFI_USE_MS_ABI and that makes HAVE_USE_MS_ABI 1 to be set in efibind.h. So if we use MS calling conventions all the time I think we are doing something wrong. Note that if our main function, called from start_func.S isn't marked as EFIAPI we might need to fix the asm or mark it EFIAPI.

So that might be worth investigating.

See https://github.com/tqh/haiku/blob/efi/headers/private/kernel/boot/platform/efi/arch/x86_64/efibind.h#L25

#if defined(GNU_EFI_USE_MS_ABI)
    #if defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 7))
        #define HAVE_USE_MS_ABI 1
    #else
        #error Compiler is too old for GNU_EFI_USE_MS_ABI
    #endif
#endif

https://github.com/tqh/haiku/blob/efi/headers/private/kernel/boot/platform/efi/arch/x86_64/efibind.h#L183

#ifndef EFIAPI // Forces EFI calling conventions reguardless of compiler options
    #ifdef _MSC_EXTENSIONS
        #define EFIAPI __cdecl // Force C calling convention for Microsoft C compiler
    #elif defined(HAVE_USE_MS_ABI)
        // Force amd64/ms calling conventions.
        #define EFIAPI __attribute__((ms_abi))
    #else
        #define EFIAPI // Substitute expresion to force C calling convention
    #endif
#endif

comment:7 Changed 5 years ago by jessicah

I'm not sure if this is the right approach to take. Can we safely assume GCC has support for these on all of our supported/targeted platforms? And is this the right place to do that? At the very least, they now become single inlined instructions with this change rather than function calls, and resolves the issue I was seeing with the EFI bootloader.

Of course, it'll need to be cleaned up to also remove the platform dependent code where necessary and simplify affected Jamfiles.

comment:8 Changed 5 years ago by tqh

I think this would be a good question on the haiku-dev mailing list. As I wrote in my previous post I still think that approach should work, however I havn't had the time to look at the build-flags yet and what needs to be done.

comment:9 Changed 5 years ago by jessicah

Has a Patch: set

comment:10 Changed 5 years ago by jessicah

Fixed in hrev47269 for x86[_64] gcc-4.3+, ARM gcc-4.8+.

comment:11 Changed 5 years ago by jessicah

Resolution: fixed
Status: newclosed
Note: See TracTickets for help on using tickets.