From 01fe251f1f81549380782e3eb6d701edee73b083 Mon Sep 17 00:00:00 2001 From: Wouter Geraedts Date: Sun, 3 Jul 2022 20:14:42 +0200 Subject: [PATCH 1/8] Added two differing swap modes for pages --- src/boot/mod.rs | 144 +++++++++----------------------------------- src/lib.rs | 4 ++ src/manager/mod.rs | 4 +- src/state.rs | 27 ++++++--- src/swap/mod.rs | 25 ++++++++ src/swap/ram.rs | 119 ++++++++++++++++++++++++++++++++++++ src/swap/scratch.rs | 128 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 328 insertions(+), 123 deletions(-) create mode 100644 src/swap/mod.rs create mode 100644 src/swap/ram.rs create mode 100644 src/swap/scratch.rs diff --git a/src/boot/mod.rs b/src/boot/mod.rs index 3db4df8..e9cc090 100644 --- a/src/boot/mod.rs +++ b/src/boot/mod.rs @@ -1,8 +1,8 @@ use crate::{ hardware::processor::Processor, hardware::{Bank, Config}, - state::{ExchangeProgress, State, Update, UpdateError}, - Address, + state::{ExchangeProgress, ExchangeStep, State, Update, UpdateError}, + swap::{MemoryError, Swap}, }; use embedded_storage::Storage; @@ -12,37 +12,29 @@ use crate::log; #[cfg(feature = "defmt")] use defmt::Format; - -/// Error occured during nemory access -#[cfg_attr(feature = "use-defmt", derive(Format))] -#[derive(Debug)] -enum MemoryError { - BankSizeNotEqual, - BankSizeZero, - ReadFailure, - WriteFailure, -} - /// Use this from your bootloader application and call boot() to do the magic, reading the current /// state via the State type and then jumping to the new image using the Jumper specified pub struct MoonbootBoot< InternalMemory: Storage, HardwareState: State, CPU: Processor, // TODO: Wrap these into a context struct like rubble? + PageSwap: Swap, const INTERNAL_PAGE_SIZE: usize, > { config: Config, internal_memory: InternalMemory, state: HardwareState, processor: CPU, + swap: PageSwap, } impl< InternalMemory: Storage, HardwareState: State, CPU: Processor, + PageSwap: Swap, const INTERNAL_PAGE_SIZE: usize, - > MoonbootBoot + > MoonbootBoot { /// create a new instance of the bootloader pub fn new( @@ -50,12 +42,14 @@ impl< internal_memory: InternalMemory, state: HardwareState, processor: CPU, - ) -> MoonbootBoot { + swap: PageSwap, + ) -> MoonbootBoot { Self { config, internal_memory, state, processor, + swap, } } @@ -84,12 +78,10 @@ impl< Update::Error(err) => Update::Error(err), }; - // TODO: Handle Progress Variable in state to recover from power loss - log::info!("New State: {:?}", state); // Step 2: Update state of Bootloader - self.state.write(state)?; + self.state.write(&state)?; // Step 3: Jump to new or unchanged firmware self.jump_to_firmware(); @@ -131,8 +123,13 @@ impl< progress ); - let exchange_result = - self.exchange_banks_with_start(progress.a, progress.b, progress.page_index); + let exchange_result = self + .swap + .exchange::( + &mut self.internal_memory, + &mut self.state, + progress, + ); if exchange_result.is_ok() { let state = self.state.read().update; @@ -168,7 +165,7 @@ impl< old.location, old.size / 1024, new.location, - new.size / 1024 + new.size / 1024, ); // Try to exchange the firmware images @@ -198,101 +195,20 @@ impl< } fn exchange_banks(&mut self, a: Bank, b: Bank) -> Result<(), MemoryError> { - self.exchange_banks_with_start(a, b, 0) + self.swap + .exchange::( + &mut self.internal_memory, + &mut self.state, + ExchangeProgress { + a, + b, + page_index: 0, + recovering: false, + step: ExchangeStep::AToScratch, + }, + ) } - fn exchange_banks_with_start( - &mut self, - a: Bank, - b: Bank, - start_index: u32, - ) -> Result<(), MemoryError> { - // TODO: Sanity Check start_index - if a.size != b.size { - return Err(MemoryError::BankSizeNotEqual); - } - - if a.size == 0 || b.size == 0 { - return Err(MemoryError::BankSizeZero); - } - - let size = a.size; // Both are equal - - let full_pages = size / INTERNAL_PAGE_SIZE as Address; - let remaining_page_length = size as usize % INTERNAL_PAGE_SIZE; - - let mut page_a_buf = [0_u8; INTERNAL_PAGE_SIZE]; - let mut page_b_buf = [0_u8; INTERNAL_PAGE_SIZE]; - // can we reduce this to 1 buf and fancy operations? - // probably not with the read/write API. - // classic memory exchange problem :) - - // Set this in the exchanging part to know whether we are in a recovery process from a - // failed update or on the initial update - let recovering = matches!(self.state.read().update, Update::Revert(_)); - - // TODO: Fix - let a_location = a.location; - let b_location = b.location; - - for page_index in start_index..full_pages { - let offset = page_index * INTERNAL_PAGE_SIZE as u32; - log::trace!( - "Exchange: Page {}, from a ({}) to b ({})", - page_index, - a_location + offset, - b_location + offset - ); - self.internal_memory - .read(a_location + offset, &mut page_a_buf) - .map_err(|_| MemoryError::ReadFailure)?; - self.internal_memory - .read(b_location + offset, &mut page_b_buf) - .map_err(|_| MemoryError::ReadFailure)?; - self.internal_memory - .write(a_location + offset, &page_b_buf) - .map_err(|_| MemoryError::WriteFailure)?; - self.internal_memory - .write(b_location + offset, &page_a_buf) - .map_err(|_| MemoryError::WriteFailure)?; - - // Store the exchange progress - let mut state = self.state.read(); - state.update = Update::Exchanging(ExchangeProgress { - a, - b, - recovering, - page_index, - }); - // TODO: Ignore the error here? - let _ = self.state.write(state); - } - // TODO: Fit this into the while loop - if remaining_page_length > 0 { - let offset = full_pages * INTERNAL_PAGE_SIZE as u32; - - self.internal_memory - .read( - a.location + offset, - &mut page_a_buf[0..remaining_page_length], - ) - .map_err(|_| MemoryError::ReadFailure)?; - self.internal_memory - .read( - b.location + offset, - &mut page_b_buf[0..remaining_page_length], - ) - .map_err(|_| MemoryError::ReadFailure)?; - self.internal_memory - .write(a.location + offset, &page_a_buf[0..remaining_page_length]) - .map_err(|_| MemoryError::WriteFailure)?; - self.internal_memory - .write(b.location + offset, &page_b_buf[0..remaining_page_length]) - .map_err(|_| MemoryError::WriteFailure)?; - } - - Ok(()) - } // Jump to the firmware image marked as bootable fn jump_to_firmware(&mut self) -> ! { let app_exec_image = self.config.boot_bank; diff --git a/src/lib.rs b/src/lib.rs index 7ca2a85..145e998 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,10 +11,12 @@ //!* Automatic Linker Script generation based on a Section/Parition Description in Rust Code mod boot; + /// Implementations for use in the bootloader pub use boot::MoonbootBoot; mod manager; + /// Implementations for use in the firmware pub use manager::MoonbootManager; @@ -22,6 +24,8 @@ pub use manager::MoonbootManager; pub mod hardware; /// Shared state management between firmware and bootloader pub mod state; +/// Abstractions on the method of flash page swapping. +pub mod swap; pub use embedded_storage; diff --git a/src/manager/mod.rs b/src/manager/mod.rs index b6d514c..ad995b8 100644 --- a/src/manager/mod.rs +++ b/src/manager/mod.rs @@ -75,7 +75,7 @@ impl< log::trace!("New state: {:?}", current_state); - self.state.write(current_state) + self.state.write(¤t_state) } // Upgrade firmware verifiying the given signature over the size of size. @@ -107,7 +107,7 @@ impl< current_state.update = Update::Request(bank); - self.state.write(current_state)?; + self.state.write(¤t_state)?; log::info!("Stored update request, jumping to bootloader! Geronimo!"); diff --git a/src/state.rs b/src/state.rs index ae3cdae..01fed21 100644 --- a/src/state.rs +++ b/src/state.rs @@ -47,6 +47,20 @@ pub enum UpdateError { InvalidSignature, } +/// Upcoming operation of the exchange process for a given page index. +#[cfg_attr(feature = "use-defmt", derive(Format))] +#[cfg_attr(feature = "derive", derive(Serialize, Deserialize))] +#[cfg_attr(feature = "ram-state", derive(Desse, DesseSized))] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ExchangeStep { + /// Copy page A to the scratch page. + AToScratch, + /// Copy page B to page A. + BToA, + /// Copy the scratch page to page B. + ScratchToB, +} + /// Store the progress of the current exchange operation #[cfg_attr(feature = "use-defmt", derive(Format))] #[cfg_attr(feature = "derive", derive(Serialize, Deserialize))] @@ -59,6 +73,8 @@ pub struct ExchangeProgress { pub(crate) b: Bank, /// Page the operation has last copied pub(crate) page_index: u32, + /// Upcoming operation of the exchange process for a given page index. + pub(crate) step: ExchangeStep, /// Whether this exchange resulted from a Request (false) or a Revert (true) pub(crate) recovering: bool, } @@ -84,7 +100,7 @@ pub trait State { /// Read the shared state fn read(&mut self) -> MoonbootState; /// Write the new state to the shared state - fn write(&mut self, data: MoonbootState) -> Result<(), ()>; + fn write(&mut self, data: &MoonbootState) -> Result<(), ()>; } /// Size of the serialized state @@ -125,8 +141,7 @@ pub mod ram { let checksum = checksum(unsafe { &_moonboot_state_data_start }); if crc == checksum { - let data = - MoonbootState::deserialize_from(unsafe { &_moonboot_state_data_start }); + let data = MoonbootState::deserialize_from(unsafe { &_moonboot_state_data_start }); log::trace!("CRC Match! {}: {:?}", crc, data); return data; } else { @@ -138,13 +153,11 @@ pub mod ram { } } - fn write(&mut self, data: MoonbootState) -> Result<(), ()> { + fn write(&mut self, data: &MoonbootState) -> Result<(), ()> { log::trace!("Writing data {:?}", data); unsafe { _moonboot_state_data_start = data.serialize() }; - log::trace!("Written data: {:?}", unsafe { - &_moonboot_state_data_start - }); + log::trace!("Written data: {:?}", unsafe { &_moonboot_state_data_start }); unsafe { _moonboot_state_crc_start = checksum(&_moonboot_state_data_start); diff --git a/src/swap/mod.rs b/src/swap/mod.rs new file mode 100644 index 0000000..477c56f --- /dev/null +++ b/src/swap/mod.rs @@ -0,0 +1,25 @@ +pub mod ram; +pub mod scratch; + +use embedded_storage::Storage; + +use crate::state::{ExchangeProgress, State}; + +/// Error occured during memory access +#[cfg_attr(feature = "use-defmt", derive(Format))] +#[derive(Debug)] +pub enum MemoryError { + BankSizeNotEqual, + BankSizeZero, + ReadFailure, + WriteFailure, +} + +pub trait Swap { + fn exchange( + &mut self, + internal_memory: &mut InternalMemory, + state: &mut HardwareState, + exchange: ExchangeProgress, + ) -> Result<(), MemoryError>; +} diff --git a/src/swap/ram.rs b/src/swap/ram.rs new file mode 100644 index 0000000..2726291 --- /dev/null +++ b/src/swap/ram.rs @@ -0,0 +1,119 @@ +use embedded_storage::Storage; + +use crate::{ + log, + state::{ExchangeProgress, State, Update}, + swap::{MemoryError, Swap}, + Address, +}; + +pub struct Ram; + +impl Swap for Ram { + fn exchange( + &mut self, + internal_memory: &mut InternalMemory, + state: &mut HardwareState, + exchange: ExchangeProgress, + ) -> Result<(), MemoryError> { + let ExchangeProgress { + a, + b, + page_index, + step, + .. + } = exchange; + + // TODO: Sanity Check start_index + if a.size != b.size { + return Err(MemoryError::BankSizeNotEqual); + } + + if a.size == 0 || b.size == 0 { + return Err(MemoryError::BankSizeZero); + } + + let size = a.size; // Both are equal + + let full_pages = size / INTERNAL_PAGE_SIZE as Address; + let remaining_page_length = size as usize % INTERNAL_PAGE_SIZE; + + let mut page_a_buf = [0_u8; INTERNAL_PAGE_SIZE]; + let mut page_b_buf = [0_u8; INTERNAL_PAGE_SIZE]; + // can we reduce this to 1 buf and fancy operations? + // probably not with the read/write API. + // classic memory exchange problem :) + + let mut last_state = state.read(); + + // Set this in the exchanging part to know whether we are in a recovery process from a + // failed update or on the initial update + let recovering = matches!(last_state.update, Update::Revert(_)); + + // TODO: Fix + let a_location = a.location; + let b_location = b.location; + + for page_index in page_index..full_pages { + let offset = page_index * INTERNAL_PAGE_SIZE as u32; + log::trace!( + "Exchange: Page {}, from a ({}) to b ({})", + page_index, + a_location + offset, + b_location + offset + ); + + internal_memory + .read(a_location + offset, &mut page_a_buf) + .map_err(|_| MemoryError::ReadFailure)?; + internal_memory + .read(b_location + offset, &mut page_b_buf) + .map_err(|_| MemoryError::ReadFailure)?; + internal_memory + .write(a_location + offset, &page_b_buf) + .map_err(|_| MemoryError::WriteFailure)?; + internal_memory + .write(b_location + offset, &page_a_buf) + .map_err(|_| MemoryError::WriteFailure)?; + + // Store the exchange progress + + last_state.update = Update::Exchanging(ExchangeProgress { + a, + b, + recovering, + page_index, + step, + }); + + state + .write(&last_state) + .map_err(|_| MemoryError::WriteFailure)?; + } + // TODO: Fit this into the while loop + if remaining_page_length > 0 { + let offset = full_pages * INTERNAL_PAGE_SIZE as u32; + + internal_memory + .read( + a.location + offset, + &mut page_a_buf[0..remaining_page_length], + ) + .map_err(|_| MemoryError::ReadFailure)?; + internal_memory + .read( + b.location + offset, + &mut page_b_buf[0..remaining_page_length], + ) + .map_err(|_| MemoryError::ReadFailure)?; + internal_memory + .write(a.location + offset, &page_a_buf[0..remaining_page_length]) + .map_err(|_| MemoryError::WriteFailure)?; + internal_memory + .write(b.location + offset, &page_b_buf[0..remaining_page_length]) + .map_err(|_| MemoryError::WriteFailure)?; + } + + Ok(()) + } +} diff --git a/src/swap/scratch.rs b/src/swap/scratch.rs new file mode 100644 index 0000000..2e95785 --- /dev/null +++ b/src/swap/scratch.rs @@ -0,0 +1,128 @@ +use core::ops::Range; + +use embedded_storage::Storage; + +use crate::{ + log, + state::{ExchangeProgress, ExchangeStep, State, Update}, + swap::{MemoryError, Swap}, + Address, +}; + +pub struct Scratch<'a> { + pub pages: &'a [Range
], +} + +impl<'a> Swap for Scratch<'a> { + fn exchange( + &mut self, + internal_memory: &mut InternalMemory, + state: &mut HardwareState, + exchange: ExchangeProgress, + ) -> Result<(), MemoryError> { + let ExchangeProgress { + a, + b, + page_index, + mut step, + .. + } = exchange; + + // TODO: Sanity Check start_index + if a.size != b.size { + return Err(MemoryError::BankSizeNotEqual); + } + + if a.size == 0 || b.size == 0 { + return Err(MemoryError::BankSizeZero); + } + + let size = a.size; // Both are equal + + let full_pages = size / INTERNAL_PAGE_SIZE as Address; + let remaining_page_length = size as usize % INTERNAL_PAGE_SIZE; + + assert_eq!(remaining_page_length, 0); + + let mut ram_buf = [0_u8; INTERNAL_PAGE_SIZE]; + + let mut last_state = state.read(); + + // Set this in the exchanging part to know whether we are in a recovery process from a + // failed update or on the initial update + let recovering = matches!(last_state.update, Update::Revert(_)); + + let a_location = a.location; + let b_location = b.location; + + let mut first = true; + for page_index in page_index..full_pages { + let offset = page_index * INTERNAL_PAGE_SIZE as u32; + + let a_location = a_location + offset; + let b_location = b_location + offset; + + let scratch_index = page_index as usize % self.pages.len(); + let scratch_location = self.pages[scratch_index].start; + + log::trace!( + "Exchange: Page {}, from a ({}) to b ({}) using scratch ({})", + page_index, + a_location, + b_location, + scratch_location + ); + + loop { + if first { + // Do not write the state to flash, as it is already recent. + first = false; + } else { + last_state.update = Update::Exchanging(ExchangeProgress { + a, + b, + recovering, + page_index, + step, + }); + state + .write(&last_state) + .map_err(|_| MemoryError::WriteFailure)?; + } + + match step { + ExchangeStep::AToScratch => { + internal_memory + .read(a_location, &mut ram_buf) + .map_err(|_| MemoryError::ReadFailure)?; + internal_memory + .write(scratch_location, &ram_buf) + .map_err(|_| MemoryError::WriteFailure)?; + step = ExchangeStep::BToA; + } + ExchangeStep::BToA => { + internal_memory + .read(b_location, &mut ram_buf) + .map_err(|_| MemoryError::ReadFailure)?; + internal_memory + .write(a_location, &ram_buf) + .map_err(|_| MemoryError::WriteFailure)?; + step = ExchangeStep::ScratchToB; + } + ExchangeStep::ScratchToB => { + internal_memory + .read(scratch_location, &mut ram_buf) + .map_err(|_| MemoryError::ReadFailure)?; + internal_memory + .write(b_location, &ram_buf) + .map_err(|_| MemoryError::WriteFailure)?; + step = ExchangeStep::AToScratch; + break; + } + } + } + } + + Ok(()) + } +} From e2d1ce797ba85ab58d67308876ddf8653bd095eb Mon Sep 17 00:00:00 2001 From: Wouter Geraedts Date: Mon, 11 Jul 2022 17:05:02 +0200 Subject: [PATCH 2/8] Refactored swap/scratch to state --- src/boot/mod.rs | 61 +++++++++-------- src/hardware/processor.rs | 3 +- src/lib.rs | 2 - src/{state.rs => state/mod.rs} | 116 ++++++++++++++------------------- src/{swap => state}/ram.rs | 64 +++++++++++++++--- src/{swap => state}/scratch.rs | 0 src/swap/mod.rs | 25 ------- 7 files changed, 136 insertions(+), 135 deletions(-) rename src/{state.rs => state/mod.rs} (65%) rename src/{swap => state}/ram.rs (68%) rename src/{swap => state}/scratch.rs (100%) delete mode 100644 src/swap/mod.rs diff --git a/src/boot/mod.rs b/src/boot/mod.rs index e9cc090..b770311 100644 --- a/src/boot/mod.rs +++ b/src/boot/mod.rs @@ -1,8 +1,7 @@ use crate::{ hardware::processor::Processor, hardware::{Bank, Config}, - state::{ExchangeProgress, ExchangeStep, State, Update, UpdateError}, - swap::{MemoryError, Swap}, + state::{Exchange, ExchangeProgress, ExchangeStep, MemoryError, State, Update, UpdateError}, }; use embedded_storage::Storage; @@ -15,47 +14,47 @@ use defmt::Format; /// Use this from your bootloader application and call boot() to do the magic, reading the current /// state via the State type and then jumping to the new image using the Jumper specified pub struct MoonbootBoot< - InternalMemory: Storage, - HardwareState: State, - CPU: Processor, // TODO: Wrap these into a context struct like rubble? - PageSwap: Swap, + STORAGE: Storage, + STATE: State, + PROCESSOR: Processor, // TODO: Wrap these into a context struct like rubble? + EXCHANGE: Exchange, const INTERNAL_PAGE_SIZE: usize, > { config: Config, - internal_memory: InternalMemory, - state: HardwareState, - processor: CPU, - swap: PageSwap, + storage: STORAGE, + state: STATE, + processor: PROCESSOR, + exchange: EXCHANGE, } impl< - InternalMemory: Storage, - HardwareState: State, - CPU: Processor, - PageSwap: Swap, + STORAGE: Storage, + STATE: State, + PROCESSOR: Processor, + EXCHANGE: Exchange, const INTERNAL_PAGE_SIZE: usize, - > MoonbootBoot + > MoonbootBoot { /// create a new instance of the bootloader pub fn new( config: Config, - internal_memory: InternalMemory, - state: HardwareState, - processor: CPU, - swap: PageSwap, - ) -> MoonbootBoot { + storage: STORAGE, + state: STATE, + processor: PROCESSOR, + exchange: EXCHANGE, + ) -> MoonbootBoot { Self { config, - internal_memory, + storage, state, processor, - swap, + exchange, } } /// Destroy this instance of the bootloader and return access to the hardware peripheral - pub fn destroy(self) -> (InternalMemory, HardwareState, CPU) { - (self.internal_memory, self.state, self.processor) + pub fn destroy(self) -> (STORAGE, STATE, PROCESSOR) { + (self.storage, self.state, self.processor) } /// Execute the update and boot logic of the bootloader @@ -124,9 +123,9 @@ impl< ); let exchange_result = self - .swap - .exchange::( - &mut self.internal_memory, + .exchange + .exchange::( + &mut self.storage, &mut self.state, progress, ); @@ -165,7 +164,7 @@ impl< old.location, old.size / 1024, new.location, - new.size / 1024, + new.size / 1024 ); // Try to exchange the firmware images @@ -195,9 +194,9 @@ impl< } fn exchange_banks(&mut self, a: Bank, b: Bank) -> Result<(), MemoryError> { - self.swap - .exchange::( - &mut self.internal_memory, + self.exchange + .exchange::( + &mut self.storage, &mut self.state, ExchangeProgress { a, diff --git a/src/hardware/processor.rs b/src/hardware/processor.rs index b447c80..33be20a 100644 --- a/src/hardware/processor.rs +++ b/src/hardware/processor.rs @@ -29,7 +29,7 @@ pub mod cortex_m { fn do_jump(&mut self, address: super::Address) -> ! { unsafe { // Set Vector Table to new vector table (unsafe but okay here) - (*cortex_m::peripheral::SCB::ptr()).vtor.write(address); + (*cortex_m::peripheral::SCB::PTR).vtor.write(address); cortex_m::asm::bootload(address as *const u32); } @@ -40,4 +40,3 @@ pub mod cortex_m { } } } - diff --git a/src/lib.rs b/src/lib.rs index 145e998..759729c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,8 +24,6 @@ pub use manager::MoonbootManager; pub mod hardware; /// Shared state management between firmware and bootloader pub mod state; -/// Abstractions on the method of flash page swapping. -pub mod swap; pub use embedded_storage; diff --git a/src/state.rs b/src/state/mod.rs similarity index 65% rename from src/state.rs rename to src/state/mod.rs index 01fed21..95f50a9 100644 --- a/src/state.rs +++ b/src/state/mod.rs @@ -1,10 +1,37 @@ +#[cfg(feature = "ram-state")] +pub mod ram; + +#[cfg(feature = "scratch-state")] +pub mod scratch; + +use embedded_storage::Storage; + +/// Error occured during memory access +#[cfg_attr(feature = "use-defmt", derive(Format))] +#[derive(Debug)] +pub enum MemoryError { + BankSizeNotEqual, + BankSizeZero, + ReadFailure, + WriteFailure, +} + +/// Abstraction for the exchange operation of the current state. +pub trait Exchange { + fn exchange( + &mut self, + internal_memory: &mut STORAGE, + state: &mut STATE, + progress: ExchangeProgress, + ) -> Result<(), MemoryError>; +} + use crate::hardware::Bank; -use crate::log; use crc::{Crc, CRC_32_CKSUM}; #[cfg(feature = "defmt")] use defmt::Format; -#[cfg(feature = "ram-state")] +#[cfg(any(feature = "ram-state", feature = "scratch-state"))] use desse::{Desse, DesseSized}; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; @@ -14,7 +41,10 @@ use serde::{Deserialize, Serialize}; // exchanged via software updates easily #[cfg_attr(feature = "use-defmt", derive(Format))] #[cfg_attr(feature = "derive", derive(Serialize, Deserialize))] -#[cfg_attr(feature = "ram-state", derive(Desse, DesseSized))] +#[cfg_attr( + any(feature = "ram-state", feature = "scratch-state"), + derive(Desse, DesseSized) +)] #[derive(Debug, PartialEq, Eq)] pub enum Update { // No update requested, just jump to the application @@ -33,7 +63,10 @@ pub enum Update { #[cfg_attr(feature = "use-defmt", derive(Format))] #[cfg_attr(feature = "derive", derive(Serialize, Deserialize))] -#[cfg_attr(feature = "ram-state", derive(Desse, DesseSized))] +#[cfg_attr( + any(feature = "ram-state", feature = "scratch-state"), + derive(Desse, DesseSized) +)] #[derive(Debug, Clone, Copy, PartialEq, Eq)] /// Errors that can occur during update pub enum UpdateError { @@ -50,7 +83,10 @@ pub enum UpdateError { /// Upcoming operation of the exchange process for a given page index. #[cfg_attr(feature = "use-defmt", derive(Format))] #[cfg_attr(feature = "derive", derive(Serialize, Deserialize))] -#[cfg_attr(feature = "ram-state", derive(Desse, DesseSized))] +#[cfg_attr( + any(feature = "ram-state", feature = "scratch-state"), + derive(Desse, DesseSized) +)] #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ExchangeStep { /// Copy page A to the scratch page. @@ -64,7 +100,10 @@ pub enum ExchangeStep { /// Store the progress of the current exchange operation #[cfg_attr(feature = "use-defmt", derive(Format))] #[cfg_attr(feature = "derive", derive(Serialize, Deserialize))] -#[cfg_attr(feature = "ram-state", derive(Desse, DesseSized))] +#[cfg_attr( + any(feature = "ram-state", feature = "scratch-state"), + derive(Desse, DesseSized) +)] #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct ExchangeProgress { /// Bank the update is coming from @@ -82,7 +121,10 @@ pub struct ExchangeProgress { /// Struct used to store the state of the bootloader situation in NVM #[cfg_attr(feature = "use-defmt", derive(Format))] #[cfg_attr(feature = "derive", derive(Serialize, Deserialize))] -#[cfg_attr(feature = "ram-state", derive(Desse, DesseSized))] +#[cfg_attr( + any(feature = "ram-state", feature = "scratch-state"), + derive(Desse, DesseSized) +)] #[derive(Debug)] pub struct MoonbootState { /// If set Request, an Update is requested. This will exchange the two images, set the update @@ -112,63 +154,3 @@ const CRC: Crc = Crc::::new(&CRC_32_CKSUM); fn checksum(bytes: &[u8]) -> StateCrcType { CRC.checksum(bytes) } - -/// State stored in the RAM -/// TODO: Move to hardware folder together with state trait? -#[cfg(feature = "ram-state")] -pub mod ram { - use super::*; - - /// State read and written to RAM. This assumes the device is never powered off / the ram is never - /// reset! - pub struct RamState; - - extern "C" { - static mut _moonboot_state_crc_start: StateCrcType; - static mut _moonboot_state_data_start: [u8; STATE_SERIALIZED_MAX_SIZE]; - // TODO: Move these as normal variables to linker sections via #[link] macro? - } - - impl State for RamState { - fn read(&mut self) -> MoonbootState { - let crc = unsafe { _moonboot_state_crc_start }; - - log::info!( - "Reading data with len: {}, CRC: {}", - STATE_SERIALIZED_MAX_SIZE, - crc - ); - - let checksum = checksum(unsafe { &_moonboot_state_data_start }); - if crc == checksum { - let data = MoonbootState::deserialize_from(unsafe { &_moonboot_state_data_start }); - log::trace!("CRC Match! {}: {:?}", crc, data); - return data; - } else { - log::trace!("CRC Mismatch! {} vs {}", crc, checksum); - } - - MoonbootState { - update: Update::None, - } - } - - fn write(&mut self, data: &MoonbootState) -> Result<(), ()> { - log::trace!("Writing data {:?}", data); - - unsafe { _moonboot_state_data_start = data.serialize() }; - log::trace!("Written data: {:?}", unsafe { &_moonboot_state_data_start }); - - unsafe { - _moonboot_state_crc_start = checksum(&_moonboot_state_data_start); - } - log::info!( - "Written len: {}, checksum: {}", - STATE_SERIALIZED_MAX_SIZE, - unsafe { _moonboot_state_crc_start } - ); - - Ok(()) - } - } -} diff --git a/src/swap/ram.rs b/src/state/ram.rs similarity index 68% rename from src/swap/ram.rs rename to src/state/ram.rs index 2726291..1c11505 100644 --- a/src/swap/ram.rs +++ b/src/state/ram.rs @@ -1,15 +1,63 @@ use embedded_storage::Storage; -use crate::{ - log, - state::{ExchangeProgress, State, Update}, - swap::{MemoryError, Swap}, - Address, -}; +use crate::{log, Address}; -pub struct Ram; +use super::*; -impl Swap for Ram { +/// State read and written to RAM. This assumes the device is never powered off / the ram is never +/// reset! +pub struct RamState; + +extern "C" { + static mut _moonboot_state_crc_start: StateCrcType; + static mut _moonboot_state_data_start: [u8; STATE_SERIALIZED_MAX_SIZE]; + // TODO: Move these as normal variables to linker sections via #[link] macro? +} + +impl State for RamState { + fn read(&mut self) -> MoonbootState { + let crc = unsafe { _moonboot_state_crc_start }; + + log::info!( + "Reading data with len: {}, CRC: {}", + STATE_SERIALIZED_MAX_SIZE, + crc + ); + + let checksum = checksum(unsafe { &_moonboot_state_data_start }); + if crc == checksum { + let data = MoonbootState::deserialize_from(unsafe { &_moonboot_state_data_start }); + log::trace!("CRC Match! {}: {:?}", crc, data); + return data; + } else { + log::trace!("CRC Mismatch! {} vs {}", crc, checksum); + } + + MoonbootState { + update: Update::None, + } + } + + fn write(&mut self, data: &MoonbootState) -> Result<(), ()> { + log::trace!("Writing data {:?}", data); + + unsafe { _moonboot_state_data_start = data.serialize() }; + log::trace!("Written data: {:?}", unsafe { &_moonboot_state_data_start }); + + unsafe { + _moonboot_state_crc_start = checksum(&_moonboot_state_data_start); + } + log::info!( + "Written len: {}, checksum: {}", + STATE_SERIALIZED_MAX_SIZE, + unsafe { _moonboot_state_crc_start } + ); + + Ok(()) + } +} + +impl Exchange for RamState { fn exchange( &mut self, internal_memory: &mut InternalMemory, diff --git a/src/swap/scratch.rs b/src/state/scratch.rs similarity index 100% rename from src/swap/scratch.rs rename to src/state/scratch.rs diff --git a/src/swap/mod.rs b/src/swap/mod.rs deleted file mode 100644 index 477c56f..0000000 --- a/src/swap/mod.rs +++ /dev/null @@ -1,25 +0,0 @@ -pub mod ram; -pub mod scratch; - -use embedded_storage::Storage; - -use crate::state::{ExchangeProgress, State}; - -/// Error occured during memory access -#[cfg_attr(feature = "use-defmt", derive(Format))] -#[derive(Debug)] -pub enum MemoryError { - BankSizeNotEqual, - BankSizeZero, - ReadFailure, - WriteFailure, -} - -pub trait Swap { - fn exchange( - &mut self, - internal_memory: &mut InternalMemory, - state: &mut HardwareState, - exchange: ExchangeProgress, - ) -> Result<(), MemoryError>; -} From 1cd3c4f5be825b43926c7849d9fd514f3dcec6f0 Mon Sep 17 00:00:00 2001 From: Wouter Geraedts Date: Mon, 11 Jul 2022 18:50:55 +0200 Subject: [PATCH 3/8] Reworked errors --- src/boot/mod.rs | 22 +++-- src/manager/mod.rs | 220 +++++++++++++++++++++++---------------------- src/state/mod.rs | 22 ++--- src/state/ram.rs | 113 ++++++++++------------- 4 files changed, 183 insertions(+), 194 deletions(-) diff --git a/src/boot/mod.rs b/src/boot/mod.rs index b770311..7481b1c 100644 --- a/src/boot/mod.rs +++ b/src/boot/mod.rs @@ -1,7 +1,7 @@ use crate::{ hardware::processor::Processor, hardware::{Bank, Config}, - state::{Exchange, ExchangeProgress, ExchangeStep, MemoryError, State, Update, UpdateError}, + state::{Exchange, ExchangeError, ExchangeProgress, ExchangeStep, State, Update, UpdateError}, }; use embedded_storage::Storage; @@ -58,13 +58,13 @@ impl< } /// Execute the update and boot logic of the bootloader - pub fn boot(&mut self) -> Result { + pub fn boot(&mut self) -> Result { // TODO: consider error handling log::info!("Booting with moonboot!"); self.processor.setup(&self.config); - let mut state = self.state.read(); + let mut state = self.state.read()?; log::info!("Old State: {:?}", state); @@ -73,7 +73,7 @@ impl< Update::None => self.handle_none(), Update::Request(bank) => self.handle_request(bank), Update::Revert(bank) => self.handle_revert(bank), - Update::Exchanging(progress) => self.handle_exchanging(progress), + Update::Exchanging(progress) => self.handle_exchanging(progress)?, Update::Error(err) => Update::Error(err), }; @@ -116,7 +116,7 @@ impl< // Handle a case of power interruption or similar, which lead to a exchange_banks being // interrupted. - fn handle_exchanging(&mut self, progress: ExchangeProgress) -> Update { + fn handle_exchanging(&mut self, progress: ExchangeProgress) -> Result { log::error!( "Firmware Update was interrupted! Trying to recover with exchange operation: {:?}", progress @@ -130,8 +130,8 @@ impl< progress, ); - if exchange_result.is_ok() { - let state = self.state.read().update; + Ok(if exchange_result.is_ok() { + let state = self.state.read()?.update; match state { Update::Exchanging(progress) => { if progress.recovering { @@ -148,7 +148,7 @@ impl< exchange_result ); Update::Error(UpdateError::ImageExchangeFailed) - } + }) } // Revert the bootable image with the image in index new_firmware. Returns Revert on success if @@ -193,7 +193,11 @@ impl< } } - fn exchange_banks(&mut self, a: Bank, b: Bank) -> Result<(), MemoryError> { + fn exchange_banks( + &mut self, + a: Bank, + b: Bank, + ) -> Result<(), ExchangeError> { self.exchange .exchange::( &mut self.storage, diff --git a/src/manager/mod.rs b/src/manager/mod.rs index ad995b8..4a7b193 100644 --- a/src/manager/mod.rs +++ b/src/manager/mod.rs @@ -3,55 +3,67 @@ use crate::{ state::{State, Update}, }; -use embedded_storage::{ReadStorage, Storage}; +use embedded_storage::Storage; use crate::log; /// Instantiate this in your application to enable mutation of the State specified in this and jump /// to the bootloader to apply any updates. pub struct MoonbootManager< - InternalMemory: Storage, - HardwareState: State, - CPU: Processor, + STORAGE: Storage, + STATE: State, + PROCESSOR: Processor, const INTERNAL_PAGE_SIZE: usize, > { config: Config, - internal_memory: InternalMemory, - state: HardwareState, - processor: CPU, + storage: STORAGE, + state: STATE, + processor: PROCESSOR, } -impl< - InternalMemory: Storage, - HardwareState: State, - CPU: Processor, - const INTERNAL_PAGE_SIZE: usize, - > MoonbootManager +pub struct InitError; + +pub enum MarkError { + UpdateQueuedButNotInstalled, + State(E), +} + +impl + MoonbootManager { pub fn new( config: Config, - internal_memory: InternalMemory, - state: HardwareState, - processor: CPU, - ) -> MoonbootManager { - Self { + storage: STORAGE, + state: STATE, + processor: PROCESSOR, + ) -> Result, InitError> { + if config.update_bank.size > config.boot_bank.size { + log::error!( + "Requested update bank {:?} is larger than boot bank {:?}", + bank, + self.config.boot_bank + ); + return Err(InitError); + } + + Ok(Self { config, - internal_memory, + storage, state, processor, - } + }) } /// Destroy this instance of the boot manager and return access to the hardware peripheral - pub fn destroy(self) -> (InternalMemory, HardwareState, CPU) { - (self.internal_memory, self.state, self.processor) + pub fn destroy(self) -> (STORAGE, STATE, PROCESSOR) { + (self.storage, self.state, self.processor) } /// Run this immediately after booting your new image successfully to mark the boot as /// succesful. If you do not do this, any reset will cause the bootloader to restore to the /// previous firmware image. - pub fn mark_boot_successful(&mut self) -> Result<(), ()> { - let mut current_state = self.state.read(); + pub fn mark_boot_successful(&mut self) -> Result<(), MarkError> { + let mut current_state = self.state.read().map_err(MarkError::State)?; log::info!( "Application running, marking boot as successful. Current state: {:?}", @@ -69,34 +81,24 @@ impl< } _ => { log::error!("There is an update queued, but it has not been installed yet. Did you skip the bootloader?"); - return Err(()); + return Err(MarkError::UpdateQueuedButNotInstalled); } }; log::trace!("New state: {:?}", current_state); - self.state.write(¤t_state) + self.state.write(¤t_state).map_err(MarkError::State) } // Upgrade firmware verifiying the given signature over the size of size. // Can only return an error or diverge (!, represented by Void while ! is not a type yet) - pub fn update(&mut self) -> Result { + pub fn update(&mut self) -> Result { // Apply the update stored in the update bank let bank = self.config.update_bank; - // TODO: Check size value! log::info!("Update requested on slot {:?}", bank); - if bank.size > self.config.boot_bank.size { - log::error!( - "Requested update bank {:?} is larger than boot bank {:?}", - bank, - self.config.boot_bank - ); - return Err(()); - } - - let mut current_state = self.state.read(); + let mut current_state = self.state.read()?; if current_state.update != Update::None { log::warn!( @@ -125,78 +127,78 @@ impl< } } -/// Easily get read access to the update bank -impl< - InternalMemory: Storage, - HardwareState: State, - CPU: Processor, - const INTERNAL_PAGE_SIZE: usize, - > core::convert::AsRef<[u8]> - for MoonbootManager -{ - #[inline] - fn as_ref(&self) -> &[u8] { - unsafe { - core::slice::from_raw_parts( - self.config.update_bank.location as *const u8, - self.config.update_bank.size as usize, - ) - } - } -} +// /// Easily get read access to the update bank +// impl< +// InternalMemory: Storage, +// HardwareState: State, +// PROCESSOR: Processor, +// const INTERNAL_PAGE_SIZE: usize, +// > core::convert::AsRef<[u8]> +// for MoonbootManager +// { +// #[inline] +// fn as_ref(&self) -> &[u8] { +// unsafe { +// core::slice::from_raw_parts( +// self.config.update_bank.location as *const u8, +// self.config.update_bank.size as usize, +// ) +// } +// } +// } -/// Read Access to the current update target slot -impl< - InternalMemory: Storage, - HardwareState: State, - CPU: Processor, - const INTERNAL_PAGE_SIZE: usize, - > ReadStorage for MoonbootManager -{ - type Error = (); // TODO +// /// Read Access to the current update target slot +// impl< +// InternalMemory: Storage, +// HardwareState: State, +// CPU: Processor, +// const INTERNAL_PAGE_SIZE: usize, +// > ReadStorage for MoonbootManager +// { +// type Error = (); // TODO - fn read(&mut self, offset: u32, bytes: &mut [u8]) -> Result<(), Self::Error> { - let bank = self.config.update_bank; // For now we always write updates to this bank. - if offset > bank.size || offset + bytes.len() as u32 > bank.size { - Err(()) // TODO: We want better error types! - } else { - // TODO! fix - let bank_start = bank.location; - log::info!("Writing at {:x}[{:x}]", bank_start, offset); - match bank.memory_unit { - crate::hardware::MemoryUnit::Internal => { - { self.internal_memory.read(bank_start + offset, bytes) }.map_err(|_| ()) - } - } - } - } +// fn read(&mut self, offset: u32, bytes: &mut [u8]) -> Result<(), Self::Error> { +// let bank = self.config.update_bank; // For now we always write updates to this bank. +// if offset > bank.size || offset + bytes.len() as u32 > bank.size { +// Err(()) // TODO: We want better error types! +// } else { +// // TODO! fix +// let bank_start = bank.location; +// log::info!("Writing at {:x}[{:x}]", bank_start, offset); +// match bank.memory_unit { +// crate::hardware::MemoryUnit::Internal => { +// { self.storage.read(bank_start + offset, bytes) }.map_err(|_| ()) +// } +// } +// } +// } - fn capacity(&self) -> usize { - self.config.update_bank.size as usize - } -} +// fn capacity(&self) -> usize { +// self.config.update_bank.size as usize +// } +// } -/// Write Access to the current update target slot -impl< - InternalMemory: Storage, - HardwareState: State, - CPU: Processor, - const INTERNAL_PAGE_SIZE: usize, - > Storage for MoonbootManager -{ - fn write(&mut self, offset: u32, bytes: &[u8]) -> Result<(), Self::Error> { - let bank = self.config.update_bank; // For now we always write updates to this bank. - if offset > bank.size || offset + bytes.len() as u32 > bank.size { - Err(()) // TODO: We want better error types! - } else { - // TODO! fix - let bank_start = bank.location; - log::info!("Writing at {:x}[{:x}]", bank_start, offset); - match bank.memory_unit { - crate::hardware::MemoryUnit::Internal => { - { self.internal_memory.write(bank_start + offset, bytes) }.map_err(|_| ()) - } - } - } - } -} +// /// Write Access to the current update target slot +// impl< +// InternalMemory: Storage, +// HardwareState: State, +// CPU: Processor, +// const INTERNAL_PAGE_SIZE: usize, +// > Storage for MoonbootManager +// { +// fn write(&mut self, offset: u32, bytes: &[u8]) -> Result<(), Self::Error> { +// let bank = self.config.update_bank; // For now we always write updates to this bank. +// if offset > bank.size || offset + bytes.len() as u32 > bank.size { +// Err(()) // TODO: We want better error types! +// } else { +// // TODO! fix +// let bank_start = bank.location; +// log::info!("Writing at {:x}[{:x}]", bank_start, offset); +// match bank.memory_unit { +// crate::hardware::MemoryUnit::Internal => { +// { self.internal_memory.write(bank_start + offset, bytes) }.map_err(|_| ()) +// } +// } +// } +// } +// } diff --git a/src/state/mod.rs b/src/state/mod.rs index 95f50a9..c7c9fb9 100644 --- a/src/state/mod.rs +++ b/src/state/mod.rs @@ -6,24 +6,22 @@ pub mod scratch; use embedded_storage::Storage; -/// Error occured during memory access -#[cfg_attr(feature = "use-defmt", derive(Format))] -#[derive(Debug)] -pub enum MemoryError { - BankSizeNotEqual, - BankSizeZero, - ReadFailure, - WriteFailure, +pub enum ExchangeError { + Storage(STORAGE), + State(STATE), + Other(OTHER), } /// Abstraction for the exchange operation of the current state. pub trait Exchange { + type OtherError; + fn exchange( &mut self, internal_memory: &mut STORAGE, state: &mut STATE, progress: ExchangeProgress, - ) -> Result<(), MemoryError>; + ) -> Result<(), ExchangeError>; } use crate::hardware::Bank; @@ -139,10 +137,12 @@ pub struct MoonbootState { /// RAM. As long as you don't want to perform update download, power cycle the device, and then /// apply the update, storing it in volatile memory is fine. pub trait State { + type Error; + /// Read the shared state - fn read(&mut self) -> MoonbootState; + fn read(&mut self) -> Result; /// Write the new state to the shared state - fn write(&mut self, data: &MoonbootState) -> Result<(), ()>; + fn write(&mut self, data: &MoonbootState) -> Result<(), Self::Error>; } /// Size of the serialized state diff --git a/src/state/ram.rs b/src/state/ram.rs index 1c11505..db45a71 100644 --- a/src/state/ram.rs +++ b/src/state/ram.rs @@ -15,7 +15,9 @@ extern "C" { } impl State for RamState { - fn read(&mut self) -> MoonbootState { + type Error = void::Void; + + fn read(&mut self) -> Result { let crc = unsafe { _moonboot_state_crc_start }; log::info!( @@ -28,17 +30,16 @@ impl State for RamState { if crc == checksum { let data = MoonbootState::deserialize_from(unsafe { &_moonboot_state_data_start }); log::trace!("CRC Match! {}: {:?}", crc, data); - return data; + Ok(data) } else { log::trace!("CRC Mismatch! {} vs {}", crc, checksum); - } - - MoonbootState { - update: Update::None, + Ok(MoonbootState { + update: Update::None, + }) } } - fn write(&mut self, data: &MoonbootState) -> Result<(), ()> { + fn write(&mut self, data: &MoonbootState) -> Result<(), Self::Error> { log::trace!("Writing data {:?}", data); unsafe { _moonboot_state_data_start = data.serialize() }; @@ -58,28 +59,21 @@ impl State for RamState { } impl Exchange for RamState { - fn exchange( + type OtherError = void::Void; + + fn exchange( &mut self, - internal_memory: &mut InternalMemory, - state: &mut HardwareState, - exchange: ExchangeProgress, - ) -> Result<(), MemoryError> { + storage: &mut STORAGE, + state: &mut STATE, + progress: ExchangeProgress, + ) -> Result<(), ExchangeError> { let ExchangeProgress { a, b, page_index, step, .. - } = exchange; - - // TODO: Sanity Check start_index - if a.size != b.size { - return Err(MemoryError::BankSizeNotEqual); - } - - if a.size == 0 || b.size == 0 { - return Err(MemoryError::BankSizeZero); - } + } = progress; let size = a.size; // Both are equal @@ -88,41 +82,36 @@ impl Exchange for RamState { let mut page_a_buf = [0_u8; INTERNAL_PAGE_SIZE]; let mut page_b_buf = [0_u8; INTERNAL_PAGE_SIZE]; - // can we reduce this to 1 buf and fancy operations? - // probably not with the read/write API. - // classic memory exchange problem :) - let mut last_state = state.read(); + let mut last_state = state.read().map_err(ExchangeError::State)?; // Set this in the exchanging part to know whether we are in a recovery process from a // failed update or on the initial update let recovering = matches!(last_state.update, Update::Revert(_)); - // TODO: Fix - let a_location = a.location; - let b_location = b.location; - for page_index in page_index..full_pages { let offset = page_index * INTERNAL_PAGE_SIZE as u32; + let a_location = a.location + offset; + let b_location = b.location + offset; log::trace!( "Exchange: Page {}, from a ({}) to b ({})", page_index, - a_location + offset, - b_location + offset + a_location, + b_location ); - internal_memory - .read(a_location + offset, &mut page_a_buf) - .map_err(|_| MemoryError::ReadFailure)?; - internal_memory - .read(b_location + offset, &mut page_b_buf) - .map_err(|_| MemoryError::ReadFailure)?; - internal_memory - .write(a_location + offset, &page_b_buf) - .map_err(|_| MemoryError::WriteFailure)?; - internal_memory - .write(b_location + offset, &page_a_buf) - .map_err(|_| MemoryError::WriteFailure)?; + storage + .read(a_location, &mut page_a_buf) + .map_err(ExchangeError::Storage)?; + storage + .read(b_location, &mut page_b_buf) + .map_err(ExchangeError::Storage)?; + storage + .write(a_location, &page_b_buf) + .map_err(ExchangeError::Storage)?; + storage + .write(b_location, &page_a_buf) + .map_err(ExchangeError::Storage)?; // Store the exchange progress @@ -134,32 +123,26 @@ impl Exchange for RamState { step, }); - state - .write(&last_state) - .map_err(|_| MemoryError::WriteFailure)?; + state.write(&last_state).map_err(ExchangeError::State)?; } // TODO: Fit this into the while loop if remaining_page_length > 0 { let offset = full_pages * INTERNAL_PAGE_SIZE as u32; + let a_location = a.location + offset; + let b_location = b.location + offset; - internal_memory - .read( - a.location + offset, - &mut page_a_buf[0..remaining_page_length], - ) - .map_err(|_| MemoryError::ReadFailure)?; - internal_memory - .read( - b.location + offset, - &mut page_b_buf[0..remaining_page_length], - ) - .map_err(|_| MemoryError::ReadFailure)?; - internal_memory - .write(a.location + offset, &page_a_buf[0..remaining_page_length]) - .map_err(|_| MemoryError::WriteFailure)?; - internal_memory - .write(b.location + offset, &page_b_buf[0..remaining_page_length]) - .map_err(|_| MemoryError::WriteFailure)?; + storage + .read(a_location, &mut page_a_buf[0..remaining_page_length]) + .map_err(ExchangeError::Storage)?; + storage + .read(b_location, &mut page_b_buf[0..remaining_page_length]) + .map_err(ExchangeError::Storage)?; + storage + .write(a_location, &page_a_buf[0..remaining_page_length]) + .map_err(ExchangeError::Storage)?; + storage + .write(b_location + offset, &page_b_buf[0..remaining_page_length]) + .map_err(ExchangeError::Storage)?; } Ok(()) From 3d7a4b7cfb7cbc26a4cb1207288497bec1d418d8 Mon Sep 17 00:00:00 2001 From: Wouter Geraedts Date: Mon, 11 Jul 2022 19:46:45 +0200 Subject: [PATCH 4/8] Use Context trait --- src/boot/mod.rs | 92 +++++++++++++++++--------------------------- src/lib.rs | 7 ++++ src/manager/mod.rs | 45 +++++++++++----------- src/state/mod.rs | 35 ++++++++--------- src/state/ram.rs | 34 ++++++++++++++-- src/state/scratch.rs | 84 ++++++++++++++++++++++++---------------- 6 files changed, 160 insertions(+), 137 deletions(-) diff --git a/src/boot/mod.rs b/src/boot/mod.rs index 7481b1c..aad1abb 100644 --- a/src/boot/mod.rs +++ b/src/boot/mod.rs @@ -1,11 +1,10 @@ use crate::{ hardware::processor::Processor, hardware::{Bank, Config}, - state::{Exchange, ExchangeError, ExchangeProgress, ExchangeStep, State, Update, UpdateError}, + state::{Exchange, ExchangeProgress, ExchangeStep, State, Update, UpdateError}, + Context, }; -use embedded_storage::Storage; - use crate::log; #[cfg(feature = "defmt")] @@ -13,36 +12,23 @@ use defmt::Format; /// Use this from your bootloader application and call boot() to do the magic, reading the current /// state via the State type and then jumping to the new image using the Jumper specified -pub struct MoonbootBoot< - STORAGE: Storage, - STATE: State, - PROCESSOR: Processor, // TODO: Wrap these into a context struct like rubble? - EXCHANGE: Exchange, - const INTERNAL_PAGE_SIZE: usize, -> { +pub struct MoonbootBoot { config: Config, - storage: STORAGE, - state: STATE, - processor: PROCESSOR, - exchange: EXCHANGE, + storage: CONTEXT::Storage, + state: CONTEXT::State, + processor: CONTEXT::Processor, + exchange: CONTEXT::Exchange, } -impl< - STORAGE: Storage, - STATE: State, - PROCESSOR: Processor, - EXCHANGE: Exchange, - const INTERNAL_PAGE_SIZE: usize, - > MoonbootBoot -{ +impl MoonbootBoot { /// create a new instance of the bootloader pub fn new( config: Config, - storage: STORAGE, - state: STATE, - processor: PROCESSOR, - exchange: EXCHANGE, - ) -> MoonbootBoot { + storage: CONTEXT::Storage, + state: CONTEXT::State, + processor: CONTEXT::Processor, + exchange: CONTEXT::Exchange, + ) -> Self { Self { config, storage, @@ -53,12 +39,12 @@ impl< } /// Destroy this instance of the bootloader and return access to the hardware peripheral - pub fn destroy(self) -> (STORAGE, STATE, PROCESSOR) { + pub fn destroy(self) -> (CONTEXT::Storage, CONTEXT::State, CONTEXT::Processor) { (self.storage, self.state, self.processor) } /// Execute the update and boot logic of the bootloader - pub fn boot(&mut self) -> Result { + pub fn boot(&mut self) -> Result::Error> { // TODO: consider error handling log::info!("Booting with moonboot!"); @@ -116,19 +102,20 @@ impl< // Handle a case of power interruption or similar, which lead to a exchange_banks being // interrupted. - fn handle_exchanging(&mut self, progress: ExchangeProgress) -> Result { + fn handle_exchanging( + &mut self, + progress: ExchangeProgress, + ) -> Result::Error> { log::error!( "Firmware Update was interrupted! Trying to recover with exchange operation: {:?}", progress ); - let exchange_result = self - .exchange - .exchange::( - &mut self.storage, - &mut self.state, - progress, - ); + let exchange_result = self.exchange.exchange::( + &mut self.storage, + &mut self.state, + progress, + ); Ok(if exchange_result.is_ok() { let state = self.state.read()?.update; @@ -168,7 +155,17 @@ impl< ); // Try to exchange the firmware images - let exchange_result = self.exchange_banks(new, old); + let exchange_result = self.exchange.exchange::( + &mut self.storage, + &mut self.state, + ExchangeProgress { + a: new, + b: old, + page_index: 0, + recovering: false, + step: ExchangeStep::AToScratch, + }, + ); if exchange_result.is_ok() { if with_failsafe_revert { // Update Firmware Update State to revert. The Application will set this to @@ -193,25 +190,6 @@ impl< } } - fn exchange_banks( - &mut self, - a: Bank, - b: Bank, - ) -> Result<(), ExchangeError> { - self.exchange - .exchange::( - &mut self.storage, - &mut self.state, - ExchangeProgress { - a, - b, - page_index: 0, - recovering: false, - step: ExchangeStep::AToScratch, - }, - ) - } - // Jump to the firmware image marked as bootable fn jump_to_firmware(&mut self) -> ! { let app_exec_image = self.config.boot_bank; diff --git a/src/lib.rs b/src/lib.rs index 759729c..37942b6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -45,6 +45,13 @@ pub(crate) use defmt as log; #[cfg(feature = "use-log")] pub(crate) use logger_crate as log; +pub trait Context { + type Storage: embedded_storage::Storage; + type State: state::State; + type Processor: hardware::processor::Processor; + type Exchange: state::Exchange; +} + #[cfg(not(any(feature = "use-log", feature = "use-defmt")))] pub(crate) mod log { macro_rules! info { diff --git a/src/manager/mod.rs b/src/manager/mod.rs index 4a7b193..daf05ac 100644 --- a/src/manager/mod.rs +++ b/src/manager/mod.rs @@ -1,24 +1,18 @@ use crate::{ hardware::{processor::Processor, Config}, state::{State, Update}, + Context, }; -use embedded_storage::Storage; - use crate::log; /// Instantiate this in your application to enable mutation of the State specified in this and jump /// to the bootloader to apply any updates. -pub struct MoonbootManager< - STORAGE: Storage, - STATE: State, - PROCESSOR: Processor, - const INTERNAL_PAGE_SIZE: usize, -> { +pub struct MoonbootManager { config: Config, - storage: STORAGE, - state: STATE, - processor: PROCESSOR, + storage: CONTEXT::Storage, + state: CONTEXT::State, + processor: CONTEXT::Processor, } pub struct InitError; @@ -28,24 +22,29 @@ pub enum MarkError { State(E), } -impl - MoonbootManager +impl + MoonbootManager { pub fn new( config: Config, - storage: STORAGE, - state: STATE, - processor: PROCESSOR, - ) -> Result, InitError> { + storage: CONTEXT::Storage, + state: CONTEXT::State, + processor: CONTEXT::Processor, + ) -> Result { if config.update_bank.size > config.boot_bank.size { log::error!( "Requested update bank {:?} is larger than boot bank {:?}", - bank, - self.config.boot_bank + config.update_bank, + config.boot_bank ); return Err(InitError); } + if config.update_bank.size == 0 || config.boot_bank.size == 0 { + log::error!("Requested banks are of zero size"); + return Err(InitError); + } + Ok(Self { config, storage, @@ -55,14 +54,16 @@ impl (STORAGE, STATE, PROCESSOR) { + pub fn destroy(self) -> (CONTEXT::Storage, CONTEXT::State, CONTEXT::Processor) { (self.storage, self.state, self.processor) } /// Run this immediately after booting your new image successfully to mark the boot as /// succesful. If you do not do this, any reset will cause the bootloader to restore to the /// previous firmware image. - pub fn mark_boot_successful(&mut self) -> Result<(), MarkError> { + pub fn mark_boot_successful( + &mut self, + ) -> Result<(), MarkError<::Error>> { let mut current_state = self.state.read().map_err(MarkError::State)?; log::info!( @@ -92,7 +93,7 @@ impl Result { + pub fn update(&mut self) -> Result::Error> { // Apply the update stored in the update bank let bank = self.config.update_bank; diff --git a/src/state/mod.rs b/src/state/mod.rs index c7c9fb9..bed0480 100644 --- a/src/state/mod.rs +++ b/src/state/mod.rs @@ -4,28 +4,11 @@ pub mod ram; #[cfg(feature = "scratch-state")] pub mod scratch; -use embedded_storage::Storage; - -pub enum ExchangeError { - Storage(STORAGE), - State(STATE), - Other(OTHER), -} - -/// Abstraction for the exchange operation of the current state. -pub trait Exchange { - type OtherError; - - fn exchange( - &mut self, - internal_memory: &mut STORAGE, - state: &mut STATE, - progress: ExchangeProgress, - ) -> Result<(), ExchangeError>; -} - use crate::hardware::Bank; +use core::fmt::Debug; +use embedded_storage::Storage; + use crc::{Crc, CRC_32_CKSUM}; #[cfg(feature = "defmt")] use defmt::Format; @@ -34,6 +17,18 @@ use desse::{Desse, DesseSized}; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; +/// Abstraction for the exchange operation of the current state. +pub trait Exchange { + type Error: Debug; + + fn exchange( + &mut self, + storage: &mut STORAGE, + state: &mut STATE, + progress: ExchangeProgress, + ) -> Result<(), Self::Error>; +} + /// Decision making states for the bootloader // TODO: Hash + Signature? Should be done on download I think! This way, the algorithms can be // exchanged via software updates easily diff --git a/src/state/ram.rs b/src/state/ram.rs index db45a71..dd61c3a 100644 --- a/src/state/ram.rs +++ b/src/state/ram.rs @@ -58,15 +58,37 @@ impl State for RamState { } } -impl Exchange for RamState { - type OtherError = void::Void; +pub enum ExchangeError { + Storage(STORAGE), + State(STATE), +} - fn exchange( +impl Debug for ExchangeError +where + STORAGE: Debug, + STATE: Debug, +{ + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::Storage(arg0) => f.debug_tuple("Storage").field(arg0).finish(), + Self::State(arg0) => f.debug_tuple("State").field(arg0).finish(), + } + } +} + +impl Exchange for RamState +where + STORAGE::Error: Debug, + STATE::Error: Debug, +{ + type Error = ExchangeError; + + fn exchange( &mut self, storage: &mut STORAGE, state: &mut STATE, progress: ExchangeProgress, - ) -> Result<(), ExchangeError> { + ) -> Result<(), Self::Error> { let ExchangeProgress { a, b, @@ -75,6 +97,10 @@ impl Exchange for RamState { .. } = progress; + assert_eq!(a.size, b.size); + assert_ne!(a.size, 0); + assert_ne!(b.size, 0); + let size = a.size; // Both are equal let full_pages = size / INTERNAL_PAGE_SIZE as Address; diff --git a/src/state/scratch.rs b/src/state/scratch.rs index 2e95785..e8eddec 100644 --- a/src/state/scratch.rs +++ b/src/state/scratch.rs @@ -1,11 +1,10 @@ -use core::ops::Range; +use core::{fmt::Debug, ops::Range}; use embedded_storage::Storage; use crate::{ log, - state::{ExchangeProgress, ExchangeStep, State, Update}, - swap::{MemoryError, Swap}, + state::{Exchange, ExchangeProgress, ExchangeStep, State, Update}, Address, }; @@ -13,29 +12,48 @@ pub struct Scratch<'a> { pub pages: &'a [Range
], } -impl<'a> Swap for Scratch<'a> { - fn exchange( +pub enum ExchangeError { + Storage(STORAGE), + State(STATE), +} + +impl Debug for ExchangeError +where + STORAGE: Debug, + STATE: Debug, +{ + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::Storage(arg0) => f.debug_tuple("Storage").field(arg0).finish(), + Self::State(arg0) => f.debug_tuple("State").field(arg0).finish(), + } + } +} + +impl<'a, STORAGE: Storage, STATE: State> Exchange for Scratch<'a> +where + STORAGE::Error: Debug, + STATE::Error: Debug, +{ + type Error = ExchangeError; + + fn exchange( &mut self, - internal_memory: &mut InternalMemory, - state: &mut HardwareState, - exchange: ExchangeProgress, - ) -> Result<(), MemoryError> { + storage: &mut STORAGE, + state: &mut STATE, + progress: ExchangeProgress, + ) -> Result<(), Self::Error> { let ExchangeProgress { a, b, page_index, mut step, .. - } = exchange; + } = progress; - // TODO: Sanity Check start_index - if a.size != b.size { - return Err(MemoryError::BankSizeNotEqual); - } - - if a.size == 0 || b.size == 0 { - return Err(MemoryError::BankSizeZero); - } + assert_eq!(a.size, b.size); + assert_ne!(a.size, 0); + assert_ne!(b.size, 0); let size = a.size; // Both are equal @@ -46,7 +64,7 @@ impl<'a> Swap for Scratch<'a> { let mut ram_buf = [0_u8; INTERNAL_PAGE_SIZE]; - let mut last_state = state.read(); + let mut last_state = state.read().map_err(ExchangeError::State)?; // Set this in the exchanging part to know whether we are in a recovery process from a // failed update or on the initial update @@ -85,37 +103,35 @@ impl<'a> Swap for Scratch<'a> { page_index, step, }); - state - .write(&last_state) - .map_err(|_| MemoryError::WriteFailure)?; + state.write(&last_state).map_err(ExchangeError::State)?; } match step { ExchangeStep::AToScratch => { - internal_memory + storage .read(a_location, &mut ram_buf) - .map_err(|_| MemoryError::ReadFailure)?; - internal_memory + .map_err(ExchangeError::Storage)?; + storage .write(scratch_location, &ram_buf) - .map_err(|_| MemoryError::WriteFailure)?; + .map_err(ExchangeError::Storage)?; step = ExchangeStep::BToA; } ExchangeStep::BToA => { - internal_memory + storage .read(b_location, &mut ram_buf) - .map_err(|_| MemoryError::ReadFailure)?; - internal_memory + .map_err(ExchangeError::Storage)?; + storage .write(a_location, &ram_buf) - .map_err(|_| MemoryError::WriteFailure)?; + .map_err(ExchangeError::Storage)?; step = ExchangeStep::ScratchToB; } ExchangeStep::ScratchToB => { - internal_memory + storage .read(scratch_location, &mut ram_buf) - .map_err(|_| MemoryError::ReadFailure)?; - internal_memory + .map_err(ExchangeError::Storage)?; + storage .write(b_location, &ram_buf) - .map_err(|_| MemoryError::WriteFailure)?; + .map_err(ExchangeError::Storage)?; step = ExchangeStep::AToScratch; break; } From 34e6326482719d228551153318a48788884b8dba Mon Sep 17 00:00:00 2001 From: Wouter Geraedts Date: Mon, 11 Jul 2022 20:10:40 +0200 Subject: [PATCH 5/8] Removed convenience storage and deref access to MoonbootManager --- src/manager/mod.rs | 76 ---------------------------------------------- 1 file changed, 76 deletions(-) diff --git a/src/manager/mod.rs b/src/manager/mod.rs index daf05ac..4f4498d 100644 --- a/src/manager/mod.rs +++ b/src/manager/mod.rs @@ -127,79 +127,3 @@ impl self.processor.do_jump(bootloader_address) } } - -// /// Easily get read access to the update bank -// impl< -// InternalMemory: Storage, -// HardwareState: State, -// PROCESSOR: Processor, -// const INTERNAL_PAGE_SIZE: usize, -// > core::convert::AsRef<[u8]> -// for MoonbootManager -// { -// #[inline] -// fn as_ref(&self) -> &[u8] { -// unsafe { -// core::slice::from_raw_parts( -// self.config.update_bank.location as *const u8, -// self.config.update_bank.size as usize, -// ) -// } -// } -// } - -// /// Read Access to the current update target slot -// impl< -// InternalMemory: Storage, -// HardwareState: State, -// CPU: Processor, -// const INTERNAL_PAGE_SIZE: usize, -// > ReadStorage for MoonbootManager -// { -// type Error = (); // TODO - -// fn read(&mut self, offset: u32, bytes: &mut [u8]) -> Result<(), Self::Error> { -// let bank = self.config.update_bank; // For now we always write updates to this bank. -// if offset > bank.size || offset + bytes.len() as u32 > bank.size { -// Err(()) // TODO: We want better error types! -// } else { -// // TODO! fix -// let bank_start = bank.location; -// log::info!("Writing at {:x}[{:x}]", bank_start, offset); -// match bank.memory_unit { -// crate::hardware::MemoryUnit::Internal => { -// { self.storage.read(bank_start + offset, bytes) }.map_err(|_| ()) -// } -// } -// } -// } - -// fn capacity(&self) -> usize { -// self.config.update_bank.size as usize -// } -// } - -// /// Write Access to the current update target slot -// impl< -// InternalMemory: Storage, -// HardwareState: State, -// CPU: Processor, -// const INTERNAL_PAGE_SIZE: usize, -// > Storage for MoonbootManager -// { -// fn write(&mut self, offset: u32, bytes: &[u8]) -> Result<(), Self::Error> { -// let bank = self.config.update_bank; // For now we always write updates to this bank. -// if offset > bank.size || offset + bytes.len() as u32 > bank.size { -// Err(()) // TODO: We want better error types! -// } else { -// // TODO! fix -// let bank_start = bank.location; -// log::info!("Writing at {:x}[{:x}]", bank_start, offset); -// match bank.memory_unit { -// crate::hardware::MemoryUnit::Internal => { -// { self.internal_memory.write(bank_start + offset, bytes) }.map_err(|_| ()) -// } -// } -// } -// } -// } From 1e1c174f32b3487062c3c35cd6a1c1d6e1002cf3 Mon Sep 17 00:00:00 2001 From: Wouter Geraedts Date: Tue, 12 Jul 2022 08:57:14 +0200 Subject: [PATCH 6/8] Moved exchange back into separate module --- src/boot/mod.rs | 3 +- src/exchange/mod.rs | 18 ++++ src/exchange/ram.rs | 127 +++++++++++++++++++++++++++++ src/{state => exchange}/scratch.rs | 3 +- src/lib.rs | 4 +- src/state/mod.rs | 16 ---- src/state/ram.rs | 121 +-------------------------- 7 files changed, 153 insertions(+), 139 deletions(-) create mode 100644 src/exchange/mod.rs create mode 100644 src/exchange/ram.rs rename src/{state => exchange}/scratch.rs (98%) diff --git a/src/boot/mod.rs b/src/boot/mod.rs index aad1abb..7d09796 100644 --- a/src/boot/mod.rs +++ b/src/boot/mod.rs @@ -1,7 +1,8 @@ use crate::{ + exchange::Exchange, hardware::processor::Processor, hardware::{Bank, Config}, - state::{Exchange, ExchangeProgress, ExchangeStep, State, Update, UpdateError}, + state::{ExchangeProgress, ExchangeStep, State, Update, UpdateError}, Context, }; diff --git a/src/exchange/mod.rs b/src/exchange/mod.rs new file mode 100644 index 0000000..6f7592e --- /dev/null +++ b/src/exchange/mod.rs @@ -0,0 +1,18 @@ +pub mod ram; + +pub mod scratch; + +use crate::state::{ExchangeProgress, State}; +use embedded_storage::Storage; + +/// Abstraction for the exchange operation of the current state. +pub trait Exchange { + type Error: core::fmt::Debug; + + fn exchange( + &mut self, + storage: &mut STORAGE, + state: &mut STATE, + progress: ExchangeProgress, + ) -> Result<(), Self::Error>; +} diff --git a/src/exchange/ram.rs b/src/exchange/ram.rs new file mode 100644 index 0000000..9be9f9a --- /dev/null +++ b/src/exchange/ram.rs @@ -0,0 +1,127 @@ +use embedded_storage::Storage; + +use crate::{ + exchange::Exchange, + log, + state::{ExchangeProgress, State, Update}, + Address, +}; + +pub struct Ram; + +pub enum ExchangeError { + Storage(STORAGE), + State(STATE), +} + +impl core::fmt::Debug for ExchangeError +where + STORAGE: core::fmt::Debug, + STATE: core::fmt::Debug, +{ + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::Storage(arg0) => f.debug_tuple("Storage").field(arg0).finish(), + Self::State(arg0) => f.debug_tuple("State").field(arg0).finish(), + } + } +} + +impl Exchange for Ram +where + STORAGE::Error: core::fmt::Debug, + STATE::Error: core::fmt::Debug, +{ + type Error = ExchangeError; + + fn exchange( + &mut self, + storage: &mut STORAGE, + state: &mut STATE, + progress: ExchangeProgress, + ) -> Result<(), Self::Error> { + let ExchangeProgress { + a, + b, + page_index, + step, + .. + } = progress; + + assert_eq!(a.size, b.size); + assert_ne!(a.size, 0); + assert_ne!(b.size, 0); + + let size = a.size; // Both are equal + + let full_pages = size / INTERNAL_PAGE_SIZE as Address; + let remaining_page_length = size as usize % INTERNAL_PAGE_SIZE; + + let mut page_a_buf = [0_u8; INTERNAL_PAGE_SIZE]; + let mut page_b_buf = [0_u8; INTERNAL_PAGE_SIZE]; + + let mut last_state = state.read().map_err(ExchangeError::State)?; + + // Set this in the exchanging part to know whether we are in a recovery process from a + // failed update or on the initial update + let recovering = matches!(last_state.update, Update::Revert(_)); + + for page_index in page_index..full_pages { + let offset = page_index * INTERNAL_PAGE_SIZE as u32; + let a_location = a.location + offset; + let b_location = b.location + offset; + log::trace!( + "Exchange: Page {}, from a ({}) to b ({})", + page_index, + a_location, + b_location + ); + + storage + .read(a_location, &mut page_a_buf) + .map_err(ExchangeError::Storage)?; + storage + .read(b_location, &mut page_b_buf) + .map_err(ExchangeError::Storage)?; + storage + .write(a_location, &page_b_buf) + .map_err(ExchangeError::Storage)?; + storage + .write(b_location, &page_a_buf) + .map_err(ExchangeError::Storage)?; + + // Store the exchange progress + + last_state.update = Update::Exchanging(ExchangeProgress { + a, + b, + recovering, + page_index, + step, + }); + + state.write(&last_state).map_err(ExchangeError::State)?; + } + // TODO: Fit this into the while loop + if remaining_page_length > 0 { + let offset = full_pages * INTERNAL_PAGE_SIZE as u32; + let a_location = a.location + offset; + let b_location = b.location + offset; + + storage + .read(a_location, &mut page_a_buf[0..remaining_page_length]) + .map_err(ExchangeError::Storage)?; + storage + .read(b_location, &mut page_b_buf[0..remaining_page_length]) + .map_err(ExchangeError::Storage)?; + storage + .write(a_location, &page_a_buf[0..remaining_page_length]) + .map_err(ExchangeError::Storage)?; + storage + .write(b_location + offset, &page_b_buf[0..remaining_page_length]) + .map_err(ExchangeError::Storage)?; + } + + Ok(()) + } +} diff --git a/src/state/scratch.rs b/src/exchange/scratch.rs similarity index 98% rename from src/state/scratch.rs rename to src/exchange/scratch.rs index e8eddec..67e4e4b 100644 --- a/src/state/scratch.rs +++ b/src/exchange/scratch.rs @@ -3,8 +3,9 @@ use core::{fmt::Debug, ops::Range}; use embedded_storage::Storage; use crate::{ + exchange::Exchange, log, - state::{Exchange, ExchangeProgress, ExchangeStep, State, Update}, + state::{ExchangeProgress, ExchangeStep, State, Update}, Address, }; diff --git a/src/lib.rs b/src/lib.rs index 37942b6..d11600c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,6 +20,8 @@ mod manager; /// Implementations for use in the firmware pub use manager::MoonbootManager; +/// Various processes for exchanging pages +pub mod exchange; /// Common hardware abstractions and associated implementations pub mod hardware; /// Shared state management between firmware and bootloader @@ -49,7 +51,7 @@ pub trait Context { type Storage: embedded_storage::Storage; type State: state::State; type Processor: hardware::processor::Processor; - type Exchange: state::Exchange; + type Exchange: exchange::Exchange; } #[cfg(not(any(feature = "use-log", feature = "use-defmt")))] diff --git a/src/state/mod.rs b/src/state/mod.rs index bed0480..18501c6 100644 --- a/src/state/mod.rs +++ b/src/state/mod.rs @@ -1,13 +1,9 @@ #[cfg(feature = "ram-state")] pub mod ram; -#[cfg(feature = "scratch-state")] -pub mod scratch; - use crate::hardware::Bank; use core::fmt::Debug; -use embedded_storage::Storage; use crc::{Crc, CRC_32_CKSUM}; #[cfg(feature = "defmt")] @@ -17,18 +13,6 @@ use desse::{Desse, DesseSized}; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; -/// Abstraction for the exchange operation of the current state. -pub trait Exchange { - type Error: Debug; - - fn exchange( - &mut self, - storage: &mut STORAGE, - state: &mut STATE, - progress: ExchangeProgress, - ) -> Result<(), Self::Error>; -} - /// Decision making states for the bootloader // TODO: Hash + Signature? Should be done on download I think! This way, the algorithms can be // exchanged via software updates easily diff --git a/src/state/ram.rs b/src/state/ram.rs index dd61c3a..4601edf 100644 --- a/src/state/ram.rs +++ b/src/state/ram.rs @@ -1,6 +1,4 @@ -use embedded_storage::Storage; - -use crate::{log, Address}; +use crate::log; use super::*; @@ -57,120 +55,3 @@ impl State for RamState { Ok(()) } } - -pub enum ExchangeError { - Storage(STORAGE), - State(STATE), -} - -impl Debug for ExchangeError -where - STORAGE: Debug, - STATE: Debug, -{ - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - match self { - Self::Storage(arg0) => f.debug_tuple("Storage").field(arg0).finish(), - Self::State(arg0) => f.debug_tuple("State").field(arg0).finish(), - } - } -} - -impl Exchange for RamState -where - STORAGE::Error: Debug, - STATE::Error: Debug, -{ - type Error = ExchangeError; - - fn exchange( - &mut self, - storage: &mut STORAGE, - state: &mut STATE, - progress: ExchangeProgress, - ) -> Result<(), Self::Error> { - let ExchangeProgress { - a, - b, - page_index, - step, - .. - } = progress; - - assert_eq!(a.size, b.size); - assert_ne!(a.size, 0); - assert_ne!(b.size, 0); - - let size = a.size; // Both are equal - - let full_pages = size / INTERNAL_PAGE_SIZE as Address; - let remaining_page_length = size as usize % INTERNAL_PAGE_SIZE; - - let mut page_a_buf = [0_u8; INTERNAL_PAGE_SIZE]; - let mut page_b_buf = [0_u8; INTERNAL_PAGE_SIZE]; - - let mut last_state = state.read().map_err(ExchangeError::State)?; - - // Set this in the exchanging part to know whether we are in a recovery process from a - // failed update or on the initial update - let recovering = matches!(last_state.update, Update::Revert(_)); - - for page_index in page_index..full_pages { - let offset = page_index * INTERNAL_PAGE_SIZE as u32; - let a_location = a.location + offset; - let b_location = b.location + offset; - log::trace!( - "Exchange: Page {}, from a ({}) to b ({})", - page_index, - a_location, - b_location - ); - - storage - .read(a_location, &mut page_a_buf) - .map_err(ExchangeError::Storage)?; - storage - .read(b_location, &mut page_b_buf) - .map_err(ExchangeError::Storage)?; - storage - .write(a_location, &page_b_buf) - .map_err(ExchangeError::Storage)?; - storage - .write(b_location, &page_a_buf) - .map_err(ExchangeError::Storage)?; - - // Store the exchange progress - - last_state.update = Update::Exchanging(ExchangeProgress { - a, - b, - recovering, - page_index, - step, - }); - - state.write(&last_state).map_err(ExchangeError::State)?; - } - // TODO: Fit this into the while loop - if remaining_page_length > 0 { - let offset = full_pages * INTERNAL_PAGE_SIZE as u32; - let a_location = a.location + offset; - let b_location = b.location + offset; - - storage - .read(a_location, &mut page_a_buf[0..remaining_page_length]) - .map_err(ExchangeError::Storage)?; - storage - .read(b_location, &mut page_b_buf[0..remaining_page_length]) - .map_err(ExchangeError::Storage)?; - storage - .write(a_location, &page_a_buf[0..remaining_page_length]) - .map_err(ExchangeError::Storage)?; - storage - .write(b_location + offset, &page_b_buf[0..remaining_page_length]) - .map_err(ExchangeError::Storage)?; - } - - Ok(()) - } -} From 40f93525b70a85c9fb1caf6f0532ed41e518e298 Mon Sep 17 00:00:00 2001 From: Wouter Geraedts Date: Tue, 12 Jul 2022 08:57:51 +0200 Subject: [PATCH 7/8] Upgraded to defmt 0.3 --- Cargo.lock | 29 +++++++++++++++-------------- Cargo.toml | 4 ++-- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1953f84..5047c94 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -53,6 +53,12 @@ version = "0.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "46afbd2983a5d5a7bd740ccb198caf5b82f45c40c09c0eed36052d91cb92e719" +[[package]] +name = "bitflags" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" + [[package]] name = "byteorder" version = "1.4.3" @@ -106,21 +112,22 @@ dependencies = [ [[package]] name = "defmt" -version = "0.2.3" +version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "15fe96f5d208164afa70583ff8f062e7697cbbb0b98e5076fbf8ac6da9edff0f" +checksum = "d3a0ae7494d9bff013d7b89471f4c424356a71e9752e0c78abe7e6c608a16bb3" dependencies = [ + "bitflags", "defmt-macros", - "semver 1.0.7", ] [[package]] name = "defmt-macros" -version = "0.2.3" +version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9bd2c3949cb76c25f48c363e61b97f05b317efe3c12fa45d54a6599c3949c85e" +checksum = "6d944432e281084511691b36e5e9c794c19c33675822c9019e3b64f5b89e10da" dependencies = [ "defmt-parser", + "proc-macro-error", "proc-macro2 1.0.37", "quote 1.0.18", "syn 1.0.91", @@ -128,9 +135,9 @@ dependencies = [ [[package]] name = "defmt-parser" -version = "0.2.2" +version = "0.3.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bc621c2b4f5f5635e34021c38af2ccb0c1dae38ba11ebee25258de8bb1cee9fe" +checksum = "0db23d29972d99baa3de2ee2ae3f104c10564a6d05a346eb3f4c4f2c0525a06e" [[package]] name = "desse" @@ -372,7 +379,7 @@ version = "0.2.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a" dependencies = [ - "semver 0.9.0", + "semver", ] [[package]] @@ -390,12 +397,6 @@ dependencies = [ "semver-parser", ] -[[package]] -name = "semver" -version = "1.0.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d65bd28f48be7196d222d95b9243287f48d27aca604e08497513019ff0502cc4" - [[package]] name = "semver-parser" version = "0.7.0" diff --git a/Cargo.toml b/Cargo.toml index a7d98f2..4ec6e59 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ moonboot-macros = { path = "./macros", version = "0.1.2" } heapless = {version = "0.7", features = ["serde"] } serde = { version = "1.0", features = ["derive"], default-features = false, optional = true } cortex-m = { version = "0.7", optional = true } -defmt = { version = "0.2", optional = true } +defmt = { version = "0.3", optional = true } logger-crate = { version = "0.4", optional = true, package = "log" } crc = "2.0" desse = { version = "0.2.1", optional = true } @@ -38,7 +38,7 @@ defmt-warn = [] defmt-error = [] [package.metadata.release] -enable-features = ["ram-state", "ram-state", "cortex-m"] +enable-features = ["ram-state", "cortex-m"] shared-version = true dependent-version = "upgrade" pre-release-replacements = [ From a301aaa9ea1e6d641f2b8029301ad5f211fcc002 Mon Sep 17 00:00:00 2001 From: Wouter Geraedts Date: Tue, 12 Jul 2022 10:35:49 +0200 Subject: [PATCH 8/8] Change scratch to use a bank defined in config --- src/boot/mod.rs | 2 ++ src/exchange/mod.rs | 6 +++++- src/exchange/ram.rs | 2 ++ src/exchange/scratch.rs | 23 ++++++++++++++++------- src/hardware/mod.rs | 2 ++ src/hardware/processor.rs | 2 +- src/manager/mod.rs | 6 +++++- src/state/mod.rs | 2 +- 8 files changed, 34 insertions(+), 11 deletions(-) diff --git a/src/boot/mod.rs b/src/boot/mod.rs index 7d09796..927716b 100644 --- a/src/boot/mod.rs +++ b/src/boot/mod.rs @@ -113,6 +113,7 @@ impl MoonbootBoot( + &self.config, &mut self.storage, &mut self.state, progress, @@ -157,6 +158,7 @@ impl MoonbootBoot( + &self.config, &mut self.storage, &mut self.state, ExchangeProgress { diff --git a/src/exchange/mod.rs b/src/exchange/mod.rs index 6f7592e..416e1e1 100644 --- a/src/exchange/mod.rs +++ b/src/exchange/mod.rs @@ -2,7 +2,10 @@ pub mod ram; pub mod scratch; -use crate::state::{ExchangeProgress, State}; +use crate::{ + hardware::Config, + state::{ExchangeProgress, State}, +}; use embedded_storage::Storage; /// Abstraction for the exchange operation of the current state. @@ -11,6 +14,7 @@ pub trait Exchange { fn exchange( &mut self, + config: &Config, storage: &mut STORAGE, state: &mut STATE, progress: ExchangeProgress, diff --git a/src/exchange/ram.rs b/src/exchange/ram.rs index 9be9f9a..984ad3f 100644 --- a/src/exchange/ram.rs +++ b/src/exchange/ram.rs @@ -2,6 +2,7 @@ use embedded_storage::Storage; use crate::{ exchange::Exchange, + hardware::Config, log, state::{ExchangeProgress, State, Update}, Address, @@ -36,6 +37,7 @@ where fn exchange( &mut self, + _config: &Config, storage: &mut STORAGE, state: &mut STATE, progress: ExchangeProgress, diff --git a/src/exchange/scratch.rs b/src/exchange/scratch.rs index 67e4e4b..2d1f908 100644 --- a/src/exchange/scratch.rs +++ b/src/exchange/scratch.rs @@ -1,21 +1,21 @@ -use core::{fmt::Debug, ops::Range}; +use core::fmt::Debug; use embedded_storage::Storage; use crate::{ exchange::Exchange, + hardware::Config, log, state::{ExchangeProgress, ExchangeStep, State, Update}, Address, }; -pub struct Scratch<'a> { - pub pages: &'a [Range
], -} +pub struct Scratch; pub enum ExchangeError { Storage(STORAGE), State(STATE), + ScratchInsufficient, } impl Debug for ExchangeError @@ -27,11 +27,12 @@ where match self { Self::Storage(arg0) => f.debug_tuple("Storage").field(arg0).finish(), Self::State(arg0) => f.debug_tuple("State").field(arg0).finish(), + Self::ScratchInsufficient => f.write_str("ScratchInsufficient"), } } } -impl<'a, STORAGE: Storage, STATE: State> Exchange for Scratch<'a> +impl Exchange for Scratch where STORAGE::Error: Debug, STATE::Error: Debug, @@ -40,6 +41,7 @@ where fn exchange( &mut self, + config: &Config, storage: &mut STORAGE, state: &mut STATE, progress: ExchangeProgress, @@ -56,6 +58,10 @@ where assert_ne!(a.size, 0); assert_ne!(b.size, 0); + if config.scratch_bank.size < INTERNAL_PAGE_SIZE as u32 { + return Err(ExchangeError::ScratchInsufficient); + } + let size = a.size; // Both are equal let full_pages = size / INTERNAL_PAGE_SIZE as Address; @@ -74,6 +80,8 @@ where let a_location = a.location; let b_location = b.location; + let scratch_bank_pages = config.scratch_bank.size / INTERNAL_PAGE_SIZE as u32; + let mut first = true; for page_index in page_index..full_pages { let offset = page_index * INTERNAL_PAGE_SIZE as u32; @@ -81,8 +89,9 @@ where let a_location = a_location + offset; let b_location = b_location + offset; - let scratch_index = page_index as usize % self.pages.len(); - let scratch_location = self.pages[scratch_index].start; + let scratch_index = page_index % scratch_bank_pages; + let scratch_offset = scratch_index * INTERNAL_PAGE_SIZE as u32; + let scratch_location = config.scratch_bank.location + scratch_offset; log::trace!( "Exchange: Page {}, from a ({}) to b ({}) using scratch ({})", diff --git a/src/hardware/mod.rs b/src/hardware/mod.rs index bdef5ce..3b9cfdf 100644 --- a/src/hardware/mod.rs +++ b/src/hardware/mod.rs @@ -47,6 +47,8 @@ pub struct Config { pub update_bank: Bank, /// bank the bootloader is contained in, switching between banks pub bootloader_bank: Bank, + /// bank the pages are temporarily stored when using the `Scratch` exchange method + pub scratch_bank: Bank, // Initial Image is stored to this bank after first update, restore on failure // pub golden_bank: Bank, /// section of RAM of this device diff --git a/src/hardware/processor.rs b/src/hardware/processor.rs index 33be20a..dcd54f3 100644 --- a/src/hardware/processor.rs +++ b/src/hardware/processor.rs @@ -35,7 +35,7 @@ pub mod cortex_m { } } - fn setup(&mut self, config: &crate::hardware::Config) { + fn setup(&mut self, _config: &crate::hardware::Config) { // Nothing to do! } } diff --git a/src/manager/mod.rs b/src/manager/mod.rs index 4f4498d..9f41206 100644 --- a/src/manager/mod.rs +++ b/src/manager/mod.rs @@ -15,9 +15,13 @@ pub struct MoonbootManager { processor: CONTEXT::Processor, } +#[cfg_attr(feature = "use-defmt", derive(Format))] +#[derive(Debug)] pub struct InitError; -pub enum MarkError { +#[cfg_attr(feature = "use-defmt", derive(Format))] +#[derive(Debug)] +pub enum MarkError { UpdateQueuedButNotInstalled, State(E), } diff --git a/src/state/mod.rs b/src/state/mod.rs index 18501c6..24d6bfb 100644 --- a/src/state/mod.rs +++ b/src/state/mod.rs @@ -116,7 +116,7 @@ pub struct MoonbootState { /// RAM. As long as you don't want to perform update download, power cycle the device, and then /// apply the update, storing it in volatile memory is fine. pub trait State { - type Error; + type Error: Debug; /// Read the shared state fn read(&mut self) -> Result;