Skip to content

Pass measurement token between RoT and SP #2138

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

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ hubtools = { git = "https://github.com/oxidecomputer/hubtools", default-features
idol = { git = "https://github.com/oxidecomputer/idolatry.git", default-features = false }
idol-runtime = { git = "https://github.com/oxidecomputer/idolatry.git", default-features = false }
lpc55_sign = { git = "https://github.com/oxidecomputer/lpc55_support", default-features = false }
measurement-token = { git = "https://github.com/oxidecomputer/lpc55_support", default-features = false }
ordered-toml = { git = "https://github.com/oxidecomputer/ordered-toml", default-features = false }
pmbus = { git = "https://github.com/oxidecomputer/pmbus", default-features = false }
salty = { version = "0.3", default-features = false }
Expand Down
1 change: 1 addition & 0 deletions app/grapefruit/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ version = "0.1.0"

[features]
dump = ["kern/dump"]
measurement-handoff = ["drv-stm32h7-startup/measurement-handoff"]

[dependencies]
cortex-m = { workspace = true }
Expand Down
3 changes: 2 additions & 1 deletion app/grapefruit/base.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ register-map = "../../drv/spartan7-loader/grapefruit/gfruit_top.json"
[kernel]
name = "grapefruit"
requires = {flash = 32768, ram = 8192}
features = ["dump"]
features = ["dump", "measurement-handoff"]
extern-regions = ["dtcm"]

[caboose]
tasks = ["control_plane_agent"]
Expand Down
6 changes: 6 additions & 0 deletions build/kconfig/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ use std::collections::{BTreeMap, BTreeSet};
/// Application configuration passed into the kernel build.
#[derive(Clone, Debug, Serialize, Deserialize)]
pub struct KernelConfig {
/// Features enabled in the kernel
pub features: Vec<String>,

/// External regions used in the kernel
pub extern_regions: BTreeMap<String, std::ops::Range<u32>>,

/// Tasks in the app image. The order of tasks is significant.
pub tasks: Vec<TaskConfig>,

Expand Down
23 changes: 21 additions & 2 deletions build/xtask/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,10 +581,27 @@ impl Config {
task: &str,
image_name: &str,
) -> Result<IndexMap<String, Range<u32>>> {
self.tasks
let extern_regions = &self
.tasks
.get(task)
.ok_or_else(|| anyhow!("no such task {task}"))?
.extern_regions
.extern_regions;
self.get_extern_regions(extern_regions, image_name)
}

pub fn kernel_extern_regions(
&self,
image_name: &str,
) -> Result<IndexMap<String, Range<u32>>> {
self.get_extern_regions(&self.kernel.extern_regions, image_name)
}

fn get_extern_regions(
&self,
extern_regions: &Vec<String>,
image_name: &str,
) -> Result<IndexMap<String, Range<u32>>> {
extern_regions
.iter()
.map(|r| {
let mut regions = self
Expand Down Expand Up @@ -710,6 +727,8 @@ pub struct Kernel {
pub features: Vec<String>,
#[serde(default)]
pub no_default_features: bool,
#[serde(default)]
pub extern_regions: Vec<String>,
}

fn default_name() -> String {
Expand Down
30 changes: 28 additions & 2 deletions build/xtask/src/dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,13 @@ pub const DEFAULT_KERNEL_STACK: u32 = 1024;
/// that generates the Humility binary necessary for Hubris's CI has run.
/// Once that binary is in place, you should be able to bump this version
/// without breaking CI.
const HUBRIS_ARCHIVE_VERSION: u32 = 9;
///
/// # Changelog
/// Version 10 requires Humility to be aware of the `handoff` kernel feature,
/// which lets the RoT inform the SP when measurements have been taken. If
/// Humility is unaware of this feature, the SP will reset itself repeatedly,
/// which interferes with subsequent programming of auxiliary flash.
const HUBRIS_ARCHIVE_VERSION: u32 = 10;
Comment on lines +45 to +51
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have a draft of the humility changes necessary ? Manufacturing relies on humility flash --check, humility flash --verify and humility tasks idle

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, it's over in mkeeter/measurement-token-handoff.

I'm wondering whether we should move the address + magic numbers into a common repo, e.g. lpc55-areas, to avoid completely magic values.

Copy link
Member

Choose a reason for hiding this comment

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

The idea of sharing the values between Hubris and Humility softwarily rather than by copy-paste is certainly appealing to me in theory...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well you're in luck: oxidecomputer/lpc55_support#94


/// `PackageConfig` contains a bundle of data that's commonly used when
/// building a full app image, grouped together to avoid passing a bunch
Expand Down Expand Up @@ -445,6 +451,18 @@ pub fn package(
}
}
}
// Same check for the kernel. This may be overly conservative, because
// the kernel is special, but we can always make it less strict later.
for r in &cfg.toml.kernel.extern_regions {
if let Some(v) = alloc_regions.get(r) {
bail!(
"cannot use region '{r}' as extern region in \
the kernel because it's used as a normal region by \
[{}]",
v.join(", ")
);
}
}

let mut extern_regions = MultiMap::new();
for (task_name, task) in cfg.toml.tasks.iter() {
Expand Down Expand Up @@ -1529,11 +1547,13 @@ fn build_kernel(
kconfig.hash(&mut image_id);
allocs.hash(&mut image_id);

let extern_regions = cfg.toml.kernel_extern_regions(image_name)?;
generate_kernel_linker_script(
"memory.x",
&allocs.kernel,
cfg.toml.kernel.stacksize.unwrap_or(DEFAULT_KERNEL_STACK),
&cfg.toml.all_regions("flash".to_string())?,
&extern_regions,
image_name,
)?;

Expand Down Expand Up @@ -1812,7 +1832,6 @@ fn generate_task_linker_script(

fn append_image_names(
linkscr: &mut std::fs::File,

images: &IndexMap<String, Range<u32>>,
image_name: &str,
) -> Result<()> {
Expand Down Expand Up @@ -1882,6 +1901,7 @@ fn generate_kernel_linker_script(
map: &BTreeMap<String, Range<u32>>,
stacksize: u32,
images: &IndexMap<String, Range<u32>>,
extern_regions: &IndexMap<String, Range<u32>>,
image_name: &str,
) -> Result<()> {
// Put the linker script somewhere the linker can find it
Expand Down Expand Up @@ -1948,6 +1968,7 @@ fn generate_kernel_linker_script(
.unwrap();

append_image_names(&mut linkscr, images, image_name)?;
append_extern_regions(&mut linkscr, extern_regions)?;
Ok(())
}

Expand Down Expand Up @@ -2859,6 +2880,11 @@ pub fn make_kconfig(
flat_shared.retain(|name, _v| used_shared_regions.contains(name.as_str()));

Ok(build_kconfig::KernelConfig {
features: toml.kernel.features.clone(),
extern_regions: toml
.kernel_extern_regions(image_name)?
.into_iter()
.collect(),
irqs,
tasks,
shared_regions: flat_shared,
Expand Down
13 changes: 13 additions & 0 deletions chips/stm32h7/memory-large.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,19 @@ read = true
write = true
execute = false # let's assume XN until proven otherwise

# Data tightly-coupled memory
#
# This region is used in production images to pass a token from the RoT to the
# SP indicating that measurement has happened. Using this region (and specific
# token values) is hard-coded in the `measurement-token` crate, which is shared
# between Hubris and Humility.
[[dtcm]]
address = 0x20000000
size = 131072
read = true
write = true
execute = false

# Network buffers are placed in sram1, which is directly accessible by the
# Ethernet MAC. We limit this use of sram1 to 64 KiB, and preserve the
# remainder to be used for disjoint purposes (e.g., as an external region).
Expand Down
1 change: 1 addition & 0 deletions drv/lpc55-swd/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ zerocopy = { workspace = true }
zerocopy-derive = { workspace = true }
bitflags = { workspace = true }
static_assertions = { workspace = true }
measurement-token = { workspace = true }

attest-api = { path = "../../task/attest-api" }
drv-lpc55-gpio-api = { path = "../lpc55-gpio-api" }
Expand Down
15 changes: 0 additions & 15 deletions drv/lpc55-swd/src/armv7debug.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,6 @@ pub trait DpAddressable {
const ADDRESS: u32;
}

// For keeping track of unwinding debug actions
bitflags! {
#[derive(PartialEq, Eq, Copy, Clone)]
pub struct Undo: u8 {
// Need self.swd_finish()
const SWD = 1 << 0;
// Need self.sp_reset_leave(true)
const RESET = 1 << 1;
// Need DEMCR = 0
const VC_CORERESET = 1 << 2;
// Need to clear debug enable.
const DEBUGEN = 1 << 3;
}
}

// RW 0x00000000 Debug Halting Control and Status Register
// Some DHCSR bits have different read vs. write meanings
// Specifically, the MAGIC value enables writing other control bits
Expand Down
Loading
Loading