May 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: test_build.sh: Allow WARNERROR to be overridden
by Martin L Roth (Code Review) 30 May '22

30 May '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62745 ) Change subject: test_build.sh: Allow WARNERROR to be overridden ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Isn't it just repeating the Makefile this way? Does it make a difference […] I'm not sure I understand your question. I think we want warnings as errors enabled for most of the jenkins builds. I just want a way to override it. Jenkins uses this script for the build, and because it's specified on the command line inside the script, I wasn't able to change it. so that I could get a complete build with -fanalyzer turned on. I mean, we could get rid of the script completely and just hardcode each of the jenkins build tasks to do what this does if that's what you're suggesting, but then members of the project wouldn't be able to update the build task. -- To view, visit https://review.coreboot.org/c/flashrom/+/62745 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iea931e57f2a6992762566dc3dbaae8bb8df5b226 Gerrit-Change-Number: 62745 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)tutanota.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 30 May 2022 18:10:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: test_build.sh: Allow WARNERROR to be overridden
by Nico Huber (Code Review) 30 May '22

30 May '22

Attention is currently required from: Martin L Roth, Edward O'Callaghan, Anastasia Klimchuk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62745 ) Change subject: test_build.sh: Allow WARNERROR to be overridden ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: Isn't it just repeating the Makefile this way? Does it make a difference if we don't provide the argument at all? -- To view, visit https://review.coreboot.org/c/flashrom/+/62745 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iea931e57f2a6992762566dc3dbaae8bb8df5b226 Gerrit-Change-Number: 62745 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)tutanota.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Martin L Roth <gaumless(a)tutanota.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 30 May 2022 17:58:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: wrap chip->read() with a helper
by Edward O'Callaghan (Code Review) 30 May '22

30 May '22

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/64800 ) Change subject: flashrom.c: wrap chip->read() with a helper ...................................................................... flashrom.c: wrap chip->read() with a helper Change-Id: I7ebb45fba241c8d6041032df5f22fd3b03fca9f5 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c 1 file changed, 38 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/00/64800/1 diff --git a/flashrom.c b/flashrom.c index 8d6b3db..339e744 100644 --- a/flashrom.c +++ b/flashrom.c @@ -330,6 +330,35 @@ return extract_param(&programmer_param, param_name, ","); } +/* + * @brief Wrapper for flash->read() with additional high-level policy. + * + * @flash flash chip + * @buf buffer to store data in + * @start start address + * @len number of bytes to read + * @return 0 on success, + * -1 if any read fails. + * + * This wrapper simplifies most cases when the flash chip needs to be read + * since policy decisions such as non-fatal error handling is centralized. + */ +static int read_flash(struct flashctx *flash, uint8_t *buf, + unsigned int start, unsigned int len) +{ + int ret; + + if (!flash || !flash->chip->read) + return -1; + + ret = flash->chip->read(flash, buf, start, len); + if (ret) { + msg_gdbg("failed to read 0x%x-0x%x\n", start, start + len - 1); + } + + return ret; +} + static int check_block_eraser(const struct flashctx *flash, int k, int log) { struct block_eraser eraser = flash->chip->block_erasers[k]; @@ -434,7 +463,7 @@ return -1; } - int ret = flash->chip->read(flash, readbuf, start, len); + int ret = read_flash(flash, readbuf, start, len); if (ret) { msg_gerr("Verification impossible because read failed " "at 0x%x (len 0x%x)\n", start, len); @@ -1064,7 +1093,7 @@ const chipoff_t region_start = entry->start; const chipsize_t region_len = entry->end - entry->start + 1; - if (flashctx->chip->read(flashctx, buffer + region_start, region_start, region_len)) + if (read_flash(flashctx, buffer + region_start, region_start, region_len)) return 1; } return 0; @@ -1282,7 +1311,7 @@ if (info->region_start > info->erase_start) { const chipoff_t start = info->erase_start; const chipsize_t len = info->region_start - info->erase_start; - if (flashctx->chip->read(flashctx, backup_contents, start, len)) { + if (read_flash(flashctx, backup_contents, start, len)) { msg_cerr("Can't read! Aborting.\n"); goto _free_ret; } @@ -1292,7 +1321,7 @@ const chipoff_t start = info->region_end + 1; const chipoff_t rel_start = start - info->erase_start; /* within this erase block */ const chipsize_t len = info->erase_end - info->region_end; - if (flashctx->chip->read(flashctx, backup_contents + rel_start, start, len)) { + if (read_flash(flashctx, backup_contents + rel_start, start, len)) { msg_cerr("Can't read! Aborting.\n"); goto _free_ret; } @@ -1379,7 +1408,7 @@ if (info->region_start > info->erase_start) { const chipoff_t start = info->erase_start; const chipsize_t len = info->region_start - info->erase_start; - if (flashctx->chip->read(flashctx, newc, start, len)) { + if (read_flash(flashctx, newc, start, len)) { msg_cerr("Can't read! Aborting.\n"); goto _free_ret; } @@ -1390,7 +1419,7 @@ const chipoff_t start = info->region_end + 1; const chipoff_t rel_start = start - info->erase_start; /* within this erase block */ const chipsize_t len = info->erase_end - info->region_end; - if (flashctx->chip->read(flashctx, newc + rel_start, start, len)) { + if (read_flash(flashctx, newc + rel_start, start, len)) { msg_cerr("Can't read! Aborting.\n"); goto _free_ret; } @@ -1490,7 +1519,7 @@ const chipoff_t region_start = entry->start; const chipsize_t region_len = entry->end - entry->start + 1; - if (flashctx->chip->read(flashctx, curcontents + region_start, region_start, region_len)) + if (read_flash(flashctx, curcontents + region_start, region_start, region_len)) return 1; if (compare_range(newcontents + region_start, curcontents + region_start, region_start, region_len)) @@ -1985,7 +2014,7 @@ */ msg_cinfo("Reading old flash chip contents... "); if (verify_all) { - if (flashctx->chip->read(flashctx, oldcontents, 0, flash_size)) { + if (read_flash(flashctx, oldcontents, 0, flash_size)) { msg_cinfo("FAILED.\n"); goto _finalize_ret; } @@ -2005,7 +2034,7 @@ if (verify_all) { msg_cerr("Checking if anything has changed.\n"); msg_cinfo("Reading current flash chip contents... "); - if (!flashctx->chip->read(flashctx, curcontents, 0, flash_size)) { + if (!read_flash(flashctx, curcontents, 0, flash_size)) { msg_cinfo("done.\n"); if (!memcmp(oldcontents, curcontents, flash_size)) { nonfatal_help_message(); -- To view, visit https://review.coreboot.org/c/flashrom/+/64800 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ebb45fba241c8d6041032df5f22fd3b03fca9f5 Gerrit-Change-Number: 64800 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: Prepare for IFD check_access support
by Edward O'Callaghan (Code Review) 30 May '22

30 May '22

Attention is currently required from: Damien Zammit, Subrata Banik. Hello build bot (Jenkins), Damien Zammit, Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62983 to look at the new patch set (#3). Change subject: Prepare for IFD check_access support ...................................................................... Prepare for IFD check_access support Based off work in collaboration with Damien Zammit. The following is intended to allow flashrom to query the spi or opaque master for access permissions before attempting to access a given region. This comes up in cases such as the ichspi driver where the CSME can deny regions access. Change-Id: If7bdcac2a4109efbc70abc2d6ae0561f39449004 Signed-off-by: Damien Zammit <damien(a)zamaudio.com> Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c M include/chipdrivers.h M include/programmer.h M include/spi.h M opaque.c M spi.c 6 files changed, 53 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/62983/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/62983 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If7bdcac2a4109efbc70abc2d6ae0561f39449004 Gerrit-Change-Number: 62983 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Damien Zammit Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashrom.c: wip check_access erase path
by Edward O'Callaghan (Code Review) 30 May '22

30 May '22

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/63751 to look at the new patch set (#2). Change subject: flashrom.c: wip check_access erase path ...................................................................... flashrom.c: wip check_access erase path Is this needed? Change-Id: I15ec3131211548ab91be6866f48c5201c9865aa3 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c 1 file changed, 12 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/63751/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/63751 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I15ec3131211548ab91be6866f48c5201c9865aa3 Gerrit-Change-Number: 63751 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: test_build.sh: Allow WARNERROR to be overridden
by Felix Singer (Code Review) 30 May '22

30 May '22

Attention is currently required from: Nico Huber, Martin L Roth, Edward O'Callaghan, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62745 ) Change subject: test_build.sh: Allow WARNERROR to be overridden ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/62745 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iea931e57f2a6992762566dc3dbaae8bb8df5b226 Gerrit-Change-Number: 62745 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)tutanota.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.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: Martin L Roth <gaumless(a)tutanota.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 30 May 2022 07:10:25 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: test_build.sh: Allow WARNERROR to be overridden
by Martin L Roth (Code Review) 30 May '22

30 May '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62745 ) Change subject: test_build.sh: Allow WARNERROR to be overridden ...................................................................... Patch Set 2: (1 comment) File test_build.sh: https://review.coreboot.org/c/flashrom/+/62745/comment/d019a266_34ca64b7 PS1, Line 4: ${WARNERROR-yes} > Maybe just add a colon. so that it is very clear? I mean […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/62745 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iea931e57f2a6992762566dc3dbaae8bb8df5b226 Gerrit-Change-Number: 62745 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)tutanota.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 30 May 2022 04:30:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Implement read_write_status for wp
by Subrata Banik (Code Review) 30 May '22

30 May '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64540 ) Change subject: ichspi.c: Implement read_write_status for wp ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/64540 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Gerrit-Change-Number: 64540 Gerrit-PatchSet: 7 Gerrit-Owner: 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 30 May 2022 04:24:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi.c: Implement read_write_status for wp
by Edward O'Callaghan (Code Review) 30 May '22

30 May '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Hello build bot (Jenkins), Subrata Banik, Nikolai Artemiev, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/64540 to look at the new patch set (#7). Change subject: ichspi.c: Implement read_write_status for wp ...................................................................... ichspi.c: Implement read_write_status for wp The ichspi hwseq path has a opaque master specialisation that allows for reading and writing STATUS1 registers. Hook the callbacks with a implementation to allow for this so that writeprotect maybe supported though this path. Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M ichspi.c 1 file changed, 74 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/64540/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/64540 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ecbe8491ecea3697922c91af26ca62276e86317 Gerrit-Change-Number: 64540 Gerrit-PatchSet: 7 Gerrit-Owner: 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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: writeprotect.c: Allow opaque masters to hook spi_{rw}_register()
by Edward O'Callaghan (Code Review) 30 May '22

30 May '22

Attention is currently required from: Nikolai Artemiev, Sergii Dmytruk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/64375 ) Change subject: writeprotect.c: Allow opaque masters to hook spi_{rw}_register() ...................................................................... Patch Set 6: (1 comment) File spi25_statusreg.c: https://review.coreboot.org/c/flashrom/+/64375/comment/0ee45d4c_3eec94f2 PS5, Line 33: if ((flash->mst->buses_supported & BUS_PROG) && flash->mst->opaque.write_register) { : return flash->mst->opaque.write_register(flash, reg, value); : } > I am inclined to move this specalisation to actually within writeprotect. […] Ack -- To view, visit https://review.coreboot.org/c/flashrom/+/64375 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3ab0d7f5f48338c8ecb118a69651c203fbc516ac Gerrit-Change-Number: 64375 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(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: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 30 May 2022 01:52:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2022