From d7fd204ede4f95669f4ce8e9e394e21c3db597cc Mon Sep 17 00:00:00 2001 From: "Cliff L. Biffle" Date: Mon, 3 Jun 2024 15:13:22 -0700 Subject: [PATCH] host: fix last-panic/boot-fail commands Memory layout changed in Hubris. --- Cargo.lock | 3 +- Cargo.toml | 2 +- cmd/host/Cargo.toml | 1 + cmd/host/src/lib.rs | 132 +++++++++++++++++++++++++++++++----- humility-core/src/hubris.rs | 10 +++ humility-doppel/src/lib.rs | 5 ++ tests/cmd/chip.trycmd | 4 +- tests/cmd/version.trycmd | 4 +- 8 files changed, 138 insertions(+), 23 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e5a4c649f..311ba40fc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1195,7 +1195,7 @@ dependencies = [ [[package]] name = "humility" -version = "0.11.6" +version = "0.11.7" dependencies = [ "anyhow", "bitfield", @@ -1631,6 +1631,7 @@ dependencies = [ "humility-cli", "humility-cmd", "humility-core", + "humility-doppel", "humility-log", "zerocopy", ] diff --git a/Cargo.toml b/Cargo.toml index cf3dc0b46..d71775a86 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,7 +17,7 @@ name = "humility" # # Be sure to check in and push all of the files that change. Happy versioning! # -version = "0.11.6" +version = "0.11.7" authors = ["Bryan Cantrill "] edition = "2018" license = "MPL-2.0" diff --git a/cmd/host/Cargo.toml b/cmd/host/Cargo.toml index 51f0b5115..c6af41edd 100644 --- a/cmd/host/Cargo.toml +++ b/cmd/host/Cargo.toml @@ -14,4 +14,5 @@ zerocopy.workspace = true humility.workspace = true humility-cmd.workspace = true humility-cli.workspace = true +humility-doppel.workspace = true humility-log.workspace = true diff --git a/cmd/host/src/lib.rs b/cmd/host/src/lib.rs index d408c8c9d..48a273c25 100644 --- a/cmd/host/src/lib.rs +++ b/cmd/host/src/lib.rs @@ -53,13 +53,14 @@ //! [0; 4096] //! ``` -use anyhow::{bail, Context, Result}; +use anyhow::{anyhow, bail, Context, Result}; use clap::{CommandFactory, Parser}; use zerocopy::FromBytes; -use humility::{core::Core, hubris::HubrisArchive}; +use humility::{core::Core, hubris::HubrisArchive, reflect, reflect::Load}; use humility_cli::{ExecutionContext, Subcommand}; use humility_cmd::{Archive, Attach, Command, CommandKind, Validate}; +use humility_doppel as doppel; use humility_log::{msg, warn}; #[derive(Parser, Debug)] @@ -77,15 +78,30 @@ struct HostArgs { cmd: HostCommand, } -fn read_var( +static SEPARATE_HOST_BOOT_FAIL_NAME: &str = "LAST_HOST_BOOT_FAIL"; + +static SEPARATE_LAST_HOST_PANIC_NAME: &str = "LAST_HOST_PANIC"; + +static HOST_STATE_BUF_NAME: &str = + "task_host_sp_comms::ServerImpl::claim_static_resources::BUFS"; + +/// Mirror type of the internal buf struct in `host_sp_comms`. Must be kept in +/// (partial) sync with that structure (fields that are present need to match, +/// other fields can be ignored). +#[derive(Load)] +struct HostStateBuf { + last_boot_fail: Vec, + last_panic: Vec, +} + +fn read_uqvar( hubris: &HubrisArchive, core: &mut dyn Core, name: &str, -) -> Result> { - msg!("reading {name}"); - let var = hubris - .lookup_variable(name) - .context(format!("could not find {name}; is this a Gimlet image?"))?; +) -> Result>> { + let Some(var) = hubris.lookup_variable(name).ok() else { + return Ok(None); + }; let mut buf: Vec = vec![0u8; var.size]; @@ -93,19 +109,84 @@ fn read_var( core.read_8(var.addr, buf.as_mut_slice())?; core.run()?; - Ok(buf) + Ok(Some(buf)) } -fn host_boot_fail(hubris: &HubrisArchive, core: &mut dyn Core) -> Result<()> { - let d = read_var(hubris, core, "LAST_HOST_BOOT_FAIL")?; - if d.iter().all(|&c| c == 0) { - println!("[0; {}]", d.len()); +fn read_qualified_state_buf( + hubris: &HubrisArchive, + core: &mut dyn Core, + name: &str, +) -> Result> { + let Some(var) = hubris.lookup_qualified_variable(name).ok() else { + return Ok(None); + }; + + let var_ty = hubris.lookup_type(var.goff)?; + + let mut buf: Vec = vec![0u8; var.size]; + + core.halt()?; + core.read_8(var.addr, &mut buf)?; + core.run()?; + + let v = reflect::load_value(hubris, &buf, var_ty, 0)?; + let as_static_cell = doppel::ClaimOnceCell::from_value(&v)?; + Ok(Some(HostStateBuf::from_value(&as_static_cell.cell.value)?)) +} + +fn print_escaped_ascii(mut bytes: &[u8]) { + // Drop trailing NULs to avoid escaping them and cluttering up our output. + while let Some((&b'\0', prefix)) = bytes.split_last() { + bytes = prefix; + } + + if bytes.is_empty() { + println!("Message contains no non-NUL bytes, not printing."); + return; } else { - match std::str::from_utf8(&d) { - Ok(s) => println!("{}", s.trim_matches('\0')), - Err(e) => println!("{:?}\n (could not decode: {e})", d), + println!("Message is {} bytes long:", bytes.len()); + } + + let mut buf = String::new(); + for &b in bytes { + match b { + b'\\' => { + // Escape any backslashes in the original, so that any escapes + // _we_ emit are unambiguous. + buf.push_str("\\\\"); + } + b'\n' | b'\r' | b'\t' | 0x20..=0x7E => { + // Pass through basic text characters that we expect we might + // see in a message. + buf.push(b as char); + } + _ => { + // Escape any other non-printable characters. + buf.push_str("\\x"); + buf.push_str(&format!("{b:02X}")); + } } } + println!("{buf}"); +} + +fn host_boot_fail(hubris: &HubrisArchive, core: &mut dyn Core) -> Result<()> { + // Try old name: + let d = read_uqvar(hubris, core, SEPARATE_HOST_BOOT_FAIL_NAME)?; + if let Some(d) = d { + print_escaped_ascii(&d); + return Ok(()); + } + // Try new name + let buf = read_qualified_state_buf(hubris, core, HOST_STATE_BUF_NAME)? + .ok_or_else(|| { + anyhow!( + "Could not find host boot variables under any known name; \ + is this a Gimlet image?" + ) + })?; + + print_escaped_ascii(&buf.last_boot_fail[..]); Ok(()) } @@ -149,8 +230,25 @@ struct IpccPanicStack { } fn host_last_panic(hubris: &HubrisArchive, core: &mut dyn Core) -> Result<()> { - let d = read_var(hubris, core, "LAST_HOST_PANIC")?; + // Try original name: + let d = read_uqvar(hubris, core, SEPARATE_LAST_HOST_PANIC_NAME)?; + if let Some(d) = d { + return print_panic(d); + } + + // Try new name: + let buf = read_qualified_state_buf(hubris, core, HOST_STATE_BUF_NAME)? + .ok_or_else(|| { + anyhow!( + "Could not find host boot variables under any known name; \ + is this a Gimlet image?" + ) + })?; + + print_panic(buf.last_panic) +} +fn print_panic(d: Vec) -> Result<()> { // Fix for https://github.com/oxidecomputer/hubris/issues/1554 // // In some cases, `ipd_cause` is unambiguous based on the first byte; diff --git a/humility-core/src/hubris.rs b/humility-core/src/hubris.rs index 953a97136..5664e4e2d 100644 --- a/humility-core/src/hubris.rs +++ b/humility-core/src/hubris.rs @@ -1899,6 +1899,16 @@ impl HubrisArchive { } } + pub fn lookup_qualified_variable( + &self, + name: &str, + ) -> Result<&HubrisVariable> { + match self.qualified_variables.get(name) { + Some(variable) => Ok(variable), + None => Err(anyhow!("variable {} not found", name)), + } + } + pub fn lookup_variables(&self, name: &str) -> Result<&Vec> { match self.variables.get_vec(name) { None => Err(anyhow!("variable {} not found", name)), diff --git a/humility-doppel/src/lib.rs b/humility-doppel/src/lib.rs index 0602dc9ec..e60c78fb4 100644 --- a/humility-doppel/src/lib.rs +++ b/humility-doppel/src/lib.rs @@ -384,6 +384,11 @@ pub enum CounterVariant { Nested(Counters), } +#[derive(Clone, Debug, Load)] +pub struct ClaimOnceCell { + pub cell: UnsafeCell, +} + #[derive(Clone, Debug, Load)] pub struct StaticCell { pub cell: UnsafeCell, diff --git a/tests/cmd/chip.trycmd b/tests/cmd/chip.trycmd index 93be53d08..25ad8492d 100644 --- a/tests/cmd/chip.trycmd +++ b/tests/cmd/chip.trycmd @@ -13,7 +13,7 @@ For more information try --help ``` $ humility --chip this-can-be-anything -V -humility 0.11.6 +humility 0.11.7 ``` @@ -28,7 +28,7 @@ For more information try --help ``` $ humility -c apx432 -V -humility 0.11.6 +humility 0.11.7 ``` diff --git a/tests/cmd/version.trycmd b/tests/cmd/version.trycmd index 5a4c7275f..6efbc2161 100644 --- a/tests/cmd/version.trycmd +++ b/tests/cmd/version.trycmd @@ -2,7 +2,7 @@ Long version flag: ``` $ humility --version -humility 0.11.6 +humility 0.11.7 ``` @@ -10,6 +10,6 @@ Short version flag: ``` $ humility -V -humility 0.11.6 +humility 0.11.7 ```