1
0
mirror of https://review.coreboot.org/flashrom.git synced 2025-07-01 14:11:15 +02:00

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>
This commit is contained in:
roccochen@chromium.com
2023-11-27 19:33:41 +08:00
committed by Anastasia Klimchuk
parent c52f949b62
commit 04deed8c8f
6 changed files with 125 additions and 2 deletions

View File

@ -473,6 +473,87 @@ impl Drop for Programmer {
}
}
/// 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 @@ impl Chip {
})
}
}
/// 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 {

View File

@ -22,6 +22,7 @@ chrono = { version = "0.4", optional = true }
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"

View File

@ -35,6 +35,8 @@
use crate::{FlashChip, FlashromError};
use libflashrom::FlashromFlags;
use std::{
ffi::{OsStr, OsString},
path::Path,
@ -295,6 +297,12 @@ impl crate::Flashrom for FlashromCmd {
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> {

View File

@ -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 @@ impl crate::Flashrom for FlashromLib {
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);
}
}

View File

@ -45,7 +45,7 @@ pub use cmd::FlashromCmd;
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 @@ pub trait Flashrom {
/// 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) -> ();
}

View File

@ -38,6 +38,7 @@ use super::types;
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 @@ impl<'a> TestEnv<'a> {
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)?;