Merge #28

28: interrupt: Support no-std pre-v6 ARM targets r=taiki-e a=taiki-e

Closes #26 

This currently disables only IRQs. This is explained in the document on safety requirements:

> - 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**.

I have a few questions:

- ~~Should we also disable FIQs? [The emulation of atomic operation on Linux seemed to disable only IRQs](https://github.com/torvalds/linux/blob/eb555cb5b794f4e12a9897f3d46d5a72104cd4a7/arch/arm/include/asm/atomic.h#L156-L170), but they may have disabled FIQs globally.~~ EDIT: see #28 (comment)
- Is there a way to test this target on CI?

Co-authored-by: Taiki Endo <te316e89@gmail.com>

CHANGELOG.md

@@ -10,6 +10,8 @@ Note: In this file, do not use the hard wrap in the middle of a sentence for com
 
 ## [Unreleased]
 
+- Support atomic CAS on no-std pre-v6 ARM targets (e.g., thumbv4t-none-eabi) under unsafe cfg `portable_atomic_unsafe_assume_single_core`. ([#28](https://github.com/taiki-e/portable-atomic/pull/28))
+
 ## [0.3.11] - 2022-08-12
 
 - Always provide atomic CAS for MSP430 and AVR. ([#31](https://github.com/taiki-e/portable-atomic/pull/31))

README.md

@@ -14,7 +14,7 @@ Portable atomic types including support for 128-bit atomics, atomic float, etc.
 - Provide `AtomicF32` and `AtomicF64`. (optional)
 <!-- - Provide generic `Atomic<T>` type. (optional) -->
 - Provide atomic load/store for targets where atomic is not available at all in the standard library. (RISC-V without A-extension, MSP430, AVR)
-- Provide atomic CAS for targets where atomic CAS is not available in the standard library. (thumbv6m, RISC-V without A-extension, MSP430, AVR) (optional and [single-core only](#optional-cfg) for ARM and RISC-V, always enabled for MSP430 and AVR)
+- Provide atomic CAS for targets where atomic CAS is not available in the standard library. (thumbv6m, pre-v6 ARM, RISC-V without A-extension, MSP430, AVR) (optional and [single-core only](#optional-cfg) for ARM and RISC-V, always enabled for MSP430 and AVR)
 - Provide equivalents on the target that the standard library's atomic-related APIs cause LLVM errors. (fence/compiler_fence on MSP430)
 - Provide stable equivalents of the standard library atomic types' unstable APIs, such as [`AtomicPtr::fetch_*`](https://github.com/rust-lang/rust/issues/99108), [`AtomicBool::fetch_not`](https://github.com/rust-lang/rust/issues/98485).
 - Make features that require newer compilers, such as [fetch_max](https://doc.rust-lang.org/std/sync/atomic/struct.AtomicUsize.html#method.fetch_max), [fetch_min](https://doc.rust-lang.org/std/sync/atomic/struct.AtomicUsize.html#method.fetch_min), [fetch_update](https://doc.rust-lang.org/std/sync/atomic/struct.AtomicPtr.html#method.fetch_update), and [stronger CAS failure ordering](https://github.com/rust-lang/rust/pull/98383) available on Rust 1.34+.
@@ -74,12 +74,14 @@ See [this list](https://github.com/taiki-e/portable-atomic/issues/10#issuecommen
   - 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**.
 
   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.)
 
   Enabling this cfg for targets that have atomic CAS will result in a compile error.
 
-  ARMv6-M (thumbv6m), RISC-V without A-extension are currently supported. See [#26] for support of no-std pre-v6 ARM and multi-core systems.
+  ARMv6-M (thumbv6m), pre-v6 ARM (e.g., thumbv4t), RISC-V without A-extension are currently supported. See [#26] for support of multi-core systems.
 
   Since all MSP430 and AVR are single-core, we always provide atomic CAS for them without this cfg.
 

src/imp/interrupt/armv4t.rs

@@ -0,0 +1,44 @@
+// 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
+
+#[cfg(not(portable_atomic_no_asm))]
+use core::arch::asm;
+
+#[derive(Clone, Copy)]
+pub(super) struct State(u32);
+
+/// Disables interrupts and returns the previous interrupt state.
+#[inline]
+#[instruction_set(arm::a32)]
+pub(super) fn disable() -> State {
+    let cpsr: u32;
+    // SAFETY: reading CPSR and disabling interrupts are 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!(
+            "mrs {0}, cpsr",
+            // We disable only IRQs. See also https://github.com/taiki-e/portable-atomic/pull/28#issuecomment-1214146912.
+            "orr {1}, {0}, 0x80",
+            "msr cpsr_c, {1}",
+            out(reg) cpsr,
+            out(reg) _,
+            options(nostack),
+        );
+    }
+    State(cpsr)
+}
+
+/// Restores the previous interrupt state.
+#[inline]
+#[instruction_set(arm::a32)]
+pub(super) unsafe fn restore(State(prev): 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),
+        );
+    }
+}

src/imp/interrupt/mod.rs

@@ -31,10 +31,19 @@
 use super::msp430 as atomic;
 #[cfg(any(target_arch = "riscv32", target_arch = "riscv64"))]
 use super::riscv as atomic;
+// On pre-v6 ARM, we cannot use core::sync::atomic here because they call the
+// `__sync_*` builtins for non-relaxed loads and stores.
 #[cfg(portable_atomic_armv6m)]
 use core::sync::atomic;
 
 #[cfg_attr(portable_atomic_armv6m, path = "armv6m.rs")]
+#[cfg_attr(
+    all(
+        target_arch = "arm",
+        not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
+    ),
+    path = "armv4t.rs"
+)]
 #[cfg_attr(target_arch = "avr", path = "avr.rs")]
 #[cfg_attr(target_arch = "msp430", path = "msp430.rs")]
 #[cfg_attr(any(target_arch = "riscv32", target_arch = "riscv64"), path = "riscv.rs")]
@@ -106,12 +115,24 @@ impl AtomicBool {
         crate::utils::assert_load_ordering(order);
         #[deny(unreachable_patterns)]
         match () {
-            #[cfg(not(target_arch = "avr"))]
+            #[cfg(not(any(
+                target_arch = "avr",
+                all(
+                    target_arch = "arm",
+                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
+                )
+            )))]
             // SAFETY: any data races are prevented by atomic intrinsics (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
             () => unsafe { (*(self as *const Self as *const atomic::AtomicBool)).load(order) },
-            #[cfg(target_arch = "avr")]
+            #[cfg(any(
+                target_arch = "avr",
+                all(
+                    target_arch = "arm",
+                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
+                )
+            ))]
             // SAFETY: any data races are prevented by disabling interrupts (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
