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