control-plane-agent: finish ComponentUpdater

This change causes SpUpdate to impl ComponentUpdater, instead of sporting functions that look a lot _like_ ComponentUpdater but aren't actually. This fixes a warning in the newest toolchain, which was concerned that operations in the non-public trait were going unused (which they were!). In order to adapt ComponentUpdater to be able to describe all three cases, I have added two associated types to it. UpdatePrepare parameterizes the difference between RoT/HostFlash updates (which want ComponentUpdatePrepare) and SP updates (which want a different SpUpdatePrepare type). SubComponent models the fact that only the SP seems to want a specifier for sub-components inside itself; the others set this to ().
oxidecomputer · Apr 4, 2024 · 2eb309e · 2eb309e
1 parent 4303eb3
commit 2eb309e
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 27 deletions.
diff --git a/task/control-plane-agent/src/mgs_gimlet.rs b/task/control-plane-agent/src/mgs_gimlet.rs
@@ -623,10 +623,10 @@ impl SpHandler for MgsHandler {
                 .ingest_chunk(&chunk.component, &chunk.id, chunk.offset, data),
             SpComponent::HOST_CPU_BOOT_FLASH => self
                 .host_flash_update
-                .ingest_chunk(&chunk.id, chunk.offset, data),
-            SpComponent::ROT | SpComponent::STAGE0 => {
-                self.rot_update.ingest_chunk(&chunk.id, chunk.offset, data)
-            }
+                .ingest_chunk(&(), &chunk.id, chunk.offset, data),
+            SpComponent::ROT | SpComponent::STAGE0 => self
+                .rot_update
+                .ingest_chunk(&(), &chunk.id, chunk.offset, data),
             _ => Err(SpError::RequestUnsupportedForComponent),
         }
     }

diff --git a/task/control-plane-agent/src/mgs_psc.rs b/task/control-plane-agent/src/mgs_psc.rs
@@ -332,9 +332,9 @@ impl SpHandler for MgsHandler {
             SpComponent::SP_ITSELF | SpComponent::SP_AUX_FLASH => self
                 .sp_update
                 .ingest_chunk(&chunk.component, &chunk.id, chunk.offset, data),
-            SpComponent::ROT | SpComponent::STAGE0 => {
-                self.rot_update.ingest_chunk(&chunk.id, chunk.offset, data)
-            }
+            SpComponent::ROT | SpComponent::STAGE0 => self
+                .rot_update
+                .ingest_chunk(&(), &chunk.id, chunk.offset, data),
             _ => Err(SpError::RequestUnsupportedForComponent),
         }
     }

diff --git a/task/control-plane-agent/src/mgs_sidecar.rs b/task/control-plane-agent/src/mgs_sidecar.rs
@@ -385,9 +385,9 @@ impl SpHandler for MgsHandler {
             SpComponent::SP_ITSELF | SpComponent::SP_AUX_FLASH => self
                 .sp_update
                 .ingest_chunk(&chunk.component, &chunk.id, chunk.offset, data),
-            SpComponent::ROT | SpComponent::STAGE0 => {
-                self.rot_update.ingest_chunk(&chunk.id, chunk.offset, data)
-            }
+            SpComponent::ROT | SpComponent::STAGE0 => self
+                .rot_update
+                .ingest_chunk(&(), &chunk.id, chunk.offset, data),
             _ => Err(SpError::RequestUnsupportedForComponent),
         }
     }

