Skip to content

Commit

Permalink
Merge #2433
Browse files Browse the repository at this point in the history
2433: use const generics to remove duplicate code for cortex-m MPU r=bradjc a=hudson-ayers

### Pull Request Overview

This pull request uses const generics to remove duplicate code for the Cortex-M MPU. It also moves mpu.rs into `arch/cortex-m`,
rather than having it in `arch/cortex-m3` and using symbolic links elsewhere. Each specific architecture re-exports the generic type as a concrete type with the correct number of regions for that architecture. I think this is more in line with how we generally handle shared code in other places. I realize that now `cortex-m/mpu.rs` would not work for all `cortex-m` platforms (e.g. `cortex-m0`), but architectures for which this does not work should just not re-export the functionality, and instead present their own MPU type. `cortex-m/src/mpu.rs` clearly indicates in the comments that it is only for use on m3/m4/m7.

We need generics because the cortex-m7 MPU has 16 regions instead of 8. The cortex-m7 MPU has already fallen out of date with the cortex-m3 MPU implementation because #2120 only updated the m3 implementation, and not the mostly duplicate code used in m7.

I used the `pub mod MPU {}` wrapper in each `lib.rs` to avoid having to modify every chip to update the path, but it could be removed.


### Testing Strategy

This pull request was tested by compiling and running blink on an Imix.


### TODO or Help Wanted

N/A


### Documentation Updated

- [x] No updates are required.

### Formatting

- [x] Ran `make prepush`.


Co-authored-by: Hudson Ayers <hayers@stanford.edu>
  • Loading branch information
bors[bot] and hudson-ayers committed Feb 18, 2021
2 parents e5c422b + 459e7e6 commit 9738878
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 733 deletions.
1 change: 1 addition & 0 deletions arch/cortex-m/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

use core::fmt::Write;

pub mod mpu;
pub mod nvic;
pub mod scb;
pub mod support;
Expand Down
48 changes: 22 additions & 26 deletions arch/cortex-m3/src/mpu.rs → arch/cortex-m/src/mpu.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Implementation of the memory protection unit for the Cortex-M3 and
//! Cortex-M4.
//! Implementation of the memory protection unit for the Cortex-M3,
//! Cortex-M4, and Cortex-M7

use core::cell::Cell;
use core::cmp;
Expand All @@ -12,7 +12,7 @@ use kernel::common::StaticRef;
use kernel::mpu;
use kernel::AppId;

/// MPU Registers for the Cortex-M3 and Cortex-M4 families
/// MPU Registers for the Cortex-M3, Cortex-M4 and Cortex-M7 families
/// Described in section 4.5 of
/// <http://infocenter.arm.com/help/topic/com.arm.doc.dui0553a/DUI0553A_cortex_m4_dgug.pdf>
#[repr(C)]
Expand Down Expand Up @@ -127,7 +127,7 @@ const MPU_BASE_ADDRESS: StaticRef<MpuRegisters> =
///
/// There should only be one instantiation of this object as it represents
/// real hardware.
pub struct MPU {
pub struct MPU<const NUM_REGIONS: usize> {
/// MMIO reference to MPU registers.
registers: StaticRef<MpuRegisters>,
/// Optimization logic. This is used to indicate which application the MPU
Expand All @@ -136,9 +136,9 @@ pub struct MPU {
hardware_is_configured_for: OptionalCell<AppId>,
}

impl MPU {
pub const unsafe fn new() -> MPU {
MPU {
impl<const NUM_REGIONS: usize> MPU<NUM_REGIONS> {
pub const unsafe fn new() -> Self {
Self {
registers: MPU_BASE_ADDRESS,
hardware_is_configured_for: OptionalCell::empty(),
}
Expand All @@ -150,35 +150,31 @@ impl MPU {
/// The cortex-m MPU has eight regions, all of which must be configured (though
/// unused regions may be configured as disabled). This struct caches the result
/// of region configuration calculation
pub struct CortexMConfig {
pub struct CortexMConfig<const NUM_REGIONS: usize> {
/// The computed region configuration for this process.
regions: [CortexMRegion; 8],
regions: [CortexMRegion; NUM_REGIONS],
/// Has the configuration changed since the last time the this process
/// configuration was written to hardware?
is_dirty: Cell<bool>,
}

const APP_MEMORY_REGION_NUM: usize = 0;

impl Default for CortexMConfig {
fn default() -> CortexMConfig {
CortexMConfig {
regions: [
CortexMRegion::empty(0),
CortexMRegion::empty(1),
CortexMRegion::empty(2),
CortexMRegion::empty(3),
CortexMRegion::empty(4),
CortexMRegion::empty(5),
CortexMRegion::empty(6),
CortexMRegion::empty(7),
],
impl<const NUM_REGIONS: usize> Default for CortexMConfig<NUM_REGIONS> {
fn default() -> Self {
// a bit of a hack to initialize array without unsafe
let mut ret = Self {
regions: [CortexMRegion::empty(0); NUM_REGIONS],
is_dirty: Cell::new(true),
};
for i in 0..NUM_REGIONS {
ret.regions[i] = CortexMRegion::empty(i);
}
ret
}
}

impl fmt::Display for CortexMConfig {
impl<const NUM_REGIONS: usize> fmt::Display for CortexMConfig<NUM_REGIONS> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "\r\n Cortex-M MPU")?;
for (i, region) in self.regions.iter().enumerate() {
Expand Down Expand Up @@ -232,7 +228,7 @@ impl fmt::Display for CortexMConfig {
}
}

impl CortexMConfig {
impl<const NUM_REGIONS: usize> CortexMConfig<NUM_REGIONS> {
fn unused_region_number(&self) -> Option<usize> {
for (number, region) in self.regions.iter().enumerate() {
if number == APP_MEMORY_REGION_NUM {
Expand Down Expand Up @@ -362,8 +358,8 @@ impl CortexMRegion {
}
}

impl kernel::mpu::MPU for MPU {
type MpuConfig = CortexMConfig;
impl<const NUM_REGIONS: usize> kernel::mpu::MPU for MPU<NUM_REGIONS> {
type MpuConfig = CortexMConfig<NUM_REGIONS>;

fn clear_mpu(&self) {
self.registers.ctrl.write(Control::ENABLE::CLEAR);
Expand Down
4 changes: 3 additions & 1 deletion arch/cortex-m3/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
#![crate_type = "rlib"]
#![no_std]

pub mod mpu;
pub mod mpu {
pub type MPU = cortexm::mpu::MPU<8>;
}

// Re-export the base generic cortex-m functions here as they are
// valid on cortex-m3.
Expand Down
4 changes: 3 additions & 1 deletion arch/cortex-m4/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
#![crate_type = "rlib"]
#![no_std]

pub mod mpu;
pub mod mpu {
pub type MPU = cortexm::mpu::MPU<8>;
}

// Re-export the base generic cortex-m functions here as they are
// valid on cortex-m4.
Expand Down
1 change: 0 additions & 1 deletion arch/cortex-m4/src/mpu.rs

This file was deleted.

4 changes: 3 additions & 1 deletion arch/cortex-m7/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
#![crate_type = "rlib"]
#![no_std]

pub mod mpu;
pub mod mpu {
pub type MPU = cortexm::mpu::MPU<16>; // Cortex-M7 MPU has 16 regions
}

// Re-export the base generic cortex-m functions here as they are
// valid on cortex-m7.
Expand Down

0 comments on commit 9738878

Please sign in to comment.