Skip to content

Commit 0256d22

Browse files
committed
Fix host flash hash invalidation
The existing hash invalidation can leave slot stuck in `HashInProgress` permanently: ``` pilot -r dublin sp exec -e 'component-active-slot host-boot-flash -s 1 -p' BRM27230037 pilot -r dublin sp exec -e 'start-host-flash-hash 0' BRM27230037 pilot -r dublin sp exec -e 'power-state A2' BRM27230037 sleep 30 pilot -r dublin sp exec -e 'power-state A0' BRM27230037 sleep 120 $ pilot -r dublin sp exec -e 'get-host-flash-hash 0' BRM27230037 Jul 28 19:22:14.066 INFO creating SP handle on interface dublin_sw0tp0, component: faux-mgs Jul 28 19:22:14.072 INFO initial discovery complete, addr: [fe80::aa40:25ff:fe04:604%5]:11111, interface: dublin_sw0tp0, socket: control-plane-agent, component: faux-mgs Error: Error response from SP: hf: Hash calcuation in progress ``` Switching the mux (i.e. going from Host <-> SP) should invalidate everything and require a recalcuation. We can also end up in this state if we write to one bank while a hash of the other bank is in progress. Fix this by splitting out the invalidation functions separately and implementing them correctly. Also fix up a few other fixes that got missed.
1 parent 63b81c8 commit 0256d22

File tree

2 files changed

+56
-12
lines changed

2 files changed

+56
-12
lines changed

drv/cosmo-hf/src/hf.rs

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,17 @@ impl ServerImpl {
322322
}
323323
}
324324

325-
fn invalidate_hash(&mut self) {
325+
// Write to flash invalidates the hash of current device
326+
fn invalidate_write(&mut self) {
327+
match self.hash.state {
328+
// Only stop the hash for our current device
329+
HashState::Hashing { dev, .. } => {
330+
if dev == self.dev {
331+
self.hash.state = HashState::NotRunning;
332+
}
333+
}
334+
_ => (),
335+
}
326336
match self.dev {
327337
HfDevSelect::Flash0 => {
328338
self.hash.cached_hash0 = SlotHash::Recalculate;
@@ -331,7 +341,13 @@ impl ServerImpl {
331341
self.hash.cached_hash1 = SlotHash::Recalculate;
332342
}
333343
}
344+
}
345+
346+
// We switched our mux, recalculate everything
347+
fn invalidate_mux_switch(&mut self) {
334348
self.hash.state = HashState::NotRunning;
349+
self.hash.cached_hash0 = SlotHash::Recalculate;
350+
self.hash.cached_hash1 = SlotHash::Recalculate;
335351
}
336352

337353
fn set_dev(
@@ -385,7 +401,7 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
385401
return Err(HfError::Sector0IsReserved.into());
386402
}
387403
self.drv.check_flash_mux_state()?;
388-
self.invalidate_hash();
404+
self.invalidate_write();
389405
// Don't use the bulk erase command, because it will erase the entire
390406
// chip. Instead, use the sector erase to erase the currently-active
391407
// virtual device.
@@ -408,7 +424,7 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
408424
self.check_addr_writable(addr, protect)?;
409425
self.drv.check_flash_mux_state()?;
410426
let addr = self.flash_addr(addr, data.len() as u32)?;
411-
self.invalidate_hash();
427+
self.invalidate_write();
412428
self.drv
413429
.flash_write(
414430
addr,
@@ -477,7 +493,7 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
477493
) -> Result<(), RequestError<HfError>> {
478494
self.drv.check_flash_mux_state()?;
479495
self.check_addr_writable(addr, protect)?;
480-
self.invalidate_hash();
496+
self.invalidate_write();
481497
self.drv.flash_sector_erase(self.flash_addr(addr, 0)?);
482498
Ok(())
483499
}
@@ -552,7 +568,7 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
552568
state: HfMuxState,
553569
) -> Result<(), RequestError<HfError>> {
554570
self.drv.set_flash_mux_state(state);
555-
self.invalidate_hash();
571+
self.invalidate_mux_switch();
556572
Ok(())
557573
}
558574

@@ -588,6 +604,16 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
588604
len: u32,
589605
) -> Result<[u8; SHA256_SZ], RequestError<HfError>> {
590606
self.drv.check_flash_mux_state()?;
607+
608+
// Need to check hash state before doing anything else
609+
// that might mess up the hash in progress
610+
match self.hash.state {
611+
HashState::Hashing { .. } => {
612+
return Err(HfError::HashInProgress.into())
613+
}
614+
_ => (),
615+
}
616+
591617
if let Err(e) = self.hash.task.init_sha256() {
592618
ringbuf_entry!(Trace::HashInitError(e));
593619
return Err(HfError::HashError.into());

drv/gimlet-hf-server/src/main.rs

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -414,7 +414,7 @@ impl ServerImpl {
414414
}
415415
// We're about to modify our flash, stop any hash in progress
416416
// and say our old one is invalid.
417-
self.invalidate_hash();
417+
self.invalidate_write();
418418
Ok(())
419419
}
420420

@@ -511,7 +511,17 @@ impl ServerImpl {
511511
}
512512
}
513513

