Xtensa doesn't save SCOMPARE1 register on context switch #21800
Assignees
Labels
Comments
dcpleung
added a commit
to dcpleung/zephyr
that referenced
this issue
Feb 12, 2020
Xtensa uses two instructions to perform atomic compare-and-set instruction: first the comparison register, then the actual instruction to do compare-and-set. There is a potential that context switching is performed before these two instructions. A restored context may have the wrong value in the comparison register. So we need to save and restore the comparison register during context switching. Fixes zephyrproject-rtos#21800 Signed-off-by: Daniel Leung <[email protected]>
jhedberg
pushed a commit
that referenced
this issue
Feb 27, 2020
Xtensa uses two instructions to perform atomic compare-and-set instruction: first the comparison register, then the actual instruction to do compare-and-set. There is a potential that context switching is performed before these two instructions. A restored context may have the wrong value in the comparison register. So we need to save and restore the comparison register during context switching. Fixes #21800 Signed-off-by: Daniel Leung <[email protected]>
hakehuang
pushed a commit
to hakehuang/zephyr
that referenced
this issue
Mar 18, 2020
Xtensa uses two instructions to perform atomic compare-and-set instruction: first the comparison register, then the actual instruction to do compare-and-set. There is a potential that context switching is performed before these two instructions. A restored context may have the wrong value in the comparison register. So we need to save and restore the comparison register during context switching. Fixes zephyrproject-rtos#21800 Signed-off-by: Daniel Leung <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Xtensa uses a somewhat odd two-instruction sequence for atomic operations. You first write the SCOMPARE1 special register to the "old value" of the memory address, and then execute a S32C1I instruction to do a compare and swap with a new value if it hasn't changed.
I realized while thinking about the implications of #21771 that the SCOMPARE1 value is not saved by the context switch code. This means that it's possible to take an interrupt between those two instructions, run some code that does more atomic stuff, and get the wrong value in the comparator register when the context is restored!
In practice, this doesn't actually break anything at the moment. The only major user of atomic_cas() currently are in the spinlock code, which always uses a literal "0" for the old value. (Also the sys_sem code uses it with arbitrary values, but that is a userspace abstraction we aren't AFAICT testing on xtensa currently). So not a P1 bug, probably, but something we need to fix for sure.
Context switch on xtensa needs to dump and restore SCOMPARE1.
The text was updated successfully, but these errors were encountered: