From e0235e114bd0aa7c2a6d4d918f8301141b77936a Mon Sep 17 00:00:00 2001 From: Matt Keeter Date: Mon, 20 Apr 2026 11:19:20 -0400 Subject: [PATCH] Automatically Load the hiffy_call result --- Cargo.lock | 1 - cmd/console-proxy/src/posix.rs | 41 ++------ cmd/hiffy/src/lib.rs | 8 +- cmd/host/src/lib.rs | 13 +-- cmd/monorail/src/lib.rs | 174 ++++++++++----------------------- cmd/net/src/lib.rs | 54 +++------- cmd/qspi/src/lib.rs | 13 +-- cmd/rendmp/src/lib.rs | 148 +++++++++++++--------------- cmd/tofino-eeprom/src/lib.rs | 10 +- humility-auxflash/src/lib.rs | 69 ++++--------- humility-core/src/reflect.rs | 20 ++-- humility-hiffy/Cargo.toml | 1 - humility-hiffy/src/lib.rs | 33 +++++-- 13 files changed, 210 insertions(+), 375 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7b19553d8..c31baa86f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2738,7 +2738,6 @@ dependencies = [ "humility-idol", "idol", "log", - "parse_int", "postcard", "thiserror 2.0.18", "zerocopy", diff --git a/cmd/console-proxy/src/posix.rs b/cmd/console-proxy/src/posix.rs index e84f66409..273bc3726 100644 --- a/cmd/console-proxy/src/posix.rs +++ b/cmd/console-proxy/src/posix.rs @@ -10,7 +10,7 @@ use std::os::unix::io::AsRawFd; use std::time::Duration; use std::{io, thread}; -use anyhow::{Context, Result, anyhow, bail}; +use anyhow::{Context, Result}; use clap::Parser; use crossbeam_channel::{Sender, select}; use picocom_map::RemapRules; @@ -53,7 +53,7 @@ impl<'a> UartConsoleHandler<'a> { fn uart_read(&mut self, buf: &mut [u8]) -> Result { let op = self.hubris.get_idol_command("ControlPlaneAgent.uart_read")?; - let value = humility_hiffy::hiffy_call( + let v = humility_hiffy::hiffy_call::( self.hubris, self.core, &mut self.context, @@ -63,14 +63,6 @@ impl<'a> UartConsoleHandler<'a> { Some(buf), )?; - let v = match value { - Ok(v) => v, - Err(e) => bail!("Got Hiffy error: {e}"), - }; - - let v = - v.as_base()?.as_u32().ok_or_else(|| anyhow!("Couldn't get U32"))?; - Ok(v as usize) } @@ -80,7 +72,7 @@ impl<'a> UartConsoleHandler<'a> { let buf = &buf[..usize::min(buf.len(), HIFFY_BUF_SIZE)]; - let value = humility_hiffy::hiffy_call( + let v = humility_hiffy::hiffy_call::( self.hubris, self.core, &mut self.context, @@ -90,14 +82,6 @@ impl<'a> UartConsoleHandler<'a> { None, )?; - let v = match value { - Ok(v) => v, - Err(e) => bail!("Got Hiffy error: {e}"), - }; - - let v = - v.as_base()?.as_u32().ok_or_else(|| anyhow!("Couldn't get U32"))?; - Ok(v as usize) } @@ -180,7 +164,7 @@ impl<'a> UartConsoleHandler<'a> { .hubris .get_idol_command("ControlPlaneAgent.set_humility_uart_client")?; - let value = humility_hiffy::hiffy_call( + humility_hiffy::hiffy_call::<()>( self.hubris, self.core, &mut self.context, @@ -189,11 +173,7 @@ impl<'a> UartConsoleHandler<'a> { None, None, )?; - - match value { - Ok(_) => Ok(()), - Err(e) => bail!("Got Hiffy error: {e}"), - } + Ok(()) } fn current_client(&mut self) -> Result<()> { @@ -201,7 +181,7 @@ impl<'a> UartConsoleHandler<'a> { .hubris .get_idol_command("ControlPlaneAgent.get_uart_client")?; - let value = humility_hiffy::hiffy_call( + let value = humility_hiffy::hiffy_call::( self.hubris, self.core, &mut self.context, @@ -211,15 +191,6 @@ impl<'a> UartConsoleHandler<'a> { None, )?; - let value = match value { - Ok(v) => v, - Err(e) => bail!("Got Hiffy error: {e}"), - }; - - let value = value - .as_enum() - .context("get_uart_client did not return an enum")?; - println!("Current console client: {}", value.disc()); Ok(()) } diff --git a/cmd/hiffy/src/lib.rs b/cmd/hiffy/src/lib.rs index 7f49c99a6..9e6e17aeb 100644 --- a/cmd/hiffy/src/lib.rs +++ b/cmd/hiffy/src/lib.rs @@ -311,7 +311,7 @@ fn hiffy(context: &mut ExecutionContext) -> Result<()> { }; ( - hiffy_call( + match hiffy_call( hubris, core, &mut context, @@ -319,7 +319,11 @@ fn hiffy(context: &mut ExecutionContext) -> Result<()> { &args, input.as_deref(), output.as_deref_mut(), - )?, + ) { + Ok(s) => Ok(s), + Err(HiffyCallError::Hiffy(s)) => Err(s), + Err(HiffyCallError::Other(e)) => return Err(e), + }, output, ) }; diff --git a/cmd/host/src/lib.rs b/cmd/host/src/lib.rs index 3bd6d2963..25a9d2da3 100644 --- a/cmd/host/src/lib.rs +++ b/cmd/host/src/lib.rs @@ -353,7 +353,7 @@ fn host_post_codes( let mut context = HiffyContext::new(hubris, core, 5000)?; let op = hubris.get_idol_command("Sequencer.post_code_buffer_len")?; - let value = humility_hiffy::hiffy_call( + let count = humility_hiffy::hiffy_call::( hubris, core, &mut context, @@ -362,12 +362,6 @@ fn host_post_codes( None, None, )?; - let Ok(reflect::Value::Base(reflect::Base::U32(count))) = value else { - bail!( - "Got bad value from post_code_buffer_len: \ - expected U32, got {value:?}" - ); - }; let op = hubris.get_idol_command("Sequencer.get_post_code")?; let handle_value = |v| { @@ -440,7 +434,7 @@ fn host_last_post_code( let mut context = HiffyContext::new(hubris, core, 5000)?; let op = hubris.get_idol_command("Sequencer.last_post_code")?; - let value = humility_hiffy::hiffy_call( + let v = humility_hiffy::hiffy_call::( hubris, core, &mut context, @@ -449,9 +443,6 @@ fn host_last_post_code( None, None, )?; - let Ok(reflect::Value::Base(reflect::Base::U32(v))) = value else { - bail!("Got bad value from last_post_code: expected U32, got {value:?}"); - }; if raw { println!("{v:08x}"); } else { diff --git a/cmd/monorail/src/lib.rs b/cmd/monorail/src/lib.rs index 13c678b4a..00fb09224 100644 --- a/cmd/monorail/src/lib.rs +++ b/cmd/monorail/src/lib.rs @@ -309,7 +309,7 @@ fn monorail_read( humility::msg!("Reading {reg} from {addr:#x}"); let op = hubris.get_idol_command("Monorail.read_vsc7448_reg")?; - let value = humility_hiffy::hiffy_call( + let value = humility_hiffy::hiffy_call::( hubris, core, context, @@ -318,30 +318,16 @@ fn monorail_read( None, None, )?; - match value { - Ok(v) => { - let value = if let Value::Base(Base::U32(v)) = v { - v - } else { - bail!("Got bad reflected value: expected U32, got {v:?}"); - }; - println!("{reg} => {value:#x}"); - - // The VSC7448 is configured to return 0x88888888 if a register is - // read too fast. This should never happen, because the `monorail` - // task configures appropriate padding bytes between setting the - // target register and reading it back. - if value == 0x88888888 { - log::warn!( - "0x88888888 typically indicates a communication issue!" - ); - } - pretty_print_fields(value, reg.fields(), 0); - } - Err(e) => { - println!("Got error: {e}"); - } + println!("{reg} => {value:#x}"); + + // The VSC7448 is configured to return 0x88888888 if a register is + // read too fast. This should never happen, because the `monorail` + // task configures appropriate padding bytes between setting the + // target register and reading it back. + if value == 0x88888888 { + log::warn!("0x88888888 typically indicates a communication issue!"); } + pretty_print_fields(value, reg.fields(), 0); Ok(()) } @@ -358,7 +344,7 @@ fn monorail_write( pretty_print_fields(value, reg.fields(), 0); let op = hubris.get_idol_command("Monorail.write_vsc7448_reg")?; - let value = humility_hiffy::hiffy_call( + humility_hiffy::hiffy_call::<()>( hubris, core, context, @@ -370,16 +356,6 @@ fn monorail_write( None, None, )?; - match value { - Ok(v) => { - if !matches!(v, Value::Base(Base::U0)) { - bail!("Got unexpected value: expected (), got {v:?}") - } - } - Err(e) => { - println!("Got error: {e}"); - } - } Ok(()) } @@ -446,7 +422,7 @@ fn monorail_phy_read( let reg = parse_phy_register(®)?; println!("Reading from port {} PHY, register {}", port, reg.name); let op = hubris.get_idol_command("Monorail.read_phy_reg")?; - let value = humility_hiffy::hiffy_call( + let value = humility_hiffy::hiffy_call::( hubris, core, context, @@ -459,19 +435,8 @@ fn monorail_phy_read( None, None, )?; - match value { - Ok(v) => { - let value = v - .as_base()? - .as_u16() - .ok_or_else(|| anyhow!("Could not get U16 from {:?}", v))?; - println!("Got result {:#x}", value); - pretty_print_fields(value as u32, ®.fields, 0); - } - Err(e) => { - println!("Got error: {}", e); - } - } + println!("Got result {:#x}", value); + pretty_print_fields(value as u32, ®.fields, 0); Ok(()) } @@ -490,7 +455,7 @@ fn monorail_phy_write( ); pretty_print_fields(value as u32, ®.fields, 0); let op = hubris.get_idol_command("Monorail.write_phy_reg")?; - let value = humility_hiffy::hiffy_call( + humility_hiffy::hiffy_call::<()>( hubris, core, context, @@ -504,16 +469,6 @@ fn monorail_phy_write( None, None, )?; - match value { - Ok(v) => { - if !matches!(v, Value::Base(Base::U0)) { - bail!("Got unexpected value: expected (), got {v:?}") - } - } - Err(e) => { - println!("Got error: {e}"); - } - } Ok(()) } @@ -724,11 +679,19 @@ fn monorail_status( let port_results = port_results .into_iter() - .map(move |r| humility_hiffy::hiffy_decode(hubris, &op_port, r)) + .map(move |r| { + humility_hiffy::hiffy_decode::( + hubris, &op_port, r, + ) + }) .collect::>>>()?; let phy_results = phy_results .into_iter() - .map(move |r| humility_hiffy::hiffy_decode(hubris, &op_phy, r)) + .map(move |r| { + humility_hiffy::hiffy_decode::( + hubris, &op_phy, r, + ) + }) .collect::>>>()?; // Decode the port and phy status values into reflect::Value @@ -794,8 +757,7 @@ fn monorail_status( } print!(" {:<3} | ", port); match port_value { - Ok(v) => { - let s = v.as_struct()?; + Ok(s) => { assert_eq!(s.name(), "PortStatus"); let (dev, serdes, mode, speed) = match &s["cfg"] { Value::Struct(cfg) => { @@ -841,8 +803,7 @@ fn monorail_status( } print!(" | "); match phy_value { - Ok(v) => { - let s = v.as_struct()?; + Ok(s) => { assert_eq!(s.name(), "PhyStatus"); let phy_ty = match &s["ty"] { Value::Enum(e) => e.disc().to_uppercase(), @@ -878,7 +839,7 @@ fn monorail_mac_table( // We need to make two HIF calls: // - Read the number of entries in the MAC table // - Loop over the table that many times, reading entries - let value = humility_hiffy::hiffy_call( + let mac_count = humility_hiffy::hiffy_call::( hubris, core, context, @@ -887,18 +848,6 @@ fn monorail_mac_table( None, None, )?; - let mac_count = match value { - Ok(v) => { - if let Value::Base(Base::U32(v)) = v { - v - } else { - bail!("Got bad reflected value: expected U32, got {:?}", v); - } - } - Err(e) => { - bail!("Got error: {e}"); - } - }; println!("Reading {mac_count} MAC addresses..."); @@ -931,13 +880,16 @@ fn monorail_mac_table( let results = context.run(core, ops.as_slice(), None)?; let results = results .into_iter() - .map(move |r| humility_hiffy::hiffy_decode(hubris, &op, r)) + .map(move |r| { + humility_hiffy::hiffy_decode::( + hubris, &op, r, + ) + }) .collect::>>>()?; let mut mac_table: BTreeMap> = BTreeMap::new(); for r in results { - if let Ok(r) = r { - let s = r.as_struct()?; + if let Ok(s) = r { assert_eq!(s.name(), "MacTableEntry"); let port = s.field::("port").unwrap(); let mac = s.field::<[u8; 6]>("mac").unwrap(); @@ -980,7 +932,7 @@ fn monorail_reset_counters( port: u8, ) -> Result<()> { let op = hubris.get_idol_command("Monorail.reset_port_counters")?; - let value = humility_hiffy::hiffy_call( + humility_hiffy::hiffy_call::<()>( hubris, core, context, @@ -989,7 +941,7 @@ fn monorail_reset_counters( None, None, )?; - value.map(|_| ()).map_err(|err| anyhow!("Got error {err}")) + Ok(()) } fn monorail_counters( @@ -999,7 +951,7 @@ fn monorail_counters( port: u8, ) -> Result<()> { let op = hubris.get_idol_command("Monorail.get_port_counters")?; - let value = humility_hiffy::hiffy_call( + let s = humility_hiffy::hiffy_call::( hubris, core, context, @@ -1008,44 +960,24 @@ fn monorail_counters( None, None, )?; - let decode_count = |s: &Value| match s { - Value::Struct(s) => { - let mc = match &s["multicast"] { - Value::Base(Base::U32(v)) => *v, - v => panic!("Expected U32, got {:?}", v), - }; - let uc = match &s["unicast"] { - Value::Base(Base::U32(v)) => *v, - v => panic!("Expected U32, got {:?}", v), - }; - let bc = match &s["broadcast"] { - Value::Base(Base::U32(v)) => *v, - v => panic!("Expected U32, got {:?}", v), - }; - (mc, uc, bc) - } - s => panic!("Expected Struct, got {:?}", s), + let decode_count = |s: &humility::reflect::Value| -> (u32, u32, u32) { + let mc = s.field("multicast").unwrap(); + let uc = s.field("unicast").unwrap(); + let bc = s.field("broadcast").unwrap(); + (mc, uc, bc) }; - match value { - Ok(v) => { - let s = v.as_struct()?; - let (rx_mc, rx_uc, rx_bc) = decode_count(&s["rx"]); - let (tx_mc, tx_uc, tx_bc) = decode_count(&s["tx"]); - println!("{} (port {port})", "Packet counters:".bold()); - println!(" Receive:"); - println!(" Unicast: {rx_uc}"); - println!(" Multicast: {rx_mc}"); - println!(" Broadcast: {rx_bc}"); - println!(" Transmit:"); - println!(" Unicast: {tx_uc}"); - println!(" Multicast: {tx_mc}"); - println!(" Broadcast: {tx_bc}"); - } - Err(e) => { - println!("Got error: {e}"); - } - } + let (rx_mc, rx_uc, rx_bc) = decode_count(&s["rx"]); + let (tx_mc, tx_uc, tx_bc) = decode_count(&s["tx"]); + println!("{} (port {port})", "Packet counters:".bold()); + println!(" Receive:"); + println!(" Unicast: {rx_uc}"); + println!(" Multicast: {rx_mc}"); + println!(" Broadcast: {rx_bc}"); + println!(" Transmit:"); + println!(" Unicast: {tx_uc}"); + println!(" Multicast: {tx_mc}"); + println!(" Broadcast: {tx_bc}"); Ok(()) } diff --git a/cmd/net/src/lib.rs b/cmd/net/src/lib.rs index d30a96445..2e7322e22 100644 --- a/cmd/net/src/lib.rs +++ b/cmd/net/src/lib.rs @@ -39,7 +39,7 @@ //! VSC8552; this is indicated with `--` in the relevant table positions. use std::collections::BTreeMap; -use anyhow::{Result, bail}; +use anyhow::Result; use clap::{CommandFactory, Parser}; use colored::Colorize; @@ -96,7 +96,7 @@ fn net_ip( ) -> Result<()> { let op = hubris.get_idol_command("Net.get_mac_address")?; - let value = humility_hiffy::hiffy_call( + let v = humility_hiffy::hiffy_call::( hubris, core, &mut hiffy_context, @@ -105,11 +105,6 @@ fn net_ip( None, None, )?; - let v = match value { - Ok(v) => v, - Err(e) => bail!("Got Hiffy error: {e}"), - }; - let v = v.as_tuple()?; assert_eq!(v.name(), "MacAddress"); let mac = v.field::<[u8; 6]>(0)?; print!("{}: ", "MAC address".bold()); @@ -152,7 +147,7 @@ fn net_mac_table( // We need to make two HIF calls: // - Read the number of entries in the MAC table // - Loop over the table that many times, reading entries - let value = humility_hiffy::hiffy_call( + let mac_count = humility_hiffy::hiffy_call::( hubris, core, &mut hiffy_context, @@ -161,18 +156,6 @@ fn net_mac_table( None, None, )?; - let mac_count = match value { - Ok(v) => { - if let Value::Base(Base::U32(v)) = v { - v - } else { - bail!("Got bad reflected value: expected U32, got {v:?}"); - } - } - Err(e) => { - bail!("Got error: {e}"); - } - }; // Due to HIF limitations (lack of Expand16 / Collect16), we're going to // limit ourselves to 255 MAC table entries. @@ -216,13 +199,16 @@ fn net_mac_table( let results = hiffy_context.run(core, ops.as_slice(), None)?; let results = results .into_iter() - .map(move |r| humility_hiffy::hiffy_decode(hubris, &op, r)) + .map(move |r| { + humility_hiffy::hiffy_decode::( + hubris, &op, r, + ) + }) .collect::>>>()?; let mut mac_table: BTreeMap> = BTreeMap::new(); for r in results { - if let Ok(r) = r { - let s = r.as_struct()?; + if let Ok(s) = r { assert_eq!(s.name(), "KszMacTableEntry"); let port = s.field::("port")?; let mac = s.field::<[u8; 6]>("mac")?; @@ -268,7 +254,7 @@ fn net_status( ) -> Result<()> { let op = hubris.get_idol_command("Net.management_link_status")?; - let value = humility_hiffy::hiffy_call( + let s = humility_hiffy::hiffy_call::( hubris, core, &mut hiffy_context, @@ -277,11 +263,6 @@ fn net_status( None, None, )?; - let v = match value { - Ok(v) => v, - Err(e) => bail!("Got Hiffy error: {e}"), - }; - let s = v.as_struct()?; assert_eq!(s.name(), "ManagementLinkStatus"); let ksz_100base_fx: Vec = s.field("ksz8463_100base_fx_link_up")?; @@ -337,7 +318,7 @@ fn net_counters( ) -> Result<()> { let op = hubris.get_idol_command("Net.management_counters")?; - let value = humility_hiffy::hiffy_call( + let s = humility_hiffy::hiffy_call::( hubris, core, &mut hiffy_context, @@ -346,23 +327,18 @@ fn net_counters( None, None, )?; - let v = match value { - Ok(v) => v, - Err(e) => bail!("Got Hiffy error: {e}"), - }; - let s = v.as_struct()?; assert_eq!(s.name(), "ManagementCounters"); if !table && !diagram { - net_counters_table(s)?; + net_counters_table(&s)?; println!(); - net_counters_diagram(s)?; + net_counters_diagram(&s)?; } else { if table { - net_counters_table(s)?; + net_counters_table(&s)?; } if diagram { - net_counters_diagram(s)?; + net_counters_diagram(&s)?; } } Ok(()) diff --git a/cmd/qspi/src/lib.rs b/cmd/qspi/src/lib.rs index 90a4966c0..c1dceace1 100644 --- a/cmd/qspi/src/lib.rs +++ b/cmd/qspi/src/lib.rs @@ -493,7 +493,7 @@ fn qspi(context: &mut ExecutionContext) -> Result<()> { s @ (Some(0) | Some(1)) => { let s = s.unwrap(); humility::msg!("Setting slot to {s}"); - let out = hiffy_call( + hiffy_call::<()>( hubris, core, &mut context, @@ -502,9 +502,6 @@ fn qspi(context: &mut ExecutionContext) -> Result<()> { None, None, )?; - if let Err(e) = out { - bail!("set_dev failed: {e}"); - } } _ => bail!("Bad slot setting"), } @@ -1027,7 +1024,7 @@ fn qspi(context: &mut ExecutionContext) -> Result<()> { 1 => "Flash1", _ => bail!("dev_select must be 0 or 1"), }; - let out = hiffy_call( + hiffy_call::<()>( hubris, core, &mut context, @@ -1036,11 +1033,7 @@ fn qspi(context: &mut ExecutionContext) -> Result<()> { None, None, )?; - if let Err(e) = out { - bail!("write_persistent_data failed: {e}"); - } else { - humility::msg!("write_persistent_data succeeded"); - } + humility::msg!("write_persistent_data succeeded"); return Ok(()); } else { bail!("expected an operation"); diff --git a/cmd/rendmp/src/lib.rs b/cmd/rendmp/src/lib.rs index fd232fe28..89993bbed 100644 --- a/cmd/rendmp/src/lib.rs +++ b/cmd/rendmp/src/lib.rs @@ -1126,7 +1126,7 @@ fn rendmp_blackbox( ) -> Result<()> { let (addr, _dev) = check_addr(&subargs, hubris)?; let op = hubris.get_idol_command("Power.rendmp_blackbox_dump")?; - let value = hiffy_call( + let e = hiffy_call::( hubris, core, context, @@ -1135,47 +1135,39 @@ fn rendmp_blackbox( None, None, )?; - match value { - Ok(Value::Enum(e)) => { - let contents = e - .contents() - .ok_or_else(|| anyhow!("missing contents in blackbox enum"))?; - let Value::Tuple(c) = contents else { - bail!("missing tuple in blackbox enum"); - }; - let Value::Array(a) = &c[0] else { - bail!("missing array in blackbox enum"); - }; - let mut data = vec![]; - for word in a.iter() { - let Value::Base(Base::U32(b)) = word else { - bail!("unexpected type in array: {word:?}"); - }; - data.extend(b.as_bytes()); - } - match e.disc() { - "Gen2p5" => println!( - "{}", - blackbox::BlackboxRamGen2p5::read_from_bytes( - data.as_slice() - ) - .map_err(|e| anyhow!( - "could not load blackbox from bytes: {e}" - ))?, - ), - "Gen2" => println!( - "{}", - blackbox::BlackboxRamGen2::read_from_bytes(data.as_slice()) - .map_err(|e| anyhow!( - "could not load blackbox from bytes: {e}" - ))?, - ), - v => bail!("unknown blackbox gen: {v:?}"), - }; - } - Ok(other) => bail!("unexpected value: {other:?}"), - Err(e) => bail!("got error: {e:?}"), + let contents = e + .contents() + .ok_or_else(|| anyhow!("missing contents in blackbox enum"))?; + let Value::Tuple(c) = contents else { + bail!("missing tuple in blackbox enum"); + }; + let Value::Array(a) = &c[0] else { + bail!("missing array in blackbox enum"); + }; + let mut data = vec![]; + for word in a.iter() { + let Value::Base(Base::U32(b)) = word else { + bail!("unexpected type in array: {word:?}"); + }; + data.extend(b.as_bytes()); } + match e.disc() { + "Gen2p5" => println!( + "{}", + blackbox::BlackboxRamGen2p5::read_from_bytes(data.as_slice()) + .map_err(|e| anyhow!( + "could not load blackbox from bytes: {e}" + ))?, + ), + "Gen2" => println!( + "{}", + blackbox::BlackboxRamGen2::read_from_bytes(data.as_slice()) + .map_err(|e| anyhow!( + "could not load blackbox from bytes: {e}" + ))?, + ), + v => bail!("unknown blackbox gen: {v:?}"), + }; Ok(()) } @@ -1625,41 +1617,36 @@ impl<'a, 'b> HifWorker<'a, 'b> { Call::WriteByte => &self.write_byte, Call::WriteWord32 => &self.write_word32, }; - let v = hiffy_decode(self.hubris, op, value) + let v = hiffy_decode::(self.hubris, op, value) .context("failed to decode {op:?} result")?; match v { - Ok(v) => { - let v = v.as_base().context("expected Base, got {v:?}")?; - match (v, call) { - (Base::U32(v), Call::ReadDma(..)) => { - out.push(Ok(Call::ReadDma(*v))) - } - (Base::U8(v), Call::ReadByte(..)) => { - out.push(Ok(Call::ReadByte(*v))) - } - (Base::U16(v), Call::ReadWord(..)) => { - out.push(Ok(Call::ReadWord(*v))) - } - (Base::U32(v), Call::ReadWord32(..)) => { - out.push(Ok(Call::ReadWord32(*v))) - } - (Base::U0, Call::WriteDma) => { - out.push(Ok(Call::WriteDma)) - } - (Base::U0, Call::WriteWord) => { - out.push(Ok(Call::WriteWord)) - } - (Base::U0, Call::WriteByte) => { - out.push(Ok(Call::WriteByte)) - } - (Base::U0, Call::WriteWord32) => { - out.push(Ok(Call::WriteWord32)) - } - (base, op) => { - bail!("got unexpected result {base} for {op:?}") - } + Ok(v) => match (v, call) { + (Base::U32(v), Call::ReadDma(..)) => { + out.push(Ok(Call::ReadDma(v))) } - } + (Base::U8(v), Call::ReadByte(..)) => { + out.push(Ok(Call::ReadByte(v))) + } + (Base::U16(v), Call::ReadWord(..)) => { + out.push(Ok(Call::ReadWord(v))) + } + (Base::U32(v), Call::ReadWord32(..)) => { + out.push(Ok(Call::ReadWord32(v))) + } + (Base::U0, Call::WriteDma) => out.push(Ok(Call::WriteDma)), + (Base::U0, Call::WriteWord) => { + out.push(Ok(Call::WriteWord)) + } + (Base::U0, Call::WriteByte) => { + out.push(Ok(Call::WriteByte)) + } + (Base::U0, Call::WriteWord32) => { + out.push(Ok(Call::WriteWord32)) + } + (base, op) => { + bail!("got unexpected result {base} for {op:?}") + } + }, Err(e) => out.push(Err(e)), } } @@ -1679,12 +1666,15 @@ fn rendmp_phase_check<'a>( .get_idol_command("Sequencer.tofino_seq_state") .or_else(|_| hubris.get_idol_command("Sequencer.get_state")) .context("could not get power state HIF operation")?; - let r = - hiffy_call(hubris, core, context, &power_state_op, &[], None, None)?; - if let Err(e) = r { - bail!("power state check got an error: {e}"); - } - if hiffy_format_result(hubris, r) != "A2" { + let r = hiffy_call(hubris, core, context, &power_state_op, &[], None, None); + let v = match r { + Ok(r) => Ok(r), + Err(HiffyCallError::Hiffy(s)) => Err(s), + Err(HiffyCallError::Other(e)) => { + return Err(e.context("power state check")); + } + }; + if hiffy_format_result(hubris, v) != "A2" { bail!("must be in A2 when checking phases"); } diff --git a/cmd/tofino-eeprom/src/lib.rs b/cmd/tofino-eeprom/src/lib.rs index e635e6428..45ca55856 100644 --- a/cmd/tofino-eeprom/src/lib.rs +++ b/cmd/tofino-eeprom/src/lib.rs @@ -85,7 +85,7 @@ impl<'a> EepromHandler<'a> { bar.set_length(out.len() as u64); for (i, chunk) in out.chunks_mut(READ_CHUNK_SIZE).enumerate() { let offset = i * READ_CHUNK_SIZE; - let value = humility_hiffy::hiffy_call( + humility_hiffy::hiffy_call::<()>( self.hubris, self.core, &mut self.context, @@ -94,9 +94,6 @@ impl<'a> EepromHandler<'a> { None, Some(chunk), )?; - if let Err(e) = value { - bail!("Got Hubris error: {:?}", e); - } bar.set_position(offset as u64); } bar.finish_and_clear(); @@ -123,7 +120,7 @@ impl<'a> EepromHandler<'a> { bar.set_length(data.len() as u64); for (i, chunk) in data.chunks(WRITE_CHUNK_SIZE).enumerate() { let offset = i * WRITE_CHUNK_SIZE; - let value = humility_hiffy::hiffy_call( + humility_hiffy::hiffy_call::<()>( self.hubris, self.core, &mut self.context, @@ -132,9 +129,6 @@ impl<'a> EepromHandler<'a> { Some(chunk), None, )?; - if let Err(e) = value { - bail!("Got Hubris error: {:?}", e); - } bar.set_position(offset as u64); } bar.set_position(data.len() as u64); diff --git a/humility-auxflash/src/lib.rs b/humility-auxflash/src/lib.rs index f09ca9ef3..43fe6ee05 100644 --- a/humility-auxflash/src/lib.rs +++ b/humility-auxflash/src/lib.rs @@ -6,7 +6,7 @@ use indicatif::{ProgressBar, ProgressStyle}; use humility::core::Core; use humility::hubris::*; -use humility_hiffy::HiffyContext; +use humility_hiffy::{HiffyCallError, HiffyContext}; use humility_idol::{HubrisIdol, IdolArgument}; const DEFAULT_SLOT_SIZE_BYTES: usize = 2 * 1024 * 1024; @@ -43,7 +43,7 @@ impl<'a> AuxFlashHandler<'a> { /// Returns the number of auxflash slots pub fn slot_count(&mut self) -> Result { let op = self.hubris.get_idol_command("AuxFlash.slot_count")?; - let value = humility_hiffy::hiffy_call( + let value = humility_hiffy::hiffy_call::( self.hubris, self.core, &mut self.context, @@ -52,12 +52,7 @@ impl<'a> AuxFlashHandler<'a> { None, None, )?; - let v = match value { - Ok(v) => v, - Err(e) => bail!("Got Hiffy error: {}", e), - }; - let v = v.as_base()?; - v.as_u32().ok_or_else(|| anyhow!("Couldn't get U32")) + Ok(value) } /// Returns the active slot, or `None` if there is no active slot @@ -65,7 +60,7 @@ impl<'a> AuxFlashHandler<'a> { let op = self .hubris .get_idol_command("AuxFlash.scan_and_get_active_slot")?; - let value = humility_hiffy::hiffy_call( + let value = humility_hiffy::hiffy_call::( self.hubris, self.core, &mut self.context, @@ -73,22 +68,18 @@ impl<'a> AuxFlashHandler<'a> { &[], None, None, - )?; - let v = match value { - Ok(v) => v, - Err(e) if e == "NoActiveSlot" => { - return Ok(None); - } - Err(e) => bail!("Got Hiffy error: {}", e), - }; - let v = v.as_base()?; - v.as_u32().ok_or_else(|| anyhow!("Couldn't get U32")).map(Some) + ); + match value { + Ok(v) => Ok(Some(v)), + Err(HiffyCallError::Hiffy(s)) if s == "NoActiveSlot" => Ok(None), + Err(e) => Err(e.into()), + } } /// Erases a single auxflash slot pub fn slot_erase(&mut self, slot: u32) -> Result<()> { let op = self.hubris.get_idol_command("AuxFlash.erase_slot")?; - let value = humility_hiffy::hiffy_call( + humility_hiffy::hiffy_call::<()>( self.hubris, self.core, &mut self.context, @@ -97,10 +88,7 @@ impl<'a> AuxFlashHandler<'a> { None, None, )?; - match value { - Ok(..) => Ok(()), - Err(e) => bail!("Got Hiffy error: {}", e), - } + Ok(()) } /// Returns the checksum of an auxflash slot @@ -109,7 +97,7 @@ impl<'a> AuxFlashHandler<'a> { /// erased). pub fn slot_status(&mut self, slot: u32) -> Result> { let op = self.hubris.get_idol_command("AuxFlash.read_slot_chck")?; - let value = humility_hiffy::hiffy_call( + let value = humility_hiffy::hiffy_call::<([u8; 32],)>( self.hubris, self.core, &mut self.context, @@ -117,23 +105,12 @@ impl<'a> AuxFlashHandler<'a> { &[("slot", IdolArgument::Scalar(u64::from(slot)))], None, None, - )?; - let v = match value { - Ok(v) => v, - Err(e) if e == "MissingChck" => return Ok(None), - Err(e) => bail!("{}", e), - }; - let array = v.as_1tuple().unwrap().as_array().unwrap(); - assert_eq!(array.len(), 32); - let mut out = [0u8; 32]; - for (o, v) in out - .iter_mut() - .zip(array.iter().map(|i| i.as_base().unwrap().as_u8().unwrap())) - { - *o = v; + ); + match value { + Ok(v) => Ok(Some(v.0)), + Err(HiffyCallError::Hiffy(e)) if e == "MissingChck" => Ok(None), + Err(e) => Err(e.into()), } - - Ok(Some(out)) } /// Reads some number of bytes from a particular slot @@ -158,7 +135,7 @@ impl<'a> AuxFlashHandler<'a> { bar.set_length(out.len() as u64); for (i, chunk) in out.chunks_mut(READ_CHUNK_SIZE).enumerate() { let offset = i * READ_CHUNK_SIZE; - let value = humility_hiffy::hiffy_call( + humility_hiffy::hiffy_call::<()>( self.hubris, self.core, &mut self.context, @@ -170,9 +147,6 @@ impl<'a> AuxFlashHandler<'a> { None, Some(chunk), )?; - if let Err(e) = value { - bail!("Got Hubris error: {:?}", e); - } bar.set_position(offset as u64); } bar.set_position(out.len() as u64); @@ -260,7 +234,7 @@ impl<'a> AuxFlashHandler<'a> { bar.set_length(data.len() as u64); for (i, chunk) in data.chunks(data_size).enumerate() { let offset = i * data_size; - let value = humility_hiffy::hiffy_call( + humility_hiffy::hiffy_call::<()>( self.hubris, self.core, &mut self.context, @@ -272,9 +246,6 @@ impl<'a> AuxFlashHandler<'a> { Some(chunk), None, )?; - if let Err(e) = value { - bail!("Got Hubris error: {:?}", e); - } bar.set_position(offset as u64); } bar.set_position(data.len() as u64); diff --git a/humility-core/src/reflect.rs b/humility-core/src/reflect.rs index a8d34ffa1..8964ff3dc 100644 --- a/humility-core/src/reflect.rs +++ b/humility-core/src/reflect.rs @@ -1077,6 +1077,16 @@ impl Load for Tuple { } } +impl Load for Array { + fn from_value(v: &Value) -> Result { + if let Value::Array(v) = v { + Ok(v.clone()) + } else { + bail!("expected array, got {v:?}"); + } + } +} + impl Load for Base { fn from_value(v: &Value) -> Result { if let Value::Base(v) = v { @@ -1132,16 +1142,6 @@ impl Load for () { } } -impl Load for Array { - fn from_value(v: &Value) -> Result { - if let Value::Array(v) = v { - Ok(v.clone()) - } else { - bail!("expected array, got {v:?}"); - } - } -} - impl Load for Option { fn from_value(v: &Value) -> Result { Ok(match v.as_enum()?.as_option()? { diff --git a/humility-hiffy/Cargo.toml b/humility-hiffy/Cargo.toml index ce1194341..a7b94d3ea 100644 --- a/humility-hiffy/Cargo.toml +++ b/humility-hiffy/Cargo.toml @@ -7,7 +7,6 @@ edition.workspace = true anyhow.workspace = true hif.workspace = true idol.workspace = true -parse_int.workspace = true postcard.workspace = true thiserror.workspace = true zerocopy.workspace = true diff --git a/humility-hiffy/src/lib.rs b/humility-hiffy/src/lib.rs index 503fc1e8f..b19d1cc7d 100644 --- a/humility-hiffy/src/lib.rs +++ b/humility-hiffy/src/lib.rs @@ -1438,11 +1438,22 @@ fn has_task_started( } } -/// Executes a Hiffy call, printing the output to the terminal +/// Error returned from [`hiffy_call`] +#[derive(thiserror::Error, Debug)] +pub enum HiffyCallError { + /// A function called in the HIF program returned an error + #[error("hiffy error: {0}")] + Hiffy(String), + /// There was an error invoking the HIF program + #[error(transparent)] + Other(#[from] anyhow::Error), +} + +/// Executes a Hiffy call, returning the output value /// /// Returns an outer error if Hiffy communication fails, or an inner error /// if the Hiffy call returns an error code (formatted as a String). -pub fn hiffy_call( +pub fn hiffy_call( hubris: &HubrisArchive, core: &mut dyn Core, context: &mut HiffyContext, @@ -1450,7 +1461,7 @@ pub fn hiffy_call( args: &[(&str, idol::IdolArgument)], lease_write: Option<&[u8]>, lease_read: Option<&mut [u8]>, -) -> Result> { +) -> Result { check_op(op)?; check_leases(op, lease_write, lease_read.as_deref())?; @@ -1488,7 +1499,10 @@ pub fn hiffy_call( let mut results = context.run(core, ops.as_slice(), lease_write)?; if results.len() != 1 { - bail!("unexpected results length: {:?}", results); + return Err(HiffyCallError::Other(anyhow!( + "unexpected results length: {:?}", + results + ))); } let mut v: Result, IpcError> = results.pop().unwrap(); @@ -1507,22 +1521,22 @@ pub fn hiffy_call( } _ => hiffy_decode(hubris, op, v)?, }; - Ok(out) + out.map_err(HiffyCallError::Hiffy) } /// Decodes a value returned from [hiffy_call] or equivalent. /// /// Returns an outer error if decoding fails, or an inner error if the Hiffy /// call returns an error code (formatted as a String). -pub fn hiffy_decode( +pub fn hiffy_decode( hubris: &HubrisArchive, op: &idol::IdolOperation, val: Result, impl Into>, -) -> Result> { +) -> Result> { let r = match val.map_err(Into::into) { Ok(val) => { let ty = hubris.lookup_type(op.ok).unwrap(); - Ok(match op.operation.encoding { + let value = match op.operation.encoding { ::idol::syntax::Encoding::Zerocopy => { humility::reflect::load_value(hubris, &val, ty, 0)? } @@ -1530,7 +1544,8 @@ pub fn hiffy_decode( | ::idol::syntax::Encoding::Hubpack => { humility::reflect::deserialize_value(hubris, &val, ty)?.0 } - }) + }; + Ok(T::from_value(&value)?) } Err(IpcError::Error(e)) => match op.error { idol::IdolError::CLike(error) => {