@@ -124,14 +145,26 @@ impl AtomicBool {
         crate::utils::assert_store_ordering(order);
         #[deny(unreachable_patterns)]
         match () {
-            #[cfg(not(target_arch = "avr"))]
+            #[cfg(not(any(
+                target_arch = "avr",
+                all(
+                    target_arch = "arm",
+                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
+                )
+            )))]
             // SAFETY: any data races are prevented by atomic intrinsics (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
             () => unsafe {
                 (*(self as *const Self as *const atomic::AtomicBool)).store(val, order);
             },
-            #[cfg(target_arch = "avr")]
+            #[cfg(any(
+                target_arch = "avr",
+                all(
+                    target_arch = "arm",
+                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
+                )
+            ))]
             // SAFETY: any data races are prevented by disabling interrupts (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
@@ -278,12 +311,24 @@ impl<T> AtomicPtr<T> {
         crate::utils::assert_load_ordering(order);
         #[deny(unreachable_patterns)]
         match () {
-            #[cfg(not(target_arch = "avr"))]
+            #[cfg(not(any(
+                target_arch = "avr",
+                all(
+                    target_arch = "arm",
+                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
+                )
+            )))]
             // SAFETY: any data races are prevented by atomic intrinsics (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
             () => unsafe { (*(self as *const Self as *const atomic::AtomicPtr<T>)).load(order) },
-            #[cfg(target_arch = "avr")]
+            #[cfg(any(
+                target_arch = "avr",
+                all(
+                    target_arch = "arm",
+                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
+                )
+            ))]
             // SAFETY: any data races are prevented by disabling interrupts (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
