[flashrom-gerrit] [M] Change in flashrom[master]: flashrom_tester: Add positive check to verify_fail_test

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/71974 )

Change subject: flashrom_tester: Add positive check to verify_fail_test
......................................................................

flashrom_tester: Add positive check to verify_fail_test

In verify_fail_test test that verify works when expected, as well as
fails when expected. A verify_region_from_file function is added to
support this.

BUG=b:235916336
BRANCH=None
TEST=None

Change-Id: Ibbcc97086466b67cfab4f6c32140bb5f2c456beb
Signed-off-by: Evan Benn 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/71974
Reviewed-by: Peter Marheine 
Tested-by: build bot (Jenkins) 
Reviewed-by: Edward O'Callaghan 
---
M util/flashrom_tester/flashrom/src/cmd.rs
M util/flashrom_tester/flashrom/src/flashromlib.rs
M util/flashrom_tester/flashrom/src/lib.rs
M util/flashrom_tester/src/tests.rs
4 files changed, 72 insertions(+), 2 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Edward O'Callaghan: Looks good to me, approved
  Peter Marheine: Looks good to me, but someone else must approve

diff --git a/util/flashrom_tester/flashrom/src/cmd.rs b/util/flashrom_tester/flashrom/src/cmd.rs
index 85cbb12..1f13a8e 100644
--- a/util/flashrom_tester/flashrom/src/cmd.rs
+++ b/util/flashrom_tester/flashrom/src/cmd.rs
@@ -269,6 +269,18 @@
         Ok(())
     }

+    fn verify_region_from_file(&self, path: &Path, region: &str) -> Result<(), FlashromError> {
+        let opts = FlashromOpt {
+            io_opt: Some(IOOpt::Verify(OperationArgs::RegionFileRegion(
+                region, path, None,
+            ))),
+            ..Default::default()
+        };
+
+        self.dispatch(opts, "verify_region_from_file")?;
+        Ok(())
+    }
+
     fn erase(&self) -> Result<(), FlashromError> {
         let opts = FlashromOpt {
             io_opt: Some(IOOpt::Erase),
diff --git a/util/flashrom_tester/flashrom/src/flashromlib.rs b/util/flashrom_tester/flashrom/src/flashromlib.rs
index bf09e6d..5e1747b 100644
--- a/util/flashrom_tester/flashrom/src/flashromlib.rs
+++ b/util/flashrom_tester/flashrom/src/flashromlib.rs
@@ -152,6 +152,27 @@
         Ok(())
     }

+    fn verify_region_from_file(&self, path: &Path, region: &str) -> Result<(), FlashromError> {
+        let mut layout = self.flashrom.borrow_mut().layout_read_fmap_from_rom()?;
+        layout.include_region(region)?;
+        let range = layout.get_region_range(region)?;
+        let region_data = fs::read(path).map_err(|error| error.to_string())?;
+        if region_data.len() != range.len() {
+            return Err(format!(
+                "verify region range ({}) does not match provided file size ({})",
+                range.len(),
+                region_data.len()
+            )
+            .into());
+        }
+        let mut buf = vec![0; self.get_size()? as usize];
+        buf[range].copy_from_slice(&region_data);
+        self.flashrom
+            .borrow_mut()
+            .image_verify(&buf, Some(layout))?;
+        Ok(())
+    }
+
     fn erase(&self) -> Result<(), FlashromError> {
         self.flashrom.borrow_mut().erase()?;
         Ok(())
diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs
index 7c86649..41393e8 100644
--- a/util/flashrom_tester/flashrom/src/lib.rs
+++ b/util/flashrom_tester/flashrom/src/lib.rs
@@ -133,7 +133,8 @@
     /// Read the whole flash to the file specified by `path`.
     fn read_into_file(&self, path: &Path) -> Result<(), FlashromError>;

-    /// Read only a region of the flash.
+    /// Read only a region of the flash into the file specified by `path`. Note
+    /// the first byte written to the file is the first byte from the region.
     fn read_region_into_file(&self, path: &Path, region: &str) -> Result<(), FlashromError>;

/// Write the whole flash to the file specified by `path`.
@@ -152,6 +153,10 @@
     /// Verify the whole flash against the file specified by `path`.
     fn verify_from_file(&self, path: &Path) -> Result<(), FlashromError>;

+    /// Verify only the region against the file specified by `path`.
+    /// Note the first byte in the file is matched against the first byte of the region.
+    fn verify_region_from_file(&self, path: &Path, region: &str) -> Result<(), FlashromError>;
+
     /// Erase the whole flash.
     fn erase(&self) -> Result<(), FlashromError>;

diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs
index 847bfec..721a789 100644
--- a/util/flashrom_tester/src/tests.rs
+++ b/util/flashrom_tester/src/tests.rs
@@ -44,6 +44,7 @@
 use std::sync::atomic::AtomicBool;

const ELOG_FILE: &str = "/tmp/elog.file";
+const FW_MAIN_B_PATH: &str = "/tmp/FW_MAIN_B.bin";

/// Iterate over tests, yielding only those tests with names matching filter_names.
 ///
@@ -312,7 +313,16 @@

/// Check that flashrom 'verify' will fail if the provided data does not match the chip data.
 fn verify_fail_test(env: &mut TestEnv) -> TestResult {
-    // Comparing the flash contents to random data says they're not the same.
+    env.ensure_golden()?;
+    // Verify that verify is Ok when the data matches. We verify only a region
+    // and not the whole chip because coprocessors or firmware may have written
+    // some data in other regions.
+    env.cmd
+        .read_region_into_file(FW_MAIN_B_PATH.as_ref(), "FW_MAIN_B")?;
+    env.cmd
+        .verify_region_from_file(FW_MAIN_B_PATH.as_ref(), "FW_MAIN_B")?;
+
+    // Verify that verify is false when the data does not match
     match env.verify(env.random_data_file()) {
         Ok(_) => Err("Verification says flash is full of random data".into()),
         Err(_) => Ok(()),