Skip to content

Commit

Permalink
Merge #1159
Browse files Browse the repository at this point in the history
1159: New MPU Interface r=bradjc a=dverhaert

### Pull Request Overview

This pull request introduces an updated MPU interface that is no longer Cortex-M4 specific. It follows up on #1126, being part of a plan to make Tock architecture-agnostic (#985). 

An overview of the key differences:    

1. The interface now has two Tock-specific functions for allocating and updating the MPU region for the app-owned portion of its RAM.
2. The interface now only supports specifying user permissions for regions (instead of also specifying supervisor permissions). Memory locations not within any allocated region are required to be inaccessible in user mode, and accessible in supervisor mode. A related change is that we no longer allocate an MPU region for the grant memory in process.rs.
3. Instead of reallocating all the MPU regions every context switch, regions are now only allocated (computed by the MPU) when they are created or updated. Configuration of the MPU registers is now the only thing that happens during context switches.

The main reasoning behind the first decision is that the app break grows over the lifetime of the process. As such, we have found it infeasible for the client to request an MPU region that is suitable for this purpose through a generic region request. The reasoning behind the second decision is that allowing simultaneous specification of both user and supervisor permissions necessitates exposing a large matrix of options that no single MPU is able to support. When decoupled, we are left with a small, logical set of user permissions that we can reasonably expect all MPUs to be able to support. Furthermore, it is more consistent with our current practice of only enabling the MPU for processes.

The new interface allows removal of code from process.rs that assumed the presence of the Cortex-M MPU, and in particular its constraints and features. For instance, in the Cortex-M, the size of an MPU region must to be a power of two and aligned to the start address, and in the case of region overlap, the permissions of the overlapped segment are those of the region with the highest region number.

For further background and discussion, see the relevant [thread on tock-dev](https://groups.google.com/forum/#!topic/tock-dev/Fduxk_0xvUE). If something remains unclear, feel free to leave a comment below.

### Testing Strategy

This pull request was tested by running the `mpu_stack_growth` app in libtock-c, as well as a few others: one that walks over a process's RAM and flash regions, and several that try to make invalid accesses outside of these regions. 

### TODO or Help Wanted

- [ ] **Fix `_start` in userland**. @alevy is working on a [branch](https://github.com/dverhaert/libtock-c/tree/fix_start_order) in libtock-c that fixes `_start` so that processes never access memory past their app break. This pull request is blocking on that branch being merged. 
- [ ] **Loading processes**. Ideally we would load processes in decreasing order of RAM size on boards that use the Cortex-M MPU. Doing so would eliminate the possibility of external fragmentation between processes on such boards.
- [ ] **Size of the `mpu_regions` array**. This field in `struct Process` is currently hard-coded to be 6 in size because the Cortex-M supports 8 regions (and two of these are reserved for the flash and RAM regions). This is the last architecture-specific code remaining in process.rs. It currently seems infeasible to make this size chip-specific, seeing as [Rust doesn't seem to support this yet](rust-lang/rust#43408).

### Documentation Updated

- [x] Updated the relevant files in `/docs`, or no updates are required.

### Formatting

- [x] Ran `make formatall`.


Co-authored-by: Conor McAvity <cmcavity@protonmail.com>
Co-authored-by: Brad Campbell <bradjc5@gmail.com>
Co-authored-by: Danilo Verhaert <daniloverhaert@gmail.com>
  • Loading branch information
4 people committed Oct 17, 2018
2 parents ae898bd + f170fb4 commit 4db9dbb
Show file tree
Hide file tree
Showing 16 changed files with 899 additions and 463 deletions.
6 changes: 3 additions & 3 deletions arch/cortex-m4/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 4db9dbb

Please sign in to comment.