Skip to content
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

Initial Rust CMSE support #189

Merged
merged 2 commits into from
Mar 14, 2020
Merged

Initial Rust CMSE support #189

merged 2 commits into from
Mar 14, 2020

Conversation

hug-dev
Copy link
Contributor

@hug-dev hug-dev commented Jan 14, 2020

Armv8-M and Armv8.1-M architecture profiles have an optional Security Extension which provides a set of Security features.
This patch adds initial support of the Cortex-M Security Extensions but providing support for the TT intrinsics and helper functions on top of it in the newly added cmse module of this crate.
The code is a Rust idiomatic implementation of the C requirements described in this document: https://developer.arm.com/docs/ecm0359818/latest

Executed assemble.sh to generate the new static libraries containing the TT* instructions. Tested check_blobs.sh locally and it passed.
Tested on QEMU using the mps2-an505 machine.

@hug-dev hug-dev requested a review from a team as a code owner January 14, 2020 12:02
@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @thejpster (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m labels Jan 14, 2020
@hug-dev
Copy link
Contributor Author

hug-dev commented Jan 14, 2020

oops forgot some Clippy lints

@nickray
Copy link

nickray commented Jan 14, 2020

This is amazing! Perhaps there would be shared interest to setup a sub-working group to advance the state of TrustZone-M in Rust? I believe @thejpster and @jonas-schievink have been experimenting with the nRF53? also ping @perlindgren and @japaric.

src/asm.rs Show resolved Hide resolved
src/asm.rs Show resolved Hide resolved
src/cmse.rs Show resolved Hide resolved
@thejpster
Copy link
Contributor

I have a couple of safety questions, but otherwise this looks great! Thank you so much :)

@jonas-schievink
Copy link
Contributor

I checked the ARMv8 Architecture Reference Manual, and it says the instruction is decoded like this:

 d = UInt(Rd); n = UInt(Rn); alt = (A == '1'); forceunpriv = (T == '1'); 
 if d IN {13,15} || n == 15 then UNPREDICTABLE; 
 if alt && !IsSecure() then UNDEFINED; 

The UNPREDICTABLE branch would cause UB, but is only taken when using invalid source/destination registers, which these wrappers don't (modulo LLVM bugs), so it doesn't matter here. The UNDEFINED branch is taken for the TTA and TTAT instructions when the processor is not in secure mode, which means they will cause a UsageFault exception, which is memory safe, so these wrapper do not need to be marked as unsafe.

Perhaps there would be shared interest to setup a sub-working group to advance the state of TrustZone-M in Rust?

Not sure we need a dedicated subgroup or subteam for this, it seems to fit the Cortex-M team perfectly. While I'm not terribly interested in TrustZone support (since I don't see any use case for it that I find personally interesting), I'm always up for reviewing PRs, and adding support to cortex-m-rt for generating veneers in Non-Secure Callable regions does kinda sound like fun.

@hug-dev
Copy link
Contributor Author

hug-dev commented Jan 20, 2020

Just to add to what @jonas-schievink said (I agree), from Armv8-M ARM (in the glossary):

UNDEFINED Indicates an instruction that generates an Undefined Instruction exception.

And rule RKJPM says that the UsageFault exception will be triggered in that case. That is only for Armv8-M Mainline though, as Baseline only has the HardFault exception.

For TrustZone support as a whole, the steps missing would be:

  • add in the Rust compiler the cmse_nonsecure_call and cmse_nonsecure_entry attribute (I was planning to start a pre-RFC on this). What I presented at Oxidize basically 🤡
  • add drivers for Memory Protection Controller and Peripheral Protection Controller

There is also some scripting needed to build, link and flash Secure and Non-Secure projects together. I am building a simple blinky example using TrustZone and grouping together all of the elements above.

bors bot added a commit that referenced this pull request Mar 1, 2020
198: Remove unnecessary parenthesis r=jonas-schievink a=hug-dev

Otherwise Cargo complains!

This should fix the CI for #193 and #189 

Co-authored-by: Hugues de Valon <hugues.devalon@arm.com>
bors bot added a commit that referenced this pull request Mar 1, 2020
198: Remove unnecessary parenthesis r=jonas-schievink a=hug-dev

Otherwise Cargo complains!

This should fix the CI for #193 and #189 

Co-authored-by: Hugues de Valon <hugues.devalon@arm.com>
Armv8-M and Armv8.1-M architecture profiles have an optional Security
Extension which provides a set of Security features.
This patch adds initial support of the Cortex-M Security Extensions but
providing support for the TT intrinsics and helper functions on top of
it in the newly added cmse module of this crate.
The code is a Rust idiomatic implementation of the C
requirements described in this document:
https://developer.arm.com/docs/ecm0359818/latest

Signed-off-by: Hugues de Valon <hugues.devalon@arm.com>
@hug-dev
Copy link
Contributor Author

hug-dev commented Mar 2, 2020

Clippy complains that clean_dcache_by_ref and clean_dcache_by_slice are missing inline attribute which seems legit.
But it also emits the following warning:

error: this match could be written as a `let` statement
  --> src/asm.rs:9:5
   |
9  | /     match () {
10 | |         #[cfg(all(cortex_m, feature = "inline-asm"))]
11 | |         () => unsafe { asm!("bkpt" :::: "volatile") },
12 | |
...  |
23 | |         () => unimplemented!(),
24 | |     }
   | |_____^
   |
   = note: `-D clippy::match-single-binding` implied by `-D warnings`
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#match_single_binding
help: consider using `let` statement
   |
9  |     let () = ();
10 |     unsafe {
11 |         extern "C" {
12 |             fn __bkpt();
13 |         }
14 |
 ...

It seems like a false positive to me. Is that a bug in Clippy? Should I allow the warning for now?

@therealprof
Copy link
Contributor

The problem is that you cannot cfg gate everything (like if statements) so in a few cases a match is the only thing that works but clippy either doesn't know or see it.

@hug-dev
Copy link
Contributor Author

hug-dev commented Mar 9, 2020

Would you be happy if I allow clippy::match-single-binding on all those match?

Clippy complains that the match expressions used for cfg gating could be
rewritten as a let statement, this is a false positive.
Also adds inline on two functions.

Signed-off-by: Hugues de Valon <hugues.devalon@arm.com>
@hug-dev
Copy link
Contributor Author

hug-dev commented Mar 14, 2020

Added a new allow for Clippy and inline to two functions in order to pass the CI. I hope it's ok to do it like this.

@thejpster
Copy link
Contributor

bors r+

@bors
Copy link
Contributor

bors bot commented Mar 14, 2020

Build succeeded

@bors bors bot merged commit 1cb6baf into rust-embedded:master Mar 14, 2020
adamgreig pushed a commit that referenced this pull request Jan 12, 2022
190: Align sections r=korken89 a=NickeZ

Hey, I had the same issue as in #189. Don't ask me why this works I looked at other linker scripts.

Fixes #189

Co-authored-by: Niklas Claesson <nicke.claesson@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants