Fix register restoration #50
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
echavemann
reviewed
Dec 9, 2024
| * 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 |
stevenewald
commented
Dec 9, 2024
| asm volatile("ldr pc, [sp, #-84]"); | ||
| void Scheduler::restore_current_task_regs(exception_stack_registers* regs) | ||
| { | ||
| __set_PSP(reinterpret_cast<unsigned>(®s[1]) + 4); |
There was a problem hiding this comment.
exception_stack_regs already contains reserved regs due to alignment. I don't know why this is +4. It works though. lol
echavemann
reviewed
Dec 9, 2024
| @@ -11,7 +11,8 @@ class GPIOPinEvent { | |||
| GPIOPinEvent( | |||
| uint8_t pin, GPIOConfiguration resistance, | |||
| aidan::GPIOEventController::GPIOEventCallback callback | |||
| ) : pin(pin) | |||
| ) : | |||
There was a problem hiding this comment.
i think ur nvim formatter is clashing with make format
echavemann
approved these changes
Dec 9, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, I tried to be clever. After returning from an async callback, it would restore registers from the stack in userspace. This made it fast (not that it really matters), but also left it vulnerable to an interrupt during register restoration, which would cause corruption of the stack.
This is a crude diagram of the stack with the new design
After yield(), assuming callback is ready
-- original stack ptr (before yield) --
-- regs pushed to stack on SVC entry from yield --
-- new regs that we created so we go to async callback on return --
-- new stack ptr (right before we return from kernel during exception) --
During callback
-- original stack ptr (before yield) --
-- regs pushed to stack on SVC entry from yield --
-- stack allocations during async callback --
-- new stack ptr --
After we return from callback, which triggers a REG_RESTORE SVC call, and brings us to kernel
-- original stack ptr (before yield) --
-- regs pushed to stack on SVC entry from yield --
-- regs pushed to stack on REG_RESTORE svc call --
-- new stack ptr --
After we return from kernel space, we will pop the regs on the stack. We can increment the stack ptr and skip over the regs pushed to stack on REG_RESTORE call (we don't care about the async callback regs at this point, we are returning from it), which will return us to the original location.