Repository navigation
Conversation
Return RestoreOutcome instead of a bare Ok, so an implementor can report source exhaustion without it looking like an actuation fault. SourceExhausted travels on the Ok side, the platform driver reports Event::RecoveryUnavailable, and the component is gated per policy instead of the whole platform locking. Matches the error contract in OpenPRoT#473. Assisted-by: Claude:claude-opus-4-6 Signed-off-by: Christina Quast <christina.quast@9elements.com>
The doc said "execute may block until the effect completes", which contradicts the IPC design where the orchestrator must stay responsive to async events (CompromiseDetected, timer expiry, nudge signals). Rewrite the contract to require non-blocking return and route I/O-bound results (verification from a remote crypto service, boot progress) through dispatch instead. In-process results that are available immediately (e.g. a local verification verdict) can still be returned as Ok(Some(event)). Assisted-by: Claude
Signed-off-by: Chris Frantz <cfrantz@google.com>
Add async_transact_start (unsafe), async_transact_complete, and async_cancel to IpcChannel, mirroring the kernel's channel_async_transact/_complete/_cancel syscalls. Readiness is polled via object_wait/wait_group_add on Signals::READABLE rather than a Future, matching the kernel's poll-via-signal model.
Splits IpcChannel into IpcInitiator/IpcHandler so a type only implements the operations its channel role can serve. Drops the host.rs stub (crate is now target-only, matching services/i2c/client-ipc and services/mctp/client-ipc) instead of shipping dead panic methods and a drifting copy of buffer.rs. Adds AsyncTransaction<H: IpcInitiator>, a safe layer over the unsafe async trio that tracks the one-transaction-per-channel invariant locally and cancels on Drop, converging with the design in OpenPRoT#482 instead of shipping both. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CI's format check requires deps sorted; fixes the presubmit failure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two-process system image (initiator/handler) modeled on pw_kernel/tests/async_ipc, but built on util_ipc's IpcInitiator/ IpcHandler/AsyncTransaction instead of raw syscalls. Covers blocking transact, async start/cancel (verifying the channel frees up), and async start/try_recv. Verified in QEMU and against rust_binary_no_panics_test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
AsyncTransaction::handle() returned &H, but the only way to get a waitable id from it was reaching into IpcHandle's public field — which only works because H happens to be IpcHandle, defeating the point of being generic over IpcInitiator. as_raw() makes "get the raw handle for object_wait/WaitGroup" part of the trait contract instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Completion and StartError each duplicated Buffers's send/recv fields instead of embedding it, so the same pair had to be kept in sync in three places. Both now hold a `buffers: Buffers` field. Also moves the "why is set_peer_user_signal on IpcHandle and not the traits" rationale from target.rs (where the impl lives) to lib.rs (where a reader looking at IpcInitiator/IpcHandler would actually ask the question). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
start() returns Result<(), StartError> so callers can reclaim buffers on failure, but that meant every call site paid a .map_err(|e| e.error)? tax to unify with surrounding pw_status::Result code, even when the buffers weren't needed back. From<StartError> for Error lets ? convert directly; StartError is still there for callers who do want the buffers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
buildifier wants the //util/ipc shorthand (not //util/ipc:ipc), and rustfmt wants imports sorted; both were flagged by the presubmit format check on fd6aee7. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
target.rs's equivalent unsafe block already carries this suppression; async_transaction.rs's call site was missing it, which is what's failing Security Audit on PR OpenPRoT#479 right now. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… docs cancel() used self.handle.async_cancel()? before taking inflight, so a syscall error left inflight set and the caller's Err carried no buffers back — they became unreachable from safe code. The kernel clears its transaction slot on every path out of the cancel syscall (traced through finish_transaction), so it's always safe to hand the buffers back regardless of the syscall's result; take() them first, unconditionally. Also documents that wrapping the same handle in two AsyncTransactions is a locally-detected Unavailable, not unsoundness, and that new() trusts its caller. Co-Authored-By: chrysh Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Three cases the original three didn't cover: dropping a pending transaction actually cancels it (proves the Drop impl works, not just that it compiles), starting twice while pending hands both buffers back in StartError, and try_recv before the handler responds reports Unavailable with the transaction still pending. The handler gates one request behind a USER signal handshake so the "not yet responded" observation is deterministic instead of a race. The whole sequence now runs 50 rounds to catch state (signals, mostly) leaking between passes. Patch from chrysh's PR review, mutation-tested against 008045a. Co-Authored-By: chrysh Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The walk rounds Duration to whole milliseconds via as_millis(), so a timeout below 1 ms silently becomes zero and the checkpoint expires immediately. Tighten the const assert from "not zero" to "at least 1 ms" so the real rule is enforced at build time. Addresses rusty's review comment on OpenPRoT#471. Assisted-by: Claude (Anthropic)
CheckpointWalk::new was the only constructor in the walk family that validated at runtime (non-empty assert). DeviceConfig::new already checks non-empty and unique-named at build time, so take &'static DeviceConfig<S, P> and drop the runtime assert. Addresses rusty's review comment on OpenPRoT#471. Assisted-by: Claude (Anthropic)
… failure A DeviceFatal BootFailed means the device itself reported its image as bad. For Isolable/Cascading components, going through recovery first is wasted work: gate them immediately via the same policy path as CorruptionDetected, so isolation and cascade happen without a recovery attempt. Required components still enter Recovering. The split is about what gating costs, not about whether a retry could work: recovery can try a different source each attempt, so a golden image may fix any component, but gating an Isolable component only holds it in reset while gating a Required one locks the platform down. The halt-on-exhaustion path applies normally. Also adds Effect::ReportBootFailed (and Report::BootFailed in the driver) so the checkpoint name and failure kind reach management software. The report is emitted before the transition for every non-stale BootFailed, regardless of kind or policy. Closes OpenPRoT#477. Assisted-by: Claude (Anthropic)
Signed-off-by: Chris Frantz <cfrantz@google.com>
The flash-service branch's services/flash/client.rs calls ErrorCode::check_status, which was missing from util/error on this branch; restore it so the baseline compiles.
Adds the AST10x0 flash-service test image's system config, generic kernel target, and server app (flash_server_bin). The server process owns the SCU (pinctrl), FMC register, and CS0 AHB read-window device mappings, and serves services_flash_server::FlashIpcServer requests over the "flash" channel using the Task 3 FMC backend. The client app, system_image, and tests are added by the next task.
Completes the pinmux-ownership change already present in the flash test image (system.json5 no longer maps the SCU into the server process): the backend driver no longer touches the shared SCU; PINCTRL_FMC_QUAD is applied once by the kernel target before any process starts.
…mirrors smc/write)
The orchestrator timer already uses arm and disarm in the watchdog sense. What revert does to a device is clear the marker that makes the next reset boot the test image. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
`trial_closed_as_expected` hands four `u32` fields straight to
`pw_log::error!`, and the macro refuses them: a `{}` argument has to be
a cast expression, whatever type it already has. So every `full_update`
scenario fails to compile, and with it the whole integration suite on
this branch.
The fields are `u32` already, so `as u32` changes no value. It is what
every other pw_log site in these scenarios writes.
Assisted-by: Claude Opus 5
Signed-off-by: Christina Quast <christina.quast@9elements.com>
Three refusals the activation path needs, all of them before the state machine commands anything. An effect that fails locks the platform, so whatever can be refused earlier must be. A candidate that authenticates with no manifest SVN is rejected by the pump: the update ends, the state machine goes back to `Ready`, and the board is told with `Report::SelfImageNoSvn`. An eRoT image carries its SVN in the signed header and the ROM compares it against the anti-rollback counter before booting, so an image without one cannot be checked at all. A managed component is different: its versionless image is installed and its floor is held, because the walk judges it after it reboots. `DriverError::SelfActivateNoSvn` stays as the guard for reaching the activation anyway. If it ever fires, the pump check and the activation disagree, which is worth locking for where a missing version alone is not. The guard names the pump check rather than what the pump reports, so the doc holds if the report changes, and `check_self_update_may_start` says what ends the refusal: the floor commit, or settlement at the next boot. `UpdateBusy` names both of its causes, because a self-update waiting for the floor is not an update in flight. GAPS.md gains the slot parity gap for the eRoT. `commit_self_svn_floor` advances the floor as soon as the trial is confirmed and no pass re-stages the slot the eRoT stopped booting from, so after the advance the ROM rejects that slot and a corruption of the running one leaves nothing to fall back to. `submit_update` refuses an update aimed at the eRoT when the board has no self-image, and when a session is still in flight: a trial being judged, or one confirmed whose SVN the floor has not taken yet. The second would overwrite the session holding that SVN, and the floor would never get it. A trial that already fell back does not block the next attempt, since nothing will confirm it and it is dropped either way. The busy check moves ahead of the self-image check. One update at a time whatever it targets, so a request aimed at the eRoT while a device is being updated is refused for being busy, not for anything about the eRoT. Tested: `bazel test //services/...` passes 38/38 and `bazel test --config=virt_ast10x0 //target/ast10x0/tests/integration/...` passes 36/36. The versionless test and the refusal tests were each checked by removing the code they cover. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
`SelfImage` holds the durable update session, the eRoT floor and the self reset. It holds no image, and the name read as one: a reviewer of the refusal path took `Board.self_image` for the candidate firmware and asked whether the image comes over PLDM. It does, into the staging region, like any other candidate. So the struct is `ErotUpdate`, the bundle a board provides so the eRoT can update itself; the field is `Board.self_update`, and the refusal for a board that wired none is `DriverError::NoErotUpdate`. The struct cannot be `SelfUpdate`, the trait of the session it holds already has that name, so it names the actor instead. `settle_self_image` and `settle_self_image_at_boot` settle the session rather than an image and become `settle_self_update_session` and `settle_self_update_at_boot`. `Report::SelfImageNoSvn` keeps its name. There `SelfImage` means the image for the eRoT, which is the reading this rename leaves to the word. Tested: `bazel test //services/...` passes 38/38 and `bazel test --config=virt_ast10x0 //target/ast10x0/tests/integration/...` passes 36/36. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
`ReportUpdateDeferred` says it is emitted "so the refusal is visible in the effect trace", which reads as every refusal being visible there. The driver refuses an update at submit, before `UpdateRequest` is dispatched: a job already in flight, a candidate that does not fit the staging region, a component the table does not name. Those never reach the state machine, so no effect records them and the frontend answers the requester from the `DriverError` instead. Tested: `bazel test //services/orchestrator/...` passes 10/10. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
Two notes, from checking a contradiction that turned out not to be one. The spare slot re-sync has no scenario because it cannot have one on any board in this tree. `queue_slot_resync` sits below the binding test in `commit_svn_floor`, which returns at once for a component that keeps its own SVN, and all three boards bind `SvnFloorBinding::SelfManaged`. A run needs a device whose floor the eRoT holds, which is a board shape no scenario has yet. `pldm_update/svn_commit` looked like it contradicted that, since it passes. It does not: it asserts the effect rather than the outcome. The machine emits `Effect::CommitSvnFloor` whatever the binding is, and the orchestrator answers the agent on seeing it, but the driver then does nothing with it for a self-managed component. No floor moves in that run or in its refusal variant, which is narrower than the names suggest and is worth saying where the other loose assertions are listed. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
The orchestrator design docs still name the effect `RestoreGoldenImage`.
The state machine emits `Effect::RecoverComponent { id, attempt }`, and the
golden image is one of several sources the driver may resolve, so the old
name also claims more than the effect says.
Both call-style mentions go with it: the entry action now shows the struct
fields, and orchestrator-model.md drops the tuple parentheses.
Assisted-by: Claude Opus 5
Signed-off-by: Christina Quast <christina.quast@9elements.com>
`UpdateIntake` is the contract of the notify/intake design (OpenPRoT#458): an update source offers a candidate and writes it into the orchestrator's staging region a chunk at a time. The FD-as-server design (OpenPRoT#464) is the one the tree follows, and in that shape the firmware device writes staging through the device server and the orchestrator never carries image bytes. Which is what the orchestrator is supposed to be. The overview says the core never reads flash and the platform doc says the orchestrator is not in the data path; `UpdateIntake::write` would have made it the transfer path for an image it only judges. So this crate has no future as written: it has no reverse dependencies, nothing implements it, and leaving it in the tree reads as a seam someone should wire. The `Progress` comparison in the capabilities crate goes with it, since the counterpart it names no longer exists. The contract is not lost. It stays in history at aae0178, with a mock and twelve host tests describing its semantics on the `intake-mock` branch, if the one case this decision does not settle revives it: who writes the eRoT's own inactive slot for a self-update, which is not the orchestrator by the same rule. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
The agent requests the eRoT's floor the way it requests any other component's, with UpdateSecurityRevision, and the state machine answers with `Effect::CommitSvnFloor`. Neither side was wired for it, so the request was dropped and `commit_self_svn_floor` had no caller outside tests. Two halves. Confirming the self trial now marks the eRoT's advance outstanding, the same point `BootConfirmed` marks for a managed device; without it the machine drops the request, which is what it does for a component with nothing to commit. And the driver routes a floor commit aimed at the eRoT to the `ErotUpdate`, because the eRoT's floor is not in its component wiring: it lives next to the session that says which SVN the trial confirmed. The kind decides which path runs, the same way it decides which activation runs. A request with no confirmed session behind it is refused, not quietly ignored, matching the managed path with no verdict recorded. What is still missing is the device side: nothing on the eRoT serves PLDM for its own update, so nothing sends it a 0x22 yet. That needs a firmware-device responder in the orchestrator process and a slot to stage into, and this is the seam it will call. Tested: `bazel test //services/...` passes 37/37 and `bazel test --config=virt_ast10x0 //target/ast10x0/tests/integration/...` passes 36/36. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
`commit_svn_floor` holds the floor advance behind a queued re-sync, and a pass that fails clears the held advance and reports `Report::SlotResyncFailed`. The floor keeps the old SVN, so the image the update replaced installs again, and nothing re-stages the spare or records the slot as stale across a reset. The agent is told the commit succeeded before any of that: the loop sends PerformSvnCommit when the state machine emits `Effect::CommitSvnFloor`, which is dispatched before the pump runs. Found while reviewing the doc comment on `commit_svn_floor` in OpenPRoT#581. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
Eighteen comments pointed at CSA sections for the why. A reader without the document got a section number and no mechanism, and the document is not in this repository. Each one now says what holds in place: a floor above the spare's SVN leaves the spare unbootable, every released device owes a boot-progress signal, recovery runs before the failure policy is consulted, isolating a component is only half of it because management software has to hear that it is down. Comments only, no code change. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
`self_trial` starts from the session an activation would have left and judges the boot after it. Nothing performed that activation, so the half before the reset had no run: the staging, the SVN the verifier reads, and the session the driver records. `self_update` runs it. The chain comes up, the orchestrator offers itself a candidate, the pump stages it in 64-byte steps, the verifier reports the SVN it carries, and `activate_self_update` records the session, marks the trial pending and requests the reset. The reset is where the run ends, so that is where it is judged: every byte of the candidate reached the slot, and the session reads `TrialPending` at the candidate's SVN, which is what the boot after the reset would find. `SelfReset::reset` does not come back, so a scenario that let it return would latch the machine into `Locked` instead. The two scenarios meet at the reset and cannot be one run: nothing on this target resets the eRoT, and the session lives in RAM, so nothing would survive it. That is in GAPS.md, with what closing it needs. The candidate is a constant the scenario serves itself, and the slot counts the bytes rather than writing them. A real one arrives over PLDM into a flash-backed slot, and neither exists yet. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
`Updating` to `Recovering` had no scenario. The machine discards the staged candidate and reports the update superseded only when a corruption transitions it out; a corruption that merely gates the component leaves the update running. That distinction mattered and nothing checked it. `full_update/recovery_preempts` arranges the corruption where it is awkward: the offer is accepted, the device is transferring, and the RoT learns the image the device is running is bad. Three things have to hold together, and each would pass on its own for the wrong reason, so the run asserts all three. The staged candidate is dropped exactly once. The requester is told the update was superseded rather than merely delayed, which is the distinction `ReportUpdateAborted` exists for. Nothing is activated. The board wires no recovery source, so the episode finds none on the first ask and the platform locks, which is where the run ends and what it checks last. GAPS.md records the rest: recovery succeeding after a preemption needs a board with a source, the scenario app dispatches `CorruptionDetected` itself because no platform code raises it, and a `Required` corruption of a component the update is not touching preempts the same way and has no scenario. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
The comments and docs the scenario added named no actor where the driver is one, wrote the bare machine for the state machine, mid-flight for mid-transfer, and arc for the Updating to Recovering transition. The preemption_outcome doc also opened with a verdict the per-check sentences already carry, so the opener goes.
Genitives, passives that hide the driver or the device as the actor, two proven images where the MCUboot word is confirm, ask for a QueryStatus request, and the bare machine for the state machine, across orchestrator_main.rs and scenario.bzl. Words only, in comments and log strings; no logic changes.
Bare floor reads as a building floor or a rate floor. Identifiers stay; no second floor stays too, since the sentence wrote SVN floor just before.
Four docs stated a verdict or pointed at an unnamed mechanism: the ready latch now names drain_reports and BmcReset, the reset counter names what run checks it for, BmcReset names its two methods, and the boot window says which failure it still catches. The three PLDM helpers say firmware device, since the managed device is in the same file. The N doc line went because DEVICES.len() is on the next line.
Entry to `Updating` and `Recovering` cleared `awaiting_floor_advance` whatever component it named. Both states carry an id, so an update or a recovery aimed at one component dropped an SVN floor advance outstanding for another component. The other component kept running its new image, the old one still in its spare slot and its SVN floor low. The SVN floor keeps the SVN it has: the Update Agent sends the commit request once, and the state machine drops a request that finds nothing recorded. Each clear now checks the id first, the way `gate_one` already does. An update aimed at the component itself still voids the SVN floor advance, since the SVN that floor advance would take belongs to the image being replaced, and a recovery for that component also voids the SVN floor advance, since the image the component booted is under suspicion. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
`a_boot_confirmed_with_no_window_open_is_ignored` asserted that no `CommitSvnFloor` followed a `BootConfirmed`. Since the SVN floor moves on the request the Update Agent sends, no input could have made that assertion fail. The test now sends the request, so it tests what its name says: nothing was recorded, so the request finds nothing to move. The test is now named `a_request_with_nothing_recorded_moves_no_floor`. INV13 is the invariant the catalogue was missing: the SVN floor commits at most once per activation, and only on a request that follows a confirm which found the window open. INV13 runs as a property test over random event tails, with the prefix that reaches an activation scripted, because a random sequence almost never produces the request, verdict, re-walk and confirm in order. The run counts the commits it saw and fails if none were reached, so a generator that stops producing that sequence fails here rather than passing over an empty set. Checked against a state machine that keeps the SVN floor advance outstanding after committing: the property fails at the second request. Comments still said the SVN floor commits on `BootConfirmed`, which stopped being true when the request took over. DSP0267 calls it a request, so these files write request, or command for the Effect enum, rather than ask, and SVN floor advance rather than advance or floor. The doc for the commit watchdog claimed the driver arms it and the event loop cancels it; only the ast10x0 runtime test calls `arm_commit`, so the doc now leaves arming to the platform and GAPS.md records the unbounded window. The PLDM role is written Update Agent, so a reader does not take it for any agent. The floor vocabulary said an image proves itself: `BootConfirmed` claimed the image proved itself healthy, `CommitSvnFloor` was decoupled from "the proven boot", and comments refused to commit an "unproven" image. An image runs unconfirmed until a boot confirms it, which is what MCUboot calls the same states, so the docs say that instead. Comments that said work or a cursor was "left alone" now name the operation that does not run. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
Follow-up to PR 608, which already capitalized the DSP0267 role names in services/orchestrator/sm. Does the same in comments, doc comments and markdown across services/orchestrator, services/pldm and target/ast10x0: "update agent" to "Update Agent", "firmware device" to "Firmware Device", wherever the text names the PLDM role rather than a generic agent or downstream device, including bare "the agent" and "the device" where no nearby sentence names the role. Every line this pass touches also gets the style fixes it owes: "ask" becomes "requests" or "sends the request", since DSP0267 names requests; the "Update Agent's" and "Firmware Device's" genitives are restructured so the role is the actor; bare "floor" becomes "SVN floor" where the anti-rollback floor is meant; and the bare "machine" becomes "state machine". Lines the pass does not touch keep their wording. Covers only files I wrote. The four under target/ast10x0/tests/pldm/firmware_update and services/pldm/tests/firmware_update_host.rs keep their wording, so their authors decide the role names there, and PR 515 does not have to rebase over a prose sweep. String literals (log lines, panic messages, assertion text) stay lowercase, matching the convention in those files, which also lowercases "pldm" itself. Assisted-by: Claude Opus 5 Signed-off-by: Christina Quast <christina.quast@9elements.com>
lib.rs declared 4 items and defined 10, which the Google Rust style guide does not allow in a module that has submodules: a non-leaf module holds mod declarations and re-exports only. TransportError, Transport and AsyncTransport go to transport.rs, DispatchError, Dispatch and the From<DispatchError> impl to dispatch.rs. A pure move: the line sets match and the only new lines are the 2 pub use statements, so util_service::TransportError and every other path outside the crate still resolves.
The constructor returns Result, and the style guide spells a fallible constructor try_new. Updates the 2 call sites in the orchestrator driver and the 6 in the test module.
use crate::model::* hid which names come from model and stopped the imports recording what rot.rs depends on. It also resolves the wrong way round when model grows: a local name beats a glob import, so a new public item in model is shadowed by whatever rot.rs already calls that, and the file keeps using the local one. model has 12 public items and rot.rs uses 10: RegionId and ChainError were never needed.
The eRoT serves the OCP Recovery interface as the device and the BMC drives it as the Recovery Agent. This crate holds the half that does not depend on the transport: the command codes 0x22 to 0x2f, the byte layouts, the field encodings and the rules for which accesses the device refuses. The SMBus framing, the PEC and the register file the server-runtime owns come later, so the crate stays host-testable and free of kernel dependencies. The AST1030 has no OCP recovery silicon, so firmware has to keep the access semantics the agent expects from hardware. "Write 1, Device Clears" means the agent writes the command value and the device zeroes the field once it has acted, not a bitwise clear of the written bits: each of the three fields that work that way has an apply_agent_write and a clear_*, and writing 0x1 to the activate field is refused rather than leaving 0xe behind. The writes take a fixed-size array so the length is settled by the type, and RecoveryCommand::check_write resolves it where the bytes arrive off the wire. Signed-off-by: Christina Quast <christina.quast@9elements.com>
chrysh
force-pushed
the
ocp-recovery-registers
branch
from
October 28, 2026 17:10
91c82bd to
f7272a6
Compare
Collaborator
Author
|
Retargeted: the SOW acceptance names upstream OpenPRoT/main, and the OCP demo work integrates on OpenPRoT ocp-global-demo-wip, so the branch was rebased onto that and the PR reopened upstream. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Q4 task 4c, first piece. The eRoT serves the OCP Recovery interface as the Device and the BMC drives it as the Recovery Agent. This crate holds the half that does not depend on the transport: the command codes 0x22 to 0x2f with their access rule, length and availability scope, the byte layouts, the field encodings, and the rules for which accesses the Device refuses.
Scope stops at the AMD design gate: no SMBus framing, no PEC, no staging or verify path, no state machine. Those answers (I2C vs I3C, forced recovery, whether the i2c read path is in 4c scope) change the transport and the entry path, not the byte layouts, so the register definitions can land ahead of them.
The AST1030 has no OCP recovery silicon, so firmware keeps the access semantics the agent expects from hardware.
Write 1, Device Clearson RECOVERY_CTRL byte 2, DEVICE_RESET byte 0 and INDIRECT_FIFO_CTRL byte 1 means the agent writes the command value and the Device zeroes the field once it has acted, not a bitwise clear of the written bits. Each of the three has anapply_agent_writeand aclear_*, and writing 0x1 to the activate field is refused rather than leaving 0xe behind. Caliptra gets that one wrong, which is where the test came from.Writes take a fixed-size array so the length is settled by the type, and
from_wireresolves it once where the bytes arrive.ImageIndexandVendorStatusLenbound the two fields a Device fills, so a Device-side mistake cannot be reported to the agent as a protocol error.Follows the
services/<proto>/apipattern:rust_libraryplusrust_test(crate = ...), zerocopy only, no kernel tags, host-testable.Checked with
bazel test //services/ocp-recovery/api:ocp_recovery_api_test(23 tests), the clippy aspect, and./pw format --all --check.Next pieces, as separate branches:
server/with the register file and dispatch, then the kernel-taggedserver-runtime/.