- flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: flashrom.c: Move {read,write}_buf_from_include_args() to layout
by Edward O'Callaghan (Code Review) 22 Jan '22

22 Jan '22

Attention is currently required from: Nico Huber. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61286 ) Change subject: flashrom.c: Move {read,write}_buf_from_include_args() to layout ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > Isn't it CLI code, technically? Yes, but it is something Anastasia wanted to see what it looked like in CB:60069 . I'm more inclined to go with the simplicity of CB:60069 . -- To view, visit https://review.coreboot.org/c/flashrom/+/61286 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I61d4460ea26e07b67c99baeae5cb3840c25cb913 Gerrit-Change-Number: 61286 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Sat, 22 Jan 2022 00:08:23 +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]: flashrom: Convert do_read() into a libflashrom user
by Edward O'Callaghan (Code Review) 22 Jan '22

22 Jan '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/60430 ) Change subject: flashrom: Convert do_read() into a libflashrom user ...................................................................... flashrom: Convert do_read() into a libflashrom user Aspire towards a goal of making cli_classic more of just a user of libflashrom than having quasi-parallel paths in flashrom.c This converts the do_read() provider wrapper into a pure libflashrom user. BUG=b:208132085 TEST=`$ sudo ./flashrom -p internal -r /tmp/bios.bin` TEST=`$ sudo ./flashrom -p internal -l /tmp/layout -i FOO -r /tmp/foo.bin` Change-Id: Id2addadb891c482ee3f69da806062d7a88776675 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/60430 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashrom.c 1 file changed, 18 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index 1cdddab..257112f 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2192,13 +2192,28 @@ int do_read(struct flashctx *const flash, const char *const filename) { - if (prepare_flash_access(flash, true, false, false, false)) + int ret; + + unsigned long size = flash->chip->total_size * 1024; + unsigned char *buf = calloc(size, sizeof(unsigned char)); + if (!buf) { + msg_gerr("Memory allocation failed!\n"); return 1; + } - const int ret = read_flash_to_file(flash, filename); + ret = flashrom_image_read(flash, buf, size); + if (ret > 0) + goto free_out; - finalize_flash_access(flash); + if (write_buf_to_include_args(flash, buf)) { + ret = 1; + goto free_out; + } + if (filename) + ret = write_buf_to_file(buf, size, filename); +free_out: + free(buf); return ret; } 4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/60430 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id2addadb891c482ee3f69da806062d7a88776675 Gerrit-Change-Number: 60430 Gerrit-PatchSet: 6 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: 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-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: cli_classic.c: Convert do_erase() to libflashrom call
by Edward O'Callaghan (Code Review) 22 Jan '22

