Skip to content

Tidy up some inline asm and add compiler fences where appropriate #264

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Sep 15, 2020
Merged

Tidy up some inline asm and add compiler fences where appropriate #264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
78d7814
Tidy up some inline asm and add compiler fences where appropriate
adamgreig Sep 3, 2020
5d5e151
Address review comments
adamgreig Sep 5, 2020
ccd20a8
Add note about LTO inlining assembly to CHANGELOG
adamgreig Sep 5, 2020
2b818cb
Update blobs
jonas-schievink Sep 10, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,17 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Associated const `PTR` is introduced to Core Peripherals to
eventually replace the existing `ptr()` API.
- A delay driver based on SysTick.
- You can now use LTO to inline assembly calls, even on stable Rust.
See the `asm/lib.rs` documentation for more details.

### Changed

- Previously, asm calls without the `inline-asm` feature enabled used pre-built
objects which were built by a GCC compiler, while `inline-asm` enabled the
use of `llvm_asm!` calls. The asm system has been replaced with a new
technique which generates Rust static libs for stable calling, and uses the
new `asm!` macro with `inline-asm`. See the `asm/lib.rs` documentation for
more details.

## [v0.6.3] - 2020-07-20

Expand Down
146 changes: 80 additions & 66 deletions asm/inline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
//! All of these functions should be blanket-`unsafe`. `cortex-m` provides safe wrappers where
//! applicable.

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

