December 2022 - flashrom-gerrit

[M] Change in flashrom[master]: tree/: Change chip restore data type from uint8_t to void ptr
by Nikolai Artemiev (Code Review) 05 Dec '22

05 Dec '22

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70349 to look at the new patch set (#4). Change subject: tree/: Change chip restore data type from uint8_t to void ptr ...................................................................... tree/: Change chip restore data type from uint8_t to void ptr Chip restore callbacks currently are used by - spi25_statusreg.c unlock functions to restore status register 1. - s25f.c to restore config register 3. Both of these cases only need to save a single uint8_t value to restore the original chip state, however storing a void pointer will allow more flexible chip restore behaviour. In particular, it will allow flashrom_wp_cfg objects to be saved and restored, enabling writeprotect-based unlocking. BUG=b:237485865,b:247421511 BRANCH=none TEST=todo Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c M include/flash.h M s25f.c M spi25_statusreg.c 4 files changed, 52 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/70349/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/70349 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Gerrit-Change-Number: 70349 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tree/: Make chip restore data a void ptr instead of uint8_t
by Nikolai Artemiev (Code Review) 05 Dec '22

05 Dec '22

Nikolai Artemiev has uploaded a new patch set (#3). ( https://review.coreboot.org/c/flashrom/+/70349 ) Change subject: tree/: Make chip restore data a void ptr instead of uint8_t ...................................................................... tree/: Make chip restore data a void ptr instead of uint8_t Chip restore callbacks currently are used by - spi25_statusreg.c unlock functions to restore status register 1. - s25f.c to restore config register 3. Both of these cases only need to save a single uint8_t value to restore the original chip state, however storing a void pointer will allow more powerful chip restore behaviour and enable writeprotect-based unlocking as flashrom_wp_cfg objects will be able to be saved and restored. BUG=b:237485865,b:247421511 BRANCH=none TEST=todo Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c M include/flash.h M s25f.c M spi25_statusreg.c 4 files changed, 51 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/70349/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/70349 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Gerrit-Change-Number: 70349 Gerrit-PatchSet: 3 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking
by Nikolai Artemiev (Code Review) 05 Dec '22

05 Dec '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Nikolai Artemiev has uploaded a new patch set (#4) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/69517 ) Change subject: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking ...................................................................... flashrom.c: Supplement `chip->unlock()` calls with wp unlocking The full writeprotect implementation has proper support and ability to unlock flash over spi25_statusreg.c. Therefore if the required bits are available for the given chip prefer proper writeprotect support instead of adhoc spi25_statusreg.c helpers. BUG=b:237485865 BRANCH=none TEST=make Change-Id: I74b3f5d3a17749ea60485b916b2d87467a5d8b2f CoAuthored-by: Nikolai Artemiev <nartemiev(a)google.com> Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c M include/flash.h 2 files changed, 70 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/69517/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/69517 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I74b3f5d3a17749ea60485b916b2d87467a5d8b2f Gerrit-Change-Number: 69517 Gerrit-PatchSet: 4 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tree/: Make chip restore data a void ptr instead of uint8_t
by Nikolai Artemiev (Code Review) 05 Dec '22

05 Dec '22

Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/70349 ) Change subject: tree/: Make chip restore data a void ptr instead of uint8_t ...................................................................... tree/: Make chip restore data a void ptr instead of uint8_t Chip restore callbacks currently are used by - spi25_statusreg.c unlock functions to restore status register 1. - s25f.c to restore config register 3. Both of these cases only need to save a single uint8_t value to restore the original chip state, however storing a void pointer will allow more powerful chip restore behaviour and enable writeprotect-based unlocking as flashrom_wp_cfg objects will be able to be saved and restored. BUG=b:237485865,b:247421511 BRANCH=none TEST=todo Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c M include/flash.h M s25f.c M spi25_statusreg.c 4 files changed, 51 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/70349/1 diff --git a/flashrom.c b/flashrom.c index 51a6b64..43410ea 100644 --- a/flashrom.c +++ b/flashrom.c @@ -97,7 +97,7 @@ } int register_chip_restore(chip_restore_fn_cb_t func, - struct flashctx *flash, uint8_t status) + struct flashctx *flash, void *data) { if (flash->chip_restore_fn_count >= MAX_CHIP_RESTORE_FUNCTIONS) { msg_perr("Tried to register more than %i chip restore" @@ -105,7 +105,7 @@ return 1; } flash->chip_restore_fn[flash->chip_restore_fn_count].func = func; - flash->chip_restore_fn[flash->chip_restore_fn_count].status = status; + flash->chip_restore_fn[flash->chip_restore_fn_count].data = data; flash->chip_restore_fn_count++; return 0; @@ -118,7 +118,7 @@ while (flash->chip_restore_fn_count > 0) { int i = --flash->chip_restore_fn_count; rc |= flash->chip_restore_fn[i].func( - flash, flash->chip_restore_fn[i].status); + flash, flash->chip_restore_fn[i].data); } return rc; diff --git a/include/flash.h b/include/flash.h index 3f78721..1059c28 100644 --- a/include/flash.h +++ b/include/flash.h @@ -509,7 +509,7 @@ enum decode_range_func decode_range; }; -typedef int (*chip_restore_fn_cb_t)(struct flashctx *flash, uint8_t status); +typedef int (*chip_restore_fn_cb_t)(struct flashctx *flash, void *data); struct flashrom_flashctx { struct flashchip *chip; @@ -542,7 +542,7 @@ int chip_restore_fn_count; struct chip_restore_func_data { chip_restore_fn_cb_t func; - uint8_t status; + void *data; } chip_restore_fn[MAX_CHIP_RESTORE_FUNCTIONS]; /* Progress reporting */ flashrom_progress_callback *progress_callback; @@ -616,7 +616,7 @@ int write_buf_to_file(const unsigned char *buf, unsigned long size, const char *filename); int prepare_flash_access(struct flashctx *, bool read_it, bool write_it, bool erase_it, bool verify_it); void finalize_flash_access(struct flashctx *); -int register_chip_restore(chip_restore_fn_cb_t func, struct flashctx *flash, uint8_t status); +int register_chip_restore(chip_restore_fn_cb_t func, struct flashctx *flash, void *data); /* Something happened that shouldn't happen, but we can go on. */ #define ERROR_FLASHROM_NONFATAL 0x100 diff --git a/s25f.c b/s25f.c index 605a560..55c59b0 100644 --- a/s25f.c +++ b/s25f.c @@ -21,6 +21,7 @@ * TODO: Implement fancy hybrid sector architecture helpers. */ +#include <stdlib.h> #include <string.h> #include "chipdrivers.h" @@ -230,10 +231,13 @@ return s25f_poll_status(flash); } -static int s25fs_restore_cr3nv(struct flashctx *flash, uint8_t cfg) +static int s25fs_restore_cr3nv(struct flashctx *flash, void *data) { int ret = 0; + uint8_t cfg = *(uint8_t *)data; + free(data); + msg_cdbg("Restoring CR3NV value to 0x%02x\n", cfg); ret |= s25fs_write_cr(flash, CR3NV_ADDR, cfg); ret |= s25fs_software_reset(flash); @@ -285,8 +289,13 @@ msg_cdbg("\n%s: CR3NV updated (0x%02x -> 0x%02x)\n", __func__, cfg, s25fs_read_cr(flash, CR3NV_ADDR)); + /* Restore CR3V when flashrom exits */ - register_chip_restore(s25fs_restore_cr3nv, flash, cfg); + uint8_t *data = calloc(sizeof(uint8_t), 1); + if (!data) + return 1; + *data = cfg; + register_chip_restore(s25fs_restore_cr3nv, flash, data); } cr3nv_checked = 1; diff --git a/spi25_statusreg.c b/spi25_statusreg.c index b178b2e..9266664 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -17,6 +17,8 @@ * GNU General Public License for more details. */ +#include <stdlib.h> + #include "flash.h" #include "chipdrivers.h" #include "spi.h" @@ -252,8 +254,11 @@ return 0; } -static int spi_restore_status(struct flashctx *flash, uint8_t status) +static int spi_restore_status(struct flashctx *flash, void *data) { + uint8_t status = *(uint8_t *)data; + free(data); + msg_cdbg("restoring chip status (0x%02x)\n", status); return spi_write_register(flash, STATUS1, status); } @@ -293,7 +298,11 @@ } /* Restore status register content upon exit in finalize_flash_access(). */ - register_chip_restore(spi_restore_status, flash, status); + uint8_t *data = calloc(sizeof(uint8_t), 1); + if (!data) + return 1; + *data = status; + register_chip_restore(spi_restore_status, flash, data); msg_cdbg("Some block protection in effect, disabling... "); if ((status & lock_mask) != 0) { -- To view, visit https://review.coreboot.org/c/flashrom/+/70349 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Gerrit-Change-Number: 70349 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking
by Nikolai Artemiev (Code Review) 05 Dec '22

05 Dec '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Nikolai Artemiev has uploaded a new patch set (#3) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/69517 ) Change subject: flashrom.c: Supplement `chip->unlock()` calls with wp unlocking ...................................................................... flashrom.c: Supplement `chip->unlock()` calls with wp unlocking The full writeprotect implementation has proper support and ability to unlock flash over spi25_statusreg.c. Therefore if the required bits are available for the given chip prefer proper writeprotect support instead of adhoc spi25_statusreg.c helpers. BUG=b:237485865 BRANCH=none TEST=make Change-Id: I74b3f5d3a17749ea60485b916b2d87467a5d8b2f CoAuthored-by: Nikolai Artemiev <nartemiev(a)google.com> Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c M include/flash.h 2 files changed, 76 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/69517/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/69517 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I74b3f5d3a17749ea60485b916b2d87467a5d8b2f Gerrit-Change-Number: 69517 Gerrit-PatchSet: 3 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashchips.c: remove WREN from GD25Q256D enter 4BA sequence
by Nico Huber (Code Review) 05 Dec '22

05 Dec '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70342 ) Change subject: flashchips.c: remove WREN from GD25Q256D enter 4BA sequence ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/70342/comment/89a52696_7227994b PS1, Line 19: FT2232H This programmer also supports native 4BA commands which are probably used by default. Was JEDEC_ENTER_4_BYTE_ADDR_MODE tested specifically? I guess trying `FEATURE_4BA_ENTER_WREN` vs. `FEATURE_4BA_ENTER` would do, i.e. without the NATIVE and EAR flags. Looking at the datasheet, there is also the `ADP` bit (STATUS3, bit 4) to consider. If the bit is set it might look like we entered 4BA mode because the chip was already in 4BA mode. -- To view, visit https://review.coreboot.org/c/flashrom/+/70342 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I96e48933f33c52c0d10a0d4cb7f7e07c1fceab99 Gerrit-Change-Number: 70342 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> 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: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 05 Dec 2022 10:36:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

05 Dec '22

Attention is currently required from: Felix Singer, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70247 ) Change subject: cbtables.c/search_lb_records: Drop unused variable `count` ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70247 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I340208f513bed57a9cc2bba880a2400352c5cc42 Gerrit-Change-Number: 70247 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 05 Dec 2022 08:49:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: Add selfcheck to unit tests
by Anastasia Klimchuk (Code Review) 05 Dec '22