diff --git a/task/control-plane-agent/src/update/host_flash.rs b/task/control-plane-agent/src/update/host_flash.rs
@@ -84,6 +84,9 @@ static_assertions::const_assert!(
 impl ComponentUpdater for HostFlashUpdate {
     const BLOCK_SIZE: usize = PAGE_SIZE_BYTES;
 
+    type UpdatePrepare = ComponentUpdatePrepare;
+    type SubComponent = ();
+
     fn prepare(
         &mut self,
         buffer: &'static UpdateBuffer,
@@ -243,6 +246,7 @@ impl ComponentUpdater for HostFlashUpdate {
 
     fn ingest_chunk(
         &mut self,
+        _sub: &(),
         id: &UpdateId,
         offset: u32,
         mut data: &[u8],

diff --git a/task/control-plane-agent/src/update/mod.rs b/task/control-plane-agent/src/update/mod.rs
@@ -3,9 +3,7 @@
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
 use crate::mgs_handler::UpdateBuffer;
-use gateway_messages::{
-    ComponentUpdatePrepare, SpError, UpdateId, UpdateStatus,
-};
+use gateway_messages::{SpError, UpdateId, UpdateStatus};
 
 #[cfg(feature = "gimlet")]
 pub(crate) mod host_flash;
@@ -30,6 +28,15 @@ pub(crate) trait ComponentUpdater {
     /// mechanism wants as a single chunk.
     const BLOCK_SIZE: usize;
 
+    /// Record provided to the `prepare` operation. Generally
+    /// `ComponentUpdatePrepare`, and would default to that if associated type
+    /// defaults were stable at the time of this writing (2024-04), which they
+    /// are not.
+    type UpdatePrepare;
+
+    /// Type used to specify sub-components within a component.
+    type SubComponent;
+
     /// Attempt to start preparing for an update, using `buffer` as the backing
     /// store for incoming data.
     ///
@@ -38,7 +45,7 @@ pub(crate) trait ComponentUpdater {
     fn prepare(
         &mut self,
         buffer: &'static UpdateBuffer,
-        update: ComponentUpdatePrepare,
+        update: Self::UpdatePrepare,
     ) -> Result<(), SpError>;
 
     /// Returns true if this task needs `step_preparation()` called.
@@ -53,6 +60,7 @@ pub(crate) trait ComponentUpdater {
     /// Attempt to ingest a single update chunk from MGS.
     fn ingest_chunk(
         &mut self,
+        sub: &Self::SubComponent,
         id: &UpdateId,
         offset: u32,
         data: &[u8],

diff --git a/task/control-plane-agent/src/update/rot.rs b/task/control-plane-agent/src/update/rot.rs
@@ -51,6 +51,9 @@ enum State {
 impl ComponentUpdater for RotUpdate {
     const BLOCK_SIZE: usize = BLOCK_SIZE_BYTES;
 
+    type UpdatePrepare = ComponentUpdatePrepare;
+    type SubComponent = ();
+
     fn prepare(
         &mut self,
         buffer: &'static UpdateBuffer,
@@ -131,6 +134,7 @@ impl ComponentUpdater for RotUpdate {
 
     fn ingest_chunk(
         &mut self,
+        _sub: &(),
         id: &UpdateId,
         offset: u32,
         mut data: &[u8],

diff --git a/task/control-plane-agent/src/update/sp.rs b/task/control-plane-agent/src/update/sp.rs
@@ -56,6 +56,7 @@
 
 use crate::mgs_common::UPDATE_SERVER;
 use crate::mgs_handler::{BorrowedUpdateBuffer, UpdateBuffer};
+use crate::update::ComponentUpdater;
 use cfg_if::cfg_if;
 use core::ops::{Deref, DerefMut};
 use drv_caboose::CabooseReader;
@@ -98,24 +99,29 @@ pub(crate) struct SpUpdate {
 }
 
 impl SpUpdate {
-    #[cfg(feature = "auxflash")]
-    pub(crate) const BLOCK_SIZE: usize =
-        crate::usize_max(BLOCK_SIZE_BYTES, drv_auxflash_api::PAGE_SIZE_BYTES);
-    #[cfg(not(feature = "auxflash"))]
-    pub(crate) const BLOCK_SIZE: usize = BLOCK_SIZE_BYTES;
-
     pub(crate) fn new() -> Self {
         Self {
             sp_task: Update::from(UPDATE_SERVER.get_task_id()),
             auxflash_task: AuxFlash::from(AUX_FLASH_SERVER.get_task_id()),
             current: None,
         }
     }
+}
 
-    pub(crate) fn prepare(
+impl ComponentUpdater for SpUpdate {
+    #[cfg(feature = "auxflash")]
+    const BLOCK_SIZE: usize =
+        crate::usize_max(BLOCK_SIZE_BYTES, drv_auxflash_api::PAGE_SIZE_BYTES);
+    #[cfg(not(feature = "auxflash"))]
+    const BLOCK_SIZE: usize = BLOCK_SIZE_BYTES;
+
+    type UpdatePrepare = SpUpdatePrepare;
+    type SubComponent = SpComponent;
+
+    fn prepare(
         &mut self,
         buffer: &'static UpdateBuffer,
-        update: SpUpdatePrepare,
+        update: Self::UpdatePrepare,
     ) -> Result<(), SpError> {
         // Do we have an update already in progress?
         match self.current.as_ref().map(|c| c.state()) {
@@ -178,14 +184,14 @@ impl SpUpdate {
         Ok(())
     }
 
-    pub(crate) fn is_preparing(&self) -> bool {
+    fn is_preparing(&self) -> bool {
         match self.current.as_ref().map(|c| c.state()) {
             Some(State::AuxFlash(s)) => s.is_preparing(),
             _ => false,
         }
     }
 
-    pub(crate) fn step_preparation(&mut self) {
+    fn step_preparation(&mut self) {
         // Do we have an update?
         let current = match self.current.as_mut() {
             Some(current) => current,
@@ -231,7 +237,7 @@ impl SpUpdate {
         });
     }
 
-    pub(crate) fn status(&self) -> UpdateStatus {
+    fn status(&self) -> UpdateStatus {
         let current = match self.current.as_ref() {
             Some(current) => current,
             None => return UpdateStatus::None,
@@ -264,7 +270,7 @@ impl SpUpdate {
         }
     }
 
-    pub(crate) fn ingest_chunk(
+    fn ingest_chunk(
         &mut self,
         component: &SpComponent,
         id: &UpdateId,
@@ -377,7 +383,7 @@ impl SpUpdate {
         })
     }
 
-    pub(crate) fn abort(&mut self, id: &UpdateId) -> Result<(), SpError> {
+    fn abort(&mut self, id: &UpdateId) -> Result<(), SpError> {
         // Do we have an update in progress? If not, nothing to do.
         let current = match self.current.as_mut() {
             Some(current) => current,