Instruction reordering in critical sections

[donRumata03]

As a strongly-ordered instruction (System Control Space interaction), interrupt enabling is guaranteed not to be reordered with other strong-ordered accesses, but that doesn't apply to normal memory. Application Note 321 on Memory Barrier Instructions says that for a general ARMv7/6-M device and for a general SCS instruction:

If the program operation relies on the ordering between an access to a SCS location, and another access to a Normal memory location, then a memory barrier instruction like DMB or DSB is required.

For CPSI{D/E} it says that:

In addition to barriers, the CPSID instruction is also self synchronizing within the instruction stream, and ensures that the effect of setting PRIMASK and/or FAULTMASK is visible on the following instruction, thus ensuring that the appropriate interrupt/exceptions are disabled. CPSIE that performs interrupt enabling is not guaranteed to be self synchronizing, with the architecture guaranteeing the recognition of the change only following an ISB

The interesting part here is that CPSIE which is executed at the end of the critical section doesn't automatically enforce the barriers:

https://github.com/rust-embedded/cortex-m/blob/1746a63ca16b68514ea23dcca1543aed00165452/src/interrupt.rs#L79C7-L79C7

(compiler fence is present there, but not the barrier instructions)

Moreover, ARM recommends writing barriers to correctly retrieve the data produced by a potential pending interrupt (Example 4 „Interrupt handling code“):

LDR R0, =0xE000E100 ; NVIC_SETENA address
MOVS R1, #0x1
STR R1, [R0] ; Enable IRQ #0
DSB ; Ensure write is completed
; (architecturally required, but not strictly
; required for existing Cortex-M processors)
ISB ; Ensure IRQ #0 is executed
CMP R8, #1 ; Value of R8 dependent on the execution
; result of IRQ #0 handler

Probably, an abstract rust caller would expect to see a consistent state of memory writes by the pended interrupt because caller's code might rely on it.

---

Do the considerations above make sense and if they do, should the barriers be added for the «potential fancy future cortex m implementation with superscalar with blackjack and hookers» and should all the perfectly real users suffer from additional instruction(s)?

---

Additionally, an obvious question arises looking at all the implementations of critical sections: the instruction just before disabling interrupts is MRS (reads special register into a general purpose one). As far as I understand, an interrupt might fire after memorizing previous interrupt state but before the interrupt disabling takes effect. In that case, any changes in the interrupt flag made by the interrupt would be discarded… Are there any special architecture requirements that guarantee absence of such behavior? Or is this behavior just considered acceptable «by definition»?

[Dirbaio]

Do the considerations above make sense and if they do, should the barriers be added for the «potential fancy future cortex m implementation with superscalar with blackjack and hookers» and should all the perfectly real users suffer from additional instruction(s)?

I imagine this won't be very popular if it's not actually needed in today's cortex-m processors.

As far as I understand, an interrupt might fire after memorizing previous interrupt state but before the interrupt disabling takes effect. In that case, any changes in the interrupt flag made by the interrupt would be discarded…

The convention is interrupt handlers can use critical sections themselves, but must always restore the interrupt state. Assuming that, it's fine if the interrupt fires after reading the interrupt state because it won't have changed.

(AFAIK this requirement on user codeis not documented anywhere? it probably should)

[donRumata03]

Thanks for your response!

Hmm, maybe an appropriate place to document the requirement is a reason for interrupt::disable's unsafety (since in interrupts the function should be paired with enable), but it's actually marked as safe so the change would be breaking...

By the begging of an interrupt interrupts are guaranteed to be globally enabled, so interrupt::enable actually doesn't need such a requirement.

Probably it is just worth mentioning in the description of disabling function

[adamgreig]

Thanks for opening the issue!

For the first issue, whether cpsie should always be followed by isb, I don't think we need to - as far as I can see, the critical section's guarantees are still upheld, and the only reason you might want isb after cpsie is to ensure a pending interrupt gets to run before any other code after the critical section. For users who need this behaviour they can call isb directly, but I don't think there are memory-safety implications of not always including it. It also seems like it's not generally needed on any existing Cortex-M implementation. That said, I don't think there would be a significant impact to adding isb after cpsie in interrupt::enable(), so if there was a good reason to do so I think we could justify it.

For the second issue, I think the troublesome scenario is only when:

1. Thread mode code wants to enter a critical section
2. It reads the current interrupt state, and saves that they are currently enabled
3. An interrupt fires before the thread mode disables them, and that interrupt disables interrupts and then returns
4. The thread mode code disables interrupts (effectively a no-op here), runs its critical section, and then
5. The thread mode code re-enables interrupts since it saw them as enabled in step 1

So the problem is that the interrupt handler disabled interrupts but the critical section then re-enabled them. Is that an issue? The purpose of this check (in interrupt::free and in the critical-section impl) is to ensure that nesting critical sections works OK (i.e. the innermost CS doesn't re-enable interrupts while still inside an outer CS), and I don't think that's broken in this scenario.

Do we actually need interrupts to always leave interrupt state enabled? It's not like they ever need to remember to leave it disabled, since of course they couldn't have run if it was disabled when they started, and similarly it's not like we could have been inside a critical section when the interrupt fired.

[donRumata03]

Thanks for clarification! Yeah, probably, both cases (firing pending interrupts immediately and the lack of discarding changes of interrupt enable flag) refer to user's own affairs while the core invariants are already maintained.