Merge pull request #50 from stevenewald/fix-reg-restore

Fix register restoration

Author: stevenewald

edge_os.ld

@@ -32,7 +32,14 @@ SECTIONS
   {
 	  PROVIDE(__start_user_programs_data = .);
 	  _build/user_*.o(.data*)
-	  _build/user_*.o(.bss*)
+	  /* _build/user_*.o(.bss*)
+	   * bss needs to be within __bss_start__ and __bss_end__ to be zeroed by the c lib on startup
+	   * however, that makes it really really hard to isolate
+	   * I don't think this current isolation technique will work for bss, so I've enabled user read/write of all bss code
+	   * If we made a real OS, we would have a different binary for each program, but because this is a mini os, we didn't bother.
+	   *
+	   * Because of this reason (and discovering it during finals week), I have disabled the MPU
+	   * */
 	  PROVIDE(__end_user_programs_data = .);
 	  ASSERT((__end_user_programs_data - __start_user_programs_data) <= 16384, "Error: .user_programs_data section exceeds 16 KB");
   } > RAM

include/drivers/gpio_pin_event.hpp

@@ -11,7 +11,8 @@ class GPIOPinEvent {
     GPIOPinEvent(
         uint8_t pin, GPIOConfiguration resistance,
         aidan::GPIOEventController::GPIOEventCallback callback
-    ) : pin(pin)
+    ) :
+        pin(pin)
     {
         // TODO: add resistance
         aidan::GPIOEventController::get().set_gpio_callback(

include/scheduler/scheduler.hpp

@@ -1,7 +1,7 @@
 #pragma once
 #include "config.hpp"
-#include "util.hpp"
 #include "task.hpp"
+#include "util.hpp"
 
 #include <stdio.h>
 
@@ -31,6 +31,7 @@ class Scheduler {
     void change_current_task_priority(uint8_t new_priority);
 
     void yield_current_task();
+    void restore_current_task_regs(exception_stack_registers* regs);
 
 private:
     void handle_first_svc_hit();

include/scheduler/task.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include "config.hpp"
+#include "util.hpp"
 
 namespace edge {
 

include/userlib/system_call_type.hpp

@@ -9,6 +9,7 @@ enum class SystemCallType : uint8_t {
     COMMAND = 2,
     SUBSCRIBE = 3,
     IPC = 4,
-    SET_FAULT_HANDLER = 5
+    SET_FAULT_HANDLER = 5,
+    RESTORE_REGS = 6
 };
 }

src/entrypoint.cpp

@@ -16,7 +16,7 @@ int main(void)
 
     edge::FaultHandler::get();
 
-    edge::scheduler.add_task(exception_task);
+    // edge::scheduler.add_task(exception_task);
 
     edge::scheduler.add_task(ipc_part1);
     edge::scheduler.add_task(ipc_part2);

src/scheduler/mpu.cpp

@@ -19,6 +19,8 @@ MpuController& MpuController::get()
 
 void MpuController::initialize_mpu() const
 {
+    // MPU disabled. Read comment in linker script to see why
+    return;
     auto user_code_start = reinterpret_cast<unsigned>(&__start_user_programs_code);
     MPU->RNR = 0;
     MPU->RBAR = (user_code_start & MPU_RBAR_ADDR_Msk);

src/scheduler/scheduler.cpp

@@ -2,8 +2,9 @@
 
 #include "drivers/driver_commands.hpp"
 #include "nrf52833.h"
-#include "scheduler/pending_process_callbacks.hpp"
 #include "scheduler/mpu.hpp"
+#include "scheduler/pending_process_callbacks.hpp"
+#include "userlib/system_call_type.hpp"
 #include "util.hpp"
 
 namespace edge {
@@ -22,6 +23,7 @@ void Scheduler::start_scheduler()
 
     NVIC_SetPriority(PendSV_IRQn, 0x3);
     NVIC_SetPriority(SysTick_IRQn, 0x1);
+
     MpuController::get().initialize_mpu();
     asm volatile("CPSIE I");
     asm volatile("SVC #0");
@@ -115,24 +117,12 @@ __attribute__((used)) void SysTick_Handler()
 // I don't think there's any way to make this cleaner lol
 __attribute__((used, naked)) USER_CODE void restore_regs()
 {
-    // Load fpscr first so we can avoid dirtying r0 after its popped
-    asm volatile("ldr r0, [sp, #96]\n"
-                 "vmsr fpscr, r0\n");
-
-    // Pop regs as usual
-    asm volatile("pop {r0, r1, r2, r3, r12, lr}");
-
-    // Skip SP and RETPSR. SP will be loaded last
-    // RETPSR should be ignored because we already popped when returning from exception
-    asm volatile("add sp, #8");
-
-    // Pop caller saved FP registers
-    asm volatile("vpop {s0-s15}");
-
-    // Skip FPSCR and 2 reserved regs
-    asm volatile("add sp, #12");
+    asm volatile("svc %0" ::"I"(SystemCallType::RESTORE_REGS));
+}
 
-    asm volatile("ldr pc, [sp, #-84]");
+void Scheduler::restore_current_task_regs(exception_stack_registers* regs)
+{
+    __set_PSP(reinterpret_cast<unsigned>(&regs[1]) + 4);
 }
 
 void Scheduler::yield_current_task()

src/svc/svc.cpp

@@ -68,6 +68,11 @@ void handle_ipc(exception_stack_registers* stack_regs)
     }
 }
 
+void restore_regs(exception_stack_registers* stack_regs)
+{
+    scheduler.restore_current_task_regs(stack_regs);
+}
+
 etl::optional<int> handle_call(exception_stack_registers* stack_regs)
 {
     auto call_type =
@@ -89,6 +94,9 @@ etl::optional<int> handle_call(exception_stack_registers* stack_regs)
         case edge::SystemCallType::SET_FAULT_HANDLER:
             handle_set_fault_handler(stack_regs);
             break;
+        case edge::SystemCallType::RESTORE_REGS:
+            restore_regs(stack_regs);
+            break;
     }
     return etl::nullopt;
 }