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


flashrom_tester: Check the WP state when setting

Check that the hardware and software WP state are as expected in the
setter methods.

BUG=b:244663741
BRANCH=None
TEST=flashrom_tester --libflashrom host

Change-Id: Ie7f90ab478dca6f92eaa0908443e3cb156ea0319
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69402
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
1 file changed, 46 insertions(+), 9 deletions(-)

diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs
index c43c7d2..bedccaf 100644
--- a/util/flashrom_tester/src/tester.rs
+++ b/util/flashrom_tester/src/tester.rs
@@ -229,28 +229,44 @@
 
     /// Set the software write protect and check that the state is as expected.
     pub fn set_sw(&mut self, enable: bool) -> Result<&mut Self, String> {
-        info!("request={}, current={}", enable, self.current.sw);
+        info!("set_sw request={}, current={}", enable, self.current.sw);
         if self.current.sw != enable {
             self.cmd
                 .wp_toggle(/* en= */ enable)
                 .map_err(|e| e.to_string())?;
         }
-        Ok(self)
+        if Self::get_sw(self.cmd).map_err(|e| e.to_string())? != enable {
+            Err(format!(
+                "Software write protect did not change state to {} when requested",
+                enable
+            ))
+        } else {
+            self.current.sw = enable;
+            Ok(self)
+        }
     }
 
-    /// Set the hardware write protect.
+    /// 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);
         if self.can_control_hw_wp() {
             if self.current.hw != enable {
                 super::utils::toggle_hw_wp(/* dis= */ !enable)?;
-                self.current.hw = enable;
-            } else if enable {
-                info!(
-                    "Ignoring attempt to enable hardware WP with {:?} programmer",
-                    self.fc
-                );
             }
+            // toggle_hw_wp does check this, but we might not have called toggle_hw_wp so check again.
+            if Self::get_hw(self.cmd)? != enable {
+                return Err(format!(
+                    "Hardware write protect did not change state to {} when requested",
+                    enable
+                ));
+            }
+        } else {
+            info!(
+                "Ignoring attempt to set hardware WP with {:?} programmer",
+                self.fc
+            );
         }
+        self.current.hw = enable;
         Ok(self)
     }
 

