From a069efe0e529078570d2928f4990bebd76046ca5 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Wed, 19 Oct 2022 22:38:50 +0900 Subject: [PATCH 1/4] interrupt: Support RISC-V supervisor mode Under portable_atomic_s_mode cfg. --- src/imp/interrupt/riscv.rs | 60 +++++++++++++++++++++++++++++--------- tools/build.sh | 13 +++++++++ 2 files changed, 59 insertions(+), 14 deletions(-) diff --git a/src/imp/interrupt/riscv.rs b/src/imp/interrupt/riscv.rs index cc6b197b..dfa728ad 100644 --- a/src/imp/interrupt/riscv.rs +++ b/src/imp/interrupt/riscv.rs @@ -1,41 +1,73 @@ -// Based on asm generated for functions of interrupt module of https://github.com/rust-embedded/riscv. +// Refs: +// - https://five-embeddev.com/riscv-isa-manual/latest/machine.html#machine-status-registers-mstatus-and-mstatush +// - https://five-embeddev.com/riscv-isa-manual/latest/supervisor.html#sstatus +// +// Generated asm: +// - riscv64gc https://godbolt.org/z/TnPvPa4c4 #[cfg(not(portable_atomic_no_asm))] use core::arch::asm; pub(super) use super::super::riscv as atomic; +// Status register +#[cfg(not(portable_atomic_s_mode))] +macro_rules! status { + () => { + "mstatus" + }; +} +#[cfg(portable_atomic_s_mode)] +macro_rules! status { + () => { + "sstatus" + }; +} + +// MIE (Machine Interrupt Enable) bit (1 << 3) +#[cfg(not(portable_atomic_s_mode))] +const MASK: usize = 0x8; +#[cfg(not(portable_atomic_s_mode))] +macro_rules! mask { + () => { + "0x8" + }; +} +// SIE (Supervisor Interrupt Enable) bit (1 << 1) +#[cfg(portable_atomic_s_mode)] +const MASK: usize = 0x2; +#[cfg(portable_atomic_s_mode)] +macro_rules! mask { + () => { + "0x2" + }; +} + #[derive(Clone, Copy)] -pub(super) struct WasEnabled(bool); +pub(super) struct State(usize); /// Disables interrupts and returns the previous interrupt state. #[inline] -pub(super) fn disable() -> WasEnabled { +pub(super) fn disable() -> State { let r: usize; // SAFETY: reading mstatus and disabling interrupts is safe. // (see module-level comments of interrupt/mod.rs on the safety of using privileged instructions) unsafe { // Do not use `nomem` and `readonly` because prevent subsequent memory accesses from being reordered before interrupts are disabled. - asm!( - "csrr {0}, mstatus", - "csrci mstatus, 0x8", - out(reg) r, - options(nostack, preserves_flags), - ); + asm!(concat!("csrrci {0}, ", status!(), ", ", mask!()), out(reg) r, options(nostack, preserves_flags)); } - // MIE (Machine Interrupt Enable) bit (1 << 3) - WasEnabled(r & 0x8 != 0) + State(r) } /// Restores the previous interrupt state. #[inline] -pub(super) unsafe fn restore(WasEnabled(was_enabled): WasEnabled) { - if was_enabled { +pub(super) unsafe fn restore(State(r): State) { + if r & MASK != 0 { // SAFETY: the caller must guarantee that the state was retrieved by the previous `disable`, // and we've checked that interrupts were enabled before disabling interrupts. unsafe { // Do not use `nomem` and `readonly` because prevent preceding memory accesses from being reordered after interrupts are enabled. - asm!("csrsi mstatus, 0x8", options(nostack, preserves_flags)); + asm!(concat!("csrsi ", status!(), ", ", mask!()), options(nostack, preserves_flags)); } } } diff --git a/tools/build.sh b/tools/build.sh index 8c533bf5..9f1ad317 100755 --- a/tools/build.sh +++ b/tools/build.sh @@ -78,6 +78,7 @@ default_targets=( known_cfgs=( docsrs portable_atomic_unsafe_assume_single_core + portable_atomic_s_mode ) x() { @@ -194,6 +195,12 @@ build() { *) RUSTFLAGS="${target_rustflags} --cfg portable_atomic_unsafe_assume_single_core" \ x cargo "${args[@]}" --feature-powerset --manifest-path tests/api-test/Cargo.toml "$@" + case "${target}" in + riscv*) + RUSTFLAGS="${target_rustflags} --cfg portable_atomic_unsafe_assume_single_core --cfg portable_atomic_s_mode" \ + x cargo "${args[@]}" --feature-powerset --manifest-path tests/api-test/Cargo.toml "$@" + ;; + esac ;; esac else @@ -222,6 +229,12 @@ build() { *) RUSTFLAGS="${target_rustflags} --cfg portable_atomic_unsafe_assume_single_core" \ x cargo "${args[@]}" --target-dir target/assume-single-core "$@" + case "${target}" in + riscv*) + RUSTFLAGS="${target_rustflags} --cfg portable_atomic_unsafe_assume_single_core --cfg portable_atomic_s_mode" \ + x cargo "${args[@]}" --target-dir target/assume-single-core-s-mode "$@" + ;; + esac # portable-atomic-util uses atomic CAS, so doesn't work on # this target without portable_atomic_unsafe_assume_single_core cfg. args+=(--exclude portable-atomic-util) From 0002a238a4ff042d81764d3a610cf393982973cb Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Wed, 19 Oct 2022 22:47:31 +0900 Subject: [PATCH 2/4] interrupt: Support disabling FIQs on pre-v6 ARM Under portable_atomic_disable_fiq cfg. --- src/imp/interrupt/armv4t.rs | 33 +++++++++++++++++++++++++-------- tools/build.sh | 9 +++++++++ 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/imp/interrupt/armv4t.rs b/src/imp/interrupt/armv4t.rs index 2a0c4b11..4010946c 100644 --- a/src/imp/interrupt/armv4t.rs +++ b/src/imp/interrupt/armv4t.rs @@ -1,8 +1,24 @@ // Refs: https://developer.arm.com/documentation/ddi0406/cb/System-Level-Architecture/The-System-Level-Programmers--Model/ARM-processor-modes-and-ARM-core-registers/Program-Status-Registers--PSRs-?lang=en#CIHJBHJA +// +// Generated asm: +// - armv5te https://godbolt.org/z/6oK9Ef7bv #[cfg(not(portable_atomic_no_asm))] use core::arch::asm; +#[cfg(not(portable_atomic_disable_fiq))] +macro_rules! if_disable_fiq { + ($tt:tt) => { + "" + }; +} +#[cfg(portable_atomic_disable_fiq)] +macro_rules! if_disable_fiq { + ($tt:tt) => { + $tt + }; +} + #[derive(Clone, Copy)] pub(super) struct State(u32); @@ -16,12 +32,13 @@ pub(super) fn disable() -> State { unsafe { // Do not use `nomem` and `readonly` because prevent subsequent memory accesses from being reordered before interrupts are disabled. asm!( - "mrs {0}, cpsr", - // We disable only IRQs. See also https://github.com/taiki-e/portable-atomic/pull/28#issuecomment-1214146912. - "orr {1}, {0}, 0x80", // I (IRQ mask) bit (1 << 7) - "msr cpsr_c, {1}", - out(reg) cpsr, - out(reg) _, + "mrs {prev}, cpsr", + "orr {new}, {prev}, 0x80", // I (IRQ mask) bit (1 << 7) + // We disable only IRQs by default. See also https://github.com/taiki-e/portable-atomic/pull/28#issuecomment-1214146912. + if_disable_fiq!("orr {new}, {new}, 0x40"), // F (FIQ mask) bit (1 << 6) + "msr cpsr_c, {new}", + prev = out(reg) cpsr, + new = out(reg) _, options(nostack, preserves_flags), ); } @@ -31,11 +48,11 @@ pub(super) fn disable() -> State { /// Restores the previous interrupt state. #[inline] #[instruction_set(arm::a32)] -pub(super) unsafe fn restore(State(prev): State) { +pub(super) unsafe fn restore(State(cpsr): State) { // SAFETY: the caller must guarantee that the state was retrieved by the previous `disable`, unsafe { // Do not use `nomem` and `readonly` because prevent preceding memory accesses from being reordered after interrupts are enabled. - asm!("msr cpsr_c, {0}", in(reg) prev, options(nostack)); + asm!("msr cpsr_c, {0}", in(reg) cpsr, options(nostack)); } } diff --git a/tools/build.sh b/tools/build.sh index 9f1ad317..753dc0f4 100755 --- a/tools/build.sh +++ b/tools/build.sh @@ -79,6 +79,7 @@ known_cfgs=( docsrs portable_atomic_unsafe_assume_single_core portable_atomic_s_mode + portable_atomic_disable_fiq ) x() { @@ -196,6 +197,10 @@ build() { RUSTFLAGS="${target_rustflags} --cfg portable_atomic_unsafe_assume_single_core" \ x cargo "${args[@]}" --feature-powerset --manifest-path tests/api-test/Cargo.toml "$@" case "${target}" in + thumbv[4-5]t* | armv[4-5]t*) + RUSTFLAGS="${target_rustflags} --cfg portable_atomic_unsafe_assume_single_core --cfg portable_atomic_disable_fiq" \ + x cargo "${args[@]}" --feature-powerset --manifest-path tests/api-test/Cargo.toml "$@" + ;; riscv*) RUSTFLAGS="${target_rustflags} --cfg portable_atomic_unsafe_assume_single_core --cfg portable_atomic_s_mode" \ x cargo "${args[@]}" --feature-powerset --manifest-path tests/api-test/Cargo.toml "$@" @@ -230,6 +235,10 @@ build() { RUSTFLAGS="${target_rustflags} --cfg portable_atomic_unsafe_assume_single_core" \ x cargo "${args[@]}" --target-dir target/assume-single-core "$@" case "${target}" in + thumbv[4-5]t* | armv[4-5]t*) + RUSTFLAGS="${target_rustflags} --cfg portable_atomic_unsafe_assume_single_core --cfg portable_atomic_disable_fiq" \ + x cargo "${args[@]}" --target-dir target/assume-single-core-disable-fiq "$@" + ;; riscv*) RUSTFLAGS="${target_rustflags} --cfg portable_atomic_unsafe_assume_single_core --cfg portable_atomic_s_mode" \ x cargo "${args[@]}" --target-dir target/assume-single-core-s-mode "$@" From 6619b73830015030a9115fbb00e58c2104e76fed Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Wed, 19 Oct 2022 23:05:09 +0900 Subject: [PATCH 3/4] Update portable_atomic_unsafe_assume_single_core docs --- README.md | 14 +++++++++----- src/imp/interrupt/README.md | 21 +++++++++++++++++++++ src/imp/interrupt/mod.rs | 2 ++ src/lib.rs | 14 +++++++++----- 4 files changed, 41 insertions(+), 10 deletions(-) create mode 100644 src/imp/interrupt/README.md diff --git a/README.md b/README.md index 30a4b84c..c3d292fc 100644 --- a/README.md +++ b/README.md @@ -94,18 +94,22 @@ See [this list](https://github.com/taiki-e/portable-atomic/issues/10#issuecommen This cfg is `unsafe`, and note the following safety requirements: - Enabling this cfg for multi-core systems is always **unsound**. - This uses privileged instructions to disable interrupts, so it usually doesn't work on unprivileged mode. - Enabling this cfg in an environment where privileged instructions are not available is also usually considered **unsound**, although the details are system-dependent. - - On pre-v6 ARM, this currently disables only IRQs. - Enabling this cfg in an environment where FIQs must also be disabled is also considered **unsound**. + Enabling this cfg in an environment where privileged instructions are not available, or if the instructions used are not sufficient to disable interrupts in the system, it is also usually considered **unsound**, although the details are system-dependent. - This is intentionally not an optional feature. (If this is an optional feature, dependencies can implicitly enable the feature, resulting in the use of unsound code without the end-user being aware of it.) + The following are known cases: + - On pre-v6 ARM, this disables only IRQs by default. For many systems (e.g., GBA) this is enough. If the system need to disable both IRQs and FIQs, you need to pass the `--cfg portable_atomic_disable_fiq` together. + - On RISC-V without A-extension, this generates code for machine-mode (M-mode) by default. If you pass the `--cfg portable_atomic_s_mode` together, this generates code for supervisor-mode (S-mode). In particular, `qemu-system-riscv*` uses [OpenSBI](https://github.com/riscv-software-src/opensbi) as the default firmware. - Enabling this cfg for targets that have atomic CAS will result in a compile error. + See also [the `interrupt` module's readme](https://github.com/taiki-e/portable-atomic/blob/HEAD/src/imp/interrupt/README.md). + + This is intentionally not an optional feature. (If this is an optional feature, dependencies can implicitly enable the feature, resulting in the use of unsound code without the end-user being aware of it.) ARMv6-M (thumbv6m), pre-v6 ARM (e.g., thumbv4t, thumbv5te), RISC-V without A-extension are currently supported. See [#33] for support of multi-core systems. Since all MSP430 and AVR are single-core, we always provide atomic CAS for them without this cfg. + Enabling this cfg for targets that have atomic CAS will result in a compile error. + Feel free to submit an issue if your target is not supported yet. ## Related Projects diff --git a/src/imp/interrupt/README.md b/src/imp/interrupt/README.md new file mode 100644 index 00000000..008c83dc --- /dev/null +++ b/src/imp/interrupt/README.md @@ -0,0 +1,21 @@ +# Implementation of disabling interrupts + +This module is used to provide atomic CAS for targets where atomic CAS is not available in the standard library. + +- On MSP430 and AVR, they are always single-core, so this module is always used. +- On ARMv6-M (thumbv6m), pre-v6 ARM (e.g., thumbv4t, thumbv5te), RISC-V without A-extension, they could be multi-core, so this module is used when `--cfg portable_atomic_unsafe_assume_single_core` is enabled. + +The implementation uses privileged instructions to disable interrupts, so it usually doesn't work on unprivileged mode. +Enabling this cfg in an environment where privileged instructions are not available, or if the instructions used are not sufficient to disable interrupts in the system, it is also usually considered **unsound**, although the details are system-dependent. + +For some targets, the implementation can be changed by explicitly enabling cfg. + +- On ARMv6-M, this disables interrupts by modifying the PRIMASK register. +- On pre-v6 ARM, this disables interrupts by modifying the I (IRQ mask) bit of the CPSR. +- On pre-v6 ARM with `--cfg portable_atomic_disable_fiq`, this disables interrupts by modifying the I (IRQ mask) bit and F (FIQ mask) bit of the CPSR. +- On RISC-V (without A-extension), this disables interrupts by modifying the MIE (Machine Interrupt Enable) bit of the `mstatus` register. +- On RISC-V (without A-extension) with `--cfg portable_atomic_s_mode`, this disables interrupts by modifying the SIE (Supervisor Interrupt Enable) bit of the `sstatus` register. +- On MSP430, this disables interrupts by modifying the GIE (Global Interrupt Enable) bit of the status register (SR). +- On AVR, this disables interrupts by modifying the I (Global Interrupt Enable) bit of the status register (SREG). + +Feel free to submit an issue if your target is not supported yet. diff --git a/src/imp/interrupt/mod.rs b/src/imp/interrupt/mod.rs index 015203e2..0600b7fb 100644 --- a/src/imp/interrupt/mod.rs +++ b/src/imp/interrupt/mod.rs @@ -17,6 +17,8 @@ // interrupts [^avr2] in atomic ops by default, is considered the latter. // MSP430 as well. // +// See also README.md of this module. +// // [^avr1]: https://github.com/llvm/llvm-project/blob/llvmorg-15.0.0/llvm/lib/Target/AVR/AVRExpandPseudoInsts.cpp#L1008 // [^avr2]: https://github.com/llvm/llvm-project/blob/llvmorg-15.0.0/llvm/test/CodeGen/AVR/atomics/load16.ll#L5 diff --git a/src/lib.rs b/src/lib.rs index 9124e848..0bc41a77 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -86,18 +86,22 @@ See [this list](https://github.com/taiki-e/portable-atomic/issues/10#issuecommen This cfg is `unsafe`, and note the following safety requirements: - Enabling this cfg for multi-core systems is always **unsound**. - This uses privileged instructions to disable interrupts, so it usually doesn't work on unprivileged mode. - Enabling this cfg in an environment where privileged instructions are not available is also usually considered **unsound**, although the details are system-dependent. - - On pre-v6 ARM, this currently disables only IRQs. - Enabling this cfg in an environment where FIQs must also be disabled is also considered **unsound**. + Enabling this cfg in an environment where privileged instructions are not available, or if the instructions used are not sufficient to disable interrupts in the system, it is also usually considered **unsound**, although the details are system-dependent. - This is intentionally not an optional feature. (If this is an optional feature, dependencies can implicitly enable the feature, resulting in the use of unsound code without the end-user being aware of it.) + The following are known cases: + - On pre-v6 ARM, this disables only IRQs by default. For many systems (e.g., GBA) this is enough. If the system need to disable both IRQs and FIQs, you need to pass the `--cfg portable_atomic_disable_fiq` together. + - On RISC-V without A-extension, this generates code for machine-mode (M-mode) by default. If you pass the `--cfg portable_atomic_s_mode` together, this generates code for supervisor-mode (S-mode). In particular, `qemu-system-riscv*` uses [OpenSBI](https://github.com/riscv-software-src/opensbi) as the default firmware. - Enabling this cfg for targets that have atomic CAS will result in a compile error. + See also [the `interrupt` module's readme](https://github.com/taiki-e/portable-atomic/blob/HEAD/src/imp/interrupt/README.md). + + This is intentionally not an optional feature. (If this is an optional feature, dependencies can implicitly enable the feature, resulting in the use of unsound code without the end-user being aware of it.) ARMv6-M (thumbv6m), pre-v6 ARM (e.g., thumbv4t, thumbv5te), RISC-V without A-extension are currently supported. See [#33] for support of multi-core systems. Since all MSP430 and AVR are single-core, we always provide atomic CAS for them without this cfg. + Enabling this cfg for targets that have atomic CAS will result in a compile error. + Feel free to submit an issue if your target is not supported yet. ## Related Projects From d4ccac2b6b21df5dcc0b9ddf89a7b5863eccb057 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Wed, 19 Oct 2022 23:05:15 +0900 Subject: [PATCH 4/4] interrupt: Defer mask until just before branch This does not change the code generation, but in the actual generated code the mask is deferred until just before the branch, like this. Since there has been some misleading discussion about this in the past, we will use code that more closely matches the generated code. --- src/imp/interrupt/armv6m.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/imp/interrupt/armv6m.rs b/src/imp/interrupt/armv6m.rs index 24a42609..1f265e30 100644 --- a/src/imp/interrupt/armv6m.rs +++ b/src/imp/interrupt/armv6m.rs @@ -1,4 +1,7 @@ // Adapted from https://github.com/rust-embedded/cortex-m. +// +// Generated asm: +// - armv6-m https://godbolt.org/z/oMjd4h3Tr #[cfg(not(portable_atomic_no_asm))] use core::arch::asm; @@ -6,11 +9,11 @@ use core::arch::asm; pub(super) use core::sync::atomic; #[derive(Clone, Copy)] -pub(super) struct WasEnabled(bool); +pub(super) struct State(u32); /// Disables interrupts and returns the previous interrupt state. #[inline] -pub(super) fn disable() -> WasEnabled { +pub(super) fn disable() -> State { let r: u32; // SAFETY: reading the priority mask register and disabling interrupts are safe. // (see module-level comments of interrupt/mod.rs on the safety of using privileged instructions) @@ -23,13 +26,13 @@ pub(super) fn disable() -> WasEnabled { options(nostack, preserves_flags), ); } - WasEnabled(r & 0x1 == 0) + State(r) } /// Restores the previous interrupt state. #[inline] -pub(super) unsafe fn restore(WasEnabled(was_enabled): WasEnabled) { - if was_enabled { +pub(super) unsafe fn restore(State(r): State) { + if r & 0x1 == 0 { // SAFETY: the caller must guarantee that the state was retrieved by the previous `disable`, // and we've checked that interrupts were enabled before disabling interrupts. unsafe {