You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by an...@apache.org on 2019/02/13 09:35:13 UTC

[mynewt-core] branch master updated: hw/mcu/nordic: Fix hal_system_start

This is an automated email from the ASF dual-hosted git repository.

andk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git


The following commit(s) were added to refs/heads/master by this push:
     new bb87a8f  hw/mcu/nordic: Fix hal_system_start
bb87a8f is described below

commit bb87a8f85e722f49f07adf51f972dc66e2246d78
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Mon Feb 11 12:11:55 2019 +0100

    hw/mcu/nordic: Fix hal_system_start
    
    While current implementation of hal_system_start works fine now, it is
    possible that compiler will "optimize" [1] code in a way that it will
    not work properly because we first set stack pointer to new location
    and then execute some more code which uses local variabled.
    
    To make sure this always works fine, let's use inline assembly here to
    set stack pointer and then jump to image.
    
    [1] while working with other Cortex-M MCU it happened to me that just
        before jumping to image, GCC inserted "ldmia sp! {r3, lr}" which
        basically attempted a read outside RAM area since stack pointer was
        already at the very end of RAM area.
---
 hw/mcu/nordic/nrf51xxx/src/hal_system_start.c | 25 +++++++++----------------
 hw/mcu/nordic/nrf52xxx/src/hal_system_start.c | 25 +++++++++----------------
 2 files changed, 18 insertions(+), 32 deletions(-)

diff --git a/hw/mcu/nordic/nrf51xxx/src/hal_system_start.c b/hw/mcu/nordic/nrf51xxx/src/hal_system_start.c
index 9aa0c73..d4581fe 100644
--- a/hw/mcu/nordic/nrf51xxx/src/hal_system_start.c
+++ b/hw/mcu/nordic/nrf51xxx/src/hal_system_start.c
@@ -27,25 +27,18 @@
  *
  * @param hdr                   The header for the image to boot.
  */
-void
+void __attribute__((naked))
 hal_system_start(void *img_start)
 {
-    typedef void jump_fn(void);
-
-    uint32_t base0entry;
-    uint32_t jump_addr;
-    jump_fn *fn;
-
-    /* First word contains initial MSP value. */
-    __set_MSP(*(uint32_t *)img_start);
-
-    /* Second word contains address of entry point (Reset_Handler). */
-    base0entry = *(uint32_t *)(img_start + 4);
-    jump_addr = base0entry;
-    fn = (jump_fn *)jump_addr;
+    uint32_t *img_data = img_start;
 
-    /* Jump to image. */
-    fn();
+    asm volatile (".syntax unified        \n"
+                  /* 1st word is stack pointer */
+                  "    msr  msp, %0       \n"
+                  /* 2nd word is a reset handler (image entry) */
+                  "    bx   %1            \n"
+                  : /* no output */
+                  : "r" (img_data[0]), "r" (img_data[1]));
 }
 
 /**
diff --git a/hw/mcu/nordic/nrf52xxx/src/hal_system_start.c b/hw/mcu/nordic/nrf52xxx/src/hal_system_start.c
index 09e8613..123353f 100644
--- a/hw/mcu/nordic/nrf52xxx/src/hal_system_start.c
+++ b/hw/mcu/nordic/nrf52xxx/src/hal_system_start.c
@@ -27,25 +27,18 @@
  *
  * @param hdr                   The header for the image to boot.
  */
-void
+void __attribute__((naked))
 hal_system_start(void *img_start)
 {
-    typedef void jump_fn(void);
-
-    uint32_t base0entry;
-    uint32_t jump_addr;
-    register jump_fn *fn;
-
-    /* First word contains initial MSP value. */
-    __set_MSP(*(uint32_t *)img_start);
-
-    /* Second word contains address of entry point (Reset_Handler). */
-    base0entry = *(uint32_t *)(img_start + 4);
-    jump_addr = base0entry;
-    fn = (jump_fn *)jump_addr;
+    uint32_t *img_data = img_start;
 
-    /* Jump to image. */
-    fn();
+    asm volatile (".syntax unified        \n"
+                  /* 1st word is stack pointer */
+                  "    msr  msp, %0       \n"
+                  /* 2nd word is a reset handler (image entry) */
+                  "    bx   %1            \n"
+                  : /* no output */
+                  : "r" (img_data[0]), "r" (img_data[1]));
 }
 
 /**