From 3d7a4b7cfb7cbc26a4cb1207288497bec1d418d8 Mon Sep 17 00:00:00 2001 From: Wouter Geraedts Date: Mon, 11 Jul 2022 19:46:45 +0200 Subject: [PATCH] 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; }