December 2022 - flashrom-gerrit

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

11 Dec '22

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70327 to look at the new patch set (#2). 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> --- M internal.c 1 file changed, 44 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/27/70327/2 -- 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: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

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

11 Dec '22

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70326 to look at the new patch set (#2). 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> --- M cli_classic.c M flashrom.8.tmpl 2 files changed, 39 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/70326/2 -- 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: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

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

11 Dec '22

Evan Benn has uploaded this change for review. ( 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. Change-Id: I2c00c351904307eeb1488c5dfaffc91d6468ee25 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70026 Reviewed-by: Sam McNally <sammc(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> (cherry picked from commit c2af789c5ea5821f61fac5532d81e94742e0e00b) --- M internal.c 1 file changed, 42 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/27/70327/1 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: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

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

11 Dec '22

Evan Benn has uploaded this change for review. ( 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 Change-Id: If457e2b8548034868105b515125d7e8b4d5f6134 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70003 Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> (cherry picked from commit 8274c6321ae2ff29bc9eb0de9dc32d289d3c2cc5) --- M cli_classic.c M flashrom.8.tmpl 2 files changed, 37 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/26/70326/1 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: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

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

11 Dec '22

Evan Benn has uploaded this change for review. ( 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. Change-Id: I6e84997f910928d3973a4e5826a2d4196bdb2916 Signed-off-by: Felix Singer <felixsinger(a)posteo.net> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70242 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Alexander Goncharov <chat(a)joursoir.net> (cherry picked from commit 48e058983d6a1e36cd37bacad88f1e9ea3bd6092) --- M test_build.sh 1 file changed, 19 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/70325/1 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: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newchange

1 0

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

11 Dec '22

Evan Benn has uploaded this change for review. ( 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. Signed-off-by: Felix Singer <felixsinger(a)posteo.net> Change-Id: I340208f513bed57a9cc2bba880a2400352c5cc42 Reviewed-on: https://review.coreboot.org/c/flashrom/+/70247 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> (cherry picked from commit 8390d73806559ceb10894b7302b0dfcb3d71bd0d) --- M cbtable.c 1 file changed, 17 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/70324/1 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: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: newchange

1 0

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

11 Dec '22

Evan Benn has uploaded this change for review. ( 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. Change-Id: I96e48933f33c52c0d10a0d4cb7f7e07c1fceab99 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70342 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Nico Huber <nico.h(a)gmx.de> (cherry picked from commit 7f3c3f5c4807f4f3c0c2de405189e86b3f8d77a3) --- M flashchips.c 1 file changed, 31 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/70323/1 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: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[1.3.x]: layout: Check return values for strdup in register_include_arg
by Evan Benn (Code Review) 11 Dec '22

11 Dec '22

Evan Benn has uploaded this change for review. ( 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 Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70006 Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: David Hendricks <david.hendricks(a)gmail.com> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> (cherry picked from commit 59ce5615b768fe9d4d2506cb13c6781cd83a12e6) --- M layout.c 1 file changed, 44 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/22/70322/1 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: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

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

11 Dec '22

Evan Benn has uploaded this change for review. ( 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 Change-Id: I2a27d8f2ba42e18be2485ae95bec1b4c874bb4f7 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/70176 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> (cherry picked from commit 80408ceafc2a9a550bb5ef7aabd772dbf5d34487) --- M README 1 file changed, 33 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/70321/1 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: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: tests: Add llvm-cov option and run target for code coverage
by Anastasia Klimchuk (Code Review) 11 Dec '22

11 Dec '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/69268 ) Change subject: tests: Add llvm-cov option and run target for code coverage ...................................................................... tests: Add llvm-cov option and run target for code coverage Code coverage can be requested with -Dllvm_cov and run with ninja llvm-cov-tests or llvm-cov-cli. BUG=b:187647884 BRANCH=None TEST=meson test; ninja llvm-cov-tests TEST=ran test_build.sh with coverage enabled TEST=jenkins ran test_build.sh with coverage disabled Change-Id: Id6c73bff46e7b88d425956a80def97082b201f56 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/69268 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M Documentation/building.md M meson.build M meson_options.txt A scripts/llvm-cov M tests/meson.build 5 files changed, 49 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/Documentation/building.md b/Documentation/building.md index e25482e..e5d9412 100644 --- a/Documentation/building.md +++ b/Documentation/building.md @@ -63,12 +63,9 @@ #### llvm https://clang.llvm.org/docs/SourceBasedCodeCoverage.html ``` -env CFLAGS="-fprofile-instr-generate -fcoverage-mapping" CC=clang meson setup buildclangcov -env LLVM_PROFILE_FILE=default.profraw ninja -C buildclangcov test -cd buildclangcov -llvm-profdata merge -sparse default.profraw -o default.profdata -llvm-cov show ./flashrom_unit_tests -instr-profile=default.profdata --format=html --output-dir=. -open index.html +env CC=clang meson setup buildclangcov -Dllvm_cov=enabled +ninja -C buildclangcov test +ninja -C buildclangcov llvm-cov-tests ``` ## System specific information diff --git a/meson.build b/meson.build index 859256f..27a284e 100644 --- a/meson.build +++ b/meson.build @@ -45,6 +45,7 @@ config_default_programmer_args = get_option('default_programmer_args') cargs = [] +link_args = [] deps = [] srcs = files( '82802ab.c', @@ -539,6 +540,11 @@ cargs += '-DCONFIG_DEFAULT_PROGRAMMER_ARGS="' + config_default_programmer_args + '"' +if get_option('llvm_cov').enabled() + cargs += ['-fprofile-instr-generate', '-fcoverage-mapping'] + link_args += ['-fprofile-instr-generate', '-fcoverage-mapping'] +endif + install_headers([ 'include/libflashrom.h', ], @@ -567,7 +573,7 @@ cargs, ], install : true, - link_args : vflag, + link_args : link_args + [vflag], link_depends : mapfile, ) @@ -601,7 +607,7 @@ ) if get_option('classic_cli').auto() or get_option('classic_cli').enabled() - executable( + classic_cli = executable( 'flashrom', files( 'cli_classic.c', @@ -612,8 +618,12 @@ include_directories : include_dir, install : true, install_dir : get_option('sbindir'), + link_args : link_args, link_with : libflashrom.get_static_lib(), # flashrom needs internal symbols of libflashrom ) + if get_option('llvm_cov').enabled() + run_target('llvm-cov-cli', command : ['scripts/llvm-cov', classic_cli]) + endif endif if get_option('ich_descriptors_tool').auto() or get_option('ich_descriptors_tool').enabled() diff --git a/meson_options.txt b/meson_options.txt index 8c04029..1990d0a 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -16,3 +16,4 @@ 'pickit2_spi', 'pony_spi', 'raiden_debug_spi', 'rayer_spi', 'realtek_mst_i2c_spi', 'satamv', 'satasii', 'serprog', 'stlinkv3_spi', 'usbblaster_spi', ], description: 'Active programmers') +option('llvm_cov', type : 'feature', value : 'disabled', description : 'build for llvm code coverage') diff --git a/scripts/llvm-cov b/scripts/llvm-cov new file mode 100755 index 0000000..b512b3d --- /dev/null +++ b/scripts/llvm-cov @@ -0,0 +1,6 @@ +#!/bin/sh + +cd "${MESON_BUILD_ROOT}" +llvm-profdata merge -sparse default.profraw -o default.profdata +llvm-cov show -instr-profile=default.profdata -format=html -output-dir=. "$@" +echo "file://${MESON_BUILD_ROOT}/index.html" diff --git a/tests/meson.build b/tests/meson.build index 0d605d2..2de4ee8 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -121,7 +121,11 @@ '-U_FORTIFY_SOURCE', ], export_dynamic : true, - link_args : mocks, + link_args : mocks + link_args, dependencies : [cmocka_dep, flashrom_test_dep], ) test('cmocka test flashrom', flashrom_tests) + +if get_option('llvm_cov').enabled() + run_target('llvm-cov-tests', command : ['../scripts/llvm-cov', flashrom_tests]) +endif -- To view, visit https://review.coreboot.org/c/flashrom/+/69268 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id6c73bff46e7b88d425956a80def97082b201f56 Gerrit-Change-Number: 69268 Gerrit-PatchSet: 19 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(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: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: merged

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit December 2022