From 9dedbc561a17598078737cbddff1572e04417cbd Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 21:14:54 +0200 Subject: [PATCH 01/11] fwmanager: 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 --- services/fwmanager/api/BUILD.bazel | 3 +- services/fwmanager/api/src/boot_control.rs | 2 +- services/fwmanager/api/src/boot_monitor.rs | 180 ------------ services/fwmanager/api/src/boot_status.rs | 36 +++ services/fwmanager/api/src/boot_watch.rs | 127 +++++++++ services/fwmanager/api/src/config.rs | 262 ++++++++++++++---- services/fwmanager/api/src/lib.rs | 27 +- .../hal-adapters/src/gpio_boot_monitor.rs | 40 +-- .../hal-adapters/src/hal_boot_control.rs | 3 +- services/fwmanager/hal-adapters/src/lib.rs | 8 +- target/mock/devices.rs | 50 +++- 11 files changed, 450 insertions(+), 288 deletions(-) delete mode 100644 services/fwmanager/api/src/boot_monitor.rs create mode 100644 services/fwmanager/api/src/boot_status.rs create mode 100644 services/fwmanager/api/src/boot_watch.rs diff --git a/services/fwmanager/api/BUILD.bazel b/services/fwmanager/api/BUILD.bazel index 49662ed9..47bcd440 100644 --- a/services/fwmanager/api/BUILD.bazel +++ b/services/fwmanager/api/BUILD.bazel @@ -7,7 +7,8 @@ rust_library( name = "fwmanager_api", srcs = [ "src/boot_control.rs", - "src/boot_monitor.rs", + "src/boot_status.rs", + "src/boot_watch.rs", "src/config.rs", "src/lib.rs", ], diff --git a/services/fwmanager/api/src/boot_control.rs b/services/fwmanager/api/src/boot_control.rs index ae13cd57..83f46ae6 100644 --- a/services/fwmanager/api/src/boot_control.rs +++ b/services/fwmanager/api/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/fwmanager/api/src/boot_monitor.rs b/services/fwmanager/api/src/boot_monitor.rs deleted file mode 100644 index 96956880..00000000 --- a/services/fwmanager/api/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/fwmanager/api/src/boot_status.rs b/services/fwmanager/api/src/boot_status.rs new file mode 100644 index 00000000..a6f84f9c --- /dev/null +++ b/services/fwmanager/api/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/fwmanager/api/src/boot_watch.rs b/services/fwmanager/api/src/boot_watch.rs new file mode 100644 index 00000000..74b3213a --- /dev/null +++ b/services/fwmanager/api/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/fwmanager/api/src/config.rs b/services/fwmanager/api/src/config.rs index 43e3ed8b..ccb17474 100644 --- a/services/fwmanager/api/src/config.rs +++ b/services/fwmanager/api/src/config.rs @@ -5,6 +5,8 @@ //! (`target//devices.rs`) declare the values; no concrete line or //! device is named here. +use crate::BootStatus; + /// What the orchestrator requires before it commits a staged image. /// /// Intentionally exhaustive (not `#[non_exhaustive]`): adding a variant is @@ -19,65 +21,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"); @@ -92,8 +132,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; } @@ -110,17 +150,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, }; @@ -148,26 +248,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/fwmanager/api/src/lib.rs b/services/fwmanager/api/src/lib.rs index f379d10b..477f4459 100644 --- a/services/fwmanager/api/src/lib.rs +++ b/services/fwmanager/api/src/lib.rs @@ -7,24 +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 (`config::DeviceConfig::checkpoints`) 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 the schema for the per-board device table, 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 -//! `fwmanager-hal-adapters`; other backends (for example an MCTP-ready -//! `BootMonitor`) implement the same traits from their own transport crate. -//! Config values live in the board device tables -//! (`target//devices.rs`). +//! 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 `fwmanager-hal-adapters`. Config +//! values live in the board device tables (`target//devices.rs`). #![cfg_attr(not(test), no_std)] mod boot_control; -mod boot_monitor; +mod boot_status; +mod boot_watch; pub mod config; 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/fwmanager/hal-adapters/src/gpio_boot_monitor.rs b/services/fwmanager/hal-adapters/src/gpio_boot_monitor.rs index d8bf0e12..7428b12d 100644 --- a/services/fwmanager/hal-adapters/src/gpio_boot_monitor.rs +++ b/services/fwmanager/hal-adapters/src/gpio_boot_monitor.rs @@ -1,10 +1,10 @@ // 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 fwmanager_api::{BootMonitor, BootStatus}; +use fwmanager_api::BootStatus; use openprot_hal_blocking::gpio_port::{ ActivePolarity, GpioError, GpioErrorKind, GpioPort, PinMask, }; @@ -13,7 +13,8 @@ use openprot_hal_blocking::gpio_port::{ /// /// 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/fwmanager/hal-adapters/src/hal_boot_control.rs b/services/fwmanager/hal-adapters/src/hal_boot_control.rs index b4ef418a..6d1e01c6 100644 --- a/services/fwmanager/hal-adapters/src/hal_boot_control.rs +++ b/services/fwmanager/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/fwmanager/hal-adapters/src/lib.rs b/services/fwmanager/hal-adapters/src/lib.rs index f4eb93c4..526e7637 100644 --- a/services/fwmanager/hal-adapters/src/lib.rs +++ b/services/fwmanager/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 `fwmanager-api` 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 //! `fwmanager-api` — 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/devices.rs b/target/mock/devices.rs index ea5b7dfe..02eb342e 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 fwmanager_api::config::{BootCheckpoint, BootSignal, CommitPolicy, DeviceConfig}; +use fwmanager_api::BootStatus; +use fwmanager_api::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 6f7370bbc9d6f6a87a54d6ea9c1d7f3499192cc6 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 21:32:32 +0200 Subject: [PATCH 02/11] fwmanager: 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 --- services/fwmanager/api/BUILD.bazel | 1 + services/fwmanager/api/src/config.rs | 223 ++++--------------------- services/fwmanager/api/src/evidence.rs | 139 +++++++++++++++ services/fwmanager/api/src/lib.rs | 11 +- target/mock/devices.rs | 41 ++--- 5 files changed, 198 insertions(+), 217 deletions(-) create mode 100644 services/fwmanager/api/src/evidence.rs diff --git a/services/fwmanager/api/BUILD.bazel b/services/fwmanager/api/BUILD.bazel index 47bcd440..fc1674c9 100644 --- a/services/fwmanager/api/BUILD.bazel +++ b/services/fwmanager/api/BUILD.bazel @@ -10,6 +10,7 @@ rust_library( "src/boot_status.rs", "src/boot_watch.rs", "src/config.rs", + "src/evidence.rs", "src/lib.rs", ], edition = "2024", diff --git a/services/fwmanager/api/src/config.rs b/services/fwmanager/api/src/config.rs index ccb17474..53155745 100644 --- a/services/fwmanager/api/src/config.rs +++ b/services/fwmanager/api/src/config.rs @@ -5,8 +5,6 @@ //! (`target//devices.rs`) declare the values; no concrete line or //! device is named here. -use crate::BootStatus; - /// What the orchestrator requires before it commits a staged image. /// /// Intentionally exhaustive (not `#[non_exhaustive]`): adding a variant is @@ -21,103 +19,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"); @@ -150,77 +103,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, }; @@ -248,7 +142,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 }]); } @@ -259,57 +156,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/services/fwmanager/api/src/evidence.rs b/services/fwmanager/api/src/evidence.rs new file mode 100644 index 00000000..5515c4ca --- /dev/null +++ b/services/fwmanager/api/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/fwmanager/api/src/lib.rs b/services/fwmanager/api/src/lib.rs index 477f4459..8468f2b5 100644 --- a/services/fwmanager/api/src/lib.rs +++ b/services/fwmanager/api/src/lib.rs @@ -7,10 +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 (`config::DeviceConfig::checkpoints`) 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 (`config::BootCheckpoint`), 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 +30,9 @@ mod boot_control; mod boot_status; mod boot_watch; pub mod config; +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/target/mock/devices.rs b/target/mock/devices.rs index 02eb342e..43967f3b 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 fwmanager_api::BootStatus; use fwmanager_api::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 d31e124ecabb329f29599fc180b196923ba7dd0f Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 21:47:42 +0200 Subject: [PATCH 03/11] fwmanager: 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 --- services/fwmanager/api/src/boot_status.rs | 25 ++++++++++++------ services/fwmanager/api/src/boot_watch.rs | 13 +++++---- services/fwmanager/api/src/evidence.rs | 32 ++++++++++++++++++----- 3 files changed, 51 insertions(+), 19 deletions(-) diff --git a/services/fwmanager/api/src/boot_status.rs b/services/fwmanager/api/src/boot_status.rs index a6f84f9c..3936356d 100644 --- a/services/fwmanager/api/src/boot_status.rs +++ b/services/fwmanager/api/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/fwmanager/api/src/boot_watch.rs b/services/fwmanager/api/src/boot_watch.rs index 74b3213a..116be844 100644 --- a/services/fwmanager/api/src/boot_watch.rs +++ b/services/fwmanager/api/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/fwmanager/api/src/evidence.rs b/services/fwmanager/api/src/evidence.rs index 5515c4ca..4a764d76 100644 --- a/services/fwmanager/api/src/evidence.rs +++ b/services/fwmanager/api/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 3bd8aba0c41bdc6759004108983a7ba1e3f61aed Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 22:00:23 +0200 Subject: [PATCH 04/11] fwmanager: 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 --- services/fwmanager/api/src/evidence.rs | 172 +++++++++++++++++++++++++ 1 file changed, 172 insertions(+) diff --git a/services/fwmanager/api/src/evidence.rs b/services/fwmanager/api/src/evidence.rs index 4a764d76..5f8f0f0d 100644 --- a/services/fwmanager/api/src/evidence.rs +++ b/services/fwmanager/api/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 716384ed19ef20f44b8f8c1ddcab8db1cef72cf4 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 22:38:38 +0200 Subject: [PATCH 05/11] fwmanager: 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 --- services/fwmanager/api/src/boot_watch.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/services/fwmanager/api/src/boot_watch.rs b/services/fwmanager/api/src/boot_watch.rs index 116be844..28de7263 100644 --- a/services/fwmanager/api/src/boot_watch.rs +++ b/services/fwmanager/api/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 1039660f2b53cb369df9754372d8db473852de93 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 22:38:38 +0200 Subject: [PATCH 06/11] fwmanager: 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 --- services/fwmanager/api/src/evidence.rs | 30 ++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/services/fwmanager/api/src/evidence.rs b/services/fwmanager/api/src/evidence.rs index 5f8f0f0d..45487281 100644 --- a/services/fwmanager/api/src/evidence.rs +++ b/services/fwmanager/api/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 `fwmanager-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 da23a4b8ccf2f03d52c1afe0431b2a48a9a13aa1 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 22:38:38 +0200 Subject: [PATCH 07/11] fwmanager: 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/fwmanager/api/src/config.rs | 45 ++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/services/fwmanager/api/src/config.rs b/services/fwmanager/api/src/config.rs index 53155745..aba4a623 100644 --- a/services/fwmanager/api/src/config.rs +++ b/services/fwmanager/api/src/config.rs @@ -39,6 +39,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, } @@ -88,12 +89,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::*; @@ -123,6 +153,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 2134206463dacef0b9e1c7a1c430e1853df2cbac Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Wed, 5 Aug 2026 22:50:19 +0200 Subject: [PATCH 08/11] fwmanager: 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/fwmanager/api/src/config.rs | 10 +++++++--- target/mock/devices.rs | 23 ++++++++++++++++++++++- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/services/fwmanager/api/src/config.rs b/services/fwmanager/api/src/config.rs index aba4a623..d2873c70 100644 --- a/services/fwmanager/api/src/config.rs +++ b/services/fwmanager/api/src/config.rs @@ -33,7 +33,10 @@ 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`](crate::EvidenceReader). 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. @@ -69,8 +72,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 43967f3b..51430a10 100644 --- a/target/mock/devices.rs +++ b/target/mock/devices.rs @@ -67,4 +67,25 @@ pub const MANAGED_DEVICES: &[DeviceConfig] = &[ }, ]; -const _: () = fwmanager_api::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 _: () = { + fwmanager_api::config::validate(MANAGED_DEVICES); + validate_signals(MANAGED_DEVICES); +}; From 1eab413cb3fd66e72018eff7b27ed61ae6f2e0af Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Thu, 6 Aug 2026 10:50:09 +0200 Subject: [PATCH 09/11] fwmanager: 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/fwmanager/api/src/config.rs | 21 +++++---------------- target/mock/devices.rs | 4 +--- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/services/fwmanager/api/src/config.rs b/services/fwmanager/api/src/config.rs index d2873c70..8ca01270 100644 --- a/services/fwmanager/api/src/config.rs +++ b/services/fwmanager/api/src/config.rs @@ -5,20 +5,6 @@ //! (`target//devices.rs`) declare the values; no concrete line or //! device is named here. -/// 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. /// @@ -56,6 +42,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, @@ -65,7 +56,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 @@ -149,7 +139,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 51430a10..0d54e89c 100644 --- a/target/mock/devices.rs +++ b/target/mock/devices.rs @@ -9,7 +9,7 @@ use core::time::Duration; -use fwmanager_api::config::{BootCheckpoint, CommitPolicy, DeviceConfig}; +use fwmanager_api::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 e550a204cb8d716d46a4513872357cdc1a7c0b8c Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Thu, 6 Aug 2026 10:55:08 +0200 Subject: [PATCH 10/11] fwmanager: 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 --- services/fwmanager/api/src/boot_watch.rs | 74 +++++++++++++----------- services/fwmanager/api/src/config.rs | 13 ++--- services/fwmanager/api/src/lib.rs | 2 +- target/mock/devices.rs | 3 - 4 files changed, 48 insertions(+), 44 deletions(-) diff --git a/services/fwmanager/api/src/boot_watch.rs b/services/fwmanager/api/src/boot_watch.rs index 28de7263..6c05cc88 100644 --- a/services/fwmanager/api/src/boot_watch.rs +++ b/services/fwmanager/api/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/fwmanager/api/src/config.rs b/services/fwmanager/api/src/config.rs index 8ca01270..afb34ef9 100644 --- a/services/fwmanager/api/src/config.rs +++ b/services/fwmanager/api/src/config.rs @@ -5,8 +5,10 @@ //! (`target//devices.rs`) declare the values; no concrete line or //! device is named here. -/// 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 @@ -27,9 +29,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. @@ -54,7 +53,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], } @@ -132,7 +132,6 @@ mod tests { name: "boot-complete", signal: 0, timeout: Duration::from_secs(1), - max_retries: 1, }; const DEVICE: DeviceConfig = DeviceConfig { diff --git a/services/fwmanager/api/src/lib.rs b/services/fwmanager/api/src/lib.rs index 8468f2b5..5b691b80 100644 --- a/services/fwmanager/api/src/lib.rs +++ b/services/fwmanager/api/src/lib.rs @@ -34,5 +34,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/target/mock/devices.rs b/target/mock/devices.rs index 0d54e89c..9ae9499e 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 5dc453af602db548153dafe083b19c647b57cb98 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Thu, 6 Aug 2026 11:31:31 +0200 Subject: [PATCH 11/11] fwmanager: Pin the orchestrator seams in the docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three facts reviewers keep having to reconstruct: 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/fwmanager/api/src/boot_watch.rs | 5 ++++- services/fwmanager/api/src/config.rs | 9 +++++++-- target/mock/devices.rs | 3 ++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/services/fwmanager/api/src/boot_watch.rs b/services/fwmanager/api/src/boot_watch.rs index 6c05cc88..94276c77 100644 --- a/services/fwmanager/api/src/boot_watch.rs +++ b/services/fwmanager/api/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/fwmanager/api/src/config.rs b/services/fwmanager/api/src/config.rs index afb34ef9..7d482912 100644 --- a/services/fwmanager/api/src/config.rs +++ b/services/fwmanager/api/src/config.rs @@ -26,8 +26,13 @@ pub struct BootCheckpoint { /// 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 9ae9499e..10ed4c8f 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`.