@@ -296,14 +341,26 @@ impl<T> AtomicPtr<T> {
         crate::utils::assert_store_ordering(order);
         #[deny(unreachable_patterns)]
         match () {
-            #[cfg(not(target_arch = "avr"))]
+            #[cfg(not(any(
+                target_arch = "avr",
+                all(
+                    target_arch = "arm",
+                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
+                )
+            )))]
             // SAFETY: any data races are prevented by atomic intrinsics (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
             () => unsafe {
                 (*(self as *const Self as *const atomic::AtomicPtr<T>)).store(ptr, order);
             },
-            #[cfg(target_arch = "avr")]
+            #[cfg(any(
+                target_arch = "avr",
+                all(
+                    target_arch = "arm",
+                    not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
+                )
+            ))]
             // SAFETY: any data races are prevented by disabling interrupts (see
             // module-level comments) and the raw pointer is valid because we got it
             // from a reference.
@@ -402,14 +459,32 @@ macro_rules! atomic_int {
                 crate::utils::assert_load_ordering(order);
                 #[deny(unreachable_patterns)]
                 match () {
-                    #[cfg(not(target_arch = "avr"))]
+                    #[cfg(not(any(
+                        target_arch = "avr",
+                        all(
+                            target_arch = "arm",
+                            not(any(
+                                target_feature = "v6",
+                                portable_atomic_target_feature = "v6"
+                            ))
+                        )
+                    )))]
                     // SAFETY: any data races are prevented by atomic intrinsics (see
                     // module-level comments) and the raw pointer is valid because we got it
                     // from a reference.
                     () => unsafe {
                         (*(self as *const Self as *const atomic::$atomic_type)).load(order)
                     },
-                    #[cfg(target_arch = "avr")]
+                    #[cfg(any(
+                        target_arch = "avr",
+                        all(
+                            target_arch = "arm",
+                            not(any(
+                                target_feature = "v6",
+                                portable_atomic_target_feature = "v6"
+                            ))
+                        )
+                    ))]
                     // SAFETY: any data races are prevented by disabling interrupts (see
                     // module-level comments) and the raw pointer is valid because we got it
                     // from a reference.
@@ -422,14 +497,32 @@ macro_rules! atomic_int {
                 crate::utils::assert_store_ordering(order);
                 #[deny(unreachable_patterns)]
                 match () {
-                    #[cfg(not(target_arch = "avr"))]
+                    #[cfg(not(any(
+                        target_arch = "avr",
+                        all(
+                            target_arch = "arm",
+                            not(any(
+                                target_feature = "v6",
+                                portable_atomic_target_feature = "v6"
+                            ))
+                        )
+                    )))]
                     // SAFETY: any data races are prevented by atomic intrinsics (see
                     // module-level comments) and the raw pointer is valid because we got it
                     // from a reference.
                     () => unsafe {
                         (*(self as *const Self as *const atomic::$atomic_type)).store(val, order);
                     },
-                    #[cfg(target_arch = "avr")]
+                    #[cfg(any(
+                        target_arch = "avr",
+                        all(
+                            target_arch = "arm",
+                            not(any(
+                                target_feature = "v6",
+                                portable_atomic_target_feature = "v6"
+                            ))
+                        )
+                    ))]
                     // SAFETY: any data races are prevented by disabling interrupts (see
                     // module-level comments) and the raw pointer is valid because we got it
                     // from a reference.

src/imp/mod.rs

@@ -112,6 +112,10 @@ mod fallback;
 )]
 #[cfg(any(
     portable_atomic_armv6m,
+    all(
+        target_arch = "arm",
+        not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
+    ),
     target_arch = "avr",
     target_arch = "msp430",
     target_arch = "riscv32",

src/lib.rs

@@ -6,7 +6,7 @@ Portable atomic types including support for 128-bit atomics, atomic float, etc.
 - Provide `AtomicF32` and `AtomicF64`. (optional)
 <!-- - Provide generic `Atomic<T>` type. (optional) -->
 - Provide atomic load/store for targets where atomic is not available at all in the standard library. (RISC-V without A-extension, MSP430, AVR)
