From 8e96978ce2706cfda6d6773fd64652d6dae0d972 Mon Sep 17 00:00:00 2001
From: mcudev <29890609+mcudev@users.noreply.github.com>
Date: Mon, 9 Oct 2017 13:55:54 -0400
Subject: [PATCH] boardloader, bootloader, firmware stage switching updates
 (#30)

---
 embed/bootloader/startup.s |  6 ++++++
 embed/firmware/startup.s   |  6 ++++++
 embed/trezorhal/util.s     | 26 ++++++++++++++------------
 3 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/embed/bootloader/startup.s b/embed/bootloader/startup.s
index 98a5803074..4205c15387 100644
--- a/embed/bootloader/startup.s
+++ b/embed/bootloader/startup.s
@@ -22,6 +22,12 @@ reset_handler:
   ldr r2, =data_size    // size in bytes
   bl memcpy
 
+  // re-enable exceptions
+  // according to "ARM Cortex-M Programming Guide to Memory Barrier Instructions" Application Note 321, section 4.7:
+  // "If it is not necessary to ensure that a pended interrupt is recognized immediately before
+  // subsequent operations, it is not necessary to insert a memory barrier instruction."
+  cpsie f
+
   // enter the application code
   bl main
 
diff --git a/embed/firmware/startup.s b/embed/firmware/startup.s
index 98a5803074..4205c15387 100644
--- a/embed/firmware/startup.s
+++ b/embed/firmware/startup.s
@@ -22,6 +22,12 @@ reset_handler:
   ldr r2, =data_size    // size in bytes
   bl memcpy
 
+  // re-enable exceptions
+  // according to "ARM Cortex-M Programming Guide to Memory Barrier Instructions" Application Note 321, section 4.7:
+  // "If it is not necessary to ensure that a pended interrupt is recognized immediately before
+  // subsequent operations, it is not necessary to insert a memory barrier instruction."
+  cpsie f
+
   // enter the application code
   bl main
 
diff --git a/embed/trezorhal/util.s b/embed/trezorhal/util.s
index b78ccb018b..aa11a5f10b 100644
--- a/embed/trezorhal/util.s
+++ b/embed/trezorhal/util.s
@@ -16,17 +16,18 @@ memset_reg:
   bne .L_loop_begin
   bx lr
 
-  .set SCB_VTOR, 0xE000ED08 // reference "Cortex-M4 Devices Generic User Guide" section 4.3
-
   .global jump_to
   .type jump_to, STT_FUNC
 jump_to:
   mov r4, r0            // save input argument r0
-  // todo: this subroutine re-points the exception handlers before the C code
-  //       that comprises them have been given a good environment to run.
-  //       so, the this needs to disable interrupts before the VTOR
-  //       switch and then the reset_handler of the next stage needs to re-enable interrupts.
-  // todo: CPSID f
+  // this subroutine re-points the exception handlers before the C code
+  // that comprises them has been given a good environment to run.
+  // therefore, this code needs to disable interrupts before the VTOR
+  // update. then, the reset_handler of the next stage needs to re-enable interrupts.
+  // the following prevents activation of all exceptions except Non-Maskable Interrupt (NMI).
+  // according to "ARM Cortex-M Programming Guide to Memory Barrier Instructions" Application Note 321, section 4.8:
+  // "there is no requirement to insert memory barrier instructions after CPSID".
+  cpsid f
   // wipe memory at the end of the current stage of code
   ldr r0, =ccmram_start // r0 - point to beginning of CCMRAM
   ldr r1, =ccmram_end   // r1 - point to byte after the end of CCMRAM
@@ -36,15 +37,16 @@ jump_to:
   ldr r1, =sram_end     // r1 - point to byte after the end of SRAM
   ldr r2, =0            // r2 - the word-sized value to be written
   bl memset_reg
-  // todo: need to think through exception handler races for the VTOR and MSP change below
-  //       there are probably corner cases still.
-  // use the next stage's exception handlers
-  ldr r0, =SCB_VTOR
-  str r4, [r0]
   // give the next stage a fresh main stack pointer
   ldr r0, [r4]
   msr msp, r0
+  // point to the next stage's exception handlers
+  // AN321, section 4.11: "a memory barrier is not required after a VTOR update"
+  .set SCB_VTOR, 0xE000ED08 // reference "Cortex-M4 Devices Generic User Guide" section 4.3
+  ldr r0, =SCB_VTOR
+  str r4, [r0]
   // go on to the next stage
+  ldr lr, =0xffffffff   // set the link register to reset value. there is no reason to return here.
   ldr r0, [r4, 4]
   bx r0