From 5b13c8bb8012c3c080bd2b09d553a36847d8b2a2 Mon Sep 17 00:00:00 2001 From: Christina Quast Date: Sun, 27 Sep 2026 10:00:41 +0200 Subject: [PATCH] driver: Bring the spare slot up to date after a commit A commit re-stages the image that just proved itself, so the slot the device stopped booting from stops holding the version before the update. Nothing activates afterwards: the payload lands in the inactive slot and stays there, which is what a fallback needs. Re-stages rather than copying between slots, because Updatable keeps slot identity on the device's side. The candidate is still in the staging region, which submit_update and discard_staged keep true by dropping the queued re-sync whenever the region changes hands. The cost is a second full transfer; copying between slots needs a seam that names them, which is a larger decision. The pump runs the pass, not the commit executor: a transfer takes minutes and an executor must return promptly. The SM is never told the result, because a verdict would start a second activation of an image the device already runs. A device that fails or stalls the pass is reported as SlotResyncFailed and the job ends there, with the running image committed either way. A re-sync occupies the job while it runs, so an update requested mid-pass is deferred: the candidate would overwrite the bytes being written. Assisted-by: Claude --- services/orchestrator/driver/src/board.rs | 5 + services/orchestrator/driver/src/driver.rs | 130 +++++++++++++-- services/orchestrator/driver/src/tests.rs | 175 +++++++++++++++++++++ 3 files changed, 298 insertions(+), 12 deletions(-) diff --git a/services/orchestrator/driver/src/board.rs b/services/orchestrator/driver/src/board.rs index f2571c0a..2ae3a67f 100644 --- a/services/orchestrator/driver/src/board.rs +++ b/services/orchestrator/driver/src/board.rs @@ -131,6 +131,11 @@ pub enum Report { /// discarded and no verdict for that request follows. Platform-wide for /// the same reason as [`Report::UpdateDeferred`]. UpdateAborted, + /// The spare slot could not be brought up to date after a commit. + /// The running image is committed and fine; the other slot still + /// holds the version from before this update, so a fallback would + /// boot older firmware. + SlotResyncFailed(ComponentId), } /// Where the driver hands its [`Report`]s. What a report becomes, a log diff --git a/services/orchestrator/driver/src/driver.rs b/services/orchestrator/driver/src/driver.rs index 09d5d03f..5fbe3330 100644 --- a/services/orchestrator/driver/src/driver.rs +++ b/services/orchestrator/driver/src/driver.rs @@ -62,9 +62,10 @@ pub enum DriverError { /// Activation was asked for before the device held the whole /// payload. The job survives, so `DiscardStaged` can still end it. CandidateNotStaged, - /// A staging step ran before the SM commanded the work. The pump - /// only reaches staging through `AuthenticateStageUpdate`, so this - /// means the phase and the flag disagree. + /// A staging step ran before the work was commanded. An update + /// reaches staging through `AuthenticateStageUpdate` and a slot + /// re-sync sets the flag when it queues the job, so this means the + /// phase and the flag disagree. UpdateNotCommanded, } @@ -114,6 +115,10 @@ pub struct PlatformDriver { /// The update job submitted by the frontend. Held until the update is /// activated or discarded. pending_update: Option, + /// The component and candidate length of the last activation, so a + /// commit can re-stage the same payload into the spare slot. Cleared + /// once that re-sync is queued. + last_activated: Option<(ComponentId, u64)>, } /// What one pump call established, before the stall rule is applied. @@ -165,6 +170,10 @@ enum UpdatePhase { /// The device holds the complete payload. The crypto service has not /// started yet. Staged, + /// The committed image is being written a second time, to bring the + /// slot the device just stopped booting from up to date. The SM is + /// not involved: this job ends in the driver. + Resyncing, // Authenticating and Authenticated arrive with the crypto // verify-client trait. Until then the pump parks at Staged. } @@ -199,6 +208,7 @@ impl PlatformDriver { watching: [false; N], verified_svn: [None; N], pending_update: None, + last_activated: None, } } @@ -211,6 +221,12 @@ impl PlatformDriver { &self.board } + /// The board wiring, to fault a capability mid-test. + #[cfg(test)] + pub(crate) fn board_mut(&mut self) -> &mut Board { + &mut self.board + } + /// The frontend half of the update handshake: record `target` as the /// component the staged candidate is for and `len` as how much of the /// staging region the candidate occupies. Must succeed BEFORE @@ -230,6 +246,10 @@ impl PlatformDriver { /// The length stays on the job because the staging region is board /// geometry and usually larger, so the reader needs to know where /// the candidate ends. + /// + /// A slot re-sync counts as in flight: it is writing the staging + /// region's bytes to a device, and a new candidate would overwrite + /// them mid-pass. pub fn submit_update(&mut self, target: ComponentId, len: u64) -> Result<(), DriverError> { self.board .updatables @@ -241,6 +261,9 @@ impl PlatformDriver { if self.pending_update.is_some() { return Err(DriverError::UpdateBusy); } + // The region is about to hold a different candidate, so the last + // activation loses its claim on a re-sync from it. + self.last_activated = None; self.pending_update = Some(UpdateJob { target, len, @@ -271,6 +294,9 @@ impl PlatformDriver { .ok_or(DriverError::UnknownComponent)?; updatable.abandon(); self.pending_update = None; + // Whatever the region held is being dropped, so a later commit + // must not re-stage from it. + self.last_activated = None; Ok(()) } @@ -300,7 +326,7 @@ impl PlatformDriver { // trait is wired into the board. return Err(DriverError::CandidateNotStaged); } - let target = job.target; + let (target, len) = (job.target, job.len); let updatable = self .board .updatables @@ -308,6 +334,7 @@ impl PlatformDriver { .ok_or(DriverError::UnknownComponent)?; updatable.activate().map_err(|_| DriverError::UpdateFault)?; self.pending_update = None; + self.last_activated = Some((target, len)); Ok(()) } @@ -323,6 +350,10 @@ impl PlatformDriver { /// `UpdateVerified` arrives with the crypto verify-client; until /// then the pump parks at `Staged` and returns idle. The job stays /// until the SM answers with `ActivateUpdate` or `DiscardStaged`. + /// + /// A slot re-sync is the exception: it ends here with no event, + /// because the SM never asked for it and a verdict would start a + /// second activation. pub fn pump_update(&mut self, now_millis: u64) -> UpdatePoll { let Some(job) = self.pending_update.as_mut() else { return UpdatePoll::idle(); @@ -343,7 +374,7 @@ impl PlatformDriver { // or the device already holds the payload and the SM owns // the next move. UpdatePhase::Submitted | UpdatePhase::Staged => return UpdatePoll::idle(), - UpdatePhase::Staging => self.poll_staging(), + UpdatePhase::Staging | UpdatePhase::Resyncing => self.poll_staging(), }; let step = match stepped { @@ -362,13 +393,23 @@ impl PlatformDriver { job.progress_since_millis = Some(now_millis); } else if now_millis.saturating_sub(since) >= self.board.update_stall_budget_millis { - return self.reject_job(); + return match phase { + UpdatePhase::Resyncing => self.end_resync(), + _ => self.reject_job(), + }; } UpdatePoll { event: None, progress: Some(progress), } } + // A re-sync ends in the driver. Telling the SM the payload + // is staged would start a second activation of an image the + // device is already running. + Step::Staged if phase == UpdatePhase::Resyncing => { + self.pending_update = None; + UpdatePoll::idle() + } Step::Staged => { job.phase = UpdatePhase::Staged; // Fail secure: no UpdateVerified until the crypto @@ -382,6 +423,10 @@ impl PlatformDriver { progress: None, } } + // A failed re-sync leaves the running image committed and the + // spare slot stale, which is a report rather than a verdict + // the SM acts on. + Step::Rejected if phase == UpdatePhase::Resyncing => self.end_resync(), Step::Rejected => self.reject_job(), } } @@ -411,6 +456,20 @@ impl PlatformDriver { } } + /// Ends a re-sync that failed or stalled. The SM never knew about + /// this job, so nothing else would clear it and the pump would + /// retry the same failure forever. The running image is committed + /// either way, so this is a report, not a verdict. + fn end_resync(&mut self) -> UpdatePoll { + let target = self.pending_update.as_ref().map(|job| job.target); + self.abandon_job(); + self.pending_update = None; + if let Some(target) = target { + self.report(Report::SlotResyncFailed(target)); + } + UpdatePoll::idle() + } + /// Ends the job the way the SM understands: the device drops what it /// staged and the verdict travels as `UpdateRejected`. The job itself /// stays until the SM answers with `DiscardStaged`, so the two sides @@ -419,17 +478,27 @@ impl PlatformDriver { if let Some(job) = self.pending_update.as_mut() { job.phase = UpdatePhase::Submitted; job.prepare_commanded = false; - let target = job.target.get() as usize; - if let Some(updatable) = self.board.updatables.get_mut(target) { - updatable.abandon(); - } } + self.abandon_job(); UpdatePoll { event: Some(Event::UpdateRejected), progress: None, } } + /// Tells the device to drop what it was staging. Leaves the job + /// itself alone: who clears it differs between an update, which the + /// SM answers for, and a re-sync, which ends here. + fn abandon_job(&mut self) { + let Some(job) = self.pending_update.as_ref() else { + return; + }; + let target = job.target.get() as usize; + if let Some(updatable) = self.board.updatables.get_mut(target) { + updatable.abandon(); + } + } + /// Target of the in-flight update, if one was submitted. pub fn pending_update(&self) -> Option { self.pending_update.as_ref().map(|job| job.target) @@ -495,7 +564,44 @@ impl PlatformDriver { return Ok(()); }; let svn = self.verified_svn[idx].ok_or(DriverError::NoVerifiedImage)?; - floor.advance(svn).map_err(|_| DriverError::SvnFloorFault) + floor.advance(svn).map_err(|_| DriverError::SvnFloorFault)?; + self.queue_slot_resync(id); + Ok(()) + } + + /// Queues a second staging pass for the image just committed, so the + /// slot the device stopped booting from stops holding the version + /// before it. The pump runs it and nothing activates afterwards: the + /// payload lands in the inactive slot and stays there. + /// + /// Re-stages rather than copying between slots, because `Updatable` + /// keeps slot identity on the device's side. The candidate is still + /// in the staging region: one job at a time, so nothing has + /// overwritten it since the activation. + /// + /// Silent when there is nothing to re-sync: a confirmed boot with no + /// update behind it, or a region that changed hands since the + /// activation, which `submit_update` and `discard_staged` record by + /// clearing the claim. The in-flight check cannot fire today, because + /// the same two methods clear the claim before a job starts. It stays + /// so a re-sync can never displace a job. A failure during the pass is + /// a report, not an error: the running image is committed either way. + fn queue_slot_resync(&mut self, id: ComponentId) { + let Some((target, len)) = self.last_activated else { + return; + }; + if target != id || self.pending_update.is_some() { + return; + } + self.last_activated = None; + self.pending_update = Some(UpdateJob { + target, + len, + phase: UpdatePhase::Resyncing, + prepare_commanded: true, + progress: Progress::start(len), + progress_since_millis: None, + }); } /// `id`'s reset actuator. @@ -657,7 +763,7 @@ pub struct UpdatePoll { /// at Staged instead. pub event: Option, /// How far the job has come, for the update source's progress - /// report. `None` once there is nothing left to report. + /// report. `None` once the job has ended, whichever way it ended. pub progress: Option, } diff --git a/services/orchestrator/driver/src/tests.rs b/services/orchestrator/driver/src/tests.rs index 332a82ed..a1204531 100644 --- a/services/orchestrator/driver/src/tests.rs +++ b/services/orchestrator/driver/src/tests.rs @@ -325,6 +325,8 @@ struct MockUpdatable { stalls: bool, /// Fails every step. faults: bool, + /// Completed stagings, so a test can tell a re-sync happened. + stagings: usize, } /// Bytes one staging step writes. @@ -341,6 +343,7 @@ impl MockUpdatable { written: 0, stalls: false, faults: false, + stagings: 0, } } @@ -455,6 +458,8 @@ impl orchestrator_capabilities::Updatable for MockUpdatable { }); } self.ready = true; + self.stagings += 1; + self.written = 0; Ok(orchestrator_capabilities::StageProgress::Ready) } @@ -2329,3 +2334,173 @@ fn commit_with_an_unreadable_session_fails_secure() { assert_eq!(floor.floor(), Ok(Svn(3))); } + +// --------------------------------------------------------------------------- +// Bringing the spare slot up to date after a commit +// --------------------------------------------------------------------------- + +/// Runs an update to activation: request, pump until the device holds +/// the payload, then feed the verdict. +/// +/// The verdict is dispatched here rather than read off the pump because +/// the pump parks at `Staged` and emits nothing until the crypto verify +/// client is wired. What follows activation is what these tests are +/// about, so they reach it the way the wired pump will. +fn activated(driver: &mut PlatformDriver, orch: &mut Orchestrator<1, 4>) { + orch.dispatch(driver, Event::PowerGood(PowerOnResult::Provisioned)); + request_update(orch, driver, C0, CANDIDATE_LEN).unwrap(); + for tick in 0..16 { + driver.pump_update(tick); + if driver.board().updatables[0].ready { + break; + } + } + assert!(driver.board().updatables[0].ready, "never staged"); + orch.dispatch(driver, Event::UpdateVerified); + assert!(driver.board().updatables[0].active, "never activated"); +} + +// The committed image is written a second time, into the slot the device +// stopped booting from. Nothing activates afterwards: the device keeps +// running what it just committed. +#[test] +fn a_commit_restages_the_image_into_the_spare_slot() { + let mut orch = orchestrator(); + let mut driver = update_driver(MockUpdatable::new()); + activated(&mut driver, &mut orch); + let staged_before = driver.board().updatables[0].stagings; + + orch.dispatch(&mut driver, Event::BootConfirmed(C0)); + + // The re-sync is queued, not run inside the executor. + assert_eq!(driver.pending_update(), Some(C0)); + for tick in 0..16 { + if driver.pending_update().is_none() { + break; + } + assert_eq!(driver.pump_update(tick).event, None, "the SM is not told"); + } + + assert_eq!(driver.pending_update(), None, "the re-sync ended"); + assert!(driver.board().updatables[0].stagings > staged_before); + assert_eq!(orch.state(), State::Ready); +} + +// A confirmed boot with no update behind it has nothing to re-sync. +#[test] +fn a_commit_without_an_activation_queues_nothing() { + let mut orch = orchestrator(); + let mut driver = update_driver(MockUpdatable::new()); + orch.dispatch(&mut driver, Event::PowerGood(PowerOnResult::Provisioned)); + + driver.commit_svn_floor(C0).expect("commit failed"); + + assert_eq!(driver.pending_update(), None); +} + +// One re-sync per activation: a second confirmed boot does not restage +// an image that is already in both slots. +#[test] +fn a_second_commit_does_not_restage_again() { + let mut orch = orchestrator(); + let mut driver = update_driver(MockUpdatable::new()); + activated(&mut driver, &mut orch); + orch.dispatch(&mut driver, Event::BootConfirmed(C0)); + for tick in 0..16 { + if driver.pending_update().is_none() { + break; + } + driver.pump_update(tick); + } + let staged_after_resync = driver.board().updatables[0].stagings; + + orch.dispatch(&mut driver, Event::BootConfirmed(C0)); + + assert_eq!(driver.pending_update(), None); + assert_eq!(driver.board().updatables[0].stagings, staged_after_resync); +} + +// The running image is committed either way, so a device that fails the +// second pass is reported, not escalated. The job is cleared, or the +// pump would retry the same failure forever. +#[test] +fn a_failed_resync_is_reported_and_ends() { + let mut orch = orchestrator(); + let mut driver = update_driver(MockUpdatable::new()); + activated(&mut driver, &mut orch); + + orch.dispatch(&mut driver, Event::BootConfirmed(C0)); + driver.board_mut().updatables[0].faults = true; + let poll = driver.pump_update(0); + + assert_eq!(poll.event, None, "no verdict reaches the SM"); + assert_eq!(driver.pending_update(), None); + assert!(driver + .board() + .report_sink + .seen + .contains(&Report::SlotResyncFailed(C0))); + assert_eq!(orch.state(), State::Ready); +} + +// A re-sync is writing the region's bytes to a device, so a new +// candidate would overwrite them mid-pass. The frontend is told to come +// back, the same answer it gets during an update. +#[test] +fn a_resync_in_flight_defers_the_next_update() { + let mut orch = orchestrator(); + let mut driver = update_driver(MockUpdatable::stepping(4)); + activated(&mut driver, &mut orch); + orch.dispatch(&mut driver, Event::BootConfirmed(C0)); + assert_eq!(driver.pending_update(), Some(C0), "the re-sync is queued"); + + assert_eq!( + driver.submit_update(C0, CANDIDATE_LEN), + Err(DriverError::UpdateBusy) + ); +} + +// A device that goes quiet mid-re-sync is not an update failure: the SM +// hears nothing and the spare slot is reported stale. +#[test] +fn a_stalled_resync_is_reported_not_rejected() { + let mut orch = orchestrator(); + let mut driver = update_driver(MockUpdatable::new()); + activated(&mut driver, &mut orch); + orch.dispatch(&mut driver, Event::BootConfirmed(C0)); + driver.board_mut().updatables[0].stalls = true; + + assert_eq!(driver.pump_update(0).event, None); + let poll = driver.pump_update(STALL_BUDGET_MILLIS); + + assert_eq!(poll.event, None, "no verdict reaches the SM"); + assert_eq!(driver.pending_update(), None); + assert!(driver + .board() + .report_sink + .seen + .contains(&Report::SlotResyncFailed(C0))); + assert_eq!(orch.state(), State::Ready); +} + +// An update that came and went between the activation and the confirmed +// boot has left different bytes in the region, so the commit must not +// re-stage from it. +#[test] +fn a_discarded_update_cancels_the_pending_resync() { + let mut orch = orchestrator(); + let mut driver = update_driver(MockUpdatable::new()); + activated(&mut driver, &mut orch); + + // A second update is submitted and then discarded before it runs. + driver.submit_update(C0, CANDIDATE_LEN).unwrap(); + driver.discard_staged().unwrap(); + + orch.dispatch(&mut driver, Event::BootConfirmed(C0)); + + assert_eq!( + driver.pending_update(), + None, + "no re-sync from a region that changed hands" + ); +}