loader: add ISB around special register writes#410
loader: add ISB around special register writes#410midnightveil wants to merge 1 commit intoseL4:mainfrom
Conversation
Signed-off-by: Julia Vassiliki <julia.vassiliki@unsw.edu.au>
|
Do we need ISBs before |
I think so, because we do an MSR to enable ERET. I think that needs to happen before executing ERET... even if ERET would be a synchronisation barrier itself. I'm not entirely sure on that point, though. |
|
|
||
| /* barrier for writes to special register */ | ||
| isb |
There was a problem hiding this comment.
A bit nitpicky, but it are called "system registers" on AArch64 and the comment is vague enough that you can as well not have it, because if you understand the comment and why the ISB is there, you don't need the comment anyway.
However, because of the ERET below, it's much more important to say why an ISB may be needed after all, because usually it can be omitted in such cases.
The best documentation about this so far I found in the bloody glossary of ARM DDI 0487, under "Context Synchronization event". It says, other than returning from an exception being one:
The effects of a Context synchronization event are:
- No instructions appearing in program order after an instruction that causes a Context
synchronization event will have performed any part of their functionality until the Context
synchronization event has occurred. - All direct and indirect writes to System registers that are made before the Context
synchronization event affect any instruction, including a direct read, that appears
in program order after the instruction causing the Context synchronization event.
To me it seems the instruction causing the context synchronisation event itself is excluded, and happens before the barrier so to speak. So changes to system registers spsr_el2 and elr_el2 which affect ERET behaviour do need an ISB.
These are required by the spec.