Skip to content

Commit

Permalink
arch/rv32i: Don't sort PMP regions when we configure the MPU
Browse files Browse the repository at this point in the history
Previously we would sort the PMP regions everytime we configured the
MPU. This was wasteful in terms of instructions but also wasted stack
space, probably because we cloned the PMPConfig regions.

This patch changes the sorting to only happen after a change is made to
the regions. This should save the number of times we have to sort,
remove sorting out of the critical path and saves a lot of stack space.

See below for the stack changes:

    ### Release build ###
    ## Before this patch ##
    80     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$15allocate_region17hb3b05a589563feedE
    0     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$24update_app_memory_region17hd6078caa7928d4d8E
    592     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$13configure_mpu17h5967285bd6ec5881E

    ## After this patch ##
    80     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$15allocate_region17hb3b05a589563feedE
    16     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$24update_app_memory_region17hd6078caa7928d4d8E
    80     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$13configure_mpu17h5967285bd6ec5881E

    ### Debug build ###
    ## Before this patch ##
    80     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$15allocate_region17h272c094b5b64d63bE
    96     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$26allocate_app_memory_region17h56b3decc4c649f8cE
    32     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$24update_app_memory_region17h55a48db93ba6d713E
    592     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$13configure_mpu17h32d25c65ffe73910E

    ## After this patch ##
    80     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$15allocate_region17h272c094b5b64d63bE
    96     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$26allocate_app_memory_region17h56b3decc4c649f8cE
    32     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$24update_app_memory_region17h55a48db93ba6d713E
    80     _ZN68_$LT$rv32i..pmp..PMPConfig$u20$as$u20$kernel..platform..mpu..MPU$GT$13configure_mpu17h32d25c65ffe73910E

NOTE: All above numbers already include: tock#2020

I haven't compared instruction diffs, but I'm assuming we reduce the
number of instructions run on app swap as well.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
  • Loading branch information
alistair23 committed Jul 17, 2020
1 parent b28163c commit bed1b92
Showing 1 changed file with 48 additions and 15 deletions.
63 changes: 48 additions & 15 deletions arch/rv32i/src/pmp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,47 @@ impl PMPConfig {
}
None
}

fn sort_regions(&mut self) {
// Get the app region address
let app_addres = if self.app_region.is_some() {
Some(
self.regions[self.app_region.unwrap_or(0)]
.unwrap()
.location
.0,
)
} else {
None
};

// Sort the regions
self.regions.sort_unstable_by(|a, b| {
let (a_start, _a_size) = match a {
Some(region) => (region.location().0 as usize, region.location().1),
None => (0xFFFF_FFFF, 0xFFFF_FFFF),
};
let (b_start, _b_size) = match b {
Some(region) => (region.location().0 as usize, region.location().1),
None => (0xFFFF_FFFF, 0xFFFF_FFFF),
};
a_start.cmp(&b_start)
});

// Update the app region after the sort
if app_addres.is_some() {
for (i, region) in self.regions.iter().enumerate() {
match region {
Some(reg) => {
if reg.location.0 == app_addres.unwrap() {
self.app_region.set(i);
}
}
None => {}
}
}
}
}
}

impl kernel::mpu::MPU for PMPConfig {
Expand Down Expand Up @@ -303,6 +344,8 @@ impl kernel::mpu::MPU for PMPConfig {
config.regions[region_num] = Some(region);
config.is_dirty.set(true);

config.sort_regions();

Some(mpu::Region::new(start as *const u8, size))
}

Expand Down Expand Up @@ -365,6 +408,8 @@ impl kernel::mpu::MPU for PMPConfig {

config.app_region.set(region_num);

config.sort_regions();

Some((region_start as *const u8, region_size))
}

Expand Down Expand Up @@ -398,6 +443,8 @@ impl kernel::mpu::MPU for PMPConfig {
config.regions[region_num] = Some(region);
config.is_dirty.set(true);

config.sort_regions();

Ok(())
}

Expand All @@ -410,22 +457,8 @@ impl kernel::mpu::MPU for PMPConfig {
// 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() {
// Sort the regions before configuring PMP in TOR mode.
let mut regions_sorted = config.regions.clone();
regions_sorted.sort_unstable_by(|a, b| {
let (a_start, _a_size) = match a {
Some(region) => (region.location().0 as usize, region.location().1),
None => (0xFFFF_FFFF, 0xFFFF_FFFF),
};
let (b_start, _b_size) = match b {
Some(region) => (region.location().0 as usize, region.location().1),
None => (0xFFFF_FFFF, 0xFFFF_FFFF),
};
a_start.cmp(&b_start)
});

for x in 0..self.total_regions {
let region = regions_sorted[x];
let region = config.regions[x];
match region {
Some(r) => {
let cfg_val = r.cfg.value;
Expand Down

0 comments on commit bed1b92

Please sign in to comment.