-
Notifications
You must be signed in to change notification settings - Fork 24
fwmanager: Add BootMonitor capability and GpioBootMonitor adapter #368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
chrysh
wants to merge
3
commits into
OpenPRoT:main
Choose a base branch
from
9elements:add-boot-monitor
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,180 @@ | ||
| // 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. | ||
| /// | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Document this only returns a subset. |
||
| /// 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<BootStatus, Self::Error>; | ||
| } | ||
|
|
||
| #[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<usize>, | ||
| 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<BootStatus, MockFault> { | ||
| 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<M: BootMonitor>(mon: &M, poll_budget: usize) -> Result<bool, M::Error> { | ||
| 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); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets make this
#[non_exhaustive].Also, do we need an
Unknownvariant?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
non_exhaustive: Don't we want every consumer to get a compile error when a value is added, so that they must consciously decide how to handle the new state. non_exhaustive forces consumers to write a _ arm, and since every BootStatus variant is decision-relevant for the orchestrator, a future variant silently falling into that arm seems worse than a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same reasoning for Unknown: a hung device reports nothing, so "unknown" is already covered by the timeout path (Booting until the orchestrator gives up). It would hide a bad state.
Do you have strong opinions for either?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No strong opinions.
I'm fine with unknown always represented by the error path.
Maybe AMD folks have an opinion whether it should be non_exhaustive or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought for a bit and exhaustive is right call here. Adding another variant should be a build error. The change should be one atomic commit that touches not only the enum but every consumer the compiler flags, reviewed together in a single PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed