Merge branch 'master' into perf/auxflash-remove-panics

Author: aapoalas

.github/workflows/release.yml

@@ -30,7 +30,7 @@ jobs:
         name: Download Brussels
         run: |
           if [[ "${BRUSSELS_RUN_ID}" == "" ]]; then
-            gh release download -R oxidecomputer/brussels -p brussels
+            gh release download -R oxidecomputer/brussels -p brussels v0.1.1
           else
             gh run download "${BRUSSELS_RUN_ID}" -R oxidecomputer/brussels -n prebuilt-binary
           fi

drv/cosmo-hf/src/apob.rs

@@ -8,7 +8,8 @@
 //! to Rev. 2.0 February 2025.
 
 use crate::{
-    hf::ServerImpl, FlashAddr, FlashDriver, PAGE_SIZE_BYTES, SECTOR_SIZE_BYTES,
+    hf::HfBufs, hf::ServerImpl, FlashAddr, FlashDriver, PAGE_SIZE_BYTES,
+    SECTOR_SIZE_BYTES,
 };
 use drv_hf_api::{
     ApobBeginError, ApobCommitError, ApobHash, ApobReadError, ApobWriteError,
@@ -237,34 +238,6 @@ impl ApobSlot {
     }
 }
 
-pub(crate) struct ApobBufs {
-    persistent_data: &'static mut [u8; APOB_PERSISTENT_DATA_STRIDE],
-    page: &'static mut [u8; PAGE_SIZE_BYTES],
-    scratch: &'static mut [u8; PAGE_SIZE_BYTES],
-}
-
-/// Grabs references to the static buffers.  Can only be called once.
-impl ApobBufs {
-    pub fn claim_statics() -> Self {
-        use static_cell::ClaimOnceCell;
-        static BUFS: ClaimOnceCell<(
-            [u8; APOB_PERSISTENT_DATA_STRIDE],
-            [u8; PAGE_SIZE_BYTES],
-            [u8; PAGE_SIZE_BYTES],
-        )> = ClaimOnceCell::new((
-            [0; APOB_PERSISTENT_DATA_STRIDE],
-            [0; PAGE_SIZE_BYTES],
-            [0; PAGE_SIZE_BYTES],
-        ));
-        let (persistent_data, page, scratch) = BUFS.claim();
-        Self {
-            persistent_data,
-            page,
-            scratch,
-        }
-    }
-}
-
 /// State machine data, which implements the logic from RFD 593
 ///
 /// See rfd.shared.oxide.computer/rfd/593#_production_strength_implementation
