From c5d3837745e94f71dd01aed644f30edb27bd0aa7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Robert=20J=C3=B6rdens?= <rj@quartiq.de>
Date: Fri, 28 May 2021 16:03:40 +0200
Subject: [PATCH] itcm: add some comments, make it safe

---
 src/hardware/configuration.rs | 53 ++++++++++++++++++++++++-----------
 1 file changed, 37 insertions(+), 16 deletions(-)

diff --git a/src/hardware/configuration.rs b/src/hardware/configuration.rs
index f762d48..23121df 100644
--- a/src/hardware/configuration.rs
+++ b/src/hardware/configuration.rs
@@ -105,8 +105,17 @@ pub struct PounderDevices {
 /// Static storage for the ethernet DMA descriptor ring.
 static mut DES_RING: ethernet::DesRing = ethernet::DesRing::new();
 
-/// Setup ITCM and load its code from flash
-unsafe fn setup_itcm() {
+/// Setup ITCM and load its code from flash.
+///
+/// For portability and maintainability this is implemented in Rust.
+/// Since this is implemented in Rust the compiler may assume that bss and data are set
+/// up already. There is no easy way to ensure this implementation will never need bss
+/// or data. Hence we can't safely run this as the cortex-m-rt `pre_init` hook before
+/// bss/data is setup.
+///
+/// Calling (through IRQ or directly) any code in ITCM before having called
+/// this method is undefined.
+fn load_itcm() {
     extern "C" {
         static mut __sitcm: u32;
         static mut __eitcm: u32;
@@ -114,18 +123,29 @@ unsafe fn setup_itcm() {
     }
     use core::{ptr, slice, sync::atomic};
 
-    // ITCM is enabled on reset on our CPU but might not be on others.
-    // Keep for completeness.
-    const ITCMCR: *mut u32 = 0xE000_EF90usize as _;
-    ptr::write_volatile(ITCMCR, ptr::read_volatile(ITCMCR) | 1);
-    atomic::fence(atomic::Ordering::SeqCst);
+    // NOTE(unsafe): Assuming the address symbols from the linker as well as
+    // the source instruction data are all valid, this is safe as it only
+    // copies linker-prepared data to where the code expects it to be.
+    // Calling it multiple times is safe as well.
 
-    let len =
-        (&__eitcm as *const u32).offset_from(&__sitcm as *const _) as usize;
-    let dst = slice::from_raw_parts_mut(&mut __sitcm as *mut _, len);
-    let src = slice::from_raw_parts(&__siitcm as *const _, len);
-    dst.copy_from_slice(src);
+    unsafe {
+        // ITCM is enabled on reset on our CPU but might not be on others.
+        // Keep for completeness.
+        const ITCMCR: *mut u32 = 0xE000_EF90usize as _;
+        ptr::write_volatile(ITCMCR, ptr::read_volatile(ITCMCR) | 1);
 
+        // Ensure ITCM is enabled before loading.
+        atomic::fence(atomic::Ordering::SeqCst);
+
+        let len =
+            (&__eitcm as *const u32).offset_from(&__sitcm as *const _) as usize;
+        let dst = slice::from_raw_parts_mut(&mut __sitcm as *mut _, len);
+        let src = slice::from_raw_parts(&__siitcm as *const _, len);
+        // Load code into ITCM.
+        dst.copy_from_slice(src);
+    }
+
+    // Ensure ITCM is loaded before potentially executing any instructions from it.
     atomic::fence(atomic::Ordering::SeqCst);
     cortex_m::asm::dsb();
     cortex_m::asm::isb();
@@ -183,12 +203,12 @@ pub fn setup(
         log::set_logger(&LOGGER)
             .map(|()| log::set_max_level(log::LevelFilter::Trace))
             .unwrap();
-        log::info!("starting...");
+        log::info!("Starting");
     }
 
-    unsafe {
-        setup_itcm();
-    }
+    // Before being able to call any code in ITCM, load that code from flash.
+    log::info!("Loading ITCM");
+    load_itcm();
 
     // Set up the system timer for RTIC scheduling.
     {
@@ -902,6 +922,7 @@ pub fn setup(
 
         #[cfg(feature = "pounder_v1_1")]
         let pounder_stamper = {
+            logger::info!("Pounder v1.1 or later");
             let etr_pin = gpioa.pa0.into_alternate_af3();
 
             // The frequency in the constructor is dont-care, as we will modify the period + clock