Added two differing swap modes for pages

src/boot/mod.rs

@@ -1,8 +1,8 @@
 use crate::{
     hardware::processor::Processor,
     hardware::{Bank, Config},
-    state::{ExchangeProgress, State, Update, UpdateError},
+    state::{ExchangeProgress, ExchangeStep, State, Update, UpdateError},
-    Address,
+    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<InternalMemory, HardwareState, CPU, INTERNAL_PAGE_SIZE>
+    > MoonbootBoot<InternalMemory, HardwareState, CPU, PageSwap, INTERNAL_PAGE_SIZE>
 {
     /// create a new instance of the bootloader
     pub fn new(
@@ -50,12 +42,14 @@ impl<
         internal_memory: InternalMemory,
         state: HardwareState,
         processor: CPU,
-    ) -> MoonbootBoot<InternalMemory, HardwareState, CPU, INTERNAL_PAGE_SIZE> {
+        swap: PageSwap,
+    ) -> MoonbootBoot<InternalMemory, HardwareState, CPU, PageSwap, INTERNAL_PAGE_SIZE> {
         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 =
+        let exchange_result = self
-            self.exchange_banks_with_start(progress.a, progress.b, progress.page_index);
+            .swap
+            .exchange::<InternalMemory, HardwareState, INTERNAL_PAGE_SIZE>(
+                &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::<InternalMemory, HardwareState, INTERNAL_PAGE_SIZE>(
+                &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;

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;
 

src/manager/mod.rs

@@ -75,7 +75,7 @@ impl<
 
         log::trace!("New state: {:?}", current_state);
 
-        self.state.write(current_state)
+        self.state.write(&current_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(&current_state)?;
 
         log::info!("Stored update request, jumping to bootloader! Geronimo!");
 

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 =
+                let data = MoonbootState::deserialize_from(unsafe { &_moonboot_state_data_start });
-                    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 {
+            log::trace!("Written data: {:?}", unsafe { &_moonboot_state_data_start });
-                &_moonboot_state_data_start
-            });
 
             unsafe {
                 _moonboot_state_crc_start = checksum(&_moonboot_state_data_start);

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<InternalMemory: Storage, HardwareState: State, const INTERNAL_PAGE_SIZE: usize>(
+        &mut self,
+        internal_memory: &mut InternalMemory,
+        state: &mut HardwareState,
+        exchange: ExchangeProgress,
+    ) -> Result<(), MemoryError>;
+}

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<InternalMemory: Storage, HardwareState: State, const INTERNAL_PAGE_SIZE: usize>(
+        &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(())
+    }
+}

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<Address>],
+}
+
+impl<'a> Swap for Scratch<'a> {
+    fn exchange<InternalMemory: Storage, HardwareState: State, const INTERNAL_PAGE_SIZE: usize>(
+        &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(())
+    }
+}