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

LoadFileProtocol and LoadFile2Protocol #1297

Merged
merged 2 commits into from
Aug 15, 2024
Merged

Conversation

phip1611
Copy link
Contributor

@phip1611 phip1611 commented Aug 7, 2024

Supersedes #945. Implements LoadFile and LoadFile2, including an integration test. The way we are implementing the LoadFile(2) protocol is roughly what certain Linux loaders do so that Linux can find its initrd. See code for references.

Steps to Undraft

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@phip1611 phip1611 changed the title xxx LoadFileProtocol and LoadFile2Protocol Aug 7, 2024
@phip1611 phip1611 marked this pull request as ready for review August 9, 2024 15:16
@phip1611 phip1611 force-pushed the load2 branch 2 times, most recently from 3b31f08 to 8ba5fa2 Compare August 9, 2024 15:25
@phip1611
Copy link
Contributor Author

phip1611 commented Aug 9, 2024

@nicholasbishop I noticed that we don't really have a streamlined strategy or code base regarding what is alloc and not alloc. Sometimes we feature-gate functionality between alloc, and sometimes we provide "non-alloc` compatible functions where a buffer is specified.

IMHO, we could also "turn the switch" to only provide first-class citizen support for the alloc feature as I think that UEFI applications without alloc are very rare. I can't really think of use-cases.

The only people don't want to use alloc are the people in safety-critical domains where they need deterministic memory management. But I think those people won't use UEFI in the first place 😁

I think we should also tell a few sentences about our strategy on that in the documentation of the crate. @nicholasbishop

@phip1611 phip1611 force-pushed the load2 branch 2 times, most recently from 71cda2f to 70ce3d8 Compare August 9, 2024 15:38
@nicholasbishop
Copy link
Member

I agree it's worth thinking more about the alloc strategy, and even if we decide to keep everything the same we should indeed document our approach more clearly. (And if we do decide changes are warranted I would want to hold off a bit as we already have large-scale changes happening in the repo.) Want to open an issue for this question?

@phip1611
Copy link
Contributor Author

phip1611 commented Aug 9, 2024

Want to open an issue for this question?

#1303

uefi-raw/src/protocol/media.rs Outdated Show resolved Hide resolved
shell.nix Outdated Show resolved Hide resolved
uefi-test-runner/src/proto/load.rs Show resolved Hide resolved
uefi/src/proto/media/load_file.rs Show resolved Hide resolved
@phip1611
Copy link
Contributor Author

Ready for the next review-round.

uefi/src/proto/mod.rs Outdated Show resolved Hide resolved
uefi/src/proto/mod.rs Outdated Show resolved Hide resolved
uefi-test-runner/src/proto/load.rs Show resolved Hide resolved
This actually tests multiple things:
- LoadFile and LoadFile2 wrappers work
- (un)install_protocol_interface works
- how our API is suited to create a custom implementation of an existing
  protocol definition
- The workflow used in Linux loaders to enable the Linux EFI stub to load
  the initrd via the LOAD_FILE2 protocol.

Further what I'm doing in this test, is already deployed in the
wild: https://github.com/nix-community/lanzaboote/blob/b7f68a50e6902f28c07a9f8d41df76f4c0a9315b/rust/uefi/linux-bootloader/src/linux_loader.rs#L142
@nicholasbishop nicholasbishop added this pull request to the merge queue Aug 15, 2024
Merged via the queue into rust-osdev:main with commit 68b85bb Aug 15, 2024
13 checks passed
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.

2 participants