Add Rust std support for x86_64-unknown-uefi by Ayush1325 · Pull Request #100316 · rust-lang/rust

Ayush1325 · 2022-08-09T10:10:50Z

Notice

I have created a smaller PR (#105861) to get just a stripped-down version of std for UEFI merged. This would help simplify the review process and speed up the merge.

Introduction

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

rust-highfive · 2022-08-09T10:10:53Z

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.

rust-highfive · 2022-08-09T10:10:54Z

⚠️ Warning ⚠️

These commits modify submodules.

Ayush1325 · 2022-08-09T10:14:33Z

r? @dvdhrm

joboet

I think this is really cool! I played a bit with UEFI myself, and while I would definitely not use std for writing bootloaders (where you need very precise control over initialisation), having it available should be really helpful when writing UEFI applications.

A small nitpick: io::Error::new allocates even for static data, so it is better to use the internal io::error::const_io_error for these cases.

library/std/src/sys_common/ucs2.rs

library/std/src/sys/uefi/stdio.rs

library/std/src/sys/uefi/os_str.rs

library/std/src/sys_common/ucs2.rs

Cargo.lock

Ayush1325 · 2022-08-11T10:59:46Z

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?

1. Add Command Protocol: For passing pipes and maybe other stuff in the future 2. Implentation of Pipe Protocol: `panic_abort_tests` now works which means almost all of libtest works. Signed-off-by: Ayush <ayushsingh1325@gmail.com>

There was a bug in vars_os() where the vriable name contained NULL on the Rust side. It is fixed now Signed-off-by: Ayush <ayushsingh1325@gmail.com>

1. Limit scope to crate 2. Make std::io::error private again 3. Fix atomic ordering 4. Remove unnecessary inlines 5. Remove r-efi re-exports: Returning void pointer in place of SystemTable pointer now. It can be cast to SystemTable pointer of user's choice. 6. Remove std::path::Prefix::UefiDevice 7. Fix CI 8. Mimic Windows OsStrExt and OsStringExt 9. move parse_lp_cmd_line to sys_common: This allows sharing parse_lp_cmd_line between Windows and UEFI. 10. No longer storing path in File struct. 11. Removed Windows panic_abort instructions since they did not work for UEFI 12. Overhaul VariableBox a. No longer implementing Deref, DerefMut, AsRef and AsRefMut for VariableBox. More about why this should be avoided can be found here: rust-lang/unsafe-code-guidelines#256 b. Only accessing contents of VariableBox using pointers Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

1. Add function pointers to Protocol 2. Using different function pointers for Null Pipes Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Trying to reduce noise produced by rustfmt in PR due to autoformat. Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Add implementation for IoSlice and IoSliceMut based on Fragment Data Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

1. Can now only initialize Global variales once. 2. Panics if `image_handle` or `system_table` is called before initializing them 3. Special case for `abort_internal()` which can be called before the initialization of Globals. Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

1. Fix CI: Fixed line number errors in output. 2. Remove unnecessary features 3. Fix uninteded downgrades on rebase in `Cargo.lock` 4. Use stack allocation for TCP4 transmit/receive 5. Remove unused files: a. library/std/src/sys/uefi/net/tcp6.rs b. library/std/src/sys_common/ucs2.rs 6. Fix Cargo.lock: Rollback unintentional upgrades. 7. Use UTF-16 for UEFI: Use Windows wtf8 implementation for UEFI. This was discussed in the PR and does not seem to cause any regressions. In face, it fixes some of the tests that were failing earlier. 8. Use Windows `std::os::ffi` for UEFI 9. Use Windows `std::sys::os_str` for UEFI Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Builds upon 4032d3a2378da875dfeb1522101d2e87033ae5c0. Use the non-panic version in places where panic should be avoided. Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

1. Implement copy 2. Fix copying cross device Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Do not ignore any tests for UEFI unless they do not fail gracefully. This should help reduce the surface area of this PR by a lot. Maybe think about ignoring them once this PR is merged. Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

1. Fix Handling connection closed by client. a. In Windows and Unix, 0-sized read requests succeed even after connection has been closed. 2. Implement timeouts using timer event. 3. Replace `unimplemented!()` and `todo!()` with `unsupported()`. 4. Fix handling u32 limits in size of vectored read/writes 5. Performance Enhancements: Create events for different actions only once. Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

1. Make Event *mut c_void. 2. Better error messages. 3. Use OnceLock instead of OnceCell in tcp4 4. Fix File::create_new 5. Performance improvements in `sys::args` and `sys::time::Instant` 6. Only use 2 events in tcp4 now. 7. Fix incorrect uses of `size_of_val`. 8. Fix UEFI CI Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Generate Entry function (`efi_main`) in compiler (similar to how C main is generated for other targets). This function passes `[image_handle, system_table]` as argv to Rust lang_start. Then initialize the global variables from `sys::init` Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

1. Another set of fixes needed due to rebase 2. Code Refactoring Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Implemented custom argument parsing for UEFI based on Section 3.4 of UEFI shell specification Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Switch to using r-efi-alloc for allocation. This is useful since a lot of UEFI projects will probably end up using this allocator anyway to have custom allocation types or to simply not use std. Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Ayush1325 · 2022-12-14T13:18:11Z

@dvdhrm, @thomcc While waiting for #105458 and #105145 to be merged, I have been thinking of how to proceed with network stuff. Initially, the plan was to use TCP protocols initially and have a better implementation based on EFI_MANAGED_NETWORK_PROTOCOL or EFI_SIMPLE_NETWORK_PROTOCOL in the future. The most promising one among them was using smoltcp for networking.

However, after playing around with smoltcp, I am not sure if a networking implementation based on it should be merged in std:

It has quite a few dependencies (which have their own dependencies), all of which will need to be pulled into std.
Many networking features are not implemented in smoltcp yet.

Writing a complete networking stack for UEFI will take a long time since I don't have any experience with networking, and I don't have enough time available. It may happen in the future, but probably not as a part of this PR.

So, I was wondering how I should proceed with networking. Here are a few options I came up with:

Just have EFI_TCP4_PROTOCOL implemented and leave everything else unsupported
Implement EFI_TCP6_PROTOCOL and leave UDP unsupported.
Implement all of the networking using EFI_TCP4_PROTOCOL, EFI_TCP6_PROTOCOL, EFI_UDP4_PROTOCOL and EFI_UDP6_PROTOCOL

TCP networking is useful because it allows running tests using remote-test-server and remote-test-client. I don't think it will be needed/used in most UEFI applications.

dvdhrm · 2022-12-16T14:04:45Z

Hey @Ayush1325 I cannot speak for upstream, so this is just my personal opinion. Please keep that in mind.

This PR is big, in lines added, commits added, and comment-history on the PR (+5,550 −37, >200 "issue-comments"). The effort required to review it and confidently sign off on it is huge. I personally am having a hard time tracking this reliably and staying on top of the changes you propose. And I am personally interested in it. So it likely is less of a priority for the people required to sign it off.

My personal recommendation to proceed would be:

Get this into a state you are happy with. The frequent updates make it very cumbersome to stay on top. If you are not confident that some bits are correct, please point this out. Get rid of FIXME annotations and replace them by a comment explaining the situation. Document all exported functions elaborately, focusing on corner-cases to ensure all situations are accounted for. Drop anything you are not happy with.
Strip this down as much as possible. The less code, the more likely it is going to be reviewed. I would very much prefer a PR that adds rust-std with an allocator and nothing else. Stub out NET, stub out FILES, stub out THREADS, stub out ARGS. If there are no stubs, make the necessary calls panic, avoid defining any optional calls. Once minimal support has landed, it is straightforward to add support for other modules later on (just be careful not to introduce stubs that break forward-compatibility). I highly admire your effort to implement as much as possible! It is highly appreciated and raises confidence that a port of std to UEFI is possible and feasible. Yet, I would very much prefer having this as follow-ups and thus reducing the amount of review required at a time. And even if minimal-std is rejected, we can easily start providing more PRs that build on the previous minimal-std PR and add the individual modules. Structuring this effort would help others like me to stay on top.
Get as much of your changes in outside of this PR. You already started this, but try more. The codegen-changes, the target-changes, the test-suite changes, I think all those can be merged already. UEFI-support is already in rustc, most of those side-changes are useful even without std-support. I mean UEFI core and alloc already work fine and are even Tier-2 now.

Maybe others prefer these all-or-nothing PRs, but for me personally it would be a lot easier to review if it was one step at a time.

Ayush1325 · 2022-12-16T16:45:45Z

Hey @Ayush1325 I cannot speak for upstream, so this is just my personal opinion. Please keep that in mind.

This PR is big, in lines added, commits added, and comment-history on the PR (+5,550 −37, >200 "issue-comments"). The effort required to review it and confidently sign off on it is huge. I personally am having a hard time tracking this reliably and staying on top of the changes you propose. And I am personally interested in it. So it likely is less of a priority for the people required to sign it off.

My personal recommendation to proceed would be:

Get this into a state you are happy with. The frequent updates make it very cumbersome to stay on top. If you are not confident that some bits are correct, please point this out. Get rid of FIXME annotations and replace them by a comment explaining the situation. Document all exported functions elaborately, focusing on corner-cases to ensure all situations are accounted for. Drop anything you are not happy with.

Strip this down as much as possible. The less code, the more likely it is going to be reviewed. I would very much prefer a PR that adds rust-std with an allocator and nothing else. Stub out NET, stub out FILES, stub out THREADS, stub out ARGS. If there are no stubs, make the necessary calls panic, avoid defining any optional calls. Once minimal support has landed, it is straightforward to add support for other modules later on (just be careful not to introduce stubs that break forward-compatibility). I highly admire your effort to implement as much as possible! It is highly appreciated and raises confidence that a port of std to UEFI is possible and feasible. Yet, I would very much prefer having this as follow-ups and thus reducing the amount of review required at a time. And even if minimal-std is rejected, we can easily start providing more PRs that build on the previous minimal-std PR and add the individual modules. Structuring this effort would help others like me to stay on top.

Get as much of your changes in outside of this PR. You already started this, but try more. The codegen-changes, the target-changes, the test-suite changes, I think all those can be merged already. UEFI-support is already in rustc, most of those side-changes are useful even without std-support. I mean UEFI core and alloc already work fine and are even Tier-2 now.

Maybe others prefer these all-or-nothing PRs, but for me personally it would be a lot easier to review if it was one step at a time.

Thanks for your comment. I will create a PR with the minimal std soon.

The reason for the gigantic PR was due to this work being done as a part of GSoC. Now that I'm just doing it in my free time, I can just break it down into smaller parts, starting with the portions I'm satisfied with.

bors · 2022-12-17T23:46:43Z

☔ The latest upstream changes (presumably #105145) made this pull request unmergeable. Please resolve the merge conflicts.

teg

This is a very exciting PR! Though I wonder if there would be a way to break it up into smaller chunks to make it easier to get merged? I left a few minor comments in-line.

library/std/src/sys/uefi/mod.rs

teg · 2022-12-18T23:43:16Z

library/std/src/sys/uefi/mod.rs

While ../unsupported returns (1, 2) from hashmap_random_keys(), is that really the right choice here? My understanding is that the only reason HashMap and friends are not in alloc is that we don't want to provide them unless backed by real randomness.

So should I unwrap? I can also use a PRNG with time as seed if that works better.

My understanding is that the only reason HashMap and friends are not in alloc is that we don't want to provide them unless backed by real randomness.

It is because liballoc can't depend on any OS existing and thus even when there is an OS it could only use a fixed seed.

I can also use a PRNG with time as seed if that works better.

That doesn't make much sense to me. The randomness here is to prevent HashDOS attacks. Time is predictable enough that you can likely perform a HashDOS attack anyway. In any case I don't think defending against a HashDOS attack is really necessary for UEFI programs. It only affects availability, not integrity or confidentiality. And UEFI programs are generally not exposed to the internet, so an attacker capable of performing a HashDOS attack can likely already hurt availability by either turning off the computer (in case of physical access) or by wiping the disk (in case of root access). If you are implementing something like netboot, an attacker could likely perform a DOS (denial of service) by responding very slowly to the requests anyway. I'm sure there are cases where not using randomness would enable a remote HashDOS attack that they couldn't do if there was randomness, but I personally don't think it is worth panicking in those cases. Especially because that is in itself effectively a DOS.

Ayush1325 · 2022-12-19T04:07:15Z

This is a very exciting PR! Though I wonder if there would be a way to break it up into smaller chunks to make it easier to get merged? I left a few minor comments in-line.

I have already created a new PR with minimal std (#105861). I will also update the description of this PR to contain that.

Dylan-DPC · 2023-05-16T13:32:26Z

Closing this in favour of #105861

rust-highfive assigned thomcc Aug 9, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 9, 2022

Ayush1325 force-pushed the uefi-master branch from 6358e8d to 5d88a0d Compare August 9, 2022 13:18

joboet reviewed Aug 9, 2022

View reviewed changes

library/std/src/sys_common/ucs2.rs Outdated Show resolved Hide resolved

library/std/src/sys_common/ucs2.rs Outdated Show resolved Hide resolved

library/std/src/sys/uefi/stdio.rs Outdated Show resolved Hide resolved

library/std/src/sys/uefi/os_str.rs Outdated Show resolved Hide resolved

Ayush1325 force-pushed the uefi-master branch 4 times, most recently from 96e5d0b to ccc4443 Compare August 10, 2022 16:59

joboet reviewed Aug 10, 2022

View reviewed changes

library/std/src/sys_common/ucs2.rs Outdated Show resolved Hide resolved

Ayush1325 force-pushed the uefi-master branch from adcb0d8 to fef19e7 Compare August 10, 2022 18:36

klensy reviewed Aug 10, 2022

View reviewed changes

Cargo.lock Outdated Show resolved Hide resolved

Ayush1325 mentioned this pull request Aug 11, 2022

Publish a new release of r-efi r-efi/r-efi#47

Closed

Ayush1325 force-pushed the uefi-master branch from 3a79f6e to d761f1c Compare August 11, 2022 08:18

This comment has been minimized.

Sign in to view

Ayush1325 requested review from bjorn3, joboet and klensy and removed request for joboet and klensy August 13, 2022 14:17

Ayush1325 added 15 commits December 1, 2022 11:40

Improve sys::process

c83cbd2

1. Add Command Protocol: For passing pipes and maybe other stuff in the future 2. Implentation of Pipe Protocol: `panic_abort_tests` now works which means almost all of libtest works. Signed-off-by: Ayush <ayushsingh1325@gmail.com>

Fix Env vars

a95933b

There was a bug in vars_os() where the vriable name contained NULL on the Rust side. It is fixed now Signed-off-by: Ayush <ayushsingh1325@gmail.com>

Improve Pipe Protocol

545899b

1. Add function pointers to Protocol 2. Using different function pointers for Null Pipes Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Revert formatting changes to tests

945049c

Trying to reduce noise produced by rustfmt in PR due to autoformat. Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Rebase on master

1baa38f

Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Implement io for UEFI

a9c1bb8

Add implementation for IoSlice and IoSliceMut based on Fragment Data Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Improve handling of Globals

2345867

Builds upon 4032d3a2378da875dfeb1522101d2e87033ae5c0. Use the non-panic version in places where panic should be avoided. Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Improve UEFI fs

d343822

1. Implement copy 2. Fix copying cross device Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Remove ignore-uefi from tests

dae6b19

Do not ignore any tests for UEFI unless they do not fail gracefully. This should help reduce the surface area of this PR by a lot. Maybe think about ignoring them once this PR is merged. Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

This comment has been minimized.

Sign in to view

Ayush1325 added 2 commits December 1, 2022 17:19

Fixes after rebase

f89fceb

1. Another set of fixes needed due to rebase 2. Code Refactoring Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Implement UEFI argument parsing

000228b

Implemented custom argument parsing for UEFI based on Section 3.4 of UEFI shell specification Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Ayush1325 mentioned this pull request Dec 5, 2022

Add batch flag to remote-test-server #105145

Merged

Use r-efi-alloc for allocator

d48d07d

Switch to using r-efi-alloc for allocation. This is useful since a lot of UEFI projects will probably end up using this allocator anyway to have custom allocation types or to simply not use std. Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>

Ayush1325 mentioned this pull request Dec 8, 2022

Allow blocking Command::output #105458

Merged

Ayush1325 mentioned this pull request Dec 18, 2022

Add Minimal Std implementation for UEFI #105861

Merged

teg reviewed Dec 18, 2022

View reviewed changes

Fixes suggested in PR

824f067

Uh oh!

Conversation

Ayush1325 commented Aug 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notice

Introduction

Related Links

Uh oh!

rust-highfive commented Aug 9, 2022

Uh oh!

rust-highfive commented Aug 9, 2022

Uh oh!

Ayush1325 commented Aug 9, 2022

Uh oh!

joboet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

Ayush1325 commented Aug 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Ayush1325 commented Dec 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dvdhrm commented Dec 16, 2022

Uh oh!

Ayush1325 commented Dec 16, 2022

Uh oh!

bors commented Dec 17, 2022

Uh oh!

teg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

teg Dec 18, 2022

Choose a reason for hiding this comment

Uh oh!

Ayush1325 Dec 19, 2022

Choose a reason for hiding this comment

Uh oh!

bjorn3 Dec 19, 2022

Choose a reason for hiding this comment

Uh oh!

Ayush1325 commented Dec 19, 2022

Uh oh!

Dylan-DPC commented May 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

17 participants

Ayush1325 commented Aug 9, 2022 •

edited

Loading

Ayush1325 commented Aug 11, 2022 •

edited

Loading

Ayush1325 commented Dec 14, 2022 •

edited

Loading