Add Rust std support for x86_64-unknown-uefi

Open Ayush1325 opened this issue 2 years ago • 63 comments

Hello everyone, I have been working on porting Rust std to UEFI as a part of my Google Summer of Code 2022 project. While there are still problems left to fix, I would like to get more people experienced with Rust to check it out and provide feedback since this is my first time working with the Rust library internals.

Information about the current state of this implementation can be found at src/doc/rustc/src/platform-support/unknown-uefi.md. Some of the biggest limitations of the current implementation are the following:

~~The custom entry point function for UEFI is present at library/std/src/sys/uefi/mod.rs, This breaks using std with the no_main feature. Maybe this can be generated by the compiler like the C entry pointer, but not sure.~~: Now generated by compiler like C-main.
sys/uefi/net module was implemented only for running tests using remote-test-server and thus is not implemented for any actual production use. If someone really wants to implement it, it would be best to do it on top of SIMPLE_NETWORK_PROTOCOL, but that won't be possible in the GSoC timeframe.
No panic=unwind or backtrace support.
~~Pipes are basically a hack and should not be used unless someone knows what they are doing.~~: Using a dedicated Pipe Protocol now. However, this protocol hasen't been tested outside of src/tests/ui yet.
~~Feature panic_abort_tests is broken. While the UEFI stdio prints the correct output, capturing test output is not working with this feature.~~: Fixed
~~should_panic is broken for tests when panic_abort_tests is disabled.~~: Fixed
Command is kinda broken. The current spawn is blocking. More about this can be found here

Feel free to try it out and provide feedback. It is possible to run tests using remote-test-client and remote-test-server. For more details, you might want to look at my blog.

For those who are wondering how a target like UEFI can benefit from std support, here are a few examples:

