Attention is currently required from: Evan Benn, Hsuan Ting Chen.

Hsuan-ting Chen would like Hsuan Ting Chen to review this change.

View Change

flashrom_tester: Fix partial_lock_test on libflashrom

partial_lock_test (Lock_top_quad, Lock_bottom_quad, Lock_bottom_half,
and Lock_top_half) tries to:
1. Disable HWWP
2. Lock partial
3. Enable HWWP
4. Try to write the locked partial and expect a failure
...

The 4th step only works on flashrom binary since the binary will set
FLASHROM_FLAG_VERIFY_AFTER_WRITE=1 by default and it will error out
while verifying.

But libflashrom does not set any flag beforehand, so it has
FLASHROM_FLAG_VERIFY_AFTER_WRITE=0 and thus it will think the write
command works normally and raise no error. This causes the issue that
flashrom_tester with libflashrom has been failed until today.

To solve this issue, there are two solutions:
1. Take care of the default flags in libflashrom
2. Always pass --noverify to flashrom binary and verify it afterwards.

To make both methods more consistent, I fix it with approach 1.

BUG=b:304439294
BRANCH=none
TEST=flashrom_tester internal --libflashrom Lock_top_quad

Change-Id: I7a8ac0c0984fef3cd9e73ed8d8097ddf429e54b2
Signed-off-by: roccochen@chromium.com <roccochen@chromium.org>
---
M bindings/rust/libflashrom/src/lib.rs
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/tester.rs
5 files changed, 74 insertions(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/79304/1
diff --git a/bindings/rust/libflashrom/src/lib.rs b/bindings/rust/libflashrom/src/lib.rs
index 3f3fbc0..97da595 100644
--- a/bindings/rust/libflashrom/src/lib.rs
+++ b/bindings/rust/libflashrom/src/lib.rs
@@ -473,6 +473,43 @@
}
}

+/// A translation of the flashrom_flag type
+///
+/// Keep this list in sync with libflashrom.h
+#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+pub enum FlashromFlag {
+ FlashromFlagForce,
+ FlashromFlagForceBoardmismatch,
+ FlashromFlagVerifyAfterWrite,
+ FlashromFlagVerifyWholeChip,
+ FlashromFlagSkipUnreadableRegions,
+ FlashromFlagSkipUnwritableRegions,
+}
+
+impl From<FlashromFlag> for libflashrom_sys::flashrom_flag {
+ fn from(e: FlashromFlag) -> Self {
+ match e {
+ FlashromFlag::FlashromFlagForce => libflashrom_sys::flashrom_flag::FLASHROM_FLAG_FORCE,
+ FlashromFlag::FlashromFlagForceBoardmismatch => {
+ libflashrom_sys::flashrom_flag::FLASHROM_FLAG_FORCE_BOARDMISMATCH
+ }
+ FlashromFlag::FlashromFlagVerifyAfterWrite => {
+ libflashrom_sys::flashrom_flag::FLASHROM_FLAG_VERIFY_AFTER_WRITE
+ }
+ FlashromFlag::FlashromFlagVerifyWholeChip => {
+ libflashrom_sys::flashrom_flag::FLASHROM_FLAG_VERIFY_WHOLE_CHIP
+ }
+ FlashromFlag::FlashromFlagSkipUnreadableRegions => {
+ libflashrom_sys::flashrom_flag::FLASHROM_FLAG_SKIP_UNREADABLE_REGIONS
+ }
+ FlashromFlag::FlashromFlagSkipUnwritableRegions => {
+ libflashrom_sys::flashrom_flag::FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS
+ }
+ e => panic!("Unexpected FlashromFlag: {:?}", e),
+ }
+ }
+}
+
impl Chip {
/// Probe for a chip
///
@@ -705,6 +742,11 @@
})
}
}
+
+ /// Set a flag in the given flash context
+ pub fn flag_set(&mut self, flag: FlashromFlag, value: bool) -> () {
+ unsafe { libflashrom_sys::flashrom_flag_set(self.ctx.as_mut(), flag.into(), value) }
+ }
}

impl Drop for Chip {
diff --git a/util/flashrom_tester/flashrom/src/cmd.rs b/util/flashrom_tester/flashrom/src/cmd.rs
index 1f13a8e..43c4af8 100644
--- a/util/flashrom_tester/flashrom/src/cmd.rs
+++ b/util/flashrom_tester/flashrom/src/cmd.rs
@@ -294,6 +294,10 @@
fn can_control_hw_wp(&self) -> bool {
self.fc.can_control_hw_wp()
}
+
+ fn set_default_flags(&self) -> () {
+ // flashrom cli has its own default flags.
+ }
}

fn flashrom_decode_opts(opts: FlashromOpt) -> Vec<OsString> {
diff --git a/util/flashrom_tester/flashrom/src/flashromlib.rs b/util/flashrom_tester/flashrom/src/flashromlib.rs
index 5e1747b..efbf968 100644
--- a/util/flashrom_tester/flashrom/src/flashromlib.rs
+++ b/util/flashrom_tester/flashrom/src/flashromlib.rs
@@ -32,7 +32,7 @@
// Software Foundation.
//

-use libflashrom::{Chip, Programmer};
+use libflashrom::{Chip, FlashromFlag, Programmer};

use std::{cell::RefCell, convert::TryFrom, fs, path::Path};

@@ -181,4 +181,25 @@
fn can_control_hw_wp(&self) -> bool {
self.fc.can_control_hw_wp()
}
+
+ fn set_default_flags(&self) -> () {
+ self.flashrom
+ .borrow_mut()
+ .flag_set(FlashromFlag::FlashromFlagForce, false);
+ self.flashrom
+ .borrow_mut()
+ .flag_set(FlashromFlag::FlashromFlagForceBoardmismatch, false);
+ self.flashrom
+ .borrow_mut()
+ .flag_set(FlashromFlag::FlashromFlagVerifyAfterWrite, true);
+ self.flashrom
+ .borrow_mut()
+ .flag_set(FlashromFlag::FlashromFlagVerifyWholeChip, true);
+ self.flashrom
+ .borrow_mut()
+ .flag_set(FlashromFlag::FlashromFlagSkipUnreadableRegions, true);
+ self.flashrom
+ .borrow_mut()
+ .flag_set(FlashromFlag::FlashromFlagSkipUnwritableRegions, true);
+ }
}
diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs
index e2c9149..887e369 100644
--- a/util/flashrom_tester/flashrom/src/lib.rs
+++ b/util/flashrom_tester/flashrom/src/lib.rs
@@ -162,4 +162,7 @@

/// Return true if the hardware write protect of this flash can be controlled.
fn can_control_hw_wp(&self) -> bool;
+
+ /// Set default flashrom flag if necessary.
+ fn set_default_flags(&self) -> ();
}
diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs
index 4629c2e..323b22b 100644
--- a/util/flashrom_tester/src/tester.rs
+++ b/util/flashrom_tester/src/tester.rs
@@ -84,6 +84,9 @@
layout_file: create_layout_file(rom_sz, Path::new("/tmp/"), print_layout),
};

+ info!("Setup default flashrom flag(s)");
+ out.cmd.set_default_flags();
+
info!("Stashing golden image for verification/recovery on completion");
out.cmd.read_into_file(&out.original_flash_contents)?;
out.cmd.verify_from_file(&out.original_flash_contents)?;

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

Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I7a8ac0c0984fef3cd9e73ed8d8097ddf429e54b2
Gerrit-Change-Number: 79304
Gerrit-PatchSet: 1
Gerrit-Owner: Hsuan-ting Chen <roccochen@google.com>
Gerrit-Reviewer: Evan Benn <evanbenn@gmail.com>
Gerrit-Reviewer: Hsuan Ting Chen <roccochen@chromium.org>
Gerrit-Attention: Hsuan Ting Chen <roccochen@chromium.org>
Gerrit-Attention: Evan Benn <evanbenn@gmail.com>
Gerrit-MessageType: newchange