arch/cortex-m: introduce CortexMVariant trait and avoid calls through extern "C" symbols

[lschuermann]

Pull Request Overview

Given different sub-variants of the Cortex-M architecture family, this introduces a trait to encapsulate differences in between Cortex-M variants. Most prominently, this includes interrupt handlers which are now passed as associated constants of trait implementations versus exported symbols, as well as the switch_to_user function, which is now exposed as a regular function by trait implementors.

This has some significant benefits over passing functions via symbols:

• By using a trait carrying proper first-level Rust functions, the type signatures of the trait and implementing functions are properly validated. Before these changes, some Cortex-M variants already used incorrect type signatures (e.g. *mut u8 instead of *const usize) for the user_stack argument. It also ensures that all functions are provided by a respective sub-architecture at compile time, instead of throwing linker errors.

• Determining the respective functions at compile time, Rust might be able to perform more aggressive inlining, especially if more device-specific proper Rust functions (non hardware-exposed symbols, i.e. not fault or interrupt handlers) were to be added.

• Most importantly, this avoid ambiguity with respect to a compiler fence being inserted by the compiler around calls to switch_to_user. The asm! macro in that function call will cause Rust to emit a compiler fence given the nomem option is not passed, but the opaque extern "C" function call obscured that code path. While this is probably fine and Rust is obliged to generate a compiler fence when switching to C code, having a traceable code path for Rust to the asm! macro will remove any remaining ambiguity and allow us to argue against requiring volatile accesses to userspace memory (during context switches). See document compiler fence before switching to userspace #2582 for further discussion of this issue.

Generally, this seems like the right way to go, which is further confirmed by our prior code using #[allow(improper_ctypes)] to annotate the extern "C" fn switch_to_user function reference.

The fact that some functions are proper trait functions, while others are exposed via associated constants is caused by the associated const functions being #[naked]. To wrap these functions in proper trait functions would require these trait functions to also be #[naked] to avoid generating a function prologue and epilogue, and we'd have to call the respective backing function from within an asm! block. The associated constants allow us to simply reference any function in scope and export it through the provided CortexMVariant trait infrastructure.

Testing Strategy

This pull request was tested by compiling. It still needs to be tested on a (few) ARM board(s). It has undergone basic tests on an nRF52DK (run an application, observe console output).

TODO or Help Wanted

N/A

Documentation Updated

• Updated the relevant files in /docs, or no updates are required.

Formatting

• Ran make prepush.

[hudson-ayers]

Some of the code movement in arch/cortex-m0/src/lib.rs made this a bit harder to review than it would have been, but overall I really like this. We had effectively been using raw function pointers to abstract over interfaces when traits are purpose built to do exactly that.

I have not tested this yet but I think it looks good.

[lschuermann]

Some of the code movement in arch/cortex-m0/src/lib.rs made this a bit harder to review than it would have been

Right, sorry. I think that one part of the code had to be moved given the introduction of the traits, whereas other parts have been shuffled around to collect the naked fns in a single location, above the trait implementation. Assuming I didn't mess this up, this really should only change the order of functions defined there.

I agree that we should've been using traits before, but feel like the idea to use associated constants was a blocker to implementing this (at least took me some time to think of that).

Will test these changes first thing tomorrow when I have access to an ARM board again.

[lschuermann]

Just tested this PR on my nRF52DK. I suppose just seeing the kernel greeting and interacting with the process console is sufficient to validate that this PR generally works, interrupt handlers are correctly referenced and called, so the associated constants idea seems to be the right way to go.

[bradjc]

I didn't go through and double check every chip but based on the description this seems like a good change.

[lschuermann]

Thanks for the feedback @hudson-ayers and @bradjc! Should be all addressed now.

[bradjc]

Looks good

[hudson-ayers]

Thanks for documenting all of this so well

[lschuermann]

bors r+

[bors]

Build succeeded:

• ci-build (macos-latest)
• ci-build (ubuntu-latest)
• ci-format (ubuntu-latest)
• ci-qemu
• ci-tests (macos-latest)
• ci-tests (ubuntu-latest)