From 5810d68e991ab6e3e3a8864c40988de39bfe38dd Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 21:14:54 +0200 Subject: [PATCH 01/16] orchestrator: Replace BootMonitor with checkpoint-embedded evidence checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A BootCheckpoint is timing policy plus its own evidence check: a capture-less fn handed the board's device context, so the channel underneath never leaks past the check and an unobservable checkpoint is unrepresentable. config.rs defines the schema (BootSignal is gone); the board table declares the checkpoints against its own context and error types. BootStatus stays as the shared vocabulary and absorbs the latch-cleared-by-reset contract; GpioBootMonitor keeps its behavior as a plain reader. BootWatch/WalkVerdict is the erased seam the orchestrator polls — timeout and retry-budget judgment lands with the walker that implements it. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- .../orchestrator/capabilities/BUILD.bazel | 3 +- .../capabilities/src/boot_control.rs | 2 +- .../capabilities/src/boot_monitor.rs | 180 ------------ .../capabilities/src/boot_status.rs | 36 +++ .../capabilities/src/boot_watch.rs | 127 +++++++++ services/orchestrator/capabilities/src/lib.rs | 30 +- services/orchestrator/config/BUILD.bazel | 1 + services/orchestrator/config/src/lib.rs | 262 ++++++++++++++---- .../hal-adapters/src/gpio_boot_monitor.rs | 40 +-- .../hal-adapters/src/hal_boot_control.rs | 3 +- services/orchestrator/hal-adapters/src/lib.rs | 8 +- target/mock/BUILD.bazel | 5 +- target/mock/devices.rs | 50 +++- 13 files changed, 457 insertions(+), 290 deletions(-) delete mode 100644 services/orchestrator/capabilities/src/boot_monitor.rs create mode 100644 services/orchestrator/capabilities/src/boot_status.rs create mode 100644 services/orchestrator/capabilities/src/boot_watch.rs diff --git a/services/orchestrator/capabilities/BUILD.bazel b/services/orchestrator/capabilities/BUILD.bazel index 7b873d202..3ff27349f 100644 --- a/services/orchestrator/capabilities/BUILD.bazel +++ b/services/orchestrator/capabilities/BUILD.bazel @@ -7,7 +7,8 @@ rust_library( name = "orchestrator_capabilities", srcs = [ "src/boot_control.rs", - "src/boot_monitor.rs", + "src/boot_status.rs", + "src/boot_watch.rs", "src/lib.rs", ], edition = "2024", diff --git a/services/orchestrator/capabilities/src/boot_control.rs b/services/orchestrator/capabilities/src/boot_control.rs index ae13cd57d..83f46ae6f 100644 --- a/services/orchestrator/capabilities/src/boot_control.rs +++ b/services/orchestrator/capabilities/src/boot_control.rs @@ -33,7 +33,7 @@ /// dev.hold_in_reset()?; /// store.set_trial(new_slot)?; // tentative boot selection — not yet committed /// dev.release()?; // boot the trial image -/// match monitor.await_boot(window)? { +/// match supervise_boot(window)? { /// Booted => store.commit(new_slot)?, // observed good => make it active /// Failed | Timeout => { /* nothing committed; previous slot still active */ } /// } diff --git a/services/orchestrator/capabilities/src/boot_monitor.rs b/services/orchestrator/capabilities/src/boot_monitor.rs deleted file mode 100644 index 969568801..000000000 --- a/services/orchestrator/capabilities/src/boot_monitor.rs +++ /dev/null @@ -1,180 +0,0 @@ -// Licensed under the Apache-2.0 license -// SPDX-License-Identifier: Apache-2.0 - -//! Observation capability: read a managed device's boot liveness. - -/// Liveness of a managed device's boot: Boot Confirmation only. -/// -/// Reports only that a device came up, never what booted; confirming the -/// running image is the one the RoT staged is attestation, a separate step. -/// `Failed` is optional device-reported evidence and never the only failure -/// path, since a hung device reports nothing — a stuck boot is caught by the -/// orchestrator's timeout, not by this enum. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum BootStatus { - /// Released, but boot completion not yet observed. - Booting, - /// Boot completion observed. - Booted, - /// Device reported a boot failure. - Failed, -} - -/// Observation capability: read a managed device's boot liveness. -/// -/// Pull-shaped: where the underlying signal is an edge or pulse, the interrupt -/// latches a flag beneath this seam and `boot_status` only reads it. No -/// callback registration, which would require allocation and invert control -/// into device implementations. -/// -/// The reported status must describe the **current** boot cycle. An -/// implementation backed by a latched signal must guarantee the latch is -/// cleared whenever the device re-enters reset, so evidence left over from a -/// previous boot never reads as [`BootStatus::Booted`]. This trait -/// deliberately has no re-arm operation: clearing is the reset path's job -/// (hardware tying the latch to the device's reset line, or the same platform -/// code that drives `BootControl`), not the observer's — a monitor that could -/// clear its own evidence would let a read race a reset. -pub trait BootMonitor { - /// The error type reported by this device's boot monitor. - /// - /// Requires [`core::error::Error`] (in `core` since Rust 1.81) so the - /// orchestrator gets `Display` and a `source()` cause chain, not just a - /// `Debug` dump. Error categories stay implementation-defined — this - /// crate names no error vocabulary of its own; a consumer that knows the - /// concrete adapter can recover its details by downcasting the - /// `&dyn core::error::Error`. - type Error: core::error::Error; - - /// Returns the current liveness of the device. - /// - /// Any given monitor may only ever produce a *subset* of [`BootStatus`], - /// depending on the signals it can access: a single ready pin yields only - /// `Booting`/`Booted`, while a fault-channel backend can also report - /// `Failed`. This is a capability difference between backends, not an - /// incomplete implementation. Consumers must still handle the full set — - /// they cannot know statically which backend they hold. - /// - /// # Errors - /// - /// Returns an error if the underlying liveness signal cannot be read. - fn boot_status(&self) -> Result; -} - -#[cfg(test)] -#[allow(clippy::bool_assert_comparison)] -mod tests { - use super::*; - use core::cell::Cell; - - // ── Trait contract ────────────────────────────────────────────────── - // MockMonitor implements the trait without any HAL dependency. If a - // HAL-specific bound sneaks back onto `Error`, this module stops - // compiling. - - struct MockMonitor { - ready_after: usize, - polls: Cell, - fail: bool, - } - - #[derive(Debug, PartialEq, Eq)] - struct MockFault; - - impl core::fmt::Display for MockFault { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - f.write_str("mock monitor fault") - } - } - - impl core::error::Error for MockFault {} - - impl BootMonitor for MockMonitor { - type Error = MockFault; - - fn boot_status(&self) -> Result { - if self.fail { - return Err(MockFault); - } - let polls = self.polls.get(); - self.polls.set(polls + 1); - Ok(if polls >= self.ready_after { - BootStatus::Booted - } else { - BootStatus::Booting - }) - } - } - - // A device that is still coming up reads Booting, then Booted once it - // is up. - #[test] - fn status_progresses_from_booting_to_booted() { - let mon = MockMonitor { - ready_after: 1, - polls: Cell::new(0), - fail: false, - }; - - assert_eq!( - mon.boot_status().expect("boot_status failed"), - BootStatus::Booting - ); - assert_eq!( - mon.boot_status().expect("boot_status failed"), - BootStatus::Booted - ); - } - - #[test] - fn errors_surface_through_the_generic_seam() { - let mon = MockMonitor { - ready_after: 0, - polls: Cell::new(0), - fail: true, - }; - - let err = comes_up_within(&mon, 1).expect_err("expected the monitor fault"); - - // Display comes from the core::error::Error bound, not a Debug dump. - assert_eq!(err.to_string(), "mock monitor fault"); - } - - // ── The orchestrator's future shape ───────────────────────────────── - // Usage examples for the future orchestrator, not API guarantees; move - // these to the orchestrator crate once it exists. - - /// Poll a monitor up to `poll_budget` times. `Booting` is not a failure; - /// `Ok(false)` means the budget ran out before the device came up. - fn comes_up_within(mon: &M, poll_budget: usize) -> Result { - for _ in 0..poll_budget { - if mon.boot_status()? == BootStatus::Booted { - return Ok(true); - } - } - Ok(false) - } - - // A device that comes up within the poll budget reads Booted. - #[test] - fn a_device_that_comes_up_within_budget_is_booted() { - let mon = MockMonitor { - ready_after: 2, - polls: Cell::new(0), - fail: false, - }; - - assert_eq!(comes_up_within(&mon, 5).expect("boot_status failed"), true); - } - - #[test] - fn a_device_that_never_comes_up_is_a_timeout_not_an_error() { - let mon = MockMonitor { - ready_after: usize::MAX, - polls: Cell::new(0), - fail: false, - }; - - assert_eq!(comes_up_within(&mon, 3).expect("boot_status failed"), false); - } -} diff --git a/services/orchestrator/capabilities/src/boot_status.rs b/services/orchestrator/capabilities/src/boot_status.rs new file mode 100644 index 000000000..a6f84f9c4 --- /dev/null +++ b/services/orchestrator/capabilities/src/boot_status.rs @@ -0,0 +1,36 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! Shared vocabulary for boot-liveness evidence. + +/// Liveness of a managed device's boot: Boot Confirmation only. +/// +/// Reports only that a device came up, never what booted; confirming the +/// running image is the one the RoT staged is attestation, a separate step. +/// `Failed` is optional device-reported evidence and never the only failure +/// path, since a hung device reports nothing — a stuck boot is caught by the +/// orchestrator's timeout, not by this enum. +/// +/// Any given evidence source may only ever produce a *subset* of these +/// statuses: a single ready pin yields only `Booting`/`Booted`, while a +/// fault-channel backend can also report `Failed`. That is a capability +/// difference between sources, not an incomplete implementation — consumers +/// must handle the full set. +/// +/// A status must describe the **current** boot cycle. Where the underlying +/// signal is an edge or pulse, it is latched beneath the read, and the latch +/// must be cleared whenever the device re-enters reset — by hardware tying +/// the latch to the device's reset line, or by the platform code that drives +/// `BootControl` — so evidence left over from a previous boot never reads as +/// [`Booted`](BootStatus::Booted). Clearing is deliberately the reset path's +/// job, not the reader's: a reader that could clear its own evidence would +/// let a read race a reset. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BootStatus { + /// Released, but boot completion not yet observed. + Booting, + /// Boot completion observed. + Booted, + /// Device reported a boot failure. + Failed, +} diff --git a/services/orchestrator/capabilities/src/boot_watch.rs b/services/orchestrator/capabilities/src/boot_watch.rs new file mode 100644 index 000000000..74b3213ad --- /dev/null +++ b/services/orchestrator/capabilities/src/boot_watch.rs @@ -0,0 +1,127 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! The orchestrator-facing seam of boot supervision. + +/// One device's boot walk, pollable without knowing the device type. +/// +/// Everything device-specific — the driver type, its error type, the +/// checkpoint list — stays inside the concrete walk; the orchestrator's +/// fleet view is uniform. Object-safe so a heterogeneous fleet can sit +/// behind `&mut dyn BootWatch`; a board preferring static dispatch wraps +/// its walks in an enum and matches, without touching anything below the +/// seam. +pub trait BootWatch { + /// Judges the walk at `now_millis` (monotonic). Never sleeps — time is + /// injected, so every decision is host-testable. + fn poll(&mut self, now_millis: u64) -> WalkVerdict; +} + +/// Everything the orchestrator needs to know about a boot walk. +/// +/// Deliberately free of device and error types: the orchestrator acts the +/// same whatever the cause, so the concrete detail is logged by the walk +/// while it is still in scope, not carried across the seam. +/// +/// Intentionally exhaustive (not `#[non_exhaustive]`): adding a verdict is +/// a breaking change, so the compiler forces every consumer — in particular +/// the orchestrator's event mapping — to handle it explicitly. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WalkVerdict { + /// Nothing to decide yet; poll again by `deadline_millis`. + Waiting { + /// When the awaited checkpoint's window expires. + deadline_millis: u64, + }, + /// Every checkpoint passed — the device is up. + Complete, + /// A window expired or the device reported failure, with retry budget + /// left; the window is re-armed. The caller re-resets the device and + /// keeps polling — what a retry re-runs is the caller's policy. + Retry { + /// The checkpoint that failed. + checkpoint: &'static str, + /// Attempts left after this one. + retries_left: u8, + }, + /// Retry budget exhausted — this boot is dead. Recovery is the + /// caller's move. + Dead { + /// The checkpoint the boot died at. + checkpoint: &'static str, + }, +} + +#[cfg(test)] +mod tests { + use super::*; + + // A BootWatch implemented against no walker at all — the seam must be + // satisfiable by anything that can produce verdicts, and must stay + // object-safe (the fleet array below fails to compile otherwise). + + struct ScriptedWalk { + verdicts: &'static [WalkVerdict], + next: usize, + } + + impl BootWatch for ScriptedWalk { + fn poll(&mut self, _now_millis: u64) -> WalkVerdict { + let v = self.verdicts[self.next]; + self.next += 1; + v + } + } + + #[test] + fn a_heterogeneous_fleet_pumps_through_the_erased_seam() { + let mut bmc = ScriptedWalk { + verdicts: &[ + WalkVerdict::Waiting { + deadline_millis: 90_000, + }, + WalkVerdict::Complete, + ], + next: 0, + }; + let mut nic = ScriptedWalk { + verdicts: &[ + WalkVerdict::Retry { + checkpoint: "heartbeat", + retries_left: 1, + }, + WalkVerdict::Dead { + checkpoint: "heartbeat", + }, + ], + next: 0, + }; + + let fleet: &mut [&mut dyn BootWatch] = &mut [&mut bmc, &mut nic]; + + let first: [WalkVerdict; 2] = [fleet[0].poll(0), fleet[1].poll(0)]; + let second: [WalkVerdict; 2] = [fleet[0].poll(1), fleet[1].poll(1)]; + + assert_eq!( + first, + [ + WalkVerdict::Waiting { + deadline_millis: 90_000 + }, + WalkVerdict::Retry { + checkpoint: "heartbeat", + retries_left: 1 + }, + ] + ); + assert_eq!( + second, + [ + WalkVerdict::Complete, + WalkVerdict::Dead { + checkpoint: "heartbeat" + }, + ] + ); + } +} diff --git a/services/orchestrator/capabilities/src/lib.rs b/services/orchestrator/capabilities/src/lib.rs index f5e7b3eb3..8b974e87b 100644 --- a/services/orchestrator/capabilities/src/lib.rs +++ b/services/orchestrator/capabilities/src/lib.rs @@ -7,23 +7,29 @@ //! single managed device's reset without knowing which controller line it //! maps to. //! -//! `BootMonitor` is the observation capability: the orchestrator reads a -//! device's boot liveness. +//! `BootStatus` is the shared vocabulary for boot-liveness evidence. There +//! is deliberately no observation *trait*: each `BootCheckpoint` a board +//! table declares (`DeviceConfig::checkpoints` in `orchestrator-config`) +//! carries its own evidence check, so how a signal is read stays inside the +//! check. +//! +//! `BootWatch` is the seam the orchestrator polls: one device's boot walk, +//! erased of every device-specific type, answering with a `WalkVerdict`. //! //! This crate is a dependency-free leaf: it holds the capability contracts, -//! and everything depends downward on it. Concrete adapters bind a trait to a -//! signal source and live in their own crates, so naming a capability never -//! drags in the stack behind it — the HAL-backed `HalBootControl` and -//! `GpioBootMonitor` are in `orchestrator-hal-adapters`; other backends (for -//! example an MCTP-ready `BootMonitor`) implement the same traits from their -//! own transport crate. The per-board device table schema lives in the -//! separate `orchestrator-config` crate; board tables -//! (`target//devices.rs`) declare the values. +//! and everything depends downward on it. Concrete adapters bind a capability +//! to a signal source and live in their own crates, so naming a capability +//! never drags in the stack behind it — the HAL-backed `HalBootControl` and +//! the `GpioBootMonitor` read helper are in `orchestrator-hal-adapters`. The +//! per-board device table schema lives in the separate `orchestrator-config` +//! crate; board tables (`target//devices.rs`) declare the values. #![cfg_attr(not(test), no_std)] mod boot_control; -mod boot_monitor; +mod boot_status; +mod boot_watch; pub use boot_control::BootControl; -pub use boot_monitor::{BootMonitor, BootStatus}; +pub use boot_status::BootStatus; +pub use boot_watch::{BootWatch, WalkVerdict}; diff --git a/services/orchestrator/config/BUILD.bazel b/services/orchestrator/config/BUILD.bazel index 55ad6b8d6..5093408c0 100644 --- a/services/orchestrator/config/BUILD.bazel +++ b/services/orchestrator/config/BUILD.bazel @@ -8,6 +8,7 @@ rust_library( srcs = ["src/lib.rs"], edition = "2024", visibility = ["//visibility:public"], + deps = ["//services/orchestrator/capabilities:orchestrator_capabilities"], ) # Host tests: build on the host platform, no kernel/QEMU. diff --git a/services/orchestrator/config/src/lib.rs b/services/orchestrator/config/src/lib.rs index 34aae93be..ca6d3208c 100644 --- a/services/orchestrator/config/src/lib.rs +++ b/services/orchestrator/config/src/lib.rs @@ -7,6 +7,8 @@ #![cfg_attr(not(test), no_std)] +use orchestrator_capabilities::BootStatus; + /// What the orchestrator requires before it commits a staged image. /// /// Intentionally exhaustive (not `#[non_exhaustive]`): adding a variant is @@ -21,65 +23,103 @@ pub enum CommitPolicy { LivenessAndAttestation, } -/// How the orchestrator observes a device's boot-progress signal. +/// One boot checkpoint: timing policy plus the evidence check itself. /// -/// Generic over the id type `G` the board's boot monitor uses to read a -/// boot-complete line, for the same reason `DeviceConfig` is generic over -/// its reset signal: signal ids are board-specific. +/// The check is handed the board's device context `D`, so the channel +/// underneath it (a GPIO line, a progress register, a message path) stays +/// inside the check and a checkpoint nothing can observe is +/// unrepresentable. /// -/// Intentionally exhaustive (not `#[non_exhaustive]`): adding a signal -/// kind is a breaking change, so every consumer that dispatches on it is -/// forced to handle the new kind explicitly. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum BootSignal { - /// The device raises a boot-complete GPIO line. - GpioBootComplete(G), - /// The device sends a heartbeat message. - Heartbeat, - /// The device's MCTP endpoint answers as ready. - MctpReady, - /// The device answers a firmware version query. - VersionQuery, -} - -/// One boot-progress checkpoint: a signal the orchestrator waits for, and -/// how long it waits. -#[derive(Debug, Clone, Copy)] -pub struct BootCheckpoint { - /// Names the checkpoint in timeout reports. +/// `passed` is a capture-less `fn` pointer rather than a closure: a table +/// of closures each capturing `&mut D` cannot exist, while the walker +/// holding the one `&mut D` and passing it in can — and capture-less +/// closures coerce to `fn` in const tables. The division of state: +/// per-checkpoint parameters belong in the `fn` body, per-device and +/// per-board state belongs in `D`. +pub struct BootCheckpoint { + /// Names the checkpoint in failure reports ("bl1", "kernel", …). pub name: &'static str, - pub signal: BootSignal, - /// How long the orchestrator waits for `signal` before it declares the - /// checkpoint — and the device's boot — failed. Expiry is the + /// Window for one attempt at this checkpoint. Expiry is the /// orchestrator's own judgment; hung devices report nothing. - pub window: core::time::Duration, + pub timeout: core::time::Duration, + /// Attempts allowed beyond the first before the failure is final. + pub max_retries: u8, + /// The evidence check. The status must describe the current boot + /// cycle — see [`BootStatus`] for the latching contract. + pub passed: fn(&mut D) -> Result, +} + +// Manual impls: deriving would demand `D: Clone`/`D: Debug` bounds the +// fields never need (`D` only appears behind the `fn` pointer). +impl Clone for BootCheckpoint { + fn clone(&self) -> Self { + *self + } +} + +impl Copy for BootCheckpoint {} + +impl core::fmt::Debug for BootCheckpoint { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("BootCheckpoint") + .field("name", &self.name) + .field("timeout", &self.timeout) + .field("max_retries", &self.max_retries) + .finish_non_exhaustive() + } } /// One managed downstream device, as declared by the board config. /// -/// Generic over the board's reset signal type `R`, which must match the +/// Generic over the board's reset signal type `R` (which must match the /// `ResetId` of the reset controller behind the board's `BootControl` -/// implementation — the compiler rejects a table whose ids the controller -/// cannot accept. +/// implementation), the board's device context `D` every evidence check +/// receives, and the board-wide check error `E` — one context and one +/// error type per table, both board-defined. /// /// Intentionally exhaustive (not `#[non_exhaustive]`): board tables /// construct this struct by literal, which the attribute would forbid. /// Adding a field is a breaking change that updates every board table. -#[derive(Debug, Clone, Copy)] -pub struct DeviceConfig { +pub struct DeviceConfig { pub name: &'static str, /// Reset signal id, passed to HalBootControl::new. pub reset_signal: R, - /// Boot-progress checkpoints, in the order the device passes them. - /// The device counts as booted when the last one is reached; a - /// checkpoint whose window expires fails the boot. - pub checkpoints: &'static [BootCheckpoint], + /// Boot checkpoints, in the order the device passes them. The device + /// counts as booted when the last one is reached; a checkpoint whose + /// window and retry budget are exhausted fails the boot. + pub checkpoints: &'static [BootCheckpoint], pub commit_policy: CommitPolicy, } +// Manual impls for the same reason as BootCheckpoint's: only `R` is held +// by value, so only `R` gets a bound. +impl Clone for DeviceConfig { + fn clone(&self) -> Self { + Self { + name: self.name, + reset_signal: self.reset_signal.clone(), + checkpoints: self.checkpoints, + commit_policy: self.commit_policy, + } + } +} + +impl Copy for DeviceConfig {} + +impl core::fmt::Debug for DeviceConfig { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("DeviceConfig") + .field("name", &self.name) + .field("reset_signal", &self.reset_signal) + .field("checkpoints", &self.checkpoints) + .field("commit_policy", &self.commit_policy) + .finish() + } +} + /// Checks a device table. Board configs call this in a const context so a /// bad table fails the build. -pub const fn validate(devices: &[DeviceConfig]) { +pub const fn validate(devices: &[DeviceConfig]) { let mut i = 0; while i < devices.len() { assert!(!devices[i].name.is_empty(), "device name must not be empty"); @@ -94,8 +134,8 @@ pub const fn validate(devices: &[DeviceConfig]) { "checkpoint name must not be empty" ); assert!( - !devices[i].checkpoints[c].window.is_zero(), - "checkpoint window must not be zero" + !devices[i].checkpoints[c].timeout.is_zero(), + "checkpoint timeout must not be zero" ); c += 1; } @@ -112,17 +152,77 @@ mod tests { // build error nobody can assert on. These tests call it at runtime to // prove the reject paths actually fire — a vacuous loop would pass // every `const _` check silently. + // + // The fixture is a staged-boot device: one monotonic progress register + // serves four checkpoints through one reader, and a poison value fails + // every one — the pattern a real SoC table is expected to use. + + const POISON: u8 = 0xFF; + + struct SocBoard { + level: u8, + fail: bool, + } + + #[derive(Debug, PartialEq, Eq)] + struct RegFault; + + impl core::fmt::Display for RegFault { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.write_str("progress register unreadable") + } + } + + impl core::error::Error for RegFault {} - const CHECKPOINT: BootCheckpoint = BootCheckpoint { - name: "boot-complete", - signal: BootSignal::GpioBootComplete(0), - window: Duration::from_secs(1), + impl SocBoard { + fn progress_at_least(&mut self, level: u8) -> Result { + if self.fail { + return Err(RegFault); + } + Ok(match self.level { + POISON => BootStatus::Failed, + l if l >= level => BootStatus::Booted, + _ => BootStatus::Booting, + }) + } + } + + // Named so the reject-path fixtures below can `..BL1` — an indexed + // `CHECKPOINTS[0]` would not promote to 'static. + const BL1: BootCheckpoint = BootCheckpoint { + name: "bl1", + timeout: Duration::from_millis(200), + max_retries: 0, + passed: |soc| soc.progress_at_least(1), }; - const DEVICE: DeviceConfig = DeviceConfig { - name: "dev", + const CHECKPOINTS: &[BootCheckpoint] = &[ + BL1, + BootCheckpoint { + name: "bl2", + timeout: Duration::from_secs(1), + max_retries: 0, + passed: |soc| soc.progress_at_least(2), + }, + BootCheckpoint { + name: "kernel", + timeout: Duration::from_secs(10), + max_retries: 2, + passed: |soc| soc.progress_at_least(3), + }, + BootCheckpoint { + name: "service", + timeout: Duration::from_secs(30), + max_retries: 2, + passed: |soc| soc.progress_at_least(4), + }, + ]; + + const DEVICE: DeviceConfig = DeviceConfig { + name: "soc", reset_signal: 0, - checkpoints: &[CHECKPOINT], + checkpoints: CHECKPOINTS, commit_policy: CommitPolicy::Liveness, }; @@ -150,26 +250,68 @@ mod tests { #[should_panic(expected = "checkpoint name must not be empty")] fn rejects_an_empty_checkpoint_name() { validate(&[DeviceConfig { - checkpoints: &[BootCheckpoint { - name: "", - ..CHECKPOINT - }], + checkpoints: &[BootCheckpoint { name: "", ..BL1 }], ..DEVICE }]); } #[test] - #[should_panic(expected = "checkpoint window must not be zero")] - fn rejects_a_zero_checkpoint_window() { + #[should_panic(expected = "checkpoint timeout must not be zero")] + fn rejects_a_zero_checkpoint_timeout() { validate(&[DeviceConfig { - checkpoints: &[ - CHECKPOINT, - BootCheckpoint { - window: Duration::ZERO, - ..CHECKPOINT - }, - ], + checkpoints: &[BootCheckpoint { + timeout: Duration::ZERO, + ..BL1 + }], ..DEVICE }]); } + + // One register, four checkpoints: each check sees exactly its own + // threshold, so a device mid-boot passes the early ones and not the + // late ones. + #[test] + fn checks_resolve_through_the_board_context() { + let mut soc = SocBoard { + level: 2, + fail: false, + }; + let read = + |soc: &mut SocBoard, i: usize| (CHECKPOINTS[i].passed)(soc).expect("check failed"); + + assert_eq!(read(&mut soc, 0), BootStatus::Booted); // bl1 + assert_eq!(read(&mut soc, 1), BootStatus::Booted); // bl2 + assert_eq!(read(&mut soc, 2), BootStatus::Booting); // kernel + assert_eq!(read(&mut soc, 3), BootStatus::Booting); // service + } + + // A poisoned register must read Failed from every checkpoint, whichever + // one the walk happens to be awaiting. + #[test] + fn a_poisoned_register_fails_every_checkpoint() { + let mut soc = SocBoard { + level: POISON, + fail: false, + }; + + for cp in CHECKPOINTS { + assert_eq!( + (cp.passed)(&mut soc).expect("check failed"), + BootStatus::Failed + ); + } + } + + #[test] + fn errors_surface_through_the_check() { + let mut soc = SocBoard { + level: 0, + fail: true, + }; + + let err = (CHECKPOINTS[0].passed)(&mut soc).expect_err("expected the register fault"); + + // Display comes from the core::error::Error bound, not a Debug dump. + assert_eq!(err.to_string(), "progress register unreadable"); + } } diff --git a/services/orchestrator/hal-adapters/src/gpio_boot_monitor.rs b/services/orchestrator/hal-adapters/src/gpio_boot_monitor.rs index a90cdcb24..3591d2b8d 100644 --- a/services/orchestrator/hal-adapters/src/gpio_boot_monitor.rs +++ b/services/orchestrator/hal-adapters/src/gpio_boot_monitor.rs @@ -1,19 +1,20 @@ // Licensed under the Apache-2.0 license // SPDX-License-Identifier: Apache-2.0 -//! HAL-backed [`BootMonitor`]: read a device's boot-complete signal off a GPIO -//! input line. +//! HAL-backed boot-status reader: read a device's boot-complete signal off a +//! GPIO input line into a [`BootStatus`]. use openprot_hal_blocking::gpio_port::{ ActivePolarity, GpioError, GpioErrorKind, GpioPort, PinMask, }; -use orchestrator_capabilities::{BootMonitor, BootStatus}; +use orchestrator_capabilities::BootStatus; /// Adapts any HAL GPIO error into a [`core::error::Error`]. /// /// GPIO ports keep implementing the HAL `GpioError`/`kind()` pattern /// unchanged; this wrapper supplies the `Display` and `core::error::Error` -/// machinery [`BootMonitor::Error`] requires, so no per-implementation work is +/// machinery the orchestrator expects of boot-evidence errors, so no +/// per-implementation work is /// needed. The underlying category stays reachable via [`MonitorError::kind`], /// and the concrete HAL error through the /// [`source()`](core::error::Error::source) chain, downcast to @@ -79,15 +80,15 @@ impl From for MonitorError { /// ready signals routinely share one). Platform configuration keeps the bank /// alive for as long as its monitors. /// -/// A single ready line can only ever answer "up yet?", so this backend +/// A single ready line can only ever answer "up yet?", so this reader /// reports the [`BootStatus::Booting`]/[`BootStatus::Booted`] subset — see -/// [`BootMonitor::boot_status`] on why that is a capability difference, not -/// an incomplete implementation. +/// [`BootStatus`] on why that is a capability difference, not an incomplete +/// implementation. /// /// Where a hardware latch is used, the platform must clear it whenever the /// device re-enters reset (typically by wiring the latch's clear to the -/// device's reset line) — [`BootMonitor`] requires that evidence from a -/// previous boot never reads as [`BootStatus::Booted`], and this adapter only +/// device's reset line) — [`BootStatus`] requires that evidence from a +/// previous boot never reads as [`BootStatus::Booted`], and this reader only /// reads the line, it cannot re-arm it. /// /// [`HalBootControl`]: crate::HalBootControl @@ -128,16 +129,16 @@ impl<'a, P: GpioPort> GpioBootMonitor<'a, P> { // `P::Error: 'static` because `source()` hands out `&(dyn Error + 'static)` // referencing the wrapped HAL error. Error types are plain data; this costs // no real implementation anything. -impl BootMonitor for GpioBootMonitor<'_, P> +impl GpioBootMonitor<'_, P> where P::Error: 'static, { - type Error = MonitorError; - + /// Returns the current liveness of the device. + /// /// # Errors /// /// Propagates any error returned by the port's `read_input`. - fn boot_status(&self) -> Result { + pub fn boot_status(&self) -> Result> { let high = self.port.read_input()?.contains(self.ready_pin); let booted = match self.active { ActivePolarity::ActiveHigh => high, @@ -156,7 +157,8 @@ mod tests { use super::*; use openprot_hal_blocking::gpio_port::GpioErrorType; - // BMC boot-complete on line 4. Normally set in config.rs. + // BMC boot-complete on line 4. Everything that is config is normally + // declared in the board device table (`target//devices.rs`). const BMC_READY: Mask = Mask(1 << 4); /// Bitmask over a single mock GPIO bank. @@ -238,15 +240,15 @@ mod tests { } fn configure(&mut self, _: Mask, _: ()) -> Result<(), MockError> { - panic!("BootMonitor must never configure pins"); + panic!("the boot-status reader must never configure pins"); } fn set_reset(&mut self, _: Mask, _: Mask) -> Result<(), MockError> { - panic!("BootMonitor must never drive outputs"); + panic!("the boot-status reader must never drive outputs"); } fn toggle(&mut self, _: Mask) -> Result<(), MockError> { - panic!("BootMonitor must never drive outputs"); + panic!("the boot-status reader must never drive outputs"); } } @@ -297,9 +299,9 @@ mod tests { GpioBootMonitor::new(&port, Mask::empty(), ActivePolarity::ActiveHigh); } - // A controller error surfaces through BootMonitor unchanged. + // A controller error surfaces through the reader unchanged. #[test] - fn port_error_propagates_through_boot_monitor() { + fn port_error_propagates_through_the_reader() { let port = MockGpioPort::failing(GpioErrorKind::HardwareFailure); let mon = GpioBootMonitor::new(&port, BMC_READY, ActivePolarity::ActiveHigh); diff --git a/services/orchestrator/hal-adapters/src/hal_boot_control.rs b/services/orchestrator/hal-adapters/src/hal_boot_control.rs index f7eb93d23..2f351a52b 100644 --- a/services/orchestrator/hal-adapters/src/hal_boot_control.rs +++ b/services/orchestrator/hal-adapters/src/hal_boot_control.rs @@ -74,7 +74,8 @@ mod tests { use core::time::Duration; use openprot_hal_blocking::system_control::{Error as HalError, ErrorKind, ErrorType}; - // Normally set in config.rs + // Everything that is config is normally declared in the board device + // table (`target//devices.rs`). const BMC_LINE: u8 = 7; #[derive(Debug, PartialEq, Eq, Clone, Copy)] diff --git a/services/orchestrator/hal-adapters/src/lib.rs b/services/orchestrator/hal-adapters/src/lib.rs index 4f71ebdb2..3c9d0c590 100644 --- a/services/orchestrator/hal-adapters/src/lib.rs +++ b/services/orchestrator/hal-adapters/src/lib.rs @@ -3,10 +3,10 @@ //! HAL-backed adapters for the Boot Orchestrator capability traits. //! -//! Each type here implements a capability trait from `orchestrator-capabilities` -//! against a HAL-blocking trait: [`HalBootControl`] drives `BootControl` over a -//! `ResetControl` line, and [`GpioBootMonitor`] reads `BootMonitor` off a -//! `GpioPort` input line. Adapters live in this crate — not in the leaf +//! Each type here binds an orchestrator-facing seam to a HAL-blocking trait: +//! [`HalBootControl`] drives `BootControl` over a `ResetControl` line, and +//! [`GpioBootMonitor`] reads a `GpioPort` input line into a `BootStatus`. +//! Adapters live in this crate — not in the leaf //! `orchestrator-capabilities` — so that depending on a capability contract //! never pulls in the HAL. A transport-backed adapter belongs in its own crate //! depending on its own stack, by the same rule. diff --git a/target/mock/BUILD.bazel b/target/mock/BUILD.bazel index a4dbf970f..751ebde4d 100644 --- a/target/mock/BUILD.bazel +++ b/target/mock/BUILD.bazel @@ -10,5 +10,8 @@ rust_library( srcs = ["devices.rs"], crate_name = "board_devices", edition = "2024", - deps = ["//services/orchestrator/config:orchestrator_config"], + deps = [ + "//services/orchestrator/capabilities:orchestrator_capabilities", + "//services/orchestrator/config:orchestrator_config", + ], ) diff --git a/target/mock/devices.rs b/target/mock/devices.rs index e5c3af88e..aa484c512 100644 --- a/target/mock/devices.rs +++ b/target/mock/devices.rs @@ -7,16 +7,40 @@ #![no_std] +use core::convert::Infallible; use core::time::Duration; -use orchestrator_config::{BootCheckpoint, BootSignal, CommitPolicy, DeviceConfig}; +use orchestrator_capabilities::BootStatus; +use orchestrator_config::{BootCheckpoint, CommitPolicy, DeviceConfig}; + +/// The mock board's device context: the signal state every checkpoint +/// check reads. Stands in for real drivers until the mock platform grows +/// them; the reset path is responsible for clearing latched fields (see +/// `BootStatus`). +#[derive(Debug, Default)] +pub struct MockBoard { + /// bmc boot-complete line. + pub bmc_ready: bool, + /// nic MCTP endpoint answers as ready. + pub nic_mctp_ready: bool, + /// nic heartbeat observed (latched). + pub nic_heartbeat: bool, +} + +const fn up(ready: bool) -> BootStatus { + if ready { + BootStatus::Booted + } else { + BootStatus::Booting + } +} /// Declaration order is the boot order: the orchestrator releases devices /// top to bottom, one at a time. /// -/// The mock board's reset controller and boot monitor both address -/// signals by plain index, so both id types are `u8`. -pub const MANAGED_DEVICES: &[DeviceConfig] = &[ +/// The mock board's reset controller addresses reset lines by plain index, +/// so the reset id type is `u8`. +pub const MANAGED_DEVICES: &[DeviceConfig] = &[ // Direct-flash SPI device (BMC archetype): the eRoT fronts its flash. // Single checkpoint: it raises a boot-complete GPIO. DeviceConfig { @@ -24,26 +48,30 @@ pub const MANAGED_DEVICES: &[DeviceConfig] = &[ reset_signal: 7, checkpoints: &[BootCheckpoint { name: "boot-complete", - signal: BootSignal::GpioBootComplete(12), - window: Duration::from_secs(90), + timeout: Duration::from_secs(90), + max_retries: 1, + passed: |b| Ok(up(b.bmc_ready)), }], commit_policy: CommitPolicy::Liveness, }, // PLDM device (NIC archetype): self-updating, SPDM-capable. Two - // checkpoints, exercising the multi-checkpoint path. + // checkpoints, exercising the multi-checkpoint path: transport up + // first, then proof the workload is alive. DeviceConfig { name: "nic", reset_signal: 3, checkpoints: &[ BootCheckpoint { name: "mctp-ready", - signal: BootSignal::MctpReady, - window: Duration::from_secs(20), + timeout: Duration::from_secs(20), + max_retries: 2, + passed: |b| Ok(up(b.nic_mctp_ready)), }, BootCheckpoint { name: "heartbeat", - signal: BootSignal::Heartbeat, - window: Duration::from_secs(10), + timeout: Duration::from_secs(10), + max_retries: 0, + passed: |b| Ok(up(b.nic_heartbeat)), }, ], commit_policy: CommitPolicy::LivenessAndAttestation, From e4b198a7e7f607ddf6ec28599d3e3ead35440602 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 21:32:32 +0200 Subject: [PATCH 02/16] orchestrator: Defunctionalize evidence checks into board-defined signal ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The embedded fn was the more general shape, but the generality went unused while its costs did not: the table stopped being pure data (unprintable, unvalidatable on mechanisms, never generatable), every check shared one &mut board context, and dispatch went indirect. A signal id is the same check defunctionalized: data in the table, an exhaustive match in the board's EvidenceReader — typically one per device, so each walk borrows only its own reader. Boot-evidence mechanisms per board are a closed set; when one can't be named, that is a new variant in that board's enum, not an API change. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- .../orchestrator/capabilities/BUILD.bazel | 1 + .../orchestrator/capabilities/src/evidence.rs | 139 +++++++++++ services/orchestrator/capabilities/src/lib.rs | 12 +- services/orchestrator/config/BUILD.bazel | 1 - services/orchestrator/config/src/lib.rs | 223 +++--------------- target/mock/BUILD.bazel | 5 +- target/mock/devices.rs | 41 ++-- 7 files changed, 199 insertions(+), 223 deletions(-) create mode 100644 services/orchestrator/capabilities/src/evidence.rs diff --git a/services/orchestrator/capabilities/BUILD.bazel b/services/orchestrator/capabilities/BUILD.bazel index 3ff27349f..09e66160d 100644 --- a/services/orchestrator/capabilities/BUILD.bazel +++ b/services/orchestrator/capabilities/BUILD.bazel @@ -9,6 +9,7 @@ rust_library( "src/boot_control.rs", "src/boot_status.rs", "src/boot_watch.rs", + "src/evidence.rs", "src/lib.rs", ], edition = "2024", diff --git a/services/orchestrator/capabilities/src/evidence.rs b/services/orchestrator/capabilities/src/evidence.rs new file mode 100644 index 000000000..5515c4caa --- /dev/null +++ b/services/orchestrator/capabilities/src/evidence.rs @@ -0,0 +1,139 @@ +// Licensed under the Apache-2.0 license +// SPDX-License-Identifier: Apache-2.0 + +//! Evidence reading: resolve a board-defined signal id to boot liveness. + +use crate::BootStatus; + +/// Reads a device's boot evidence, one signal at a time. +/// +/// Implemented by board wiring — typically once per managed device, so +/// each device's boot walk borrows only its own reader. `G` is the +/// board's signal vocabulary; an exhaustive `match` on it keeps dispatch +/// direct and makes a forgotten signal a compile error, not a runtime +/// hole. +/// +/// The status must describe the **current** boot cycle — see +/// [`BootStatus`] for the latching contract (evidence is cleared by the +/// reset path, never by the reader). +pub trait EvidenceReader { + /// The error type reported by this reader. + /// + /// Requires [`core::error::Error`] (in `core` since Rust 1.81) so the + /// orchestrator gets `Display` and a `source()` cause chain, not just + /// a `Debug` dump. Error categories stay implementation-defined — + /// this crate names no error vocabulary of its own. + type Error: core::error::Error; + + /// Returns the current liveness evidence for `signal`. + /// + /// # Errors + /// + /// Returns an error if the evidence channel behind `signal` cannot be + /// read. + fn read(&mut self, signal: &G) -> Result; +} + +#[cfg(test)] +mod tests { + use super::*; + + // A reader implemented against no HAL at all — the contract must be + // satisfiable from any stack. One monotonic progress register serves + // four staged-boot signals through one reader (the pattern a real SoC + // board is expected to use); a poison value fails every signal. + + const POISON: u8 = 0xFF; + + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + enum TestSignal { + /// Booted once the progress register reaches this level + /// (1 = bl1, 2 = bl2, 3 = kernel, 4 = service). + Progress(u8), + } + + struct SocReader { + level: u8, + fail: bool, + } + + #[derive(Debug, PartialEq, Eq)] + struct RegFault; + + impl core::fmt::Display for RegFault { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.write_str("progress register unreadable") + } + } + + impl core::error::Error for RegFault {} + + impl EvidenceReader for SocReader { + type Error = RegFault; + + fn read(&mut self, signal: &TestSignal) -> Result { + if self.fail { + return Err(RegFault); + } + let TestSignal::Progress(threshold) = *signal; + Ok(match self.level { + POISON => BootStatus::Failed, + l if l >= threshold => BootStatus::Booted, + _ => BootStatus::Booting, + }) + } + } + + // One register, four signals: each read sees exactly its own + // threshold, so a device mid-boot passes the early stages and not the + // late ones. + #[test] + fn one_reader_serves_a_staged_boot() { + let mut soc = SocReader { + level: 2, + fail: false, + }; + let mut read = |threshold| { + soc.read(&TestSignal::Progress(threshold)) + .expect("read failed") + }; + + assert_eq!(read(1), BootStatus::Booted); // bl1 + assert_eq!(read(2), BootStatus::Booted); // bl2 + assert_eq!(read(3), BootStatus::Booting); // kernel + assert_eq!(read(4), BootStatus::Booting); // service + } + + // A poisoned register must read Failed for every signal, whichever + // stage the walk happens to be awaiting. + #[test] + fn a_poisoned_register_fails_every_signal() { + let mut soc = SocReader { + level: POISON, + fail: false, + }; + + for threshold in 1..=4 { + assert_eq!( + soc.read(&TestSignal::Progress(threshold)) + .expect("read failed"), + BootStatus::Failed + ); + } + } + + #[test] + fn errors_surface_through_the_reader() { + let mut soc = SocReader { + level: 0, + fail: true, + }; + + let err = soc + .read(&TestSignal::Progress(1)) + .expect_err("expected the register fault"); + + // Display comes from the core::error::Error bound, not a Debug dump. + assert_eq!(err.to_string(), "progress register unreadable"); + } +} diff --git a/services/orchestrator/capabilities/src/lib.rs b/services/orchestrator/capabilities/src/lib.rs index 8b974e87b..8a6557453 100644 --- a/services/orchestrator/capabilities/src/lib.rs +++ b/services/orchestrator/capabilities/src/lib.rs @@ -7,11 +7,11 @@ //! single managed device's reset without knowing which controller line it //! maps to. //! -//! `BootStatus` is the shared vocabulary for boot-liveness evidence. There -//! is deliberately no observation *trait*: each `BootCheckpoint` a board -//! table declares (`DeviceConfig::checkpoints` in `orchestrator-config`) -//! carries its own evidence check, so how a signal is read stays inside the -//! check. +//! `BootStatus` is the shared vocabulary for boot-liveness evidence, and +//! `EvidenceReader` resolves a board-defined signal id to it. The schema +//! names no signal kinds: each board's device table declares its +//! checkpoints as data (`BootCheckpoint` in `orchestrator-config`), and the +//! board's reader gives the ids meaning. //! //! `BootWatch` is the seam the orchestrator polls: one device's boot walk, //! erased of every device-specific type, answering with a `WalkVerdict`. @@ -29,7 +29,9 @@ mod boot_control; mod boot_status; mod boot_watch; +mod evidence; pub use boot_control::BootControl; pub use boot_status::BootStatus; pub use boot_watch::{BootWatch, WalkVerdict}; +pub use evidence::EvidenceReader; diff --git a/services/orchestrator/config/BUILD.bazel b/services/orchestrator/config/BUILD.bazel index 5093408c0..55ad6b8d6 100644 --- a/services/orchestrator/config/BUILD.bazel +++ b/services/orchestrator/config/BUILD.bazel @@ -8,7 +8,6 @@ rust_library( srcs = ["src/lib.rs"], edition = "2024", visibility = ["//visibility:public"], - deps = ["//services/orchestrator/capabilities:orchestrator_capabilities"], ) # Host tests: build on the host platform, no kernel/QEMU. diff --git a/services/orchestrator/config/src/lib.rs b/services/orchestrator/config/src/lib.rs index ca6d3208c..f3c8b0213 100644 --- a/services/orchestrator/config/src/lib.rs +++ b/services/orchestrator/config/src/lib.rs @@ -7,8 +7,6 @@ #![cfg_attr(not(test), no_std)] -use orchestrator_capabilities::BootStatus; - /// What the orchestrator requires before it commits a staged image. /// /// Intentionally exhaustive (not `#[non_exhaustive]`): adding a variant is @@ -23,103 +21,58 @@ pub enum CommitPolicy { LivenessAndAttestation, } -/// One boot checkpoint: timing policy plus the evidence check itself. -/// -/// The check is handed the board's device context `D`, so the channel -/// underneath it (a GPIO line, a progress register, a message path) stays -/// inside the check and a checkpoint nothing can observe is -/// unrepresentable. +/// One boot checkpoint: a signal the orchestrator waits for, how long it +/// waits per attempt, and how many failed attempts it tolerates. /// -/// `passed` is a capture-less `fn` pointer rather than a closure: a table -/// of closures each capturing `&mut D` cannot exist, while the walker -/// holding the one `&mut D` and passing it in can — and capture-less -/// closures coerce to `fn` in const tables. The division of state: -/// per-checkpoint parameters belong in the `fn` body, per-device and -/// per-board state belongs in `D`. -pub struct BootCheckpoint { +/// `signal` is a board-defined id — the schema attaches no meaning to it +/// and names no signal kinds. Each board defines its own vocabulary (a +/// small enum: a GPIO line, a progress-register threshold, a message-path +/// readiness) and gives it meaning in its `EvidenceReader`. The id is a +/// defunctionalized evidence check: data in the table instead of a +/// function, so the table stays printable, comparable, const-checkable — +/// and could one day be generated instead of written. +#[derive(Debug, Clone, Copy)] +pub struct BootCheckpoint { /// Names the checkpoint in failure reports ("bl1", "kernel", …). pub name: &'static str, + /// Board-defined signal id, resolved by the board's `EvidenceReader`. + pub signal: G, /// Window for one attempt at this checkpoint. Expiry is the /// orchestrator's own judgment; hung devices report nothing. pub timeout: core::time::Duration, /// Attempts allowed beyond the first before the failure is final. pub max_retries: u8, - /// The evidence check. The status must describe the current boot - /// cycle — see [`BootStatus`] for the latching contract. - pub passed: fn(&mut D) -> Result, -} - -// Manual impls: deriving would demand `D: Clone`/`D: Debug` bounds the -// fields never need (`D` only appears behind the `fn` pointer). -impl Clone for BootCheckpoint { - fn clone(&self) -> Self { - *self - } -} - -impl Copy for BootCheckpoint {} - -impl core::fmt::Debug for BootCheckpoint { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - f.debug_struct("BootCheckpoint") - .field("name", &self.name) - .field("timeout", &self.timeout) - .field("max_retries", &self.max_retries) - .finish_non_exhaustive() - } } /// One managed downstream device, as declared by the board config. /// /// Generic over the board's reset signal type `R` (which must match the /// `ResetId` of the reset controller behind the board's `BootControl` -/// implementation), the board's device context `D` every evidence check -/// receives, and the board-wide check error `E` — one context and one -/// error type per table, both board-defined. +/// implementation) and its boot-signal vocabulary `G`, for the same +/// reason: signal ids are board-specific. /// /// Intentionally exhaustive (not `#[non_exhaustive]`): board tables /// construct this struct by literal, which the attribute would forbid. /// Adding a field is a breaking change that updates every board table. -pub struct DeviceConfig { +#[derive(Debug, Clone, Copy)] +pub struct DeviceConfig { pub name: &'static str, /// Reset signal id, passed to HalBootControl::new. pub reset_signal: R, /// Boot checkpoints, in the order the device passes them. The device /// counts as booted when the last one is reached; a checkpoint whose /// window and retry budget are exhausted fails the boot. - pub checkpoints: &'static [BootCheckpoint], + pub checkpoints: &'static [BootCheckpoint], pub commit_policy: CommitPolicy, } -// Manual impls for the same reason as BootCheckpoint's: only `R` is held -// by value, so only `R` gets a bound. -impl Clone for DeviceConfig { - fn clone(&self) -> Self { - Self { - name: self.name, - reset_signal: self.reset_signal.clone(), - checkpoints: self.checkpoints, - commit_policy: self.commit_policy, - } - } -} - -impl Copy for DeviceConfig {} - -impl core::fmt::Debug for DeviceConfig { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - f.debug_struct("DeviceConfig") - .field("name", &self.name) - .field("reset_signal", &self.reset_signal) - .field("checkpoints", &self.checkpoints) - .field("commit_policy", &self.commit_policy) - .finish() - } -} - /// Checks a device table. Board configs call this in a const context so a /// bad table fails the build. -pub const fn validate(devices: &[DeviceConfig]) { +/// +/// Only schema-shape checks are possible here; checks on the board's own +/// types (signal ranges, uniqueness) belong next to the table that defines +/// their meaning, in a board-local `const fn` run alongside this one. +pub const fn validate(devices: &[DeviceConfig]) { let mut i = 0; while i < devices.len() { assert!(!devices[i].name.is_empty(), "device name must not be empty"); @@ -152,77 +105,18 @@ mod tests { // build error nobody can assert on. These tests call it at runtime to // prove the reject paths actually fire — a vacuous loop would pass // every `const _` check silently. - // - // The fixture is a staged-boot device: one monotonic progress register - // serves four checkpoints through one reader, and a poison value fails - // every one — the pattern a real SoC table is expected to use. - const POISON: u8 = 0xFF; - - struct SocBoard { - level: u8, - fail: bool, - } - - #[derive(Debug, PartialEq, Eq)] - struct RegFault; - - impl core::fmt::Display for RegFault { - fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - f.write_str("progress register unreadable") - } - } - - impl core::error::Error for RegFault {} - - impl SocBoard { - fn progress_at_least(&mut self, level: u8) -> Result { - if self.fail { - return Err(RegFault); - } - Ok(match self.level { - POISON => BootStatus::Failed, - l if l >= level => BootStatus::Booted, - _ => BootStatus::Booting, - }) - } - } - - // Named so the reject-path fixtures below can `..BL1` — an indexed - // `CHECKPOINTS[0]` would not promote to 'static. - const BL1: BootCheckpoint = BootCheckpoint { - name: "bl1", - timeout: Duration::from_millis(200), - max_retries: 0, - passed: |soc| soc.progress_at_least(1), + const CHECKPOINT: BootCheckpoint = BootCheckpoint { + name: "boot-complete", + signal: 0, + timeout: Duration::from_secs(1), + max_retries: 1, }; - const CHECKPOINTS: &[BootCheckpoint] = &[ - BL1, - BootCheckpoint { - name: "bl2", - timeout: Duration::from_secs(1), - max_retries: 0, - passed: |soc| soc.progress_at_least(2), - }, - BootCheckpoint { - name: "kernel", - timeout: Duration::from_secs(10), - max_retries: 2, - passed: |soc| soc.progress_at_least(3), - }, - BootCheckpoint { - name: "service", - timeout: Duration::from_secs(30), - max_retries: 2, - passed: |soc| soc.progress_at_least(4), - }, - ]; - - const DEVICE: DeviceConfig = DeviceConfig { - name: "soc", + const DEVICE: DeviceConfig = DeviceConfig { + name: "dev", reset_signal: 0, - checkpoints: CHECKPOINTS, + checkpoints: &[CHECKPOINT], commit_policy: CommitPolicy::Liveness, }; @@ -250,7 +144,10 @@ mod tests { #[should_panic(expected = "checkpoint name must not be empty")] fn rejects_an_empty_checkpoint_name() { validate(&[DeviceConfig { - checkpoints: &[BootCheckpoint { name: "", ..BL1 }], + checkpoints: &[BootCheckpoint { + name: "", + ..CHECKPOINT + }], ..DEVICE }]); } @@ -261,57 +158,9 @@ mod tests { validate(&[DeviceConfig { checkpoints: &[BootCheckpoint { timeout: Duration::ZERO, - ..BL1 + ..CHECKPOINT }], ..DEVICE }]); } - - // One register, four checkpoints: each check sees exactly its own - // threshold, so a device mid-boot passes the early ones and not the - // late ones. - #[test] - fn checks_resolve_through_the_board_context() { - let mut soc = SocBoard { - level: 2, - fail: false, - }; - let read = - |soc: &mut SocBoard, i: usize| (CHECKPOINTS[i].passed)(soc).expect("check failed"); - - assert_eq!(read(&mut soc, 0), BootStatus::Booted); // bl1 - assert_eq!(read(&mut soc, 1), BootStatus::Booted); // bl2 - assert_eq!(read(&mut soc, 2), BootStatus::Booting); // kernel - assert_eq!(read(&mut soc, 3), BootStatus::Booting); // service - } - - // A poisoned register must read Failed from every checkpoint, whichever - // one the walk happens to be awaiting. - #[test] - fn a_poisoned_register_fails_every_checkpoint() { - let mut soc = SocBoard { - level: POISON, - fail: false, - }; - - for cp in CHECKPOINTS { - assert_eq!( - (cp.passed)(&mut soc).expect("check failed"), - BootStatus::Failed - ); - } - } - - #[test] - fn errors_surface_through_the_check() { - let mut soc = SocBoard { - level: 0, - fail: true, - }; - - let err = (CHECKPOINTS[0].passed)(&mut soc).expect_err("expected the register fault"); - - // Display comes from the core::error::Error bound, not a Debug dump. - assert_eq!(err.to_string(), "progress register unreadable"); - } } diff --git a/target/mock/BUILD.bazel b/target/mock/BUILD.bazel index 751ebde4d..a4dbf970f 100644 --- a/target/mock/BUILD.bazel +++ b/target/mock/BUILD.bazel @@ -10,8 +10,5 @@ rust_library( srcs = ["devices.rs"], crate_name = "board_devices", edition = "2024", - deps = [ - "//services/orchestrator/capabilities:orchestrator_capabilities", - "//services/orchestrator/config:orchestrator_config", - ], + deps = ["//services/orchestrator/config:orchestrator_config"], ) diff --git a/target/mock/devices.rs b/target/mock/devices.rs index aa484c512..0b7d29d55 100644 --- a/target/mock/devices.rs +++ b/target/mock/devices.rs @@ -7,32 +7,21 @@ #![no_std] -use core::convert::Infallible; use core::time::Duration; -use orchestrator_capabilities::BootStatus; use orchestrator_config::{BootCheckpoint, CommitPolicy, DeviceConfig}; -/// The mock board's device context: the signal state every checkpoint -/// check reads. Stands in for real drivers until the mock platform grows -/// them; the reset path is responsible for clearing latched fields (see -/// `BootStatus`). -#[derive(Debug, Default)] -pub struct MockBoard { - /// bmc boot-complete line. - pub bmc_ready: bool, - /// nic MCTP endpoint answers as ready. - pub nic_mctp_ready: bool, - /// nic heartbeat observed (latched). - pub nic_heartbeat: bool, -} - -const fn up(ready: bool) -> BootStatus { - if ready { - BootStatus::Booted - } else { - BootStatus::Booting - } +/// The mock board's boot-signal vocabulary. The schema carries these +/// opaquely; only this board's `EvidenceReader` gives them meaning. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum MockSignal { + /// A boot-complete GPIO line, by index. + Gpio(u8), + /// The device's MCTP endpoint answers as ready. + MctpReady, + /// The device sends a heartbeat message (latched; the reset path + /// clears it). + Heartbeat, } /// Declaration order is the boot order: the orchestrator releases devices @@ -40,7 +29,7 @@ const fn up(ready: bool) -> BootStatus { /// /// The mock board's reset controller addresses reset lines by plain index, /// so the reset id type is `u8`. -pub const MANAGED_DEVICES: &[DeviceConfig] = &[ +pub const MANAGED_DEVICES: &[DeviceConfig] = &[ // Direct-flash SPI device (BMC archetype): the eRoT fronts its flash. // Single checkpoint: it raises a boot-complete GPIO. DeviceConfig { @@ -48,9 +37,9 @@ pub const MANAGED_DEVICES: &[DeviceConfig] = &[ reset_signal: 7, checkpoints: &[BootCheckpoint { name: "boot-complete", + signal: MockSignal::Gpio(12), timeout: Duration::from_secs(90), max_retries: 1, - passed: |b| Ok(up(b.bmc_ready)), }], commit_policy: CommitPolicy::Liveness, }, @@ -63,15 +52,15 @@ pub const MANAGED_DEVICES: &[DeviceConfig] = &[ checkpoints: &[ BootCheckpoint { name: "mctp-ready", + signal: MockSignal::MctpReady, timeout: Duration::from_secs(20), max_retries: 2, - passed: |b| Ok(up(b.nic_mctp_ready)), }, BootCheckpoint { name: "heartbeat", + signal: MockSignal::Heartbeat, timeout: Duration::from_secs(10), max_retries: 0, - passed: |b| Ok(up(b.nic_heartbeat)), }, ], commit_policy: CommitPolicy::LivenessAndAttestation, From 0cf81e50a21634ca9442d624a3e7b3d5078add06 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 21:47:42 +0200 Subject: [PATCH 03/16] orchestrator: Let devices report failure and its retriability as evidence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A device that knows it failed should end the wait early, and one that knows a retry is pointless should say so, instead of the orchestrator burning its window and budget to find out. BootStatus::Failed splits into FailedRetriable (consumes budget immediately) and FailedFatal (ends the boot regardless of budget). Timeouts stay the orchestrator's own judgment — hung devices report nothing — and channel trouble stays in the reader's Error, distinct from a device-reported verdict. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- .../capabilities/src/boot_status.rs | 25 ++++++++++----- .../capabilities/src/boot_watch.rs | 13 +++++--- .../orchestrator/capabilities/src/evidence.rs | 32 +++++++++++++++---- 3 files changed, 51 insertions(+), 19 deletions(-) diff --git a/services/orchestrator/capabilities/src/boot_status.rs b/services/orchestrator/capabilities/src/boot_status.rs index a6f84f9c4..3936356de 100644 --- a/services/orchestrator/capabilities/src/boot_status.rs +++ b/services/orchestrator/capabilities/src/boot_status.rs @@ -7,15 +7,18 @@ /// /// Reports only that a device came up, never what booted; confirming the /// running image is the one the RoT staged is attestation, a separate step. -/// `Failed` is optional device-reported evidence and never the only failure -/// path, since a hung device reports nothing — a stuck boot is caught by the -/// orchestrator's timeout, not by this enum. +/// The failure variants are optional device-reported evidence and never the +/// only failure path, since a hung device reports nothing — a stuck boot is +/// caught by the orchestrator's timeout, not by this enum. What they buy is +/// speed and judgment: a device that knows it failed ends the wait early, +/// and a device that knows a retry is pointless says so, instead of the +/// orchestrator burning its window and retry budget to find out. /// /// Any given evidence source may only ever produce a *subset* of these /// statuses: a single ready pin yields only `Booting`/`Booted`, while a -/// fault-channel backend can also report `Failed`. That is a capability -/// difference between sources, not an incomplete implementation — consumers -/// must handle the full set. +/// fault channel or progress-code register can also report the failure +/// variants. That is a capability difference between sources, not an +/// incomplete implementation — consumers must handle the full set. /// /// A status must describe the **current** boot cycle. Where the underlying /// signal is an edge or pulse, it is latched beneath the read, and the latch @@ -31,6 +34,12 @@ pub enum BootStatus { Booting, /// Boot completion observed. Booted, - /// Device reported a boot failure. - Failed, + /// Device reported a failure worth another attempt (transient + /// self-test miss, brown-out during bring-up). Consumes retry budget + /// immediately instead of waiting out the window. + FailedRetriable, + /// Device reported a terminal failure (corrupt image, configuration + /// mismatch). Ends the boot regardless of remaining retry budget — + /// re-running the same image cannot change the verdict. + FailedFatal, } diff --git a/services/orchestrator/capabilities/src/boot_watch.rs b/services/orchestrator/capabilities/src/boot_watch.rs index 74b3213ad..116be844a 100644 --- a/services/orchestrator/capabilities/src/boot_watch.rs +++ b/services/orchestrator/capabilities/src/boot_watch.rs @@ -35,17 +35,20 @@ pub enum WalkVerdict { }, /// Every checkpoint passed — the device is up. Complete, - /// A window expired or the device reported failure, with retry budget - /// left; the window is re-armed. The caller re-resets the device and - /// keeps polling — what a retry re-runs is the caller's policy. + /// The attempt failed — a window expired, or the device reported + /// [`FailedRetriable`](crate::BootStatus::FailedRetriable) (which ends + /// the wait early) — and retry budget remains; the window is re-armed. + /// The caller re-resets the device and keeps polling — what a retry + /// re-runs is the caller's policy. Retry { /// The checkpoint that failed. checkpoint: &'static str, /// Attempts left after this one. retries_left: u8, }, - /// Retry budget exhausted — this boot is dead. Recovery is the - /// caller's move. + /// This boot is dead: retry budget exhausted, or the device reported + /// [`FailedFatal`](crate::BootStatus::FailedFatal) — a verdict no + /// remaining budget can overturn. Recovery is the caller's move. Dead { /// The checkpoint the boot died at. checkpoint: &'static str, diff --git a/services/orchestrator/capabilities/src/evidence.rs b/services/orchestrator/capabilities/src/evidence.rs index 5515c4caa..4a764d762 100644 --- a/services/orchestrator/capabilities/src/evidence.rs +++ b/services/orchestrator/capabilities/src/evidence.rs @@ -41,9 +41,11 @@ mod tests { // A reader implemented against no HAL at all — the contract must be // satisfiable from any stack. One monotonic progress register serves // four staged-boot signals through one reader (the pattern a real SoC - // board is expected to use); a poison value fails every signal. + // board is expected to use); fault codes in the same register carry + // the device's own judgment, fatal or retriable, for every signal. const POISON: u8 = 0xFF; + const TRANSIENT: u8 = 0xEE; #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum TestSignal { @@ -77,7 +79,8 @@ mod tests { } let TestSignal::Progress(threshold) = *signal; Ok(match self.level { - POISON => BootStatus::Failed, + POISON => BootStatus::FailedFatal, + TRANSIENT => BootStatus::FailedRetriable, l if l >= threshold => BootStatus::Booted, _ => BootStatus::Booting, }) @@ -104,10 +107,11 @@ mod tests { assert_eq!(read(4), BootStatus::Booting); // service } - // A poisoned register must read Failed for every signal, whichever - // stage the walk happens to be awaiting. + // Fault codes must read the same for every signal, whichever stage + // the walk happens to be awaiting — and they carry the device's own + // retriability judgment. #[test] - fn a_poisoned_register_fails_every_signal() { + fn a_poisoned_register_fails_every_signal_fatally() { let mut soc = SocReader { level: POISON, fail: false, @@ -117,7 +121,23 @@ mod tests { assert_eq!( soc.read(&TestSignal::Progress(threshold)) .expect("read failed"), - BootStatus::Failed + BootStatus::FailedFatal + ); + } + } + + #[test] + fn a_transient_fault_reads_retriable_for_every_signal() { + let mut soc = SocReader { + level: TRANSIENT, + fail: false, + }; + + for threshold in 1..=4 { + assert_eq!( + soc.read(&TestSignal::Progress(threshold)) + .expect("read failed"), + BootStatus::FailedRetriable ); } } From a3791154267de8c58c7ef161df81663d59cb96aa Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 22:00:23 +0200 Subject: [PATCH 04/16] orchestrator: Exercise message-path evidence in the reader tests A timeout is never on the wire: a hung endpoint reads Booting forever, and only the orchestrator's clock turns silence into a verdict. The message path carries the active verdicts (device failure codes) and channel trouble, each on its own channel. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- .../orchestrator/capabilities/src/evidence.rs | 172 ++++++++++++++++++ 1 file changed, 172 insertions(+) diff --git a/services/orchestrator/capabilities/src/evidence.rs b/services/orchestrator/capabilities/src/evidence.rs index 4a764d762..5f8f0f0d3 100644 --- a/services/orchestrator/capabilities/src/evidence.rs +++ b/services/orchestrator/capabilities/src/evidence.rs @@ -156,4 +156,176 @@ mod tests { // Display comes from the core::error::Error bound, not a Debug dump. assert_eq!(err.to_string(), "progress register unreadable"); } + + // ── Message-path evidence (NIC archetype) ─────────────────────────── + // A timeout is never on the wire: a hung device sends nothing, the + // reader reports Booting forever, and only the orchestrator's clock + // (the checkpoint's window, judged by the walker) turns that silence + // into a verdict. The three channels stay separate: silence → Booting; + // the device speaks → FailedRetriable/FailedFatal ends the wait early; + // the channel breaks → Err. + + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + enum NicSignal { + /// The endpoint answers a control query as ready. + MctpReady, + /// A heartbeat message arrived (latched; reset clears it). + Heartbeat, + } + + struct MockNicEndpoint { + /// Control queries are answered after this many reads; `None` = + /// the device is hung. Silence is the only "timeout signal" a + /// device has — there is no message for it. + responds_after: Option, + reads: usize, + /// Device-sent failure notification, latched (reset clears it) — + /// what the message path *can* carry: an active verdict. + fault_code: Option, + /// Heartbeat arrival, latched by the transport. + heartbeat_seen: bool, + /// Injected transport fault: the channel itself breaks. + bus_fault: bool, + } + + impl MockNicEndpoint { + fn silent() -> Self { + Self { + responds_after: None, + reads: 0, + fault_code: None, + heartbeat_seen: false, + bus_fault: false, + } + } + } + + #[derive(Debug, PartialEq, Eq)] + struct MctpFault; + + impl core::fmt::Display for MctpFault { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.write_str("mctp transport fault") + } + } + + impl core::error::Error for MctpFault {} + + impl EvidenceReader for MockNicEndpoint { + type Error = MctpFault; + + fn read(&mut self, signal: &NicSignal) -> Result { + if self.bus_fault { + return Err(MctpFault); + } + match signal { + NicSignal::MctpReady => { + if let Some(code) = self.fault_code { + return Ok(match code { + 0xEE => BootStatus::FailedRetriable, + _ => BootStatus::FailedFatal, + }); + } + // Query answered => evidence; no answer => no evidence + // yet. NOT an error — the channel is fine, the device + // is silent. + self.reads += 1; + Ok(match self.responds_after { + Some(n) if self.reads > n => BootStatus::Booted, + _ => BootStatus::Booting, + }) + } + NicSignal::Heartbeat => Ok(match self.heartbeat_seen { + true => BootStatus::Booted, + false => BootStatus::Booting, + }), + } + } + } + + // A hung endpoint is Booting on every read, forever — turning that + // into a timeout is the walker's job, on the orchestrator's clock. + #[test] + fn a_hung_endpoint_reads_booting_forever() { + let mut nic = MockNicEndpoint::silent(); + + for _ in 0..100 { + assert_eq!( + nic.read(&NicSignal::MctpReady).expect("read failed"), + BootStatus::Booting + ); + } + } + + #[test] + fn silence_ends_once_the_endpoint_answers() { + let mut nic = MockNicEndpoint { + responds_after: Some(2), + ..MockNicEndpoint::silent() + }; + + assert_eq!( + nic.read(&NicSignal::MctpReady).expect("read failed"), + BootStatus::Booting + ); + assert_eq!( + nic.read(&NicSignal::MctpReady).expect("read failed"), + BootStatus::Booting + ); + assert_eq!( + nic.read(&NicSignal::MctpReady).expect("read failed"), + BootStatus::Booted + ); + } + + // A device that is up enough to talk reports its own verdict and ends + // the wait early — no window needs to expire. + #[test] + fn a_talking_device_reports_its_own_verdict() { + let mut nic = MockNicEndpoint { + fault_code: Some(0xEE), + ..MockNicEndpoint::silent() + }; + assert_eq!( + nic.read(&NicSignal::MctpReady).expect("read failed"), + BootStatus::FailedRetriable + ); + + let mut nic = MockNicEndpoint { + fault_code: Some(0x03), + ..MockNicEndpoint::silent() + }; + assert_eq!( + nic.read(&NicSignal::MctpReady).expect("read failed"), + BootStatus::FailedFatal + ); + } + + // Channel trouble is the reader's Error — distinct from both silence + // and a device-reported verdict. + #[test] + fn a_broken_channel_is_an_error_not_evidence() { + let mut nic = MockNicEndpoint { + bus_fault: true, + ..MockNicEndpoint::silent() + }; + + let err = nic + .read(&NicSignal::MctpReady) + .expect_err("expected the transport fault"); + assert_eq!(err.to_string(), "mctp transport fault"); + } + + #[test] + fn a_latched_heartbeat_reads_booted() { + let mut nic = MockNicEndpoint { + heartbeat_seen: true, + ..MockNicEndpoint::silent() + }; + + assert_eq!( + nic.read(&NicSignal::Heartbeat).expect("read failed"), + BootStatus::Booted + ); + } } From 68eb7a85b5bd5530afefd11729bf72c9845b9275 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 22:38:38 +0200 Subject: [PATCH 05/16] orchestrator: Carry the re-armed deadline in WalkVerdict::Retry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Retry re-arms the window, but the caller had no way to know until when — it would have had to reach into the checkpoint's timeout and do the walker's arithmetic itself. Retry now carries deadline_millis exactly like Waiting: one scheduling rule for both verdicts. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- .../orchestrator/capabilities/src/boot_watch.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/services/orchestrator/capabilities/src/boot_watch.rs b/services/orchestrator/capabilities/src/boot_watch.rs index 116be844a..28de7263b 100644 --- a/services/orchestrator/capabilities/src/boot_watch.rs +++ b/services/orchestrator/capabilities/src/boot_watch.rs @@ -37,14 +37,19 @@ pub enum WalkVerdict { Complete, /// The attempt failed — a window expired, or the device reported /// [`FailedRetriable`](crate::BootStatus::FailedRetriable) (which ends - /// the wait early) — and retry budget remains; the window is re-armed. - /// The caller re-resets the device and keeps polling — what a retry - /// re-runs is the caller's policy. + /// the wait early) — and retry budget remains; the window is re-armed + /// from the poll that judged it. The caller re-resets the device and + /// keeps polling — what a retry re-runs is the caller's policy. Retry { /// The checkpoint that failed. checkpoint: &'static str, /// Attempts left after this one. retries_left: u8, + /// When the re-armed window expires: the judging poll's + /// `now_millis` plus the checkpoint's `timeout`. The caller + /// schedules against this exactly as it does for `Waiting` — + /// no deadline arithmetic of its own. + deadline_millis: u64, }, /// This boot is dead: retry budget exhausted, or the device reported /// [`FailedFatal`](crate::BootStatus::FailedFatal) — a verdict no @@ -92,6 +97,7 @@ mod tests { WalkVerdict::Retry { checkpoint: "heartbeat", retries_left: 1, + deadline_millis: 30_000, }, WalkVerdict::Dead { checkpoint: "heartbeat", @@ -113,7 +119,8 @@ mod tests { }, WalkVerdict::Retry { checkpoint: "heartbeat", - retries_left: 1 + retries_left: 1, + deadline_millis: 30_000 }, ] ); From 42828404285385f3de738aa63052839262ded72f Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 22:38:38 +0200 Subject: [PATCH 06/16] orchestrator: Document wiring a concrete reader into EvidenceReader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adapter crates cannot implement EvidenceReader themselves — a board's signal vocabulary G is not theirs to know. Show the intended shape on the trait: the board impl owns the match, the hardware binding is made once at construction, the signal id proves the right reader was wired. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- .../orchestrator/capabilities/src/evidence.rs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/services/orchestrator/capabilities/src/evidence.rs b/services/orchestrator/capabilities/src/evidence.rs index 5f8f0f0d3..9173a0a28 100644 --- a/services/orchestrator/capabilities/src/evidence.rs +++ b/services/orchestrator/capabilities/src/evidence.rs @@ -16,6 +16,36 @@ use crate::BootStatus; /// The status must describe the **current** boot cycle — see /// [`BootStatus`] for the latching contract (evidence is cleared by the /// reset path, never by the reader). +/// +/// # Wiring a concrete reader +/// +/// Concrete readers (e.g. `GpioBootMonitor` in `orchestrator-hal-adapters`) +/// stay signal-agnostic — an adapter crate cannot know a board's `G`. +/// The board impl owns the match; the hardware binding is made once, at +/// construction, and the signal id just proves the right reader was +/// wired: +/// +/// ```ignore +/// /// bmc wiring: one ready line behind the board's signal vocabulary. +/// struct BmcReader<'a, P: GpioPort> { +/// // (port, pin, polarity) bound at bring-up from the table's Gpio(12). +/// ready: GpioBootMonitor<'a, P>, +/// } +/// +/// impl EvidenceReader for BmcReader<'_, P> +/// where +/// P::Error: 'static, +/// { +/// type Error = MonitorError; +/// +/// fn read(&mut self, signal: &MockSignal) -> Result { +/// match signal { +/// MockSignal::Gpio(_) => self.ready.boot_status(), +/// other => unreachable!("bmc reader wired to {other:?}"), +/// } +/// } +/// } +/// ``` pub trait EvidenceReader { /// The error type reported by this reader. /// From 19f00217bd198ff76685d97819df4255ce5dc559 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 22:38:38 +0200 Subject: [PATCH 07/16] orchestrator: Reject duplicate checkpoint names; pin max_retries=0 meaning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Failure reports identify a checkpoint by name, so a duplicate within a device would make them ambiguous — validate now rejects it at build time (str comparison by hand: == on &str is not const). Also state explicitly that max_retries=0 means the one attempt is all the device gets. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- services/orchestrator/config/src/lib.rs | 45 +++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/services/orchestrator/config/src/lib.rs b/services/orchestrator/config/src/lib.rs index f3c8b0213..2a63cc6f8 100644 --- a/services/orchestrator/config/src/lib.rs +++ b/services/orchestrator/config/src/lib.rs @@ -41,6 +41,7 @@ pub struct BootCheckpoint { /// orchestrator's own judgment; hung devices report nothing. pub timeout: core::time::Duration, /// Attempts allowed beyond the first before the failure is final. + /// `0` means the one attempt is all the device gets. pub max_retries: u8, } @@ -90,12 +91,41 @@ pub const fn validate(devices: &[DeviceConfig]) { !devices[i].checkpoints[c].timeout.is_zero(), "checkpoint timeout must not be zero" ); + // Failure reports identify a checkpoint by name; a duplicate + // would make them ambiguous. + let mut d = c + 1; + while d < devices[i].checkpoints.len() { + assert!( + !str_eq( + devices[i].checkpoints[c].name, + devices[i].checkpoints[d].name + ), + "checkpoint names must be unique per device" + ); + d += 1; + } c += 1; } i += 1; } } +// `==` on `&str` is not const; compare bytes by hand. +const fn str_eq(a: &str, b: &str) -> bool { + let (a, b) = (a.as_bytes(), b.as_bytes()); + if a.len() != b.len() { + return false; + } + let mut i = 0; + while i < a.len() { + if a[i] != b[i] { + return false; + } + i += 1; + } + true +} + #[cfg(test)] mod tests { use super::*; @@ -125,6 +155,21 @@ mod tests { validate(&[DEVICE]); } + #[test] + #[should_panic(expected = "checkpoint names must be unique")] + fn rejects_duplicate_checkpoint_names() { + validate(&[DeviceConfig { + checkpoints: &[ + CHECKPOINT, + BootCheckpoint { + signal: 1, + ..CHECKPOINT + }, + ], + ..DEVICE + }]); + } + #[test] #[should_panic(expected = "device name must not be empty")] fn rejects_an_empty_device_name() { From a088e2ea623dd73d05bd011cd7842bb203d15d7b Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 22:50:19 +0200 Subject: [PATCH 08/16] orchestrator: Anchor the signal-id docs; show board-local validation The signal field now says on the spot why it is an id and who resolves it, and validate points at the mock table, which demonstrates the board-local const fence for checks the generic validate cannot do (gpio line within the bank). Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- services/orchestrator/config/src/lib.rs | 9 ++++++--- target/mock/devices.rs | 23 ++++++++++++++++++++++- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/services/orchestrator/config/src/lib.rs b/services/orchestrator/config/src/lib.rs index 2a63cc6f8..1e04d666e 100644 --- a/services/orchestrator/config/src/lib.rs +++ b/services/orchestrator/config/src/lib.rs @@ -35,7 +35,9 @@ pub enum CommitPolicy { pub struct BootCheckpoint { /// Names the checkpoint in failure reports ("bl1", "kernel", …). pub name: &'static str, - /// Board-defined signal id, resolved by the board's `EvidenceReader`. + /// Board-defined signal id, resolved by the board's `EvidenceReader` + /// (in `orchestrator-capabilities`). An id rather than a function, so + /// the table stays pure data — the type-level docs say why. pub signal: G, /// Window for one attempt at this checkpoint. Expiry is the /// orchestrator's own judgment; hung devices report nothing. @@ -71,8 +73,9 @@ pub struct DeviceConfig { /// bad table fails the build. /// /// Only schema-shape checks are possible here; checks on the board's own -/// types (signal ranges, uniqueness) belong next to the table that defines -/// their meaning, in a board-local `const fn` run alongside this one. +/// types (signal ranges, uniqueness of signal ids) belong next to the +/// table that defines their meaning, in a board-local `const fn` run +/// alongside this one — `target/mock/devices.rs` shows the pattern. pub const fn validate(devices: &[DeviceConfig]) { let mut i = 0; while i < devices.len() { diff --git a/target/mock/devices.rs b/target/mock/devices.rs index 0b7d29d55..30b6739a0 100644 --- a/target/mock/devices.rs +++ b/target/mock/devices.rs @@ -67,4 +67,25 @@ pub const MANAGED_DEVICES: &[DeviceConfig] = &[ }, ]; -const _: () = orchestrator_config::validate(MANAGED_DEVICES); +/// Board-local checks the generic `validate` cannot do — it knows the +/// schema's shape, not this board's meanings. Same const-fence pattern: +/// a bad signal fails the build. +const fn validate_signals(devices: &[DeviceConfig]) { + let mut i = 0; + while i < devices.len() { + let mut c = 0; + while c < devices[i].checkpoints.len() { + if let MockSignal::Gpio(line) = devices[i].checkpoints[c].signal { + // The mock ready-line bank packs 32 lines, SGPIO-style. + assert!(line < 32, "gpio signal names a line outside the bank"); + } + c += 1; + } + i += 1; + } +} + +const _: () = { + orchestrator_config::validate(MANAGED_DEVICES); + validate_signals(MANAGED_DEVICES); +}; From 7356e9baa57287451c44db0d2b766af62b719a7f Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Thu, 6 Aug 2026 10:50:09 +0200 Subject: [PATCH 09/16] orchestrator: Drop CommitPolicy from the device table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Whether a device must attest before its update commits follows from what kind of device it is (iRoT-backed or symbiont — the orchestrator's ComponentKind); the CSA defines only that distinction. A second table knob could only agree with the kind or contradict it. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- services/orchestrator/config/src/lib.rs | 21 +++++---------------- target/mock/devices.rs | 4 +--- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/services/orchestrator/config/src/lib.rs b/services/orchestrator/config/src/lib.rs index 1e04d666e..f4144dc28 100644 --- a/services/orchestrator/config/src/lib.rs +++ b/services/orchestrator/config/src/lib.rs @@ -7,20 +7,6 @@ #![cfg_attr(not(test), no_std)] -/// What the orchestrator requires before it commits a staged image. -/// -/// Intentionally exhaustive (not `#[non_exhaustive]`): adding a variant is -/// a breaking change, so the compiler forces every match on the policy — -/// in particular the orchestrator's commit decision — to handle the new -/// variant explicitly instead of falling into a wildcard arm. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum CommitPolicy { - /// The device reports it came up. - Liveness, - /// Liveness plus SPDM re-attestation of the running image. - LivenessAndAttestation, -} - /// One boot checkpoint: a signal the orchestrator waits for, how long it /// waits per attempt, and how many failed attempts it tolerates. /// @@ -57,6 +43,11 @@ pub struct BootCheckpoint { /// Intentionally exhaustive (not `#[non_exhaustive]`): board tables /// construct this struct by literal, which the attribute would forbid. /// Adding a field is a breaking change that updates every board table. +/// +/// Deliberately says nothing about attestation or commit requirements: +/// those follow from what kind of device this is (iRoT-backed or +/// symbiont, the orchestrator's `ComponentKind`), not from a table +/// setting — a second knob would only let the two disagree. #[derive(Debug, Clone, Copy)] pub struct DeviceConfig { pub name: &'static str, @@ -66,7 +57,6 @@ pub struct DeviceConfig { /// counts as booted when the last one is reached; a checkpoint whose /// window and retry budget are exhausted fails the boot. pub checkpoints: &'static [BootCheckpoint], - pub commit_policy: CommitPolicy, } /// Checks a device table. Board configs call this in a const context so a @@ -150,7 +140,6 @@ mod tests { name: "dev", reset_signal: 0, checkpoints: &[CHECKPOINT], - commit_policy: CommitPolicy::Liveness, }; #[test] diff --git a/target/mock/devices.rs b/target/mock/devices.rs index 30b6739a0..a5fa71a9d 100644 --- a/target/mock/devices.rs +++ b/target/mock/devices.rs @@ -9,7 +9,7 @@ use core::time::Duration; -use orchestrator_config::{BootCheckpoint, CommitPolicy, DeviceConfig}; +use orchestrator_config::{BootCheckpoint, DeviceConfig}; /// The mock board's boot-signal vocabulary. The schema carries these /// opaquely; only this board's `EvidenceReader` gives them meaning. @@ -41,7 +41,6 @@ pub const MANAGED_DEVICES: &[DeviceConfig] = &[ timeout: Duration::from_secs(90), max_retries: 1, }], - commit_policy: CommitPolicy::Liveness, }, // PLDM device (NIC archetype): self-updating, SPDM-capable. Two // checkpoints, exercising the multi-checkpoint path: transport up @@ -63,7 +62,6 @@ pub const MANAGED_DEVICES: &[DeviceConfig] = &[ max_retries: 0, }, ], - commit_policy: CommitPolicy::LivenessAndAttestation, }, ]; From 4e1f03107921507721e08f3dec69f180b597d368 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Thu, 6 Aug 2026 10:55:08 +0200 Subject: [PATCH 10/16] orchestrator: Leave retry and terminal decisions to the orchestrator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WalkVerdict now reports observation only: Failed{checkpoint, cause} replaces Retry/Dead/retries_left — the state machine's ComponentStatus.retry and Recovering→RecoveryFailed path already own those decisions, and a second counter could only agree or disagree with the first. max_retries leaves the table for the same reason: a retry re-resets the device and re-runs the whole walk, so budgets are per boot attempt, owned where boot attempts are owned. The device's own judgment still flows up as FailureCause::{TimedOut, DeviceRetriable, DeviceFatal} — the one input the retry decision needs. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- .../capabilities/src/boot_watch.rs | 74 ++++++++++--------- services/orchestrator/capabilities/src/lib.rs | 2 +- services/orchestrator/config/src/lib.rs | 13 ++-- target/mock/devices.rs | 3 - 4 files changed, 48 insertions(+), 44 deletions(-) diff --git a/services/orchestrator/capabilities/src/boot_watch.rs b/services/orchestrator/capabilities/src/boot_watch.rs index 28de7263b..6c05cc88f 100644 --- a/services/orchestrator/capabilities/src/boot_watch.rs +++ b/services/orchestrator/capabilities/src/boot_watch.rs @@ -19,9 +19,15 @@ pub trait BootWatch { /// Everything the orchestrator needs to know about a boot walk. /// -/// Deliberately free of device and error types: the orchestrator acts the -/// same whatever the cause, so the concrete detail is logged by the walk -/// while it is still in scope, not carried across the seam. +/// Observation only: the walk judges checkpoint windows, never lives. +/// Retry counts and terminal calls belong to the orchestrator state +/// machine (`ComponentStatus.retry`, the `Recovering` → `RecoveryFailed` +/// path) — a verdict that carried a retry budget would be a second owner +/// for the same decision, free to disagree with the first. +/// +/// Deliberately free of device and error types: the concrete detail is +/// logged by the walk while it is still in scope, not carried across the +/// seam. /// /// Intentionally exhaustive (not `#[non_exhaustive]`): adding a verdict is /// a breaking change, so the compiler forces every consumer — in particular @@ -35,31 +41,33 @@ pub enum WalkVerdict { }, /// Every checkpoint passed — the device is up. Complete, - /// The attempt failed — a window expired, or the device reported - /// [`FailedRetriable`](crate::BootStatus::FailedRetriable) (which ends - /// the wait early) — and retry budget remains; the window is re-armed - /// from the poll that judged it. The caller re-resets the device and - /// keeps polling — what a retry re-runs is the caller's policy. - Retry { - /// The checkpoint that failed. - checkpoint: &'static str, - /// Attempts left after this one. - retries_left: u8, - /// When the re-armed window expires: the judging poll's - /// `now_millis` plus the checkpoint's `timeout`. The caller - /// schedules against this exactly as it does for `Waiting` — - /// no deadline arithmetic of its own. - deadline_millis: u64, - }, - /// This boot is dead: retry budget exhausted, or the device reported - /// [`FailedFatal`](crate::BootStatus::FailedFatal) — a verdict no - /// remaining budget can overturn. Recovery is the caller's move. - Dead { - /// The checkpoint the boot died at. + /// This boot attempt failed at `checkpoint`; the walk is over. + /// Whether to try again, recover, or give up is the orchestrator's + /// decision — a retry re-resets the device and starts a fresh walk. + Failed { + /// The checkpoint the attempt died at. checkpoint: &'static str, + /// Why it died — the one input the retry decision needs. + cause: FailureCause, }, } +/// Why a boot attempt failed at a checkpoint. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FailureCause { + /// The window expired; the device reported nothing. + TimedOut, + /// The device reported a failure worth another attempt + /// ([`FailedRetriable`](crate::BootStatus::FailedRetriable)) — the + /// wait ended early. + DeviceRetriable, + /// The device reported a terminal failure + /// ([`FailedFatal`](crate::BootStatus::FailedFatal)) — re-running the + /// same image cannot change the verdict, whatever retry budget the + /// orchestrator has left. + DeviceFatal, +} + #[cfg(test)] mod tests { use super::*; @@ -94,13 +102,13 @@ mod tests { }; let mut nic = ScriptedWalk { verdicts: &[ - WalkVerdict::Retry { + WalkVerdict::Failed { checkpoint: "heartbeat", - retries_left: 1, - deadline_millis: 30_000, + cause: FailureCause::TimedOut, }, - WalkVerdict::Dead { + WalkVerdict::Failed { checkpoint: "heartbeat", + cause: FailureCause::DeviceFatal, }, ], next: 0, @@ -117,10 +125,9 @@ mod tests { WalkVerdict::Waiting { deadline_millis: 90_000 }, - WalkVerdict::Retry { + WalkVerdict::Failed { checkpoint: "heartbeat", - retries_left: 1, - deadline_millis: 30_000 + cause: FailureCause::TimedOut }, ] ); @@ -128,8 +135,9 @@ mod tests { second, [ WalkVerdict::Complete, - WalkVerdict::Dead { - checkpoint: "heartbeat" + WalkVerdict::Failed { + checkpoint: "heartbeat", + cause: FailureCause::DeviceFatal }, ] ); diff --git a/services/orchestrator/capabilities/src/lib.rs b/services/orchestrator/capabilities/src/lib.rs index 8a6557453..85f7019eb 100644 --- a/services/orchestrator/capabilities/src/lib.rs +++ b/services/orchestrator/capabilities/src/lib.rs @@ -33,5 +33,5 @@ mod evidence; pub use boot_control::BootControl; pub use boot_status::BootStatus; -pub use boot_watch::{BootWatch, WalkVerdict}; +pub use boot_watch::{BootWatch, FailureCause, WalkVerdict}; pub use evidence::EvidenceReader; diff --git a/services/orchestrator/config/src/lib.rs b/services/orchestrator/config/src/lib.rs index f4144dc28..ffc34abb8 100644 --- a/services/orchestrator/config/src/lib.rs +++ b/services/orchestrator/config/src/lib.rs @@ -7,8 +7,10 @@ #![cfg_attr(not(test), no_std)] -/// One boot checkpoint: a signal the orchestrator waits for, how long it -/// waits per attempt, and how many failed attempts it tolerates. +/// One boot checkpoint: a signal the orchestrator waits for, and how long +/// it waits. Retry policy is deliberately not table data: a retry +/// re-resets the device and re-runs the whole walk, so budgets are +/// per boot attempt and owned by the orchestrator state machine. /// /// `signal` is a board-defined id — the schema attaches no meaning to it /// and names no signal kinds. Each board defines its own vocabulary (a @@ -28,9 +30,6 @@ pub struct BootCheckpoint { /// Window for one attempt at this checkpoint. Expiry is the /// orchestrator's own judgment; hung devices report nothing. pub timeout: core::time::Duration, - /// Attempts allowed beyond the first before the failure is final. - /// `0` means the one attempt is all the device gets. - pub max_retries: u8, } /// One managed downstream device, as declared by the board config. @@ -55,7 +54,8 @@ pub struct DeviceConfig { pub reset_signal: R, /// Boot checkpoints, in the order the device passes them. The device /// counts as booted when the last one is reached; a checkpoint whose - /// window and retry budget are exhausted fails the boot. + /// window expires fails the attempt — whether to retry or recover is + /// the orchestrator's decision, not table data. pub checkpoints: &'static [BootCheckpoint], } @@ -133,7 +133,6 @@ mod tests { name: "boot-complete", signal: 0, timeout: Duration::from_secs(1), - max_retries: 1, }; const DEVICE: DeviceConfig = DeviceConfig { diff --git a/target/mock/devices.rs b/target/mock/devices.rs index a5fa71a9d..6008e1b30 100644 --- a/target/mock/devices.rs +++ b/target/mock/devices.rs @@ -39,7 +39,6 @@ pub const MANAGED_DEVICES: &[DeviceConfig] = &[ name: "boot-complete", signal: MockSignal::Gpio(12), timeout: Duration::from_secs(90), - max_retries: 1, }], }, // PLDM device (NIC archetype): self-updating, SPDM-capable. Two @@ -53,13 +52,11 @@ pub const MANAGED_DEVICES: &[DeviceConfig] = &[ name: "mctp-ready", signal: MockSignal::MctpReady, timeout: Duration::from_secs(20), - max_retries: 2, }, BootCheckpoint { name: "heartbeat", signal: MockSignal::Heartbeat, timeout: Duration::from_secs(10), - max_retries: 0, }, ], }, From b6327153309426dccba03c933711b586ef29cf71 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Thu, 6 Aug 2026 11:31:31 +0200 Subject: [PATCH 11/16] orchestrator: Pin the orchestrator seams in the docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pin three facts the docs left implicit: checkpoint timeouts are table data the walk consumes — the clockless state machine never sees a duration, a component's boot timeout is just its walk over the windows; the device table is the authority the chain is built from; and Complete maps to ComponentReady or Booted by component kind, in the shell. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- services/orchestrator/capabilities/src/boot_watch.rs | 5 ++++- services/orchestrator/config/src/lib.rs | 9 +++++++-- target/mock/devices.rs | 3 ++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/services/orchestrator/capabilities/src/boot_watch.rs b/services/orchestrator/capabilities/src/boot_watch.rs index 6c05cc88f..94276c772 100644 --- a/services/orchestrator/capabilities/src/boot_watch.rs +++ b/services/orchestrator/capabilities/src/boot_watch.rs @@ -39,7 +39,10 @@ pub enum WalkVerdict { /// When the awaited checkpoint's window expires. deadline_millis: u64, }, - /// Every checkpoint passed — the device is up. + /// Every checkpoint passed — the device is up. Which state-machine + /// event this becomes is the shell's mapping, by component kind: + /// `ComponentReady` for an iRoT-backed device, `Booted` for a + /// symbiont. Complete, /// This boot attempt failed at `checkpoint`; the walk is over. /// Whether to try again, recover, or give up is the orchestrator's diff --git a/services/orchestrator/config/src/lib.rs b/services/orchestrator/config/src/lib.rs index ffc34abb8..c544a79fa 100644 --- a/services/orchestrator/config/src/lib.rs +++ b/services/orchestrator/config/src/lib.rs @@ -27,8 +27,13 @@ pub struct BootCheckpoint { /// (in `orchestrator-capabilities`). An id rather than a function, so /// the table stays pure data — the type-level docs say why. pub signal: G, - /// Window for one attempt at this checkpoint. Expiry is the - /// orchestrator's own judgment; hung devices report nothing. + /// Window for one attempt at this checkpoint. Expiry is the boot + /// walk's own judgment; hung devices report nothing. + /// + /// The orchestrator state machine never sees this value — it is + /// clockless. The walk consumes the windows and reports expiry as a + /// failed attempt; a component's whole boot timeout is nothing more + /// than its walk over these windows, in order. pub timeout: core::time::Duration, } diff --git a/target/mock/devices.rs b/target/mock/devices.rs index 6008e1b30..33af731cb 100644 --- a/target/mock/devices.rs +++ b/target/mock/devices.rs @@ -25,7 +25,8 @@ pub enum MockSignal { } /// Declaration order is the boot order: the orchestrator releases devices -/// top to bottom, one at a time. +/// top to bottom, one at a time. This table is the authority — the +/// orchestrator's chain of trust is built from it, never beside it. /// /// The mock board's reset controller addresses reset lines by plain index, /// so the reset id type is `u8`. From 7d6bea8e4add9215f4fd926a394362c2e11111fb Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Fri, 7 Aug 2026 11:14:07 +0200 Subject: [PATCH 12/16] orchestrator: Make invalid table entries unconstructible Every schema check is per-device, so the constructors can run them all. BootCheckpoint::new and DeviceConfig::new are const fn -- board tables still build in const context, so a bad table is still a build error -- but the fields are private now, and a checkpoint or device entry that violates the schema cannot be constructed at all. The free validate() is gone with the loophole it carried: it had to be remembered, and a board table that dropped the const fence compiled fine while broken. Construction is the one gate every entry passes. Board-local checks keep the const-fence pattern (validate_signals in the mock table), reading through the new accessors. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- services/orchestrator/config/src/lib.rs | 213 ++++++++++++++---------- target/mock/devices.rs | 56 +++---- 2 files changed, 146 insertions(+), 123 deletions(-) diff --git a/services/orchestrator/config/src/lib.rs b/services/orchestrator/config/src/lib.rs index c544a79fa..f4e742412 100644 --- a/services/orchestrator/config/src/lib.rs +++ b/services/orchestrator/config/src/lib.rs @@ -4,6 +4,11 @@ //! Schema for the per-board device table. Board device tables //! (`target//devices.rs`) declare the values; no concrete line or //! device is named here. +//! +//! Invariants are enforced in the `const fn` constructors, so an invalid +//! table is a build error and there is no validate step to forget. Checks +//! on board-defined types belong next to the table that gives them +//! meaning (`target/mock/devices.rs` shows the pattern). #![cfg_attr(not(test), no_std)] @@ -12,21 +17,57 @@ /// re-resets the device and re-runs the whole walk, so budgets are /// per boot attempt and owned by the orchestrator state machine. /// -/// `signal` is a board-defined id — the schema attaches no meaning to it -/// and names no signal kinds. Each board defines its own vocabulary (a +/// The signal is a board-defined id — the schema attaches no meaning to +/// it and names no signal kinds. Each board defines its own vocabulary (a /// small enum: a GPIO line, a progress-register threshold, a message-path /// readiness) and gives it meaning in its `EvidenceReader`. The id is a /// defunctionalized evidence check: data in the table instead of a /// function, so the table stays printable, comparable, const-checkable — /// and could one day be generated instead of written. +/// +/// Fields are private so a checkpoint that violates the schema is +/// unrepresentable: [`new`](Self::new) is the only way in, and it checks. #[derive(Debug, Clone, Copy)] pub struct BootCheckpoint { + name: &'static str, + signal: G, + timeout: core::time::Duration, +} + +impl BootCheckpoint { + /// Declares a checkpoint. `const`, so board tables run the checks at + /// build time. + /// + /// # Panics + /// + /// Panics — a build error in const context — if `name` is empty or + /// `timeout` is zero. + #[must_use] + pub const fn new(name: &'static str, signal: G, timeout: core::time::Duration) -> Self { + assert!(!name.is_empty(), "checkpoint name must not be empty"); + assert!(!timeout.is_zero(), "checkpoint timeout must not be zero"); + Self { + name, + signal, + timeout, + } + } + /// Names the checkpoint in failure reports ("bl1", "kernel", …). - pub name: &'static str, + /// Unique within a device's checkpoint list. + #[must_use] + pub const fn name(&self) -> &'static str { + self.name + } + /// Board-defined signal id, resolved by the board's `EvidenceReader` /// (in `orchestrator-capabilities`). An id rather than a function, so /// the table stays pure data — the type-level docs say why. - pub signal: G, + #[must_use] + pub const fn signal(&self) -> &G { + &self.signal + } + /// Window for one attempt at this checkpoint. Expiry is the boot /// walk's own judgment; hung devices report nothing. /// @@ -34,7 +75,10 @@ pub struct BootCheckpoint { /// clockless. The walk consumes the windows and reports expiry as a /// failed attempt; a component's whole boot timeout is nothing more /// than its walk over these windows, in order. - pub timeout: core::time::Duration, + #[must_use] + pub const fn timeout(&self) -> core::time::Duration { + self.timeout + } } /// One managed downstream device, as declared by the board config. @@ -44,67 +88,79 @@ pub struct BootCheckpoint { /// implementation) and its boot-signal vocabulary `G`, for the same /// reason: signal ids are board-specific. /// -/// Intentionally exhaustive (not `#[non_exhaustive]`): board tables -/// construct this struct by literal, which the attribute would forbid. -/// Adding a field is a breaking change that updates every board table. -/// /// Deliberately says nothing about attestation or commit requirements: /// those follow from what kind of device this is (iRoT-backed or /// symbiont, the orchestrator's `ComponentKind`), not from a table /// setting — a second knob would only let the two disagree. +/// +/// Fields are private so a device entry that violates the schema is +/// unrepresentable: [`new`](Self::new) is the only way in, and it checks. #[derive(Debug, Clone, Copy)] pub struct DeviceConfig { - pub name: &'static str, - /// Reset signal id, passed to HalBootControl::new. - pub reset_signal: R, - /// Boot checkpoints, in the order the device passes them. The device - /// counts as booted when the last one is reached; a checkpoint whose - /// window expires fails the attempt — whether to retry or recover is - /// the orchestrator's decision, not table data. - pub checkpoints: &'static [BootCheckpoint], + name: &'static str, + reset_signal: R, + checkpoints: &'static [BootCheckpoint], } -/// Checks a device table. Board configs call this in a const context so a -/// bad table fails the build. -/// -/// Only schema-shape checks are possible here; checks on the board's own -/// types (signal ranges, uniqueness of signal ids) belong next to the -/// table that defines their meaning, in a board-local `const fn` run -/// alongside this one — `target/mock/devices.rs` shows the pattern. -pub const fn validate(devices: &[DeviceConfig]) { - let mut i = 0; - while i < devices.len() { - assert!(!devices[i].name.is_empty(), "device name must not be empty"); +impl DeviceConfig { + /// Declares a managed device. `const`, so board tables run the checks + /// at build time. + /// + /// # Panics + /// + /// Panics — a build error in const context — if `name` is empty, if + /// `checkpoints` is empty, or if two checkpoints share a name + /// (failure reports identify a checkpoint by name; a duplicate would + /// make them ambiguous). + #[must_use] + pub const fn new( + name: &'static str, + reset_signal: R, + checkpoints: &'static [BootCheckpoint], + ) -> Self { + assert!(!name.is_empty(), "device name must not be empty"); assert!( - !devices[i].checkpoints.is_empty(), + !checkpoints.is_empty(), "device must declare at least one boot checkpoint" ); let mut c = 0; - while c < devices[i].checkpoints.len() { - assert!( - !devices[i].checkpoints[c].name.is_empty(), - "checkpoint name must not be empty" - ); - assert!( - !devices[i].checkpoints[c].timeout.is_zero(), - "checkpoint timeout must not be zero" - ); - // Failure reports identify a checkpoint by name; a duplicate - // would make them ambiguous. + while c < checkpoints.len() { let mut d = c + 1; - while d < devices[i].checkpoints.len() { + while d < checkpoints.len() { assert!( - !str_eq( - devices[i].checkpoints[c].name, - devices[i].checkpoints[d].name - ), + !str_eq(checkpoints[c].name, checkpoints[d].name), "checkpoint names must be unique per device" ); d += 1; } c += 1; } - i += 1; + Self { + name, + reset_signal, + checkpoints, + } + } + + /// The device's name in reports and logs. + #[must_use] + pub const fn name(&self) -> &'static str { + self.name + } + + /// Reset signal id, passed to HalBootControl::new. + #[must_use] + pub const fn reset_signal(&self) -> &R { + &self.reset_signal + } + + /// Boot checkpoints, in the order the device passes them. The device + /// counts as booted when the last one is reached; a checkpoint whose + /// window expires fails the attempt — whether to retry or recover is + /// the orchestrator's decision, not table data. + #[must_use] + pub const fn checkpoints(&self) -> &'static [BootCheckpoint] { + self.checkpoints } } @@ -129,79 +185,56 @@ mod tests { use super::*; use core::time::Duration; - // Board tables run validate() at compile time, where a rejection is a - // build error nobody can assert on. These tests call it at runtime to - // prove the reject paths actually fire — a vacuous loop would pass - // every `const _` check silently. + // Board tables run the constructors at compile time, where a + // rejection is a build error nobody can assert on. These tests call + // them at runtime to prove the reject paths actually fire. - const CHECKPOINT: BootCheckpoint = BootCheckpoint { - name: "boot-complete", - signal: 0, - timeout: Duration::from_secs(1), - }; + const CHECKPOINT: BootCheckpoint = + BootCheckpoint::new("boot-complete", 0, Duration::from_secs(1)); - const DEVICE: DeviceConfig = DeviceConfig { - name: "dev", - reset_signal: 0, - checkpoints: &[CHECKPOINT], - }; + // Same name, different signal: each checkpoint is individually valid, + // so the pair only trips the device-level duplicate check. + const CHECKPOINT_DUP: BootCheckpoint = + BootCheckpoint::new("boot-complete", 1, Duration::from_secs(1)); #[test] fn accepts_a_valid_table() { - validate(&[DEVICE]); + let device = DeviceConfig::new("dev", 0u8, &[CHECKPOINT]); + assert_eq!(device.name(), "dev"); + assert_eq!(*device.reset_signal(), 0); + assert_eq!(device.checkpoints().len(), 1); + assert_eq!(device.checkpoints()[0].name(), "boot-complete"); + assert_eq!(*device.checkpoints()[0].signal(), 0); + assert_eq!(device.checkpoints()[0].timeout(), Duration::from_secs(1)); } #[test] #[should_panic(expected = "checkpoint names must be unique")] fn rejects_duplicate_checkpoint_names() { - validate(&[DeviceConfig { - checkpoints: &[ - CHECKPOINT, - BootCheckpoint { - signal: 1, - ..CHECKPOINT - }, - ], - ..DEVICE - }]); + let _ = DeviceConfig::new("dev", 0u8, &[CHECKPOINT, CHECKPOINT_DUP]); } #[test] #[should_panic(expected = "device name must not be empty")] fn rejects_an_empty_device_name() { - validate(&[DEVICE, DeviceConfig { name: "", ..DEVICE }]); + let _ = DeviceConfig::new("", 0u8, &[CHECKPOINT]); } #[test] #[should_panic(expected = "at least one boot checkpoint")] fn rejects_an_empty_checkpoint_list() { - validate(&[DeviceConfig { - checkpoints: &[], - ..DEVICE - }]); + let _ = DeviceConfig::new("dev", 0u8, &[] as &[BootCheckpoint]); } #[test] #[should_panic(expected = "checkpoint name must not be empty")] fn rejects_an_empty_checkpoint_name() { - validate(&[DeviceConfig { - checkpoints: &[BootCheckpoint { - name: "", - ..CHECKPOINT - }], - ..DEVICE - }]); + let _ = BootCheckpoint::new("", 0u8, Duration::from_secs(1)); } #[test] #[should_panic(expected = "checkpoint timeout must not be zero")] fn rejects_a_zero_checkpoint_timeout() { - validate(&[DeviceConfig { - checkpoints: &[BootCheckpoint { - timeout: Duration::ZERO, - ..CHECKPOINT - }], - ..DEVICE - }]); + let _ = BootCheckpoint::new("boot-complete", 0u8, Duration::ZERO); } } diff --git a/target/mock/devices.rs b/target/mock/devices.rs index 33af731cb..98802697e 100644 --- a/target/mock/devices.rs +++ b/target/mock/devices.rs @@ -33,45 +33,38 @@ pub enum MockSignal { pub const MANAGED_DEVICES: &[DeviceConfig] = &[ // Direct-flash SPI device (BMC archetype): the eRoT fronts its flash. // Single checkpoint: it raises a boot-complete GPIO. - DeviceConfig { - name: "bmc", - reset_signal: 7, - checkpoints: &[BootCheckpoint { - name: "boot-complete", - signal: MockSignal::Gpio(12), - timeout: Duration::from_secs(90), - }], - }, + DeviceConfig::new( + "bmc", + 7, + &[BootCheckpoint::new( + "boot-complete", + MockSignal::Gpio(12), + Duration::from_secs(90), + )], + ), // PLDM device (NIC archetype): self-updating, SPDM-capable. Two // checkpoints, exercising the multi-checkpoint path: transport up // first, then proof the workload is alive. - DeviceConfig { - name: "nic", - reset_signal: 3, - checkpoints: &[ - BootCheckpoint { - name: "mctp-ready", - signal: MockSignal::MctpReady, - timeout: Duration::from_secs(20), - }, - BootCheckpoint { - name: "heartbeat", - signal: MockSignal::Heartbeat, - timeout: Duration::from_secs(10), - }, + DeviceConfig::new( + "nic", + 3, + &[ + BootCheckpoint::new("mctp-ready", MockSignal::MctpReady, Duration::from_secs(20)), + BootCheckpoint::new("heartbeat", MockSignal::Heartbeat, Duration::from_secs(10)), ], - }, + ), ]; -/// Board-local checks the generic `validate` cannot do — it knows the -/// schema's shape, not this board's meanings. Same const-fence pattern: -/// a bad signal fails the build. +/// Board-local checks the schema constructors cannot do — they know the +/// schema's shape, not this board's meanings. Const-fence pattern: a bad +/// signal fails the build. const fn validate_signals(devices: &[DeviceConfig]) { let mut i = 0; while i < devices.len() { + let checkpoints = devices[i].checkpoints(); let mut c = 0; - while c < devices[i].checkpoints.len() { - if let MockSignal::Gpio(line) = devices[i].checkpoints[c].signal { + while c < checkpoints.len() { + if let MockSignal::Gpio(line) = *checkpoints[c].signal() { // The mock ready-line bank packs 32 lines, SGPIO-style. assert!(line < 32, "gpio signal names a line outside the bank"); } @@ -81,7 +74,4 @@ const fn validate_signals(devices: &[DeviceConfig]) { } } -const _: () = { - orchestrator_config::validate(MANAGED_DEVICES); - validate_signals(MANAGED_DEVICES); -}; +const _: () = validate_signals(MANAGED_DEVICES); From 5d9974cbe713e727d98eafa382aa6e552de26d86 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Fri, 7 Aug 2026 11:23:11 +0200 Subject: [PATCH 13/16] orchestrator: Fold BootStatus into the evidence module MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Within the crate only EvidenceReader consumes BootStatus — it is the trait's return vocabulary — so the enum does not earn a module of its own. The crate-root re-export is unchanged; no import anywhere moves. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast --- .../orchestrator/capabilities/BUILD.bazel | 1 - .../capabilities/src/boot_status.rs | 45 ------------------- .../orchestrator/capabilities/src/evidence.rs | 29 +++++++++++- services/orchestrator/capabilities/src/lib.rs | 4 +- 4 files changed, 28 insertions(+), 51 deletions(-) delete mode 100644 services/orchestrator/capabilities/src/boot_status.rs diff --git a/services/orchestrator/capabilities/BUILD.bazel b/services/orchestrator/capabilities/BUILD.bazel index 09e66160d..011d2e11c 100644 --- a/services/orchestrator/capabilities/BUILD.bazel +++ b/services/orchestrator/capabilities/BUILD.bazel @@ -7,7 +7,6 @@ rust_library( name = "orchestrator_capabilities", srcs = [ "src/boot_control.rs", - "src/boot_status.rs", "src/boot_watch.rs", "src/evidence.rs", "src/lib.rs", diff --git a/services/orchestrator/capabilities/src/boot_status.rs b/services/orchestrator/capabilities/src/boot_status.rs deleted file mode 100644 index 3936356de..000000000 --- a/services/orchestrator/capabilities/src/boot_status.rs +++ /dev/null @@ -1,45 +0,0 @@ -// Licensed under the Apache-2.0 license -// SPDX-License-Identifier: Apache-2.0 - -//! Shared vocabulary for boot-liveness evidence. - -/// Liveness of a managed device's boot: Boot Confirmation only. -/// -/// Reports only that a device came up, never what booted; confirming the -/// running image is the one the RoT staged is attestation, a separate step. -/// The failure variants are optional device-reported evidence and never the -/// only failure path, since a hung device reports nothing — a stuck boot is -/// caught by the orchestrator's timeout, not by this enum. What they buy is -/// speed and judgment: a device that knows it failed ends the wait early, -/// and a device that knows a retry is pointless says so, instead of the -/// orchestrator burning its window and retry budget to find out. -/// -/// Any given evidence source may only ever produce a *subset* of these -/// statuses: a single ready pin yields only `Booting`/`Booted`, while a -/// fault channel or progress-code register can also report the failure -/// variants. That is a capability difference between sources, not an -/// incomplete implementation — consumers must handle the full set. -/// -/// A status must describe the **current** boot cycle. Where the underlying -/// signal is an edge or pulse, it is latched beneath the read, and the latch -/// must be cleared whenever the device re-enters reset — by hardware tying -/// the latch to the device's reset line, or by the platform code that drives -/// `BootControl` — so evidence left over from a previous boot never reads as -/// [`Booted`](BootStatus::Booted). Clearing is deliberately the reset path's -/// job, not the reader's: a reader that could clear its own evidence would -/// let a read race a reset. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub enum BootStatus { - /// Released, but boot completion not yet observed. - Booting, - /// Boot completion observed. - Booted, - /// Device reported a failure worth another attempt (transient - /// self-test miss, brown-out during bring-up). Consumes retry budget - /// immediately instead of waiting out the window. - FailedRetriable, - /// Device reported a terminal failure (corrupt image, configuration - /// mismatch). Ends the boot regardless of remaining retry budget — - /// re-running the same image cannot change the verdict. - FailedFatal, -} diff --git a/services/orchestrator/capabilities/src/evidence.rs b/services/orchestrator/capabilities/src/evidence.rs index 9173a0a28..be3e85e0a 100644 --- a/services/orchestrator/capabilities/src/evidence.rs +++ b/services/orchestrator/capabilities/src/evidence.rs @@ -1,9 +1,34 @@ // Licensed under the Apache-2.0 license // SPDX-License-Identifier: Apache-2.0 -//! Evidence reading: resolve a board-defined signal id to boot liveness. +//! Evidence reading: the boot-liveness vocabulary and the reader that +//! resolves a board-defined signal id to it. -use crate::BootStatus; +/// Liveness of a managed device's boot: Boot Confirmation only — whether +/// the device came up, never what booted (that is attestation). +/// +/// The failure variants are optional device-reported evidence, never the +/// only failure path: a hung device reports nothing, so a stuck boot is +/// caught by the observer's timeout, not by this enum. Sources may +/// produce only a subset (a ready pin yields just `Booting`/`Booted`); +/// consumers must handle the full set. +/// +/// A status must describe the **current** boot cycle: latched evidence +/// is cleared by the reset path, never by the reader — a reader that +/// could clear its own evidence would let a read race a reset. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BootStatus { + /// Released, but boot completion not yet observed. + Booting, + /// Boot completion observed. + Booted, + /// Device reported a failure worth another attempt (transient + /// self-test miss); ends the wait early instead of burning the window. + FailedRetriable, + /// Device reported a terminal failure (corrupt image) — re-running + /// the same image cannot change the verdict. + FailedFatal, +} /// Reads a device's boot evidence, one signal at a time. /// diff --git a/services/orchestrator/capabilities/src/lib.rs b/services/orchestrator/capabilities/src/lib.rs index 85f7019eb..0de7196fe 100644 --- a/services/orchestrator/capabilities/src/lib.rs +++ b/services/orchestrator/capabilities/src/lib.rs @@ -27,11 +27,9 @@ #![cfg_attr(not(test), no_std)] mod boot_control; -mod boot_status; mod boot_watch; mod evidence; pub use boot_control::BootControl; -pub use boot_status::BootStatus; pub use boot_watch::{BootWatch, FailureCause, WalkVerdict}; -pub use evidence::EvidenceReader; +pub use evidence::{BootStatus, EvidenceReader}; From 3b87d2438b4086b55f0036713094be29d88e771c Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Fri, 7 Aug 2026 16:47:54 +0200 Subject: [PATCH 14/16] orchestrator: Move component classification into the config schema Kind and failure policy are per-device board policy, so their vocabulary belongs to the device-table schema crate, where boards declare the rest of their boot behavior; the state machine depends on the schema crate and re-exports both types, keeping the reducer's API unchanged while giving the classification a single home next to the table that will declare it. Assisted-by: Claude:claude-fable-5 --- services/orchestrator/config/src/lib.rs | 46 ++++++++++++++++++++++++ services/orchestrator/sm/BUILD.bazel | 1 + services/orchestrator/sm/src/model.rs | 48 +++---------------------- 3 files changed, 51 insertions(+), 44 deletions(-) diff --git a/services/orchestrator/config/src/lib.rs b/services/orchestrator/config/src/lib.rs index f4e742412..b5d4cf6c8 100644 --- a/services/orchestrator/config/src/lib.rs +++ b/services/orchestrator/config/src/lib.rs @@ -12,6 +12,52 @@ #![cfg_attr(not(test), no_std)] +/// How a device in the chain of trust is classified. Declared per device +/// in the board table; the orchestrator supervises accordingly. +/// +/// Corresponds directly to the two-tier model in the CSA architecture +/// document: `Active` = eRoT gate + iRoT gate; `Passive` = eRoT gate +/// only. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ComponentKind { + /// Has an integrated iRoT (e.g. Caliptra). Both the eRoT-side checks + /// (signature + SVN) and the iRoT-side check (local self-verification) + /// apply. The orchestrator waits for the device's readiness report + /// (`ComponentReady`) before advancing the chain walk. + Active, + /// No integrated iRoT. The eRoT's signature + SVN check is the only + /// *trust* gate, so the chain walk advances speculatively after the + /// device's reset release without blocking on readiness. The released + /// device is still watched for boot-progress liveness (`Booted`) + /// under the same per-device watchdog as an `Active` device's + /// readiness: a passive device that never reports in before its + /// timeout is recovered like any other boot failure. CSA + /// boot-progress checkpointing is device-agnostic — every released + /// device owes a boot-progress signal, iRoT or not. + Passive, +} + +/// Recovery-failure classification: what the orchestrator does once a +/// device's restore attempts are **exhausted** (its per-device retry +/// count reaches the board's retry budget). Every verification or +/// corruption failure is retried first, regardless of this +/// classification — CSA's "recover first" principle. This value is +/// consulted only after retries are exhausted. +/// +/// (The narrative design docs sometimes call the `Required` outcome +/// "platform halt" — same behavior, this is the type-level name.) +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FailurePolicy { + /// Stop the boot sequence entirely: the orchestrator locks down. + Required, + /// Hold this device in reset and continue booting the rest of the + /// platform. + Isolable, + /// Hold this device **and** any device whose `depends_on` names it + /// (transitively), then continue booting the rest of the platform. + Cascading, +} + /// One boot checkpoint: a signal the orchestrator waits for, and how long /// it waits. Retry policy is deliberately not table data: a retry /// re-resets the device and re-runs the whole walk, so budgets are diff --git a/services/orchestrator/sm/BUILD.bazel b/services/orchestrator/sm/BUILD.bazel index 6707f9fa0..76bfd8729 100644 --- a/services/orchestrator/sm/BUILD.bazel +++ b/services/orchestrator/sm/BUILD.bazel @@ -14,6 +14,7 @@ rust_library( edition = "2024", visibility = ["//visibility:public"], deps = [ + "//services/orchestrator/config:orchestrator_config", "@rust_crates//:heapless", ], ) diff --git a/services/orchestrator/sm/src/model.rs b/services/orchestrator/sm/src/model.rs index fff7cb6a2..ef542b4a0 100644 --- a/services/orchestrator/sm/src/model.rs +++ b/services/orchestrator/sm/src/model.rs @@ -20,50 +20,10 @@ impl ComponentId { } } -/// How a component in the trust chain is classified. The board supplies one -/// [`ComponentKind`] per [`ComponentId`] when building the chain. -/// -/// Corresponds directly to the two-tier model in the CSA architecture document: -/// `Active` = eRoT gate + iRoT gate; `Passive` = eRoT gate only. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum ComponentKind { - /// Has an integrated iRoT (e.g. Caliptra). Both eRoT-side (signature + SVN) - /// and iRoT-side (local self-verification) checks apply. The machine waits in - /// [`State::AwaitingReady`] for [`Event::ComponentReady`] before advancing. - Active, - /// No integrated iRoT. The eRoT's signature + SVN check is the only *trust* - /// gate, so the chain walk advances speculatively after `ReleaseReset` - /// without blocking in [`State::AwaitingReady`]. The released component is - /// still watched for boot-progress liveness ([`Event::Booted`]) under the - /// same per-component watchdog as an `Active` component's - /// [`Event::ComponentReady`]: a passive device that never reports in before - /// its [`Event::Timeout`] is recovered like any other boot failure. CSA - /// boot-progress checkpointing is device-agnostic — every released device - /// owes a boot-progress signal, iRoT or not. - Passive, -} - -/// Recovery-failure classification: what the machine does once a required -/// component's restore attempts are **exhausted** (its per-component retry -/// count reaches `max_retry`). Every verification or corruption failure enters -/// [`State::Recovering`] and is retried first, regardless of this -/// classification — CSA's "recover first" principle. This value is consulted -/// only after retries are exhausted. -/// -/// (The narrative design docs sometimes call the `Required` outcome "platform -/// halt" — same behavior, this is the type-level name.) -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum FailurePolicy { - /// Stop the boot sequence entirely: self-emits [`Event::RecoveryFailed`], - /// which drives the machine to [`State::Locked`]. - Required, - /// Hold this component in reset (added to `Rot.gated`) and continue - /// booting the rest of the platform. - Isolable, - /// Hold this component **and** any component whose `depends_on` names it - /// (transitively), then continue booting the rest of the platform. - Cascading, -} +// The component-classification vocabulary lives in the device-table schema +// crate (single source of truth: boards declare kind and policy per device in +// their table). Re-exported here so the reducer's API is unchanged. +pub use orchestrator_config::{ComponentKind, FailurePolicy}; /// Opaque recovery-region key supplied by the board at chain-build time. /// Components sharing a `RegionId` are restored together: when any region From 1e4769a74f1dc42bffd152487459f16400fe2ec5 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Fri, 7 Aug 2026 16:49:54 +0200 Subject: [PATCH 15/16] orchestrator: Declare kind, policy, and dependency in the device table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One fact, one home: what a device is (ComponentKind), what its failure means for the platform (FailurePolicy), and which earlier device it cascades with (depends_on, by name) are board facts, so they belong in the board's device table next to its reset wiring and checkpoints. The new const DeviceTable wrapper is the single place cross-entry invariants are checked — non-empty, within the orchestrator's cursor bound, unique device names, and every dependency naming a strictly earlier entry — so an invalid table is unconstructible and fails the build; per-entry constructors cannot see the whole table, which is why the wrapper owns these checks. The mock board declares its archetypes accordingly: the direct-flash bmc is Passive and Required, the self-updating nic is Active and Isolable. Assisted-by: Claude:claude-fable-5 --- services/orchestrator/config/src/lib.rs | 213 ++++++++++++++++++++++-- target/mock/devices.rs | 25 ++- 2 files changed, 219 insertions(+), 19 deletions(-) diff --git a/services/orchestrator/config/src/lib.rs b/services/orchestrator/config/src/lib.rs index b5d4cf6c8..b15f569ab 100644 --- a/services/orchestrator/config/src/lib.rs +++ b/services/orchestrator/config/src/lib.rs @@ -6,9 +6,13 @@ //! device is named here. //! //! Invariants are enforced in the `const fn` constructors, so an invalid -//! table is a build error and there is no validate step to forget. Checks -//! on board-defined types belong next to the table that gives them -//! meaning (`target/mock/devices.rs` shows the pattern). +//! table is a build error and there is no validate step to forget. +//! Per-entry checks live in [`DeviceConfig::new`] and +//! [`BootCheckpoint::new`]; cross-entry checks (unique device names, +//! dependency ordering) live in [`DeviceTable::new`] — each invariant is +//! checked in exactly one place. Checks on board-defined types belong +//! next to the table that gives them meaning (`target/mock/devices.rs` +//! shows the pattern). #![cfg_attr(not(test), no_std)] @@ -134,17 +138,21 @@ impl BootCheckpoint { /// implementation) and its boot-signal vocabulary `G`, for the same /// reason: signal ids are board-specific. /// -/// Deliberately says nothing about attestation or commit requirements: -/// those follow from what kind of device this is (iRoT-backed or -/// symbiont, the orchestrator's `ComponentKind`), not from a table -/// setting — a second knob would only let the two disagree. +/// Attestation and commit requirements are deliberately not separate +/// settings: they follow from [`kind`](Self::kind) (iRoT-backed or +/// symbiont) — a second knob would only let the two disagree. /// /// Fields are private so a device entry that violates the schema is /// unrepresentable: [`new`](Self::new) is the only way in, and it checks. +/// Cross-entry invariants (name uniqueness, dependency ordering) are +/// [`DeviceTable::new`]'s job — one check, one place. #[derive(Debug, Clone, Copy)] pub struct DeviceConfig { name: &'static str, reset_signal: R, + kind: ComponentKind, + failure_policy: FailurePolicy, + depends_on: Option<&'static str>, checkpoints: &'static [BootCheckpoint], } @@ -162,6 +170,8 @@ impl DeviceConfig { pub const fn new( name: &'static str, reset_signal: R, + kind: ComponentKind, + failure_policy: FailurePolicy, checkpoints: &'static [BootCheckpoint], ) -> Self { assert!(!name.is_empty(), "device name must not be empty"); @@ -184,10 +194,25 @@ impl DeviceConfig { Self { name, reset_signal, + kind, + failure_policy, + depends_on: None, checkpoints, } } + /// Builder: declare that this device is held whenever the named + /// device is held (cascade). Only meaningful when the *named* device + /// is [`FailurePolicy::Cascading`]. The name must belong to a device + /// declared **earlier** in the table — checked by + /// [`DeviceTable::new`], not here (a single entry cannot see the + /// table). + #[must_use] + pub const fn with_depends_on(mut self, dependency: &'static str) -> Self { + self.depends_on = Some(dependency); + self + } + /// The device's name in reports and logs. #[must_use] pub const fn name(&self) -> &'static str { @@ -200,6 +225,24 @@ impl DeviceConfig { &self.reset_signal } + /// Trust-chain classification (iRoT gate or not). + #[must_use] + pub const fn kind(&self) -> ComponentKind { + self.kind + } + + /// What happens once this device's recovery is exhausted. + #[must_use] + pub const fn failure_policy(&self) -> FailurePolicy { + self.failure_policy + } + + /// Name of the earlier table entry this device cascades with, if any. + #[must_use] + pub const fn depends_on(&self) -> Option<&'static str> { + self.depends_on + } + /// Boot checkpoints, in the order the device passes them. The device /// counts as booted when the last one is reached; a checkpoint whose /// window expires fails the attempt — whether to retry or recover is @@ -210,6 +253,71 @@ impl DeviceConfig { } } +/// The board's device table: every managed device, in boot order. The +/// only way to get one is [`new`](Self::new), which proves the +/// cross-entry invariants at build time — so holding a `DeviceTable` *is* +/// the proof, and downstream conversions (the orchestrator's chain of +/// trust) need no failure path of their own. +#[derive(Debug, Clone, Copy)] +pub struct DeviceTable { + devices: &'static [DeviceConfig], +} + +impl DeviceTable { + /// Declares the board's device table. `const`, so a bad table is a + /// build error. + /// + /// # Panics + /// + /// Panics — a build error in const context — if the table is empty, + /// longer than `u8::MAX` (the orchestrator's cursor bound), declares + /// two devices with the same name, or contains a `depends_on` that + /// does not name a **strictly earlier** entry (which also rules out + /// dangling and self dependencies: a dependency is always walked + /// before its dependents). + #[must_use] + pub const fn new(devices: &'static [DeviceConfig]) -> Self { + assert!(!devices.is_empty(), "device table must not be empty"); + assert!( + devices.len() <= u8::MAX as usize, + "device table exceeds the orchestrator's cursor bound" + ); + let mut i = 0; + while i < devices.len() { + let mut j = i + 1; + while j < devices.len() { + assert!( + !str_eq(devices[i].name, devices[j].name), + "device names must be unique" + ); + j += 1; + } + if let Some(dep) = devices[i].depends_on { + let mut found_earlier = false; + let mut k = 0; + while k < i { + if str_eq(devices[k].name, dep) { + found_earlier = true; + } + k += 1; + } + assert!( + found_earlier, + "depends_on must name a device declared earlier in the table" + ); + } + i += 1; + } + Self { devices } + } + + /// The devices, in declaration order — which is the boot order. + #[must_use] + pub const fn devices(&self) -> &'static [DeviceConfig] { + self.devices + } +} + // `==` on `&str` is not const; compare bytes by hand. const fn str_eq(a: &str, b: &str) -> bool { let (a, b) = (a.as_bytes(), b.as_bytes()); @@ -243,33 +351,116 @@ mod tests { const CHECKPOINT_DUP: BootCheckpoint = BootCheckpoint::new("boot-complete", 1, Duration::from_secs(1)); + const fn device(name: &'static str) -> DeviceConfig { + DeviceConfig::new( + name, + 0u8, + ComponentKind::Passive, + FailurePolicy::Required, + &[CHECKPOINT], + ) + } + #[test] fn accepts_a_valid_table() { - let device = DeviceConfig::new("dev", 0u8, &[CHECKPOINT]); + const DEVICE: DeviceConfig = DeviceConfig::new( + "dev", + 0u8, + ComponentKind::Active, + FailurePolicy::Isolable, + &[CHECKPOINT], + ); + let table = DeviceTable::new(&[DEVICE]); + let device = &table.devices()[0]; assert_eq!(device.name(), "dev"); assert_eq!(*device.reset_signal(), 0); + assert_eq!(device.kind(), ComponentKind::Active); + assert_eq!(device.failure_policy(), FailurePolicy::Isolable); + assert_eq!(device.depends_on(), None); assert_eq!(device.checkpoints().len(), 1); assert_eq!(device.checkpoints()[0].name(), "boot-complete"); assert_eq!(*device.checkpoints()[0].signal(), 0); assert_eq!(device.checkpoints()[0].timeout(), Duration::from_secs(1)); } + #[test] + fn accepts_a_backward_dependency() { + const ROOT: DeviceConfig = device("root"); + const LEAF: DeviceConfig = device("leaf").with_depends_on("root"); + let table = DeviceTable::new(&[ROOT, LEAF]); + assert_eq!(table.devices()[1].depends_on(), Some("root")); + } + + #[test] + #[should_panic(expected = "device table must not be empty")] + fn rejects_an_empty_table() { + let _ = DeviceTable::new(&[] as &[DeviceConfig]); + } + + #[test] + #[should_panic(expected = "device names must be unique")] + fn rejects_duplicate_device_names() { + const A: DeviceConfig = device("dev"); + const B: DeviceConfig = device("dev"); + let _ = DeviceTable::new(&[A, B]); + } + + #[test] + #[should_panic(expected = "depends_on must name a device declared earlier")] + fn rejects_an_unknown_dependency() { + const LEAF: DeviceConfig = device("leaf").with_depends_on("ghost"); + let _ = DeviceTable::new(&[LEAF]); + } + + #[test] + #[should_panic(expected = "depends_on must name a device declared earlier")] + fn rejects_a_forward_dependency() { + const LEAF: DeviceConfig = device("leaf").with_depends_on("root"); + const ROOT: DeviceConfig = device("root"); + let _ = DeviceTable::new(&[LEAF, ROOT]); + } + + #[test] + #[should_panic(expected = "depends_on must name a device declared earlier")] + fn rejects_a_self_dependency() { + const DEV: DeviceConfig = device("dev").with_depends_on("dev"); + let _ = DeviceTable::new(&[DEV]); + } + #[test] #[should_panic(expected = "checkpoint names must be unique")] fn rejects_duplicate_checkpoint_names() { - let _ = DeviceConfig::new("dev", 0u8, &[CHECKPOINT, CHECKPOINT_DUP]); + let _ = DeviceConfig::new( + "dev", + 0u8, + ComponentKind::Passive, + FailurePolicy::Required, + &[CHECKPOINT, CHECKPOINT_DUP], + ); } #[test] #[should_panic(expected = "device name must not be empty")] fn rejects_an_empty_device_name() { - let _ = DeviceConfig::new("", 0u8, &[CHECKPOINT]); + let _ = DeviceConfig::new( + "", + 0u8, + ComponentKind::Passive, + FailurePolicy::Required, + &[CHECKPOINT], + ); } #[test] #[should_panic(expected = "at least one boot checkpoint")] fn rejects_an_empty_checkpoint_list() { - let _ = DeviceConfig::new("dev", 0u8, &[] as &[BootCheckpoint]); + let _ = DeviceConfig::new( + "dev", + 0u8, + ComponentKind::Passive, + FailurePolicy::Required, + &[] as &[BootCheckpoint], + ); } #[test] diff --git a/target/mock/devices.rs b/target/mock/devices.rs index 98802697e..8c0a9c6ee 100644 --- a/target/mock/devices.rs +++ b/target/mock/devices.rs @@ -9,7 +9,9 @@ use core::time::Duration; -use orchestrator_config::{BootCheckpoint, DeviceConfig}; +use orchestrator_config::{ + BootCheckpoint, ComponentKind, DeviceConfig, DeviceTable, FailurePolicy, +}; /// The mock board's boot-signal vocabulary. The schema carries these /// opaquely; only this board's `EvidenceReader` gives them meaning. @@ -30,30 +32,37 @@ pub enum MockSignal { /// /// The mock board's reset controller addresses reset lines by plain index, /// so the reset id type is `u8`. -pub const MANAGED_DEVICES: &[DeviceConfig] = &[ +pub const MANAGED_DEVICES: DeviceTable = DeviceTable::new(&[ // Direct-flash SPI device (BMC archetype): the eRoT fronts its flash. - // Single checkpoint: it raises a boot-complete GPIO. + // No iRoT, so the eRoT's check is the only trust gate (Passive), and + // the platform is pointless without its BMC (Required). Single + // checkpoint: it raises a boot-complete GPIO. DeviceConfig::new( "bmc", 7, + ComponentKind::Passive, + FailurePolicy::Required, &[BootCheckpoint::new( "boot-complete", MockSignal::Gpio(12), Duration::from_secs(90), )], ), - // PLDM device (NIC archetype): self-updating, SPDM-capable. Two - // checkpoints, exercising the multi-checkpoint path: transport up - // first, then proof the workload is alive. + // PLDM device (NIC archetype): self-updating, SPDM-capable — an iRoT + // of its own (Active), and the platform can serve degraded without it + // (Isolable). Two checkpoints, exercising the multi-checkpoint path: + // transport up first, then proof the workload is alive. DeviceConfig::new( "nic", 3, + ComponentKind::Active, + FailurePolicy::Isolable, &[ BootCheckpoint::new("mctp-ready", MockSignal::MctpReady, Duration::from_secs(20)), BootCheckpoint::new("heartbeat", MockSignal::Heartbeat, Duration::from_secs(10)), ], ), -]; +]); /// Board-local checks the schema constructors cannot do — they know the /// schema's shape, not this board's meanings. Const-fence pattern: a bad @@ -74,4 +83,4 @@ const fn validate_signals(devices: &[DeviceConfig]) { } } -const _: () = validate_signals(MANAGED_DEVICES); +const _: () = validate_signals(MANAGED_DEVICES.devices()); From cce1780f47d6382423fcacff01d89ce08e12fc5a Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Fri, 7 Aug 2026 16:50:33 +0200 Subject: [PATCH 16/16] orchestrator: Derive the chain of trust from the device table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Holding a DeviceTable is proof of the chain invariants — its const constructor already rejected every malformed table at build time — so Chain::from_table is infallible and ChainError with its TryFrom impl, a second copy of the same rules that could drift into disagreement, is deleted; each invariant now has exactly one checker and the type system carries the proof across the crate boundary. The conversion derives everything: table index becomes the component id (declaration order is walk order), kind and policy are copied, and depends_on names resolve to the ids of their earlier entries. The mock board derives its chain capacity and effect-buffer size from the table and declares the one underivable board fact, the retry budget; its new host test drives table, chain, and orchestrator end to end, checking that the passive bmc release advances the walk speculatively. Reducer tests keep building ad-hoc chains through a cfg(test)-only constructor, so no unchecked door exists in production builds. Assisted-by: Claude:claude-fable-5 --- services/orchestrator/sm/src/model.rs | 132 ++++++++++--------------- services/orchestrator/sm/src/tests.rs | 136 +++++++++----------------- target/mock/BUILD.bazel | 10 +- target/mock/devices.rs | 68 ++++++++++++- 4 files changed, 176 insertions(+), 170 deletions(-) diff --git a/services/orchestrator/sm/src/model.rs b/services/orchestrator/sm/src/model.rs index ef542b4a0..361c6c4b5 100644 --- a/services/orchestrator/sm/src/model.rs +++ b/services/orchestrator/sm/src/model.rs @@ -339,94 +339,66 @@ pub enum State { /// A validated **chain of trust**: the ordered list of components the eRoT /// walks, verifies, and supervises, in walk order. /// -/// Build one with [`TryFrom`]/[`TryInto`] from a `heapless::Vec` of -/// `(ComponentId, ComponentAttrs)` pairs. The conversion is the single place -/// the reducer's structural invariants are enforced, so a malformed chain -/// fails closed at the boundary instead of misbehaving later: -/// -/// - the chain is non-empty, -/// - every [`ComponentId`] is unique, -/// - every `depends_on` names a component that exists and appears *strictly -/// earlier* in the chain (no dangling, forward, or self dependencies — a -/// dependency is always walked before its dependents), -/// - the length fits `u8`, the `cursor` index type. -/// -/// ```ignore -/// let mut v = heapless::Vec::<_, 4>::new(); -/// v.push((ComponentId::new(0), ComponentAttrs::passive_required())).unwrap(); -/// let chain: Chain<4> = v.try_into()?; -/// ``` +/// Built from the board's validated [`DeviceTable`](orchestrator_config::DeviceTable) +/// via [`from_table`](Self::from_table). The structural invariants the reducer +/// relies on — non-empty, unique ids, every `depends_on` strictly earlier in +/// the walk, length within the `cursor`'s `u8` bound — are proved once, at +/// build time, by `DeviceTable::new`; holding a table is holding the proof, so +/// the conversion here cannot fail and no second copy of the rules exists. #[derive(Clone, Debug)] pub struct Chain { entries: heapless::Vec<(ComponentId, ComponentAttrs), N>, } -/// Why a `heapless::Vec` of components is not a valid [`Chain`]. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum ChainError { - /// The chain has no components. - Empty, - /// The chain is longer than `u8::MAX`, so `cursor` could not index it. - TooLong, - /// The same [`ComponentId`] appears more than once. - DuplicateId(ComponentId), - /// A `depends_on` names an id that is not in the chain. - UnknownDependency { - component: ComponentId, - depends_on: ComponentId, - }, - /// A `depends_on` names a component that does not appear strictly earlier - /// in the chain (a forward reference or a self-reference). A dependency - /// must be walked before its dependents. - ForwardDependency { - component: ComponentId, - depends_on: ComponentId, - }, -} - impl Chain { - /// Consume the validated chain, yielding its components in walk order. - pub(crate) fn into_entries(self) -> heapless::Vec<(ComponentId, ComponentAttrs), N> { - self.entries + /// Derive the chain of trust from the board's device table: index `i` + /// becomes `ComponentId(i)` (declaration order is walk order), kind and + /// failure policy are copied, and `depends_on` names resolve to the ids + /// of their (strictly earlier) entries. `recovery_region` has no table + /// home yet — nothing consumes it — so every component gets the default + /// region. + /// + /// # Panics + /// + /// Panics if `N` is smaller than the table. Unreachable when `N` is + /// derived from the same table (`table.devices().len()`), which is the + /// only intended call shape. + pub fn from_table(table: &orchestrator_config::DeviceTable) -> Self { + let devices = table.devices(); + assert!( + devices.len() <= N, + "chain capacity N is smaller than the device table" + ); + let mut entries = heapless::Vec::new(); + for (i, device) in devices.iter().enumerate() { + let depends_on = device.depends_on().map(|dep| { + let position = devices + .iter() + .position(|d| d.name() == dep) + .expect("DeviceTable::new proved every dependency exists"); + ComponentId::new(position as u8) + }); + let attrs = ComponentAttrs { + kind: device.kind(), + failure_policy: device.failure_policy(), + recovery_region: RegionId::new(0), + depends_on, + }; + let _ = entries.push((ComponentId::new(i as u8), attrs)); + } + Self { entries } } -} -impl TryFrom> for Chain { - type Error = ChainError; + /// Test-only back door for reducer tests that build ad-hoc chains + /// without a board table. Not compiled into production builds, so + /// `from_table` stays the only way to obtain a `Chain` there. + #[cfg(test)] + pub(crate) fn new_unchecked(entries: heapless::Vec<(ComponentId, ComponentAttrs), N>) -> Self { + Self { entries } + } - fn try_from( - entries: heapless::Vec<(ComponentId, ComponentAttrs), N>, - ) -> Result { - if entries.is_empty() { - return Err(ChainError::Empty); - } - if entries.len() > u8::MAX as usize { - return Err(ChainError::TooLong); - } - for (i, (id, _)) in entries.iter().enumerate() { - if entries[..i].iter().any(|(prev, _)| prev == id) { - return Err(ChainError::DuplicateId(*id)); - } - } - for (i, (id, attrs)) in entries.iter().enumerate() { - if let Some(dep) = attrs.depends_on { - match entries.iter().position(|(cid, _)| *cid == dep) { - None => { - return Err(ChainError::UnknownDependency { - component: *id, - depends_on: dep, - }); - } - Some(j) if j >= i => { - return Err(ChainError::ForwardDependency { - component: *id, - depends_on: dep, - }); - } - Some(_) => {} - } - } - } - Ok(Self { entries }) + /// Consume the validated chain, yielding its components in walk order. + pub(crate) fn into_entries(self) -> heapless::Vec<(ComponentId, ComponentAttrs), N> { + self.entries } } diff --git a/services/orchestrator/sm/src/tests.rs b/services/orchestrator/sm/src/tests.rs index df188ba0b..1be69b789 100644 --- a/services/orchestrator/sm/src/tests.rs +++ b/services/orchestrator/sm/src/tests.rs @@ -58,8 +58,7 @@ fn drive( chain: heapless::Vec<(ComponentId, ComponentAttrs), CAPACITY>, script: &[Event], ) -> (Vec, State) { - let mut orch = - Orchestrator::::new(chain.try_into().expect("valid chain"), MAX_RETRY); + let mut orch = Orchestrator::::new(Chain::new_unchecked(chain), MAX_RETRY); let mut platform = Recorder::new(); for &event in script { orch.dispatch(&mut platform, event); @@ -347,7 +346,7 @@ fn retry_count_resets_after_successful_recovery() { let mut c = heapless::Vec::<(ComponentId, ComponentAttrs), CAPACITY>::new(); c.push((C0, ComponentAttrs::passive_required())) .expect("fits"); - let mut orch = Orchestrator::::new(c.try_into().expect("valid chain"), 2); + let mut orch = Orchestrator::::new(Chain::new_unchecked(c), 2); let mut effects = Vec::new(); for ev in [ @@ -392,7 +391,7 @@ fn retry_budget_is_per_component() { .expect("fits"); c.push((C1, ComponentAttrs::passive_required())) .expect("fits"); - let mut orch = Orchestrator::::new(c.try_into().expect("valid chain"), 2); + let mut orch = Orchestrator::::new(Chain::new_unchecked(c), 2); let mut effects = Vec::new(); for ev in [ @@ -422,7 +421,7 @@ fn custom_retry_cap_latches_sooner() { let mut c = heapless::Vec::<(ComponentId, ComponentAttrs), CAPACITY>::new(); c.push((C0, ComponentAttrs::passive_required())) .expect("fits"); - let mut orch = Orchestrator::::new(c.try_into().expect("valid chain"), 1); + let mut orch = Orchestrator::::new(Chain::new_unchecked(c), 1); let mut effects = Vec::new(); for ev in [ BOOT, @@ -447,7 +446,7 @@ fn custom_capacity_walks_full_chain() { c.push((id, ComponentAttrs::passive_required())) .expect("3 fits"); } - let mut orch = Orchestrator::<3, 8>::new(c.try_into().expect("valid chain"), MAX_RETRY); + let mut orch = Orchestrator::<3, 8>::new(Chain::new_unchecked(c), MAX_RETRY); let mut effects = Vec::new(); for ev in [ BOOT, @@ -1422,7 +1421,7 @@ fn locked_is_terminal() { let mut c: heapless::Vec<(ComponentId, ComponentAttrs), CAPACITY> = heapless::Vec::new(); c.push((C0, ComponentAttrs::passive_required())).unwrap(); // max_retry = 1 so the first failed restore latches immediately. - let mut orch = Orchestrator::::new(c.try_into().expect("valid chain"), 1); + let mut orch = Orchestrator::::new(Chain::new_unchecked(c), 1); let mut effects: Vec = Vec::new(); for ev in [BOOT, Event::VerificationFailed(C0), Event::Restored(C0)] { @@ -1485,12 +1484,10 @@ fn isolable_first_component_exhausts_then_walk_continues() { #[test] fn speculative_read_effects_are_emitted_together() { let mut orch = Orchestrator::::new( - chain(&[ + Chain::new_unchecked(chain(&[ (C0, ComponentAttrs::active_required()), (C1, ComponentAttrs::passive_required()), - ]) - .try_into() - .expect("valid chain"), + ])), MAX_RETRY, ); let mut effects: Vec = Vec::new(); @@ -1542,77 +1539,45 @@ fn single_active_chain_goes_directly_to_ready() { ); } -/// An empty component list is not a valid chain of trust. -#[test] -fn chain_rejects_empty() { - let empty = heapless::Vec::<(ComponentId, ComponentAttrs), CAPACITY>::new(); - assert_eq!(Chain::try_from(empty).unwrap_err(), ChainError::Empty); -} - -/// A repeated `ComponentId` is rejected: the reducer's linear id lookups would -/// otherwise be ambiguous. -#[test] -fn chain_rejects_duplicate_id() { - let v = chain(&[ - (C0, ComponentAttrs::passive_required()), - (C0, ComponentAttrs::passive_required()), - ]); - assert_eq!(Chain::try_from(v).unwrap_err(), ChainError::DuplicateId(C0),); -} - -/// A `depends_on` that names a component not in the chain is rejected. -#[test] -fn chain_rejects_unknown_dependency() { - let v = chain(&[(C1, ComponentAttrs::passive_required().with_depends_on(C0))]); - assert_eq!( - Chain::try_from(v).unwrap_err(), - ChainError::UnknownDependency { - component: C1, - depends_on: C0, - }, - ); -} - -/// A dependency must appear strictly earlier in the walk than its dependent; -/// a forward reference is rejected. -#[test] -fn chain_rejects_forward_dependency() { - let v = chain(&[ - (C0, ComponentAttrs::passive_required().with_depends_on(C1)), - (C1, ComponentAttrs::passive_cascading()), - ]); - assert_eq!( - Chain::try_from(v).unwrap_err(), - ChainError::ForwardDependency { - component: C0, - depends_on: C1, - }, - ); -} +/// The chain is derived from the board's `DeviceTable`: index becomes id, +/// kind/policy are copied, and `depends_on` names resolve to the ids of +/// their earlier entries. The table's invariants are proved at its own +/// construction (`DeviceTable::new`, tested in `orchestrator-config`), so +/// this conversion is infallible — only the mapping itself needs checking. +#[test] +fn chain_derives_from_device_table() { + use core::time::Duration; + use orchestrator_config::{BootCheckpoint, DeviceConfig, DeviceTable}; + + const CHECKPOINT: BootCheckpoint = + BootCheckpoint::new("boot-complete", 0, Duration::from_secs(1)); + const ROOT: DeviceConfig = DeviceConfig::new( + "root", + 0u8, + ComponentKind::Passive, + FailurePolicy::Cascading, + &[CHECKPOINT], + ); + const LEAF: DeviceConfig = DeviceConfig::new( + "leaf", + 1u8, + ComponentKind::Active, + FailurePolicy::Required, + &[CHECKPOINT], + ) + .with_depends_on("root"); + const TABLE: DeviceTable = DeviceTable::new(&[ROOT, LEAF]); -/// A component may not depend on itself. -#[test] -fn chain_rejects_self_dependency() { - let v = chain(&[(C0, ComponentAttrs::passive_required().with_depends_on(C0))]); + let entries = Chain::::from_table(&TABLE).into_entries(); assert_eq!( - Chain::try_from(v).unwrap_err(), - ChainError::ForwardDependency { - component: C0, - depends_on: C0, - }, + entries.as_slice(), + &[ + (C0, ComponentAttrs::passive_cascading()), + (C1, ComponentAttrs::active_required().with_depends_on(C0)), + ], ); } -/// A well-formed chain with a backward dependency validates successfully. -#[test] -fn chain_accepts_valid_dependency() { - let v = chain(&[ - (C0, ComponentAttrs::passive_cascading()), - (C1, ComponentAttrs::passive_required().with_depends_on(C0)), - ]); - assert!(Chain::try_from(v).is_ok()); -} - /// A [`Platform`] that records every effect and fails a chosen one, to exercise /// the effect failure channel. struct FailOn { @@ -1649,8 +1614,7 @@ impl Platform for FailOn { fn effect_failure_latches_lockdown() { let mut c = heapless::Vec::<(ComponentId, ComponentAttrs), CAPACITY>::new(); c.push((C0, ComponentAttrs::passive_required())).unwrap(); - let mut orch = - Orchestrator::::new(c.try_into().expect("valid chain"), MAX_RETRY); + let mut orch = Orchestrator::::new(Chain::new_unchecked(c), MAX_RETRY); let mut plat = FailOn::new(Effect::ReleaseReset(C0)); orch.dispatch(&mut plat, BOOT); // ReadFirmware/VerifyFirmware C0 — both succeed @@ -1668,8 +1632,7 @@ fn failed_isolation_actuation_latches_lockdown() { let mut c = heapless::Vec::<(ComponentId, ComponentAttrs), CAPACITY>::new(); c.push((C0, ComponentAttrs::passive_required())).unwrap(); c.push((C1, ComponentAttrs::passive_isolable())).unwrap(); - let mut orch = - Orchestrator::::new(c.try_into().expect("valid chain"), MAX_RETRY); + let mut orch = Orchestrator::::new(Chain::new_unchecked(c), MAX_RETRY); let mut plat = FailOn::new(Effect::AssertReset(C1)); orch.dispatch(&mut plat, BOOT); @@ -1689,8 +1652,7 @@ fn failed_isolation_actuation_latches_lockdown() { fn failed_restore_actuation_latches_lockdown() { let mut c = heapless::Vec::<(ComponentId, ComponentAttrs), CAPACITY>::new(); c.push((C0, ComponentAttrs::passive_required())).unwrap(); - let mut orch = - Orchestrator::::new(c.try_into().expect("valid chain"), MAX_RETRY); + let mut orch = Orchestrator::::new(Chain::new_unchecked(c), MAX_RETRY); let mut plat = FailOn::new(Effect::RecoverComponent(C0)); orch.dispatch(&mut plat, BOOT); @@ -1709,8 +1671,7 @@ fn failed_restore_actuation_latches_lockdown() { fn failed_lockdown_actuation_does_not_loop() { let mut c = heapless::Vec::<(ComponentId, ComponentAttrs), CAPACITY>::new(); c.push((C0, ComponentAttrs::passive_required())).unwrap(); - let mut orch = - Orchestrator::::new(c.try_into().expect("valid chain"), MAX_RETRY); + let mut orch = Orchestrator::::new(Chain::new_unchecked(c), MAX_RETRY); let mut plat = FailOn::new(Effect::LatchLockdown); // An unprovisioned power-on latches immediately; the latch actuation fails. @@ -1736,7 +1697,7 @@ fn failed_lockdown_actuation_does_not_loop() { #[test] fn batch_actuation_is_fail_fast() { let mut orch = Orchestrator::::new( - passive_required(&[C0, C1]).try_into().expect("valid chain"), + Chain::new_unchecked(passive_required(&[C0, C1])), MAX_RETRY, ); let mut plat = FailOn::new(Effect::ReleaseReset(C0)); @@ -1921,8 +1882,7 @@ fn property_verify_before_release_holds_under_random_sequences() { (C1, ComponentAttrs::active_isolable()), (C2, ComponentAttrs::passive_required()), ]); - let mut orch = - Orchestrator::::new(ch.try_into().expect("valid chain"), MAX_RETRY); + let mut orch = Orchestrator::::new(Chain::new_unchecked(ch), MAX_RETRY); let mut platform = Recorder::new(); // Power on first — usually a clean provisioned boot, occasionally a diff --git a/target/mock/BUILD.bazel b/target/mock/BUILD.bazel index a4dbf970f..e247ddb6f 100644 --- a/target/mock/BUILD.bazel +++ b/target/mock/BUILD.bazel @@ -1,7 +1,7 @@ # Licensed under the Apache-2.0 license # SPDX-License-Identifier: Apache-2.0 -load("@rules_rust//rust:defs.bzl", "rust_library") +load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test") package(default_visibility = ["//visibility:public"]) @@ -12,3 +12,11 @@ rust_library( edition = "2024", deps = ["//services/orchestrator/config:orchestrator_config"], ) + +# Host test: the table this board declares is everything the orchestrator +# needs — table → chain → orchestrator, end to end. +rust_test( + name = "devices_test", + crate = ":devices", + deps = ["//services/orchestrator/sm:orchestrator_sm"], +) diff --git a/target/mock/devices.rs b/target/mock/devices.rs index 8c0a9c6ee..b12270442 100644 --- a/target/mock/devices.rs +++ b/target/mock/devices.rs @@ -28,7 +28,8 @@ pub enum MockSignal { /// Declaration order is the boot order: the orchestrator releases devices /// top to bottom, one at a time. This table is the authority — the -/// orchestrator's chain of trust is built from it, never beside it. +/// orchestrator's chain of trust is built from it +/// (`Chain::from_table`), never beside it. /// /// The mock board's reset controller addresses reset lines by plain index, /// so the reset id type is `u8`. @@ -64,6 +65,20 @@ pub const MANAGED_DEVICES: DeviceTable = DeviceTable::new(&[ ), ]); +/// Derived, not declared: the orchestrator's chain capacity is exactly the +/// table's length. +pub const DEVICE_COUNT: usize = MANAGED_DEVICES.devices().len(); + +/// Derived, not declared: the orchestrator's proven effect-buffer floor +/// (`E >= 2 * N + 2`), with no headroom — headroom would be a second, +/// hand-picked number. +pub const EFFECT_CAP: usize = 2 * DEVICE_COUNT + 2; + +/// Consecutive failed-restore attempts per device before its failure +/// policy is consulted. A genuine board fact — not derivable — so it is +/// declared here, next to the rest of the board's boot policy. +pub const MAX_RETRY: u8 = 3; + /// Board-local checks the schema constructors cannot do — they know the /// schema's shape, not this board's meanings. Const-fence pattern: a bad /// signal fails the build. @@ -84,3 +99,54 @@ const fn validate_signals(devices: &[DeviceConfig]) { } const _: () = validate_signals(MANAGED_DEVICES.devices()); + +#[cfg(test)] +mod tests { + extern crate std; + + use std::vec::Vec; + + use openprot_orchestrator_sm::{ + Chain, ComponentId, Effect, Event, Orchestrator, PowerOnResult, State, + }; + + use super::*; + + /// End-to-end handoff: the table this board declares is everything the + /// orchestrator needs. Kind and policy come from the table too — the + /// bmc is `Passive`, so releasing it must advance the walk speculatively + /// instead of blocking in `AwaitingReady`. + #[test] + fn table_feeds_the_orchestrator() { + let chain = Chain::::from_table(&MANAGED_DEVICES); + let mut orch = Orchestrator::::new(chain, MAX_RETRY); + let bmc = ComponentId::new(0); + let nic = ComponentId::new(1); + + let mut effects: Vec = Vec::new(); + orch.dispatch_with(Event::PowerGood(PowerOnResult::Provisioned), |e| { + effects.push(e); + Ok(()) + }); + assert_eq!( + effects, + [Effect::ReadFirmware(bmc), Effect::VerifyFirmware(bmc)] + ); + assert_eq!(orch.state(), State::PreSupervision); + + effects.clear(); + orch.dispatch_with(Event::VerificationPassed(bmc), |e| { + effects.push(e); + Ok(()) + }); + assert_eq!( + effects, + [ + Effect::ReleaseReset(bmc), + Effect::ReadFirmware(nic), + Effect::VerifyFirmware(nic), + ] + ); + assert_eq!(orch.state(), State::PreSupervision); + } +}