Reworked errors

This commit is contained in:
Wouter Geraedts 2022-07-11 18:50:55 +02:00
parent e2d1ce797b
commit 1cd3c4f5be
4 changed files with 183 additions and 194 deletions

View file

@ -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<void::Void, ()> {
pub fn boot(&mut self) -> Result<void::Void, STATE::Error> {
// 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<Update, STATE::Error> {
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<STORAGE::Error, STATE::Error, EXCHANGE::OtherError>> {
self.exchange
.exchange::<STORAGE, STATE, INTERNAL_PAGE_SIZE>(
&mut self.storage,

View file

@ -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<InternalMemory, HardwareState, CPU, INTERNAL_PAGE_SIZE>
pub struct InitError;
pub enum MarkError<E> {
UpdateQueuedButNotInstalled,
State(E),
}
impl<STORAGE: Storage, STATE: State, PROCESSOR: Processor, const INTERNAL_PAGE_SIZE: usize>
MoonbootManager<STORAGE, STATE, PROCESSOR, INTERNAL_PAGE_SIZE>
{
pub fn new(
config: Config,
internal_memory: InternalMemory,
state: HardwareState,
processor: CPU,
) -> MoonbootManager<InternalMemory, HardwareState, CPU, INTERNAL_PAGE_SIZE> {
Self {
storage: STORAGE,
state: STATE,
processor: PROCESSOR,
) -> Result<MoonbootManager<STORAGE, STATE, PROCESSOR, INTERNAL_PAGE_SIZE>, 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<STATE::Error>> {
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(&current_state)
self.state.write(&current_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<void::Void, ()> {
pub fn update(&mut self) -> Result<void::Void, STATE::Error> {
// 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<InternalMemory, HardwareState, CPU, INTERNAL_PAGE_SIZE>
{
#[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<InternalMemory, HardwareState, CPU, INTERNAL_PAGE_SIZE>
// {
// #[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<InternalMemory, HardwareState, CPU, INTERNAL_PAGE_SIZE>
{
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<InternalMemory, HardwareState, CPU, INTERNAL_PAGE_SIZE>
// {
// 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<InternalMemory, HardwareState, CPU, INTERNAL_PAGE_SIZE>
{
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<InternalMemory, HardwareState, CPU, INTERNAL_PAGE_SIZE>
// {
// 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(|_| ())
// }
// }
// }
// }
// }

View file

@ -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, STATE, OTHER> {
Storage(STORAGE),
State(STATE),
Other(OTHER),
}
/// Abstraction for the exchange operation of the current state.
pub trait Exchange {
type OtherError;
fn exchange<STORAGE: Storage, STATE: State, const INTERNAL_PAGE_SIZE: usize>(
&mut self,
internal_memory: &mut STORAGE,
state: &mut STATE,
progress: ExchangeProgress,
) -> Result<(), MemoryError>;
) -> Result<(), ExchangeError<STORAGE::Error, STATE::Error, Self::OtherError>>;
}
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<MoonbootState, Self::Error>;
/// 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

View file

@ -15,7 +15,9 @@ extern "C" {
}
impl State for RamState {
fn read(&mut self) -> MoonbootState {
type Error = void::Void;
fn read(&mut self) -> Result<MoonbootState, void::Void> {
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 {
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<InternalMemory: Storage, HardwareState: State, const INTERNAL_PAGE_SIZE: usize>(
type OtherError = void::Void;
fn exchange<STORAGE: Storage, STATE: State, const INTERNAL_PAGE_SIZE: usize>(
&mut self,
internal_memory: &mut InternalMemory,
state: &mut HardwareState,
exchange: ExchangeProgress,
) -> Result<(), MemoryError> {
storage: &mut STORAGE,
state: &mut STATE,
progress: ExchangeProgress,
) -> Result<(), ExchangeError<STORAGE::Error, STATE::Error, Self::OtherError>> {
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(())