22 Jan '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/60068 ) Change subject: cli_classic.c: Convert do_erase() to libflashrom call ...................................................................... cli_classic.c: Convert do_erase() to libflashrom call Inline emergency_help_message() to cli_classic call site. This leaves do_erase() a redudant wrapper and moves us a step closer to cli_classic as a pure libflashrom user by using flashrom_flash_erase(). BUG=b:208132085 TEST=`flashrom -E` Change-Id: I8566164e7dbad69cf478b24208014f10fb99e4d0 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/60068 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M cli_classic.c M flash.h M flashrom.c 3 files changed, 14 insertions(+), 21 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/cli_classic.c b/cli_classic.c index 1b9c5ba..3cd78d4 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -840,8 +840,18 @@ ret = do_read(fill_flash, filename); else if (extract_it) ret = do_extract(fill_flash); - else if (erase_it) - ret = do_erase(fill_flash); + else if (erase_it) { + ret = flashrom_flash_erase(fill_flash); + /* + * FIXME: Do we really want the scary warning if erase failed? + * After all, after erase the chip is either blank or partially + * blank or it has the old contents. A blank chip won't boot, + * so if the user wanted erase and reboots afterwards, the user + * knows very well that booting won't work. + */ + if (ret) + emergency_help_message(); + } else if (write_it) ret = do_write(fill_flash, filename, referencefile); else if (verify_it) diff --git a/flash.h b/flash.h index 391a2d4..11e6389 100644 --- a/flash.h +++ b/flash.h @@ -344,6 +344,7 @@ int read_flash_to_file(struct flashctx *flash, const char *filename); int verify_range(struct flashctx *flash, const uint8_t *cmpbuf, unsigned int start, unsigned int len); int need_erase(const uint8_t *have, const uint8_t *want, unsigned int len, enum write_granularity gran, const uint8_t erased_value); +void emergency_help_message(void); void print_version(void); void print_buildinfo(void); void print_banner(void); @@ -355,7 +356,6 @@ void finalize_flash_access(struct flashctx *); int do_read(struct flashctx *, const char *filename); int do_extract(struct flashctx *); -int do_erase(struct flashctx *); int do_write(struct flashctx *, const char *const filename, const char *const referencefile); int do_verify(struct flashctx *, const char *const filename); int register_chip_restore(chip_restore_fn_cb_t func, struct flashctx *flash, uint8_t status); diff --git a/flashrom.c b/flashrom.c index dd443b9..1cdddab 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1553,7 +1553,7 @@ "https://www.flashrom.org/Contact for details), thanks!\n"); } -static void emergency_help_message(void) +void emergency_help_message(void) { msg_gerr("Your flash chip is in an unknown state.\n"); #if CONFIG_INTERNAL == 1 @@ -2208,23 +2208,6 @@ return do_read(flash, NULL); } -int do_erase(struct flashctx *const flash) -{ - const int ret = flashrom_flash_erase(flash); - - /* - * FIXME: Do we really want the scary warning if erase failed? - * After all, after erase the chip is either blank or partially - * blank or it has the old contents. A blank chip won't boot, - * so if the user wanted erase and reboots afterwards, the user - * knows very well that booting won't work. - */ - if (ret) - emergency_help_message(); - - return ret; -} - int do_write(struct flashctx *const flash, const char *const filename, const char *const referencefile) { const size_t flash_size = flash->chip->total_size * 1024; 5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/60068 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8566164e7dbad69cf478b24208014f10fb99e4d0 Gerrit-Change-Number: 60068 Gerrit-PatchSet: 7 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: Makeile: reenable internal programmer for mipsel
by Nico Huber (Code Review) 22 Jan '22

22 Jan '22

Attention is currently required from: Thomas Heijligen. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61300 ) Change subject: Makeile: reenable internal programmer for mipsel ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Patchset: PS3: Looks good, but it's actually called MIPSEL (don't ask me why!). (I still need to check default behavior, CONFIG_EVERYTHING etc.) -- To view, visit https://review.coreboot.org/c/flashrom/+/61300 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia607ea60c3d7d15fe231fa412595992dadc535ad Gerrit-Change-Number: 61300 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Fri, 21 Jan 2022 16:00:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess: fix build on non-x86 targets
by Nico Huber (Code Review) 22 Jan '22

22 Jan '22

Attention is currently required from: Thomas Heijligen, Paul Menzel, Peter Marheine. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61194 ) Change subject: hwaccess: fix build on non-x86 targets ...................................................................... Patch Set 4: (1 comment) Patchset: PS1: > I've now opted to put the x86 guards in the headers rather than in the files that include them because otherwise the guards would need to be duplicated across three other files. These three other files happen to be almost completely wrapped in `#if x86`, right? It's just the #include that needs to be guarded as well, I guess (beside the one call to rget_io_perms()). Let's better change those files that are already a #if mess and not those that we cleaned up only just. Mid-term we should separate the source code further, i.e. have x86 code only in files compiled for x86, then we won't need no more #if. -- To view, visit https://review.coreboot.org/c/flashrom/+/61194 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I20f122679c30340b2c73afd7419e79644ddc3c4e Gerrit-Change-Number: 61194 Gerrit-PatchSet: 4 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> 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-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 21 Jan 2022 15:53:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makeile: reenable internal programmer for mipsel
by Thomas Heijligen (Code Review) 22 Jan '22

