From 8c5e761db963902ad5846465130b39addb07e933 Mon Sep 17 00:00:00 2001 From: "Jes B. Klinke" Date: Fri, 17 Jan 2025 20:41:41 -0800 Subject: [PATCH] Revert "[opentitantool] Refuse downgrading HyperDebug firmware" This reverts commit 7a0134a9b4d5620435b9750683083662acc7d6ab, such that opentitantool no longer requires the `--force` switch in order to downgrade firmware with `opentitantool transport update-firmware`. Instead, opentitantool will be back to the original behavior of overwriting firmware in any case when the version number does not match exactly. The original reason for not downgrading was that our branches had diverged, such that they had different firmware versions, resulting in CI machines constantly upgrading/downgrading as CI runs from different branches were scheduled. PR #25917, however introduced a faulty HyperDebug firmware with never version number, which is now on a number of CI machines, and will not be downgraded automatically, with the result that "innocent" other PRs will have CI failures. I think it is clear that we should avoid such excessive flashing by keeping the HyperDebug firmware version in sync between branches. (We do not need to keep all of opentitantool logic in sync, just the built-in firmware version.) For now, in order to avoid CI failures, we should merge this PR, and accept some upgrading/downgrading. Change-Id: I57739777ab3d86fa7e0375834cf9140e8e6ac984 Signed-off-by: Jes B. Klinke --- .../src/transport/hyperdebug/dfu.rs | 89 ------------------- 1 file changed, 89 deletions(-) diff --git a/sw/host/opentitanlib/src/transport/hyperdebug/dfu.rs b/sw/host/opentitanlib/src/transport/hyperdebug/dfu.rs index 27ce990d479dc..d988a1c4bd773 100644 --- a/sw/host/opentitanlib/src/transport/hyperdebug/dfu.rs +++ b/sw/host/opentitanlib/src/transport/hyperdebug/dfu.rs @@ -10,7 +10,6 @@ use regex::Regex; use serde_annotate::Annotate; use std::any::Any; use std::cell::RefCell; -use std::cmp::Ordering; use crate::transport::{ Capabilities, Capability, ProgressIndicator, Transport, TransportError, UpdateFirmware, @@ -196,14 +195,6 @@ pub fn update_firmware( ); return Ok(None); } - if is_older_than(new_version, current_version)? { - log::warn!( - "Will not downgrade from {} to {}. Consider --force.", - current_version, - new_version, - ); - return Ok(None); - } } } @@ -487,83 +478,3 @@ fn wait_for_idle(dfu_device: &UsbBackend, dfu_interface: u8) -> Result { } } } - -/// Returns true if the two version strings have the same text prefix, e.g. "hyperdebug_", -/// differing only in the subsequent numbers, and furthermore that the numbers in `version_a` are -/// strictly "less than" those in `version_b`. -fn is_older_than(version_a: &str, version_b: &str) -> Result { - let apos = version_a.find(char::is_numeric).unwrap_or(version_a.len()); - let bpos = version_b.find(char::is_numeric).unwrap_or(version_b.len()); - if version_a[..apos] != version_b[..bpos] { - return Ok(false); - } - let version_a = &version_a[apos..]; - let version_b = &version_b[apos..]; - if version_a.is_empty() || version_b.is_empty() { - return Ok(false); - } - let apos = version_a - .find(|ch: char| !char::is_numeric(ch)) - .unwrap_or(version_a.len()); - let bpos = version_b - .find(|ch: char| !char::is_numeric(ch)) - .unwrap_or(version_b.len()); - let aval = version_a[..apos].parse::()?; - let bval = version_b[..bpos].parse::()?; - match aval.cmp(&bval) { - Ordering::Less => Ok(true), - Ordering::Greater => Ok(false), - // Exact match so far, recursively inspect any further numbers in the string. - Ordering::Equal => is_older_than(&version_a[apos..], &version_b[apos..]), - } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_is_older_than() { - // Ordering of dates, with fallback to sequence suffix. - assert_eq!( - is_older_than("hyp_20240101_99", "hyp_20240801_01").unwrap(), - true - ); - assert_eq!( - is_older_than("hyp_20240801_01", "hyp_20240101_99").unwrap(), - false - ); - assert_eq!( - is_older_than("hyp_20240101_01", "hyp_20240101_02").unwrap(), - true - ); - assert_eq!( - is_older_than("hyp_20240101_02", "hyp_20240101_01").unwrap(), - false - ); - assert_eq!( - is_older_than("hyp_20240101_01", "hyp_20240101_01").unwrap(), - false - ); - - // Lexicographical ordering of version string. - assert_eq!(is_older_than("fancy_1.2.5", "fancy_1.11.1").unwrap(), true); - assert_eq!(is_older_than("fancy_1.11.1", "fancy_1.2.5").unwrap(), false); - assert_eq!(is_older_than("fancy_1.2.2", "fancy_1.2.11").unwrap(), true); - assert_eq!(is_older_than("fancy_1.2.11", "fancy_1.2.2").unwrap(), false); - assert_eq!( - is_older_than("fancy_1.2.11", "fancy_1.2.11").unwrap(), - false - ); - - // Not comparable, neither is considered "older" than the other. - assert_eq!( - is_older_than("fancy_1.2.5", "hyperdebug_20240101_02").unwrap(), - false - ); - assert_eq!( - is_older_than("hyperdebug_20240101_02", "fancy_1.2.5").unwrap(), - false - ); - } -}