@@ -397,7 +370,7 @@ impl ApobState {
     ///
     /// Searches for an active slot in the metadata regions, updating the offset
     /// in the FPGA driver if found, and erases unused or invalid slots.
-    pub(crate) fn init(drv: &mut FlashDriver, buf: &mut ApobBufs) -> Self {
+    pub(crate) fn init(drv: &mut FlashDriver, buf: &mut HfBufs) -> Self {
         // Look up persistent data, which specifies an active slot
         let out = if let Some(s) = Self::get_slot(drv) {
             // Erase the inactive slot, in preparation for writing
@@ -448,7 +421,7 @@ impl ApobState {
     }
 
     /// Erases the given APOB slot
-    fn slot_erase(drv: &mut FlashDriver, buf: &mut ApobBufs, slot: ApobSlot) {
+    fn slot_erase(drv: &mut FlashDriver, buf: &mut HfBufs, slot: ApobSlot) {
         static_assertions::const_assert!(
             APOB_SLOT_SIZE.is_multiple_of(SECTOR_SIZE_BYTES)
         );
@@ -463,7 +436,7 @@ impl ApobState {
     /// If `size > APOB_SLOT_SIZE`
     fn slot_erase_range(
         drv: &mut FlashDriver,
-        buf: &mut ApobBufs,
+        buf: &mut HfBufs,
         slot: ApobSlot,
         size: u32,
     ) {
@@ -578,7 +551,7 @@ impl ApobState {
     pub(crate) fn write(
         &mut self,
         drv: &mut FlashDriver,
-        buf: &mut ApobBufs,
+        buf: &mut HfBufs,
         offset: u32,
         data: Leased<R, [u8]>,
     ) -> Result<(), ApobWriteError> {
@@ -647,8 +620,7 @@ impl ApobState {
             // If any byte is not a match, then we have to do a flash write
             // (otherwise, it's an idempotent write and we can skip it)
             if needs_write {
-                drv.flash_write(addr, &mut &buf.page[..n])
-                    .map_err(|_| ApobWriteError::WriteFailed)?;
+                drv.flash_write(addr, &buf.page[..n]);
             }
         }
         Ok(())
@@ -657,7 +629,7 @@ impl ApobState {
     pub(crate) fn read(
         &mut self,
         drv: &mut FlashDriver,
-        buf: &mut ApobBufs,
+        buf: &mut HfBufs,
         offset: u32,
         data: Leased<W, [u8]>,
     ) -> Result<usize, ApobReadError> {
@@ -716,7 +688,7 @@ impl ApobState {
     pub(crate) fn commit(
         &mut self,
         drv: &mut FlashDriver,
-        buf: &mut ApobBufs,
+        buf: &mut HfBufs,
     ) -> Result<(), ApobCommitError> {
         drv.check_flash_mux_state()
             .map_err(|_| ApobCommitError::InvalidState)?;
@@ -776,7 +748,7 @@ impl ApobState {
 
     fn apob_validate(
         drv: &mut FlashDriver,
-        buf: &mut ApobBufs,
+        buf: &mut HfBufs,
         expected_length: u32,
         expected_hash: ApobHash,
         write_slot: ApobSlot,
@@ -852,17 +824,17 @@ impl ApobState {
 
     fn write_raw_persistent_data(
         drv: &mut FlashDriver,
-        buf: &mut ApobBufs,
+        buf: &mut HfBufs,
         data: ApobRawPersistentData,
         meta: Meta,
     ) {
         let mut found: Option<FlashAddr> = None;
         for offset in (0..APOB_META_SIZE).step_by(APOB_PERSISTENT_DATA_STRIDE) {
             let addr = meta.flash_addr(offset).unwrap_lite();
             // Infallible when using a slice
-            drv.flash_read(addr, &mut buf.persistent_data.as_mut_slice())
+            drv.flash_read(addr, &mut buf.apob_persistent_data.as_mut_slice())
                 .unwrap_lite();
-            if buf.persistent_data.iter().all(|c| *c == 0xFF) {
+            if buf.apob_persistent_data.iter().all(|c| *c == 0xFF) {
                 found = Some(addr);
                 break;
             }
@@ -872,7 +844,6 @@ impl ApobState {
             drv.flash_sector_erase(addr);
             addr
         });
-        // Infallible when using a slice
-        drv.flash_write(addr, &mut data.as_bytes()).unwrap_lite();
+        drv.flash_write(addr, data.as_bytes());
     }
 }

drv/cosmo-hf/src/hf.rs

@@ -9,15 +9,16 @@ use drv_hf_api::{
     HF_PERSISTENT_DATA_STRIDE,
 };
 use idol_runtime::{
-    LeaseBufReader, LeaseBufWriter, Leased, LenLimit, NotificationHandler,
+    ClientError, LeaseBufWriter, Leased, LenLimit, NotificationHandler,
     RequestError, R, W,
 };
 use ringbuf::ringbuf_entry_root as ringbuf_entry;
 use userlib::{set_timer_relative, task_slot, RecvMessage, UnwrapLite};
 use zerocopy::{FromZeros, IntoBytes};
 
 use crate::{
-    apob, FlashAddr, FlashDriver, Trace, PAGE_SIZE_BYTES, SECTOR_SIZE_BYTES,
+    apob, apob::APOB_PERSISTENT_DATA_STRIDE, FlashAddr, FlashDriver, Trace,
+    PAGE_SIZE_BYTES, SECTOR_SIZE_BYTES,
 };
 
 task_slot!(HASH, hash_driver);
@@ -33,7 +34,36 @@ pub struct ServerImpl {
     hash: HashData,
 
     pub(crate) apob_state: apob::ApobState,
-    pub(crate) apob_buf: apob::ApobBufs,
+    pub(crate) buf: HfBufs,
+}
+
+pub(crate) struct HfBufs {
+    pub(crate) apob_persistent_data:
+        &'static mut [u8; APOB_PERSISTENT_DATA_STRIDE],
+    pub(crate) page: &'static mut [u8; PAGE_SIZE_BYTES],
+    pub(crate) scratch: &'static mut [u8; PAGE_SIZE_BYTES],
+}
+
+/// Grabs references to the static buffers.  Can only be called once.
+impl HfBufs {
+    pub fn claim_statics() -> Self {
+        use static_cell::ClaimOnceCell;
+        static BUFS: ClaimOnceCell<(
+            [u8; APOB_PERSISTENT_DATA_STRIDE],
+            [u8; PAGE_SIZE_BYTES],
+            [u8; PAGE_SIZE_BYTES],
+        )> = ClaimOnceCell::new((
+            [0; APOB_PERSISTENT_DATA_STRIDE],
+            [0; PAGE_SIZE_BYTES],
+            [0; PAGE_SIZE_BYTES],
+        ));
+        let (apob_persistent_data, page, scratch) = BUFS.claim();
+        Self {
+            apob_persistent_data,
+            page,
+            scratch,
+        }
+    }
 }
 
 /// This tunes how many bytes we hash in a single async timer notification
@@ -49,15 +79,15 @@ impl ServerImpl {
     /// Persistent data is loaded from the flash chip and used to select `dev`;
     /// in addition, it is made redundant (written to both virtual devices).
     pub fn new(mut drv: FlashDriver) -> Self {
-        let mut apob_buf = apob::ApobBufs::claim_statics();
-        let apob_state = apob::ApobState::init(&mut drv, &mut apob_buf);
+        let mut buf = HfBufs::claim_statics();
+        let apob_state = apob::ApobState::init(&mut drv, &mut buf);
 
         let mut out = Self {
             dev: drv_hf_api::HfDevSelect::Flash0,
             drv,
             hash: HashData::new(HASH.get_task_id()),
             apob_state,
-            apob_buf,
+            buf,
         };
         out.drv.set_flash_mux_state(HfMuxState::SP);
         out.ensure_persistent_data_is_redundant();
@@ -203,10 +233,7 @@ impl ServerImpl {
                 addr
             }
         };
-        // flash_write is infallible when given a slice
-        self.drv
-            .flash_write(addr, &mut raw_data.as_bytes())
-            .unwrap_lite();
+        self.drv.flash_write(addr, raw_data.as_bytes());
     }
 
     /// Ensures that the persistent data is consistent between the virtual devs
@@ -421,12 +448,12 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
         self.drv.check_flash_mux_state()?;
         let addr = self.flash_addr(addr, data.len() as u32)?;
         self.invalidate_write();
-        self.drv
-            .flash_write(
-                addr,
-                &mut LeaseBufReader::<_, 32>::from(data.into_inner()),
-            )
-            .map_err(|()| RequestError::went_away())
+        // Read the entire data block into our address space.
+        data.read_range(0..data.len(), &mut self.buf.scratch[..data.len()])
+            .map_err(|_| RequestError::Fail(ClientError::WentAway))?;
+
+        self.drv.flash_write(addr, &self.buf.scratch[..data.len()]);
+        Ok(())
     }
 
     fn page_program_dev(
@@ -527,7 +554,7 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
         data: Leased<R, [u8]>,
     ) -> Result<(), RequestError<drv_hf_api::ApobWriteError>> {
         self.apob_state
-            .write(&mut self.drv, &mut self.apob_buf, offset, data)
+            .write(&mut self.drv, &mut self.buf, offset, data)
             .map_err(RequestError::from)
     }
 
@@ -536,7 +563,7 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
         _: &RecvMessage,
     ) -> Result<(), RequestError<drv_hf_api::ApobCommitError>> {
         self.apob_state
-            .commit(&mut self.drv, &mut self.apob_buf)
+            .commit(&mut self.drv, &mut self.buf)
             .map_err(RequestError::from)
     }
 
@@ -555,7 +582,7 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
         data: Leased<W, [u8]>,
     ) -> Result<usize, RequestError<drv_hf_api::ApobReadError>> {
         self.apob_state
-            .read(&mut self.drv, &mut self.apob_buf, offset, data)
+            .read(&mut self.drv, &mut self.buf, offset, data)
             .map_err(RequestError::from)
     }
 
@@ -589,7 +616,7 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
             // This also unlocks the APOB so it can be written (once muxed back
             // to the SP).
             self.apob_state =
-                apob::ApobState::init(&mut self.drv, &mut self.apob_buf);
+                apob::ApobState::init(&mut self.drv, &mut self.buf);
         }
         self.drv.set_flash_mux_state(state);
         self.invalidate_mux_switch();