- flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[master]: flashrom.c:Add function to align region to sector boundaries
by Edward O'Callaghan (Code Review) 14 Dec '22

14 Dec '22

Attention is currently required from: Aarya. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65642 ) Change subject: flashrom.c:Add function to align region to sector boundaries ...................................................................... Patch Set 61: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/65642/comment/fc07a04d_9de4964c PS57, Line 1183: layout[i].layout_list[j] > I feel this way the code is more easy to understand -- jth block according to ith erase function. easier because you wrote it ;) The intermediates give meaning and types to otherwise very deep and long predicates with much indirection (many pointer dereferences) embedded within. -- To view, visit https://review.coreboot.org/c/flashrom/+/65642 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I215ea4986aa23360fc65ff761f4e49c6069160ac Gerrit-Change-Number: 65642 Gerrit-PatchSet: 61 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Simon Buhrow 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Tue, 13 Dec 2022 23:48:05 +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

1 0

[M] Change in flashrom[master]: flashrom.c:Add function to get a flattened view of the chip erase blocks
by Edward O'Callaghan (Code Review) 14 Dec '22

14 Dec '22

Attention is currently required from: Simon Buhrow, Nico Huber, Thomas Heijligen, Paul Menzel, Aarya. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65879 ) Change subject: flashrom.c:Add function to get a flattened view of the chip erase blocks ...................................................................... Patch Set 45: (2 comments) File flashrom.c: https://review.coreboot.org/c/flashrom/+/65879/comment/c05da115_8ec82c6e PS41, Line 1237: : struct erase_layout *create_erase_layout(struct flashctx *const flashctx); : > These prototyes here are temporary until the functions are used. […] I can see what the prototypes are being used for but I don't think this is a good manner to deploy a large change. You could create a `erasure_layout.c` object and have a header for that and have the entry-points in `flashrom.c` switch between the old static implementation or the new one inside `erasure_layout.c`. Adding one function per commit that doesn't get called serves no real purpose. File flashrom.c: https://review.coreboot.org/c/flashrom/+/65879/comment/6c1a8064_12b100c2 PS42, Line 1249: index > Yeah `i` and `index` won't always be same. maybe a better identifier than "index" and short hand for "i"ndex then? -- To view, visit https://review.coreboot.org/c/flashrom/+/65879 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iafe78de00daa55f7114bd4ce09465dd88074ece4 Gerrit-Change-Number: 65879 Gerrit-PatchSet: 45 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Simon Buhrow 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Tue, 13 Dec 2022 23:44:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Simon Buhrow Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[1.3.x]: flashchips.c: remove WREN from GD25Q256D enter 4BA sequence
by Anastasia Klimchuk (Code Review) 13 Dec '22

13 Dec '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/70323 ) Change subject: flashchips.c: remove WREN from GD25Q256D enter 4BA sequence ...................................................................... flashchips.c: remove WREN from GD25Q256D enter 4BA sequence As noted in a comment on `commit 86fc9cf7ab221bc54ef6f10252e296fc2d7a22d2`, the GD25Q256D datasheet indicates that the chip does not require a WREN command to enter 4BA mode. Testing has confirmed that a WREN command is not required, so change the flashchip feature flags from FEATURE_4BA_WREN to FEATURE_4BA. Ticket: https://ticket.coreboot.org/issues/356 BUG=none BRANCH=none TEST=read/write/erase/verify GD25Q256D flash with FT2232H programmer TEST=called spi_enter_exit_4ba(true), dumped registers, checked ADS=1. Original-Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/70342 Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Original-Reviewed-by: Nico Huber <nico.h(a)gmx.de> (cherry picked from commit 7f3c3f5c4807f4f3c0c2de405189e86b3f8d77a3) Change-Id: I96e48933f33c52c0d10a0d4cb7f7e07c1fceab99 Signed-off-by: Evan Benn <evanbenn(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70323 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M flashchips.c 1 file changed, 38 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved Angel Pons: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index 76f3fbb..9d5b22c 100644 --- a/flashchips.c +++ b/flashchips.c @@ -6753,7 +6753,7 @@ .model_id = GIGADEVICE_GD25Q256D, .total_size = 32768, .page_size = 256, - .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_WREN | + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA | FEATURE_WRSR_EXT2 | FEATURE_WRSR2 | FEATURE_WRSR3, .tested = TEST_OK_PREWB, .probe = PROBE_SPI_RDID, -- To view, visit https://review.coreboot.org/c/flashrom/+/70323 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: I96e48933f33c52c0d10a0d4cb7f7e07c1fceab99 Gerrit-Change-Number: 70323 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[1.3.x]: layout: Check return values for strdup in register_include_arg
by Anastasia Klimchuk (Code Review) 13 Dec '22