-- Provide atomic CAS for targets where atomic CAS is not available in the standard library. (thumbv6m, RISC-V without A-extension, MSP430, AVR) (optional and [single-core only](#optional-cfg) for ARM and RISC-V, always enabled for MSP430 and AVR)
+- Provide atomic CAS for targets where atomic CAS is not available in the standard library. (thumbv6m, pre-v6 ARM, RISC-V without A-extension, MSP430, AVR) (optional and [single-core only](#optional-cfg) for ARM and RISC-V, always enabled for MSP430 and AVR)
 - Provide equivalents on the target that the standard library's atomic-related APIs cause LLVM errors. (fence/compiler_fence on MSP430)
 - Provide stable equivalents of the standard library atomic types' unstable APIs, such as [`AtomicPtr::fetch_*`](https://github.com/rust-lang/rust/issues/99108), [`AtomicBool::fetch_not`](https://github.com/rust-lang/rust/issues/98485).
 - Make features that require newer compilers, such as [fetch_max](https://doc.rust-lang.org/std/sync/atomic/struct.AtomicUsize.html#method.fetch_max), [fetch_min](https://doc.rust-lang.org/std/sync/atomic/struct.AtomicUsize.html#method.fetch_min), [fetch_update](https://doc.rust-lang.org/std/sync/atomic/struct.AtomicPtr.html#method.fetch_update), and [stronger CAS failure ordering](https://github.com/rust-lang/rust/pull/98383) available on Rust 1.34+.
@@ -66,12 +66,14 @@ See [this list](https://github.com/taiki-e/portable-atomic/issues/10#issuecommen
   - 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**.
 
   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.)
 
   Enabling this cfg for targets that have atomic CAS will result in a compile error.
 
-  ARMv6-M (thumbv6m), RISC-V without A-extension are currently supported. See [#26] for support of no-std pre-v6 ARM and multi-core systems.
+  ARMv6-M (thumbv6m), pre-v6 ARM (e.g., thumbv4t), RISC-V without A-extension are currently supported. See [#26] for support of multi-core systems.
 
   Since all MSP430 and AVR are single-core, we always provide atomic CAS for them without this cfg.
 
@@ -176,6 +178,17 @@ See [this list](https://github.com/taiki-e/portable-atomic/issues/10#issuecommen
     ),
     feature(asm_experimental_arch)
 )]
+// non-Linux armv4t (tier 3)
+#![cfg_attr(
+    all(
+        portable_atomic_nightly,
+        target_arch = "arm",
+        not(target_has_atomic = "ptr"),
+        not(any(target_feature = "v6", portable_atomic_target_feature = "v6")),
+        any(test, portable_atomic_unsafe_assume_single_core)
+    ),
+    feature(isa_attribute)
+)]
 // Old nightly only
 // These features are already stable or have already been removed from compilers,
 // and can safely be enabled for old nightly as long as version detection works.
@@ -246,6 +259,10 @@ compile_error!(
         not(portable_atomic_no_atomic_cas),
         not(any(
             portable_atomic_armv6m,
+            all(
+                target_arch = "arm",
+                not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
+            ),
             all(
                 any(target_arch = "riscv32", target_arch = "riscv64"),
                 portable_atomic_no_atomic_cas
@@ -260,6 +277,10 @@ compile_error!(
         target_has_atomic = "ptr",
         not(any(
             portable_atomic_armv6m,
+            all(
+                target_arch = "arm",
+                not(any(target_feature = "v6", portable_atomic_target_feature = "v6"))
+            ),
             all(
                 any(target_arch = "riscv32", target_arch = "riscv64"),
                 not(target_has_atomic = "ptr")

tests/gba/.cargo/config.toml

@@ -0,0 +1,8 @@
+[build]
+target-dir = "../../target"
+
+[target.thumbv4t-none-eabi]
+runner = "mgba -l 4"
+
+[target.'cfg(all(target_arch = "arm", target_os = "none"))']
+rustflags = ["-C", "link-arg=-Tlinker.ld"]

tests/gba/Cargo.toml

@@ -0,0 +1,20 @@
+[package]
+name = "gba-test"
+version = "0.0.0"
+edition = "2021"
+publish = false
+
+[workspace]
+resolver = "2"
+
+[dependencies]
+portable-atomic = { path = "../.." }
+
+gba = "0.5"
+paste = "1"
+
+[profile.dev]
+panic = "abort"
+
+[profile.release]
+panic = "abort"