From e66628c70494ba375083af15ea1fc03b0dca1579 Mon Sep 17 00:00:00 2001 From: Matt Keeter Date: Thu, 7 May 2026 14:17:56 -0400 Subject: [PATCH] Make idol_result return a Loadable type --- cmd/sbrmi/src/lib.rs | 57 +++++++++++++-------------------------- cmd/vpd/src/lib.rs | 44 +++++++++--------------------- humility-hiffy/src/lib.rs | 13 ++++----- 3 files changed, 38 insertions(+), 76 deletions(-) diff --git a/cmd/sbrmi/src/lib.rs b/cmd/sbrmi/src/lib.rs index 3aca8c42..91368b0e 100644 --- a/cmd/sbrmi/src/lib.rs +++ b/cmd/sbrmi/src/lib.rs @@ -89,7 +89,7 @@ //! desired target thread; full MCA information can similarly be retrieved //! using the `--mca` option and specifyin a desired thread. -use anyhow::{Result, anyhow}; +use anyhow::Result; use clap::{ArgGroup, CommandFactory, Parser}; use colored::Colorize; use hif::*; @@ -155,21 +155,14 @@ fn call_cpuid( ops.push(Op::Done); let results = context.run(core, ops.as_slice(), None)?; - let result = context.idol_result(&op, &results[0])?; - let registers = result.as_struct()?["value"].as_array()?; - - let register = |ndx| { - let b: &reflect::Value = ®isters[ndx]; - b.as_base()? - .as_u32() - .ok_or_else(|| anyhow!("couldn't decode index {ndx} as u32")) - }; + let result = context.idol_result::(&op, &results[0])?; + let registers = result.field::<[u32; 4]>("value")?; Ok(CpuIdResult { - eax: register(0)?, - ebx: register(1)?, - ecx: register(2)?, - edx: register(3)?, + eax: registers[0], + ebx: registers[1], + ecx: registers[2], + edx: registers[3], }) } @@ -234,12 +227,10 @@ fn cpuid( Ok(()) } -fn threadmap(arr: &reflect::Array) -> Result> { +fn threadmap(arr: &[u8]) -> Result> { let mut rval = HashSet::new(); - for (base, elem) in arr.iter().enumerate() { - let val = elem.as_base()?.as_u8().unwrap(); - + for (base, val) in arr.iter().enumerate() { for bit in 0..8 { if val & (1 << bit) != 0 { rval.insert(((base * 8) + bit) as u8); @@ -405,12 +396,8 @@ fn mca( let ipid_results = &results[nbanks as usize..]; for (bank, r) in results[..nbanks as usize].iter().enumerate() { - let status = context.idol_result(&op, r)?.as_base()?.as_u64().unwrap(); - let ipid = context - .idol_result(&op, &ipid_results[bank])? - .as_base()? - .as_u64() - .unwrap(); + let status = context.idol_result::(&op, r)?; + let ipid = context.idol_result::(&op, &ipid_results[bank])?; if !all_mca { if status == 0 { @@ -457,7 +444,7 @@ fn mca( let mut values = HashMap::new(); for (ndx, reg) in reg_results.iter().enumerate() { - let v = context.idol_result(&op, reg)?.as_base()?.as_u64().unwrap(); + let v = context.idol_result::(&op, reg)?; values.insert(allregs[ndx], v); } @@ -551,23 +538,15 @@ fn sbrmi(context: &mut ExecutionContext) -> Result<()> { let results = context.run(core, ops.as_slice(), None)?; - let nthreads = context - .idol_result(&nthreads, &results[0])? - .as_base()? - .as_u8() - .unwrap(); + let nthreads = context.idol_result::(&nthreads, &results[0])?; - let result = context.idol_result(&enabled, &results[1])?; - let enabled = threadmap(result.as_struct()?["value"].as_array()?)?; + let result = context.idol_result::>(&enabled, &results[1])?; + let enabled = threadmap(&result)?; - let result = context.idol_result(&alert, &results[2])?; - let alert = threadmap(result.as_struct()?["value"].as_array()?)?; + let result = context.idol_result::>(&alert, &results[2])?; + let alert = threadmap(&result)?; - let mcg_cap = context - .idol_result(&mcg_cap, &results[3])? - .as_base()? - .as_u64() - .unwrap(); + let mcg_cap = context.idol_result::(&mcg_cap, &results[3])?; if subargs.mca { let thread = subargs.thread.unwrap(); diff --git a/cmd/vpd/src/lib.rs b/cmd/vpd/src/lib.rs index 101386f4..f69b43ab 100644 --- a/cmd/vpd/src/lib.rs +++ b/cmd/vpd/src/lib.rs @@ -417,13 +417,9 @@ fn vpd_write( let results = context.run(core, ops.as_slice(), None)?; for (o, result) in results.iter().enumerate() { - if let Err(err) = context.idol_result(&op, result) { - bail!( - "failed to write VPD at offset {}: {:?}", - offset + o, - err - ); - } + context.idol_result::<()>(&op, result).with_context(|| { + format!("failed to write VPD at offset {}", offset + o) + })?; } offset += results.len(); @@ -472,20 +468,9 @@ fn vpd_read_at( let results = context.run(core, ops.as_slice(), None)?; let r = context - .idol_result(op, &results[0]) + .idol_result::(op, &results[0]) .with_context(|| format!("failed to read at offset {offset}"))?; - let contents = r.as_struct()?["value"].as_array()?; - let mut rval = vec![]; - - for b in contents.iter() { - if let reflect::Base::U8(val) = b.as_base()? { - rval.push(*val); - } else { - bail!("expected array of U8; found {:?}", contents); - } - } - - Ok(rval) + r.field::>("value") } fn vpd_slurp( @@ -592,9 +577,7 @@ fn vpd_lock( } }; - if let Err(err) = vpd_read(hubris, core, subargs) { - bail!("can't lock VPD: {err}"); - } + vpd_read(hubris, core, subargs).context("can't lock VPD")?; let payload = op.payload(&[("index", idol::IdolArgument::Scalar(index as u64))])?; @@ -606,9 +589,9 @@ fn vpd_lock( let results = context.run(core, ops.as_slice(), None)?; - if let Err(err) = context.idol_result(&op, &results[0]) { - bail!("failed to lock {index}: {err:?}"); - } + context + .idol_result::<()>(&op, &results[0]) + .with_context(|| format!("failed to lock {index}"))?; humility::msg!("successfully locked VPD"); @@ -710,11 +693,10 @@ fn vpd_lock_all( let mut success = 0; for (ndx, r) in locking.iter().zip(results.iter()) { - if let Err(err) = context.idol_result(&lock_op, r) { - humility::warn!("failed to lock VPD {ndx}: {err:?}"); - } else { - success += 1; - } + context + .idol_result::<()>(&lock_op, r) + .with_context(|| format!("failed to lock VPD {ndx}"))?; + success += 1; } if success != locking.len() { diff --git a/humility-hiffy/src/lib.rs b/humility-hiffy/src/lib.rs index b19d1cc7..7c0b4633 100644 --- a/humility-hiffy/src/lib.rs +++ b/humility-hiffy/src/lib.rs @@ -991,17 +991,17 @@ impl<'a> HiffyContext<'a> { } /// Convenience routine to pull out the result of an Idol call - pub fn idol_result( + pub fn idol_result( &mut self, op: &idol::IdolOperation, result: &Result, IpcError>, - ) -> Result { + ) -> Result { use humility::reflect::{deserialize_value, load_value}; - match result { + let value = match result { Ok(val) => { let ty = self.hubris.lookup_type(op.ok).unwrap(); - Ok(match op.operation.encoding { + match op.operation.encoding { ::idol::syntax::Encoding::Zerocopy => { load_value(self.hubris, val, ty, 0)? } @@ -1009,10 +1009,11 @@ impl<'a> HiffyContext<'a> { | ::idol::syntax::Encoding::Hubpack => { deserialize_value(self.hubris, val, ty)?.0 } - }) + } } Err(e) => bail!("{}", op.strerror(*e)), - } + }; + T::from_value(&value) } /// Begins HIF execution. This is potentially non-blocking with respect to