Skip to content

Commit

Permalink
fix(threads/armv8): don't save high regs on first schedule
Browse files Browse the repository at this point in the history
With future-proof-iot#262, the `current_pid` is not set before the scheduler is triggered
for the first time, and therefore `None` on the first run.
The pointer to `current_high_regs` is therefore set to a null pointer.
This causes nrf52840 to hard fault when trying to store registers r4-r11.

To fix this, we now skip storing/ loading of r4-r11 if the pointer is
zero, i.e. when there was no previously running thread whose context has
to be saved.
  • Loading branch information
elenaf9 committed Sep 27, 2024
1 parent 1fff0d7 commit 0074a85
Showing 1 changed file with 33 additions and 14 deletions.
47 changes: 33 additions & 14 deletions src/riot-rs-threads/src/arch/cortex_m.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,18 @@ unsafe extern "C" fn PendSV() {
* so let's use '99' forward ('99f')
*/
beq 99f
msr.n psp, r0
// r1 == 0 means that there was no previous thread.
// This is only the case if the scheduler was triggered for the first time,
// which also means that next thread has no stored context yet.
// Storing and loading of r4-r11 therefore can be skipped.
cmp r1, #0
beq 99f
stmia r1, {{r4-r11}}
ldmia r2, {{r4-r11}}
msr.n psp, r0
99:
movw LR, #0xFFFd
movt LR, #0xFFFF
Expand All @@ -110,6 +119,15 @@ unsafe extern "C" fn PendSV() {
cmp r0, #0
beq 99f
msr.n psp, r0
// r1 == 0 means that there was no previous thread.
// This is only the case if the scheduler was triggered for the first time,
// which also means that next thread has no stored context yet.
// Storing and loading of r4-r11 therefore can be skipped.
cmp r1, #0
beq 99f
//stmia r1!, {{r4-r7}}
str r4, [r1, #16]
str r5, [r1, #20]
Expand All @@ -134,7 +152,6 @@ unsafe extern "C" fn PendSV() {
mov r8, r4
ldmia r2!, {{r4-r7}}
msr.n psp, r0
99:
ldr r0, 999f
mov LR, r0
Expand All @@ -158,6 +175,7 @@ unsafe extern "C" fn PendSV() {
/// - `0` in `r0` if the next thread in the runqueue is the currently running thread
/// - Else it writes into the following registers:
/// - `r1`: pointer to [`Thread::high_regs`] from old thread (to store old register state)
/// or null pointer if there was no previously running thread.
/// - `r2`: pointer to [`Thread::high_regs`] from new thread (to load new register state)
/// - `r0`: stack-pointer for new thread
///
Expand All @@ -183,27 +201,28 @@ unsafe fn sched() -> u128 {
}
};

let current_high_regs;
// `current_high_regs` will be null if there is no current thread.
// This is only the case once, when the very first thread starts running.
// The returned `r1` therefore will be null, and saving/ restoring
// the context is skipped.
let mut current_high_regs = core::ptr::null();
if let Some(current_pid) = threads.current_pid() {
if next_pid == current_pid {
return Some(0);
}

threads.threads[usize::from(current_pid)].sp =
cortex_m::register::psp::read() as usize;
threads.current_thread = Some(next_pid);
let current = &mut threads.threads[usize::from(current_pid)];
current.sp = cortex_m::register::psp::read() as usize;
current_high_regs = current.data.as_ptr();
}

current_high_regs = threads.threads[usize::from(current_pid)].data.as_ptr();
} else {
threads.current_thread = Some(next_pid);
current_high_regs = core::ptr::null();
};
threads.current_thread = Some(next_pid);

let next = &threads.threads[usize::from(next_pid)];
let next_sp = next.sp as usize;
let next_high_regs = next.data.as_ptr() as usize;
let next_sp = next.sp;
let next_high_regs = next.data.as_ptr();

// PendSV expects these three pointers in r0, r1 and r2:
// The caller (`PendSV`) expects these three pointers in r0, r1 and r2:
// r0 = &next.sp
// r1 = &current.high_regs
// r2 = &next.high_regs
Expand Down

0 comments on commit 0074a85

Please sign in to comment.