Skip to content

Commit

Permalink
fix(interrupts): replace compiler fences with potentially-synchronizi…
Browse files Browse the repository at this point in the history
…ng assembly (#440)

* fix(interrupts): replace compiler fences with potentially-synchronizing assembly

Compiler fences only synchronize with atomic instructions. When creating a thread-local critical section, we need to prevent reordering of any reads and writes across interrupt toggles, not just atomic ones. To achieve this, we omit `nomem` from `asm!`. Since then, the assembly might potentially perform synchronizing operations such as acquiring or releasing a lock, the compiler won't move any reads and writes through these assembly blocks.

Signed-off-by: Martin Kröning <martin.kroening@eonerc.rwth-aachen.de>

* fix(interrupts): add `preserves_flags` option

`IF` from `EFLAGS` must not be restored upon exiting the asm block.
https://doc.rust-lang.org/stable/reference/inline-assembly.html#rules-for-inline-assembly

Signed-off-by: Martin Kröning <martin.kroening@eonerc.rwth-aachen.de>

---------

Signed-off-by: Martin Kröning <martin.kroening@eonerc.rwth-aachen.de>
  • Loading branch information
mkroening committed Oct 13, 2023
1 parent 9d1e486 commit e3ab047
Showing 1 changed file with 6 additions and 7 deletions.
13 changes: 6 additions & 7 deletions src/instructions/interrupts.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
//! Enabling and disabling interrupts

use core::arch::asm;
use core::sync::atomic::{compiler_fence, Ordering};

/// Returns whether interrupts are enabled.
#[inline]
Expand All @@ -16,10 +15,10 @@ pub fn are_enabled() -> bool {
/// This is a wrapper around the `sti` instruction.
#[inline]
pub fn enable() {
// Prevent earlier writes to be moved beyond this point
compiler_fence(Ordering::Release);
// Omit `nomem` to imitate a lock release. Otherwise, the compiler
// is free to move reads and writes through this asm block.
unsafe {
asm!("sti", options(nomem, nostack));
asm!("sti", options(preserves_flags, nostack));
}
}

Expand All @@ -28,10 +27,10 @@ pub fn enable() {
/// This is a wrapper around the `cli` instruction.
#[inline]
pub fn disable() {
// Prevent future writes to be moved before this point.
compiler_fence(Ordering::Acquire);
// Omit `nomem` to imitate a lock acquire. Otherwise, the compiler
// is free to move reads and writes through this asm block.
unsafe {
asm!("cli", options(nomem, nostack));
asm!("cli", options(preserves_flags, nostack));
}
}

Expand Down

0 comments on commit e3ab047

Please sign in to comment.