Attention is currently required from: Evan Benn.

View Change

5 comments:

• Patchset:

  • Patch Set #1:

    Thanks for the prompt code review!

    I think this fix is fine in the mean time, but obviously the write itself should fail if it is being prevented due to write protect, not the verify.

    I can confirm that this is the current ChromeOS behavior. I tested it by:

    • Disable HWWP
    • Enable SWWP
    • Enable HWWP
    • Write all zero to GBB with `flashrom -p internal -i GBB -w GBB.zero.bin` and expect a fail
    • Write again with `-n`: `flashrom -p internal -i GBB -w GBB.zero.bin -n` and expect a success
    • Check that GBB was not changed

    Did this regress at some point?
    I cannot tell when this behavior changed, but the current behavior in ChromeOS is that "Write without verification will not fail."

    I remember there were differences here between chromeos and upstream flashrom, have those diverged more or are in sync now?
    I'll have a look. I can't observe any for now.

    ---

    However, regardless of the behavior, it makes more sense to run libflashrom and the flashrom binary with exactly the same flags.

• File util/flashrom_tester/flashrom/src/cmd.rs:

  • Patch Set #1, Line 299: // flashrom cli has its own default flags.

    `flashrom cli sets these flags by default`

    Done

• File util/flashrom_tester/flashrom/src/flashromlib.rs:

  • Patch Set #1, Line 200: .flag_set(FlashromFlag::FlashromFlagSkipUnreadableRegions, true);

    I don't see these last two being set in cli_classic. […]

    Those should be CrOS only default value. Let me remove them (and land only in CrOS repo: [CL](https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+/5065053/1))

• File util/flashrom_tester/flashrom/src/lib.rs:

  • Patch Set #1, Line 166: /// Set default flashrom flag if necessary.

    `Set flags to the defaults used by the flashrom cli`

    Done

• File util/flashrom_tester/src/tester.rs:

  • Patch Set #1, Line 87: info!("Setup default flashrom flag(s)");

    I don't think this is a necessary log

    Done

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