Skip to content

Commit

Permalink
Multi region packing (#1599)
Browse files Browse the repository at this point in the history
This PR modifies the build system to let flash use all available MPU regions,
resulting in tighter packing and smaller total image size.  This shrinks the
`sidecar-rev-c` image by 256 KiB, and the `gimlet-f` image by 188 KiB.

In other words, tasks go from having exactly 1 flash region to _at least_ 1
flash region; this is mostly plumbing that change from
`suggest_memory_region_size` all the way through the `kconfig`.

The change does makes task packing trickier, because tasks can be placed in one
of two orientations: largest-chunk-first, or largest-chunk-last.  I went for a
very dumb O(N^2) algorithm that checks every unplaced task and picks the best;
we're far from having > 100 tasks, so I'm not worried about bad scaling (famous
last words!).

In addition, it updates `xtask/src/sizes.rs` to print the individual chunks when
the `-v` flag is specified.
  • Loading branch information
mkeeter authored Feb 5, 2024
1 parent ead0d5d commit eb8e539
Show file tree
Hide file tree
Showing 8 changed files with 697 additions and 209 deletions.
2 changes: 1 addition & 1 deletion app/sidecar/base.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ fwid = true

[kernel]
name = "sidecar"
requires = {flash = 24600, ram = 6256}
requires = {flash = 25184, ram = 6256}
features = ["dump"]

[caboose]
Expand Down
20 changes: 19 additions & 1 deletion build/kconfig/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub struct TaskConfig {
///
/// The name is the "output" assignment that generated this region,
/// typically (but not necessarily!) either `"ram"` or `"flash"`.
pub owned_regions: BTreeMap<String, RegionConfig>,
pub owned_regions: BTreeMap<String, MultiRegionConfig>,

/// Names of regions (in the app-level `shared_regions`) that this task
/// needs access to.
Expand Down Expand Up @@ -117,6 +117,24 @@ pub struct RegionConfig {
pub attributes: RegionAttributes,
}

/// Description of one memory span containing multiple adjacent regions
///
/// This is equivalent to [`RegionConfig`], but represents a single memory span
/// that should be configured as multiple regions in the MPU.
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct MultiRegionConfig {
/// 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.)
pub base: u32,
/// Size of region, in bytes for each chunk. The platform likely has
/// alignment requirements for this; it must meet them. (For example, on
/// ARMv7-M, it must be a power of two greater than 16.)
pub sizes: Vec<u32>,
/// Flags describing what can be done with this region.
pub attributes: RegionAttributes,
}

#[derive(Copy, Clone, Debug, Serialize, Deserialize)]
pub struct RegionAttributes {
/// Region can be read by tasks that include it.
Expand Down
9 changes: 5 additions & 4 deletions build/xtask/src/clippy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use std::path::PathBuf;

use anyhow::{bail, Result};
use indexmap::IndexMap;

use crate::config::Config;

Expand Down Expand Up @@ -49,8 +48,10 @@ pub fn run(

let build_config = if name == "kernel" {
// Build dummy allocations for each task
let fake_sizes: IndexMap<_, _> =
[("flash", 64), ("ram", 64)].into_iter().collect();
let fake_sizes = crate::dist::TaskRequest {
memory: [("flash", 64), ("ram", 64)].into_iter().collect(),
spare_regions: 0,
};
let task_sizes = toml
.tasks
.keys()
Expand All @@ -71,7 +72,7 @@ pub fn run(
let mut entry_points: std::collections::HashMap<_, _> = allocs
.tasks
.iter()
.map(|(k, v)| (k.clone(), v["flash"].start))
.map(|(k, v)| (k.clone(), v["flash"].start()))
.collect();

// add a dummy caboose point
Expand Down
85 changes: 76 additions & 9 deletions build/xtask/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

use std::collections::{hash_map::DefaultHasher, BTreeMap, BTreeSet};
use std::collections::{hash_map::DefaultHasher, BTreeMap, BTreeSet, VecDeque};
use std::hash::Hasher;
use std::ops::Range;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -461,15 +461,24 @@ impl Config {
}

/// Suggests an appropriate size for the given task (or "kernel"), given
/// its true size. The size depends on MMU implementation, dispatched
/// based on the `target` in the config file.
pub fn suggest_memory_region_size(&self, name: &str, size: u64) -> u64 {
/// its true size and a number of available regions. The size depends on
/// MMU implementation, dispatched based on the `target` in the config file.
///
/// The returned `Vec<u64>` always has the largest value first.
pub fn suggest_memory_region_size(
&self,
name: &str,
size: u64,
regions: usize,
) -> VecDeque<u64> {
match name {
"kernel" => {
// Nearest chunk of 16
((size + 15) / 16) * 16
[((size + 15) / 16) * 16].into_iter().collect()
}
_ => self.mpu_alignment().suggest_memory_region_size(size),
_ => self
.mpu_alignment()
.suggest_memory_region_size(size, regions),
}
}

Expand Down Expand Up @@ -525,10 +534,68 @@ enum MpuAlignment {

impl MpuAlignment {
/// Suggests a minimal memory region size fitting the given number of bytes
fn suggest_memory_region_size(&self, size: u64) -> u64 {
///
/// If multiple regions are available, then we may use them for efficiency.
/// The resulting `Vec` is guaranteed to have the largest value first.
fn suggest_memory_region_size(
&self,
mut size: u64,
regions: usize,
) -> VecDeque<u64> {
match self {
MpuAlignment::PowerOfTwo => size.next_power_of_two(),
MpuAlignment::Chunk(c) => ((size + c - 1) / c) * c,
MpuAlignment::PowerOfTwo => {
const MIN_MPU_REGION_SIZE: u64 = 32;
let mut out = VecDeque::new();
for _ in 0..regions {
let s =
(size.next_power_of_two() / 2).max(MIN_MPU_REGION_SIZE);
out.push_back(s);
size = size.saturating_sub(s);
if size == 0 {
break;
}
}
if size > 0 {
if let Some(s) = out.back_mut() {
*s *= 2;
} else {
out.push_back(size.next_power_of_two());
}
}
// Merge duplicate regions at the end
while out.len() >= 2 {
let n = out.len();
if out[n - 1] == out[n - 2] {
out.pop_back();
*out.back_mut().unwrap() *= 2;
} else {
break;
}
}
// Split the initial (largest) region into as many smaller
// regions as we can fit. This doesn't change total size, but
// can make alignment more flexible, since smaller regions have
// less stringent alignment requirements.
while out[0] > MIN_MPU_REGION_SIZE {
let largest = out[0];
let n = out.iter().filter(|c| **c == largest).count();
if out.len() + n > regions {
break;
}
// Replace `n` instances of `largest` at the start of `out`
// with `n * 2` instances of `largest / 2`
for _ in 0..n {
out.pop_front();
}
for _ in 0..n * 2 {
out.push_front(largest / 2);
}
}
out
}
MpuAlignment::Chunk(c) => {
[((size + c - 1) / c) * c].into_iter().collect()
}
}
}
/// Returns the desired alignment for a region of a particular size
Expand Down
Loading

0 comments on commit eb8e539

Please sign in to comment.