Edward O'Callaghan submitted this change.

View Change



3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Peter Marheine: Looks good to me, but someone else must approve
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(-)

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.

To view, visit change 69418. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ic3f89ff5d22e74e4e6c94e755b936e58cb27182d
Gerrit-Change-Number: 69418
Gerrit-PatchSet: 6
Gerrit-Owner: Evan Benn <evanbenn@google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec@chromium.org>
Gerrit-Reviewer: Peter Marheine <pmarheine@chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-MessageType: merged