From 38aeba4b0a3ce18e784a7a553060057049f0bdd1 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 28 Apr 2025 06:42:07 -0700 Subject: [PATCH 01/18] WIP --- Cargo.lock | 1 - Cargo.toml | 2 ++ sp-sim/src/update.rs | 58 ++++++++++++++++++++++++++++++++++++++------ 3 files changed, 53 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1400353ae56..10bed7e2511 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4273,7 +4273,6 @@ dependencies = [ [[package]] name = "hubtools" version = "0.4.6" -source = "git+https://github.com/oxidecomputer/hubtools.git?branch=main#6054a3c2ad89473e551a9a1d06cbd7845e100630" dependencies = [ "digest", "hex", diff --git a/Cargo.toml b/Cargo.toml index 5e4db73fbda..9d1e4239edb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -914,6 +914,8 @@ opt-level = 3 # progenitor-client = { path = "../progenitor/progenitor-client" } # steno = { path = "../steno" } +[patch."https://github.com/oxidecomputer/hubtools.git"] +hubtools = { path = "../hubtools/hubtools" } # [patch."https://github.com/oxidecomputer/crucible"] # crucible-agent-client = { path = "../crucible/agent-client" } # crucible-pantry-client = { path = "../crucible/pantry-client" } diff --git a/sp-sim/src/update.rs b/sp-sim/src/update.rs index 73b9f4657f8..dea691cfc58 100644 --- a/sp-sim/src/update.rs +++ b/sp-sim/src/update.rs @@ -17,6 +17,7 @@ use gateway_messages::SpError; use gateway_messages::UpdateChunk; use gateway_messages::UpdateId; use gateway_messages::UpdateInProgressStatus; +use hubtools::RawHubrisImage; pub(crate) struct SimSpUpdate { state: UpdateState, @@ -186,6 +187,7 @@ impl SimSpUpdate { UpdateState::NotPrepared | UpdateState::Aborted(_) | UpdateState::Completed { .. } => { + // XXX-dap shouldn't we fail in some of these cases? let slot = if component == SpComponent::HOST_CPU_BOOT_FLASH { match self.active_host_slot { Some(slot) => slot, @@ -202,11 +204,32 @@ impl SimSpUpdate { slot, data: Cursor::new(vec![0u8; total_size].into_boxed_slice()), }; + + if let Some(caboose) = self.caboose_mut(component, slot) { + *caboose = CabooseValue::InvalidMissing; + } + Ok(()) } } } + fn caboose_mut( + &mut self, + component: SpComponent, + slot: u16, + ) -> Option<&mut CabooseValue> { + match (component, slot) { + // SP always updates slot 0. + (SpComponent::SP_ITSELF, 0) => Some(&mut self.caboose_sp_inactive), + (SpComponent::ROT, 0) => Some(&mut self.caboose_rot_a), + (SpComponent::ROT, 1) => Some(&mut self.caboose_rot_b), + // RoT bootloader always updates slot 1. + (SpComponent::STAGE0, 1) => Some(&mut self.caboose_stage0next), + _ => None, + } + } + pub(crate) fn status(&self) -> gateway_messages::UpdateStatus { self.state.to_message() } @@ -219,8 +242,9 @@ impl SimSpUpdate { match &mut self.state { UpdateState::Prepared { component, id, slot, data } => { // Ensure that the update ID and target component are correct. - if chunk.id != *id || chunk.component != *component { - return Err(SpError::InvalidUpdateId { sp_update_id: *id }); + let id = *id; + if chunk.id != id || chunk.component != *component { + return Err(SpError::InvalidUpdateId { sp_update_id: id }); }; if data.position() != u64::from(chunk.offset) { return Err(SpError::UpdateInProgress( @@ -245,11 +269,29 @@ impl SimSpUpdate { .insert(*slot, data.clone()); } - self.state = UpdateState::Completed { - component: *component, - id: *id, - data, - }; + let component = *component; + let slot = *slot; + if let Some(caboose_value) = + self.caboose_mut(component, slot) + { + let caboose = RawHubrisImage::from_binary( + data.clone().to_vec(), + 0, + 0, + ) + .and_then(|image| image.read_caboose()); + match caboose { + Ok(caboose) => { + *caboose_value = CabooseValue::Caboose(caboose); + } + Err(_) => { + // XXX-dap log error + *caboose_value = CabooseValue::InvalidMissing; + } + }; + } + + self.state = UpdateState::Completed { component, id, data }; } Ok(()) @@ -279,6 +321,7 @@ impl SimSpUpdate { } pub(crate) fn sp_reset(&mut self) { + // XXX-dap copy state fields match &self.state { UpdateState::Completed { data, component, .. } => { if *component == SpComponent::SP_ITSELF { @@ -292,6 +335,7 @@ impl SimSpUpdate { } pub(crate) fn rot_reset(&mut self) { + // XXX-dap copy state fields match &self.state { UpdateState::Completed { data, component, .. } => { if *component == SpComponent::ROT { From 627ff293f15109d20f3c3c3db700739ddd4fdb1a Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 28 Apr 2025 13:30:04 -0700 Subject: [PATCH 02/18] rot active slot needs to move into update module too --- sp-sim/src/gimlet.rs | 37 ++----------------------------- sp-sim/src/sidecar.rs | 22 ++----------------- sp-sim/src/update.rs | 51 +++++++++++++++++++++++++++++++++++++++---- 3 files changed, 51 insertions(+), 59 deletions(-) diff --git a/sp-sim/src/gimlet.rs b/sp-sim/src/gimlet.rs index 006851d637e..03e5560dcbe 100644 --- a/sp-sim/src/gimlet.rs +++ b/sp-sim/src/gimlet.rs @@ -6,8 +6,6 @@ use crate::Responsiveness; use crate::SimulatedSp; use crate::config::GimletConfig; use crate::config::SpComponentConfig; -use crate::helpers::rot_slot_id_from_u16; -use crate::helpers::rot_slot_id_to_u16; use crate::helpers::rot_state_v2; use crate::sensors::Sensors; use crate::serial_number_padded; @@ -33,7 +31,6 @@ use gateway_messages::MgsResponse; use gateway_messages::RotBootInfo; use gateway_messages::RotRequest; use gateway_messages::RotResponse; -use gateway_messages::RotSlotId; use gateway_messages::SpComponent; use gateway_messages::SpError; use gateway_messages::SpPort; @@ -629,7 +626,6 @@ struct Handler { attached_mgs: AttachedMgsSerialConsole, incoming_serial_console: HashMap>>, - rot_active_slot: RotSlotId, power_state: PowerState, startup_options: StartupOptions, update_state: SimSpUpdate, @@ -683,7 +679,6 @@ impl Handler { serial_number, attached_mgs, incoming_serial_console, - rot_active_slot: RotSlotId::A, power_state: PowerState::A2, startup_options: StartupOptions::empty(), update_state: SimSpUpdate::new( @@ -1193,14 +1188,7 @@ impl SpHandler for Handler { &self.log, "asked for component active slot"; "component" => ?component, ); - match component { - SpComponent::ROT => Ok(rot_slot_id_to_u16(self.rot_active_slot)), - // The only active component is stage0 - SpComponent::STAGE0 => Ok(0), - // The real SP returns `RequestUnsupportedForComponent` for anything - // other than the RoT, including SP_ITSELF. - _ => Err(SpError::RequestUnsupportedForComponent), - } + self.update_state.component_get_active_slot(component) } fn component_set_active_slot( @@ -1215,28 +1203,7 @@ impl SpHandler for Handler { "slot" => slot, "persist" => persist, ); - match component { - SpComponent::ROT => { - self.rot_active_slot = rot_slot_id_from_u16(slot)?; - Ok(()) - } - SpComponent::STAGE0 => { - if slot == 1 { - return Ok(()); - } else { - Err(SpError::RequestUnsupportedForComponent) - } - } - SpComponent::HOST_CPU_BOOT_FLASH => { - self.update_state.set_active_host_slot(slot); - Ok(()) - } - _ => { - // The real SP returns `RequestUnsupportedForComponent` for anything - // other than the RoT and host boot flash, including SP_ITSELF. - Err(SpError::RequestUnsupportedForComponent) - } - } + self.update_state.component_set_active_slot(component, slot, persist) } fn component_action( diff --git a/sp-sim/src/sidecar.rs b/sp-sim/src/sidecar.rs index 3047108a9e0..35269ebd6b1 100644 --- a/sp-sim/src/sidecar.rs +++ b/sp-sim/src/sidecar.rs @@ -8,8 +8,6 @@ use crate::config::Config; use crate::config::SidecarConfig; use crate::config::SimulatedSpsConfig; use crate::config::SpComponentConfig; -use crate::helpers::rot_slot_id_from_u16; -use crate::helpers::rot_slot_id_to_u16; use crate::helpers::rot_state_v2; use crate::sensors::Sensors; use crate::serial_number_padded; @@ -40,7 +38,6 @@ use gateway_messages::PowerState; use gateway_messages::RotBootInfo; use gateway_messages::RotRequest; use gateway_messages::RotResponse; -use gateway_messages::RotSlotId; use gateway_messages::SpComponent; use gateway_messages::SpError; use gateway_messages::SpPort; @@ -383,7 +380,6 @@ struct Handler { serial_number: String, ignition: FakeIgnition, - rot_active_slot: RotSlotId, power_state: PowerState, update_state: SimSpUpdate, @@ -432,7 +428,6 @@ impl Handler { leaked_component_description_strings, serial_number, ignition, - rot_active_slot: RotSlotId::A, power_state: PowerState::A2, update_state: SimSpUpdate::new( BaseboardKind::Sidecar, @@ -911,13 +906,7 @@ impl SpHandler for Handler { &self.log, "asked for component active slot"; "component" => ?component, ); - if component == SpComponent::ROT { - Ok(rot_slot_id_to_u16(self.rot_active_slot)) - } else { - // The real SP returns `RequestUnsupportedForComponent` for anything - // other than the RoT, including SP_ITSELF. - Err(SpError::RequestUnsupportedForComponent) - } + self.update_state.component_get_active_slot(component) } fn component_set_active_slot( @@ -932,14 +921,7 @@ impl SpHandler for Handler { "slot" => slot, "persist" => persist, ); - if component == SpComponent::ROT { - self.rot_active_slot = rot_slot_id_from_u16(slot)?; - Ok(()) - } else { - // The real SP returns `RequestUnsupportedForComponent` for anything - // other than the RoT, including SP_ITSELF. - Err(SpError::RequestUnsupportedForComponent) - } + self.update_state.component_set_active_slot(component, slot, persist) } fn component_action( diff --git a/sp-sim/src/update.rs b/sp-sim/src/update.rs index dea691cfc58..62a4bcdd545 100644 --- a/sp-sim/src/update.rs +++ b/sp-sim/src/update.rs @@ -363,10 +363,6 @@ impl SimSpUpdate { self.last_host_phase1_update_data.get(&slot).cloned() } - pub(crate) fn set_active_host_slot(&mut self, slot: u16) { - self.active_host_slot = Some(slot); - } - pub(crate) fn get_component_caboose_value( &mut self, component: SpComponent, @@ -392,6 +388,53 @@ impl SimSpUpdate { pub(crate) fn rot_state(&self) -> RotStateV3 { self.rot_state } + + pub(crate) fn component_set_active_slot( + &mut self, + component: SpComponent, + slot: u16, + persist: bool, + ) -> Result<(), SpError> { + match component { + SpComponent::ROT => { + // XXX-dap + Ok(()) + } + SpComponent::STAGE0 => { + if slot == 1 { + // XXX-dap + return Ok(()); + } else { + Err(SpError::RequestUnsupportedForComponent) + } + } + SpComponent::HOST_CPU_BOOT_FLASH => { + // XXX-dap + self.active_host_slot = Some(slot); + Ok(()) + } + _ => { + // The real SP returns `RequestUnsupportedForComponent` for + // anything other than the RoT and host boot flash, including + // SP_ITSELF. + Err(SpError::RequestUnsupportedForComponent) + } + } + } + + pub(crate) fn component_get_active_slot( + &mut self, + component: SpComponent, + ) -> Result { + match component { + SpComponent::ROT => todo!(), // XXX-dap + // The only active component is stage0 + SpComponent::STAGE0 => Ok(0), + // The real SP returns `RequestUnsupportedForComponent` for anything + // other than the RoT, including SP_ITSELF. + _ => Err(SpError::RequestUnsupportedForComponent), + } + } } /// Specifies what kind of device we're constructing caboose metadata for From 9a1e6020495dc78f49fb920aa123a7f7f6e39039 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 28 Apr 2025 15:41:58 -0700 Subject: [PATCH 03/18] impl more update, reset behavior --- sp-sim/src/update.rs | 40 ++++++++++++++++++++++++++++++++++------ 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/sp-sim/src/update.rs b/sp-sim/src/update.rs index 62a4bcdd545..7fe7056a71c 100644 --- a/sp-sim/src/update.rs +++ b/sp-sim/src/update.rs @@ -10,6 +10,7 @@ use crate::SIM_GIMLET_BOARD; use crate::SIM_ROT_BOARD; use crate::SIM_ROT_STAGE0_BOARD; use crate::SIM_SIDECAR_BOARD; +use crate::helpers::rot_slot_id_to_u16; use gateway_messages::RotSlotId; use gateway_messages::RotStateV3; use gateway_messages::SpComponent; @@ -25,6 +26,7 @@ pub(crate) struct SimSpUpdate { last_rot_update_data: Option>, last_host_phase1_update_data: BTreeMap>, active_host_slot: Option, + pending_stage0_update: bool, caboose_sp_active: CabooseValue, caboose_sp_inactive: CabooseValue, @@ -161,6 +163,8 @@ impl SimSpUpdate { // set that slot as active first. active_host_slot: None, + pending_stage0_update: false, + caboose_sp_active, caboose_sp_inactive, caboose_rot_a, @@ -321,11 +325,18 @@ impl SimSpUpdate { } pub(crate) fn sp_reset(&mut self) { - // XXX-dap copy state fields match &self.state { UpdateState::Completed { data, component, .. } => { if *component == SpComponent::SP_ITSELF { self.last_sp_update_data = Some(data.clone()); + + // Swap the cabooses to simulate what a real SP does in + // terms of swapping the active slot upon reset after an + // update. + std::mem::swap( + &mut self.caboose_sp_active, + &mut self.caboose_sp_inactive, + ); } } UpdateState::NotPrepared @@ -335,7 +346,6 @@ impl SimSpUpdate { } pub(crate) fn rot_reset(&mut self) { - // XXX-dap copy state fields match &self.state { UpdateState::Completed { data, component, .. } => { if *component == SpComponent::ROT { @@ -346,6 +356,17 @@ impl SimSpUpdate { | UpdateState::Prepared { .. } | UpdateState::Aborted(_) => (), } + + if let Some(new_boot_pref) = + self.rot_state.pending_persistent_boot_preference.take() + { + self.rot_state.persistent_boot_preference = new_boot_pref; + self.rot_state.active = new_boot_pref; + } + + // We do not currently simulate changes to the transient boot + // preference. + self.rot_state.transient_boot_preference = None; } pub(crate) fn last_sp_update_data(&self) -> Option> { @@ -397,19 +418,24 @@ impl SimSpUpdate { ) -> Result<(), SpError> { match component { SpComponent::ROT => { - // XXX-dap + if persist { + self.rot_state.pending_persistent_boot_preference = + Some(rot_slot_id_from_u16(slot)); + } else { + self.rot_state.transient_boot_preference = + Some(rot_slot_id_from_u16(slot)); + } Ok(()) } SpComponent::STAGE0 => { if slot == 1 { - // XXX-dap + self.pending_stage0_update = true; return Ok(()); } else { Err(SpError::RequestUnsupportedForComponent) } } SpComponent::HOST_CPU_BOOT_FLASH => { - // XXX-dap self.active_host_slot = Some(slot); Ok(()) } @@ -427,7 +453,9 @@ impl SimSpUpdate { component: SpComponent, ) -> Result { match component { - SpComponent::ROT => todo!(), // XXX-dap + SpComponent::ROT => { + rot_slot_id_to_u16(self.rot_state.persistent_boot_preference) + } // The only active component is stage0 SpComponent::STAGE0 => Ok(0), // The real SP returns `RequestUnsupportedForComponent` for anything From e6bab4739e1a3ecb59709610640df98015c29fb8 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 28 Apr 2025 15:58:55 -0700 Subject: [PATCH 04/18] fixes --- Cargo.lock | 1 + Cargo.toml | 2 +- sp-sim/src/update.rs | 14 +++++++------- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 10bed7e2511..22ff7cf93fa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4273,6 +4273,7 @@ dependencies = [ [[package]] name = "hubtools" version = "0.4.6" +source = "git+ssh://git@github.com/oxidecomputer/hubtools?branch=dap%2Fread-caboose-from-image#79c8674da595200576ef9a5309aaf72afc8c4a9c" dependencies = [ "digest", "hex", diff --git a/Cargo.toml b/Cargo.toml index 9d1e4239edb..3e5f16f73f1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -915,7 +915,7 @@ opt-level = 3 # steno = { path = "../steno" } [patch."https://github.com/oxidecomputer/hubtools.git"] -hubtools = { path = "../hubtools/hubtools" } +hubtools = { git = "ssh://git@github.com/oxidecomputer/hubtools", branch = "dap/read-caboose-from-image" } # [patch."https://github.com/oxidecomputer/crucible"] # crucible-agent-client = { path = "../crucible/agent-client" } # crucible-pantry-client = { path = "../crucible/pantry-client" } diff --git a/sp-sim/src/update.rs b/sp-sim/src/update.rs index 7fe7056a71c..ad42863a6ec 100644 --- a/sp-sim/src/update.rs +++ b/sp-sim/src/update.rs @@ -10,6 +10,7 @@ use crate::SIM_GIMLET_BOARD; use crate::SIM_ROT_BOARD; use crate::SIM_ROT_STAGE0_BOARD; use crate::SIM_SIDECAR_BOARD; +use crate::helpers::rot_slot_id_from_u16; use crate::helpers::rot_slot_id_to_u16; use gateway_messages::RotSlotId; use gateway_messages::RotStateV3; @@ -420,10 +421,10 @@ impl SimSpUpdate { SpComponent::ROT => { if persist { self.rot_state.pending_persistent_boot_preference = - Some(rot_slot_id_from_u16(slot)); + Some(rot_slot_id_from_u16(slot)?); } else { self.rot_state.transient_boot_preference = - Some(rot_slot_id_from_u16(slot)); + Some(rot_slot_id_from_u16(slot)?); } Ok(()) } @@ -453,9 +454,9 @@ impl SimSpUpdate { component: SpComponent, ) -> Result { match component { - SpComponent::ROT => { - rot_slot_id_to_u16(self.rot_state.persistent_boot_preference) - } + SpComponent::ROT => Ok(rot_slot_id_to_u16( + self.rot_state.persistent_boot_preference, + )), // The only active component is stage0 SpComponent::STAGE0 => Ok(0), // The real SP returns `RequestUnsupportedForComponent` for anything @@ -499,13 +500,12 @@ enum CabooseValue { /// emulate an actual caboose Caboose(hubtools::Caboose), /// emulate "the image does not include a caboose" - // This will be used shortly. #[allow(dead_code)] InvalidMissing, /// emulate "the image caboose does not contain 'KEY'" InvalidMissingAllKeys, /// emulate "failed to read data from the caboose" (erased) - // This will be used shortly. + // XXX-dap #[allow(dead_code)] InvalidFailedRead, } From e4db05c902c2bc56cdd361556761f467e97a24ae Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 28 Apr 2025 16:02:04 -0700 Subject: [PATCH 05/18] change the way I patch --- Cargo.lock | 37 +++++++++++++++++++++++++++++-------- Cargo.toml | 5 ++--- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 22ff7cf93fa..1041d6524f5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3591,7 +3591,7 @@ dependencies = [ "gateway-messages", "hex", "hubpack", - "hubtools", + "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=main)", "lru-cache", "lzss", "nix 0.27.1", @@ -4273,7 +4273,28 @@ dependencies = [ [[package]] name = "hubtools" version = "0.4.6" -source = "git+ssh://git@github.com/oxidecomputer/hubtools?branch=dap%2Fread-caboose-from-image#79c8674da595200576ef9a5309aaf72afc8c4a9c" +source = "git+https://github.com/oxidecomputer/hubtools.git?branch=dap%2Fread-caboose-from-image#79c8674da595200576ef9a5309aaf72afc8c4a9c" +dependencies = [ + "digest", + "hex", + "lpc55_areas", + "lpc55_sign", + "object 0.30.4", + "path-slash", + "rsa", + "thiserror 1.0.69", + "tlvc 0.3.1 (git+https://github.com/oxidecomputer/tlvc)", + "tlvc-text", + "toml 0.7.8", + "x509-cert", + "zerocopy 0.6.6", + "zip 0.6.6", +] + +[[package]] +name = "hubtools" +version = "0.4.6" +source = "git+https://github.com/oxidecomputer/hubtools.git?branch=main#6054a3c2ad89473e551a9a1d06cbd7845e100630" dependencies = [ "digest", "hex", @@ -6274,7 +6295,7 @@ dependencies = [ "gateway-client", "gateway-messages", "gateway-test-utils", - "hubtools", + "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=dap%2Fread-caboose-from-image)", "id-map", "internal-dns-resolver", "internal-dns-types", @@ -7328,7 +7349,7 @@ dependencies = [ "http-body-util", "httpmock", "httptest", - "hubtools", + "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=dap%2Fread-caboose-from-image)", "hyper", "hyper-rustls 0.26.0", "hyper-staticfile", @@ -11845,7 +11866,7 @@ dependencies = [ "gateway-messages", "gateway-types", "hex", - "hubtools", + "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=dap%2Fread-caboose-from-image)", "nix 0.29.0", "omicron-common", "omicron-workspace-hack", @@ -13184,7 +13205,7 @@ dependencies = [ "fs-err 2.11.0", "futures", "hex", - "hubtools", + "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=main)", "indent_write", "itertools 0.13.0", "parse-size", @@ -13520,7 +13541,7 @@ dependencies = [ "fs-err 3.1.0", "futures", "hex", - "hubtools", + "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=dap%2Fread-caboose-from-image)", "omicron-common", "omicron-test-utils", "omicron-workspace-hack", @@ -14100,7 +14121,7 @@ dependencies = [ "hickory-resolver", "http", "http-body-util", - "hubtools", + "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=dap%2Fread-caboose-from-image)", "hyper", "illumos-utils", "installinator", diff --git a/Cargo.toml b/Cargo.toml index 3e5f16f73f1..6948a026c79 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -460,7 +460,8 @@ http-body-util = "0.1.3" http-range = "0.1.5" httpmock = "0.8.0-alpha.1" httptest = "0.16.3" -hubtools = { git = "https://github.com/oxidecomputer/hubtools.git", branch = "main" } +# XXX-dap +hubtools = { git = "https://github.com/oxidecomputer/hubtools.git", branch = "dap/read-caboose-from-image" } humantime = "2.1.0" hyper = "1.6.0" hyper-util = "0.1.11" @@ -914,8 +915,6 @@ opt-level = 3 # progenitor-client = { path = "../progenitor/progenitor-client" } # steno = { path = "../steno" } -[patch."https://github.com/oxidecomputer/hubtools.git"] -hubtools = { git = "ssh://git@github.com/oxidecomputer/hubtools", branch = "dap/read-caboose-from-image" } # [patch."https://github.com/oxidecomputer/crucible"] # crucible-agent-client = { path = "../crucible/agent-client" } # crucible-pantry-client = { path = "../crucible/pantry-client" } From 9ecef33ce03e6ab215680e0a11f943de0254fc7b Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 28 Apr 2025 16:08:05 -0700 Subject: [PATCH 06/18] fake stage0 update too --- sp-sim/src/update.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/sp-sim/src/update.rs b/sp-sim/src/update.rs index ad42863a6ec..ac18aa2e5ef 100644 --- a/sp-sim/src/update.rs +++ b/sp-sim/src/update.rs @@ -358,6 +358,8 @@ impl SimSpUpdate { | UpdateState::Aborted(_) => (), } + // If there was a pending persistent boot preference set, apply that now + // (and unset it). if let Some(new_boot_pref) = self.rot_state.pending_persistent_boot_preference.take() { @@ -368,6 +370,22 @@ impl SimSpUpdate { // We do not currently simulate changes to the transient boot // preference. self.rot_state.transient_boot_preference = None; + + if self.pending_stage0_update { + // If there was a pending stage0 update, apply that now. All this + // means is changing the stage0 FWID and caboose to match the + // stage0next FWID and caboose. We need something to put into the + // stage0next FWID and caboose, so we arbitrairly pick whatever was + // the stage0 one. + std::mem::swap( + &mut self.rot_state.stage0_fwid, + &mut self.rot_state.stage0next_fwid, + ); + std::mem::swap( + &mut self.caboose_stage0, + &mut self.caboose_stage0next, + ); + } } pub(crate) fn last_sp_update_data(&self) -> Option> { From 5817eacbd00adfb108d26fea618ac79f31468bd2 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 28 Apr 2025 16:13:53 -0700 Subject: [PATCH 07/18] use stage0 error --- sp-sim/src/update.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/sp-sim/src/update.rs b/sp-sim/src/update.rs index ac18aa2e5ef..b53267b4f31 100644 --- a/sp-sim/src/update.rs +++ b/sp-sim/src/update.rs @@ -210,8 +210,20 @@ impl SimSpUpdate { data: Cursor::new(vec![0u8; total_size].into_boxed_slice()), }; + // When the SP or RoT begins preparing an update, we can start + // seeing various errors when we try to read the caboose for the + // slot we're updating. Empirically, during the SP update we + // tend to see InvalidMissing. During the stage0next update, we + // see InvalidFailedRead. It'd be nice to get these exactly + // right, but it's most important that consumers handle both + // cases, which they will if they test updates to both of these + // components. if let Some(caboose) = self.caboose_mut(component, slot) { - *caboose = CabooseValue::InvalidMissing; + *caboose = if component == SpComponent::STAGE0 { + CabooseValue::InvalidFailedRead + } else { + CabooseValue::InvalidMissing + } } Ok(()) @@ -518,13 +530,10 @@ enum CabooseValue { /// emulate an actual caboose Caboose(hubtools::Caboose), /// emulate "the image does not include a caboose" - #[allow(dead_code)] InvalidMissing, /// emulate "the image caboose does not contain 'KEY'" InvalidMissingAllKeys, /// emulate "failed to read data from the caboose" (erased) - // XXX-dap - #[allow(dead_code)] InvalidFailedRead, } From cd8612970ab6ad66f453e2fa5ce98dcbcbf327bb Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 28 Apr 2025 16:15:48 -0700 Subject: [PATCH 08/18] fix update state after reset --- sp-sim/src/update.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/sp-sim/src/update.rs b/sp-sim/src/update.rs index b53267b4f31..70f2bcd81ee 100644 --- a/sp-sim/src/update.rs +++ b/sp-sim/src/update.rs @@ -356,6 +356,8 @@ impl SimSpUpdate { | UpdateState::Prepared { .. } | UpdateState::Aborted(_) => (), } + + self.state = UpdateState::NotPrepared; } pub(crate) fn rot_reset(&mut self) { @@ -370,6 +372,8 @@ impl SimSpUpdate { | UpdateState::Aborted(_) => (), } + self.state = UpdateState::NotPrepared; + // If there was a pending persistent boot preference set, apply that now // (and unset it). if let Some(new_boot_pref) = From 8a5069186ca592a58ccc40942c53caed1e7a9432 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 28 Apr 2025 16:40:16 -0700 Subject: [PATCH 09/18] more fixes (WIP) --- sp-sim/src/update.rs | 79 +++++++++++++++++++++++++++++++++----------- 1 file changed, 60 insertions(+), 19 deletions(-) diff --git a/sp-sim/src/update.rs b/sp-sim/src/update.rs index 70f2bcd81ee..eb4e13dee18 100644 --- a/sp-sim/src/update.rs +++ b/sp-sim/src/update.rs @@ -12,6 +12,7 @@ use crate::SIM_ROT_STAGE0_BOARD; use crate::SIM_SIDECAR_BOARD; use crate::helpers::rot_slot_id_from_u16; use crate::helpers::rot_slot_id_to_u16; +use gateway_messages::Fwid; use gateway_messages::RotSlotId; use gateway_messages::RotStateV3; use gateway_messages::SpComponent; @@ -22,19 +23,38 @@ use gateway_messages::UpdateInProgressStatus; use hubtools::RawHubrisImage; pub(crate) struct SimSpUpdate { + /// tracks the state of any ongoing simulated update + /// + /// Both the hardware and this simulator enforce that only one update to any + /// SP-managed component can be in flight at a time. state: UpdateState, + + /// data from the last completed SP update (exposed for testing) last_sp_update_data: Option>, + /// data from the last completed RoT update (exposed for testing) last_rot_update_data: Option>, + /// data from the last completed phase1 update for each slot (exposed for + /// testing) last_host_phase1_update_data: BTreeMap>, active_host_slot: Option, + + /// records whether a change to the stage0 "active slot" has been requested pending_stage0_update: bool, + // XXX-dap review the lifecycle of all of these fields + /// current caboose for the SP active slot caboose_sp_active: CabooseValue, + /// current caboose for the SP inactive slot caboose_sp_inactive: CabooseValue, + /// current caboose for the RoT slot A caboose_rot_a: CabooseValue, + /// current caboose for the RoT slot B caboose_rot_b: CabooseValue, + /// current caboose for stage0 caboose_stage0: CabooseValue, + /// current caboose for stage0next caboose_stage0next: CabooseValue, + /// current RoT boot and version information rot_state: RotStateV3, } @@ -338,14 +358,16 @@ impl SimSpUpdate { } pub(crate) fn sp_reset(&mut self) { + // Check if an SP update completed since the last reset. match &self.state { UpdateState::Completed { data, component, .. } => { if *component == SpComponent::SP_ITSELF { + // Save the data that we received so that we can expose it + // for tests. self.last_sp_update_data = Some(data.clone()); - // Swap the cabooses to simulate what a real SP does in - // terms of swapping the active slot upon reset after an - // update. + // Swap the cabooses to simulate the real SP behavior of + // swapping which slot is active after an update. std::mem::swap( &mut self.caboose_sp_active, &mut self.caboose_sp_inactive, @@ -357,21 +379,32 @@ impl SimSpUpdate { | UpdateState::Aborted(_) => (), } + // Clear any in-progress update. self.state = UpdateState::NotPrepared; } pub(crate) fn rot_reset(&mut self) { - match &self.state { + // Check if an RoT update completed since the last reset. + let stage0next_data = match &self.state { UpdateState::Completed { data, component, .. } => { if *component == SpComponent::ROT { + // Save the data that we received so that we can expose it + // for tests. self.last_rot_update_data = Some(data.clone()); } + + if *component == SpComponent::STAGE0 { + Some(data.clone()) + } else { + None + } } UpdateState::NotPrepared | UpdateState::Prepared { .. } - | UpdateState::Aborted(_) => (), - } + | UpdateState::Aborted(_) => None, + }; + // Clear any in-progress update. self.state = UpdateState::NotPrepared; // If there was a pending persistent boot preference set, apply that now @@ -388,19 +421,26 @@ impl SimSpUpdate { self.rot_state.transient_boot_preference = None; if self.pending_stage0_update { - // If there was a pending stage0 update, apply that now. All this - // means is changing the stage0 FWID and caboose to match the - // stage0next FWID and caboose. We need something to put into the - // stage0next FWID and caboose, so we arbitrairly pick whatever was - // the stage0 one. - std::mem::swap( - &mut self.rot_state.stage0_fwid, - &mut self.rot_state.stage0next_fwid, - ); - std::mem::swap( - &mut self.caboose_stage0, - &mut self.caboose_stage0next, - ); + // If there was a pending stage0 update (i.e., a change to the + // stage0 "active slot"), apply that now. All this means is + // changing the stage0 FWID and caboose to match the stage0next FWID + // and caboose. + self.rot_state.stage0_fwid = self.rot_state.stage0next_fwid; + self.caboose_stage0 = self.caboose_stage0next.clone(); + } + + // If stage0next was updated, compute a new digest for it and + // store that. (We don't need to update the caboose here + // because we did that when we received the last packet.) + if let Some(_data) = stage0next_data { + // Note: this is NOT how the FWID is really computed by the + // RoT. We cannot easily replicate that here. But the + // consumer can't, either, so it doesn't really matter. + // What matters is that it changes when an update happens + // (and it's nice that it's the same for a given Hubris + // image). + // XXX-dap + self.rot_state.stage0next_fwid = Fwid::Sha3_256([0xee; 32]); } } @@ -530,6 +570,7 @@ impl BaseboardKind { } /// Represents a simulated caboose +#[derive(Clone)] enum CabooseValue { /// emulate an actual caboose Caboose(hubtools::Caboose), From e082429b5931150ed5f2c7c820be8ca493596873 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Mon, 28 Apr 2025 20:42:52 -0700 Subject: [PATCH 10/18] more simulation --- sp-sim/src/update.rs | 118 +++++++++++++++++++++---------------------- 1 file changed, 59 insertions(+), 59 deletions(-) diff --git a/sp-sim/src/update.rs b/sp-sim/src/update.rs index eb4e13dee18..6af53e10406 100644 --- a/sp-sim/src/update.rs +++ b/sp-sim/src/update.rs @@ -238,6 +238,9 @@ impl SimSpUpdate { // right, but it's most important that consumers handle both // cases, which they will if they test updates to both of these // components. + // XXX-dap should we instead calculate the caboose on-demand + // from the data we've received? (Will that be annoying for the + // initial condition?) if let Some(caboose) = self.caboose_mut(component, slot) { *caboose = if component == SpComponent::STAGE0 { CabooseValue::InvalidFailedRead @@ -328,7 +331,8 @@ impl SimSpUpdate { }; } - self.state = UpdateState::Completed { component, id, data }; + self.state = + UpdateState::Completed { component, id, slot, data }; } Ok(()) @@ -358,54 +362,55 @@ impl SimSpUpdate { } pub(crate) fn sp_reset(&mut self) { + // Clear any update, whatever state it was in. + let prev_state = + std::mem::replace(&mut self.state, UpdateState::NotPrepared); + // Check if an SP update completed since the last reset. - match &self.state { - UpdateState::Completed { data, component, .. } => { - if *component == SpComponent::SP_ITSELF { - // Save the data that we received so that we can expose it - // for tests. - self.last_sp_update_data = Some(data.clone()); - - // Swap the cabooses to simulate the real SP behavior of - // swapping which slot is active after an update. - std::mem::swap( - &mut self.caboose_sp_active, - &mut self.caboose_sp_inactive, - ); - } + if let UpdateState::Completed { data, component, .. } = prev_state { + if component == SpComponent::SP_ITSELF { + // Save the data that we received so that we can expose it + // for tests. + self.last_sp_update_data = Some(data.clone()); + + // Swap the cabooses to simulate the real SP behavior of + // swapping which slot is active after an update. + std::mem::swap( + &mut self.caboose_sp_active, + &mut self.caboose_sp_inactive, + ); } - UpdateState::NotPrepared - | UpdateState::Prepared { .. } - | UpdateState::Aborted(_) => (), } - - // Clear any in-progress update. - self.state = UpdateState::NotPrepared; } pub(crate) fn rot_reset(&mut self) { - // Check if an RoT update completed since the last reset. - let stage0next_data = match &self.state { - UpdateState::Completed { data, component, .. } => { - if *component == SpComponent::ROT { - // Save the data that we received so that we can expose it - // for tests. - self.last_rot_update_data = Some(data.clone()); - } + // Clear any update, whatever state it was in. + let prev_state = + std::mem::replace(&mut self.state, UpdateState::NotPrepared); - if *component == SpComponent::STAGE0 { - Some(data.clone()) - } else { - None - } + // Apply changes from a successfully-completed update. + if let UpdateState::Completed { data, component, slot, .. } = prev_state + { + if component == SpComponent::ROT { + // Compute a new FWID for the affected slot. + // unwrap(): we've already validated this. + let slot = rot_slot_id_from_u16(slot).unwrap(); + let fwidp = match slot { + RotSlotId::A => &mut self.rot_state.slot_a_fwid, + RotSlotId::B => &mut self.rot_state.slot_b_fwid, + }; + *fwidp = fake_fwid_compute(&data); + + // For an RoT update, save the data that we received so that + // we can expose it for tests. + self.last_rot_update_data = Some(data); + } else if component == SpComponent::STAGE0 { + // Similarly, for a bootloader update, compute a new FWID + // for stage0next. + let fwid = fake_fwid_compute(&data); + self.rot_state.stage0next_fwid = fwid; } - UpdateState::NotPrepared - | UpdateState::Prepared { .. } - | UpdateState::Aborted(_) => None, - }; - - // Clear any in-progress update. - self.state = UpdateState::NotPrepared; + } // If there was a pending persistent boot preference set, apply that now // (and unset it). @@ -420,28 +425,13 @@ impl SimSpUpdate { // preference. self.rot_state.transient_boot_preference = None; + // If there was a pending stage0 update (i.e., a change to the stage0 + // "active slot"), apply that now. All this means is changing the + // stage0 FWID and caboose to match the stage0next FWID and caboose. if self.pending_stage0_update { - // If there was a pending stage0 update (i.e., a change to the - // stage0 "active slot"), apply that now. All this means is - // changing the stage0 FWID and caboose to match the stage0next FWID - // and caboose. self.rot_state.stage0_fwid = self.rot_state.stage0next_fwid; self.caboose_stage0 = self.caboose_stage0next.clone(); } - - // If stage0next was updated, compute a new digest for it and - // store that. (We don't need to update the caboose here - // because we did that when we received the last packet.) - if let Some(_data) = stage0next_data { - // Note: this is NOT how the FWID is really computed by the - // RoT. We cannot easily replicate that here. But the - // consumer can't, either, so it doesn't really matter. - // What matters is that it changes when an update happens - // (and it's nice that it's the same for a given Hubris - // image). - // XXX-dap - self.rot_state.stage0next_fwid = Fwid::Sha3_256([0xee; 32]); - } } pub(crate) fn last_sp_update_data(&self) -> Option> { @@ -633,6 +623,7 @@ enum UpdateState { Completed { component: SpComponent, id: UpdateId, + slot: u16, data: Box<[u8]>, }, } @@ -662,3 +653,12 @@ impl UpdateState { } } } + +/// Computes a FWID for the Hubris image contained in `data` +// This is NOT the way real FWIDs are computed. For our purposes, all we really +// care about is that the value changes when the image changes, and maybe that +// the value doesn't change when the image doesn't change. +fn fake_fwid_compute(_data: &[u8]) -> Fwid { + // XXX-dap + Fwid::Sha3_256([0xee; 32]) +} From 003eb67b0f4db431299f7944c1c3f78463052911 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Tue, 29 Apr 2025 09:20:33 -0700 Subject: [PATCH 11/18] back to real hubtools --- Cargo.lock | 45 ++++++++++++--------------------------------- Cargo.toml | 3 +-- 2 files changed, 13 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1041d6524f5..75961122325 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1615,7 +1615,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "117725a109d387c937a1533ce01b450cbde6b88abceea8473c4d7a85853cda3c" dependencies = [ "lazy_static", - "windows-sys 0.59.0", + "windows-sys 0.48.0", ] [[package]] @@ -3591,7 +3591,7 @@ dependencies = [ "gateway-messages", "hex", "hubpack", - "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=main)", + "hubtools", "lru-cache", "lzss", "nix 0.27.1", @@ -4273,28 +4273,7 @@ dependencies = [ [[package]] name = "hubtools" version = "0.4.6" -source = "git+https://github.com/oxidecomputer/hubtools.git?branch=dap%2Fread-caboose-from-image#79c8674da595200576ef9a5309aaf72afc8c4a9c" -dependencies = [ - "digest", - "hex", - "lpc55_areas", - "lpc55_sign", - "object 0.30.4", - "path-slash", - "rsa", - "thiserror 1.0.69", - "tlvc 0.3.1 (git+https://github.com/oxidecomputer/tlvc)", - "tlvc-text", - "toml 0.7.8", - "x509-cert", - "zerocopy 0.6.6", - "zip 0.6.6", -] - -[[package]] -name = "hubtools" -version = "0.4.6" -source = "git+https://github.com/oxidecomputer/hubtools.git?branch=main#6054a3c2ad89473e551a9a1d06cbd7845e100630" +source = "git+https://github.com/oxidecomputer/hubtools.git?branch=main#cec2560e9a0126e9e687d51b385a57891abc87c3" dependencies = [ "digest", "hex", @@ -5356,7 +5335,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4979f22fdb869068da03c9f7528f8297c6fd2606bc3a4affe42e6a823fdb8da4" dependencies = [ "cfg-if", - "windows-targets 0.52.6", + "windows-targets 0.48.5", ] [[package]] @@ -6295,7 +6274,7 @@ dependencies = [ "gateway-client", "gateway-messages", "gateway-test-utils", - "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=dap%2Fread-caboose-from-image)", + "hubtools", "id-map", "internal-dns-resolver", "internal-dns-types", @@ -7349,7 +7328,7 @@ dependencies = [ "http-body-util", "httpmock", "httptest", - "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=dap%2Fread-caboose-from-image)", + "hubtools", "hyper", "hyper-rustls 0.26.0", "hyper-staticfile", @@ -11838,7 +11817,7 @@ version = "0.8.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "03c3c6b7927ffe7ecaa769ee0e3994da3b8cafc8f444578982c83ecb161af917" dependencies = [ - "heck 0.5.0", + "heck 0.4.1", "proc-macro2", "quote", "syn 2.0.98", @@ -11866,7 +11845,7 @@ dependencies = [ "gateway-messages", "gateway-types", "hex", - "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=dap%2Fread-caboose-from-image)", + "hubtools", "nix 0.29.0", "omicron-common", "omicron-workspace-hack", @@ -13205,7 +13184,7 @@ dependencies = [ "fs-err 2.11.0", "futures", "hex", - "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=main)", + "hubtools", "indent_write", "itertools 0.13.0", "parse-size", @@ -13541,7 +13520,7 @@ dependencies = [ "fs-err 3.1.0", "futures", "hex", - "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=dap%2Fread-caboose-from-image)", + "hubtools", "omicron-common", "omicron-test-utils", "omicron-workspace-hack", @@ -14121,7 +14100,7 @@ dependencies = [ "hickory-resolver", "http", "http-body-util", - "hubtools 0.4.6 (git+https://github.com/oxidecomputer/hubtools.git?branch=dap%2Fread-caboose-from-image)", + "hubtools", "hyper", "illumos-utils", "installinator", @@ -14245,7 +14224,7 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf221c93e13a30d793f7645a0e7762c55d169dbb0a49671918a2319d289b10bb" dependencies = [ - "windows-sys 0.59.0", + "windows-sys 0.48.0", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 6948a026c79..5e4db73fbda 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -460,8 +460,7 @@ http-body-util = "0.1.3" http-range = "0.1.5" httpmock = "0.8.0-alpha.1" httptest = "0.16.3" -# XXX-dap -hubtools = { git = "https://github.com/oxidecomputer/hubtools.git", branch = "dap/read-caboose-from-image" } +hubtools = { git = "https://github.com/oxidecomputer/hubtools.git", branch = "main" } humantime = "2.1.0" hyper = "1.6.0" hyper-util = "0.1.11" From 1c83ef30cb8ba0d7aaaecb4cc691d5614945fcfc Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Tue, 29 Apr 2025 09:48:54 -0700 Subject: [PATCH 12/18] compute fwid --- Cargo.lock | 2 ++ sp-sim/Cargo.toml | 2 ++ sp-sim/src/update.rs | 17 ++++++++++------- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 75961122325..9c94a9dc240 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11840,6 +11840,7 @@ dependencies = [ "anyhow", "async-trait", "clap", + "digest", "dropshot 0.16.0", "futures", "gateway-messages", @@ -11850,6 +11851,7 @@ dependencies = [ "omicron-common", "omicron-workspace-hack", "serde", + "sha3", "slog", "slog-dtrace", "thiserror 1.0.69", diff --git a/sp-sim/Cargo.toml b/sp-sim/Cargo.toml index 5c852c82572..f2e8da43fdd 100644 --- a/sp-sim/Cargo.toml +++ b/sp-sim/Cargo.toml @@ -11,6 +11,7 @@ workspace = true anyhow.workspace = true async-trait.workspace = true clap.workspace = true +digest.workspace = true dropshot.workspace = true futures.workspace = true gateway-messages.workspace = true @@ -19,6 +20,7 @@ hex = { workspace = true, features = [ "serde" ] } hubtools.workspace = true omicron-common.workspace = true serde.workspace = true +sha3.workspace = true slog.workspace = true slog-dtrace.workspace = true thiserror.workspace = true diff --git a/sp-sim/src/update.rs b/sp-sim/src/update.rs index 6af53e10406..4a338a5611c 100644 --- a/sp-sim/src/update.rs +++ b/sp-sim/src/update.rs @@ -21,6 +21,8 @@ use gateway_messages::UpdateChunk; use gateway_messages::UpdateId; use gateway_messages::UpdateInProgressStatus; use hubtools::RawHubrisImage; +use sha3::Digest; +use sha3::Sha3_256; pub(crate) struct SimSpUpdate { /// tracks the state of any ongoing simulated update @@ -654,11 +656,12 @@ impl UpdateState { } } -/// Computes a FWID for the Hubris image contained in `data` -// This is NOT the way real FWIDs are computed. For our purposes, all we really -// care about is that the value changes when the image changes, and maybe that -// the value doesn't change when the image doesn't change. -fn fake_fwid_compute(_data: &[u8]) -> Fwid { - // XXX-dap - Fwid::Sha3_256([0xee; 32]) +/// Computes a fake FWID for the Hubris image contained in `data` +fn fake_fwid_compute(data: &[u8]) -> Fwid { + // This is NOT the way real FWIDs are computed. For our purposes, all we + // really care about is that the value changes when the image changes, and + // maybe that the value doesn't change when the image doesn't change. + let mut digest = Sha3_256::default(); + digest.update(data); + Fwid::Sha3_256(digest.finalize().into()) } From 9be9e84698f666fd64bb2b717411865910df8963 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Tue, 29 Apr 2025 11:14:57 -0700 Subject: [PATCH 13/18] support get-active-slot for SP since real SPs do --- sp-sim/src/update.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sp-sim/src/update.rs b/sp-sim/src/update.rs index 4a338a5611c..c5157518f15 100644 --- a/sp-sim/src/update.rs +++ b/sp-sim/src/update.rs @@ -520,13 +520,13 @@ impl SimSpUpdate { component: SpComponent, ) -> Result { match component { + // The only active component for SP is slot 0. + SpComponent::SP_ITSELF => Ok(0), SpComponent::ROT => Ok(rot_slot_id_to_u16( self.rot_state.persistent_boot_preference, )), // The only active component is stage0 SpComponent::STAGE0 => Ok(0), - // The real SP returns `RequestUnsupportedForComponent` for anything - // other than the RoT, including SP_ITSELF. _ => Err(SpError::RequestUnsupportedForComponent), } } From df2d83cfb676b15b4d47ce151fa6ce65cb3ff85d Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Tue, 29 Apr 2025 11:42:07 -0700 Subject: [PATCH 14/18] update SP update tests to use new sp-sim behavior --- clients/gateway-client/src/lib.rs | 1 + nexus/mgs-updates/tests/sp_updater.rs | 177 +++++++++++++++++++++++--- 2 files changed, 160 insertions(+), 18 deletions(-) diff --git a/clients/gateway-client/src/lib.rs b/clients/gateway-client/src/lib.rs index df4fd63b959..a25c2ea8747 100644 --- a/clients/gateway-client/src/lib.rs +++ b/clients/gateway-client/src/lib.rs @@ -66,6 +66,7 @@ progenitor::generate_api!( RotImageError = { derives = [ PartialEq, Eq, PartialOrd, Ord] }, RotSlot = { derives = [PartialEq, Eq, PartialOrd, Ord] }, RotState = { derives = [PartialEq, Eq, PartialOrd, Ord] }, + SpComponentCaboose = { derives = [PartialEq, Eq] }, SpIdentifier = { derives = [Copy, PartialEq, Hash, Eq] }, SpIgnition = { derives = [PartialEq, Eq, PartialOrd, Ord] }, SpIgnitionSystemType = { derives = [Copy, PartialEq, Eq, PartialOrd, Ord] }, diff --git a/nexus/mgs-updates/tests/sp_updater.rs b/nexus/mgs-updates/tests/sp_updater.rs index ab57f9ac17e..dc6e8a90c2e 100644 --- a/nexus/mgs-updates/tests/sp_updater.rs +++ b/nexus/mgs-updates/tests/sp_updater.rs @@ -4,6 +4,7 @@ //! Tests `SpUpdater`'s delivery of updates to SPs via MGS +use gateway_client::SpComponent; use gateway_client::types::SpType; use gateway_messages::{SpPort, UpdateInProgressStatus, UpdateStatus}; use gateway_test_utils::setup as mgs_setup; @@ -24,14 +25,21 @@ use tokio::net::TcpStream; use tokio::sync::mpsc; use uuid::Uuid; -fn make_fake_sp_image(board: &str) -> Vec { - let caboose = CabooseBuilder::default() +fn make_fake_sp_archive(board: &str) -> Vec { + let caboose = make_fake_sp_archive_caboose(board); + make_fake_sp_archive_with_caboose(&caboose) +} + +fn make_fake_sp_archive_caboose(board: &str) -> hubtools::Caboose { + CabooseBuilder::default() .git_commit("fake-git-commit") .board(board) .version("0.0.0") .name("fake-name") - .build(); + .build() +} +fn make_fake_sp_archive_with_caboose(caboose: &hubtools::Caboose) -> Vec { let mut builder = HubrisArchiveBuilder::with_fake_image(); builder.write_caboose(caboose.as_slice()).unwrap(); builder.build_to_vec().unwrap() @@ -43,19 +51,21 @@ async fn test_sp_updater_updates_sled() { let mgstestctx = mgs_setup::test_setup("test_sp_updater_updates_sled", SpPort::One) .await; + let log = &mgstestctx.logctx.log; // Configure an MGS client. - let mut mgs_clients = - MgsClients::from_clients([gateway_client::Client::new( - &mgstestctx.client.url("/").to_string(), - mgstestctx.logctx.log.new(slog::o!("component" => "MgsClient")), - )]); + let mgs_client = gateway_client::Client::new( + &mgstestctx.client.url("/").to_string(), + log.new(slog::o!("component" => "MgsClient")), + ); + let mut mgs_clients = MgsClients::from_clients([mgs_client.clone()]); // Configure and instantiate an `SpUpdater`. let sp_type = SpType::Sled; let sp_slot = 0; let update_id = Uuid::new_v4(); - let hubris_archive = make_fake_sp_image(SIM_GIMLET_BOARD); + let deployed_caboose = make_fake_sp_archive_caboose(SIM_GIMLET_BOARD); + let hubris_archive = make_fake_sp_archive_with_caboose(&deployed_caboose); let sp_updater = SpUpdater::new( sp_type, @@ -65,6 +75,29 @@ async fn test_sp_updater_updates_sled() { &mgstestctx.logctx.log, ); + // Grab the initial cabooses so that we can later check that they've + // changed. + let caboose_active_before = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::SP_ITSELF.const_as_str(), + 0, + ) + .await + .unwrap() + .into_inner(); + let caboose_inactive_before = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::SP_ITSELF.const_as_str(), + 1, + ) + .await + .unwrap() + .into_inner(); + // Run the update. sp_updater.update(&mut mgs_clients).await.expect("update failed"); @@ -85,6 +118,46 @@ async fn test_sp_updater_updates_sled() { hubris_archive.image.data.len() ); + // Fetch the final cabooses. + let caboose_active_after = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::SP_ITSELF.const_as_str(), + 0, + ) + .await + .unwrap() + .into_inner(); + let caboose_inactive_after = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::SP_ITSELF.const_as_str(), + 1, + ) + .await + .unwrap() + .into_inner(); + + // The active caboose should match the one we deployed. + assert_ne!(caboose_inactive_before, caboose_active_after); + assert_eq!( + deployed_caboose.board().unwrap(), + caboose_active_after.board.as_bytes() + ); + assert_eq!( + deployed_caboose.git_commit().unwrap(), + caboose_active_after.git_commit.as_bytes(), + ); + assert_eq!( + deployed_caboose.version().unwrap(), + caboose_active_after.version.as_bytes(), + ); + + // The inactive caboose now should match the previously active one. + assert_eq!(caboose_inactive_after, caboose_active_before); + mgstestctx.teardown().await; } @@ -94,18 +167,21 @@ async fn test_sp_updater_updates_switch() { let mgstestctx = mgs_setup::test_setup("test_sp_updater_updates_switch", SpPort::One) .await; + let log = &mgstestctx.logctx.log; // Configure an MGS client. - let mut mgs_clients = - MgsClients::from_clients([gateway_client::Client::new( - &mgstestctx.client.url("/").to_string(), - mgstestctx.logctx.log.new(slog::o!("component" => "MgsClient")), - )]); + let mgs_client = gateway_client::Client::new( + &mgstestctx.client.url("/").to_string(), + log.new(slog::o!("component" => "MgsClient")), + ); + let mut mgs_clients = MgsClients::from_clients([mgs_client.clone()]); + // Configure and instantiate an `SpUpdater`. let sp_type = SpType::Switch; let sp_slot = 0; let update_id = Uuid::new_v4(); - let hubris_archive = make_fake_sp_image(SIM_SIDECAR_BOARD); + let deployed_caboose = make_fake_sp_archive_caboose(SIM_SIDECAR_BOARD); + let hubris_archive = make_fake_sp_archive_with_caboose(&deployed_caboose); let sp_updater = SpUpdater::new( sp_type, @@ -115,8 +191,33 @@ async fn test_sp_updater_updates_switch() { &mgstestctx.logctx.log, ); + // Grab the initial cabooses so that we can later check that they've + // changed. + let caboose_active_before = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::SP_ITSELF.const_as_str(), + 0, + ) + .await + .unwrap() + .into_inner(); + let caboose_inactive_before = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::SP_ITSELF.const_as_str(), + 1, + ) + .await + .unwrap() + .into_inner(); + + // Run the update. sp_updater.update(&mut mgs_clients).await.expect("update failed"); + // Ensure the SP received the complete update. let last_update_image = mgstestctx.simrack.sidecars[sp_slot as usize] .last_sp_update_data() .await @@ -133,6 +234,46 @@ async fn test_sp_updater_updates_switch() { hubris_archive.image.data.len() ); + // Fetch the final cabooses. + let caboose_active_after = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::SP_ITSELF.const_as_str(), + 0, + ) + .await + .unwrap() + .into_inner(); + let caboose_inactive_after = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::SP_ITSELF.const_as_str(), + 1, + ) + .await + .unwrap() + .into_inner(); + + // The active caboose should match the one we deployed. + assert_ne!(caboose_inactive_before, caboose_active_after); + assert_eq!( + deployed_caboose.board().unwrap(), + caboose_active_after.board.as_bytes() + ); + assert_eq!( + deployed_caboose.git_commit().unwrap(), + caboose_active_after.git_commit.as_bytes(), + ); + assert_eq!( + deployed_caboose.version().unwrap(), + caboose_active_after.version.as_bytes(), + ); + + // The inactive caboose now should match the previously active one. + assert_eq!(caboose_inactive_after, caboose_active_before); + mgstestctx.teardown().await; } @@ -189,7 +330,7 @@ async fn test_sp_updater_remembers_successful_mgs_instance() { let sp_type = SpType::Sled; let sp_slot = 0; let update_id = Uuid::new_v4(); - let hubris_archive = make_fake_sp_image(SIM_GIMLET_BOARD); + let hubris_archive = make_fake_sp_archive(SIM_GIMLET_BOARD); let sp_updater = SpUpdater::new( sp_type, @@ -307,7 +448,7 @@ async fn test_sp_updater_switches_mgs_instances_on_failure() { let sp_type = SpType::Sled; let sp_slot = 0; let update_id = Uuid::new_v4(); - let hubris_archive = make_fake_sp_image(SIM_GIMLET_BOARD); + let hubris_archive = make_fake_sp_archive(SIM_GIMLET_BOARD); let sp_updater = SpUpdater::new( sp_type, @@ -461,7 +602,7 @@ async fn test_sp_updater_delivers_progress() { let sp_type = SpType::Sled; let sp_slot = 0; let update_id = Uuid::new_v4(); - let hubris_archive = make_fake_sp_image(SIM_GIMLET_BOARD); + let hubris_archive = make_fake_sp_archive(SIM_GIMLET_BOARD); let sp_updater = SpUpdater::new( sp_type, From e2c8954900a2c82ea26fae39e0b36f1e3fdbb95c Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Tue, 29 Apr 2025 15:33:55 -0700 Subject: [PATCH 15/18] fix and add tests for sp-sim RoT update --- nexus/mgs-updates/tests/rot_updater.rs | 295 +++++++++++++++++++++++-- sp-sim/src/gimlet.rs | 6 +- sp-sim/src/sidecar.rs | 6 +- sp-sim/src/update.rs | 25 ++- 4 files changed, 300 insertions(+), 32 deletions(-) diff --git a/nexus/mgs-updates/tests/rot_updater.rs b/nexus/mgs-updates/tests/rot_updater.rs index 37b4f07ad3c..155fd199668 100644 --- a/nexus/mgs-updates/tests/rot_updater.rs +++ b/nexus/mgs-updates/tests/rot_updater.rs @@ -4,8 +4,9 @@ //! Tests `RotUpdater`'s delivery of updates to RoTs via MGS -use gateway_client::types::{RotSlot, SpType}; -use gateway_messages::SpPort; +use gateway_client::SpComponent; +use gateway_client::types::{GetRotBootInfoParams, RotSlot, SpType}; +use gateway_messages::{RotBootInfo, SpPort}; use gateway_test_utils::setup as mgs_setup; use hubtools::RawHubrisArchive; use hubtools::{CabooseBuilder, HubrisArchiveBuilder}; @@ -23,14 +24,21 @@ use tokio::net::TcpStream; use tokio::sync::mpsc; use uuid::Uuid; -fn make_fake_rot_image() -> Vec { - let caboose = CabooseBuilder::default() +fn make_fake_rot_archive() -> Vec { + let caboose = make_fake_rot_archive_caboose(); + make_fake_rot_archive_with_caboose(&caboose) +} + +fn make_fake_rot_archive_caboose() -> hubtools::Caboose { + CabooseBuilder::default() .git_commit("fake-git-commit") .board(SIM_ROT_BOARD) .version("0.0.0") .name("fake-name") - .build(); + .build() +} +fn make_fake_rot_archive_with_caboose(caboose: &hubtools::Caboose) -> Vec { let mut builder = HubrisArchiveBuilder::with_fake_image(); builder.write_caboose(caboose.as_slice()).unwrap(); builder.build_to_vec().unwrap() @@ -42,19 +50,21 @@ async fn test_rot_updater_updates_sled() { let mgstestctx = mgs_setup::test_setup("test_rot_updater_updates_sled", SpPort::One) .await; + let log = &mgstestctx.logctx.log; // Configure an MGS client. - let mut mgs_clients = - MgsClients::from_clients([gateway_client::Client::new( - &mgstestctx.client.url("/").to_string(), - mgstestctx.logctx.log.new(slog::o!("component" => "MgsClient")), - )]); + let mgs_client = gateway_client::Client::new( + &mgstestctx.client.url("/").to_string(), + log.new(slog::o!("component" => "MgsClient")), + ); + let mut mgs_clients = MgsClients::from_clients([mgs_client.clone()]); // Configure and instantiate an `RotUpdater`. let sp_type = SpType::Sled; let sp_slot = 0; let update_id = Uuid::new_v4(); - let hubris_archive = make_fake_rot_image(); + let deployed_caboose = make_fake_rot_archive_caboose(); + let hubris_archive = make_fake_rot_archive_with_caboose(&deployed_caboose); let target_rot_slot = RotSlot::B; let rot_updater = RotUpdater::new( @@ -66,6 +76,59 @@ async fn test_rot_updater_updates_sled() { &mgstestctx.logctx.log, ); + // Grab the initial cabooses, RoT state, and RoT boot info so that we can + // later check that they've changed. + let caboose_a_before = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::ROT.const_as_str(), + 0, + ) + .await + .unwrap() + .into_inner(); + let caboose_b_before = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::ROT.const_as_str(), + 1, + ) + .await + .unwrap() + .into_inner(); + let rot_state_before = + mgs_client.sp_get(sp_type, sp_slot).await.unwrap().into_inner().rot; + let rot_boot_info_before = mgs_client + .sp_rot_boot_info( + sp_type, + sp_slot, + SpComponent::ROT.const_as_str(), + &GetRotBootInfoParams { + version: RotBootInfo::HIGHEST_KNOWN_VERSION, + }, + ) + .await + .unwrap() + .into_inner(); + + // This test updates slot B, so we're assuming that the simulated SP is + // running out of slot A. + assert_eq!(rot_state_before, rot_boot_info_before); + let gateway_client::types::RotState::V3 { + slot_a_fwid: slot_a_fwid_before, + slot_b_fwid: slot_b_fwid_before, + active: RotSlot::A, + pending_persistent_boot_preference: None, + persistent_boot_preference: RotSlot::A, + transient_boot_preference: None, + .. + } = rot_state_before + else { + panic!("unexpected initial RoT state: {:?}", rot_state_before); + }; + // Run the update. rot_updater.update(&mut mgs_clients).await.expect("update failed"); @@ -86,6 +149,73 @@ async fn test_rot_updater_updates_sled() { hubris_archive.image.data.len() ); + // Grab the final cabooses and other state so we can verify that they + // changed as expected. + let caboose_a_after = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::ROT.const_as_str(), + 0, + ) + .await + .unwrap() + .into_inner(); + let caboose_b_after = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::ROT.const_as_str(), + 1, + ) + .await + .unwrap() + .into_inner(); + let rot_state_after = + mgs_client.sp_get(sp_type, sp_slot).await.unwrap().into_inner().rot; + let rot_boot_info_after = mgs_client + .sp_rot_boot_info( + sp_type, + sp_slot, + SpComponent::ROT.const_as_str(), + &GetRotBootInfoParams { + version: RotBootInfo::HIGHEST_KNOWN_VERSION, + }, + ) + .await + .unwrap() + .into_inner(); + assert_eq!(caboose_a_before, caboose_a_after); + assert_ne!(caboose_b_before, caboose_b_after); + assert_eq!( + deployed_caboose.board().unwrap(), + caboose_b_after.board.as_bytes() + ); + assert_eq!( + deployed_caboose.git_commit().unwrap(), + caboose_b_after.git_commit.as_bytes(), + ); + assert_eq!( + deployed_caboose.version().unwrap(), + caboose_b_after.version.as_bytes(), + ); + + assert_eq!(rot_state_after, rot_boot_info_after); + let gateway_client::types::RotState::V3 { + slot_a_fwid: slot_a_fwid_after, + slot_b_fwid: slot_b_fwid_after, + active: RotSlot::B, + pending_persistent_boot_preference: None, + persistent_boot_preference: RotSlot::B, + transient_boot_preference: None, + .. + } = rot_state_after + else { + panic!("unexpected final RoT state: {:?}", rot_state_after); + }; + assert_eq!(slot_a_fwid_before, slot_a_fwid_after); + assert_ne!(slot_b_fwid_before, slot_b_fwid_after); + mgstestctx.teardown().await; } @@ -95,18 +225,21 @@ async fn test_rot_updater_updates_switch() { let mgstestctx = mgs_setup::test_setup("test_rot_updater_updates_switch", SpPort::One) .await; + let log = &mgstestctx.logctx.log; // Configure an MGS client. - let mut mgs_clients = - MgsClients::from_clients([gateway_client::Client::new( - &mgstestctx.client.url("/").to_string(), - mgstestctx.logctx.log.new(slog::o!("component" => "MgsClient")), - )]); + let mgs_client = gateway_client::Client::new( + &mgstestctx.client.url("/").to_string(), + log.new(slog::o!("component" => "MgsClient")), + ); + let mut mgs_clients = MgsClients::from_clients([mgs_client.clone()]); + // Configure and instantiate an `RotUpdater`. let sp_type = SpType::Switch; let sp_slot = 0; let update_id = Uuid::new_v4(); - let hubris_archive = make_fake_rot_image(); + let deployed_caboose = make_fake_rot_archive_caboose(); + let hubris_archive = make_fake_rot_archive_with_caboose(&deployed_caboose); let target_rot_slot = RotSlot::B; let rot_updater = RotUpdater::new( @@ -118,8 +251,63 @@ async fn test_rot_updater_updates_switch() { &mgstestctx.logctx.log, ); + // Grab the initial cabooses, RoT state, and RoT boot info so that we can + // later check that they've changed. + let caboose_a_before = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::ROT.const_as_str(), + 0, + ) + .await + .unwrap() + .into_inner(); + let caboose_b_before = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::ROT.const_as_str(), + 1, + ) + .await + .unwrap() + .into_inner(); + let rot_state_before = + mgs_client.sp_get(sp_type, sp_slot).await.unwrap().into_inner().rot; + let rot_boot_info_before = mgs_client + .sp_rot_boot_info( + sp_type, + sp_slot, + SpComponent::ROT.const_as_str(), + &GetRotBootInfoParams { + version: RotBootInfo::HIGHEST_KNOWN_VERSION, + }, + ) + .await + .unwrap() + .into_inner(); + + // This test updates slot B, so we're assuming that the simulated SP is + // running out of slot A. + assert_eq!(rot_state_before, rot_boot_info_before); + let gateway_client::types::RotState::V3 { + slot_a_fwid: slot_a_fwid_before, + slot_b_fwid: slot_b_fwid_before, + active: RotSlot::A, + pending_persistent_boot_preference: None, + persistent_boot_preference: RotSlot::A, + transient_boot_preference: None, + .. + } = rot_state_before + else { + panic!("unexpected initial RoT state: {:?}", rot_state_before); + }; + + // Run the update. rot_updater.update(&mut mgs_clients).await.expect("update failed"); + // Ensure the RoT received the complete update. let last_update_image = mgstestctx.simrack.sidecars[sp_slot as usize] .last_rot_update_data() .await @@ -136,6 +324,73 @@ async fn test_rot_updater_updates_switch() { hubris_archive.image.data.len() ); + // Grab the final cabooses and other state so we can verify that they + // changed as expected. + let caboose_a_after = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::ROT.const_as_str(), + 0, + ) + .await + .unwrap() + .into_inner(); + let caboose_b_after = mgs_client + .sp_component_caboose_get( + sp_type, + sp_slot, + SpComponent::ROT.const_as_str(), + 1, + ) + .await + .unwrap() + .into_inner(); + let rot_state_after = + mgs_client.sp_get(sp_type, sp_slot).await.unwrap().into_inner().rot; + let rot_boot_info_after = mgs_client + .sp_rot_boot_info( + sp_type, + sp_slot, + SpComponent::ROT.const_as_str(), + &GetRotBootInfoParams { + version: RotBootInfo::HIGHEST_KNOWN_VERSION, + }, + ) + .await + .unwrap() + .into_inner(); + assert_eq!(caboose_a_before, caboose_a_after); + assert_ne!(caboose_b_before, caboose_b_after); + assert_eq!( + deployed_caboose.board().unwrap(), + caboose_b_after.board.as_bytes() + ); + assert_eq!( + deployed_caboose.git_commit().unwrap(), + caboose_b_after.git_commit.as_bytes(), + ); + assert_eq!( + deployed_caboose.version().unwrap(), + caboose_b_after.version.as_bytes(), + ); + + assert_eq!(rot_state_after, rot_boot_info_after); + let gateway_client::types::RotState::V3 { + slot_a_fwid: slot_a_fwid_after, + slot_b_fwid: slot_b_fwid_after, + active: RotSlot::B, + pending_persistent_boot_preference: None, + persistent_boot_preference: RotSlot::B, + transient_boot_preference: None, + .. + } = rot_state_after + else { + panic!("unexpected final RoT state: {:?}", rot_state_after); + }; + assert_eq!(slot_a_fwid_before, slot_a_fwid_after); + assert_ne!(slot_b_fwid_before, slot_b_fwid_after); + mgstestctx.teardown().await; } @@ -192,7 +447,7 @@ async fn test_rot_updater_remembers_successful_mgs_instance() { let sp_type = SpType::Sled; let sp_slot = 0; let update_id = Uuid::new_v4(); - let hubris_archive = make_fake_rot_image(); + let hubris_archive = make_fake_rot_archive(); let target_rot_slot = RotSlot::B; let rot_updater = RotUpdater::new( @@ -312,7 +567,7 @@ async fn test_rot_updater_switches_mgs_instances_on_failure() { let sp_type = SpType::Sled; let sp_slot = 0; let update_id = Uuid::new_v4(); - let hubris_archive = make_fake_rot_image(); + let hubris_archive = make_fake_rot_archive(); let target_rot_slot = RotSlot::B; let rot_updater = RotUpdater::new( @@ -458,7 +713,7 @@ async fn test_rot_updater_delivers_progress() { let sp_type = SpType::Sled; let sp_slot = 0; let update_id = Uuid::new_v4(); - let hubris_archive = make_fake_rot_image(); + let hubris_archive = make_fake_rot_archive(); let target_rot_slot = RotSlot::B; let target_sp = &mgstestctx.simrack.gimlets[sp_slot as usize]; diff --git a/sp-sim/src/gimlet.rs b/sp-sim/src/gimlet.rs index 03e5560dcbe..81bc312cba7 100644 --- a/sp-sim/src/gimlet.rs +++ b/sp-sim/src/gimlet.rs @@ -975,8 +975,7 @@ impl SpHandler for Handler { "received SP update prepare request"; "update" => ?update, ); - self.update_state.prepare( - SpComponent::SP_ITSELF, + self.update_state.sp_update_prepare( update.id, update.sp_image_size.try_into().unwrap(), ) @@ -992,10 +991,11 @@ impl SpHandler for Handler { "update" => ?update, ); - self.update_state.prepare( + self.update_state.component_update_prepare( update.component, update.id, update.total_size.try_into().unwrap(), + update.slot, ) } diff --git a/sp-sim/src/sidecar.rs b/sp-sim/src/sidecar.rs index 35269ebd6b1..eda63dbfed7 100644 --- a/sp-sim/src/sidecar.rs +++ b/sp-sim/src/sidecar.rs @@ -696,8 +696,7 @@ impl SpHandler for Handler { "received update prepare request"; "update" => ?update, ); - self.update_state.prepare( - SpComponent::SP_ITSELF, + self.update_state.sp_update_prepare( update.id, update.sp_image_size.try_into().unwrap(), ) @@ -712,10 +711,11 @@ impl SpHandler for Handler { "received update prepare request"; "update" => ?update, ); - self.update_state.prepare( + self.update_state.component_update_prepare( update.component, update.id, update.total_size.try_into().unwrap(), + update.slot, ) } diff --git a/sp-sim/src/update.rs b/sp-sim/src/update.rs index c5157518f15..888ffa1eaab 100644 --- a/sp-sim/src/update.rs +++ b/sp-sim/src/update.rs @@ -199,30 +199,43 @@ impl SimSpUpdate { } } - // TODO-completeness Split into `sp_prepare` and `component_prepare` when we - // need to simulate aux flash-related (SP update only) things. - pub(crate) fn prepare( + pub(crate) fn sp_update_prepare( + &mut self, + id: UpdateId, + total_size: usize, + ) -> Result<(), SpError> { + // SP updates always target slot 0. + let slot = 0; + self.component_update_prepare( + SpComponent::SP_ITSELF, + id, + total_size, + slot, + ) + } + + pub(crate) fn component_update_prepare( &mut self, component: SpComponent, id: UpdateId, total_size: usize, + slot: u16, ) -> Result<(), SpError> { match &self.state { state @ UpdateState::Prepared { .. } => { Err(SpError::UpdateInProgress(state.to_message())) } + // XXX-dap shouldn't we fail in some of these cases? UpdateState::NotPrepared | UpdateState::Aborted(_) | UpdateState::Completed { .. } => { - // XXX-dap shouldn't we fail in some of these cases? let slot = if component == SpComponent::HOST_CPU_BOOT_FLASH { match self.active_host_slot { Some(slot) => slot, None => return Err(SpError::InvalidSlotForComponent), } } else { - // We don't manage SP or RoT slots, so just use 0 - 0 + slot }; self.state = UpdateState::Prepared { From b355377f7c11b2322309770a89fb0e47f2783bfc Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Tue, 29 Apr 2025 15:37:45 -0700 Subject: [PATCH 16/18] digest is not needed here --- Cargo.lock | 1 - sp-sim/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9c94a9dc240..1d17fb3ea07 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11840,7 +11840,6 @@ dependencies = [ "anyhow", "async-trait", "clap", - "digest", "dropshot 0.16.0", "futures", "gateway-messages", diff --git a/sp-sim/Cargo.toml b/sp-sim/Cargo.toml index f2e8da43fdd..b337f604915 100644 --- a/sp-sim/Cargo.toml +++ b/sp-sim/Cargo.toml @@ -11,7 +11,6 @@ workspace = true anyhow.workspace = true async-trait.workspace = true clap.workspace = true -digest.workspace = true dropshot.workspace = true futures.workspace = true gateway-messages.workspace = true From 786e4aa518bea71eec65ec6ea05b1ffa2220dcc1 Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Tue, 29 Apr 2025 15:44:52 -0700 Subject: [PATCH 17/18] clean up XXXs --- sp-sim/src/update.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/sp-sim/src/update.rs b/sp-sim/src/update.rs index 888ffa1eaab..93f4acd2d8b 100644 --- a/sp-sim/src/update.rs +++ b/sp-sim/src/update.rs @@ -43,7 +43,6 @@ pub(crate) struct SimSpUpdate { /// records whether a change to the stage0 "active slot" has been requested pending_stage0_update: bool, - // XXX-dap review the lifecycle of all of these fields /// current caboose for the SP active slot caboose_sp_active: CabooseValue, /// current caboose for the SP inactive slot @@ -225,7 +224,6 @@ impl SimSpUpdate { state @ UpdateState::Prepared { .. } => { Err(SpError::UpdateInProgress(state.to_message())) } - // XXX-dap shouldn't we fail in some of these cases? UpdateState::NotPrepared | UpdateState::Aborted(_) | UpdateState::Completed { .. } => { @@ -253,9 +251,6 @@ impl SimSpUpdate { // right, but it's most important that consumers handle both // cases, which they will if they test updates to both of these // components. - // XXX-dap should we instead calculate the caboose on-demand - // from the data we've received? (Will that be annoying for the - // initial condition?) if let Some(caboose) = self.caboose_mut(component, slot) { *caboose = if component == SpComponent::STAGE0 { CabooseValue::InvalidFailedRead @@ -340,7 +335,6 @@ impl SimSpUpdate { *caboose_value = CabooseValue::Caboose(caboose); } Err(_) => { - // XXX-dap log error *caboose_value = CabooseValue::InvalidMissing; } }; From e67443243317915c304b63c191b4eeabb7c48c8e Mon Sep 17 00:00:00 2001 From: David Pacheco Date: Tue, 29 Apr 2025 15:48:02 -0700 Subject: [PATCH 18/18] hakari --- Cargo.lock | 1 + workspace-hack/Cargo.toml | 2 ++ 2 files changed, 3 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 1d17fb3ea07..e96bac927d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7909,6 +7909,7 @@ dependencies = [ "getrandom 0.3.1", "group", "hashbrown 0.15.1", + "heck 0.4.1", "hickory-proto", "hmac", "hyper", diff --git a/workspace-hack/Cargo.toml b/workspace-hack/Cargo.toml index 8c4f4c52de9..607e3d7ee8d 100644 --- a/workspace-hack/Cargo.toml +++ b/workspace-hack/Cargo.toml @@ -62,6 +62,7 @@ generic-array = { version = "0.14.7", default-features = false, features = ["mor getrandom-6f8ce4dd05d13bba = { package = "getrandom", version = "0.2.15", default-features = false, features = ["js", "rdrand", "std"] } group = { version = "0.13.0", default-features = false, features = ["alloc"] } hashbrown = { version = "0.15.1" } +heck = { version = "0.4.1" } hickory-proto = { version = "0.24.4", features = ["text-parsing"] } hmac = { version = "0.12.1", default-features = false, features = ["reset"] } hyper = { version = "1.6.0", features = ["full"] } @@ -185,6 +186,7 @@ generic-array = { version = "0.14.7", default-features = false, features = ["mor getrandom-6f8ce4dd05d13bba = { package = "getrandom", version = "0.2.15", default-features = false, features = ["js", "rdrand", "std"] } group = { version = "0.13.0", default-features = false, features = ["alloc"] } hashbrown = { version = "0.15.1" } +heck = { version = "0.4.1" } hickory-proto = { version = "0.24.4", features = ["text-parsing"] } hmac = { version = "0.12.1", default-features = false, features = ["reset"] } hyper = { version = "1.6.0", features = ["full"] }