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 <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65119
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>

util/flashrom_tester/flashrom/src/cmd.rs

@@ -64,6 +64,7 @@ pub struct IOOpt<'a> {
     pub write: Option<&'a str>,  // -w <file>
     pub verify: Option<&'a str>, // -v <file>
     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<String> {
     }
 
     // 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());

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>;
 

util/flashrom_tester/src/cros_sysinfo.rs

@@ -59,22 +59,3 @@ pub fn system_info() -> IoResult<String> {
 pub fn bios_info() -> IoResult<String> {
     dmidecode_dispatch(&["-q", "-t0"])
 }
-
-pub fn eventlog_list() -> Result<String, std::io::Error> {
-    elogtool_dispatch(&["list"])
-}
-
-fn elogtool_dispatch<S: AsRef<OsStr> + Debug>(args: &[S]) -> IoResult<String> {
-    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)
-}

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 {