-
Notifications
You must be signed in to change notification settings - Fork 152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix interrupt::free()
unsoundness on multicore systems.
#111
Conversation
This shouldn't be a breaking change since Edition 2021 came out in Rust 1.56, and MSRV is already higher than that (Rust 1.59)
This is unsound on multicore systems because it only disables interrupts in the current core. For multicore chips, a chip-specific critical section implementation is needed instead. Unsoundness is fixed by not returning the `CriticalSection` token. This is a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, lgtm. Defer to disasm/almindor for final review.
@@ -6,7 +6,10 @@ | |||
/// at most once in the whole lifetime of the program. | |||
/// | |||
/// # Note | |||
/// this macro is unsound on multi-core systems | |||
/// | |||
/// this macro requires a `critical-section` implementation to be set. For single core systems, you can |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe feature-gate this macro with critical-section-single-core
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The macro can still be used if critical-section-single-core
is not enabled, if the user supplies a c-s implementation from somewhere else (such as a chip-specific impl for a multicore chip, from its HAL crate).
Folded into #110 |
depends on #109, #110
This is unsound on multicore systems because it only disables interrupts in the
current core. For multicore chips, a chip-specific critical section implementation
is needed instead.
Unsoundness is fixed by not returning the
CriticalSection
token.This is a breaking change.
This is the riscv equivalent of rust-embedded/cortex-m#447