Skip to content

Commit

Permalink
Revert "[opentitantool] Refuse downgrading HyperDebug firmware"
Browse files Browse the repository at this point in the history
This reverts commit 7a0134a, 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 <[email protected]>
  • Loading branch information
jesultra committed Jan 18, 2025
1 parent de9efe7 commit 8c5e761
Showing 1 changed file with 0 additions and 89 deletions.
89 changes: 0 additions & 89 deletions sw/host/opentitanlib/src/transport/hyperdebug/dfu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}
}

Expand Down Expand Up @@ -487,83 +478,3 @@ fn wait_for_idle(dfu_device: &UsbBackend, dfu_interface: u8) -> Result<u8> {
}
}
}

/// 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<bool> {
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::<u64>()?;
let bval = version_b[..bpos].parse::<u64>()?;
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
);
}
}

0 comments on commit 8c5e761

Please sign in to comment.