Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/79304?usp=email )
Change subject: flashrom_tester: Fix partial_lock_test on libflashrom ......................................................................
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(-)
Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved
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)?;