-
Notifications
You must be signed in to change notification settings - Fork 186
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 efi_rng
opt-in backend
#570
base: master
Are you sure you want to change the base?
Conversation
impl Error { | ||
pub(crate) const BOOT_SERVICES_UNAVAILABLE: Error = Self::new_internal(10); | ||
pub(crate) const NO_RNG_HANDLE: Error = Self::new_internal(11); | ||
pub(crate) const TEMP_EFI_ERROR: Error = Self::new_internal(12); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need TEMP_EFI_ERROR
now if we have #569 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I wrote in the OP, to encapsulate the UEFI status codes we need a wider state than 32-bits. Technically, on x86-64 we need 64 bits, but we probably can use NonZeroU64
since status codes with prefixes 0x1
, 0x3
, and 0x7
are used for "warning" codes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two questions:
- Do we think that adding in this backend is a blocker for 0.3?
- Is there a reason to have this be opt-in rather than our default implementation for the UEFI target? (Especially now that an alternative backend like
rdrand
can be enabled even on supported targets).
No, it's not a blocker and may be left for a later release.
There are two reasons:
|
The implementation is based on the
std
code. I am not sure why we need the try loop over protocol handles instead of just getting the first handle, but I decided to just followstd
here.Because the backend requires access to
BootServices
it has to rely on the unstableuefi_std
feature. We could alternatively use theuefi
crate, but it would tie the backend to theuefi
crate environment, which is probably not desirable.TEMP_EFI_ERROR
is used as a temporary solution until #568 is resolved, which I plan to do in a separate PR.cc @usamoi @nicholasbishop