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

pmp: add PMP struct #2064

Merged
merged 2 commits into from
Aug 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
56 changes: 30 additions & 26 deletions arch/rv32i/src/pmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,22 @@ register_bitfields![u8,
]
];

/// Main PMP struct.
pub struct PMP {
/// The application that the MPU was last configured for. Used (along with
/// the `is_dirty` flag) to determine if MPU can skip writing the
/// configuration to hardware.
last_configured_for: MapCell<AppId>,
}

impl PMP {
pub const unsafe fn new() -> PMP {
PMP {
last_configured_for: MapCell::empty(),
}
}
}

/// Struct storing configuration for a RISC-V PMP region.
#[derive(Copy, Clone)]
pub struct PMPRegion {
Expand Down Expand Up @@ -136,37 +152,28 @@ impl PMPRegion {
}
}

pub trait PMPConfigType: Default + Copy + Clone + Sized {}

/// Struct storing region configuration for RISCV PMP.
pub struct PMPConfig<N: PMPConfigType> {
pub struct PMPConfig {
Comment on lines -142 to +156
Copy link
Contributor

Choose a reason for hiding this comment

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

This change is fine, but just noting that it is moving us more towards the macro approach as we get further away from the array generators.

Copy link
Contributor

Choose a reason for hiding this comment

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

For general awareness, the minimum subset of const generics that will be stabilized now has its own feature gate (rust-lang/rust#74877). Its not stable yet, but clearly on the fast track now.

/// Array of PMP regions. Each region requires two physical entries.
regions: N,
regions: [Option<PMPRegion>; $x / 2],
/// Indicates if the configuration has changed since the last time it was
/// written to hardware.
is_dirty: Cell<bool>,
/// The application that the MPU was last configured for. Used (along with
/// the `is_dirty` flag) to determine if MPU can skip writing the
/// configuration to hardware.
last_configured_for: MapCell<AppId>,
/// Which region index is used for app memory (if it has been configured).
app_memory_region: OptionalCell<usize>,
}

impl PMPConfigType for [Option<PMPRegion>; $x / 2] {}

impl Default for PMPConfig<[Option<PMPRegion>; $x / 2]> {
impl Default for PMPConfig {
fn default() -> Self {
PMPConfig {
regions: [None; $x / 2],
is_dirty: Cell::new(true),
last_configured_for: MapCell::empty(),
app_memory_region: OptionalCell::empty(),
}
}
}

impl fmt::Display for PMPConfig<[Option<PMPRegion>; $x / 2]> {
impl fmt::Display for PMPConfig {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
writeln!(f, "PMP regions:")?;
for (n, region) in self.regions.iter().enumerate() {
Expand All @@ -179,7 +186,7 @@ impl fmt::Display for PMPConfig<[Option<PMPRegion>; $x / 2]> {
}
}

impl PMPConfig<[Option<PMPRegion>; $x / 2]> {
impl PMPConfig {
fn unused_region_number(&self) -> Option<usize> {
for (number, region) in self.regions.iter().enumerate() {
if self.app_memory_region.contains(&number) {
Expand Down Expand Up @@ -234,18 +241,15 @@ impl PMPConfig<[Option<PMPRegion>; $x / 2]> {
}
}

impl kernel::mpu::MPU for PMPConfig<[Option<PMPRegion>; $x / 2]> {
type MpuConfig = PMPConfig<[Option<PMPRegion>; $x / 2]>;
impl kernel::mpu::MPU for PMP {
type MpuConfig = PMPConfig;

fn enable_mpu(&self) {}

fn disable_mpu(&self) {
// The length of `self.regions` here refers to the number of memory
// slices we can protect with the PMP. Each slice requires two PMP
// entries to protect, so this is half of the number physical hardware
// PMP configuration entries. Therefore, we double the number of regions
// to clear all the relevant `pmpcfg` entries.
for x in 0..(self.regions.len() * 2) {
// We want to disable all of the hardware entries, so we use `$x` here,
// and not `$x / 2`.
for x in 0..$x {
match x % 4 {
0 => {
csr::CSR.pmpcfg[x / 4].modify(
Expand Down Expand Up @@ -300,7 +304,7 @@ impl kernel::mpu::MPU for PMPConfig<[Option<PMPRegion>; $x / 2]> {
}

fn number_total_regions(&self) -> usize {
self.regions.len()
$x / 2
}

fn allocate_region(
Expand Down Expand Up @@ -464,7 +468,7 @@ impl kernel::mpu::MPU for PMPConfig<[Option<PMPRegion>; $x / 2]> {
// Skip PMP configuration if it is already configured for this app and the MPU
// configuration of this app has not changed.
if !last_configured_for_this_app || config.is_dirty.get() {
for (x, region) in self.regions.iter().enumerate() {
for (x, region) in config.regions.iter().enumerate() {
match region {
Some(r) => {
let cfg_val = r.cfg.value as u32;
Expand Down Expand Up @@ -514,6 +518,6 @@ impl kernel::mpu::MPU for PMPConfig<[Option<PMPRegion>; $x / 2]> {
self.last_configured_for.put(*app_id);
}
}
}
};
}
};
}
6 changes: 3 additions & 3 deletions chips/arty_e21_chip/src/chip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ extern "C" {
}

pub struct ArtyExx {
pmp: pmp::PMPConfig<[Option<pmp::PMPRegion>; 2]>,
pmp: pmp::PMP,
userspace_kernel_boundary: rv32i::syscall::SysCall,
clic: rv32i::clic::Clic,
}
Expand All @@ -27,7 +27,7 @@ impl ArtyExx {
let in_use_interrupts: u64 = 0x1FFFF0080;

ArtyExx {
pmp: pmp::PMPConfig::default(),
pmp: pmp::PMP::new(),
userspace_kernel_boundary: rv32i::syscall::SysCall::new(),
clic: rv32i::clic::Clic::new(in_use_interrupts),
}
Expand Down Expand Up @@ -106,7 +106,7 @@ impl ArtyExx {
}

impl kernel::Chip for ArtyExx {
type MPU = pmp::PMPConfig<[Option<pmp::PMPRegion>; 2]>;
type MPU = pmp::PMP;
type UserspaceKernelBoundary = rv32i::syscall::SysCall;
type SchedulerTimer = ();
type WatchDog = ();
Expand Down
6 changes: 3 additions & 3 deletions chips/e310x/src/chip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ PMPConfigMacro!(8);

pub struct E310x<A: 'static + Alarm<'static>> {
userspace_kernel_boundary: rv32i::syscall::SysCall,
pmp: PMPConfig<[Option<PMPRegion>; 4]>,
pmp: PMP,
scheduler_timer: kernel::VirtualSchedulerTimer<A>,
}

impl<A: 'static + Alarm<'static>> E310x<A> {
pub unsafe fn new(alarm: &'static A) -> Self {
Self {
userspace_kernel_boundary: rv32i::syscall::SysCall::new(),
pmp: PMPConfig::default(),
pmp: PMP::new(),
scheduler_timer: kernel::VirtualSchedulerTimer::new(alarm),
}
}
Expand All @@ -54,7 +54,7 @@ impl<A: 'static + Alarm<'static>> E310x<A> {
}

impl<A: 'static + Alarm<'static>> kernel::Chip for E310x<A> {
type MPU = PMPConfig<[Option<PMPRegion>; 4]>;
type MPU = PMP;
type UserspaceKernelBoundary = rv32i::syscall::SysCall;
type SchedulerTimer = kernel::VirtualSchedulerTimer<A>;
type WatchDog = ();
Expand Down
6 changes: 3 additions & 3 deletions chips/earlgrey/src/chip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@ pub const CHIP_FREQ: u32 = CONFIG.chip_freq;

pub struct EarlGrey<A: 'static + Alarm<'static>> {
userspace_kernel_boundary: SysCall,
pmp: PMPConfig<[Option<PMPRegion>; 2]>,
pmp: PMP,
scheduler_timer: kernel::VirtualSchedulerTimer<A>,
}

impl<A: 'static + Alarm<'static>> EarlGrey<A> {
pub unsafe fn new(alarm: &'static A) -> Self {
Self {
userspace_kernel_boundary: SysCall::new(),
pmp: PMPConfig::default(),
pmp: PMP::new(),
scheduler_timer: kernel::VirtualSchedulerTimer::new(alarm),
}
}
Expand Down Expand Up @@ -110,7 +110,7 @@ impl<A: 'static + Alarm<'static>> EarlGrey<A> {
}

impl<A: 'static + Alarm<'static>> kernel::Chip for EarlGrey<A> {
type MPU = PMPConfig<[Option<PMPRegion>; 2]>;
type MPU = PMP;
type UserspaceKernelBoundary = SysCall;
type SchedulerTimer = kernel::VirtualSchedulerTimer<A>;
type WatchDog = ();
Expand Down