13 Dec '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/70322 ) Change subject: layout: Check return values for strdup in register_include_arg ...................................................................... layout: Check return values for strdup in register_include_arg strdup return values should be checked for NULL to catch the potential error case of out of memory. This patch re-writes ternary conditionals so that strdup return values could be checked for all branches fof execution. Follow up on commit 45d50a101e8073191e6d88143990ed91d3bfe815 Ticket: https://ticket.coreboot.org/issues/372 Original-Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/70006 Original-Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Original-Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> (cherry picked from commit 59ce5615b768fe9d4d2506cb13c6781cd83a12e6) Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28 Signed-off-by: Evan Benn <evanbenn(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70322 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M layout.c 1 file changed, 51 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved Angel Pons: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/layout.c b/layout.c index be88428..a1bb147 100644 --- a/layout.c +++ b/layout.c @@ -120,7 +120,7 @@ struct layout_include_args *tmp; char *colon; char *name; - char *file; + char *file = NULL; /* file is optional, so defaults to NULL */ if (arg == NULL) { msg_gerr("<NULL> is a bad region name.\n"); @@ -133,8 +133,22 @@ msg_gerr("Missing filename parameter in %s\n", arg); return 1; } - name = colon ? strndup(arg, colon - arg) : strdup(arg); - file = colon ? strdup(colon + 1) : NULL; + + if (colon) { + name = strndup(arg, colon - arg); + if (!name) { + msg_gerr("Out of memory"); + goto error; + } + + file = strdup(colon + 1); + if (!file) { + msg_gerr("Out of memory"); + goto error; + } + } else { + name = strdup(arg); + } for (tmp = *args; tmp; tmp = tmp->next) { if (!strcmp(tmp->name, name)) { -- To view, visit https://review.coreboot.org/c/flashrom/+/70322 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28 Gerrit-Change-Number: 70322 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[1.3.x]: cli, manpage: Clean up occurrences of old image argument
by Anastasia Klimchuk (Code Review) 13 Dec '22

13 Dec '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/70326 ) Change subject: cli, manpage: Clean up occurrences of old image argument ...................................................................... cli, manpage: Clean up occurrences of old image argument --include argument was introduced and replaced --image argument in commit 45d50a101e8073191e6d88143990ed91d3bfe815 This patch cleans up remaining few places where old `--image` argument was mentioned so that now all the documentation has `--include`. --image is deprecated. Both old --image and new --include have the same short version -i and it remains the same. The code remains the same since the code handles --include already. Tested by running flashrom -h man ./flashrom.8.tmpl Ticket: https://ticket.coreboot.org/issues/372 Original-Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/70003 Original-Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Original-Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> (cherry picked from commit 8274c6321ae2ff29bc9eb0de9dc32d289d3c2cc5) Change-Id: If457e2b8548034868105b515125d7e8b4d5f6134 Signed-off-by: Evan Benn <evanbenn(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70326 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M cli_classic.c M flashrom.8.tmpl 2 files changed, 44 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve Anastasia Klimchuk: Looks good to me, approved diff --git a/cli_classic.c b/cli_classic.c index 4649c73..4f4f074 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -71,8 +71,9 @@ " --fmap read ROM layout from fmap embedded in ROM\n" " --fmap-file <fmapfile> read ROM layout from fmap in <fmapfile>\n" " --ifd read layout from an Intel Firmware Descriptor\n" - " -i | --image <region>[:<file>] only read/write image <region> from layout\n" + " -i | --include <region>[:<file>] only read/write image <region> from layout\n" " (optionally with data from <file>)\n" + " --image <region>[:<file>] deprecated, please use --include\n" " -o | --output <logfile> log output to <logfile>\n" " --flash-contents <ref-file> assume flash contents to be <ref-file>\n" " -L | --list-supported print supported devices\n" @@ -613,7 +614,8 @@ {"ifd", 0, NULL, OPTION_IFD}, {"fmap", 0, NULL, OPTION_FMAP}, {"fmap-file", 1, NULL, OPTION_FMAP_FILE}, - {"image", 1, NULL, 'i'}, + {"image", 1, NULL, 'i'}, // (deprecated): back compatibility. + {"include", 1, NULL, 'i'}, {"flash-contents", 1, NULL, OPTION_FLASH_CONTENTS}, {"flash-name", 0, NULL, OPTION_FLASH_NAME}, {"flash-size", 0, NULL, OPTION_FLASH_SIZE}, diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index b977908..4bb9683 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -49,7 +49,7 @@ (\fB\-\-flash\-name\fR|\fB\-\-flash\-size\fR| [\fB\-E\fR|\fB\-x\fR|\fB\-r\fR <file>|\fB\-w\fR <file>|\fB\-v\fR <file>] [(\fB\-l\fR <file>|\fB\-\-ifd\fR|\fB \-\-fmap\fR|\fB\-\-fmap-file\fR <file>) - [\fB\-i\fR <image>[:<file>]]] + [\fB\-i\fR <include>[:<file>]]] [\fB\-\-wp\-status\fR] [\fB\-\-wp\-list\fR] [\fB\-\-wp\-enable\fR|\fB\-\-wp\-disable\fR] [\fB\-\-wp\-range\fR <start>,<length>|\fB\-\-wp\-region\fR <region>] [\fB\-n\fR] [\fB\-N\fR] [\fB\-f\fR])] -- To view, visit https://review.coreboot.org/c/flashrom/+/70326 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: If457e2b8548034868105b515125d7e8b4d5f6134 Gerrit-Change-Number: 70326 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[1.3.x]: cbtables.c/search_lb_records: Drop unused variable `count`
by Anastasia Klimchuk (Code Review) 13 Dec '22

