From f69ae677e01e37aefa89271096ead0131de175ac Mon Sep 17 00:00:00 2001 From: Matt Keeter Date: Thu, 26 Mar 2026 16:14:23 -0400 Subject: [PATCH 1/2] hiffy net plumbing --- humility-core/src/core.rs | 1 + humility-core/src/hubris.rs | 33 +++ humility-doppel/src/lib.rs | 16 +- humility-dump-agent/src/hiffy.rs | 2 +- humility-hiffy/src/lib.rs | 447 +++++++++++++++++++++++-------- humility-net-core/src/lib.rs | 70 ++--- 6 files changed, 415 insertions(+), 154 deletions(-) diff --git a/humility-core/src/core.rs b/humility-core/src/core.rs index 26f1fc2a..98a21cc3 100644 --- a/humility-core/src/core.rs +++ b/humility-core/src/core.rs @@ -126,6 +126,7 @@ pub trait Core { pub enum NetAgent { UdpRpc, DumpAgent, + Hiffy, } pub fn attach_dump( diff --git a/humility-core/src/hubris.rs b/humility-core/src/hubris.rs index 651e4ea5..eb3f9c45 100644 --- a/humility-core/src/hubris.rs +++ b/humility-core/src/hubris.rs @@ -6708,6 +6708,39 @@ impl HubrisModule { } } } + + /// Looks up enum variants by name, casting to a particular integer type + pub fn get_enum_variants_by_name + TryFrom>( + &self, + hubris: &HubrisArchive, + name: &str, + ) -> Result> + where + >::Error: std::error::Error + Send + Sync + 'static, + >::Error: std::error::Error + Send + Sync + 'static, + { + let Some(enum_ty) = self.lookup_enum_byname(hubris, name)? else { + bail!("could not find enum `{name}`"); + }; + enum_ty + .variants + .iter() + .map(|v| { + let Some(tag) = v.tag else { + bail!("variant `{}` has no tag", v.name); + }; + let t = match tag { + Tag::Signed(i) => T::try_from(i).with_context(|| { + format!("variant tag {i} for {} does not fit", v.name) + })?, + Tag::Unsigned(i) => T::try_from(i).with_context(|| { + format!("variant tag {i} for {} does not fit", v.name) + })?, + }; + Ok((v.name.clone(), t)) + }) + .collect::>>() + } } #[derive(Copy, Clone, Debug, Default)] diff --git a/humility-doppel/src/lib.rs b/humility-doppel/src/lib.rs index 9c68c06c..c1824b0c 100644 --- a/humility-doppel/src/lib.rs +++ b/humility-doppel/src/lib.rs @@ -38,7 +38,7 @@ use humility::reflect::{self, Base, Load, Ptr, Value}; use indexmap::IndexMap; use std::convert::TryInto; use std::fmt; -use zerocopy::{AsBytes, LittleEndian, U16, U64}; +use zerocopy::{AsBytes, LittleEndian, U16, U32, U64}; #[derive(Copy, Clone, Debug, Eq, PartialEq, Load)] pub struct TaskDesc { @@ -429,6 +429,20 @@ pub struct RpcHeader { pub nbytes: U16, } +/// Double of the RPC types from `hiffy` (with the `net` feature enabled) +pub mod hiffy { + use super::*; + + #[derive(Copy, Clone, Debug, AsBytes)] + #[repr(C)] + pub struct RpcHeader { + pub image_id: U64, + pub version: U16, + pub operation: U16, + pub arg: U32, + } +} + impl humility::reflect::Load for CountedRingbuf { fn from_value(v: &Value) -> Result { let rb_struct = v.as_struct()?; diff --git a/humility-dump-agent/src/hiffy.rs b/humility-dump-agent/src/hiffy.rs index b56d0804..303fd03d 100644 --- a/humility-dump-agent/src/hiffy.rs +++ b/humility-dump-agent/src/hiffy.rs @@ -182,7 +182,7 @@ impl DumpAgent for HiffyDumpAgent<'_> { // returned data size and the Hiffy context's `rdata` array size. let op = self.hubris.get_idol_command("DumpAgent.read_dump")?; let rsize = self.hubris.lookup_type(op.ok)?.size(self.hubris)?; - let chunksize = (self.context.rdata_size() / rsize) - 1; + let chunksize = (self.context.rstack_size() / rsize) - 1; let mut rval = vec![]; loop { diff --git a/humility-hiffy/src/lib.rs b/humility-hiffy/src/lib.rs index 84da6c71..c3238e0b 100644 --- a/humility-hiffy/src/lib.rs +++ b/humility-hiffy/src/lib.rs @@ -9,7 +9,7 @@ use hif::*; use humility::core::{Core, NetAgent}; use humility::hubris::*; use humility::reflect::{self, Load, Value}; -use humility_doppel::{RpcHeader, SchedState, StaticCell, TaskState}; +use humility_doppel::{RpcHeader, SchedState, StaticCell, TaskState, hiffy}; use humility_idol as idol; pub use humility_idol::IpcError; use postcard::{take_from_bytes, to_slice}; @@ -34,6 +34,22 @@ enum State { pub struct HiffyContext<'a> { hubris: &'a HubrisArchive, + scratch_size: usize, + timeout: u32, + state: State, + functions: HiffyFunctions, + hiffy: HiffyImpl<'a>, +} + +// Constants when running with the `NetUdpRpc` impl, which runs the program on +// the host computer. These values are much larger than anything we'd see on +// the embedded system, so we can run any program. +const HIFFY_NET_TEXT_SIZE: usize = 65536; +const HIFFY_NET_RSTACK_SIZE: usize = 65536; +const HIFFY_NET_SCRATCH_SIZE: usize = 65536; + +/// Variables used when interacting with the `hiffy` task +struct HiffyVars<'a> { ready: &'a HubrisVariable, kick: &'a HubrisVariable, text: &'a HubrisVariable, @@ -42,12 +58,40 @@ pub struct HiffyContext<'a> { requests: &'a HubrisVariable, errors: &'a HubrisVariable, failure: &'a HubrisVariable, - scratch_size: usize, - timeout: u32, - state: State, - functions: HiffyFunctions, - rpc_results: Vec, IpcError>>, - rpc_reply_type: Option<&'a HubrisEnum>, +} + +impl<'a> HiffyVars<'a> { + fn new(hubris: &'a HubrisArchive) -> Result { + Ok(Self { + ready: HiffyContext::variable(hubris, "HIFFY_READY", true)?, + kick: HiffyContext::variable(hubris, "HIFFY_KICK", true)?, + text: HiffyContext::variable(hubris, "HIFFY_TEXT", false)?, + data: HiffyContext::variable(hubris, "HIFFY_DATA", false)?, + rstack: HiffyContext::variable(hubris, "HIFFY_RSTACK", false)?, + requests: HiffyContext::variable(hubris, "HIFFY_REQUESTS", true)?, + errors: HiffyContext::variable(hubris, "HIFFY_ERRORS", true)?, + failure: HiffyContext::variable(hubris, "HIFFY_FAILURE", false)?, + }) + } +} + +enum HiffyImpl<'a> { + /// We are physically attached with a debugger + Debugger(HiffyVars<'a>), + /// We are communicating using the `udprpc` task + /// + /// In this mode, we must run the HIF program locally (on the big computer), + /// and perform `send` function calls with network messages. + NetUdpRpc { + results: Vec, IpcError>>, + reply_type: &'a HubrisEnum, + }, + /// We are communicating using the `hiffy` task + NetHiffy { + vars: HiffyVars<'a>, + ops: HiffyNetOps, + errs: HashMap, + }, } #[derive(Clone, Debug)] @@ -205,7 +249,57 @@ impl<'a> HiffyContext<'a> { core: &mut dyn Core, timeout: u32, ) -> Result> { - if !core.is_net() { + let hiffy = if core.is_net() { + if let Some(hiffy_task) = hubris.lookup_task("hiffy") + && hubris.does_task_have_feature(hiffy_task, "net").unwrap() + { + // We will get enum variants by finding the `enum RpcOp` in the + // archive, then getting its variant tags. + let hiffy_task = hubris.lookup_module(hiffy_task)?; + let errs = hiffy_task + .get_enum_variants_by_name(hubris, "RpcReply")? + .into_iter() + .map(|(name, tag)| (tag, name)) + .collect(); + let tags = + hiffy_task.get_enum_variants_by_name(hubris, "RpcOp")?; + let get_tag = |name| -> Result { + tags.get(name) + .ok_or_else(|| anyhow!("no variant with name `{name}`")) + .cloned() + }; + HiffyImpl::NetHiffy { + vars: HiffyVars::new(hubris)?, + ops: HiffyNetOps { + write_text: get_tag("WriteHiffyText")?, + write_data: get_tag("HiffyWriteData")?, + kick: get_tag("HiffyKick")?, + }, + errs, + } + } else { + let reply_type = { + let rpc_task = + hubris.lookup_task("udprpc").ok_or_else(|| { + anyhow!( + "Could not find `udprpc` task in this image. \ + Only -dev and -lab images include `udprpc`; \ + if you are running a production image, it is \ + not available" + ) + })?; + hubris + .lookup_module(rpc_task)? + .lookup_enum_byname(hubris, "RpcReply")? + .ok_or_else(|| anyhow!("failed to find RpcReply"))? + }; + HiffyImpl::NetUdpRpc { results: vec![], reply_type } + } + } else { + HiffyImpl::Debugger(HiffyVars::new(hubris)?) + }; + + if !matches!(hiffy, HiffyImpl::NetUdpRpc { .. }) { core.op_start()?; let (major, minor) = ( @@ -243,29 +337,29 @@ impl<'a> HiffyContext<'a> { } } - let scratch_size = match ( - core.is_net(), - Self::variable(hubris, "HIFFY_SCRATCH", false), - ) { - (false, Ok(scratch)) => { - let mut buf: Vec = vec![]; - buf.resize_with(scratch.size, Default::default); - - core.op_start()?; - core.read_8(scratch.addr, buf.as_mut_slice())?; - core.op_done()?; - - let def = hubris.lookup_struct(scratch.goff)?; - let val: Value = - Value::Struct(reflect::load_struct(hubris, &buf, def, 0)?); - let scratch_cell: StaticCell = StaticCell::from_value(&val)?; - scratch_cell.cell.value.as_array()?.len() - } - _ => { - // Backwards/network compatibility - // Previous versions stored a 256 byte array on the stack - 256 - } + let scratch_size = if matches!(hiffy, HiffyImpl::NetUdpRpc { .. }) { + HIFFY_NET_SCRATCH_SIZE + } else { + // Get the size of the HIFFY_SCRATCH variable, falling back to 256 + // bytes for older images (which used a fixed-size stack array) + Self::variable(hubris, "HIFFY_SCRATCH", false) + .map(|scratch| -> Result { + let mut buf: Vec = vec![]; + buf.resize_with(scratch.size, Default::default); + + core.op_start()?; + core.read_8(scratch.addr, buf.as_mut_slice())?; + core.op_done()?; + + let def = hubris.lookup_struct(scratch.goff)?; + let val: Value = Value::Struct(reflect::load_struct( + hubris, &buf, def, 0, + )?); + let scratch_cell: StaticCell = + StaticCell::from_value(&val)?; + Ok(scratch_cell.cell.value.as_array()?.len()) + }) + .unwrap_or(Ok(256))? }; let mut function_map = HashMap::new(); @@ -344,51 +438,39 @@ impl<'a> HiffyContext<'a> { Ok(Self { hubris, - ready: Self::variable(hubris, "HIFFY_READY", true)?, - kick: Self::variable(hubris, "HIFFY_KICK", true)?, - text: Self::variable(hubris, "HIFFY_TEXT", false)?, - data: Self::variable(hubris, "HIFFY_DATA", false)?, - rstack: Self::variable(hubris, "HIFFY_RSTACK", false)?, - requests: Self::variable(hubris, "HIFFY_REQUESTS", true)?, - errors: Self::variable(hubris, "HIFFY_ERRORS", true)?, - failure: Self::variable(hubris, "HIFFY_FAILURE", false)?, + hiffy, scratch_size, timeout, state: State::Initialized, functions: HiffyFunctions(function_map), - rpc_reply_type: if core.is_net() { - let rpc_task = - hubris.lookup_task("udprpc").ok_or_else(|| { - anyhow!( - "Could not find `udprpc` task in this image. \ - Only -dev and -lab images include `udprpc`; \ - are you running a production image?" - ) - })?; - - Some( - hubris - .lookup_module(rpc_task)? - .lookup_enum_byname(hubris, "RpcReply")? - .ok_or_else(|| anyhow!("failed to find RpcReply"))?, - ) - } else { - None - }, - rpc_results: Vec::new(), }) } pub fn data_size(&self) -> usize { - self.data.size + match &self.hiffy { + HiffyImpl::Debugger(vars) | HiffyImpl::NetHiffy { vars, .. } => { + vars.data.size + } + HiffyImpl::NetUdpRpc { .. } => 0, // not supported + } } pub fn text_size(&self) -> usize { - self.text.size + match &self.hiffy { + HiffyImpl::Debugger(vars) | HiffyImpl::NetHiffy { vars, .. } => { + vars.text.size + } + HiffyImpl::NetUdpRpc { .. } => HIFFY_NET_TEXT_SIZE, + } } - pub fn rdata_size(&self) -> usize { - self.rstack.size + pub fn rstack_size(&self) -> usize { + match &self.hiffy { + HiffyImpl::Debugger(vars) | HiffyImpl::NetHiffy { vars, .. } => { + vars.rstack.size + } + HiffyImpl::NetUdpRpc { .. } => HIFFY_NET_RSTACK_SIZE, + } } /// @@ -396,7 +478,7 @@ impl<'a> HiffyContext<'a> { /// pub fn ops_size(&self, ops: &[Op]) -> Result { let mut text: Vec = vec![]; - text.resize_with(self.text.size, Default::default); + text.resize_with(self.text_size(), Default::default); let mut total = 0; for op in ops { @@ -436,6 +518,10 @@ impl<'a> HiffyContext<'a> { fn perform_rpc(&mut self, core: &mut dyn Core, ops: &[Op]) -> Result<()> { let send = self.get_function("Send", 4).context("could not find Send")?; + let HiffyImpl::NetUdpRpc { results, reply_type } = &mut self.hiffy + else { + bail!("cannot call perform_rpc on this hiffy implementation"); + }; // Bail out immediately if the program makes a call other than Send if ops.iter().any(|op| matches!(*op, Op::Call(id) if id != send.id)) { @@ -445,18 +531,13 @@ impl<'a> HiffyContext<'a> { // Find the socket that we'll be using to communicate let udprpc = self.hubris.manifest.get_socket_by_task("udprpc")?; - // Pick values that are much larger than we'd ever see on a machine - const HIFFY_TEXT_SIZE: usize = 65536; - const HIFFY_RSTACK_SIZE: usize = 65536; - const HIFFY_SCRATCH_SIZE: usize = 65536; - // hard-coded values in task/hiffy/src/main.rs const NLABELS: usize = 4; let mut stack = [None; 32]; - let mut rstack = vec![0u8; HIFFY_RSTACK_SIZE]; - let mut scratch = vec![0u8; HIFFY_SCRATCH_SIZE]; - let mut text = vec![0u8; HIFFY_TEXT_SIZE]; + let mut rstack = vec![0u8; HIFFY_NET_RSTACK_SIZE]; + let mut scratch = vec![0u8; HIFFY_NET_SCRATCH_SIZE]; + let mut text = vec![0u8; HIFFY_NET_TEXT_SIZE]; // Serialize opcodes into `text` let buf = &mut text.as_mut_slice(); @@ -705,7 +786,7 @@ impl<'a> HiffyContext<'a> { if let Err(e) = v { bail!("Hiffy execution error: {e:?}"); } - assert_eq!(self.rpc_results.len(), 0); + assert_eq!(results.len(), 0); HIFFY_SEND_WORKSPACE.with(|workspace| { let workspace = workspace.borrow(); @@ -721,11 +802,10 @@ impl<'a> HiffyContext<'a> { // want to continue processing in this case; toss our error. // if buf[0] != 0 { - let rpc_reply_type = self.rpc_reply_type.unwrap(); // TODO: this assumes that the reply enum can be represented // by a u8 (buf[0] is a u8) and will not work with larger // discriminants, or signed discriminants. - match rpc_reply_type + match reply_type .lookup_variant_by_tag(Tag::from(buf[0])) { Some(e) => { @@ -758,9 +838,9 @@ impl<'a> HiffyContext<'a> { let rval = u32::from_be_bytes(buf[1..5].try_into().unwrap()); if rval == 0 { - self.rpc_results.push(Ok(buf[5..].to_vec())); + results.push(Ok(buf[5..].to_vec())); } else { - self.rpc_results.push(Err(IpcError::from(rval))); + results.push(Err(IpcError::from(rval))); } } // Dummy values for errors and requests, since we'll instantly return @@ -939,28 +1019,43 @@ impl<'a> HiffyContext<'a> { } } - if core.is_net() { - if data.is_some() { - bail!( - "cannot execute HIF operations with local data \ - over the network" + let (vars, writer) = match &self.hiffy { + HiffyImpl::NetUdpRpc { .. } => { + if data.is_some() { + bail!( + "cannot execute HIF operations with local data \ + over the network" + ); + } + return self.perform_rpc(core, ops); + } + HiffyImpl::NetHiffy { vars, ops, errs } => { + let image_id = u64::from_le_bytes( + self.hubris.image_id().unwrap().try_into().unwrap(), ); + let buf_size = self + .hubris + .manifest + .get_socket_by_task("hiffy") + .expect("missing socket for `hiffy` task?") + .rx + .bytes; + (vars, HiffyWrite::Net { image_id, buf_size, ops: *ops, errs }) } - - return self.perform_rpc(core, ops); - } + HiffyImpl::Debugger(vars) => (vars, HiffyWrite::Debugger(vars)), + }; if let Some(data) = data - && data.len() > self.data.size + && data.len() > self.data_size() { bail!( "data size ({}) exceeds maximum data size ({})", data.len(), - self.data.size + self.data_size() ); } - let mut text = vec![0u8; self.text.size]; + let mut text = vec![0u8; self.text_size()]; core.op_start()?; @@ -1022,6 +1117,13 @@ impl<'a> HiffyContext<'a> { expected in normal firmware." ); syscall_observed = true; + } else if matches!(self.hiffy, HiffyImpl::NetHiffy { .. }) { + // When using the hiffy network backend, we usually see the task as + // `ready` (`SchedState::Runnable`), which doesn't count as "the + // task has started" according to the rules of `has_task_started`. + // We'll be optimistic and assume that it's running; if it's not + // running, then it won't reply to packets! + syscall_observed = true; } let mut lap = 0; @@ -1034,7 +1136,7 @@ impl<'a> HiffyContext<'a> { if has_task_started(self.hubris, core, hiffy_task.unwrap())? { syscall_observed = true; } - } else if core.read_word_32(self.ready.addr)? == 1 { + } else if core.read_word_32(vars.ready.addr)? == 1 { ready = true; break; } @@ -1084,16 +1186,16 @@ impl<'a> HiffyContext<'a> { } } - core.write_8(self.text.addr, &buf[0..current])?; + writer.write_8(core, Var::Text, &buf[0..current])?; if let Some(data) = data { - core.write_8(self.data.addr, data)?; + writer.write_8(core, Var::Data, data)?; } - let prev_errors_count = core.read_word_32(self.errors.addr)?; - let prev_requests_count = core.read_word_32(self.requests.addr)?; + let prev_errors_count = core.read_word_32(vars.errors.addr)?; + let prev_requests_count = core.read_word_32(vars.requests.addr)?; - core.write_word_32(self.kick.addr, 1)?; + writer.kick(core)?; self.state = State::Kicked { kick_time: Instant::now(), @@ -1127,19 +1229,22 @@ impl<'a> HiffyContext<'a> { bail!("invalid state for waiting: {:?}", self.state); }; - // - // If this is over the network, our calls are already done by the - // time we're here; immediately transition to `ResultsReady`. - // - if core.is_net() { - self.state = State::ResultsReady; - return Ok(true); - } + let vars = match &self.hiffy { + HiffyImpl::Debugger(vars) | HiffyImpl::NetHiffy { vars, .. } => { + vars + } + HiffyImpl::NetUdpRpc { .. } => { + // If this is over the network, our calls are already done by + // the time we're here; transition to `ResultsReady` now. + self.state = State::ResultsReady; + return Ok(true); + } + }; core.op_start()?; - let new_requests_count = core.read_word_32(self.requests.addr)?; - let new_errors_count = core.read_word_32(self.errors.addr)?; + let new_requests_count = core.read_word_32(vars.requests.addr)?; + let new_errors_count = core.read_word_32(vars.errors.addr)?; core.op_done()?; @@ -1157,10 +1262,10 @@ impl<'a> HiffyContext<'a> { // HIFFY_FAILURE to provide some additional context. // let mut buf: Vec = vec![]; - buf.resize_with(self.failure.size, Default::default); + buf.resize_with(vars.failure.size, Default::default); core.op_start()?; - let r = core.read_8(self.failure.addr, buf.as_mut_slice()); + let r = core.read_8(vars.failure.addr, buf.as_mut_slice()); core.op_done()?; match r { @@ -1171,7 +1276,7 @@ impl<'a> HiffyContext<'a> { }; let hubris = self.hubris; - let f = hubris.printfmt(&buf, self.failure.goff, fmt)?; + let f = hubris.printfmt(&buf, vars.failure.goff, fmt)?; // If Hiffy reports `Invalid`, this could be due to a // patch version mismatch, i.e. Humility trying to use @@ -1225,19 +1330,24 @@ impl<'a> HiffyContext<'a> { bail!("invalid state for consuming results: {:?}", self.state); } - if core.is_net() { - let results = std::mem::take(&mut self.rpc_results); - self.state = State::ResultsConsumed; - return Ok(results); - } + let vars = match &mut self.hiffy { + HiffyImpl::Debugger(vars) | HiffyImpl::NetHiffy { vars, .. } => { + vars + } + HiffyImpl::NetUdpRpc { results, .. } => { + let results = std::mem::take(results); + self.state = State::ResultsConsumed; + return Ok(results); + } + }; let mut rstack: Vec = vec![]; - rstack.resize_with(self.rstack.size, Default::default); + rstack.resize_with(vars.rstack.size, Default::default); core.op_start()?; let mut rvec = vec![]; - core.read_8(self.rstack.addr, rstack.as_mut_slice())?; + core.read_8(vars.rstack.addr, rstack.as_mut_slice())?; core.op_done()?; @@ -1268,10 +1378,6 @@ impl<'a> HiffyContext<'a> { Ok(rvec) } - pub fn rstack_size(&self) -> usize { - self.rstack.size - } - pub fn scratch_size(&self) -> usize { self.scratch_size } @@ -1574,3 +1680,108 @@ fn check_leases( } Ok(()) } + +/// Abstraction for writing hiffy values over multiple transports +enum HiffyWrite<'a, 'b> { + Debugger(&'a HiffyVars<'b>), + Net { + image_id: u64, + buf_size: usize, + ops: HiffyNetOps, + errs: &'a HashMap, + }, +} + +/// Opcodes used by the `hiffy` network backend +#[derive(Copy, Clone)] +struct HiffyNetOps { + write_text: u16, + write_data: u16, + kick: u16, +} + +/// Variable to write with a [`HiffyWriter`] +enum Var { + Text, + Data, +} + +impl<'a, 'b> HiffyWrite<'a, 'b> { + /// Writes a buffer to a particular variable + /// + /// The caller is responsible for making sure that the buffer fits + fn write_8(&self, core: &mut dyn Core, v: Var, data: &[u8]) -> Result<()> { + match self { + HiffyWrite::Debugger(vars) => { + let addr = match v { + Var::Text => vars.text.addr, + Var::Data => vars.data.addr, + }; + core.write_8(addr, data) + } + HiffyWrite::Net { image_id, buf_size, ops, errs } => { + let op = match v { + Var::Text => ops.write_text, + Var::Data => ops.write_data, + }; + // Chosen to be smaller than packet size + let Some(chunk_size) = buf_size + .checked_sub(std::mem::size_of::()) + else { + bail!("buffer size {buf_size} is smaller than `RpcHeader`"); + }; + for (i, chunk) in data.chunks(chunk_size).enumerate() { + let offset = u32::try_from(i * chunk_size).unwrap(); + let header = hiffy::RpcHeader { + image_id: (*image_id).into(), + version: 1.into(), + operation: op.into(), + arg: offset.into(), + }; + let mut packet = header.as_bytes().to_vec(); + packet.extend(chunk); + core.send(packet.as_bytes(), NetAgent::Hiffy) + .context("failed to send write op to hiffy")?; + Self::net_recv(core, errs)?; + } + Ok(()) + } + } + } + fn kick(&self, core: &mut dyn Core) -> Result<()> { + match self { + HiffyWrite::Debugger(vars) => core.write_word_32(vars.kick.addr, 1), + HiffyWrite::Net { image_id, ops, errs, .. } => { + let header = hiffy::RpcHeader { + image_id: (*image_id).into(), + version: 1.into(), + operation: ops.kick.into(), + arg: 0.into(), + }; + core.send(header.as_bytes(), NetAgent::Hiffy) + .context("failed to send OP_KICK to hiffy")?; + Self::net_recv(core, errs)?; + Ok(()) + } + } + } + fn net_recv(core: &mut dyn Core, errs: &HashMap) -> Result<()> { + let mut buf = [0u8; 64]; + let n = core + .recv(&mut buf, NetAgent::Hiffy) + .context("failed to receive reply from hiffy")?; + if n == 0 { + bail!("got empty packet"); + } + match buf[0] { + 0 => Ok(()), + i => { + if let Some(v) = errs.get(&i) { + bail!("received error {v} ({i}): {:x?}", &buf[1..n]) + } else { + bail!("received unknown error {i}: {:x?}", &buf[1..n]) + } + } + } + } +} diff --git a/humility-net-core/src/lib.rs b/humility-net-core/src/lib.rs index c342357e..e55e3706 100644 --- a/humility-net-core/src/lib.rs +++ b/humility-net-core/src/lib.rs @@ -39,6 +39,9 @@ pub struct NetCore { /// Socket to communicate with the dump agent, or `None` if it's not present dump_agent_socket: Option, + /// Socket to communicate with `hiffy`, or `None` if it's not present + hiffy_socket: Option, + /// Map of RAM regions, or `None` if the dump agent can't read RAM ram: Option>, @@ -77,9 +80,20 @@ impl NetCore { .map(open_socket) .transpose()?; + // We'll look up the `hiffy` task by name, because it doesn't implement + // a particular interface. If someone named it something else, that's + // their problem. + let hiffy_socket = hubris + .manifest + .get_socket_by_task("hiffy") + .ok() + .map(open_socket) + .transpose()?; + let mut out = Self { udprpc_socket, dump_agent_socket, + hiffy_socket, flash: HubrisFlashMap::new(hubris)?, ram: None, // filled in below imageid: hubris @@ -278,6 +292,23 @@ impl NetCore { Ok(()) } + + fn get_socket_for(&self, target: NetAgent) -> Result<&UdpSocket> { + match target { + NetAgent::UdpRpc => self + .udprpc_socket + .as_ref() + .ok_or_else(|| anyhow!("no `udprpc` socket")), + NetAgent::DumpAgent => self + .dump_agent_socket + .as_ref() + .ok_or_else(|| anyhow!("no dump agent socket")), + NetAgent::Hiffy => self + .hiffy_socket + .as_ref() + .ok_or_else(|| anyhow!("no `hiffy` socket")), + } + } } #[rustfmt::skip::macros(bail)] @@ -297,47 +328,18 @@ impl Core for NetCore { if let Some(d) = self.dump_agent_socket.as_ref() { d.set_read_timeout(Some(timeout))?; } + if let Some(d) = self.hiffy_socket.as_ref() { + d.set_read_timeout(Some(timeout))?; + } Ok(()) } fn send(&self, buf: &[u8], target: NetAgent) -> Result { - match target { - NetAgent::UdpRpc => { - if let Some(d) = self.udprpc_socket.as_ref() { - d.send(buf) - } else { - bail!("no `udprpc` socket"); - } - } - NetAgent::DumpAgent => { - if let Some(d) = self.dump_agent_socket.as_ref() { - d.send(buf) - } else { - bail!("no dump agent socket") - } - } - } - .map_err(anyhow::Error::from) + self.get_socket_for(target)?.send(buf).map_err(anyhow::Error::from) } fn recv(&self, buf: &mut [u8], target: NetAgent) -> Result { - match target { - NetAgent::UdpRpc => { - if let Some(d) = self.udprpc_socket.as_ref() { - d.recv(buf) - } else { - bail!("no `udprpc` socket"); - } - } - NetAgent::DumpAgent => { - if let Some(d) = self.dump_agent_socket.as_ref() { - d.recv(buf) - } else { - bail!("no dump agent socket") - } - } - } - .map_err(anyhow::Error::from) + self.get_socket_for(target)?.recv(buf).map_err(anyhow::Error::from) } fn read_8(&mut self, addr: u32, data: &mut [u8]) -> Result<()> { From b524b06efe3d8dfeb8d8ea8a8fab2c69158e303a Mon Sep 17 00:00:00 2001 From: Matt Keeter Date: Thu, 16 Apr 2026 14:57:44 -0400 Subject: [PATCH 2/2] Remove suspicious unwrap --- humility-core/src/hubris.rs | 7 +++++++ humility-hiffy/src/lib.rs | 9 ++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/humility-core/src/hubris.rs b/humility-core/src/hubris.rs index eb3f9c45..1cf9748b 100644 --- a/humility-core/src/hubris.rs +++ b/humility-core/src/hubris.rs @@ -2091,6 +2091,13 @@ impl HubrisArchive { } } + pub fn lookup_module_by_name(&self, name: &str) -> Result<&HubrisModule> { + match self.modules.values().find(|m| m.name == name) { + Some(module) => Ok(module), + None => Err(anyhow!("no such task: {name}")), + } + } + pub fn lookup_module_by_iface(&self, name: &str) -> Option<&HubrisModule> { (0..self.ntasks()) .map(|t| self.lookup_module(HubrisTask::Task(t as u32)).unwrap()) diff --git a/humility-hiffy/src/lib.rs b/humility-hiffy/src/lib.rs index c3238e0b..122fe0b1 100644 --- a/humility-hiffy/src/lib.rs +++ b/humility-hiffy/src/lib.rs @@ -250,12 +250,15 @@ impl<'a> HiffyContext<'a> { timeout: u32, ) -> Result> { let hiffy = if core.is_net() { - if let Some(hiffy_task) = hubris.lookup_task("hiffy") - && hubris.does_task_have_feature(hiffy_task, "net").unwrap() + if hubris + .manifest + .task_features + .get("hiffy") + .is_some_and(|f| f.contains(&"net".to_owned())) { // We will get enum variants by finding the `enum RpcOp` in the // archive, then getting its variant tags. - let hiffy_task = hubris.lookup_module(hiffy_task)?; + let hiffy_task = hubris.lookup_module_by_name("hiffy")?; let errs = hiffy_task .get_enum_variants_by_name(hubris, "RpcReply")? .into_iter()