22 Jan '22

Attention is currently required from: Nico Huber. Hello build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/61300 to look at the new patch set (#3). Change subject: Makeile: reenable internal programmer for mipsel ...................................................................... Makeile: reenable internal programmer for mipsel Make CONFIG_INTERNAL an alias for CONFIG_INTERNAL_X86 or CONFIG_INTERNAL_MIPS on the respective platforms. This allows for handling different system dependencies. Change-Id: Ia607ea60c3d7d15fe231fa412595992dadc535ad Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile 1 file changed, 25 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/00/61300/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/61300 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia607ea60c3d7d15fe231fa412595992dadc535ad Gerrit-Change-Number: 61300 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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-MessageType: newpatchset

1 0

Change in flashrom[master]: meson: build hwaccess only on x86
by Thomas Heijligen (Code Review) 22 Jan '22

22 Jan '22

Thomas Heijligen has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/61198 ) Change subject: meson: build hwaccess only on x86 ...................................................................... Abandoned In favor of CB:61287 -- To view, visit https://review.coreboot.org/c/flashrom/+/61198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iafabe8b4b2a95697773a739501dfc62d880d3f6d Gerrit-Change-Number: 61198 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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: Peter Marheine <pmarheine(a)chromium.org> 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-MessageType: abandon

1 0

Change in flashrom[master]: meson: sync programmer dependencies from Makefile
by Thomas Heijligen (Code Review) 22 Jan '22

22 Jan '22

Attention is currently required from: Peter Marheine. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61287 ) Change subject: meson: sync programmer dependencies from Makefile ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/61287 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iae93111fd48865f3fe8dd0eb637349b9a0c4affc Gerrit-Change-Number: 61287 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> 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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 21 Jan 2022 14:54:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess: fix build on non-x86 targets
by Thomas Heijligen (Code Review) 22 Jan '22

22 Jan '22

Attention is currently required from: Paul Menzel, Peter Marheine. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61194 ) Change subject: hwaccess: fix build on non-x86 targets ...................................................................... Patch Set 4: Code-Review+1 (3 comments) Patchset: PS4: I've updated the Makefile for building the internal programmer on mipsle. On other platforms the internal programmer will fail to initialize. File hwaccess_x86_io.h: https://review.coreboot.org/c/flashrom/+/61194/comment/4d66f73d_887aa833 PS4, Line 41: #define HAVE_RGET_IO_PERMS 1 not needed File internal.c: https://review.coreboot.org/c/flashrom/+/61194/comment/18d245b4_9bf7bd3a PS4, Line 245: if (rget_io_perms()) { You can move this under the x86 guard at line 275. No need for HAVE_RGET_IO_PERMS -- To view, visit https://review.coreboot.org/c/flashrom/+/61194 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I20f122679c30340b2c73afd7419e79644ddc3c4e Gerrit-Change-Number: 61194 Gerrit-PatchSet: 4 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> 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-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 21 Jan 2022 14:52:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makeile: reenable internal programmer for mipsel
by Thomas Heijligen (Code Review) 22 Jan '22

22 Jan '22

Attention is currently required from: Nico Huber. Hello build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/61300 to look at the new patch set (#2). Change subject: Makeile: reenable internal programmer for mipsel ...................................................................... Makeile: reenable internal programmer for mipsel Make CONFIG_INTERNAL an alias for CONFIG_INTERNAL_X86 or CONFIG_INTERNAL_MIPS on the respective platforms. This allows for handling different system dependencies like DEPENDS_ON_X86_IO without the need to have an empty implementaion of those. Change-Id: Ia607ea60c3d7d15fe231fa412595992dadc535ad Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile 1 file changed, 25 insertions(+), 13 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/00/61300/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/61300 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia607ea60c3d7d15fe231fa412595992dadc535ad Gerrit-Change-Number: 61300 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> 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-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit