From 5810d68e991ab6e3e3a8864c40988de39bfe38dd Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 21:14:54 +0200 Subject: [PATCH 01/13] 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 7b873d20..3ff27349 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 ae13cd57..83f46ae6 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 96956880..00000000 --- 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 00000000..a6f84f9c --- /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 00000000..74b3213a --- /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 f5e7b3eb..8b974e87 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 55ad6b8d..5093408c 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 34aae93b..ca6d3208 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 a90cdcb2..3591d2b8 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 f7eb93d2..2f351a52 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 4f71ebdb..3c9d0c59 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 a4dbf970..751ebde4 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 e5c3af88..aa484c51 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/13] 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 3ff27349..09e66160 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 00000000..5515c4ca --- /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 8b974e87..8a655745 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 5093408c..55ad6b8d 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 ca6d3208..f3c8b021 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 751ebde4..a4dbf970 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 aa484c51..0b7d29d5 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/13] 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 a6f84f9c..3936356d 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 74b3213a..116be844 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 5515c4ca..4a764d76 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/13] 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 4a764d76..5f8f0f0d 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/13] 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 116be844..28de7263 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/13] 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 5f8f0f0d..9173a0a2 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/13] 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 f3c8b021..2a63cc6f 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/13] 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 2a63cc6f..1e04d666 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 0b7d29d5..30b6739a 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/13] 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 1e04d666..f4144dc2 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 30b6739a..a5fa71a9 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/13] 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 28de7263..6c05cc88 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 8a655745..85f7019e 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 f4144dc2..ffc34abb 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 a5fa71a9..6008e1b3 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/13] 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 6c05cc88..94276c77 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 ffc34abb..c544a79f 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 6008e1b3..33af731c 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/13] 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 c544a79f..f4e74241 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 33af731c..98802697 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/13] 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 09e66160..011d2e11 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 3936356d..00000000 --- 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 9173a0a2..be3e85e0 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 85f7019e..0de7196f 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};