flashrom_tester: Call crossystem with write protect argument

crossystem uses flashrom to gather data on some platforms. To avoid
firmware lock deadlock, call crossystem before initialising libflashrom.
When querying hardware write protect status, provide an argument to
crossystem so that only that field is queried. This also avoids the
deadlock, and improves performance.

BUG=b:239496316
BRANCH=None
TEST=on trogdor(arm), grunt(amd), hatch(intel):
TEST=flashrom_tester --libflashrom /usr/sbin/flashrom host Coreboot_ELOG_sanity
TEST=flashrom_tester /usr/sbin/flashrom host Coreboot_ELOG_sanity

Change-Id: I7d94cfc6ccbfbec91f12151eb0004724ccfc4e00
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65962
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>

3 files changed, 22 insertions, 92 deletions

diff --git a/util/flashrom_tester/src/main.rs b/util/flashrom_tester/src/main.rs
index 69bb92dbd..dd09f9a30 100644
--- a/util/flashrom_tester/src/main.rs
+++ b/util/flashrom_tester/src/main.rs
@@ -113,6 +113,10 @@ fn main() {
     );
     debug!("Args parsed and logging initialized OK");
 
+    debug!("Collecting crossystem info");
+    let crossystem =
+        flashrom_tester::utils::collect_crosssystem(&[]).expect("could not run crossystem");
+
     let flashrom_path = matches
         .value_of("flashrom_binary")
         .expect("flashrom_binary should be required");
@@ -143,6 +147,7 @@ fn main() {
         output_format,
         test_names,
         Some(handle_sigint()),
+        crossystem,
     ) {
         eprintln!("Failed to run tests: {:?}", e);
         std::process::exit(1);
diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs
index 7c6b8e8c9..ed912e952 100644
--- a/util/flashrom_tester/src/tests.rs
+++ b/util/flashrom_tester/src/tests.rs
@@ -86,6 +86,7 @@ pub fn generic<'a, TN: Iterator<Item = &'a str>>(
     output_format: OutputFormat,
     test_names: Option<TN>,
     terminate_flag: Option<&AtomicBool>,
+    crossystem: String,
 ) -> Result<(), Box<dyn std::error::Error>> {
     utils::ac_power_warning();
 
@@ -106,10 +107,7 @@ pub fn generic<'a, TN: Iterator<Item = &'a str>>(
         }
     }
 
-    info!(
-        "Record crossystem information.\n{}",
-        utils::collect_crosssystem()?
-    );
+    info!("Record crossystem information.\n{}", crossystem);
 
     // Register tests to run:
     let tests: &[&dyn TestCase] = &[
diff --git a/util/flashrom_tester/src/utils.rs b/util/flashrom_tester/src/utils.rs
index 717e103ea..e8fbb5311 100644
--- a/util/flashrom_tester/src/utils.rs
+++ b/util/flashrom_tester/src/utils.rs
@@ -139,12 +139,23 @@ fn pause() {
 }
 
 pub fn get_hardware_wp() -> std::result::Result<bool, String> {
-    let (_, wp) = parse_crosssystem(&collect_crosssystem()?)?;
-    Ok(wp)
+    let wp_s_val = collect_crosssystem(&["wpsw_cur"])?.parse::<u32>();
+    match wp_s_val {
+        Ok(v) => {
+            if v == 1 {
+                return Ok(true);
+            } else if v == 0 {
+                return Ok(false);
+            } else {
+                return Err("Unknown write protect value".into());
+            }
+        }
+        Err(_) => return Err("Cannot parse write protect value".into()),
+    }
 }
 
-pub fn collect_crosssystem() -> Result<String, String> {
-    let cmd = match Command::new("crossystem").output() {
+pub fn collect_crosssystem(args: &[&str]) -> Result<String, String> {
+    let cmd = match Command::new("crossystem").args(args).output() {
         Ok(x) => x,
         Err(e) => return Err(format!("Failed to run crossystem: {}", e)),
     };
@@ -156,39 +167,6 @@ pub fn collect_crosssystem() -> Result<String, String> {
     Ok(String::from_utf8_lossy(&cmd.stdout).into_owned())
 }
 
-fn parse_crosssystem(s: &str) -> Result<(Vec<&str>, bool), &'static str> {
-    // grep -v 'fwid +=' | grep -v 'hwid +='
-    let sysinfo = s
-        .split_terminator("\n")
-        .filter(|s| !s.contains("fwid +=") && !s.contains("hwid +="));
-
-    let state_line = match sysinfo.clone().filter(|s| s.starts_with("wpsw_cur")).next() {
-        None => return Err("No wpsw_cur in system info"),
-        Some(line) => line,
-    };
-    let wp_s_val = state_line
-        .trim_start_matches("wpsw_cur")
-        .trim_start_matches(' ')
-        .trim_start_matches('=')
-        .trim_start_matches(' ')
-        .get(..1)
-        .unwrap()
-        .parse::<u32>();
-
-    match wp_s_val {
-        Ok(v) => {
-            if v == 1 {
-                return Ok((sysinfo.collect(), true));
-            } else if v == 0 {
-                return Ok((sysinfo.collect(), false));
-            } else {
-                return Err("Unknown state value");
-            }
-        }
-        Err(_) => return Err("Cannot parse state value"),
-    }
-}
-
 pub fn translate_command_error(output: &std::process::Output) -> std::io::Error {
     use std::io::{Error, ErrorKind};
     // There is two cases on failure;
@@ -260,55 +238,4 @@ mod tests {
             }
         );
     }
-
-    #[test]
-    fn parse_crosssystem() {
-        use super::parse_crosssystem;
-
-        assert_eq!(
-            parse_crosssystem("This is not the tool you are looking for").err(),
-            Some("No wpsw_cur in system info")
-        );
-
-        assert_eq!(
-            parse_crosssystem("wpsw_cur = ERROR").err(),
-            Some("Cannot parse state value")
-        );
-
-        assert_eq!(
-            parse_crosssystem("wpsw_cur = 3").err(),
-            Some("Unknown state value")
-        );
-
-        assert_eq!(
-            parse_crosssystem("wpsw_cur = 0"),
-            Ok((vec!["wpsw_cur = 0"], false))
-        );
-
-        assert_eq!(
-            parse_crosssystem("wpsw_cur = 1"),
-            Ok((vec!["wpsw_cur = 1"], true))
-        );
-
-        assert_eq!(
-            parse_crosssystem("wpsw_cur=1"),
-            Ok((vec!["wpsw_cur=1"], true))
-        );
-
-        assert_eq!(
-            parse_crosssystem(
-                "fwid += 123wpsw_cur\n\
-                 hwid += aaaaa\n\
-                 wpsw_boot                  = 0                      # [RO/int]\n\
-                 wpsw_cur                   = 1                      # [RO/int]\n"
-            ),
-            Ok((
-                vec![
-                    "wpsw_boot                  = 0                      # [RO/int]",
-                    "wpsw_cur                   = 1                      # [RO/int]"
-                ],
-                true
-            ))
-        );
-    }
 }