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 Minimal Std implementation for UEFI #105861

Merged
merged 8 commits into from
Sep 24, 2023

Conversation

Ayush1325
Copy link
Contributor

Implemented modules:

  1. alloc
  2. os_str
  3. env
  4. math

Related Links

Tracking Issue: #100499
API Change Proposal: rust-lang/libs-team#87

Additional Information

This was originally part of #100316. Since that PR was becoming too unwieldy and cluttered, and with suggestion from @dvdhrm, I have extracted a minimal std implementation to this PR.

The example in src/doc/rustc/src/platform-support/unknown-uefi.md has been tested for x86_64-unknown-uefi and i686-unknown-uefi in OVMF. It would be great if someone more familiar with AARCH64 can help with testing for that target.

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

@rustbot
Copy link
Collaborator

rustbot commented Dec 18, 2022

r? @m-ou-se

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 18, 2022
@rustbot
Copy link
Collaborator

rustbot commented Dec 18, 2022

These commits modify compiler targets.
(See the Target Tier Policy.)

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

Copy link
Contributor

@dvdhrm dvdhrm left a comment

Choose a reason for hiding this comment

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

Thanks a lot! This looks so much easier to review! A few initial comments inline, and I will see whether I can give this a thorough review over the next week.

compiler/rustc_codegen_ssa/src/base.rs Outdated Show resolved Hide resolved
compiler/rustc_codegen_ssa/src/base.rs Show resolved Hide resolved
src/tools/tidy/src/deps.rs Outdated Show resolved Hide resolved
@Ayush1325 Ayush1325 force-pushed the uefi-std-minimial branch 2 times, most recently from 22f8692 to 3472331 Compare December 18, 2022 11:31
@Ayush1325 Ayush1325 force-pushed the uefi-std-minimial branch 2 times, most recently from ce514e8 to 798515a Compare December 19, 2022 04:51
@rust-log-analyzer

This comment has been minimized.

@Ayush1325 Ayush1325 force-pushed the uefi-std-minimial branch 2 times, most recently from 955ff41 to 84031f3 Compare December 19, 2022 11:11
@bors
Copy link
Contributor

bors commented Jan 8, 2023

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

@Ayush1325
Copy link
Contributor Author

@m-ou-se So is there anything I should do or any changes to this PR? Alternatively, maybe @thomcc should be assigned to this since he was assigned to the previous PR, and thus is more familiar with this work.

@Ayush1325
Copy link
Contributor Author

@dvdhrm ping

@thomcc
Copy link
Member

thomcc commented Jan 26, 2023

Alternatively, maybe @thomcc should be assigned to this since he was assigned to the previous #100316, and thus is more familiar with this work.

I don't have as much time for reviews as I used to, but do have more context and a shorter review backlog than @m-ou-se, so sure, I don't mind taking it. I probably won't get to it for a week or two though, FYI.

r? @thomcc

@rustbot rustbot assigned thomcc and unassigned m-ou-se Jan 26, 2023
library/std/src/os/uefi/env.rs Outdated Show resolved Hide resolved
library/std/src/sys/uefi/common.rs Outdated Show resolved Hide resolved
library/std/src/sys/uefi/alloc.rs Outdated Show resolved Hide resolved
library/std/src/sys/uefi/alloc.rs Outdated Show resolved Hide resolved
Copy link
Member

@joboet joboet left a comment

Choose a reason for hiding this comment

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

io::Error should always use the unpacked representation, because the packed representation assumes that error codes are always 32-bits, which they are not in 64-bit UEFI.

Comment on lines 377 to 379
// `RawOsError` must be an alias for `i32`.
const _: fn(RawOsError) -> i32 = |os| os;

Copy link
Member

Choose a reason for hiding this comment

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

Bitpacking needs the bit size of the OS error to be smaller than the pointer size to be able to add a tag value (that's why I added this assertion). Hence UEFI must use the unpacked representation for io::Error.

library/std/src/os/mod.rs Outdated Show resolved Hide resolved
@bors
Copy link
Contributor

bors commented Sep 21, 2023

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

@workingjubilee
Copy link
Member

augh.

Implemented modules:
1. alloc
2. os_str
3. env
4. math

Tracking Issue: rust-lang#100499
API Change Proposal: rust-lang/libs-team#87

This was originally part of rust-lang#100316. Since
that PR was becoming too unwieldy and cluttered, and with suggestion
from @dvdhrm, I have extracted a minimal std implementation to this PR.

Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>
Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>
- Make BootServices unavailable if ExitBootServices event is signaled.
- Use thread locals for SystemTable and ImageHandle

Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>
Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>
- Some comment fixes.
- Make some functions unsafe.
- Make helpers module private.
- Rebase on master
- Update r-efi to v4.2.0

Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>
Some changes from this commit will probably be converted to its own PR.

Signed-off-by: Ayush Singh <ayushsingh1325@gmail.com>
- Update Example
- Add thread_parking to sys::uefi
- Fix unsafe in unsafe errors
- Improve docs
- Improve os/exit
- Some asserts
- Switch back to atomics

Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>
- Hide Docs
- Use repr_unpacked error

Signed-off-by: Ayush Singh <ayushdevel1325@gmail.com>
@Ayush1325 Ayush1325 reopened this Sep 22, 2023
@Ayush1325
Copy link
Contributor Author

Well, I mistakenly pushed master after the last rebase, which closed the PR due to 0 delta.

@workingjubilee
Copy link
Member

Mmmkay. Took another look. Apologies, again.

@bors r+

@bors
Copy link
Contributor

bors commented Sep 24, 2023

📌 Commit 984ecef has been approved by workingjubilee

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 24, 2023
@bors
Copy link
Contributor

bors commented Sep 24, 2023

⌛ Testing commit 984ecef with merge c7224e3...

@bors
Copy link
Contributor

bors commented Sep 24, 2023

☀️ Test successful - checks-actions
Approved by: workingjubilee
Pushing c7224e3 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 24, 2023
@bors bors merged commit c7224e3 into rust-lang:master Sep 24, 2023
14 of 22 checks passed
@rustbot rustbot added this to the 1.74.0 milestone Sep 24, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c7224e3): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
3.7% [3.7%, 3.7%] 1
Regressions ❌
(secondary)
2.5% [1.1%, 4.9%] 5
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.9% [-1.0%, -0.7%] 3
All ❌✅ (primary) 3.7% [3.7%, 3.7%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 630.239s -> 629.545s (-0.11%)
Artifact size: 317.10 MiB -> 317.31 MiB (0.07%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. O-UEFI UEFI S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.