fwmanager: table-declared boot checkpoints replace BootMonitor

[chrysh]

Traits and schema only — the walker implementing them follows in a separate PR, so this one stays reviewable on the contract alone.

What

• BootMonitor is removed. A BootCheckpoint names its evidence as a board-defined signal id (signal: G, timeout, retry budget) — the schema names no signal kinds, and everything that is config is declared in the board device table (target/mock/devices.rs); config.rs only defines what that configuration has to look like.
• EvidenceReader<G> resolves a signal id to BootStatus — board wiring, typically one per managed device, exhaustive match, direct dispatch.
• BootStatus grows device-reported verdicts: FailedRetriable (consumes retry budget immediately instead of waiting out the window) and FailedFatal (ends the boot regardless of budget). Timeouts stay the observer's own judgment — hung devices report nothing; the reader tests pin this down with an MCTP-shaped mock where silence reads Booting forever.
• BootWatch/WalkVerdict is the erased seam the orchestrator polls: Waiting/Complete/Retry/Dead per device, free of device and error types.
• GpioBootMonitor keeps its behavior as a plain reader (inherent method, no trait).

Reading order

The first commit is the checkpoint-embedded-fn variant; the second defunctionalizes it into signal ids and records why (table stays pure data, borrows stay per-device, dispatch stays direct). The pair is kept deliberately as the design record — review the combined diff if you only want the endpoint. Commits three and four add the failure verdicts and the message-path reader tests.

Supersedes

• Boot progress walker 9elements/openprot#22 and fwmanager: Add the boot-progress walk over checkpoint windows #395 (both closed): the old BootWalk/MonitorMap over BootMonitor.
• fwmanager: downstream update lifecycle tests with slot and boot-progress capabilities #392's await_boot will need rework once this lands.

Test

bazel test //services/fwmanager/... (13 api tests), bazel build //target/mock/...; device-table validate runs in const context, so a bad table is a build error.

Migration (for integrators)

BootMonitor is replaced by EvidenceReader<G> + BootStatus; the checkpoint schema changed shape:

// before: one monitor per device, checkpoint carries a schema-level signal kind
BootCheckpoint { name: "boot-complete", signal: BootSignal::GpioBootComplete(12),
                 window: Duration::from_secs(90) }
impl BootMonitor for MyMonitor { fn boot_status(&self) -> Result<BootStatus, E> }

// after: signal id is the board's own enum; window -> timeout; new max_retries
BootCheckpoint { name: "boot-complete", signal: MySignal::Gpio(12),
                 timeout: Duration::from_secs(90), max_retries: 1 }
impl EvidenceReader<MySignal> for MyBmcReader {
    fn read(&mut self, signal: &MySignal) -> Result<BootStatus, E>
}

• max_retries: 0 = the single attempt is all the device gets.
• BootStatus::Failed split into FailedRetriable / FailedFatal (device-reported verdicts; timeouts remain the observer's judgment).
• Checkpoint names must be unique per device (build-time validate).
• WalkVerdict::Retry carries deadline_millis of the re-armed window — schedule against it exactly like Waiting.

[rusty1968]

The CommitPolicy type models a redundant setting. It asks "must this device attest before its update commits?" — but whether a device can attest is already decided by if it is an Active or Passive device (the orchestrator-sm::ComponentKind).

The CSA defines only the iRoT-vs-symbiont distinction and lets attestation follow from it; CommitPolicy adds a separate attestation setting the CSA never defines, so it contradicts the source architecture it's meant to reflect.

This violates DRY (don't repeat yourself).

[chrysh]

Agreed. Dropped in 1eab413. Commit gating follows from ComponentKind (passive or active).

[rusty1968]

Retry & terminal decisions belong to the Orchestrator State Machine

WalkVerdict's retries_left, Retry, and Dead re-implement what orchestrator-sm already owns: ComponentStatus.retry/max_retry and the Recovering→RecoveryFailed path. uplicating the retry count means one side can think retries remain while the other says it's exhausted.

[chrysh]

Agreed, a retry re-resets the device, which restarts the whole walk — you can't retry "kernel" without re-passing bl1 and bl2 — so retry budgets are inherently per boot attempt, and boot attempts are owned by the orchestrator state machine. Fixed in e550a20: WalkVerdict is observation-only now (Waiting / Complete / Failed{checkpoint, cause}), retries_left/Retry/Dead are gone, and max_retries left the device table. The device's own judgment is still signalled by FailureCause::{TimedOut, DeviceRetriable, DeviceFatal}. DeviceFatal says no remaining budget can overturn the verdict.

[chrysh]

@rusty1968 three design notes worth discussing:

Timeouts stay per-checkpoint, and the SM never sees them. The device table declares a window per checkpoint (bl1 can get 200 ms while the service checkpoint gets 30 s); the boot walk consumes those windows and reports expiry as a failed attempt. I think Lou mentioned something about per checkpoint timeouts.

Open question 1 — FailureCause is richer than the SM's event vocabulary. The walk reports why an attempt died: TimedOut, DeviceRetriable, DeviceFatal (device-reported, e.g. a fault code meaning "re-running this image cannot change the verdict"). Today all three only become Event::Timeout(id), and every boot failure enters recovery — so DeviceFatal gets recovery-retried until max_retry exhausts anyway. Should the SM grow a fatal fast-path (straight to RecoveryFailed/isolation), or should we shrink the cause vocabulary until it's needed?

Open question 2 — where does ComponentAttrs get configured? The chain needs kind/failure_policy/depends_on per component, which is per-device board policy — but only the SM's tests supply it today, and after dropping CommitPolicy our table is pure name/reset/checkpoints. Does DeviceConfig grow those fields, or does a separate board-side table feed the chain?

[leongross]

Do we really need an entirely new file for one enum definition? Or are we planning to extend this in the near future?

[leongross]

Can we check this already in the Checkpoint creation?