Skip to content

Commit

Permalink
arm_m: improve apply_memory_protection performance
Browse files Browse the repository at this point in the history
This alters the v6/v7 routine to switch off the MPU before updating it,
and switch it back on after, like v8 does. This has allowed me to use
cheaper code to do the update. That, combined with some field ordering
changes to optimize for access order in the generated code, cuts the
cycles spent in this routine by roughly half.
  • Loading branch information
cbiffle committed Dec 19, 2024
1 parent 937eecb commit 1dbd28c
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 12 deletions.
31 changes: 21 additions & 10 deletions sys/kern/src/arch/arm_m.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,12 +349,17 @@ pub fn reinitialize(task: &mut task::Task) {
task.save_mut().exc_return = EXC_RETURN_CONST;
}

/// PMSAv6/7-style precomputed region data.
///
/// This struct is `repr(C)` to preserve the order of its fields, which happens
/// to match the order of registers in the MPU. While we don't bit-copy the
/// struct directly, this does improve code generation in practice.
#[cfg(any(armv6m, armv7m))]
#[derive(Copy, Clone, Debug)]
#[repr(C)]
pub struct RegionDescExt {
rasr: u32,
rbar: u32,
rasr: u32,
}

#[cfg(any(armv6m, armv7m))]
Expand Down Expand Up @@ -437,9 +442,8 @@ pub const fn compute_region_extension_data(
let rasr =
(xn as u32) << 28 | ap << 24 | tex << 19 | scb << 16 | l2size << 1 | 1;

// Build the RBAR contents with the VALID bit set, but without the region
// number, since we don't know it in this context.
let rbar = base | (1 << 4);
// Build the RBAR contents without the VALID bit or region number.
let rbar = base;
RegionDescExt { rasr, rbar }
}

Expand All @@ -461,16 +465,23 @@ pub fn apply_memory_protection(task: &task::Task) {
unsafe {
mpu.ctrl.write(0);
}
cortex_m::asm::isb();

for (i, region) in task.region_table().iter().enumerate() {
// With the MPU off, we can use the update fast-path.
let data = region.arch_data;
// With the MPU off, there are no particular constraints on the order in
// which we write these fields.
//
// Safety: we're messing with memory protection, so from the API's point
// of view this is very unsafe. But we're loading values generated by
// our (trusted) build script, which only affect tasks and not us. So
// this should be safe by default.
unsafe {
// Set region base address and also select which region we're
// updating.
mpu.rbar.write(region.arch_data.rbar | i as u32);
// Select a region.
mpu.rnr.write(i as u32);
// Set region base address.
mpu.rbar.write(data.rbar);
// Configure the region.
mpu.rasr.write(region.arch_data.rasr);
mpu.rasr.write(data.rasr);
}
}

Expand Down
6 changes: 4 additions & 2 deletions sys/kern/src/descs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ bitflags::bitflags! {
#[derive(Copy, Clone, Debug)]
#[repr(C)]
pub struct RegionDesc {
/// Architecture-specific additional data to make context switch cheaper.
/// Should be first in the struct to improve context switch code generation.
pub arch_data: crate::arch::RegionDescExt,

/// Address of start of region. The platform likely has alignment
/// requirements for this; it must meet them. (For example, on ARMv7-M, it
/// must be naturally aligned for the size.)
Expand All @@ -97,8 +101,6 @@ pub struct RegionDesc {
pub size: u32,
/// Flags describing what can be done with this region.
pub attributes: RegionAttributes,
/// Architecture-specific additional data to make context switch cheaper.
pub arch_data: crate::arch::RegionDescExt,
}

impl RegionDesc {
Expand Down

0 comments on commit 1dbd28c

Please sign in to comment.