Attention is currently required from: Felix Singer, Thomas Heijligen.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71119 )
Change subject: flashrom.c: Add switch for legacy impl of erasure path
......................................................................
Patch Set 13: -Code-Review
--
To view, visit https://review.coreboot.org/c/flashrom/+/71119
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ib5660db0067c1c799dcb5c8e83b4a4826b236442
Gerrit-Change-Number: 71119
Gerrit-PatchSet: 13
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: 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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Thu, 26 Jan 2023 07:43:35 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/72454 )
Change subject: flashrom.c: Fix skip flag typo in read_flash()
......................................................................
flashrom.c: Fix skip flag typo in read_flash()
Only picked up by internal test infra in ChromeOS when
instrumenting futility(1).
The following error was incured,
```
>> Starting firmware updater.
>> Target image: images/bios-kindred.ro-12672-141-0.rw-12672-141-0.bin (RO:Google_Kindred.12672.141.0, RW/A:Google_Kindred.12672.141.0, RW/B:Google_Kindred.12672.141.0).
INFO: update_firmware: Loading current system firmware...
INFO: load_system_firmware: flashrom -r <IMAGE> -p host
Warning: Setting BIOS Control at 0xdc from 0x8b to 0x89 failed.
New value is 0x8b.
At least some flash regions are read protected. You have to use a flash
layout and include only accessible regions. For write operations, you'll
additionally need the --noverify-all switch. See manpage for more details.
read_flash: cannot read inside Management Engine region (0x001000..0x3fffff).
Read operation failed!
ERROR: do_update: Cannot load system active firmware.
```
despite the appropriate flag being set within futility(1).
BUG=b:266748702
TEST=cros fw updates.
Change-Id: Ie04cd62020ca29775fc66a81d2fadd32a5aab5cd
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M flashrom.c
1 file changed, 34 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/54/72454/1
diff --git a/flashrom.c b/flashrom.c
index 67b71f0..f873300 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -573,7 +573,7 @@
uint8_t *rbuf = buf + addr - start;
if (region.read_prot) {
- if (flash->flags.skip_unwritable_regions) {
+ if (flash->flags.skip_unreadable_regions) {
msg_gdbg("%s: cannot read inside %s region (%#08x..%#08x), "
"filling (%#08x..%#08x) with erased value instead.\n",
__func__, region.name, region.start, region.end - 1,
--
To view, visit https://review.coreboot.org/c/flashrom/+/72454
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ie04cd62020ca29775fc66a81d2fadd32a5aab5cd
Gerrit-Change-Number: 72454
Gerrit-PatchSet: 1
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: newchange
Attention is currently required from: Nikolai Artemiev.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70514 )
Change subject: flashrom: Check for flash access restricitons in read_flash()
......................................................................
Patch Set 5:
(1 comment)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/70514/comment/b2a98682_2ae9a5ad
PS5, Line 550: unwritable
shouldn't this be 'unreadable' based on the commit message, this CB:70128 and the identifier?
--
To view, visit https://review.coreboot.org/c/flashrom/+/70514
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I22c795d7d08ef8bf773733d9952967b2fa2ef299
Gerrit-Change-Number: 70514
Gerrit-PatchSet: 5
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Wed, 25 Jan 2023 23:35:53 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev.
Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69517 )
Change subject: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking
......................................................................
Patch Set 14: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/69517
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I74b3f5d3a17749ea60485b916b2d87467a5d8b2f
Gerrit-Change-Number: 69517
Gerrit-PatchSet: 14
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.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-Comment-Date: Wed, 25 Jan 2023 10:40:16 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Alexander Goncharov.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72408 )
Change subject: dummyflasher: use new API to register shutdown function
......................................................................
Patch Set 2:
(1 comment)
File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/72408/comment/5b9de1af_4a670d17
PS2, Line 910:
What if you try:
if (!emu_data)
return 0;
The reason I thought about it, is that on first run of shutdown emu_data is freed. And then emu_data->anything is going to segfault right?
emu_data is shared between masters if multiple are registered.
hope I am not missing anything
The good thing about dummyflasher, it's [relatively] easy to add new unit tests for it. If there is any combination of buses that is not covered by unit test, it can be added, can be added in this patch. And then the best thing is: unit tests can fully test this patch!
and unit tests are checking there is no memory leaks.
--
To view, visit https://review.coreboot.org/c/flashrom/+/72408
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I0c67c25b0f53cd8c564c4ea0f09f2728e856f6ea
Gerrit-Change-Number: 72408
Gerrit-PatchSet: 2
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Wed, 25 Jan 2023 10:34:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Alexander Goncharov.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72430 )
Change subject: dummyflasher: fix propagation of register_*_master() return values
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/72430
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3a7eeb3cdd814db18b0717ae8b40ecadb4c32f7c
Gerrit-Change-Number: 72430
Gerrit-PatchSet: 1
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Wed, 25 Jan 2023 10:27:09 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment