- flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: tests/: Add subregion alignment unit-test
by Edward O'Callaghan (Code Review) 05 Jan '23

05 Jan '23

Attention is currently required from: Sam McNally, Anastasia Klimchuk. Hello Sam McNally, build bot (Jenkins), Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71659 to look at the new patch set (#2). Change subject: tests/: Add subregion alignment unit-test ...................................................................... tests/: Add subregion alignment unit-test A written region that is sized below that of the erasure granularity can result in a incorrectly read region that does not include prior content within the region before the write op. This was dealt with in ChromeOS downstream by expanding out the read to match the erase granularity however does not seem to impact upstream. Add a unit-test to avoid regression as this is important behaviour to cover. Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/chip.c M tests/tests.c M tests/tests.h 3 files changed, 120 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/71659/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71659 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Gerrit-Change-Number: 71659 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom.c: squash subregion fix
by Edward O'Callaghan (Code Review) 05 Jan '23

05 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71680 ) Change subject: flashrom.c: squash subregion fix ...................................................................... flashrom.c: squash subregion fix Change-Id: I0cce707ed88f6c4b600ba76926bb9e60b93e066b Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c 1 file changed, 82 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/80/71680/1 diff --git a/flashrom.c b/flashrom.c index 62f38f8..6f5d54c 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1187,6 +1187,65 @@ return chip - flashchips; } +/* + * Gets the lowest erase granularity; it is used when + * deciding if the layout map needs to be adjusted such that erase boundaries + * match this granularity. Returns -1 if unsuccessful. + */ +static int get_required_erase_size(struct flashctx *flash) +{ + int i, erase_size_found = 0; + unsigned int required_erase_size; + /* + * Find eraseable block size for read alignment. + * FIXME: This assumes the smallest block erase size is useable + * by erase_and_write_flash(). + */ + required_erase_size = ~0; + for (i = 0; i < NUM_ERASEFUNCTIONS; i++) { + struct block_eraser eraser = flash->chip->block_erasers[i]; + int j; + for (j = 0; j < NUM_ERASEREGIONS; j++) { + unsigned int size = eraser.eraseblocks[j].size; + if (size && (size < required_erase_size)) { + required_erase_size = size; + erase_size_found = 1; + } + } + } + /* likely an error in flashchips[] */ + if (!erase_size_found) { + msg_cerr("%s: No usable erase size found.\n", __func__); + return -1; + } + return required_erase_size; +} +static int round_to_erasable_block_boundary(const int required_erase_size, + const struct romentry *entry, + chipoff_t *rounded_start, + chipsize_t* rounded_len) { + unsigned int start_align, len_align; + const struct flash_region *region = &entry->region; + if (required_erase_size < 0) + return 1; + /* round down to nearest eraseable block boundary */ + start_align = region->start % required_erase_size; + *rounded_start = region->start - start_align; + /* round up to nearest eraseable block boundary */ + *rounded_len = region->end - *rounded_start + 1; + len_align = *rounded_len % required_erase_size; + if (len_align) + *rounded_len = *rounded_len + required_erase_size - len_align; + if (start_align || len_align) { + msg_gdbg("\n%s: Re-aligned partial read due to eraseable " + "block size requirement:\n\tstart: 0x%06x, " + "len: 0x%06x, aligned start: 0x%06x, len: 0x%06x\n", + __func__, region->start, region->end - region->start + 1, + *rounded_start, *rounded_len); + } + return 0; +} + /** * @brief Reads the included layout regions into a buffer. * @@ -1198,15 +1257,22 @@ * @return 0 on success, * 1 if any read fails. */ -static int read_by_layout(struct flashctx *const flashctx, uint8_t *const buffer) +static int read_by_layout(struct flashctx *const flashctx, uint8_t *const buffer, + bool align_to_erasable_block_boundary) { const struct flashrom_layout *const layout = get_layout(flashctx); const struct romentry *entry = NULL; + const int required_erase_size = get_required_erase_size(flashctx); while ((entry = layout_next_included(layout, entry))) { const struct flash_region *region = &entry->region; - const chipoff_t region_start = region->start; - const chipsize_t region_len = region->end - region->start + 1; + chipoff_t region_start = region->start; + chipsize_t region_len = region->end - region->start + 1; + + if (align_to_erasable_block_boundary && + round_to_erasable_block_boundary(required_erase_size, entry, + &region_start, &region_len)) + return 1; if (read_flash(flashctx, buffer + region_start, region_start, region_len)) return 1; @@ -1386,7 +1452,7 @@ if (info->curcontents) { msg_cinfo("Reading current flash chip contents... "); - if (read_by_layout(flashctx, info->curcontents)) { + if (read_by_layout(flashctx, info->curcontents, true)) { /* Now we are truly screwed. Read failed as well. */ msg_cerr("Can't read anymore! Aborting.\n"); /* We have no idea about the flash chip contents, so @@ -1990,7 +2056,7 @@ msg_cinfo("Reading flash... "); int ret = 1; - if (read_by_layout(flashctx, buffer)) { + if (read_by_layout(flashctx, buffer, false)) { msg_cerr("Read operation failed!\n"); msg_cinfo("FAILED.\n"); goto _finalize_ret; @@ -2089,7 +2155,7 @@ } memcpy(curcontents, oldcontents, flash_size); } else { - if (read_by_layout(flashctx, curcontents)) { + if (read_by_layout(flashctx, curcontents, true)) { msg_cinfo("FAILED.\n"); goto _finalize_ret; } -- To view, visit https://review.coreboot.org/c/flashrom/+/71680 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0cce707ed88f6c4b600ba76926bb9e60b93e066b Gerrit-Change-Number: 71680 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: tests/: Add subregion alignment unit-test
by Edward O'Callaghan (Code Review) 05 Jan '23

05 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71659 ) Change subject: tests/: Add subregion alignment unit-test ...................................................................... tests/: Add subregion alignment unit-test Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/chip.c M tests/tests.c M tests/tests.h 3 files changed, 113 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/71659/1 diff --git a/tests/chip.c b/tests/chip.c index 1d9cdc9..5f89d05 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -102,6 +102,29 @@ return 0; } +static int unittest_print_cb(enum flashrom_log_level level, const char *fmt, va_list ap) +{ + int ret = 0; + FILE *output_type = stdout; + + va_list logfile_args; + va_copy(logfile_args, ap); + + if (level < FLASHROM_MSG_INFO) + output_type = stderr; + + if (level <= verbose_screen) { + ret = vfprintf(output_type, fmt, ap); + /* msg_*spew often happens inside chip accessors in possibly + * time-critical operations. Don't slow them down by flushing. */ + if (level != FLASHROM_MSG_SPEW) + fflush(output_type); + } + + va_end(logfile_args); + return ret; +} + static void setup_chip(struct flashrom_flashctx *flashctx, struct flashrom_layout **layout, struct flashchip *chip, const char *programmer_param, const struct io_mock *io) { @@ -418,6 +441,84 @@ free(newcontents); } +void write_chip_subregion_with_dummyflasher_test_success(void **state) // XXX +{ + (void) state; /* unused */ + + static struct io_mock_fallback_open_state data = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock chip_io = { + .fallback_open_state = &data, + }; + + struct flashrom_flashctx flashctx = { 0 }; + struct flashrom_layout *layout; + struct flashchip mock_chip = chip_W25Q128_V; + const uint32_t mock_chip_size = mock_chip.total_size * KiB; + /* + * Dummyflasher is capable to emulate W25Q128.V, so we ask it to do this. + * Nothing to mock, dummy is taking care of this already. + */ + char *param_dup = strdup("bus=spi,emulate=W25Q128FV"); + +#define MOCK_CHIP_SUBREGION_CONTENTS 0xCC + /** + * Prepare newcontents as follows: + * {MOCK_CHIP_SUBREGION_CONTENTS} [..] {MOCK_CHIP_SUBREGION_CONTENTS}. + */ + uint8_t *newcontents = calloc(1, mock_chip_size); + assert_non_null(newcontents); + memset(newcontents, MOCK_CHIP_SUBREGION_CONTENTS, mock_chip_size); + + setup_chip(&flashctx, &layout, &mock_chip, param_dup, &chip_io); + flashrom_flag_set(&flashctx, FLASHROM_FLAG_VERIFY_AFTER_WRITE, true); + flashrom_flag_set(&flashctx, FLASHROM_FLAG_VERIFY_WHOLE_CHIP, false); + flashrom_set_log_callback((flashrom_log_callback *)&unittest_print_cb); + + /** + * Prepare mock chip content and release setup_chip() layout for our + * custom ones. + */ + assert_int_equal(0, flashrom_image_write(&flashctx, newcontents, mock_chip_size, NULL)); + flashrom_layout_release(layout); + + /** + * Create subregion smaller than erase granularity of chip. + */ + printf("Creating custom subregion layout... "); + assert_int_equal(0, flashrom_layout_new(&layout)); + printf("Adding and including subregion0... "); + assert_int_equal(0, flashrom_layout_add_region(layout, 0, (1 * KiB), "subregion0")); + assert_int_equal(0, flashrom_layout_include_region(layout, "subregion0")); + flashrom_layout_set(&flashctx, layout); + printf("Subregion layout configuration done.\n"); + + /** + * Prepare newcontents as follows: + * 0xAA 0xAA {MOCK_CHIP_SUBREGION_CONTENTS} [..] {MOCK_CHIP_SUBREGION_CONTENTS}. + */ + printf("Subregion chip W op..\n"); + memset(newcontents, 0xAA, 2); + assert_int_equal(0, flashrom_image_write(&flashctx, newcontents, mock_chip_size, NULL)); + printf("Subregion chip W op done.\n"); + + printf("Entire chip V op..\n"); + //flashrom_layout_set(&flashctx, NULL); // use default layout. + flashrom_layout_release(layout); + assert_int_equal(0, flashrom_layout_new(&layout)); + assert_int_equal(0, flashrom_layout_add_region(layout, 0, mock_chip_size - 1, "entire")); + assert_int_equal(0, flashrom_layout_include_region(layout, "entire")); + flashrom_layout_set(&flashctx, layout); + assert_int_equal(0, flashrom_image_verify(&flashctx, newcontents, mock_chip_size)); + printf("Entire chip V op done.\n"); + + teardown(&layout); + free(param_dup); + free(newcontents); +} + static size_t verify_chip_fread(void *state, void *buf, size_t size, size_t len, FILE *fp) { /* diff --git a/tests/tests.c b/tests/tests.c index a1dcaca..b96dee5 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -480,6 +480,7 @@ cmocka_unit_test(read_chip_with_dummyflasher_test_success), cmocka_unit_test(write_chip_test_success), cmocka_unit_test(write_chip_with_dummyflasher_test_success), + cmocka_unit_test(write_chip_subregion_with_dummyflasher_test_success), cmocka_unit_test(verify_chip_test_success), cmocka_unit_test(verify_chip_with_dummyflasher_test_success), }; diff --git a/tests/tests.h b/tests/tests.h index 6a12fdb..ee583f1 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -80,6 +80,7 @@ void read_chip_with_dummyflasher_test_success(void **state); void write_chip_test_success(void **state); void write_chip_with_dummyflasher_test_success(void **state); +void write_chip_subregion_with_dummyflasher_test_success(void **state); void verify_chip_test_success(void **state); void verify_chip_with_dummyflasher_test_success(void **state); -- To view, visit https://review.coreboot.org/c/flashrom/+/71659 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Gerrit-Change-Number: 71659 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: rust: Add license and other metadata to Cargo.toml
by Angel Pons (Code Review) 05 Jan '23

05 Jan '23

Attention is currently required from: Evan Benn, Peter Marheine. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71618 ) Change subject: rust: Add license and other metadata to Cargo.toml ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/71618 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibdab16713395509be511e45c5eae946496020429 Gerrit-Change-Number: 71618 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 05 Jan 2023 00:40:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: programmer.h: Guard against sending spi commands on non-spi mst
by Jonathon Hall (Code Review) 04 Jan '23

04 Jan '23

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons. Jonathon Hall has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71269 ) Change subject: programmer.h: Guard against sending spi commands on non-spi mst ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: I also ran into this issue and can confirm that this fixes it, W25Q256FV on Coffee Lake. Thanks Edward! -- To view, visit https://review.coreboot.org/c/flashrom/+/71269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7cce4f9c032d33c01bf616e27a50b9727a40fe1b Gerrit-Change-Number: 71269 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 04 Jan 2023 16:14:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: rust: Add license and other metadata to Cargo.toml
by Evan Benn (Code Review) 04 Jan '23

04 Jan '23

Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71618 ) Change subject: rust: Add license and other metadata to Cargo.toml ...................................................................... rust: Add license and other metadata to Cargo.toml Add missing license to the Cargo.toml rust files, and some other metadata that might be useful. BUG=None BRANCH=None TEST=None Change-Id: Ibdab16713395509be511e45c5eae946496020429 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M bindings/rust/libflashrom-sys/Cargo.toml M bindings/rust/libflashrom/Cargo.toml M util/flashrom_tester/Cargo.toml M util/flashrom_tester/flashrom/Cargo.toml 4 files changed, 33 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/18/71618/1 diff --git a/bindings/rust/libflashrom-sys/Cargo.toml b/bindings/rust/libflashrom-sys/Cargo.toml index 54e016d..af84b8a 100644 --- a/bindings/rust/libflashrom-sys/Cargo.toml +++ b/bindings/rust/libflashrom-sys/Cargo.toml @@ -1,6 +1,12 @@ [package] name = "libflashrom-sys" version = "0.1.0" +description = "Native bindings to the libflashrom library." +readme = "../README" +homepage = "https://www.flashrom.org/" +repository = "https://review.coreboot.org/plugins/gitiles/flashrom/" +license = "GPL-2.0-only" +categories = ["external-ffi-bindings", "hardware-support"] links = "flashrom" build = "build.rs" edition = "2021" diff --git a/bindings/rust/libflashrom/Cargo.toml b/bindings/rust/libflashrom/Cargo.toml index 68636df..1d8fb52 100644 --- a/bindings/rust/libflashrom/Cargo.toml +++ b/bindings/rust/libflashrom/Cargo.toml @@ -1,10 +1,14 @@ [package] name = "libflashrom" version = "0.1.0" +description = "Bindings to the libflashrom library." +readme = "../README" +homepage = "https://www.flashrom.org/" +repository = "https://review.coreboot.org/plugins/gitiles/flashrom/" +license = "GPL-2.0-only" +categories = ["api-bindings", "hardware-support"] edition = "2021" -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - [dependencies] libflashrom-sys = { path = "../libflashrom-sys" } libc = "0.2.124" diff --git a/util/flashrom_tester/Cargo.toml b/util/flashrom_tester/Cargo.toml index a7b6881..948553a 100644 --- a/util/flashrom_tester/Cargo.toml +++ b/util/flashrom_tester/Cargo.toml @@ -3,6 +3,8 @@ version = "1.6.0" authors = ["Edward O'Callaghan <quasisec(a)chromium.org>", "Peter Marheine <pmarheine(a)chromium.org>"] +description = "A tool to verify flashrom and flash chip behaviour." +license = "GPL-2.0-only" edition = "2018" build = "build.rs" diff --git a/util/flashrom_tester/flashrom/Cargo.toml b/util/flashrom_tester/flashrom/Cargo.toml index a2447d0..4d4fc2f 100644 --- a/util/flashrom_tester/flashrom/Cargo.toml +++ b/util/flashrom_tester/flashrom/Cargo.toml @@ -3,6 +3,8 @@ version = "1.0.0" authors = ["Edward O'Callaghan <quasisec(a)chromium.org>", "Peter Marheine <pmarheine(a)chromium.org>"] +description = "Flashrom abstraction for the flashrom_tester tool." +license = "GPL-2.0-only" edition = "2018" [dependencies] -- To view, visit https://review.coreboot.org/c/flashrom/+/71618 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibdab16713395509be511e45c5eae946496020429 Gerrit-Change-Number: 71618 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: programmer.h: Guard against sending spi commands on non-spi mst
by Edward O'Callaghan (Code Review) 04 Jan '23

04 Jan '23

Attention is currently required from: Felix Singer, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71269 ) Change subject: programmer.h: Guard against sending spi commands on non-spi mst ...................................................................... Patch Set 2: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/71269/comment/79edb5d5_14e8c830 PS1, Line 1937: spi_master_no_4ba_modes > Wouldn't it make more sense to add the bustype check inside this function? sort of but not really. This is a chip bustype check not a master bustype check. Adding a predicate to `spi_master_no_4ba_modes()` is not related to preventing the subsequent calls to `spi_{enter,exit}_4ba()`. I think what you are asking for is a `spi_chip_4ba()` func which I have now done? -- To view, visit https://review.coreboot.org/c/flashrom/+/71269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7cce4f9c032d33c01bf616e27a50b9727a40fe1b Gerrit-Change-Number: 71269 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 04 Jan 2023 01:28:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: programmer.h: Guard against sending spi commands on non-spi mst
by Edward O'Callaghan (Code Review) 04 Jan '23

04 Jan '23

Attention is currently required from: Felix Singer, Edward O'Callaghan. Hello Felix Singer, build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71269 to look at the new patch set (#2). Change subject: programmer.h: Guard against sending spi commands on non-spi mst ...................................................................... programmer.h: Guard against sending spi commands on non-spi mst Validate (flash->chip->bustype == BUS_SPI) as ich copies the chip flags in the opaque master and tries incorrectly to issue 4BA commands which results in failure. The issue was detected only in the case of chips >16MB, in this case 'W25Q256FV' that has the feature bits: ``` .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER_WREN | FEATURE_4BA_EAR_C5C8 | FEATURE_4BA_READ | FEATURE_4BA_FAST_READ | FEATURE_WRSR2, ``` The regression was noticed from, commit 0741727925b841c2479b993204ce58c5eb75185a ichspi.c: Read chip ID and use it to populate `flash->chip` TEST=In the case of 'W25Q256FV' on TigerLake. Change-Id: I7cce4f9c032d33c01bf616e27a50b9727a40fe1b Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c M include/programmer.h 2 files changed, 33 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/71269/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7cce4f9c032d33c01bf616e27a50b9727a40fe1b Gerrit-Change-Number: 71269 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: sb600spi.c: Move promontory code into a mmap_read=yes mode
by Edward O'Callaghan (Code Review) 04 Jan '23

