Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UEFI target support #1406

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

xiaoyuxlu
Copy link

@xiaoyuxlu xiaoyuxlu commented Sep 30, 2021

build.rs:

ABI of rust UEFI target x86_64-unknown-uefi is like windows. So add an AsmTarget like windows and it also need preassemble.

Currently must use Clang to compile c code for rust UEFI target.

rand.rs:

use rdrand-x86_64.pl from BoringSSL to generate random numbers.

Copy link
Owner

@briansmith briansmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this.

In .github/ci.yml where have some blocks like this:

          - target: aarch64-apple-darwin
            host_os: macos-latest
            # GitHub Actions doesn't have a way to run this target yet.
            cargo_options: --no-run

Please add an analogous one for the x86_64 UEFI target.

Similarly there is:

        target:
          - aarch64-apple-ios
          - ...
          - x86_64-unknown-linux-gnu

Add the x86_64 UEFI target to that.

Also, could you point me to a way that we could emulate the UEFI environment well enough to run tests? It would be ideal if we could get this emulated environment working in GitHub Actions, but if this is a large amount of work, we can go with the build-only approach for now.

build.rs Outdated Show resolved Hide resolved
build.rs Outdated Show resolved Hide resolved
build.rs Outdated Show resolved Hide resolved
build.rs Outdated Show resolved Hide resolved
src/rand.rs Outdated Show resolved Hide resolved
@briansmith
Copy link
Owner

Since we need to force clang for this target, mk/install-build-tools.sh can be modified here to add this target:

--target=i686-unknown-linux-musl|--target=x86_64-unknown-linux-musl)
  use_clang=1

Please make the analogous change in mk/cargo.sh.

This should make CI work and will also make it easier for anybody, especially me, to build locally.

@briansmith
Copy link
Owner

Does CPUID work reliabily (as expected) in the UEFI environment?

@briansmith
Copy link
Owner

Also, are there any limitations or special considerations on CPU features (AVX2, AES-NI, etc.) for the EUFI environment?

@xiaoyuxlu
Copy link
Author

xiaoyuxlu commented Oct 9, 2021

A few days ago was China National Day. Sorry for the late reply.
I appreciate you for watching the pull request carefully and give a lot of comments.

I will continue to work on it and make this patch better.

Thanks for this.

In .github/ci.yml where have some blocks like this:

          - target: aarch64-apple-darwin
            host_os: macos-latest
            # GitHub Actions doesn't have a way to run this target yet.
            cargo_options: --no-run

Please add an analogous one for the x86_64 UEFI target.

Similarly there is:

        target:
          - aarch64-apple-ios
          - ...
          - x86_64-unknown-linux-gnu

Add the x86_64 UEFI target to that.

Also, could you point me to a way that we could emulate the UEFI environment well enough to run tests? It would be ideal if we could get this emulated environment working in GitHub Actions, but if this is a large amount of work, we can go with the build-only approach for now.

Currently, I don't have a good way to run tests. I will try to find a way. I think we can build-only for now.

Does CPUID work reliabily (as expected) in the UEFI environment?

I think so.
UEFI environment always run in long-mode, in x86_64 it always run in 64bit.

Also, are there any limitations or special considerations on CPU features (AVX2, AES-NI, etc.) for the EUFI environment?

There is no special things.
REF: https://github.com/rust-lang/rust/blob/master/compiler/rustc_target/src/spec/x86_64_unknown_uefi.rs#L15

Xiaoyu Lu added 3 commits October 19, 2021 09:16
Rust UEFI target `x86_64-unknown-uefi` using BoringSSL's `CRYPTO_rdrand`
 & `CRYPTO_rdrand_mutiple8_buf` to obtain random numbers under
the X86_64 architecture.
Because UEFI needs to be compiled without std in nightly toolchain.
So add CI separately.
@jyao1
Copy link

jyao1 commented Jan 17, 2022

@briansmith, would you please review this patch to see if there is any other comment?

@tiziano88
Copy link

This patch would be useful to us too as part of https://github.com/project-oak/oak , is there anything in particular that is blocking it? Can we help resolve it? Thanks @xiaoyuxlu and @briansmith for the work so far

jul-sh added a commit to jul-sh/oak that referenced this pull request Apr 4, 2022
Ring has been cloned cloned into third_party, using commit 32b2c6c39ef459aad47f476fe9139b8d57532ab1 from https://github.com/briansmith/ring. This is done so that we will be able to later port our own patch onto it, specifically UEFI compatibility from: briansmith/ring#1406.

The cloned version is the unreleased version 0.17, which contains some API changes. Our code has been updated to accomodate them.
jul-sh added a commit to jul-sh/oak that referenced this pull request Apr 4, 2022
Ring has been cloned cloned into third_party, using commit 32b2c6c39ef459aad47f476fe9139b8d57532ab1 from https://github.com/briansmith/ring. This is done so that we will be able to later port our own patch onto it, specifically UEFI compatibility from: briansmith/ring#1406.

The cloned version is the unreleased version 0.17, which contains some API changes. Our code has been updated to accommodate them.
jul-sh added a commit to jul-sh/oak that referenced this pull request Apr 4, 2022
Ring has been cloned cloned into third_party, using commit 32b2c6c39ef459aad47f476fe9139b8d57532ab1 from https://github.com/briansmith/ring. This is done so that we will be able to later port our own patch onto it, specifically UEFI compatibility from: briansmith/ring#1406.
jul-sh added a commit to jul-sh/oak that referenced this pull request Apr 4, 2022
Ring has been cloned cloned into third_party, using commit 32b2c6c39ef459aad47f476fe9139b8d57532ab1 from https://github.com/briansmith/ring. This is done so that we will be able to later port our own patch onto it, specifically UEFI compatibility from: briansmith/ring#1406.
jul-sh added a commit to jul-sh/oak that referenced this pull request Apr 5, 2022
Ring has been cloned cloned into third_party, using commit 32b2c6c39ef459aad47f476fe9139b8d57532ab1 from https://github.com/briansmith/ring. This is done so that we will be able to later port our own patch onto it, specifically UEFI compatibility from: briansmith/ring#1406.
jul-sh added a commit to jul-sh/oak that referenced this pull request Apr 5, 2022
Ring has been cloned cloned into third_party, using commit 32b2c6c39ef459aad47f476fe9139b8d57532ab1 from https://github.com/briansmith/ring. This is done so that we will be able to later port our own patch onto it, specifically UEFI compatibility from: briansmith/ring#1406.
jul-sh added a commit to jul-sh/oak that referenced this pull request Apr 6, 2022
Ring has been cloned cloned into third_party, using commit 32b2c6c39ef459aad47f476fe9139b8d57532ab1 from https://github.com/briansmith/ring. This is done so that we will be able to later port our own patch onto it, specifically UEFI compatibility from: briansmith/ring#1406.
jul-sh added a commit to jul-sh/oak that referenced this pull request Apr 6, 2022
Ring has been cloned cloned into third_party, using commit 32b2c6c39ef459aad47f476fe9139b8d57532ab1 from https://github.com/briansmith/ring. This is done so that we will be able to later port our own patch onto it, specifically UEFI compatibility from: briansmith/ring#1406.
jul-sh added a commit to project-oak/oak that referenced this pull request Apr 6, 2022
* Clone ring from the head of the ring repository into third_party

Ring has been cloned cloned into third_party, using commit 32b2c6c39ef459aad47f476fe9139b8d57532ab1 from https://github.com/briansmith/ring. This is done so that we will be able to later port our own patch onto it, specifically UEFI compatibility from: briansmith/ring#1406.

* exclude ring from workspace

* format

* increment cache version
@jyao1
Copy link

jyao1 commented Apr 13, 2022

This has been there for half a year.

@briansmith, would you please review this patch to see if there is any other comment?

Copy link
Owner

@briansmith briansmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of all the rdrand stuff, please add a less-safe-getrandom-rdrand non-default feature to ring. Then change rand.rs so to add UEFI to the lost of supported operating systems/environments if and only if that feature is also enabled.

Presumably then SGX and similar would do likewise.

override: true
components: rust-src

# Currently, there is no 'rust-std' for target 'x86_64-unknown-uefi'.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still the case? Or can we simplify all this now?

@@ -82,6 +82,7 @@ include = [
"crypto/fipsmodule/modes/asm/ghash-x86.pl",
"crypto/fipsmodule/modes/asm/ghash-x86_64.pl",
"crypto/fipsmodule/modes/asm/ghashv8-armx.pl",
"crypto/fipsmodule/rand/asm/rdrand-x86_64.pl",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can go away since we'd use getrandom.

@@ -64,6 +64,7 @@ const RING_SRCS: &[(&[&str], &str)] = &[
(&[X86_64], "crypto/fipsmodule/ec/asm/p256-x86_64-asm.pl"),
(&[X86_64], "crypto/fipsmodule/modes/asm/aesni-gcm-x86_64.pl"),
(&[X86_64], "crypto/fipsmodule/modes/asm/ghash-x86_64.pl"),
(&[X86_64], "crypto/fipsmodule/rand/asm/rdrand-x86_64.pl"),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

@@ -222,7 +223,7 @@ const ASM_TARGETS: &[AsmTarget] = &[
preassemble: true,
},
AsmTarget {
oss: &[WINDOWS],
oss: &[WINDOWS, UEFI],
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This deserves a comment about why UEFI uses Windows ASM stuff unlike everything else.

@@ -381,8 +384,9 @@ fn pregenerate_asm_main() {
perlasm(&perlasm_src_dsts, asm_target);

if asm_target.preassemble {
// Preassembly is currently only done for Windows targets.
assert_eq!(&asm_target.oss, &[WINDOWS]);
// Preassembly is currently done for Windows and UEFI targets.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's just remove the comment since there's now a preassemble field that makes this clear, and it duplicates the assertion below.

@@ -460,7 +464,7 @@ fn build_c_code(

// For Windows we also pregenerate the object files for non-Git builds so
// the user doesn't need to install the assembler.
if use_pregenerated && target.os == WINDOWS {
if use_pregenerated && supports_preassembly(&target.arch, &target.os) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this to look at the preassemble field.

@@ -554,7 +558,7 @@ fn compile(p: &Path, target: &Target, include_dir: &Path, out_dir: &Path) -> Str
p.to_str().expect("Invalid path").into()
} else {
let out_file = obj_path(out_dir, p);
let cmd = if target.os != WINDOWS || ext != "asm" {
let cmd = if !supports_preassembly(&target.arch, &target.os) || ext != "asm" {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

@@ -627,17 +632,25 @@ fn cc(file: &Path, ext: &str, target: &Target, include_dir: &Path, out_file: &Pa
}
}

// UEFI is a baremental freestanding environment without stdlib.
let freestanding = target.os == UEFI;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this still true? I heard there is a libstd now.

let mut c = Command::new(&get_command(
"NASM_EXECUTABLE",
"./target/tools/windows/nasm/nasm",
));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this to a separate PR.

@briansmith
Copy link
Owner

This is a very old PR. If you were to rebase it onto today's main, most of it would go away as explained above. If/when this is ready, I will release a 17.x update that adds UEFI support.

@xiaoyuxlu
Copy link
Author

This is a very old PR. If you were to rebase it onto today's main, most of it would go away as explained above. If/when this is ready, I will release a 17.x update that adds UEFI support.

You're right.
Will do. Thank you very much.

@hugusmaximus
Copy link

hugusmaximus commented Dec 30, 2023

Hi there! I'm not a developer nor I am too familiarized with some specific OS internals and faced a problem trying to compile "tiny_http" crate with "rustls" support on a aarch64 platform and Hermit (https://github.com/hermit-os/hermit-rs/tree/main) unikernel. After reading this thread I think I may be facing the same problem as other people trying to compile on other platforms... This is my error:

error[E0425]: cannot find function `fill_impl` in this scope
   --> /home/hugo/.cargo/registry/src/index.crates.io-6f17d22bba15001f/ring-0.16.20/src/rand.rs:160:9
    |
160 |         fill_impl(dest)
    |         ^^^^^^^^^
    |
help: you might have meant to call the method
    |
160 |         self.fill_impl(dest)
    |         +++++

For more information about this error, try `rustc --explain E0425`.
error: could not compile `ring` (lib) due to 1 previous error

@briansmith does the release 17.x of ring solve my problem too? Any hint on how I need to proceed to solve that? Does it require intervention of the developer of Hermit OS?

(Edited: found this in case this can help: rust-random/getrandom#199)

Best,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants