Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74930 )
Change subject: flashrom: Use WP-based unlocking on opaque masters
......................................................................
flashrom: Use WP-based unlocking on opaque masters
Flashrom only tries to use WP-based unlocking if it detectes that WP
operations are supported. However WP support was detected in a way that
ignored WP operations provided by opaque masters.
This stopped flashrom from automatically unlocking with some opaque
masters, particularly linux_mtd.
BUG=b:280111380
BRANCH=none
TEST=Checked flashrom automatically unlocked flash on strongbad (MTD)
Change-Id: I1774ad64d82ae47cd085df6045e17e283855c01f
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M flashrom.c
1 file changed, 23 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/30/74930/1
diff --git a/flashrom.c b/flashrom.c
index 07ed0df..faffe51 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -2093,7 +2093,8 @@
/* Given the existence of read locks, we want to unlock for read, erase and write. */
int ret = 1;
- if (flash->chip->decode_range != NO_DECODE_RANGE_FUNC) {
+ if (flash->chip->decode_range != NO_DECODE_RANGE_FUNC ||
+ (flash->mst->buses_supported & BUS_PROG && flash->mst->opaque.wp_write_cfg)) {
ret = unlock_flash_wp(flash);
if (ret)
msg_cerr("Failed to unlock flash status reg with wp support.\n");
--
To view, visit https://review.coreboot.org/c/flashrom/+/74930
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I1774ad64d82ae47cd085df6045e17e283855c01f
Gerrit-Change-Number: 74930
Gerrit-PatchSet: 1
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: newchange
Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Anastasia Klimchuk.
Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74872 )
Change subject: flashrom.c:Enable erase path optimisation
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS2:
> How else can we check if the region is write protected from this erase path. […]
Currently we are checking for wp from region->write_prot but that only works for BUS_PROG. For all else we attempt to erase and the failure of the erase function tells us something is wrong. This makes it difficult to figure out the reason for failure especially if its because the region is wp since the code already does the checks for writable (the first case) before attempting to erase. I feel there should be get_region for all programmers, if possible :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/74872
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22
Gerrit-Change-Number: 74872
Gerrit-PatchSet: 3
Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 02 May 2023 17:51:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: comment
Aarya has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/74882 )
Change subject: erasure_layout.c: Fix incorrect return value in erase_write
......................................................................
erasure_layout.c: Fix incorrect return value in erase_write
On failure of erasefn in erase_write it didn't set the error value in
ret which caused send success status as return value.
Change-Id: Ia3bd5fd250dcd0a03f0281c478b9bacb71872f31
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M erasure_layout.c
1 file changed, 17 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/82/74882/1
diff --git a/erasure_layout.c b/erasure_layout.c
index a802ae4..108fea4 100644
--- a/erasure_layout.c
+++ b/erasure_layout.c
@@ -328,9 +328,12 @@
addr, addr + len - 1);
free(region.name);
- if (erasefn(flashctx, addr, len))
+ if (erasefn(flashctx, addr, len)) {
+ ret = -1;
goto _end;
+ }
if (check_erased_range(flashctx, addr, len)) {
+ ret = - 1;
msg_cerr("ERASE FAILED!\n");
goto _end;
}
--
To view, visit https://review.coreboot.org/c/flashrom/+/74882
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ia3bd5fd250dcd0a03f0281c478b9bacb71872f31
Gerrit-Change-Number: 74882
Gerrit-PatchSet: 1
Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: newchange
Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Anastasia Klimchuk.
Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74872 )
Change subject: flashrom.c:Enable erase path optimisation
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> I don't know what the bug is however it seems to indicate something is wrong in the new path `get_re […]
How else can we check if the region is write protected from this erase path. The region->write_prot was supposed to do that I guess.
--
To view, visit https://review.coreboot.org/c/flashrom/+/74872
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22
Gerrit-Change-Number: 74872
Gerrit-PatchSet: 2
Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 02 May 2023 15:48:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Comment-In-Reply-To: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk.
Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74225 )
Change subject: doc: Add doc how to add docs
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
I understand that this is only an initial version of the document, but I think that it's important to write about the next things:
* copyright (Under what license will the documentation be published? What sources of information can be used? etc)
* writing style - formal or informal? jokes? are pictures allowed?
* only flashrom related documentation? or doc about general things (ROM/EEPROM, bitbang, chip packages and so on) can be placed there?
--
To view, visit https://review.coreboot.org/c/flashrom/+/74225
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I356a286ad2f3334392efadda366b0ca0f8042752
Gerrit-Change-Number: 74225
Gerrit-PatchSet: 5
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 02 May 2023 15:38:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Richard Hughes, Edward O'Callaghan, Angel Pons, Daniel Campello.
Hello build bot (Jenkins), Nico Huber, Sean Rhodes, Thomas Heijligen, Edward O'Callaghan, Angel Pons, Daniel Campello, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/64663
to look at the new patch set (#3).
Change subject: libflashrom: Allow getting the progress_state from the flashctx
......................................................................
libflashrom: Allow getting the progress_state from the flashctx
External users of the libflashrom API cannot peek into the abstract
flashrom_flashctx structure, unlike the local CLI. Provide some API to
get the flashrom_progress struct from a flashrom_flashctx.
This allows fwupd to use the new flashrom_set_progress_callback() API
without using a global variable with the callback data.
Tested with fwupd in https://github.com/fwupd/fwupd/pull/4675
Change-Id: I322bf56ff92f7b4d0ffc92768e9f0cdf7cb82010
Signed-off-by: Richard Hughes <richard(a)hughsie.com>
---
M cli_output.c
M include/libflashrom.h
M libflashrom.c
M libflashrom.map
4 files changed, 36 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/64663/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/64663
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I322bf56ff92f7b4d0ffc92768e9f0cdf7cb82010
Gerrit-Change-Number: 64663
Gerrit-PatchSet: 3
Gerrit-Owner: Richard Hughes <richard(a)hughsie.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Richard Hughes <richard(a)hughsie.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Daniel Campello <campello(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Aarya, Nikolai Artemiev, Anastasia Klimchuk.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74872 )
Change subject: flashrom.c:Enable erase path optimisation
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> This change breaks the unit test for full chip erase when write protect is enabled (https://ticket. […]
I don't know what the bug is however it seems to indicate something is wrong in the new path `get_region` not being set I don't think is the root cause of anything, I would expect that to be `NULL` for most drivers.
--
To view, visit https://review.coreboot.org/c/flashrom/+/74872
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22
Gerrit-Change-Number: 74872
Gerrit-PatchSet: 2
Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Tue, 02 May 2023 02:00:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Anastasia Klimchuk.
Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/74872 )
Change subject: flashrom.c:Enable erase path optimisation
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
This change breaks the unit test for full chip erase when write protect is enabled (https://ticket.coreboot.org/issues/488). I tracked the reason to be get_flash_region in flashrom.c:372 where flash->mst->spi.get_region has no value assigned so the final else block is being executed instead of the BUS_SPI one.
--
To view, visit https://review.coreboot.org/c/flashrom/+/74872
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22
Gerrit-Change-Number: 74872
Gerrit-PatchSet: 2
Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 01 May 2023 13:00:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Aarya.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/74872
to look at the new patch set (#2).
Change subject: flashrom.c:Enable erase path optimisation
......................................................................
flashrom.c:Enable erase path optimisation
Set the use_legacy_erase_path flag to flase to enable erase path
optmisatoptimisation by default.
Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22
Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com>
---
M flashrom.c
1 file changed, 14 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/74872/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/74872
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie13e43b18b20dbb956b569e554953a19eb32ea22
Gerrit-Change-Number: 74872
Gerrit-PatchSet: 2
Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-MessageType: newpatchset