Skip to content

Commit

Permalink
auxflash: make startup a lot faster.
Browse files Browse the repository at this point in the history
The auxflash server was computing a SHA3 of every potentially occupied
slot in the QSPI flash, only to compare it to _the stored SHA3_ and then
compare it to _the expected SHA3_ and then throw it away. This has been
causing Sidecar startup to be linear in the number of valid images that
have ever been flashed to auxflash, up to 16.

This change rearranges the logic, at least for startup. For each chunk,
we now see if it even claims to have the right SHA. Only then do we
compute the actual SHA to validate. This reduces an 11.6s delay observed
on one Sidecar to just over 1s, and knocks 6s off the boot (something
else is still delaying for 5s).

I haven't changed the behavior of the externally exposed
`read_slot_checksum` operation, because it has no documentation and I
can't figure out what it's used for, so I'm not sure I would get the
semantics right or know how to test it.

Fixes #1955.
  • Loading branch information
cbiffle committed Dec 26, 2024
1 parent 11da9c0 commit 06501cf
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 32 deletions.
42 changes: 17 additions & 25 deletions drv/auxflash-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ pub struct AuxFlashBlob {
/// Extension trait to do auxflash operations on anything that
/// implements `TlvcRead`.
pub trait TlvcReadAuxFlash {
fn read_checksum(self) -> Result<AuxFlashChecksum, AuxFlashError>;
fn calculate_checksum(self) -> Result<AuxFlashChecksum, AuxFlashError>;
fn read_stored_checksum(self) -> Result<AuxFlashChecksum, AuxFlashError>;
fn get_blob_by_tag(
self,
slot: u32,
Expand All @@ -89,29 +90,31 @@ impl<R> TlvcReadAuxFlash for R
where
R: TlvcRead,
{
fn read_checksum(self) -> Result<AuxFlashChecksum, AuxFlashError> {
fn read_stored_checksum(self) -> Result<AuxFlashChecksum, AuxFlashError> {
let mut reader = TlvcReader::begin(self)
.map_err(|_| AuxFlashError::TlvcReaderBeginFailed)?;

let mut chck_expected = None;
let mut chck_actual = None;
while let Ok(Some(chunk)) = reader.next() {
if &chunk.header().tag == b"CHCK" {
if chck_expected.is_some() {
return Err(AuxFlashError::MultipleChck);
} else if chunk.len() != 32 {
if chunk.len() != 32 {
return Err(AuxFlashError::BadChckSize);
}
let mut out = [0; 32];
chunk
.read_exact(0, &mut out)
.map_err(|_| AuxFlashError::ChunkReadFail)?;
chck_expected = Some(out);
} else if &chunk.header().tag == b"AUXI" {
if chck_actual.is_some() {
return Err(AuxFlashError::MultipleAuxi);
}
return Ok(AuxFlashChecksum(out));
}
}
Err(AuxFlashError::MissingChck)
}

fn calculate_checksum(self) -> Result<AuxFlashChecksum, AuxFlashError> {
let mut reader = TlvcReader::begin(self)
.map_err(|_| AuxFlashError::TlvcReaderBeginFailed)?;

while let Ok(Some(chunk)) = reader.next() {
if &chunk.header().tag == b"AUXI" {
// Read data and calculate the checksum using a scratch buffer
let mut sha = Sha3_256::new();
let mut scratch = [0u8; 256];
Expand All @@ -126,23 +129,12 @@ where
}
let sha_out = sha.finalize();

// Save the checksum in `chck_actual`
let mut out = [0; 32];
out.copy_from_slice(sha_out.as_slice());
chck_actual = Some(out);
}
}
match (chck_expected, chck_actual) {
(None, _) => Err(AuxFlashError::MissingChck),
(_, None) => Err(AuxFlashError::MissingAuxi),
(Some(a), Some(b)) => {
if a != b {
Err(AuxFlashError::ChckMismatch)
} else {
Ok(AuxFlashChecksum(chck_expected.unwrap()))
}
return Ok(AuxFlashChecksum(out));
}
}
Err(AuxFlashError::MissingAuxi)
}

fn get_blob_by_tag(
Expand Down
40 changes: 33 additions & 7 deletions drv/auxflash-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ impl ServerImpl {
&self,
slot: u32,
) -> Result<AuxFlashChecksum, AuxFlashError> {
read_slot_checksum(&self.qspi, slot)
read_and_check_slot_checksum(&self.qspi, slot)
}

/// Checks that the matched slot in this even/odd pair also has valid data.
Expand Down Expand Up @@ -434,16 +434,36 @@ impl NotificationHandler for ServerImpl {

fn scan_for_active_slot(qspi: &Qspi) -> Option<u32> {
for i in 0..SLOT_COUNT {
if let Ok(chck) = read_slot_checksum(qspi, i) {
if chck.0 == AUXI_CHECKSUM {
return Some(i);
}
let handle = SlotReader {
qspi,
base: i * SLOT_SIZE as u32,
};

let Ok(chck) = handle.read_stored_checksum() else {
// Just skip to the next slot if it's empty or invalid.
continue;
};

if chck.0 != AUXI_CHECKSUM {
// If it's not the chunk we're interested in, don't bother hashing
// it.
continue;
}

let Ok(actual) = handle.calculate_checksum() else {
// TODO: this ignores I/O errors, but, this is how the code has
// always been structured...
continue;
};

if chck == actual {
return Some(i);
}
}
None
}

fn read_slot_checksum(
fn read_and_check_slot_checksum(
qspi: &Qspi,
slot: u32,
) -> Result<AuxFlashChecksum, AuxFlashError> {
Expand All @@ -454,7 +474,13 @@ fn read_slot_checksum(
qspi,
base: slot * SLOT_SIZE as u32,
};
handle.read_checksum()
let claimed = handle.read_stored_checksum()?;
let actual = handle.calculate_checksum()?;
if claimed == actual {
Ok(actual)
} else {
Err(AuxFlashError::ChckMismatch)
}
}

////////////////////////////////////////////////////////////////////////////////
Expand Down

0 comments on commit 06501cf

Please sign in to comment.