-
Notifications
You must be signed in to change notification settings - Fork 24
Add boot reset control #352
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
Closed
Closed
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
16c7e79
board: rename monitor to spi_monitor
chrysh b7dc554
spimonitor: prefix bare Monitor* names with SpiMonitor
chrysh 79135e1
fwmanager: Add BootControl trait and HAL adapter
chrysh 54e2764
fwmanager: Extract BootControl into its own module
chrysh 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| # Licensed under the Apache-2.0 license | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| load("@rules_rust//rust:defs.bzl", "rust_library", "rust_test") | ||
|
|
||
| rust_library( | ||
| name = "fwmanager_api", | ||
| srcs = [ | ||
| "src/boot_control.rs", | ||
| "src/lib.rs", | ||
| ], | ||
| edition = "2024", | ||
| visibility = ["//visibility:public"], | ||
| deps = [ | ||
| "//hal/blocking", | ||
| ], | ||
| ) | ||
|
|
||
| # Host tests: build on the host platform, no kernel/QEMU. | ||
| rust_test( | ||
| name = "fwmanager_api_test", | ||
| crate = ":fwmanager_api", | ||
| ) |
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,206 @@ | ||
| // Licensed under the Apache-2.0 license | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| //! Actuation capability: hold a managed device in reset and release it. | ||
|
|
||
| use openprot_hal_blocking::system_control::ResetControl; | ||
|
|
||
| /// Actuation capability: hold a managed device in reset and release it. | ||
| /// | ||
| /// Stateless pass-through by design — sequencing discipline (hold before | ||
| /// release, release only after verification) belongs to the orchestrator | ||
| /// flows, where it is observable behavior. | ||
| /// | ||
| /// # How the orchestrator uses it | ||
| /// | ||
| /// During verified release the orchestrator parks a device in | ||
| /// reset, verifies its active slot while nothing is running, then releases | ||
| /// it to boot the image it just checked: | ||
| /// | ||
| /// ```ignore | ||
| /// // `dev` is this device's BootControl, obtained from the registry. | ||
| /// fn verified_release<D: BootControl>(dev: &mut D) -> Result<(), D::Error> { | ||
| /// dev.hold_in_reset()?; // freeze the device; its flash is now safe to inspect | ||
| /// verify_active_slot()?; // re-hash + signature check (a separate capability) | ||
| /// dev.release()?; // run the just-verified image | ||
| /// Ok(()) | ||
| /// } | ||
| /// ``` | ||
| /// | ||
| /// In a trial boot the same hold/release pair brackets a | ||
| /// watchdog-bounded window; the new slot is committed only if a good boot is | ||
| /// observed, otherwise the device falls back to the previous slot: | ||
| /// | ||
| /// ```ignore | ||
| /// 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)? { | ||
| /// Booted => store.commit(new_slot)?, // observed good => make it active | ||
| /// Failed | Timeout => { /* nothing committed; previous slot still active */ } | ||
| /// } | ||
| /// ``` | ||
| pub trait BootControl { | ||
| /// The error type reported by this device's boot control. | ||
| type Error: core::fmt::Debug; | ||
|
|
||
| /// Holds the device in reset. | ||
| fn hold_in_reset(&mut self) -> Result<(), Self::Error>; | ||
|
|
||
| /// Releases the device from reset. | ||
| fn release(&mut self) -> Result<(), Self::Error>; | ||
| } | ||
|
|
||
| /// Binds one reset line of a HAL reset controller to one managed device. | ||
| pub struct HalBootControl<C: ResetControl> { | ||
| controller: C, | ||
| reset_id: C::ResetId, | ||
| } | ||
|
|
||
| impl<C: ResetControl> HalBootControl<C> { | ||
| /// Creates the binding of `controller`'s line `reset_id` to a device. | ||
| pub fn new(controller: C, reset_id: C::ResetId) -> Self { | ||
| Self { | ||
| controller, | ||
| reset_id, | ||
| } | ||
| } | ||
|
|
||
| /// Read access to the underlying controller. | ||
| pub fn controller(&self) -> &C { | ||
| &self.controller | ||
| } | ||
| } | ||
|
|
||
| impl<C: ResetControl> BootControl for HalBootControl<C> { | ||
| type Error = C::Error; | ||
|
|
||
| fn hold_in_reset(&mut self) -> Result<(), Self::Error> { | ||
| self.controller.reset_assert(&self.reset_id) | ||
| } | ||
|
|
||
| fn release(&mut self) -> Result<(), Self::Error> { | ||
| self.controller.reset_deassert(&self.reset_id) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use core::time::Duration; | ||
| use openprot_hal_blocking::system_control::{Error, ErrorKind, ErrorType}; | ||
|
|
||
| // Normally set in config.rs | ||
| const BMC_LINE: u8 = 7; | ||
|
|
||
| #[derive(Debug, PartialEq, Eq, Clone, Copy)] | ||
| enum Call { | ||
| Assert(u8), | ||
| Deassert(u8), | ||
| } | ||
|
|
||
| #[derive(Debug, PartialEq, Eq, Clone, Copy)] | ||
| struct FakeError(ErrorKind); | ||
|
|
||
| impl Error for FakeError { | ||
| fn kind(&self) -> ErrorKind { | ||
| self.0 | ||
| } | ||
| } | ||
|
|
||
| /// Fake HAL reset controller: records every call it receives. | ||
| struct FakeResetController { | ||
| calls: Vec<Call>, | ||
| fail: Option<ErrorKind>, | ||
| } | ||
|
|
||
| impl FakeResetController { | ||
| fn new() -> Self { | ||
| Self { | ||
| calls: Vec::new(), | ||
| fail: None, | ||
| } | ||
| } | ||
|
|
||
| fn failing(kind: ErrorKind) -> Self { | ||
| Self { | ||
| calls: Vec::new(), | ||
| fail: Some(kind), | ||
| } | ||
| } | ||
|
|
||
| fn calls(&self) -> &[Call] { | ||
| &self.calls | ||
| } | ||
| } | ||
|
|
||
| impl ErrorType for FakeResetController { | ||
| type Error = FakeError; | ||
| } | ||
|
|
||
| impl ResetControl for FakeResetController { | ||
| type ResetId = u8; // Reset line is GPIO here. Real driver should use Enum | ||
|
|
||
| fn reset_assert(&mut self, reset_id: &u8) -> Result<(), FakeError> { | ||
| if let Some(kind) = self.fail { | ||
| return Err(FakeError(kind)); | ||
| } | ||
| self.calls.push(Call::Assert(*reset_id)); | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn reset_deassert(&mut self, reset_id: &u8) -> Result<(), FakeError> { | ||
| if let Some(kind) = self.fail { | ||
| return Err(FakeError(kind)); | ||
| } | ||
| self.calls.push(Call::Deassert(*reset_id)); | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn reset_pulse(&mut self, _: &u8, _: Duration) -> Result<(), FakeError> { | ||
| panic!( | ||
| "BootControl must never pulse: hold and release are distinct orchestrator steps" | ||
| ); | ||
| } | ||
|
|
||
| fn reset_is_asserted(&self, _: &u8) -> Result<bool, FakeError> { | ||
| panic!("BootControl does not query line state"); | ||
| } | ||
| } | ||
|
|
||
| // `hold_in_reset()` must assert exactly the configured line (BMC = 7) | ||
| // and nothing else. | ||
| #[test] | ||
| fn holding_a_device_in_reset_asserts_its_configured_line() { | ||
| let mut bmc = HalBootControl::new(FakeResetController::new(), BMC_LINE); | ||
|
|
||
| bmc.hold_in_reset().expect("hold_in_reset failed"); | ||
|
|
||
| assert_eq!(bmc.controller().calls(), &[Call::Assert(BMC_LINE)]); | ||
| } | ||
|
|
||
| #[test] | ||
| fn holding_a_device_in_reset_deasserts_its_configured_line() { | ||
| let mut bmc = HalBootControl::new(FakeResetController::new(), BMC_LINE); | ||
|
|
||
| bmc.hold_in_reset().expect("hold_in_reset failed"); | ||
| bmc.release().expect("release failed"); | ||
|
|
||
| assert_eq!( | ||
| bmc.controller().calls(), | ||
| &[Call::Assert(BMC_LINE), Call::Deassert(BMC_LINE)] | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn controller_error_propagates_through_boot_control() { | ||
| let mut bmc = HalBootControl::new( | ||
| FakeResetController::failing(ErrorKind::InvalidResetId), | ||
| BMC_LINE, | ||
| ); | ||
| let err = bmc | ||
| .hold_in_reset() | ||
| .expect_err("expected the controller error to propagate"); | ||
| assert_eq!(err.kind(), ErrorKind::InvalidResetId); | ||
| } | ||
| } |
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,15 @@ | ||
| // Licensed under the Apache-2.0 license | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| //! Device-facing capability traits for the Boot Orchestrator. | ||
| //! | ||
| //! `BootControl` is the actuation capability: the orchestrator drives a | ||
| //! single managed device's reset without knowing which controller line it | ||
| //! maps to. The binding of a HAL reset controller line to a device happens | ||
| //! once, in platform configuration, via [`HalBootControl`]. | ||
|
|
||
| #![cfg_attr(not(test), no_std)] | ||
|
|
||
| mod boot_control; | ||
|
|
||
| pub use boot_control::{BootControl, HalBootControl}; |
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
We should keep the overall structure of this code base in mind. The firmware manager will have APIs for several components, so maybe we should encapsulate this code into a separate module, such as
boot_control.rsand keep uselib.rsas top level importer for the other modules to come? For reference see structure ofmctpservice:services/mctp/api/src/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.
yes, I also noticed that, thanks!
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 moved it in the latest commit.