Anastasia Klimchuk submitted this 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>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/79304
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
---
M bindings/rust/libflashrom/src/lib.rs
M util/flashrom_tester/Cargo.toml
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
6 files changed, 125 insertions(+), 2 deletions(-)
diff --git a/bindings/rust/libflashrom/src/lib.rs b/bindings/rust/libflashrom/src/lib.rs
index 3f3fbc0..c3d4ebd 100644
--- a/bindings/rust/libflashrom/src/lib.rs
+++ b/bindings/rust/libflashrom/src/lib.rs
@@ -473,6 +473,87 @@
}
}
+/// 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),
+ }
+ }
+}
+
+/// Various flags of the flashrom context
+///
+/// Keep the struct in sync with the flags in flash.h
+#[derive(Debug)]
+pub struct FlashromFlags {
+ pub force: bool,
+ pub force_boardmismatch: bool,
+ pub verify_after_write: bool,
+ pub verify_whole_chip: bool,
+ pub skip_unreadable_regions: bool,
+ pub skip_unwritable_regions: bool,
+}
+
+/// Keep the default values in sync with cli_classic.c
+impl Default for FlashromFlags {
+ fn default() -> Self {
+ Self {
+ force: false,
+ force_boardmismatch: false,
+ verify_after_write: true,
+ verify_whole_chip: true,
+ // These flags are introduced to address issues related to CSME locking parts. Setting
+ // them to false as default values
+ skip_unreadable_regions: false,
+ skip_unwritable_regions: false,
+ }
+ }
+}
+
+impl fmt::Display for FlashromFlags {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ write!(
+ f,
+ "FlashromFlags {{ force: {}, force_boardmismatch: {}, verify_after_write: {}, verify_whole_chip: {}, skip_unreadable_regions: {}, skip_unwritable_regions: {} }}",
+ self.force,
+ self.force_boardmismatch,
+ self.verify_after_write,
+ self.verify_whole_chip,
+ self.skip_unreadable_regions,
+ self.skip_unwritable_regions,
+ )
+ }
+}
+
impl Chip {
/// Probe for a chip
///
@@ -705,6 +786,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/Cargo.toml b/util/flashrom_tester/Cargo.toml
index e6ed9c0..a76a5b4 100644
--- a/util/flashrom_tester/Cargo.toml
+++ b/util/flashrom_tester/Cargo.toml
@@ -22,6 +22,7 @@
clap = { version = "2.33", default-features = false, optional = true }
flashrom = { path = "flashrom/" }
libc = "0.2"
+libflashrom = { path = "../../bindings/rust/libflashrom" }
log = { version = "0.4", features = ["std"] }
rand = "0.6.4"
serde_json = "1"
diff --git a/util/flashrom_tester/flashrom/src/cmd.rs b/util/flashrom_tester/flashrom/src/cmd.rs
index 9e66fe2..00c92cb 100644
--- a/util/flashrom_tester/flashrom/src/cmd.rs
+++ b/util/flashrom_tester/flashrom/src/cmd.rs
@@ -35,6 +35,8 @@
use crate::{FlashChip, FlashromError};
+use libflashrom::FlashromFlags;
+
use std::{
ffi::{OsStr, OsString},
path::Path,
@@ -295,6 +297,12 @@
fn can_control_hw_wp(&self) -> bool {
self.fc.can_control_hw_wp()
}
+
+ fn set_flags(&self, flags: &FlashromFlags) -> () {
+ // The flashrom CLI sets its own default flags,
+ // and we currently have no need for custom flags,
+ // so this set_flags function is intentionally a no-op.
+ }
}
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..fb74188 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, FlashromFlags, 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_flags(&self, flags: &FlashromFlags) -> () {
+ self.flashrom
+ .borrow_mut()
+ .flag_set(FlashromFlag::FlashromFlagForce, flags.force);
+ self.flashrom
+ .borrow_mut()
+ .flag_set(FlashromFlag::FlashromFlagForceBoardmismatch, flags.force_boardmismatch);
+ self.flashrom
+ .borrow_mut()
+ .flag_set(FlashromFlag::FlashromFlagVerifyAfterWrite, flags.verify_after_write);
+ self.flashrom
+ .borrow_mut()
+ .flag_set(FlashromFlag::FlashromFlagVerifyWholeChip, flags.verify_whole_chip);
+ self.flashrom
+ .borrow_mut()
+ .flag_set(FlashromFlag::FlashromFlagSkipUnreadableRegions, flags.skip_unreadable_regions);
+ self.flashrom
+ .borrow_mut()
+ .flag_set(FlashromFlag::FlashromFlagSkipUnwritableRegions, flags.skip_unwritable_regions);
+ }
}
diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs
index e2c9149..e5ba9a8 100644
--- a/util/flashrom_tester/flashrom/src/lib.rs
+++ b/util/flashrom_tester/flashrom/src/lib.rs
@@ -45,7 +45,7 @@
pub use flashromlib::FlashromLib;
pub use libflashrom::{
- flashrom_log_level, FLASHROM_MSG_DEBUG, FLASHROM_MSG_DEBUG2, FLASHROM_MSG_ERROR,
+ flashrom_log_level, FlashromFlags, FLASHROM_MSG_DEBUG, FLASHROM_MSG_DEBUG2, FLASHROM_MSG_ERROR,
FLASHROM_MSG_INFO, FLASHROM_MSG_SPEW, FLASHROM_MSG_WARN,
};
@@ -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 flags used by the flashrom cli.
+ fn set_flags(&self, flags: &FlashromFlags) -> ();
}
diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs
index 4629c2e..c4098f2 100644
--- a/util/flashrom_tester/src/tester.rs
+++ b/util/flashrom_tester/src/tester.rs
@@ -38,6 +38,7 @@
use super::utils::{self, LayoutSizes};
use flashrom::FlashromError;
use flashrom::{FlashChip, Flashrom};
+use libflashrom::FlashromFlags;
use serde_json::json;
use std::fs::File;
use std::io::Write;
@@ -83,6 +84,9 @@
random_data: "/tmp/random_content.bin".into(),
layout_file: create_layout_file(rom_sz, Path::new("/tmp/"), print_layout),
};
+ let flags = FlashromFlags::default();
+ info!("Set flags: {}", flags);
+ out.cmd.set_flags(&flags);
info!("Stashing golden image for verification/recovery on completion");
out.cmd.read_into_file(&out.original_flash_contents)?;
To view, visit change 79304. To unsubscribe, or for help writing mail filters, visit settings.