Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/69418 )
(
3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: flashrom_tester: partial_lock: Use WriteProtectState cache ......................................................................
flashrom_tester: partial_lock: Use WriteProtectState cache
partial_lock test was bypassing the WriteProtectState cache of the software write protect by directly calling env.cmd.wp_range. It was also unnesesarily disabling software WP. Fix those issues and more clearly document what the test is doing and expecting.
BUG=b:244663741 BRANCH=None TEST=flashrom_tester --libflashrom host
Change-Id: Ic3f89ff5d22e74e4e6c94e755b936e58cb27182d Signed-off-by: Evan Benn evanbenn@chromium.org Reviewed-on: https://review.coreboot.org/c/flashrom/+/69418 Reviewed-by: Peter Marheine pmarheine@chromium.org Reviewed-by: Edward O'Callaghan quasisec@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M util/flashrom_tester/src/tester.rs M util/flashrom_tester/src/tests.rs 2 files changed, 47 insertions(+), 4 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/src/tester.rs b/util/flashrom_tester/src/tester.rs index d19e45c..1fa44a8 100644 --- a/util/flashrom_tester/src/tester.rs +++ b/util/flashrom_tester/src/tester.rs @@ -246,6 +246,24 @@ } }
+ // Set software write protect with a custom range + pub fn set_range(&mut self, range: (i64, i64), enable: bool) -> Result<&mut Self, String> { + info!("set_range request={}, current={}", enable, self.current.sw); + self.cmd + .wp_range(range, enable) + .map_err(|e| e.to_string())?; + let actual_state = Self::get_sw(self.cmd).map_err(|e| e.to_string())?; + if actual_state != enable { + Err(format!( + "set_range request={}, real={}", + enable, actual_state + )) + } else { + self.current.sw = enable; + Ok(self) + } + } + /// Set the hardware write protect if supported and check that the state is as expected. pub fn set_hw(&mut self, enable: bool) -> Result<&mut Self, String> { info!("set_hw request={}, current={}", enable, self.current.hw); diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs index e6968c8..3e39e4d 100644 --- a/util/flashrom_tester/src/tests.rs +++ b/util/flashrom_tester/src/tests.rs @@ -272,10 +272,12 @@ env.ensure_golden()?;
let (wp_section_name, start, len) = utils::layout_section(env.layout(), section); - // Disable software WP so we can do range protection, but hardware WP - // must remain enabled for (most) range protection to do anything. - env.wp.set_hw(false)?.set_sw(false)?; - env.cmd.wp_range((start, len), true)?; + // Disable hardware WP so we can modify the protected range. + env.wp.set_hw(false)?; + // Then enable software WP so the range is enforced and enable hardware + // WP so that flashrom does not disable software WP during the + // operation. + env.wp.set_range((start, len), true)?; env.wp.set_hw(true)?;
// Check that we cannot write to the protected region.