Writing UEFI shell applications. This includes stuff like benchmarks, self-test utilities, etc. Many drivers should also be able to use std.
Finding UEFI target bugs. During this work, I have found 3 numeric tests that cause CPU exceptions for UEFI (they are fixed now. Also, I have found 2 additional bugs (which seem like bugs in llvm soft-float) which went unnoticed because there was no easy way to do any broad testing.
Provide a stable interface for library developers. The current std contains some functions under std::os::uefi::env to provide access to the underlying SystemTable and SystemHandle, which are essential for writing anything for UEFI.

Related Links

Aug 09 '22 10:08 Ayush1325

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @thomcc (or someone else) soon.

Please see the contribution instructions for more information.

Aug 09 '22 10:08 rust-highfive

:warning: Warning :warning:

These commits modify submodules.

Aug 09 '22 10:08 rust-highfive

r? @dvdhrm

Aug 09 '22 10:08 Ayush1325

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling xz2 v0.1.6
   Compiling toml v0.5.9
    Finished dev [unoptimized] target(s) in 47.88s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Updating git repository `https://github.com/r-efi/r-efi`
---
     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | / pub enum Prefix<'a> {
144  | |     /// Verbatim prefix, e.g., `\\?\cat_pics`.
145  | |     ///
146  | |     /// Verbatim prefixes consist of `\\?\` immediately followed by the given
...    |
193  | |     UefiDevice(&'a OsStr),
     | |_^
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 11 '22 09:08 rust-log-analyzer

I would like other people's opinions about adding UefiPrefix to std::path::Prefix. The reason this was added is that a file path on UEFI looks like the following:

{Volume Device Path}/\\{file path from root}

Thus I added UefiPrefix to represent the Volume Device Path as Prefix (Since it is similar to windows drives. Uefi Shell treats it like drives providing a shorthand to access them as well). However, seeing the CI, it might break quite a few crates, so maybe I should use one of the pre-existing enum variants and change the documentation?

Aug 11 '22 10:08 Ayush1325

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling xz2 v0.1.6
   Compiling toml v0.5.9
    Finished dev [unoptimized] target(s) in 53.58s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Updating git repository `https://github.com/r-efi/r-efi`
---
     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | / pub enum Prefix<'a> {
144  | |     /// Verbatim prefix, e.g., `\\?\cat_pics`.
145  | |     ///
146  | |     /// Verbatim prefixes consist of `\\?\` immediately followed by the given
...    |
193  | |     UefiDevice(&'a OsStr),
     | |_^
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 11 '22 13:08 rust-log-analyzer

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling xz2 v0.1.6
   Compiling toml v0.5.9
    Finished dev [unoptimized] target(s) in 45.52s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Updating git repository `https://github.com/r-efi/r-efi`
---
     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | / pub enum Prefix<'a> {
144  | |     /// Verbatim prefix, e.g., `\\?\cat_pics`.
145  | |     ///
146  | |     /// Verbatim prefixes consist of `\\?\` immediately followed by the given
...    |
193  | |     UefiDevice(&'a OsStr),
     | |_^
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 11 '22 18:08 rust-log-analyzer

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling xz2 v0.1.6
   Compiling toml v0.5.9
    Finished dev [unoptimized] target(s) in 46.01s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Updating git repository `https://github.com/r-efi/r-efi`
---
     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | pub enum Prefix<'a> {
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 13 '22 06:08 rust-log-analyzer

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling xz2 v0.1.6
   Compiling toml v0.5.9
    Finished dev [unoptimized] target(s) in 48.23s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Updating git repository `https://github.com/r-efi/r-efi`
---
     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | pub enum Prefix<'a> {
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 14 '22 08:08 rust-log-analyzer

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling xz2 v0.1.6
   Compiling toml v0.5.9
    Finished dev [unoptimized] target(s) in 46.39s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Updating git repository `https://github.com/r-efi/r-efi`
---
     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | pub enum Prefix<'a> {
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 14 '22 08:08 rust-log-analyzer

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling xz2 v0.1.6
   Compiling toml v0.5.9
    Finished dev [unoptimized] target(s) in 49.14s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Updating git repository `https://github.com/r-efi/r-efi`
---
     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | pub enum Prefix<'a> {
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 14 '22 13:08 rust-log-analyzer

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling xz2 v0.1.6
   Compiling toml v0.5.9
    Finished dev [unoptimized] target(s) in 54.18s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Updating git repository `https://github.com/r-efi/r-efi`
---
     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | pub enum Prefix<'a> {
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 15 '22 09:08 rust-log-analyzer

:umbrella: The latest upstream changes (presumably #100677) made this pull request unmergeable. Please resolve the merge conflicts.

Aug 17 '22 18:08 bors

So, I'm not reviewing this yet (still marked as draft), but I'll mention these early:

This adds some new apis in std::os::uefi. These will need to go through the API change proposal process documented here: https://std-dev-guide.rust-lang.org/feature-lifecycle/api-change-proposals.html
This seems to implement some stdlib traits for types from the r-efi. I'm not sure that's something we want.

Aug 18 '22 18:08 thomcc

1. This adds some new apis in std::os::uefi. These will need to go through the API change proposal process documented here: https://std-dev-guide.rust-lang.org/feature-lifecycle/api-change-proposals.html

I see. I will start working on that.

2. This seems to implement some stdlib traits for types from the `r-efi`. I'm not sure that's something we want.

Yeah, I was also thinking about converting them to normal functions and moving them back to std::sys

Aug 18 '22 18:08 Ayush1325

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling xz2 v0.1.6
   Compiling toml v0.5.9
    Finished dev [unoptimized] target(s) in 58.49s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Updating git repository `https://github.com/r-efi/r-efi`
---
     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | pub enum Prefix<'a> {
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 21 '22 19:08 rust-log-analyzer

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

   Compiling xz2 v0.1.6
   Compiling toml v0.5.9
    Finished dev [unoptimized] target(s) in 58.29s
Checking stage0 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
    Updating git repository `https://github.com/r-efi/r-efi`
---
     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | pub enum Prefix<'a> {
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 22 '22 21:08 rust-log-analyzer

I'm somewhat concerned about ~15MB of binaries committed here for testing. Is it a temporary measure? I suspect that the infra team can help with hosting them elsewhere.

Aug 22 '22 23:08 petrochenkov

Yes, those shouldn't be in-tree. We can move them to S3 pretty easily and provide you with some download URLs - please reach out on #t-infra in Zulip.

I didn't review in detail, but it would be good to establish how those files are generated / what is in them and document that (e.g., a README in the docker directory), so that later down the line we have some guidance on how to reproduce + adjust that. (If it makes sense, we can also setup dedicated build pipeline that constructs them).

Aug 22 '22 23:08 Mark-Simulacrum

I'm somewhat concerned about ~15MB of binaries committed here for testing. Is it a temporary measure? I suspect that the infra team can help with hosting them elsewhere.

Good catch; I have been thinking of using Fedora Docker image instead of Ubuntu since Fedora has the required and updated OVMF packages by default, removing the need for storing binaries.

Aug 23 '22 05:08 Ayush1325

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | pub enum Prefix<'a> {
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 23 '22 15:08 rust-log-analyzer

I'm going to mark this as waiting on author until you're ready for review. While it will take quite some time to actually review this, for now it doesn't seem quite finished. Please indicate it's ready for review once it is with @rustbot ready.

Aug 24 '22 04:08 thomcc

Hi @thomcc, can you elaborate on what a finished state of this PR should look like? More specifically, what should be fixed before it can be merged.

I do not have much idea about how the issues mentioned in my original PR commit can be fixed ( or in case of net, process and pipe whether they should even be fixed).

I am mostly making the changes/fixes requested on this PR now instead of adding new stuff (since most of the important things for UEFI have been implemented), so any guidance will be great.

Aug 24 '22 05:08 Ayush1325

Sorry, if it's already for review (e.g. you aren't planning on making more changes) I can try to start on it. I misunderstood the state of the PR, since it seemed like you were actively making changes.

I do think some of the issues up top are probably problems, like the pipes -- Depending on how hacky/broken they are we probably don't want to expose them. But I guess I can take a look and let you know.

Anyway, I'll mark this as waiting on review then. Note that because of the size of the PR (and because I can't start this week), it may take some time. I'll try to do it incrementally and give some feedback as I go, though.

@rustbot ready

Aug 24 '22 12:08 thomcc

I do think some of the issues up top are probably problems, like the pipes -- Depending on how hacky/broken they are we probably don't want to expose them. But I guess I can take a look and let you know.

I see, I will try finding a better way to implement pipes. Maybe write a simple protocol that works for Rust std.

Anyway, I'll mark this as waiting on review then. Note that because of the size of the PR (and because I can't start this week), it may take some time. I'll try to do it incrementally and give some feedback as I go, though.

Sure, incremental feedback is fine. It's my first time working on bot UEFI and Rust internals, so there are bound to be rough edges.

Aug 24 '22 12:08 Ayush1325

I see, I will try finding a better way to implement pipes. Maybe write a simple protocol that works for Rust std.

To be clear, not exposing something is probably better than exposing something broken, or having an extremely complicated impl under the hood.

Aug 25 '22 00:08 thomcc

@thomcc So I went ahead with writing a protocol for Pipes. The reason was to fix the panic_abort_tests feature in libtest, which now works perfectly. The new pipes are much more flexible and can be improved iteratively. Finally, they don't cause any regressions.

Aug 27 '22 11:08 Ayush1325

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | pub enum Prefix<'a> {
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 27 '22 11:08 rust-log-analyzer

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | pub enum Prefix<'a> {
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 27 '22 14:08 rust-log-analyzer

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

     |
note: `std::path::Prefix` defined here
    --> /checkout/library/std/src/path.rs:143:1
     |
143  | pub enum Prefix<'a> {
     = note: the matched value is of type `std::path::Prefix`
     = note: the matched value is of type `std::path::Prefix`
help: ensure that all possible cases are being handled by adding a match arm with a wildcard pattern or an explicit pattern as shown
     |
1892 ~             Prefix::Disk(drive) => Utf8Prefix::Disk(drive),
1893 ~             _ => todo!(),

For more information about this error, try `rustc --explain E0004`.
error: could not compile `camino` due to previous error
warning: build failed, waiting for other jobs to finish...

Aug 28 '22 06:08 rust-log-analyzer

rust rust copied to clipboard

Add Rust std support for x86_64-unknown-uefi

Related Links

rust
rust copied to clipboard