uefi-rs icon indicating copy to clipboard operation
uefi-rs copied to clipboard

`[un]install_multiple_protocol_interfaces()` requires but is currently incompatible with feature `c_variadic`

Open timrobertsdev opened this issue 2 years ago • 11 comments

Tracking issue

While implementing the missing protocol handler functions in BootServices, I found that extern "efiapi" is incompatible with Rust's current (unstable) implementation of c variadics. Rustc indicates that C variadics must be extern "C" or extern "cdecl", thus install_multiple_protocol_interfaces() and uninstall_multiple_protocol_interfaces() cannot be implemented at this time.

I'll bring this up on the C variadic tracking issue in the next couple days if it hasn't been already.

Rust tracking issue: https://github.com/rust-lang/rust/issues/44930

EDIT: technically I guess this could be possible with some clever and wildly unsafe asm but I'd much rather wait for Rust to support it.

timrobertsdev avatar Oct 19 '21 21:10 timrobertsdev

Update: Per Josh Triplett on the rustlang zulip, extern "C" on the "x86_64-unknown-uefi" target should be the proper UEFI ABI, so we can actually implement this, using extern "C" instead of extern "efiapi".

timrobertsdev avatar Oct 20 '21 09:10 timrobertsdev

@timrobertsdev does this mean we should replace all extern "efiapi" declarations in the crate with extern "C", and everything will just work correctly? Or is this workaround just for the protocol handler functions?

GabrielMajeri avatar Dec 11 '21 09:12 GabrielMajeri

The way I understood it, since we're targeting EFI, extern "C" will always be the correct calling convention. I've replaced all extern "efiapi" with extern "C" locally and everything still builds and runs/tests correctly.

timrobertsdev avatar Dec 14 '21 20:12 timrobertsdev

@timrobertsdev Ah, I've realized there's a little problem which stops us from switching everything over to extern "C".

The reason we've introduced the efiabi calling convention was to avoid issues like https://github.com/rust-osdev/uefi-rs/issues/38 -- if uefi-rs is used as a library in an OS kernel/driver which interacts with UEFI structures but which is built as an ELF binary, then it won't be able to call the methods correctly, unless they're declared with efiabi.

I'm not sure what the correct solution would be here.

GabrielMajeri avatar Dec 22 '21 16:12 GabrielMajeri

Perhaps the [un]install_multiple_protocol_interfaces function pointers could be declared with extern "C", and then the wrapper methods that call them could be guarded behind a #[cfg(...)] that only allows them to be called from uefi arches, until such time as efiabi supports variadics.

nicholasbishop avatar Dec 22 '21 20:12 nicholasbishop

@nicholasbishop That sounds like a good solution to me.

Would an ELF binary be capable of calling these functions (or any boot service functions, I guess)? I had assumed that boot services would be exited by the time a kernel is running.

timrobertsdev avatar Dec 22 '21 22:12 timrobertsdev

Fair point, I think Linux at least exits boot services quite early in the EFI stub.

nicholasbishop avatar Dec 23 '21 01:12 nicholasbishop

Would an ELF binary be capable of calling these functions (or any boot service functions, I guess)? I had assumed that boot services would be exited by the time a kernel is running.

That's the issue... we can't really guarantee this. It's what a logical, sane UEFI-based application/kernel would do: first load an ELF binary, then exist boot services and jump into it. But if we want to be pedantic and respect Rust's safety guarantees in all cases, something with a cfg guard like @nicholasbishop suggested would be more appropriate.

GabrielMajeri avatar Dec 23 '21 13:12 GabrielMajeri

Quick update here, on current nightly it looks like this is now possible to use efiabi with variadics. However, it's currently gated by the extended_varargs_abi_support feature so I would recommend we not add it for now to avoid requiring more unstable features.

(Also, as far as I can tell, InstallMultipleProtocolInterfaces is just a thin wrapper around calling InstallProtocolServices. We don't yet have the latter implemented, but we could add that easily I think.)

nicholasbishop avatar Nov 04 '22 21:11 nicholasbishop

Just chiming in here to say that getting the calling convention right isn't your only concern if you want to (correctly) support cross ABI calls. If any structs are involved you have to make sure that they are laid out correctly according to the EFI ABI. In particular, x86 sysv ABI has long long 4-byte aligned if it appears within a struct. The MS/EFI ABI expects it to be 8-byte aligned.

This will cause loaded image protocol (in particular) to have a bad memory layout if the uefi-rs definition were used in ELF ABI context. Whether that is a use-case you want to support or not is a different matter.

medhefgo avatar Apr 10 '23 13:04 medhefgo

At least with the install_multiple_protocol_interfaces API, that shouldn't be a problem. It's only callable while boot services are active, which means you are running a UEFI executable that uses the efiabi calling convention. I think we only need to worry about cross-ABI calls when in runtime-services mode after an OS loads.

nicholasbishop avatar Apr 11 '23 02:04 nicholasbishop