#[inline(always)]
pub unsafe fn __bkpt() {
asm!("bkpt");
Expand All @@ -20,45 +22,62 @@ pub unsafe fn __control_r() -> u32 {

#[inline(always)]
pub unsafe fn __control_w(w: u32) {
asm!("msr CONTROL, {}", in(reg) w);
// ISB is required after writing to CONTROL,
// per ARM architectural requirements (see Application Note 321).
asm!(
"msr CONTROL, {}",
"isb",
in(reg) w
);

// Ensure memory accesses are not reordered around the CONTROL update.
compiler_fence(Ordering::SeqCst);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only affects memory instructions. Probably a good idea to clarify that in the comments.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've done so, but will the asm! block's default memory clobber ensure no memory accesses can be reordered around the block anyway? Will the compiler fence even survive being turned into an FFI call?

I've added these fences on the understanding that the compiler might otherwise reorder instructions around the asm call, but I don't fully see what the fence would prevent that's not prevented by the asm! block and/or FFI call (with or without LTO inlining).

}

#[inline(always)]
pub unsafe fn __cpsid() {
asm!("cpsid i");

// Ensure no subsequent memory accesses are reordered to before interrupts are disabled.
compiler_fence(Ordering::SeqCst);
}

#[inline(always)]
pub unsafe fn __cpsie() {
// Ensure no preceeding memory accesses are reordered to after interrupts are enabled.
compiler_fence(Ordering::SeqCst);

asm!("cpsie i");
}

#[inline(always)]
pub unsafe fn __delay(cyc: u32) {
asm!("
1:
nop
subs {}, #1
bne 1b
// Branch to 1 instead of delay does not generate R_ARM_THM_JUMP8 relocation, which breaks
// linking on the thumbv6m-none-eabi target
", in(reg) cyc);
// Use local labels to avoid R_ARM_THM_JUMP8 relocations which fail on thumbv6m.
asm!(
"1:",
"nop",
"subs {}, #1",
"bne 1b",
in(reg) cyc
);
}

// FIXME do we need compiler fences here or should we expect them in the caller?
#[inline(always)]
pub unsafe fn __dmb() {
asm!("dmb 0xF");
asm!("dmb");
compiler_fence(Ordering::SeqCst);
}

#[inline(always)]
pub unsafe fn __dsb() {
asm!("dsb 0xF");
asm!("dsb");
compiler_fence(Ordering::SeqCst);
}

#[inline(always)]
pub unsafe fn __isb() {
asm!("isb 0xF");
asm!("isb");
compiler_fence(Ordering::SeqCst);
}

#[inline(always)]
Expand Down Expand Up @@ -93,28 +112,28 @@ pub unsafe fn __nop() {
#[inline(always)]
pub unsafe fn __pc_r() -> u32 {
let r;
asm!("mov {}, R15", out(reg) r);
asm!("mov {}, pc", out(reg) r);
r
}

// NOTE: No FFI shim, this requires inline asm.
#[inline(always)]
pub unsafe fn __pc_w(val: u32) {
asm!("mov R15, {}", in(reg) val);
asm!("mov pc, {}", in(reg) val);
}

// NOTE: No FFI shim, this requires inline asm.
#[inline(always)]
pub unsafe fn __lr_r() -> u32 {
let r;
asm!("mov {}, R14", out(reg) r);
asm!("mov {}, lr", out(reg) r);
r
}

// NOTE: No FFI shim, this requires inline asm.
#[inline(always)]
pub unsafe fn __lr_w(val: u32) {
asm!("mov R14, {}", in(reg) val);
asm!("mov lr, {}", in(reg) val);
}

#[inline(always)]
Expand Down Expand Up @@ -161,6 +180,8 @@ pub unsafe fn __wfi() {
pub use self::v7m::*;
#[cfg(any(armv7m, armv8m_main))]
mod v7m {
use core::sync::atomic::{Ordering, compiler_fence};

#[inline(always)]
pub unsafe fn __basepri_max(val: u8) {
asm!("msr BASEPRI_MAX, {}", in(reg) val);
Expand All @@ -185,45 +206,42 @@ mod v7m {
r
}

// FIXME: compiler_fences necessary?
#[inline(always)]
pub unsafe fn __enable_icache() {
asm!(
"
ldr r0, =0xE000ED14 @ CCR
mrs r2, PRIMASK @ save critical nesting info
cpsid i @ mask interrupts
ldr r1, [r0] @ read CCR
orr.w r1, r1, #(1 << 17) @ Set bit 17, IC
str r1, [r0] @ write it back
dsb @ ensure store completes
isb @ synchronize pipeline
msr PRIMASK, r2 @ unnest critical section
",
out("r0") _,
out("r1") _,
out("r2") _,
"ldr {0}, =0xE000ED14", // CCR
"mrs {2}, PRIMASK", // save critical nesting info
"cpsid i", // mask interrupts
"ldr {1}, [{0}]", // read CCR
"orr.w {1}, {1}, #(1 << 17)", // Set bit 17, IC
"str {1}, [{0}]", // write it back
"dsb", // ensure store completes
"isb", // synchronize pipeline
"msr PRIMASK, {2}", // unnest critical section
out(reg) _,
out(reg) _,
out(reg) _,
);
compiler_fence(Ordering::SeqCst);
}

#[inline(always)]
pub unsafe fn __enable_dcache() {
asm!(
"
ldr r0, =0xE000ED14 @ CCR
mrs r2, PRIMASK @ save critical nesting info
cpsid i @ mask interrupts
ldr r1, [r0] @ read CCR
orr.w r1, r1, #(1 << 16) @ Set bit 16, DC
str r1, [r0] @ write it back
dsb @ ensure store completes
isb @ synchronize pipeline
msr PRIMASK, r2 @ unnest critical section
",
out("r0") _,
out("r1") _,
out("r2") _,
"ldr {0}, =0xE000ED14", // CCR
"mrs {2}, PRIMASK", // save critical nesting info
"cpsid i", // mask interrupts
"ldr {1}, [{0}]", // read CCR
"orr.w {1}, {1}, #(1 << 16)", // Set bit 16, DC
"str {1}, [{0}]", // write it back
"dsb", // ensure store completes
"isb", // synchronize pipeline
"msr PRIMASK, {2}", // unnest critical section
out(reg) _,
out(reg) _,
out(reg) _,
);
compiler_fence(Ordering::SeqCst);
}
}

Expand All @@ -234,34 +252,30 @@ mod v7em {
#[inline(always)]
pub unsafe fn __basepri_max_cm7_r0p1(val: u8) {
asm!(
"
mrs r1, PRIMASK
cpsid i
tst.w r1, #1
msr BASEPRI_MAX, {}
it ne
bxne lr
cpsie i
",
"mrs {1}, PRIMASK",
"cpsid i",
"tst.w {1}, #1",
"msr BASEPRI_MAX, {0}",
"it ne",
"bxne lr",
"cpsie i",
in(reg) val,
out("r1") _,
out(reg) _,
);
}

#[inline(always)]
pub unsafe fn __basepri_w_cm7_r0p1(val: u8) {
asm!(
"
mrs r1, PRIMASK
cpsid i
tst.w r1, #1
msr BASEPRI, {}
it ne
bxne lr
cpsie i
",
"mrs {1}, PRIMASK",
"cpsid i",
"tst.w {1}, #1",
"msr BASEPRI, {0}",
"it ne",
"bxne lr",
"cpsie i",
in(reg) val,
out("r1") _,
out(reg) _,
);
}
}
Expand Down
Binary file modified bin/thumbv6m-none-eabi-lto.a
View file
Open in desktop
Binary file not shown.
Binary file modified bin/thumbv6m-none-eabi.a
View file
Open in desktop
Binary file not shown.
Binary file modified bin/thumbv7em-none-eabi-lto.a
View file
Open in desktop
Binary file not shown.
Binary file modified bin/thumbv7em-none-eabi.a
View file
Open in desktop
Binary file not shown.
Binary file modified bin/thumbv7em-none-eabihf-lto.a
View file
Open in desktop
Binary file not shown.
Binary file modified bin/thumbv7em-none-eabihf.a
View file
Open in desktop
Binary file not shown.
Binary file modified bin/thumbv7m-none-eabi-lto.a
View file
Open in desktop
Binary file not shown.
Binary file modified bin/thumbv7m-none-eabi.a
View file
Open in desktop
Binary file not shown.
Binary file modified bin/thumbv8m.base-none-eabi-lto.a
View file
Open in desktop
Binary file not shown.
Binary file modified bin/thumbv8m.base-none-eabi.a
View file
Open in desktop
Binary file not shown.
Binary file modified bin/thumbv8m.main-none-eabi-lto.a
View file
Open in desktop
Binary file not shown.
Binary file modified bin/thumbv8m.main-none-eabi.a
View file
Open in desktop
Binary file not shown.
Binary file modified bin/thumbv8m.main-none-eabihf-lto.a
View file
Open in desktop
Binary file not shown.
Binary file modified bin/thumbv8m.main-none-eabihf.a
View file
Open in desktop
Binary file not shown.