From 4342cc0f14e2945d7642e75e44346c13ca23089b Mon Sep 17 00:00:00 2001 From: Evan Benn Date: Fri, 10 Jun 2022 16:43:02 +1000 Subject: flashrom_tester: Remove subprocess from elog_sanity_test Make elog_sanity_test read the elog region itself, instead of calling out to elogtool. This avoids the need to subprocess and resolves a deadlock when elogtool attempts to obtain a flash reading lock. TEST=/usr/bin/flashrom_tester host Coreboot_ELOG_sanity TEST=flashrom --image RW_ELOG -p host -r /tmp/file.tmp2 # comparison TEST=hexdump the file and check magic signature == 0x474f4c45 Change-Id: I8ac63e15e063f9c0928e3e185154bb083b367ba9 Signed-off-by: Evan Benn Reviewed-on: https://review.coreboot.org/c/flashrom/+/65119 Tested-by: build bot (Jenkins) Reviewed-by: Peter Marheine Reviewed-by: Edward O'Callaghan --- util/flashrom_tester/flashrom/src/cmd.rs | 21 ++++++++++++++++++++ util/flashrom_tester/flashrom/src/lib.rs | 3 +++ util/flashrom_tester/src/cros_sysinfo.rs | 19 ------------------ util/flashrom_tester/src/tests.rs | 33 ++++++++++++++++++-------------- 4 files changed, 43 insertions(+), 33 deletions(-) (limited to 'util/flashrom_tester') diff --git a/util/flashrom_tester/flashrom/src/cmd.rs b/util/flashrom_tester/flashrom/src/cmd.rs index f0466da85..fab89ec07 100644 --- a/util/flashrom_tester/flashrom/src/cmd.rs +++ b/util/flashrom_tester/flashrom/src/cmd.rs @@ -64,6 +64,7 @@ pub struct IOOpt<'a> { pub write: Option<&'a str>, // -w pub verify: Option<&'a str>, // -v pub erase: bool, // -E + pub region: Option<&'a str>, // --image } #[derive(PartialEq, Debug)] @@ -264,6 +265,22 @@ impl crate::Flashrom for FlashromCmd { Ok(()) } + fn read_region(&self, path: &str, region: &str) -> Result<(), FlashromError> { + let opts = FlashromOpt { + io_opt: IOOpt { + read: Some(path), + region: Some(region), + ..Default::default() + }, + ..Default::default() + }; + + let (stdout, _) = self.dispatch(opts)?; + let output = String::from_utf8_lossy(stdout.as_slice()); + debug!("read():\n{}", output); + Ok(()) + } + fn write(&self, path: &str) -> Result<(), FlashromError> { let opts = FlashromOpt { io_opt: IOOpt { @@ -338,6 +355,10 @@ fn flashrom_decode_opts(opts: FlashromOpt) -> Vec { } // io_opt + if let Some(region) = opts.io_opt.region { + params.push("--image".to_string()); + params.push(region.to_string()); + } if opts.io_opt.read.is_some() { params.push("-r".to_string()); params.push(opts.io_opt.read.unwrap().to_string()); diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs index 924a71315..fff5863bc 100644 --- a/util/flashrom_tester/flashrom/src/lib.rs +++ b/util/flashrom_tester/flashrom/src/lib.rs @@ -114,6 +114,9 @@ pub trait Flashrom { /// Read the whole flash to the file specified by `path`. fn read(&self, path: &str) -> Result<(), FlashromError>; + /// Read only a region of the flash. + fn read_region(&self, path: &str, region: &str) -> Result<(), FlashromError>; + /// Write the whole flash to the file specified by `path`. fn write(&self, path: &str) -> Result<(), FlashromError>; diff --git a/util/flashrom_tester/src/cros_sysinfo.rs b/util/flashrom_tester/src/cros_sysinfo.rs index 4d1b347fd..7073eddf2 100644 --- a/util/flashrom_tester/src/cros_sysinfo.rs +++ b/util/flashrom_tester/src/cros_sysinfo.rs @@ -59,22 +59,3 @@ pub fn system_info() -> IoResult { pub fn bios_info() -> IoResult { dmidecode_dispatch(&["-q", "-t0"]) } - -pub fn eventlog_list() -> Result { - elogtool_dispatch(&["list"]) -} - -fn elogtool_dispatch + Debug>(args: &[S]) -> IoResult { - info!("elogtool_dispatch() running: /usr/bin/elogtool {:?}", args); - - let output = Command::new("/usr/bin/elogtool") - .args(args) - .stdin(Stdio::null()) - .output()?; - if !output.status.success() { - return Err(utils::translate_command_error(&output)); - } - - let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); - Ok(stdout) -} diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs index 208e98cb4..e08242830 100644 --- a/util/flashrom_tester/src/tests.rs +++ b/util/flashrom_tester/src/tests.rs @@ -38,11 +38,13 @@ use super::tester::{self, OutputFormat, TestCase, TestEnv, TestResult}; use super::utils::{self, LayoutNames}; use flashrom::{FlashChip, Flashrom}; use std::collections::{HashMap, HashSet}; -use std::fs::File; +use std::convert::TryInto; +use std::fs::{self, File}; use std::io::{BufRead, Write}; use std::sync::atomic::AtomicBool; const LAYOUT_FILE: &'static str = "/tmp/layout.file"; +const ELOG_FILE: &'static str = "/tmp/elog.file"; /// Iterate over tests, yielding only those tests with names matching filter_names. /// @@ -233,25 +235,28 @@ fn lock_test(env: &mut TestEnv) -> TestResult { fn elog_sanity_test(env: &mut TestEnv) -> TestResult { // Check that the elog contains *something*, as an indication that Coreboot - // is actually able to write to the Flash. Because this invokes elogtool on - // the host, it doesn't make sense to run for other chips. + // is actually able to write to the Flash. This only makes sense for chips + // running Coreboot, which we assume is just host. if env.chip_type() != FlashChip::HOST { info!("Skipping ELOG sanity check for non-host chip"); return Ok(()); } - // elogtool reads the flash, it should be back in the golden state + // flash should be back in the golden state env.ensure_golden()?; - // Output is one event per line, drop empty lines in the interest of being defensive. - let event_count = cros_sysinfo::eventlog_list()? - .lines() - .filter(|l| !l.is_empty()) - .count(); - - if event_count == 0 { - Err("ELOG contained no events".into()) - } else { - Ok(()) + + const ELOG_RW_REGION_NAME: &str = "RW_ELOG"; + env.cmd.read_region(ELOG_FILE, ELOG_RW_REGION_NAME)?; + + // Just checking for the magic numer + // TODO: improve this test to read the events + if fs::metadata(ELOG_FILE)?.len() < 4 { + return Err("ELOG contained no data".into()); + } + let data = fs::read(ELOG_FILE)?; + if u32::from_be_bytes(data[0..4].try_into()?) != 0x474f4c45 { + return Err("ELOG had bad magic number".into()); } + Ok(()) } fn host_is_chrome_test(_env: &mut TestEnv) -> TestResult { -- cgit v1.2.3