514-
fn invalidate_hash(&mut self) {
514+
// Write to flash invalidates the hash of the single bank
515+
fn invalidate_write(&mut self) {
516+
match self.hash.state {
517+
// Only stop the hash for our current device
518+
HashState::Hashing { dev, .. } => {
519+
if dev == self.dev_state {
520+
self.hash.state = HashState::NotRunning;
521+
}
522+
}
523+
_ => (),
524+
}
515525
match self.dev_state {
516526
HfDevSelect::Flash0 => {
517527
self.hash.cached_hash0 = SlotHash::Recalculate;
@@ -520,7 +530,13 @@ impl ServerImpl {
520530
self.hash.cached_hash1 = SlotHash::Recalculate;
521531
}
522532
}
533+
}
534+
535+
// We switched our mux, recalculate everything
536+
fn invalidate_mux_switch(&mut self) {
523537
self.hash.state = HashState::NotRunning;
538+
self.hash.cached_hash0 = SlotHash::Recalculate;
539+
self.hash.cached_hash1 = SlotHash::Recalculate;
524540
}
525541
}
526542

@@ -719,7 +735,7 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
719735
// This is probably slight over kill but caching is best effort
720736
// and if we're swapping our mux back the host has probably
721737
// gone into A2...
722-
self.invalidate_hash();
738+
self.invalidate_mux_switch();
723739
Ok(())
724740
}
725741

@@ -754,15 +770,15 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
754770
len: u32,
755771
) -> Result<[u8; SHA256_SZ], RequestError<HfError>> {
756772
self.check_muxed_to_sp()?;
757-
if self.hash.task.init_sha256().is_err() {
758-
return Err(HfError::HashError.into());
759-
}
760773
match self.hash.state {
761774
HashState::Hashing { .. } => {
762775
return Err(HfError::HashInProgress.into())
763776
}
764777
_ => (),
765778
}
779+
if self.hash.task.init_sha256().is_err() {
780+
return Err(HfError::HashError.into());
781+
}
766782
let begin = addr as usize;
767783
let end = match begin.checked_add(len as usize) {
768784
Some(end) => {
@@ -804,7 +820,9 @@ impl idl::InOrderHostFlashImpl for ServerImpl {
804820
// Need to check hash state before doing anything else
805821
// that might mess up the hash in progress
806822
match self.hash.state {
807-
HashState::Hashing { .. } => return Err(HfError::HashError.into()),
823+
HashState::Hashing { .. } => {
824+
return Err(HfError::HashInProgress.into())
825+
}
808826
_ => (),
809827
}
810828

0 commit comments

Comments
 (0)