diff --git a/Cargo.lock b/Cargo.lock index 5d7973c2b6..ab356871cb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2137,6 +2137,7 @@ dependencies = [ "idol", "idol-runtime", "num-traits", + "ringbuf", "stm32h7", "userlib", "zerocopy 0.6.6", diff --git a/app/demo-stm32h7-nucleo/app-h743.toml b/app/demo-stm32h7-nucleo/app-h743.toml index 155c0d215f..a027771c32 100644 --- a/app/demo-stm32h7-nucleo/app-h743.toml +++ b/app/demo-stm32h7-nucleo/app-h743.toml @@ -138,7 +138,6 @@ start = true features = ["h743"] priority = 3 name = "drv-stm32h7-rng" -max-sizes = {flash = 8192, ram = 512} stacksize = 256 start = true task-slots = ["sys", "user_leds"] diff --git a/app/demo-stm32h7-nucleo/app-h753.toml b/app/demo-stm32h7-nucleo/app-h753.toml index da6864e8e2..a7d0129c18 100644 --- a/app/demo-stm32h7-nucleo/app-h753.toml +++ b/app/demo-stm32h7-nucleo/app-h753.toml @@ -189,7 +189,6 @@ start = true features = ["h753"] priority = 3 name = "drv-stm32h7-rng" -max-sizes = {flash = 8192, ram = 512} stacksize = 256 start = true task-slots = ["sys", "user_leds"] diff --git a/app/gemini-bu/app.toml b/app/gemini-bu/app.toml index b198602031..c223105eb2 100644 --- a/app/gemini-bu/app.toml +++ b/app/gemini-bu/app.toml @@ -156,7 +156,6 @@ start = true features = ["h753"] name = "drv-stm32h7-rng" priority = 6 -max-sizes = {flash = 8192, ram = 512} uses = ["rng"] start = true stacksize = 256 diff --git a/app/gimletlet/base-gimletlet2.toml b/app/gimletlet/base-gimletlet2.toml index 6775222986..98ae9139eb 100644 --- a/app/gimletlet/base-gimletlet2.toml +++ b/app/gimletlet/base-gimletlet2.toml @@ -105,7 +105,6 @@ start = true features = ["h753"] name = "drv-stm32h7-rng" priority = 6 -max-sizes = {flash = 8192, ram = 512} uses = ["rng"] start = true stacksize = 256 diff --git a/app/sidecar/base.toml b/app/sidecar/base.toml index 0e2c5f80df..a678b782f8 100644 --- a/app/sidecar/base.toml +++ b/app/sidecar/base.toml @@ -57,7 +57,6 @@ owner = {name = "sprot", notification = "rot_irq"} features = ["h753"] name = "drv-stm32h7-rng" priority = 6 -max-sizes = {flash = 8192, ram = 512} uses = ["rng"] start = true stacksize = 256 diff --git a/drv/stm32h7-rng/Cargo.toml b/drv/stm32h7-rng/Cargo.toml index 6d57920432..2105379a64 100644 --- a/drv/stm32h7-rng/Cargo.toml +++ b/drv/stm32h7-rng/Cargo.toml @@ -12,6 +12,7 @@ zerocopy = { workspace = true } drv-rng-api = { path = "../rng-api" } drv-stm32xx-sys-api = { path = "../stm32xx-sys-api" } userlib = { path = "../../sys/userlib", features = ["panic-messages"] } +ringbuf = { path = "../../lib/ringbuf" } [build-dependencies] idol = { workspace = true } diff --git a/drv/stm32h7-rng/src/main.rs b/drv/stm32h7-rng/src/main.rs index 074b4c862a..294852b348 100644 --- a/drv/stm32h7-rng/src/main.rs +++ b/drv/stm32h7-rng/src/main.rs @@ -23,10 +23,17 @@ use userlib::*; task_slot!(SYS, sys); +ringbuf::ringbuf!(Trace, 32, Trace::Blank); + +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +enum Trace { + Blank, + ClockError, + SeedError, +} + struct Stm32h7Rng { - cr: &'static device::rng::CR, - dr: &'static device::rng::DR, - sr: &'static device::rng::SR, + regs: &'static device::rng::RegisterBlock, sys: Sys, } @@ -34,62 +41,116 @@ impl Stm32h7Rng { fn new() -> Self { let registers = unsafe { &*device::RNG::ptr() }; Stm32h7Rng { - cr: ®isters.cr, - dr: ®isters.dr, - sr: ®isters.sr, + regs: registers, sys: Sys::from(SYS.get_task_id()), } } - fn init(&mut self) -> Result<(), RngError> { + fn init(&mut self) { + // Reset the RNG, so that if we restart, so does the hardware. + self.sys.enter_reset(Peripheral::Rng); + self.sys.leave_reset(Peripheral::Rng); + + // Turn on the RNG's clock and bus connection. self.sys.enable_clock(Peripheral::Rng); - self.enable_rng(); - if self.is_clock_error() { - let err = RngError::ClockError; - return Err(err); - } - Ok(()) + // Turn it on. This _starts_ the initialization process, which takes + // time on the order of microseconds. The process ends with setting + // either the DRDY bit or an ERROR bit in SR, which we'll notice when we + // attempt to pull data out of it. + self.regs.cr.modify(|_, w| w.rngen().set_bit()); } fn read(&mut self) -> Result { - let mut retries = 10; - while !self.is_data_ready() && retries > 0 { + for _retry in 0..10 { + // Sample our status register. + let sr = self.regs.sr.read(); + + // We do not expect clock errors. The clock configuration is static, + // and clock errors can only occur if the RNG kernel clock is + // waaaaay below the AHB clock, which we don't do. But, we're + // interested in finding out they _were happening_ if they happen, + // so: + if sr.ceis().bit_is_set() { + // TODO this should be an ereport + ringbuf::ringbuf_entry!(Trace::ClockError); + // Clear it so we don't repeat ourselves. The two writable bits + // in this register are write-one-to-preserve, so weirdly we + // want to write SEIS to 1 here. + self.regs.sr.modify(|_, w| { + w.seis().set_bit(); + w.ceis().clear_bit(); + w + }); + } + + // If an error occurred, return it so that our caller can attempt + // recovery. + if sr.seis().bit_is_set() { + // TODO this should be an ereport + ringbuf::ringbuf_entry!(Trace::SeedError); + // Clear it so we don't repeat ourselves. The two writable bits + // in this register are write-one-to-preserve, so weirdly we + // want to write CEIS to 1 here. + self.regs.sr.modify(|_, w| { + w.ceis().set_bit(); + w.seis().clear_bit(); + w + }); + return Err(RngError::SeedError); + } + // If data is ready, we can yield it. + if sr.drdy().bit_is_set() { + let data = self.regs.dr.read().rndata().bits(); + // There's a note in section 34.3.5 of the reference manual that + // reads: + // + // > When data is not ready (DRDY = 0) RNG_DR returns zero. It + // > is recommended to always verify that RNG_DR is different + // > from zero. Because when it is the case a seed error + // > occurred between RNG_SR polling and RND_DR output reading + // > (rare event). + // + // Why it's not sufficient to check SR.SEIS after reading DR, I + // couldn't tell you. But we'll implement their weird + // recommendation. + if data != 0 { + return Ok(data); + } + } + + // Otherwise, keep trying after waiting a tick. hl::sleep_for(1); - retries -= 1; - } - if !self.is_data_ready() { - return Err(RngError::NoData); } - if self.is_seed_error() { - return Err(RngError::SeedError); - } - Ok(self.dr.read().rndata().bits()) - } - fn enable_rng(&self) { - self.cr.modify(|_, w| w.rngen().set_bit()); + Err(RngError::NoData) } - fn is_clock_error_detect(&self) -> bool { - self.cr.read().ced().bits() - } - - fn is_clock_error(&self) -> bool { - // if clock error detection is disabled, CECS & CEIS won't be valid - if self.is_clock_error_detect() { - return self.sr.read().cecs().bits() - || self.sr.read().ceis().bits(); + fn attempt_recovery(&mut self) -> Result<(), RngError> { + // The recovery procedure is in section 34.3.7 of the manual. + // + // First, we clear the SEIS flag. The CEIS and SEIS flags are both + // rc_w0, meaning writing 1 has no effect, and 0 clears. So to clear + // SEIS, we have to _set_ CEIS too. + self.regs.sr.write(|w| { + w.seis().clear_bit(); + w.ceis().set_bit(); + w + }); + + // Next, we read 12 words to clear the pipeline. The number of words is + // 12. Not 11, not 10. + for _ in 0..12 { + let _ = self.regs.dr.read(); } - false - } - - fn is_data_ready(&self) -> bool { - self.sr.read().drdy().bits() - } - fn is_seed_error(&self) -> bool { - self.sr.read().secs().bits() || self.sr.read().seis().bits() + // Finally, we check whether SEIS is clear. + if self.regs.sr.read().seis().bit_is_clear() { + // Yay! + Ok(()) + } else { + Err(RngError::SeedError) + } } } @@ -109,23 +170,47 @@ impl idl::InOrderRngImpl for Stm32h7RngServer { _: &RecvMessage, dest: idol_runtime::Leased, ) -> Result> { + let len = dest.len(); let mut cnt = 0; - for _ in 0..(dest.len() / 4) { - let ent = self.rng.read()?; - dest.write_range(cnt..cnt + 4, &ent.to_ne_bytes()[0..4]) - .map_err(|_| RequestError::Fail(ClientError::WentAway))?; - cnt += 4; - } - let remain = dest.len() - cnt; - if remain > 4 { - panic!("RNG state machine bork"); - } - if remain > 0 { - let ent = self.rng.read()?; - dest.write_range(cnt..dest.len(), &ent.to_ne_bytes()[0..remain]) - .map_err(|_| RequestError::Fail(ClientError::WentAway))?; - cnt += remain; + // Keep track of whether we're making progress for recovery purposes. We + // start this at 'true' so that, if we detect a seed error at the start + // of this function, we'll try to recover at least once. + let mut made_progress = true; + + while cnt < len { + match self.rng.read() { + Ok(word) => { + // We have 32 shiny bits of data. Prepare to write that + // much, or less if we're at the tail, to the client. + let word_bytes = &word.to_ne_bytes(); + let chunk_size = usize::min(len - cnt, 4); + dest.write_range( + cnt..cnt + chunk_size, + &word_bytes[..chunk_size], + ) + .map_err(|_| RequestError::Fail(ClientError::WentAway))?; + cnt += chunk_size; + + // Note that we successfully did a thing, for recovery + // purposes. + made_progress = true; + } + Err(RngError::SeedError) if made_progress => { + // It may be worth attempting recovery. Recovery may not + // succeed, in which case this'll just bomb right out of + // here and send an error to the client. + self.rng.attempt_recovery()?; + + // Clear the progress flag so that if another seed error + // happens immediately, we'll exit. + made_progress = false; + } + Err(e) => { + // We have no way to recover from the other cases. + return Err(e.into()); + } + } } Ok(cnt) @@ -146,7 +231,7 @@ impl NotificationHandler for Stm32h7RngServer { #[export_name = "main"] fn main() -> ! { let mut rng = Stm32h7Rng::new(); - rng.init().expect("init failed"); + rng.init(); let mut srv = Stm32h7RngServer::new(rng); let mut buffer = [0u8; idl::INCOMING_SIZE];