05 Dec '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/69620 ) ( 9 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: tests: Add selfcheck to unit tests ...................................................................... tests: Add selfcheck to unit tests Add unit tests for programmer_table, flashchips, and board_matches structs. The tests are derived from the selfcheck function, checking that the required fields have been filled in. BUG=b:140595239 BRANCH=None TEST=meson test Change-Id: I41cd014d9bf909296b6c28e3e00548e6883ff41a Signed-off-by: Evan Benn <evanbenn(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/69620 Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M board_enable.c M include/programmer.h M tests/meson.build A tests/selfcheck.c M tests/tests.c M tests/tests.h 6 files changed, 196 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/board_enable.c b/board_enable.c index 4903c0f..bf26173 100644 --- a/board_enable.c +++ b/board_enable.c @@ -2524,6 +2524,7 @@ #endif { 0, 0, 0, 0, 0, 0, 0, 0, NULL, NULL, NULL, P3, NULL, NULL, 0, NT, NULL}, /* end marker */ }; +const size_t board_matches_size = ARRAY_SIZE(board_matches); int selfcheck_board_enables(void) { diff --git a/include/programmer.h b/include/programmer.h index 16448ea..7bcef02 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -193,6 +193,7 @@ }; extern const struct board_match board_matches[]; +extern const size_t board_matches_size; struct board_info { const char *vendor; diff --git a/tests/meson.build b/tests/meson.build index e846d91..b8a5ba8 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -22,6 +22,7 @@ 'layout.c', 'chip.c', 'chip_wp.c', + 'selfcheck.c', ) if not programmer.get('dummy').get('active') diff --git a/tests/selfcheck.c b/tests/selfcheck.c new file mode 100644 index 0000000..aeccf18 --- /dev/null +++ b/tests/selfcheck.c @@ -0,0 +1,156 @@ +/* + * This file is part of the flashrom project. + * + * Copyright 2022 Google LLC + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "flash.h" +#include "string.h" + +#include "include/test.h" +#include "programmer.h" +#include "tests.h" +#include <cmocka.h> + + +#define assert_table(assertion, message, index, name) \ + do { \ + if (!(assertion)) \ + fail_msg(message " for index:%zu name:%s", (index), (name) ? (name) : "unknown"); \ + } while (0) + + +void selfcheck_programmer_table(void **state) +{ + (void)state; /* unused */ + + size_t i; + for (i = 0; i < programmer_table_size; i++) { + const struct programmer_entry *const p = programmer_table[i]; + assert_table(p, "programmer entry is null", i, "unknown"); + assert_table(p->name, "programmer name is null", i, p->name); + bool type_good = false; + switch (p->type) { + case PCI: + case USB: + case OTHER: + type_good = true; + } + assert_table(type_good, "programmer type is invalid", i, p->name); + /* internal has its device list stored separately. */ + if (strcmp("internal", p->name) != 0) + assert_table(p->devs.note, "programmer devs.note is null", i, p->name); + assert_table(p->init, "programmer init is null", i, p->name); + } +} + +void selfcheck_flashchips_table(void **state) +{ + (void)state; /* unused */ + + size_t i; + assert_true(flashchips_size > 1); + assert_true(flashchips[flashchips_size - 1].name == NULL); + for (i = 0; i < flashchips_size - 1; i++) { + const struct flashchip *chip = &flashchips[i]; + assert_table(chip->vendor, "chip vendor is null", i, chip->name); + assert_table(chip->name, "chip name is null", i, chip->name); + assert_table(chip->bustype != BUS_NONE, "chip bustype is BUS_NONE", i, chip->name); + } +} + +void selfcheck_eraseblocks(void **state) +{ + (void)state; /* unused */ + + size_t chip_index; + for (chip_index = 0; chip_index < flashchips_size - 1; chip_index++) { + size_t i, j, k; + const struct flashchip *chip = &flashchips[chip_index]; + unsigned int prev_eraseblock_count = chip->total_size * 1024; + + for (k = 0; k < NUM_ERASEFUNCTIONS; k++) { + unsigned int done = 0; + struct block_eraser eraser = chip->block_erasers[k]; + unsigned int curr_eraseblock_count = 0; + + for (i = 0; i < NUM_ERASEREGIONS; i++) { + /* Blocks with zero size are bugs in flashchips.c. */ + if (eraser.eraseblocks[i].count && !eraser.eraseblocks[i].size) { + fail_msg("Flash chip %s erase function %zu region %zu has size 0", + chip->name, k, i); + } + /* Blocks with zero count are bugs in flashchips.c. */ + if (!eraser.eraseblocks[i].count && eraser.eraseblocks[i].size) { + fail_msg("Flash chip %s erase function %zu region %zu has count 0", + chip->name, k, i); + } + done += eraser.eraseblocks[i].count * eraser.eraseblocks[i].size; + curr_eraseblock_count += eraser.eraseblocks[i].count; + } + /* Empty eraseblock definition with erase function. */ + if (!done && eraser.block_erase) { + printf("Strange: Empty eraseblock definition with non-empty erase function chip %s function %zu. Not an error.\n", + chip->name, k); + } + + if (!done) + continue; + if (done != chip->total_size * 1024) { + fail_msg( + "Flash chip %s erase function %zu region walking resulted in 0x%06x bytes total, expected 0x%06x bytes.", + chip->name, k, done, chip->total_size * 1024); + assert_true(false); + } + + if (!eraser.block_erase) + continue; + /* Check if there are identical erase functions for different + * layouts. That would imply "magic" erase functions. The + * easiest way to check this is with function pointers. + */ + for (j = k + 1; j < NUM_ERASEFUNCTIONS; j++) { + if (eraser.block_erase == chip->block_erasers[j].block_erase) { + fail_msg("Flash chip %s erase function %zu and %zu are identical.", + chip->name, k, j); + } + } + if (curr_eraseblock_count > prev_eraseblock_count) { + fail_msg("Flash chip %s erase function %zu is not in order", chip->name, k); + } + prev_eraseblock_count = curr_eraseblock_count; + } + } +} + +#if CONFIG_INTERNAL == 1 +void selfcheck_board_matches_table(void **state) +{ + (void)state; /* unused */ + + size_t i; + + assert_true(board_matches_size > 1); + assert_true(board_matches[board_matches_size - 1].vendor_name == NULL); + for (i = 0; i < board_matches_size - 1; i++) { + const struct board_match *b = &board_matches[i]; + assert_table(b->vendor_name, "board vendor_name is null", i, b->board_name); + assert_table(b->board_name, "board boad_name is null", i, b->board_name); + if ((!b->first_vendor || !b->first_device || !b->second_vendor || !b->second_device) + || ((!b->lb_vendor) ^ (!b->lb_part)) || (!b->max_rom_decode_parallel && !b->enable)) + fail_msg("Board enable for %s %s is misdefined.\n", b->vendor_name, b->board_name); + } +} + +#else + SKIP_TEST(selfcheck_board_matches_table) +#endif /* CONFIG_INTERNAL */ diff --git a/tests/tests.c b/tests/tests.c index fe70204..abbac64 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -381,6 +381,15 @@ }; ret |= cmocka_run_group_tests_name("helpers.c tests", helpers_tests, NULL, NULL); + const struct CMUnitTest selfcheck[] = { + cmocka_unit_test(selfcheck_programmer_table), + cmocka_unit_test(selfcheck_flashchips_table), + cmocka_unit_test(selfcheck_eraseblocks), + cmocka_unit_test(selfcheck_board_matches_table), + }; + ret |= cmocka_run_group_tests_name("selfcheck.c tests", selfcheck, + NULL, NULL); + const struct CMUnitTest flashrom_tests[] = { cmocka_unit_test(flashbuses_to_text_test_success), }; diff --git a/tests/tests.h b/tests/tests.h index bcca9e8..b36ac70 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -89,4 +89,10 @@ void full_chip_erase_with_wp_dummyflasher_test_success(void **state); void partial_chip_erase_with_wp_dummyflasher_test_success(void **state); +/* selfcheck.c */ +void selfcheck_programmer_table(void **state); +void selfcheck_flashchips_table(void **state); +void selfcheck_eraseblocks(void **state); +void selfcheck_board_matches_table(void **state); + #endif /* TESTS_H */ -- To view, visit https://review.coreboot.org/c/flashrom/+/69620 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I41cd014d9bf909296b6c28e3e00548e6883ff41a Gerrit-Change-Number: 69620 Gerrit-PatchSet: 11 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

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

05 Dec '22

Attention is currently required from: Felix Singer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70242 ) Change subject: test_build.sh: Switch to meson setup <dir> ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS3: I assume this patch is fully tested by jenkins :) -- To view, visit https://review.coreboot.org/c/flashrom/+/70242 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6e84997f910928d3973a4e5826a2d4196bdb2916 Gerrit-Change-Number: 70242 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Mon, 05 Dec 2022 03:28:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashchips.c: remove WREN from GD25Q256D enter 4BA sequence
by Eric Lai (Code Review) 05 Dec '22

05 Dec '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Nikolai Artemiev. Eric Lai has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70342 ) Change subject: flashchips.c: remove WREN from GD25Q256D enter 4BA sequence ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/70342/comment/f57a51b0_1c574ce3 PS1, Line 9: CB:35480 please use commit hash. -- To view, visit https://review.coreboot.org/c/flashrom/+/70342 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I96e48933f33c52c0d10a0d4cb7f7e07c1fceab99 Gerrit-Change-Number: 70342 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> 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: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 05 Dec 2022 02:37:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit December 2022