13 Dec '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/70324 ) Change subject: cbtables.c/search_lb_records: Drop unused variable `count` ...................................................................... cbtables.c/search_lb_records: Drop unused variable `count` Clang 15 complains about it. Remove it. Original-Signed-off-by: Felix Singer <felixsinger(a)posteo.net> Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/70247 Original-Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Original-Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> (cherry picked from commit 8390d73806559ceb10894b7302b0dfcb3d71bd0d) Change-Id: I340208f513bed57a9cc2bba880a2400352c5cc42 Signed-off-by: Evan Benn <evanbenn(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70324 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M cbtable.c 1 file changed, 24 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified David Hendricks: Looks good to me, approved Angel Pons: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/cbtable.c b/cbtable.c index 2b339d5..38f9cbe 100644 --- a/cbtable.c +++ b/cbtable.c @@ -295,13 +295,10 @@ static void search_lb_records(struct lb_record *rec, struct lb_record *last, unsigned long addr) { struct lb_record *next; - int count; - count = 0; for (next = next_record(rec); (rec < last) && (next <= last); rec = next, addr += rec->size) { next = next_record(rec); - count++; if (rec->tag == LB_TAG_MAINBOARD) { find_mainboard(rec, addr); break; -- To view, visit https://review.coreboot.org/c/flashrom/+/70324 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: I340208f513bed57a9cc2bba880a2400352c5cc42 Gerrit-Change-Number: 70324 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[1.3.x]: README: Add information about meson and link build instructions
by Anastasia Klimchuk (Code Review) 13 Dec '22

13 Dec '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/70321 ) Change subject: README: Add information about meson and link build instructions ...................................................................... README: Add information about meson and link build instructions The patch adds one paragraph of information about meson into the README file. This meant to be the minimum required to unblock release candidate. README file will have a more substantial upgrade soon. Ticket: https://ticket.coreboot.org/issues/354 Original-Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/70176 Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> (cherry picked from commit 80408ceafc2a9a550bb5ef7aabd772dbf5d34487) Change-Id: I2a27d8f2ba42e18be2485ae95bec1b4c874bb4f7 Signed-off-by: Evan Benn <evanbenn(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70321 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M README 1 file changed, 39 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/README b/README index 49af09e..d00524e 100644 --- a/README +++ b/README @@ -46,6 +46,18 @@ Build Instructions ------------------ +flashrom supports building with make and meson. + +Meson build system supports almost all the environments, although not exactly +all of them. Full meson support is on the roadmap in the nearest future. +To build flashrom with meson, follow the instruction and information in +/Documentation/building.md + +If you are unsure which build system to use, and/or don't know what's the +difference, use make for now. + +The rest of Build Instructions below refers to building flashrom with make. + To build flashrom you need to install the following software: * C compiler (GCC / clang) -- To view, visit https://review.coreboot.org/c/flashrom/+/70321 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: I2a27d8f2ba42e18be2485ae95bec1b4c874bb4f7 Gerrit-Change-Number: 70321 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[1.3.x]: internal.c: laptop_ok global state can become stale
by Anastasia Klimchuk (Code Review) 13 Dec '22

13 Dec '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/70327 ) Change subject: internal.c: laptop_ok global state can become stale ...................................................................... internal.c: laptop_ok global state can become stale Craask and similar DUT's are erroneously probing random second chips. ``` Found chipset "Intel Alder Lake-N". Enabling flash write... SPI Configuration is locked down. FREG0: Flash Descriptor region (0x00000000-0x00000fff) is read-write. FREG1: BIOS region (0x003a0000-0x00ffffff) is read-write. FREG2: Management Engine region (0x00001000-0x0039ffff) is read-write. OK. Found Winbond flash chip "W25Q128.V..M" (16384 kB, Programmer-specific) on host. Warning: Setting BIOS Control at 0xdc from 0x8b to 0x89 failed. New value is 0x8b. Found MoselVitelic flash chip "V29C51000T" (64 kB, Parallel) mapped at physical address 0x00000000ffff0000. ``` This seems to be due to `laptop_ok` becoming a stale global state after the first operation leading to probing on unrelated buses. Therefore unconditionally reset the global state upon entry into the internal driver. BUG=b:260518132,b:260151917 TEST=Craask reportly no longer finds duplicate chip. Original-Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/70026 Original-Reviewed-by: Sam McNally <sammc(a)google.com> Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> (cherry picked from commit c2af789c5ea5821f61fac5532d81e94742e0e00b) Change-Id: I2c00c351904307eeb1488c5dfaffc91d6468ee25 Signed-off-by: Evan Benn <evanbenn(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70327 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M internal.c 1 file changed, 48 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/internal.c b/internal.c index 8c834e2..85512f4 100644 --- a/internal.c +++ b/internal.c @@ -211,6 +211,9 @@ if (ret) return ret; + /* Unconditionally reset global state from previous operation. */ + laptop_ok = false; + /* Default to Parallel/LPC/FWH flash devices. If a known host controller * is found, the host controller init routine sets the * internal_buses_supported bitfield. -- To view, visit https://review.coreboot.org/c/flashrom/+/70327 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: I2c00c351904307eeb1488c5dfaffc91d6468ee25 Gerrit-Change-Number: 70327 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[1.3.x]: test_build.sh: Switch to meson setup <dir>
by Anastasia Klimchuk (Code Review) 13 Dec '22

13 Dec '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/70325 ) Change subject: test_build.sh: Switch to meson setup <dir> ...................................................................... test_build.sh: Switch to meson setup <dir> Running the setup command as `meson [options]` instead of `meson setup [options]` is ambiguous and deprecated. Original-Signed-off-by: Felix Singer <felixsinger(a)posteo.net> Original-Reviewed-on: https://review.coreboot.org/c/flashrom/+/70242 Original-Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Original-Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Original-Reviewed-by: Alexander Goncharov <chat(a)joursoir.net> (cherry picked from commit 48e058983d6a1e36cd37bacad88f1e9ea3bd6092) Change-Id: I6e84997f910928d3973a4e5826a2d4196bdb2916 Signed-off-by: Evan Benn <evanbenn(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70325 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M test_build.sh 1 file changed, 25 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Angel Pons: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/test_build.sh b/test_build.sh index 813d0b9..c2f74d2 100755 --- a/test_build.sh +++ b/test_build.sh @@ -65,7 +65,7 @@ return fi - meson ${programmer_dir} ${meson_opts} -Dprogrammer=${programmer} + meson setup ${programmer_dir} ${meson_opts} -Dprogrammer=${programmer} ninja ${ninja_opts} -C ${programmer_dir} ninja ${ninja_opts} -C ${programmer_dir} test done -- To view, visit https://review.coreboot.org/c/flashrom/+/70325 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: 1.3.x Gerrit-Change-Id: I6e84997f910928d3973a4e5826a2d4196bdb2916 Gerrit-Change-Number: 70325 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[master]: flashrom.c: Plumb 'all_skipped' global state into func param
by Aarya (Code Review) 13 Dec '22

13 Dec '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67093 ) Change subject: flashrom.c: Plumb 'all_skipped' global state into func param ...................................................................... Patch Set 24: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/67093/comment/d51054ce_bfb895ee PS4, Line 11: > Maybe you did it already: I think this can be tested with dummyflasher by writing the same image twi […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/67093 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2346c869c47b48604360b0facf9313aae086c8dd Gerrit-Change-Number: 67093 Gerrit-PatchSet: 24 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 13 Dec 2022 13:44:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit