orchestrator: Add the platform-driver skeleton behind the Platform seam - #418
orchestrator: Add the platform-driver skeleton behind the Platform seam#418chrysh wants to merge 4 commits into
Conversation
294a48f to
2af78c4
Compare
ImageSource is board-supplied access to a component's active image (interposed flash, PLDM transfer, test double); Verifier judges it, and its error means the check could not run — a bad image is Verdict::Rejected, never an error. BoardTypes and Board bundle one board's types and wired instances; a new seam adds an associated type and a field, not another parameter. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
One executor method per SM effect, each documenting its obligation from the platform-boundary contract (orchestrator-model.md §6); unimplemented executors fail closed. read_firmware opens the image source; verify_firmware has the verifier judge it and queues the verdict, which the driver loop feeds back via take_event. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
2af78c4 to
4556f0f
Compare
|
In #357 we discussed not naming a component shell due to the ambiguity of this name. |
PR OpenPRoT#357 already settled this word: 'shell' reads as bash, and the docs say platform driver. The crate follows: PlatformDriver in driver.rs, DriverError, openprot_orchestrator_driver. The outer pump is now the event loop, freeing 'driver' for the struct. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
chrysh
changed the title
True, I renamed it to PlatformDriver |
The executor stages the image (opens the source, marks it staged); it does not read it. Review on OpenPRoT#418 flagged the mismatch. The SM effect stays ReadFirmware — only the driver method is renamed. Assisted-by: Claude:claude-fable-5 Signed-off-by: Christina Quast <christina.quast@9elements.com> Claude-Session: https://claude.ai/code/session_018vLnXX7edp1R5YxQA3FM3h
| #[derive(Debug, Clone, Copy, PartialEq, Eq)] | ||
| pub enum Verdict { | ||
| /// Reported as `Event::VerificationPassed`. | ||
| Authentic, |
There was a problem hiding this comment.
nit: if we cryptographically authenticated an image, I would name it Authenticated, since this is common naming in cryptographic contexts.
| if self.staged != Some(id) { | ||
| return Err(ShellError::NoImage); | ||
| } |
There was a problem hiding this comment.
NoImage seems confusing, since this code location should inform the API consumer that the image is not staged and hence cannot be read, not that there is no image in the system for the Id, right?
Maybe use NotStaged or sth similar.
| .board | ||
| .images | ||
| .get_mut(id.get() as usize) | ||
| .ok_or(ShellError::UnknownComponent)?; |
There was a problem hiding this comment.
Shouldn't the id validity check happen before we check if it is staged? Also, if the ID does not exist for the board images, it could never have been staged, right?
| fn valid_image() -> std::vec::Vec<u8> { | ||
| let mut image = std::vec![0u8; IMAGE_LEN]; | ||
| image[..4].copy_from_slice(&IMAGE_MAGIC); | ||
| image[4..IMAGE_LEN - 1].fill(0xAB); | ||
| image[IMAGE_LEN - 1] = image[..IMAGE_LEN - 1].iter().fold(0, |acc, b| acc ^ b); | ||
| image | ||
| } |
There was a problem hiding this comment.
Will we at some point verify this exact image format? Then it would make sense to create an abstraction struct for this and serialze/deserialze it in a generic way and implement verification methods for this.
I agree it is overkill for a unit test, but when we have the image format at some point, we should remove this; it looks messy, and raw byte generation is prone to later image change regressions.
| pub fn release_reset(&mut self, _id: ComponentId) -> Result<(), DriverError> { | ||
| Err(DriverError::NotImplemented) | ||
| } |
There was a problem hiding this comment.
Instead of stubbing these functions, why not provide them by traits and extend the generic inherent implementation block over these traits? Something like:
type BootControl: orchestrator_capabilities::BootControl;For the board, incorporate
pub boot_controls: [B::BootControl; N]And then have a generic implementation
pub fn assert_reset(&mut self, id: ComponentId) -> Result<(), DriverError> {
self.board
.boot_controls
.get_mut(id.get() as usize)
.ok_or(DriverError::UnknownComponent)?
.hold_in_reset()
.map_err(|_| DriverError::BootControlFault)
}3 participants
TL;DR: Adds
services/orchestrator/driver, the effect-executing layer around the orchestrator state machine — the platform driver (#357's name for it; the last commit renames it from the working name "shell"). Only a skeleton: the actual implementation lands step by step.board.rs(trait-only commit):ImageSource— board-supplied access to a component's active firmware image (interposed flash, PLDM transfer, test double);Verifier— judges an image, where an error means the check could not run (a bad image isVerdict::Rejected, so an actuation fault cannot forge a verdict);BoardTypes/Board— one marker type and one bundle struct per board, so a new seam adds an associated type and a field, never another parameter.driver.rs:PlatformDriver, one executor method per SM effect, each documenting its obligation from the platform-boundary contract (orchestrator-model.md §6).read_firmware/verify_firmwareare implemented; the rest returnNotImplementedand the SM fail-closes on them.PlatformDriver::take_event. orchestrator: Let Platform::execute return a follow-up event #419 proposes lettingPlatform::executereturn the event instead, which deletes this queue.The platform driver is board-agnostic: mock vs hardware is invisible from inside. Tests drive the real SM against a RAM-backed board.