-
Notifications
You must be signed in to change notification settings - Fork 35
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
Implement basic support for the TRNG #99
Conversation
Limited configurability but it works.
Very cool addition! I wonder if this could implement std::rand::RngCore so it could be used with the std rand module? Doesn't seem too far off, would need an impl for next_u64, fill_bytes, and try_fill_bytes in addition to the already existing next_u32 |
There's two issues with supporting RngCore that I know of:
I'll add an RngCore implementation (probably as a wrapper created by |
Added optional RngCore impl. Open to opinions on the struct's name. (Can't be an optional implementation on the original TRNG struct unless the original I used intradoc links in a couple places of the documentation. They aren't stable yet, but their stabilization is occurring with Rust 1.48 (around five days from now). TRNG docs render fine on beta. |
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.
Looks great to me! Thanks for all the thorough testing and documentation.
How should this work?
let peripherals = imxrt1060_hal::Peripherals::take().unwrap();
let mut unclocked_trng = peripherals.trng;
// Set my TRNG settings, which I expect to persist
// between clocked & unclocked states.
unclocked_trng.set_retry_count(13);
unclocked_trng.set_sample_mode(SampleMode::Raw);
let mut trng = unclocked_trng.clock(&mut peripherals.ccm);
// Do random number things
let unclocked_again = trng.disable(&mut peripherals.ccm);
let mut trng = unclocked_again.clock(&mut peripherals.ccm);
// Retry count is 1, not 13
// Sample mode is VonNeumannRaw, not Raw
A user might expect the TRNG settings to persist across clocked and unclocked states. But it looks like we forget the previous settings on every transition from clocked to unclocked. This is OK with me; we might note this in the disable
documentation.
imxrt1060-hal/src/trng.rs
Outdated
|
||
/// The specified retry count was outside of the valid range of `1..=15`. | ||
#[derive(Copy, Clone, Debug)] | ||
pub struct InvalidRetryCountError; |
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.
This error struct could be constructed by anyone outside of the HAL crate. That's not a big deal, but we might want to give others the guarantee that we're the only people that can create this.
pub struct InvalidRetryCountError; | |
pub struct InvalidRetryCountError(()); |
Could demonstrate this with a failing doctest:
/// ```compile_fail
/// use imxrt1060_hal as hal;
/// use hal::trng::InvalidRetryCountError;
/// let err = InvalidRetryCountError(());
/// ```
#[cfg(doctest)]
struct UnconstructibleError;
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.
Thanks for catching that! Wasn't paying enough attention.
The config errors of I2C and SPI (ClockSpeedError
, PinLowTimeoutError
, BusIdleTimeoutError
, ModeError
) are also missing the private empty tuple. (I think I just mimicked them without using my brain.) Will make a PR to also make those unconstructible.
Added a slightly modified version of that test. With the (())
suffix it passed on both the wrongly constructible error and the correct private error.
My original expectation for |
This looks good to me as well |
This adds support for the True Random Number Generator. It has a bunch of configurable things, few of which are safe to touch without reading the Security Reference Manual. The SRM requires a licensing agreement (and NDA) with NXP, so this is written with reference to the MCUXpresso SDK.
I think this driver should work for any i.MX chip other than the 1015 and 1021, which need
TRNG_ACC
inMCTL
set to 1 after entering run mode. That field doesn't exist on other i.MX RT chips (it'sUNUSED5
). I think everything else is identical for the imxrt chips.The output passes dieharder and practrand (out to 2GB, anyway). The TRNG is very slow and it took weeks to generate that much data.
This retrieves all 512 bits of entropy at once from the TRNG, rather than reading 32 bits at a time. This is done for two reasons:
Includes support for embedded-hal's RNG
Read
trait, which may change in 1.0.Disabling the TRNG is slightly weird so I implemented support for that.
Hopefully I didn't miss anything in this.