04 Jan '23

Attention is currently required from: Thomas Heijligen, Arthur Heymans. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71565 ) Change subject: sb600spi.c: Move promontory code into a mmap_read=yes mode ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/71565/comment/70f0f78d_8675d2de PS2, Line 10: This is not : hardware specific I believe it is hardware specific to controllers after a certain generation. i.e., supported on SPI100 controllers (Zen etc) however not supported on pre-SPI100 controllers was my understanding. -- To view, visit https://review.coreboot.org/c/flashrom/+/71565 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13eb5a646d3569170b3911ae7b3127cd3e6022aa Gerrit-Change-Number: 71565 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 03 Jan 2023 23:51:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: chipset_enable.c: Add TL UP3 and UP4/Y id's
by Angel Pons (Code Review) 03 Jan '23

03 Jan '23

Attention is currently required from: Sam McNally, Edward O'Callaghan. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71576 ) Change subject: chipset_enable.c: Add TL UP3 and UP4/Y id's ...................................................................... Patch Set 1: (1 comment) File chipset_enable.c: https://review.coreboot.org/c/flashrom/+/71576/comment/acd6174a_b5a5ac11 PS1, Line 2083: {0x8086, 0xa083, B_S, DEP, "Intel", "Tiger Lake U Premium 3", enable_flash_pch500}, > As far as I can tell, 0xa083 isn't used after all, so we don't need to add this. Let's keep it, but mark it as untested (change `DEP` to `NT`). -- To view, visit https://review.coreboot.org/c/flashrom/+/71576 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I77120612f7a770ae1319f4cd82913fa465f355f5 Gerrit-Change-Number: 71576 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 03 Jan 2023 06:23:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sam McNally <sammc(a)google.com> Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit