From 466f65efb874aeed135757741d670d6db9e7fb8c Mon Sep 17 00:00:00 2001 From: Laura Abbott Date: Mon, 27 Apr 2026 09:13:24 -0400 Subject: [PATCH 1/4] Remove gdb support Our gdb support has gotten spotty over the years. Remove built in humility support. --- Cargo.lock | 14 -- Cargo.toml | 2 - README.md | 17 -- cmd/gdb/Cargo.toml | 16 -- cmd/gdb/src/lib.rs | 228 --------------------- humility-bin/Cargo.toml | 1 - humility-core/src/hubris.rs | 52 ----- humility-probes-core/src/gdb.rs | 346 -------------------------------- humility-probes-core/src/lib.rs | 20 +- 9 files changed, 1 insertion(+), 695 deletions(-) delete mode 100644 cmd/gdb/Cargo.toml delete mode 100644 cmd/gdb/src/lib.rs delete mode 100644 humility-probes-core/src/gdb.rs diff --git a/Cargo.lock b/Cargo.lock index fc43c5efb..22b87a18b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1590,7 +1590,6 @@ dependencies = [ "humility-cmd-exec", "humility-cmd-extract", "humility-cmd-flash", - "humility-cmd-gdb", "humility-cmd-gimlet", "humility-cmd-gpio", "humility-cmd-hash", @@ -1890,19 +1889,6 @@ dependencies = [ "tempfile", ] -[[package]] -name = "humility-cmd-gdb" -version = "0.1.0" -dependencies = [ - "anyhow", - "clap", - "ctrlc", - "humility-cli", - "humility-cmd", - "humility-core", - "tempfile", -] - [[package]] name = "humility-cmd-gimlet" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index a5da2a938..b625ff8b8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,7 +37,6 @@ members = [ "cmd/exec", "cmd/extract", "cmd/flash", - "cmd/gdb", "cmd/gimlet", "cmd/gpio", "cmd/hash", @@ -144,7 +143,6 @@ cmd-ereport = { path = "./cmd/ereport", package = "humility-cmd-ereport" } cmd-exec = { path = "./cmd/exec", package = "humility-cmd-exec" } cmd-extract = { path = "./cmd/extract", package = "humility-cmd-extract" } cmd-flash = { path = "./cmd/flash", package = "humility-cmd-flash" } -cmd-gdb = { path = "./cmd/gdb", package = "humility-cmd-gdb" } cmd-gimlet = { path = "./cmd/gimlet", package = "humility-cmd-gimlet" } cmd-gpio = { path = "./cmd/gpio", package = "humility-cmd-gpio" } cmd-hash = { path = "./cmd/hash", package = "humility-cmd-hash" } diff --git a/README.md b/README.md index f2e43e6be..568ace3b9 100644 --- a/README.md +++ b/README.md @@ -245,7 +245,6 @@ a specified target. (In the above example, one could execute `humility - [humility exec](#humility-exec): execute command within context of an environment - [humility extract](#humility-extract): extract all or part of a Hubris archive - [humility flash](#humility-flash): flash archive onto attached device -- [humility gdb](#humility-gdb): Attach to a running system using GDB - [humility gimlet](#humility-gimlet): Gimlet-specific diagnostic commands - [humility gpio](#humility-gpio): GPIO pin manipulation - [humility hash](#humility-hash): Access to the HASH block @@ -834,22 +833,6 @@ will be programmed after the image is written. See RFD 311 for more information about auxiliary flash management. -### `humility gdb` - -This command launches GDB and attaches to a running device. - -By default, the user must be running `openocd` or `pyocd` in a separate -terminal. - -The `--run-openocd` option automatically launches `openocd` based on the -`openocd.cfg` file included in the build archive. - -When using `pyocd`, it must be launched with the `--persist` option, -because `humility gdb` connects to it multiple times (once to check the -app id, then again to run the console). - - - ### `humility gimlet` `humility gimlet` contacts a (recent firmware version) Gimlet over the diff --git a/cmd/gdb/Cargo.toml b/cmd/gdb/Cargo.toml deleted file mode 100644 index bd631ae8e..000000000 --- a/cmd/gdb/Cargo.toml +++ /dev/null @@ -1,16 +0,0 @@ -[package] -name = "humility-cmd-gdb" -version = "0.1.0" -edition.workspace = true -description = "Attach to a running system using GDB" - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - -[dependencies] -humility = { workspace = true } -humility-cmd = { workspace = true } -humility-cli = { workspace = true } -anyhow = { workspace = true } -clap = { workspace = true } -ctrlc = { workspace = true } -tempfile = { workspace = true } diff --git a/cmd/gdb/src/lib.rs b/cmd/gdb/src/lib.rs deleted file mode 100644 index 1c6605b58..000000000 --- a/cmd/gdb/src/lib.rs +++ /dev/null @@ -1,228 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -//! ## `humility gdb` -//! -//! This command launches GDB and attaches to a running device. -//! -//! By default, the user must be running `openocd` or `pyocd` in a separate -//! terminal. -//! -//! The `--run-openocd` option automatically launches `openocd` based on the -//! `openocd.cfg` file included in the build archive. -//! -//! When using `pyocd`, it must be launched with the `--persist` option, -//! because `humility gdb` connects to it multiple times (once to check the -//! app id, then again to run the console). -//! - -use std::process::{Command, Stdio}; - -use humility_cli::ExecutionContext; -use humility_cmd::{Archive, Command as HumilityCmd, CommandKind}; - -use anyhow::{Context, Result, bail}; -use clap::{CommandFactory, Parser}; - -#[derive(Parser, Debug)] -#[clap( - name = "gdb", about = env!("CARGO_PKG_DESCRIPTION"), -)] -struct GdbArgs { - /// when set, calls `load` and `stepi` upon attaching - #[clap(long, short)] - load: bool, - - /// when set, runs an OpenOCD process before starting GDB - #[clap(long, group = "run_openocd_group")] - run_openocd: bool, - - /// specifies the `openocd` executable to run - #[clap(long, requires = "run_openocd_group")] - openocd: Option, - - /// specifies the probe serial number to use with OpenOCD - #[clap(long, requires = "run_openocd_group")] - serial: Option, - - #[clap(long)] - /// Path to gdb script to run - gdb_script: Option, -} - -fn gdb(context: &mut ExecutionContext) -> Result<()> { - let hubris = context.archive.as_ref().unwrap(); - - if context.cli.probe.is_some() { - bail!("Cannot specify --probe with `gdb` subcommand"); - } - - let subargs = GdbArgs::try_parse_from(&context.cli.cmd)?; - let serial = context.cli.get_probe_serial(subargs.serial.as_deref())?; - - let work_dir = tempfile::tempdir()?; - let name = match &hubris.manifest.name { - Some(name) => name, - None => bail!("Could not get app name from manifest"), - }; - let elf_dir = work_dir.path().join("target").join(name).join("dist"); - std::fs::create_dir_all(&elf_dir)?; - hubris.extract_elfs_to(&elf_dir)?; - - hubris - .extract_file_to( - "debug/openocd.gdb", - &work_dir.path().join("openocd.gdb"), - ) - .context("GDB config missing. Is your Hubris build too old?")?; - - let gdb_script_path = work_dir.path().join("script.gdb"); - // We moved the gdb file out of the hubris archive because it was - // preventing reproducible builds. An extraction is non fatal - let r = hubris.extract_file_to("debug/script.gdb", &gdb_script_path); - - if let Some(path) = subargs.gdb_script { - // It's possible we overwrite the script in the old archive but - // that seems like behavior someone would want when actively - // passing in a path; print a warning to that effect - if r.is_ok() { - humility::warn!("--gdb_script is overriding GDB script in archive"); - } - std::fs::copy(path, &gdb_script_path)?; - } - - if !gdb_script_path.exists() { - bail!( - "GDB script missing; recent archives don't include it. Pass --gdb-script or use xtask gdb" - ); - } - - hubris - .extract_file_to("img/final.elf", &work_dir.path().join("final.elf"))?; - - let mut gdb_cmd = None; - - const GDB_NAMES: [&str; 2] = ["arm-none-eabi-gdb", "gdb-multiarch"]; - for candidate in &GDB_NAMES { - if Command::new(candidate) - .arg("--version") - .stdout(Stdio::piped()) - .status() - .is_ok() - { - gdb_cmd = Some(candidate); - break; - } - } - - // Select the GDB command - let gdb_cmd = gdb_cmd.ok_or_else(|| { - anyhow::anyhow!("GDB not found. Tried: {:?}", GDB_NAMES) - })?; - - // If OpenOCD is requested, then run it in a subprocess here, with an RAII - // handle to ensure that it's killed before the program exits. - struct OpenOcdRunner(std::process::Child); - impl Drop for OpenOcdRunner { - fn drop(&mut self) { - self.0.kill().expect("Could not kill `openocd`") - } - } - let _openocd = if subargs.run_openocd { - hubris - .extract_file_to( - "debug/openocd.cfg", - &work_dir.path().join("openocd.cfg"), - ) - .context("openocd config missing. Is your Hubris build too old?")?; - let mut cmd = Command::new( - subargs.openocd.unwrap_or_else(|| "openocd".to_string()), - ); - cmd.arg("-f").arg("openocd.cfg"); - if let Some(serial) = serial { - cmd.arg("-c") - .arg("interface hla") - .arg("-c") - .arg(format!("hla_serial {}", serial)); - } - cmd.current_dir(work_dir.path()); - cmd.stdin(Stdio::piped()); - Some(OpenOcdRunner(cmd.spawn().context("Could not start `openocd`")?)) - } else { - None - }; - - // Alright, here's where it gets awkward. We are either - // - Running OpenOCD, launched by the block above - // - Running OpenOCD in a separate terminal, through humility openocd - // or manually - // - Running PyOCD in a separate terminal - // - // If we aren't loading new firmware, then we want to check that the - // running firmware matches the image. However, we can't use - // humility_cmd::attach like normal, because that's not compatible with - // PyOCD (which doesn't expose the TCL port). - // - // Instead, we fall back to the one interface that we _know_ these three - // cases have in common: GDB. Also, GDB is _terrible_: `print/x` doesn't - // work if we're attached to the target and have all of our sections - // loaded. - if !subargs.load { - let mut cmd = Command::new(gdb_cmd); - let image_id_addr = hubris.image_id_addr().unwrap(); - let image_id = hubris.image_id().unwrap(); - cmd.arg("-q") - .arg("-x") - .arg("openocd.gdb") - .arg("-ex") - .arg(format!( - "dump binary memory image_id {} {}", - image_id_addr, - image_id_addr as usize + image_id.len(), - )) - .arg("-ex") - .arg("set confirm off") - .arg("-ex") - .arg("quit"); - cmd.current_dir(work_dir.path()); - let status = cmd.status()?; - if !status.success() { - anyhow::bail!("could not get image_id, see output for details"); - } - let image_id_actual = std::fs::read(work_dir.path().join("image_id"))?; - if image_id_actual != image_id { - bail!( - "Invalid image ID: expected {:?}, got {:?}", - image_id, - image_id_actual - ); - } - } - - let mut cmd = Command::new(gdb_cmd); - cmd.arg("-q").arg("-x").arg("script.gdb").arg("-x").arg("openocd.gdb"); - if subargs.load { - // start the process but immediately halt the processor - cmd.arg("-ex").arg("load").arg("-ex").arg("stepi"); - } - cmd.arg("final.elf"); - cmd.current_dir(work_dir.path()); - - // Run GDB, ignoring Ctrl-C (so it can handle them) - ctrlc::set_handler(|| {}).expect("Error setting Ctrl-C handler"); - let status = cmd.status()?; - if !status.success() { - anyhow::bail!("command failed, see output for details"); - } - Ok(()) -} - -pub fn init() -> HumilityCmd { - HumilityCmd { - app: GdbArgs::command(), - name: "gdb", - run: gdb, - kind: CommandKind::Unattached { archive: Archive::Required }, - } -} diff --git a/humility-bin/Cargo.toml b/humility-bin/Cargo.toml index 5bae0090c..5848b9201 100644 --- a/humility-bin/Cargo.toml +++ b/humility-bin/Cargo.toml @@ -48,7 +48,6 @@ cmd-ereport = { workspace = true } cmd-exec = { workspace = true } cmd-extract = { workspace = true } cmd-flash = { workspace = true, optional = true } -cmd-gdb = { workspace = true } cmd-gimlet = { workspace = true } cmd-gpio = { workspace = true } cmd-hash = { workspace = true } diff --git a/humility-core/src/hubris.rs b/humility-core/src/hubris.rs index 77cea5251..9a6da3842 100644 --- a/humility-core/src/hubris.rs +++ b/humility-core/src/hubris.rs @@ -1509,41 +1509,6 @@ impl HubrisArchive { Ok(()) } - fn for_each_task Result<()>>( - archive: &mut zip::ZipArchive>, - mut f: F, - ) -> Result<()> { - use rayon::prelude::*; - let files = (0..archive.len()) - .into_par_iter() - .map(|i| { - // ZipArchive is cheap to clone since the backing is cheap - let mut archive = archive.clone(); - let mut file = archive.by_index(i)?; - let path = Path::new(file.name()); - let pieces = path.iter().collect::>(); - - // - // If the second-to-last element of our path is "task", we have - // a winner! - // - if pieces.len() < 2 || pieces[pieces.len() - 2] != "task" { - return Ok(None); - } - - let mut buffer = Vec::new(); - file.read_to_end(&mut buffer)?; - Ok(Some((file.name().to_owned(), buffer))) - }) - .filter_map(|f| f.transpose()) - .collect::)>, anyhow::Error>>()?; - - for (file_name, buffer) in files { - f(Path::new(&file_name), &buffer)?; - } - Ok(()) - } - fn merge(&mut self, loader: HubrisObjectLoader) -> Result<()> { if loader.imageid.is_some() { self.imageid = loader.imageid; @@ -3661,23 +3626,6 @@ impl HubrisArchive { std::fs::write(target, &buffer).map_err(Into::into) } - /// Copies the kernel and every task ELF file to the given directory. - pub fn extract_elfs_to(&self, p: &Path) -> Result<()> { - self.extract_file_to("elf/kernel", &p.join("kernel"))?; - - // `stage0` is only present in some cases, so we ignore errors here - let _ = self.extract_file_to("img/stage0", &p.join("stage0")); - - let cursor = Cursor::new(self.archive.as_slice()); - let mut archive = zip::ZipArchive::new(cursor)?; - Self::for_each_task(&mut archive, |path, buffer| { - let file_name = p.join(path.file_name().unwrap()); - std::fs::write(file_name, buffer)?; - Ok(()) - })?; - Ok(()) - } - pub fn lookup_feature(&self, feature: &str) -> Result> { let mut rval = vec![]; diff --git a/humility-probes-core/src/gdb.rs b/humility-probes-core/src/gdb.rs deleted file mode 100644 index 41bd8022e..000000000 --- a/humility-probes-core/src/gdb.rs +++ /dev/null @@ -1,346 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -use anyhow::{Result, anyhow, bail}; -use humility::core::Core; -use humility_arch_arm::ARMRegister; -use std::fmt; -use std::io::{Read, Write}; -use std::net::TcpStream; -use std::path::Path; -use std::time::Duration; - -#[derive(Copy, Clone, Debug, PartialEq)] -pub(crate) enum GDBServer { - OpenOCD, - JLink, -} - -impl fmt::Display for GDBServer { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!( - f, - "{}", - match self { - GDBServer::OpenOCD => "OpenOCD", - GDBServer::JLink => "JLink", - } - ) - } -} - -pub struct GDBCore { - stream: TcpStream, - server: GDBServer, - halted: bool, -} - -const GDB_PACKET_START: char = '$'; -const GDB_PACKET_END: char = '#'; -const GDB_PACKET_ACK: char = '+'; -const GDB_PACKET_HALT: u8 = 3; - -#[rustfmt::skip::macros(anyhow, bail)] -impl GDBCore { - fn prepcmd(&mut self, cmd: &str) -> Vec { - let mut payload = vec![GDB_PACKET_START as u8]; - - let mut cksum = 0; - - for b in cmd.as_bytes() { - payload.push(*b); - cksum += *b as u32; - } - - // - // Tack on the goofy checksum beyond the end of the packet. - // - let trailer = &format!("{}{:02x}", GDB_PACKET_END, cksum % 256); - - for b in trailer.as_bytes() { - payload.push(*b); - } - - log::trace!("sending {}", std::str::from_utf8(&payload).unwrap()); - payload - } - - fn firecmd(&mut self, cmd: &str) -> Result<()> { - let mut rbuf = vec![0; 1024]; - let payload = self.prepcmd(cmd); - - self.stream.write_all(&payload)?; - - // - // We are expecting no result -- just an ack. - // - let rval = self.stream.read(&mut rbuf)?; - - if rval != 1 { - bail!("cmd {} returned {} bytes: {:?}", cmd, rval, - std::str::from_utf8(&rbuf)); - } - - if rbuf[0] != GDB_PACKET_ACK as u8 { - bail!("cmd {} incorrectly ack'd: {:?}", cmd, rbuf); - } - - Ok(()) - } - - fn sendack(&mut self) -> Result<()> { - self.stream.write_all(&[GDB_PACKET_ACK as u8])?; - Ok(()) - } - - fn recv(&mut self, expectack: bool) -> Result { - let mut rbuf = vec![0; 1024]; - let mut result = String::new(); - - loop { - let rval = self.stream.read(&mut rbuf)?; - - result.push_str(std::str::from_utf8(&rbuf[0..rval])?); - log::trace!("response: {}", result); - - // - // We are done when we have our closing delimter followed by - // the two byte checksum. - // - if result.find(GDB_PACKET_END) == Some(result.len() - 3) { - break; - } - } - - // - // We have our response, so ack it back - // - self.sendack()?; - - // - // In our result, we should have exactly one opening and exactly - // one closing delimiter -- and, if expectack is set, at least - // one ACK as well. - // - let start = match result.find(GDB_PACKET_START) { - Some(ndx) => ndx, - None => { - bail!("missing start of packet: \"{}\"", result); - } - }; - - // - // By merits of being here, we know we have our end-of-packet... - // - let end = result.find(GDB_PACKET_END).unwrap(); - - if end < start { - bail!("start/end inverted: \"{}\"", result); - } - - match result.find(GDB_PACKET_ACK) { - Some(ack) => { - if expectack && ack > start { - bail!("found response but no ack: \"{}\"", result); - } - - if !expectack && ack < start { - bail!("found spurious ack: \"{}\"", result); - } - } - - None => { - if expectack { - bail!("did not find expected ack: \"{}\"", result); - } - } - } - - Ok(result[start + 1..end].to_string()) - } - - fn sendcmd(&mut self, cmd: &str) -> Result { - let payload = self.prepcmd(cmd); - self.stream.write_all(&payload)?; - self.recv(true) - } - - fn send_32(&mut self, cmd: &str) -> Result { - let rstr = self.sendcmd(cmd)?; - let mut buf: Vec = vec![]; - - for i in (0..rstr.len()).step_by(2) { - buf.push(u8::from_str_radix(&rstr[i..=i + 1], 16)?); - } - - log::trace!("command {} returned {}", cmd, rstr); - - match rstr.len() { - 2 => Ok(u32::from(u8::from_le_bytes(buf[..].try_into().unwrap()))), - 4 => Ok(u32::from(u16::from_le_bytes(buf[..].try_into().unwrap()))), - 8 => Ok(u32::from_le_bytes(buf[..].try_into().unwrap())), - 16 => { - // - // Amazingly, for some 32-bit register values under certain - // circumstances the JLink seems to return a 64-bit value - // (!). We confirm that this value is - // representable and return it. - // - let val = u64::from_le_bytes(buf[..].try_into().unwrap()); - - if val > u32::MAX.into() { - Err(anyhow!("bad 64-bit return on cmd {}: {}", cmd, rstr)) - } else { - Ok(val as u32) - } - } - _ => Err(anyhow!("bad return on cmd {}: {}", cmd, rstr)), - } - } - - pub(crate) fn new(server: GDBServer) -> Result { - let port = match server { - GDBServer::OpenOCD => 3333, - GDBServer::JLink => 2331, - }; - - let host = format!("127.0.0.1:{}", port); - let addr = host.parse()?; - let timeout = Duration::from_millis(100); - - let stream = - TcpStream::connect_timeout(&addr, timeout).map_err(|_| { - anyhow!( - "can't connect to {} GDB server on \ - port {}; is it running?", - server, port - ) - })?; - - // - // Both the OpenOCD and JLink GDB servers stop the target upon - // connection. This is helpful in that we know the state that - // we're in -- but it's also not the state that we want to be - // in. We explicitly run the target before returning. - // - let mut core = Self { stream, server, halted: true }; - - let supported = core.sendcmd("qSupported")?; - log::trace!("{} supported string: {}", server, supported); - - core.run()?; - - Ok(core) - } -} - -#[rustfmt::skip::macros(anyhow, bail)] -impl Core for GDBCore { - fn info(&self) -> (String, Option) { - ("GDB".to_string(), None) - } - - fn read_word_32(&mut self, addr: u32) -> Result { - self.send_32(&format!("m{:x},4", addr)) - } - - fn read_8(&mut self, addr: u32, data: &mut [u8]) -> Result<()> { - let cmd = format!("m{:x},{:x}", addr, data.len()); - - let rstr = self.sendcmd(&cmd)?; - - if rstr.len() > data.len() * 2 { - bail!("bad read_8 on cmd {} \ - (expected {}, found {}): {}", - cmd, data.len() * 2, rstr.len(), rstr); - } - - for (idx, i) in (0..rstr.len()).step_by(2).enumerate() { - data[idx] = u8::from_str_radix(&rstr[i..=i + 1], 16)?; - } - - Ok(()) - } - - fn read_reg(&mut self, reg: ARMRegister) -> Result { - use num_traits::ToPrimitive; - let cmd = &format!("p{:02X}", ARMRegister::to_u16(®).unwrap()); - - let rval = self.send_32(cmd); - - if self.server == GDBServer::JLink { - // - // Maddeningly, the JLink stops the target whenever a register - // is read. - // - self.firecmd("c")?; - } - - rval - } - - fn write_reg(&mut self, _reg: ARMRegister, _value: u32) -> Result<()> { - Err(anyhow!( - "{} GDB target does not support modifying state", self.server - )) - } - - fn write_word_32(&mut self, _addr: u32, _data: u32) -> Result<()> { - Err(anyhow!( - "{} GDB target does not support modifying state", self.server - )) - } - - fn write_8(&mut self, _addr: u32, _data: &[u8]) -> Result<()> { - Err(anyhow!( - "{} GDB target does not support modifying state", self.server - )) - } - - fn halt(&mut self) -> Result<()> { - self.stream.write_all(&[GDB_PACKET_HALT])?; - - let reply = self.recv(false)?; - log::trace!("halt reply: {}", reply); - self.halted = true; - - Ok(()) - } - - fn run(&mut self) -> Result<()> { - // - // The OpenOCD target in particular loses its mind if told to - // continue to when it's already running, insisting on - // sending a reply with an elaborate message that we don't - // know to wait on -- so we only continue a target if we know - // it to be halted. - // - if self.halted { - self.firecmd("c")?; - self.halted = false; - } - - Ok(()) - } - - fn step(&mut self) -> Result<()> { - Ok(()) - } - - fn load(&mut self, _path: &Path) -> Result<()> { - bail!("Flash loading is not supported with GDB"); - } - - fn reset(&mut self) -> Result<()> { - bail!("Reset is not supported with GDB"); - } - - fn reset_and_halt(&mut self, _dur: std::time::Duration) -> Result<()> { - bail!("Reset is not supported with OpenOCD"); - } - - fn wait_for_halt(&mut self, _dur: std::time::Duration) -> Result<()> { - bail!("Wait for halt is not supported with GDB"); - } -} diff --git a/humility-probes-core/src/lib.rs b/humility-probes-core/src/lib.rs index 74c856ae7..c72d28b0b 100644 --- a/humility-probes-core/src/lib.rs +++ b/humility-probes-core/src/lib.rs @@ -12,7 +12,6 @@ use anyhow::{Result, anyhow, bail}; use humility::hubris::HubrisArchive; use humility::msg; -mod gdb; mod openocd; mod probe_rs; mod unattached; @@ -131,7 +130,7 @@ pub fn attach_to_probe( probe_info.serial_number, ))) } - "ocd" | "ocdgdb" | "jlink" => { + "ocd" => { bail!("Probe only attachment with {} is not supported", probe) } "auto" => attach_to_probe("usb", speed_khz), @@ -211,26 +210,9 @@ pub fn attach_to_chip( return Ok(probe); } - if let Ok(probe) = attach_to_chip("jlink", chip, speed_khz) { - return Ok(probe); - } - attach_to_chip("usb", chip, speed_khz) } - "ocdgdb" => { - let core = gdb::GDBCore::new(gdb::GDBServer::OpenOCD)?; - crate::msg!("attached via OpenOCD's GDB server"); - - Ok(Box::new(core)) - } - - "jlink" => { - let core = gdb::GDBCore::new(gdb::GDBServer::JLink)?; - crate::msg!("attached via JLink"); - - Ok(Box::new(core)) - } _ => match TryInto::::try_into(probe) { Ok(selector) => { let vidpid = probe; From db01794c18c26315bb4256528891939213859558 Mon Sep 17 00:00:00 2001 From: Laura Abbott Date: Wed, 29 Apr 2026 15:37:33 -0400 Subject: [PATCH 2/4] Drop openocd support from flashing We haven't really used this in a long time and there is a good chance it's probably broken. --- Cargo.lock | 76 +-------------- Cargo.toml | 2 - README.md | 23 ----- cmd/flash/src/lib.rs | 204 +--------------------------------------- cmd/openocd/Cargo.toml | 13 --- cmd/openocd/src/lib.rs | 98 ------------------- humility-bin/Cargo.toml | 1 - 7 files changed, 5 insertions(+), 412 deletions(-) delete mode 100644 cmd/openocd/Cargo.toml delete mode 100644 cmd/openocd/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 22b87a18b..b88b16c0e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -307,15 +307,6 @@ dependencies = [ "zeroize", ] -[[package]] -name = "block2" -version = "0.6.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cdeb9d870516001442e364c5220d3574d2da8dc765554b4a617230d33fa58ef5" -dependencies = [ - "objc2", -] - [[package]] name = "bumpalo" version = "3.20.2" @@ -911,17 +902,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "ctrlc" -version = "3.5.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e0b1fab2ae45819af2d0731d60f2afe17227ebb1a1538a236da84c93e9a60162" -dependencies = [ - "dispatch2", - "nix 0.31.2", - "windows-sys 0.61.2", -] - [[package]] name = "ctutils" version = "0.4.2" @@ -1080,18 +1060,6 @@ dependencies = [ "zeroize", ] -[[package]] -name = "dispatch2" -version = "0.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e0e367e4e7da84520dedcac1901e4da967309406d1e51017ae1abfb97adbd38" -dependencies = [ - "bitflags 2.11.1", - "block2", - "libc", - "objc2", -] - [[package]] name = "document-features" version = "0.2.12" @@ -1606,7 +1574,6 @@ dependencies = [ "humility-cmd-monorail", "humility-cmd-mwocp", "humility-cmd-net", - "humility-cmd-openocd", "humility-cmd-pmbus", "humility-cmd-power", "humility-cmd-powershelf", @@ -2150,18 +2117,6 @@ dependencies = [ "parse_int", ] -[[package]] -name = "humility-cmd-openocd" -version = "0.1.0" -dependencies = [ - "anyhow", - "clap", - "ctrlc", - "humility-cli", - "humility-cmd", - "tempfile", -] - [[package]] name = "humility-cmd-pmbus" version = "0.1.0" @@ -3243,7 +3198,7 @@ version = "1.1.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c0aeb26bf5e836cc1c341c8106051b573f1766dfa05aa87f0b98be5e51b02303" dependencies = [ - "nix 0.29.0", + "nix", "winapi", ] @@ -3362,18 +3317,6 @@ dependencies = [ "memoffset", ] -[[package]] -name = "nix" -version = "0.31.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5d6d0705320c1e6ba1d912b5e37cf18071b6c2e9b7fa8215a1e8a7651966f5d3" -dependencies = [ - "bitflags 2.11.1", - "cfg-if", - "cfg_aliases", - "libc", -] - [[package]] name = "nom" version = "7.1.3" @@ -3436,21 +3379,6 @@ dependencies = [ "libc", ] -[[package]] -name = "objc2" -version = "0.6.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3a12a8ed07aefc768292f076dc3ac8c48f3781c8f2d5851dd3d98950e8c5a89f" -dependencies = [ - "objc2-encode", -] - -[[package]] -name = "objc2-encode" -version = "4.1.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ef25abbcd74fb2609453eb695bd2f860d389e457f67dc17cafc8b8cbc89d0c33" - [[package]] name = "object" version = "0.27.1" @@ -4729,7 +4657,7 @@ dependencies = [ "libc", "log", "memmem", - "nix 0.29.0", + "nix", "num-derive 0.4.2", "num-traits", "ordered-float", diff --git a/Cargo.toml b/Cargo.toml index b625ff8b8..41030bdbd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,7 +53,6 @@ members = [ "cmd/monorail", "cmd/mwocp", "cmd/net", - "cmd/openocd", "cmd/pmbus", "cmd/power", "cmd/probe", @@ -159,7 +158,6 @@ cmd-map = { path = "./cmd/map", package = "humility-cmd-map" } cmd-monorail = { path = "./cmd/monorail", package = "humility-cmd-monorail" } cmd-mwocp = { path = "./cmd/mwocp", package = "humility-cmd-mwocp" } cmd-net = { path = "./cmd/net", package = "humility-cmd-net" } -cmd-openocd = { path = "./cmd/openocd", package = "humility-cmd-openocd" } cmd-pmbus = { path = "./cmd/pmbus", package = "humility-cmd-pmbus" } cmd-power = { path = "./cmd/power", package = "humility-cmd-power" } cmd-powershelf = { path = "./cmd/powershelf", package = "humility-cmd-powershelf" } diff --git a/README.md b/README.md index 568ace3b9..074b5d845 100644 --- a/README.md +++ b/README.md @@ -261,7 +261,6 @@ a specified target. (In the above example, one could execute `humility - [humility monorail](#humility-monorail): Management network control and debugging - [humility mwocp](#humility-mwocp): Murata power shelf operations - [humility net](#humility-net): Management network device-side control and debugging -- [humility openocd](#humility-openocd): Run OpenOCD for the given archive - [humility pmbus](#humility-pmbus): scan for and read PMBus devices - [humility power](#humility-power): show power-related information - [humility powershelf](#humility-powershelf): inspect powershelf over the management network @@ -814,19 +813,6 @@ by specifying `-V` (`--verify`). Similarly, if one wishes to *only* check the image against the archive (and not flash at all), specify `-C` (`--check`). -This attempts to natively flash the part within Humility using probe-rs, -but for some parts or configurations, it may need to use OpenOCD as a -child process to flash it. If OpenOCD is required but not installed (or -isn't in the path), this will fail. If OpenOCD is used, temporary files -are created as part of this process; if they are to be retained, the `-R` -(`--retain-temporaries`) flag should be set. To see what would be -executed without actually executing any commands, use the `-n` -(`--dry-run`) flag. Should use of OpenOCD need to be forced (that is, -should probe-rs flashing fail), the `-O` (`--force-openocd`) flag can be -used. That said, OpenOCD should generally be discouraged; the disposition -is to extend probe-rs to support any parts that must be flashed via -OpenOCD. - If the specified archive includes auxiliary flash data and the new image includes a task with the `AuxFlash` API, two slots of auxiliary flash will be programmed after the image is written. See RFD 311 for more @@ -1830,15 +1816,6 @@ addition, the MAC-side counters are only supported on the VSC8562, not the VSC8552; this is indicated with `--` in the relevant table positions. -### `humility openocd` - -This command launches OpenOCD based on the config file in a build archive, -which then allows one to connect with either GDB or directly via telnet. -If the intention is to only run GDB, note that `humility gdb --run-openocd` -will both run OpenOCD and run a foreground GDB that is connected to it. - - - ### `humility pmbus` Operates on PMBus devices in the system. To list all PMBus devices, use diff --git a/cmd/flash/src/lib.rs b/cmd/flash/src/lib.rs index b079ddbcb..4eb6902a8 100644 --- a/cmd/flash/src/lib.rs +++ b/cmd/flash/src/lib.rs @@ -14,19 +14,6 @@ //! check the image against the archive (and not flash at all), specify //! `-C` (`--check`). //! -//! This attempts to natively flash the part within Humility using probe-rs, -//! but for some parts or configurations, it may need to use OpenOCD as a -//! child process to flash it. If OpenOCD is required but not installed (or -//! isn't in the path), this will fail. If OpenOCD is used, temporary files -//! are created as part of this process; if they are to be retained, the `-R` -//! (`--retain-temporaries`) flag should be set. To see what would be -//! executed without actually executing any commands, use the `-n` -//! (`--dry-run`) flag. Should use of OpenOCD need to be forced (that is, -//! should probe-rs flashing fail), the `-O` (`--force-openocd`) flag can be -//! used. That said, OpenOCD should generally be discouraged; the disposition -//! is to extend probe-rs to support any parts that must be flashed via -//! OpenOCD. -//! //! If the specified archive includes auxiliary flash data and the new image //! includes a task with the `AuxFlash` API, two slots of auxiliary flash //! will be programmed after the image is written. See RFD 311 for more @@ -36,11 +23,8 @@ use anyhow::{Context, Result, anyhow, bail}; use clap::{CommandFactory, Parser}; use humility::{core::Core, hubris::*}; use humility_auxflash::AuxFlashHandler; -use humility_cli::{Cli, ExecutionContext}; +use humility_cli::ExecutionContext; use humility_cmd::{Archive, Command, CommandKind}; -use path_slash::PathExt; -use std::io::Write; -use std::process::ExitStatus; #[derive(Parser, Debug)] #[clap(name = "flash", about = env!("CARGO_PKG_DESCRIPTION"))] @@ -83,146 +67,6 @@ struct FlashArgs { verbose: bool, } -fn force_openocd( - hubris: &mut HubrisArchive, - args: &Cli, - subargs: &FlashArgs, - config: &HubrisFlashMeta, - elf: &[u8], -) -> Result<()> { - // Images that include auxiliary flash data *must* be programmed through - // probe-rs, because we use the resulting ProbeCore to program the - // auxiliary flash (via hiffy) - if hubris.read_auxflash_data()?.is_some() { - bail!("cannot program an image with auxiliary flash through OpenOCD"); - } - - // - // We need to attach to (1) confirm that we're plugged into something - // and (2) extract serial information. - // - let probe = match &args.probe { - Some(p) => p, - None => "auto", - }; - - let serial = { - let mut c = humility_probes_core::attach(probe, hubris, args.speed)?; - let core = c.as_mut(); - - validate(hubris, core, subargs)?; - if subargs.check { - return Ok(()); - } - - core.run()?; - core.info().1 - }; - - let dryrun = |cmd: &std::process::Command| { - humility::msg!("would execute: {cmd:?}"); - }; - - let payload = match &config.program { - Some(FlashProgram::OpenOcd(payload)) => payload, - Some(other) => { - bail!( - "cannot force OpenOCD for non-OpenOCD \ - flash configuration: {other:?}", - ); - } - None => { - bail!( - "cannot force OpenOCD, this archive was \ - built after support was removed" - ); - } - }; - - let mut flash = std::process::Command::new("openocd"); - - // - // We need to create a temporary file to hold our OpenOCD - // configuration file and the SREC file that we're going to - // actually program. - // - let mut conf = tempfile::NamedTempFile::new()?; - let srec = tempfile::NamedTempFile::new()?; - - if let Some(serial) = serial { - humility::msg!("specifying serial {serial}"); - - // - // In OpenOCD 0.11 dev, hla_serial has been deprecated, and - // using it results in this warning: - // - // DEPRECATED! use 'adapter serial' not 'hla_serial' - // - // Unfortunately, the newer variant ("adapter serial") does - // not exist prior to this interface being deprecated; in - // order to allow execution on older OpenOCD variants, we - // deliberately use the deprecated interface. (And yes, it - // would probably be convenient if OpenOCD just made the old - // thing work instead of shouting about it and then doing it - // anyway.) - // - writeln!(conf, "interface hla\nhla_serial {}", serial)?; - } - - if let FlashProgramConfig::Payload(payload) = payload { - write!(conf, "{}", payload)?; - } else { - bail!("unexpected OpenOCD payload: {:?}", payload); - } - - std::fs::write(&srec, generate_srec_from_elf(elf)?)?; - - // - // OpenOCD only deals with slash paths, not native paths - // (regardless of platform), so we turn our paths into slash - // paths. - // - let conf_path = conf.path().to_slash_lossy().into_owned(); - let srec_path = srec.path().to_slash_lossy().into_owned(); - - if subargs.retain || subargs.dryrun { - humility::msg!("retaining OpenOCD config as {:?}", conf.path()); - humility::msg!("retaining srec as {srec_path}"); - conf.keep()?; - srec.keep()?; - } - - for arg in &config.args { - match arg { - FlashArgument::Direct(val) => { - flash.arg(val); - } - FlashArgument::FormattedPayload(pre, post) => { - flash.arg(format!("{} {} {}", pre, srec_path, post)); - } - FlashArgument::Config => { - flash.arg(&conf_path); - } - _ => { - anyhow::bail!("unexpected OpenOCD argument {:?}", arg); - } - } - } - - if subargs.dryrun { - dryrun(&flash); - return Ok(()); - } - - let status = nice_status(&mut flash)?; - - if !status.success() { - anyhow::bail!("flash command ({:?}) failed; see output", flash); - } - - Ok(()) -} - /// Validates the image and auxiliary flash against our subcommand /// /// If `subargs.check` is true, returns `Ok(())` on a clean check and `Err(..)` @@ -351,14 +195,7 @@ fn flashcmd(context: &mut ExecutionContext) -> Result<()> { let config = hubris.load_flash_config()?; if subargs.force_openocd { - humility::msg!("forcing flashing using OpenOCD"); - return force_openocd( - hubris, - &context.cli, - &subargs, - &config.metadata, - &config.elf, - ); + bail!("openocd no longer supported"); } let probe = match &context.cli.probe { @@ -368,15 +205,7 @@ fn flashcmd(context: &mut ExecutionContext) -> Result<()> { let chip = match config.chip { Some(c) => c, - None => { - return force_openocd( - hubris, - &context.cli, - &subargs, - &config.metadata, - &config.elf, - ); - } + None => bail!("Archive is very old and missing a chip"), }; humility::msg!("attaching with chip set to {chip:x?}"); @@ -515,19 +344,6 @@ fn program_auxflash( } } -/// Executes `command` for its exit status, so that stdout/stderr are shared -/// with this process. This is equivalent to calling `Command::status` except -/// that, if the command fails to even _start,_ we provide a more detailed error -/// message than the default "no such file or directory." -fn nice_status(command: &mut std::process::Command) -> Result { - command.status().with_context(|| { - format!( - "unable to execute {:?}, is it in your PATH and executable?", - command.get_program(), - ) - }) -} - pub fn init() -> Command { Command { app: FlashArgs::command(), @@ -586,20 +402,6 @@ fn elf_chunks(elf_data: &[u8]) -> Result> { Ok(addr_slices) } -fn generate_srec_from_elf(data: &[u8]) -> Result { - let mut records = vec![srec::Record::S0("humility!".into())]; - - for (addr, slice) in elf_chunks(data)? { - records.push(srec::Record::S3(srec::Data { - address: srec::Address32(addr), - data: slice.to_vec(), - })); - } - records.push(srec::Record::S7(srec::Address32(0))); // bogus entry point - - Ok(srec::writer::generate_srec_file(&records)) -} - fn generate_ihex_from_elf(data: &[u8]) -> Result { // Build up IHEX records from that information. let mut records = vec![]; diff --git a/cmd/openocd/Cargo.toml b/cmd/openocd/Cargo.toml deleted file mode 100644 index dda94eb33..000000000 --- a/cmd/openocd/Cargo.toml +++ /dev/null @@ -1,13 +0,0 @@ -[package] -name = "humility-cmd-openocd" -version = "0.1.0" -edition.workspace = true -description = "Run OpenOCD for the given archive" - -[dependencies] -humility-cmd = { workspace = true } -humility-cli = { workspace = true } -anyhow = { workspace = true } -clap = { workspace = true } -ctrlc = { workspace = true } -tempfile = { workspace = true } diff --git a/cmd/openocd/src/lib.rs b/cmd/openocd/src/lib.rs deleted file mode 100644 index 5e75ecb09..000000000 --- a/cmd/openocd/src/lib.rs +++ /dev/null @@ -1,98 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -//! ## `humility openocd` -//! -//! This command launches OpenOCD based on the config file in a build archive, -//! which then allows one to connect with either GDB or directly via telnet. -//! If the intention is to only run GDB, note that `humility gdb --run-openocd` -//! will both run OpenOCD and run a foreground GDB that is connected to it. -//! - -use std::process; - -use humility_cli::ExecutionContext; -use humility_cmd::{Archive, Command, CommandKind}; - -use anyhow::{Context, Result, bail}; -use clap::{CommandFactory, Parser}; - -#[derive(Parser, Debug)] -#[clap( - name = "openocd", about = env!("CARGO_PKG_DESCRIPTION"), -)] -struct OcdArgs { - /// specifies the `openocd` executable to run - #[clap(short, long)] - exec: Option, - - /// specifies the probe serial number to use with OpenOCD - #[clap(long)] - serial: Option, - - /// Extra options to pass to `openocd` - #[clap(last = true)] - extra_options: Vec, -} - -fn openocd(context: &mut ExecutionContext) -> Result<()> { - if context.cli.probe.is_some() { - bail!("Cannot specify --probe with `openocd` subcommand"); - } - - let subargs = OcdArgs::try_parse_from(&context.cli.cmd)?; - let serial = context.cli.get_probe_serial(subargs.serial.as_deref())?; - - let hubris = context.archive.as_ref().unwrap(); - - let work_dir = tempfile::tempdir()?; - hubris - .extract_file_to( - "debug/openocd.cfg", - &work_dir.path().join("openocd.cfg"), - ) - .context("OpenOCD config missing. Is your Hubris build too old?")?; - let mut cmd = process::Command::new( - subargs.exec.unwrap_or_else(|| "openocd".to_string()), - ); - cmd.arg("-f").arg("openocd.cfg"); - if let Some(serial) = serial { - cmd.arg("-c") - .arg("interface hla") - .arg("-c") - .arg(format!("hla_serial {}", serial)); - } - cmd.current_dir(work_dir.path()); - - for opt in subargs.extra_options { - cmd.arg(opt); - } - - // Run OpenOCD, ignoring Ctrl-C (so it can handle them) - ctrlc::set_handler(|| {}).expect("Error setting Ctrl-C handler"); - let status = cmd.status()?; - - // Then, check on the OpenOCD status. - // - // If OpenOCD is killed by Ctrl-C (which is typical), `status.success()` - // will be `false`, but `status.code()` will be `None` (based on - // `ExitStatus` docs), so this saves us from printing a spurious error - // message. - if !status.success() && status.code().is_some() { - bail!( - "command failed ({:?}), see output for details", - status.code().unwrap() - ); - } - Ok(()) -} - -pub fn init() -> Command { - Command { - app: OcdArgs::command(), - name: "openocd", - run: openocd, - kind: CommandKind::Unattached { archive: Archive::Required }, - } -} diff --git a/humility-bin/Cargo.toml b/humility-bin/Cargo.toml index 5848b9201..a4061e7cf 100644 --- a/humility-bin/Cargo.toml +++ b/humility-bin/Cargo.toml @@ -64,7 +64,6 @@ cmd-map = { workspace = true } cmd-monorail = { workspace = true } cmd-mwocp = { workspace = true } cmd-net = { workspace = true } -cmd-openocd = { workspace = true } cmd-pmbus = { workspace = true } cmd-power = { workspace = true } cmd-powershelf = { workspace = true } From 9ec560d14f5753339131772615172c8bef582a46 Mon Sep 17 00:00:00 2001 From: Laura Abbott Date: Wed, 29 Apr 2026 15:42:23 -0400 Subject: [PATCH 3/4] Drop openocd backend Once agian, has not been used in an extended amount of time. --- humility-probes-core/src/lib.rs | 25 +-- humility-probes-core/src/openocd.rs | 232 ---------------------------- 2 files changed, 1 insertion(+), 256 deletions(-) delete mode 100644 humility-probes-core/src/openocd.rs diff --git a/humility-probes-core/src/lib.rs b/humility-probes-core/src/lib.rs index c72d28b0b..e53214563 100644 --- a/humility-probes-core/src/lib.rs +++ b/humility-probes-core/src/lib.rs @@ -12,7 +12,6 @@ use anyhow::{Result, anyhow, bail}; use humility::hubris::HubrisArchive; use humility::msg; -mod openocd; mod probe_rs; mod unattached; @@ -130,9 +129,6 @@ pub fn attach_to_probe( probe_info.serial_number, ))) } - "ocd" => { - bail!("Probe only attachment with {} is not supported", probe) - } "auto" => attach_to_probe("usb", speed_khz), _ => match TryInto::::try_into(probe) { Ok(selector) => { @@ -192,26 +188,7 @@ pub fn attach_to_chip( can_flash, ))) } - "ocd" => { - let mut core = openocd::OpenOCDCore::new()?; - let version = core.sendcmd("version")?; - - if !version.contains("Open On-Chip Debugger") { - bail!("version string unrecognized: \"{}\"", version); - } - - crate::msg!("attached via OpenOCD"); - - Ok(Box::new(core)) - } - - "auto" => { - if let Ok(probe) = attach_to_chip("ocd", chip, speed_khz) { - return Ok(probe); - } - - attach_to_chip("usb", chip, speed_khz) - } + "auto" => attach_to_chip("usb", chip, speed_khz), _ => match TryInto::::try_into(probe) { Ok(selector) => { diff --git a/humility-probes-core/src/openocd.rs b/humility-probes-core/src/openocd.rs deleted file mode 100644 index 0fb856114..000000000 --- a/humility-probes-core/src/openocd.rs +++ /dev/null @@ -1,232 +0,0 @@ -// This Source Code Form is subject to the terms of the Mozilla Public -// License, v. 2.0. If a copy of the MPL was not distributed with this -// file, You can obtain one at https://mozilla.org/MPL/2.0/. - -use crate::probe_rs::CORE_MAX_READSIZE; -use anyhow::{Result, anyhow, bail, ensure}; -use humility::core::Core; -use humility_arch_arm::ARMRegister; -use std::io::{Read, Write}; -use std::net::TcpStream; -use std::path::Path; -use std::time::Duration; - -const OPENOCD_COMMAND_DELIMITER: u8 = 0x1a; - -pub struct OpenOCDCore { - stream: TcpStream, -} - -#[rustfmt::skip::macros(anyhow, bail)] -impl OpenOCDCore { - pub(crate) fn sendcmd(&mut self, cmd: &str) -> Result { - let mut rbuf = vec![0; 1024]; - let mut result = String::with_capacity(16); - - let mut str = String::from(cmd); - str.push(OPENOCD_COMMAND_DELIMITER as char); - - self.stream.write_all(str.as_bytes())?; - - loop { - let rval = self.stream.read(&mut rbuf)?; - - if rbuf[rval - 1] == OPENOCD_COMMAND_DELIMITER { - result.push_str(std::str::from_utf8(&rbuf[0..rval - 1])?); - break; - } - - result.push_str(std::str::from_utf8(&rbuf[0..rval])?); - } - - // - // Surely not surprisingly, OpenOCD doesn't have a coherent way of - // indicating that a command has failed. We fall back to assuming - // that any return value that contains "Error: " or "invalid command - // name" is in fact an error. - // - if result.contains("Error: ") { - Err(anyhow!("OpenOCD command \"{}\" failed with \"{}\"", cmd, result)) - } else if result.contains("invalid command name ") { - Err(anyhow!("OpenOCD command \"{}\" invalid: \"{}\"", cmd, result)) - } else { - Ok(result) - } - } - - pub(crate) fn new() -> Result { - let addr = "127.0.0.1:6666".parse()?; - let timeout = Duration::from_millis(100); - let stream = - TcpStream::connect_timeout(&addr, timeout).map_err(|_| { - anyhow!("can't connect to OpenOCD on port 6666; is it running?") - })?; - - Ok(Self { stream }) - } -} - -#[rustfmt::skip::macros(anyhow, bail)] -impl Core for OpenOCDCore { - fn info(&self) -> (String, Option) { - ("OpenOCD".to_string(), None) - } - - fn read_word_32(&mut self, addr: u32) -> Result { - let result = self.sendcmd(&format!("mrw 0x{:x}", addr))?; - Ok(result.parse::()?) - } - - fn read_8(&mut self, addr: u32, data: &mut [u8]) -> Result<()> { - ensure!( - data.len() <= CORE_MAX_READSIZE, - "read of {} bytes at 0x{:x} exceeds max of {}", - data.len(), - addr, - CORE_MAX_READSIZE - ); - - // - // To read an array, we put it in a TCL variable called "output" - // and then dump the variable. - // - let cmd = format!("mem2array output 8 0x{:x} {}", addr, data.len()); - - self.sendcmd("array unset output")?; - self.sendcmd(&cmd)?; - - let mut index = None; - let mut seen = vec![false; data.len()]; - - let result = self.sendcmd("return $output")?; - - // - // Entirely on-brand, if the mem2array command has failed wildly, - // OpenOCD won't actually return an error to us -- it will merely - // fail to set the variable (and we will therefore fail when - // we attempt to retrieve the variable). If we fail to - // retrieve the variable, we infer it to be a failure to - // perform the read and bail explicitly. - // - if result.contains("no such variable") { - bail!("read at 0x{:x} for {} bytes failed", addr, data.len()); - } - - // - // The output here is bonkers: instead of being (merely) the array, - // it's an (undelimited) set of 2-tuples of (index, value) -- sorted - // in strict alphabetical order by index (!!). (That is, index 100 - // comes before, say, index 11.) - // - for val in result.split(' ') { - match index { - None => { - let idx = val.parse::()?; - - if idx >= data.len() { - bail!("\"{}\": illegal index {}", cmd, idx); - } - - if seen[idx] { - bail!("\"{}\": duplicate index {}", cmd, idx); - } - - seen[idx] = true; - index = Some(idx); - } - - Some(idx) => { - data[idx] = val.parse::()?; - index = None; - } - } - } - - for v in seen.iter().enumerate() { - ensure!(v.1, "\"{}\": missing index {}", cmd, v.0); - } - - Ok(()) - } - - fn write_reg(&mut self, _reg: ARMRegister, _val: u32) -> Result<()> { - // This does not work right now, TODO? - // - Err(anyhow!( - "Writing registers is not currently supported with OpenOCD" - )) - } - - fn read_reg(&mut self, reg: ARMRegister) -> Result { - use num_traits::ToPrimitive; - - let cmd = format!("reg {}", ARMRegister::to_u16(®).unwrap()); - let rval = self.sendcmd(&cmd)?; - - if let Some(line) = rval.lines().next() - && let Some(val) = line.split_whitespace().last() - && let Ok(rval) = parse_int::parse::(val) - { - return Ok(rval); - } - - Err(anyhow!("\"{}\": malformed return value: {:?}", cmd, rval)) - } - - fn write_word_32(&mut self, addr: u32, data: u32) -> Result<()> { - self.sendcmd(&format!("mww 0x{:x} 0x{:x}", addr, data))?; - Ok(()) - } - - fn write_8(&mut self, addr: u32, data: &[u8]) -> Result<()> { - // - // Perform the writes one byte at a time. We (obviously?) don't - // expect these writes to be large (they are likely due to HIF - // execution), but if they become so, this can be made up to 4X faster - // (and, it must be said, significantly more complicate) by using mww - // for the word-aligned writes within the data payload. - // - for (i, b) in data.iter().enumerate() { - self.sendcmd(&format!("mwb 0x{:x} 0x{:x}", addr + i as u32, b))?; - } - - Ok(()) - } - - fn halt(&mut self) -> Result<()> { - // - // On OpenOCD, we don't halt: if GDB is connected, it gets really, - // really confused! (There is unfortunately no way to know if GDB is - // connected or not, but because the OpenOCD target is most often used - // when GDB is running, we assume that it is indeed running.) - // - Ok(()) - } - - fn run(&mut self) -> Result<()> { - // - // Well, see above. - // - Ok(()) - } - - fn step(&mut self) -> Result<()> { - todo!(); - } - - fn load(&mut self, _path: &Path) -> Result<()> { - bail!("Flash loading is not supported with OpenOCD"); - } - - fn reset(&mut self) -> Result<()> { - bail!("Reset is not supported with OpenOCD"); - } - - fn reset_and_halt(&mut self, _dur: std::time::Duration) -> Result<()> { - bail!("Reset is not supported with OpenOCD"); - } - - fn wait_for_halt(&mut self, _dur: std::time::Duration) -> Result<()> { - bail!("Wait for halt is not supported with OpenOCD"); - } -} From 69ebc265be4fba3834ec0be986d9cdee5d8ad6bb Mon Sep 17 00:00:00 2001 From: Laura Abbott Date: Fri, 1 May 2026 14:53:10 -0400 Subject: [PATCH 4/4] Drop some more legacy flashing behavior --- humility-core/src/hubris.rs | 93 +------------------------------------ 1 file changed, 1 insertion(+), 92 deletions(-) diff --git a/humility-core/src/hubris.rs b/humility-core/src/hubris.rs index 9a6da3842..580c467b6 100644 --- a/humility-core/src/hubris.rs +++ b/humility-core/src/hubris.rs @@ -594,36 +594,8 @@ impl HubrisFlashMap { } } -// -// This is the Hubris definition -// -#[derive(Debug, Deserialize)] -pub enum FlashProgram { - PyOcd(Vec), - OpenOcd(FlashProgramConfig), -} - -#[derive(Debug, Deserialize)] -pub enum FlashProgramConfig { - Path(Vec), - Payload(String), -} - -#[derive(Debug, Deserialize)] -pub enum FlashArgument { - Direct(String), - Payload, - FormattedPayload(String, String), - Config, -} - #[derive(Debug, Deserialize)] pub struct HubrisFlashMeta { - /// Legacy flash program. Not included in new archives. - pub program: Option, - /// Arguments for legacy flash program, or empty if not used. - #[serde(default)] - pub args: Vec, /// Chip name used by probe-rs. pub chip: Option, } @@ -1663,70 +1635,7 @@ impl HubrisArchive { // This is incredibly ugly! It also gives us backwards compatibility! let chip: Option = match config.chip { Some(ref chip) => Some(chip.to_string()), - None => match &config.program { - Some(FlashProgram::PyOcd(args)) => { - let s69 = regex::Regex::new(r"lpc55s69").unwrap(); - let s28 = regex::Regex::new(r"lpc55s28").unwrap(); - let mut c: Option = None; - for arg in args { - c = match arg { - FlashArgument::Direct(s) => { - if s69.is_match(s) { - Some("LPC55S69JBD100".to_string()) - } else if s28.is_match(s) { - Some("LPC55S28JBD64".to_string()) - } else { - None - } - } - _ => None, - }; - if c.is_some() { - break; - } - } - c - } - Some(FlashProgram::OpenOcd(a)) => match a { - FlashProgramConfig::Payload(d) => { - let h7 = - regex::Regex::new(r"find target/stm32h7").unwrap(); - let f3 = - regex::Regex::new(r"find target/stm32f3").unwrap(); - let f4 = - regex::Regex::new(r"find target/stm32f4").unwrap(); - let g0 = - regex::Regex::new(r"find target/stm32g0").unwrap(); - - let mut c: Option = None; - - for s in d.split('\n') { - if h7.is_match(s) { - c = Some("STM32H753ZITx".to_string()); - break; - } - if f3.is_match(s) { - c = Some("STM32F301C6Tx".to_string()); - break; - } - if f4.is_match(s) { - c = Some("STM32F401CBUx".to_string()); - break; - } - if g0.is_match(s) { - c = Some("STM32G030C6Tx".to_string()); - break; - } - } - c - } - _ => bail!("Unexpected config?"), - }, - None => { - bail!("archive flash.ron is missing both probe-rs chip \ - name and legacy flash config"); - } - }, + None => bail!("must specify a chip in your config"), }; Ok(HubrisFlashConfig {