November 2022 - flashrom-gerrit

[M] Change in flashrom[master]: writeprotect,ichspi,spi25: handle register access constraints
by Nikolai Artemiev (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Subrata Banik, Angel Pons, Nikolai Artemiev. Hello build bot (Jenkins), Subrata Banik, Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69129 to look at the new patch set (#9). Change subject: writeprotect,ichspi,spi25: handle register access constraints ...................................................................... writeprotect,ichspi,spi25: handle register access constraints Make the spi25 register read/write functions return SPI_INVALID_OPCODE if the programmer blocks the read/write opcode for the register. Likewise, make ichspi read/write register functions return SPI_INVALID_OPCODE for registers >SR1 as they cannot be accessd. Make writeprotect ignore SPI_INVALID_OPCODE unless it is trying to read/write SR1, which should always be supported. BUG=b:253715389,b:253713774 BRANCH=none TEST=flashrom --wp-{enable,disable,range,list,status} on dedede Change-Id: I2145749dcc51f4556550650dab5aa1049f879c45 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M ichspi.c M spi25_statusreg.c M writeprotect.c 3 files changed, 75 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/29/69129/9 -- To view, visit https://review.coreboot.org/c/flashrom/+/69129 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2145749dcc51f4556550650dab5aa1049f879c45 Gerrit-Change-Number: 69129 Gerrit-PatchSet: 9 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Nikolai Artemiev (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Subrata Banik, Edward O'Callaghan, Angel Pons. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69195 ) Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... Patch Set 5: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/69195/comment/efdfffc3_1b666e60 PS3, Line 21: FIX: w,E > downstream also has https://chromium.googlesource. […] This was due to write/erase trying to overwrite the ME/flash descriptor regions: ``` FREG0: Flash Descriptor region (0x00000000-0x00000fff) is read-only. FREG1: BIOS region (0x00381000-0x00ffffff) is read-write. FREG2: Management Engine region (0x00001000-0x00380fff) is read-only. ``` Write and erase work if just the writable BIOS region is used: ``` # cat test_layout 0x00381000:0x00ffffff bios # flashrom -{w,E} --layout test_layout -i bios ``` File ichspi.c: https://review.coreboot.org/c/flashrom/+/69195/comment/22c63bbf_abda23bc PS2, Line 1486: flash->chip->tested = entry->tested; > Well as long as CB:69842 isn't re-introduced. Some chip entries may need to be updated with WP test status but this shouldn't cause universal warnings for opaque programmers as in CB:69842. -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 5 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 29 Nov 2022 05:24:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Nikolai Artemiev (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Subrata Banik, Angel Pons, Nikolai Artemiev. Hello build bot (Jenkins), Subrata Banik, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69195 to look at the new patch set (#5). Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... ichspi.c: Read chip ID and use it to populate `flash->chip` Read the flash chip vendor/device ID using hardware sequencing, find the corresponding flashchip entry, and copy it over to `flash->chip`. Identifying the chip was not previously required as ICH hardware sequencing handles chip-level details related to read/write/erase ops. However writeprotect operations require the chip entry to be identified so that chip->reg_bits can be used to compute status register values. BUG=b:253715389,b:253713774 BRANCH=none TEST=flashrom on dedede (JSL) identifies "W25Q128.V..M" chip TEST=flashrom -{r,v} on dedede TEST=write/erase bios region on dedede: flashrom -{E,w} --layout <(echo '0x381000:0xffffff bios') -i bios Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M ichspi.c 1 file changed, 90 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/69195/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 5 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Nikolai Artemiev (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Subrata Banik, Angel Pons, Nikolai Artemiev. Hello build bot (Jenkins), Subrata Banik, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69195 to look at the new patch set (#4). Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... ichspi.c: Read chip ID and use it to populate `flash->chip` Read the flash chip vendor/device ID using hardware sequencing, find the corresponding flashchip entry, and copy it over to `flash->chip`. Identifying the chip was not previously required as ICH hardware sequencing handles chip-level details related to read/write/erase ops. However writeprotect operations require the chip entry to be identified so that chip->reg_bits can be used to compute status register values. BUG=b:253715389,b:253713774 BRANCH=none TEST=flashrom on dedede (JSL) identifies "W25Q128.V..M" chip TEST=flashrom -{r,v} works on dedede TEST=flashrom -{w,E} works on dedede with layout file for BIOS region Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M ichspi.c 1 file changed, 89 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/69195/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: internal.c: laptop_ok global state can become stale
by Anastasia Klimchuk (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Edward O'Callaghan. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70026 ) Change subject: internal.c: laptop_ok global state can become stale ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: I assume -1 should go away after rebase? that build run does not exist anymore :\ -- To view, visit https://review.coreboot.org/c/flashrom/+/70026 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2c00c351904307eeb1488c5dfaffc91d6468ee25 Gerrit-Change-Number: 70026 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 29 Nov 2022 01:12:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: writeprotect,ichspi,spi25: handle register access constraints
by Edward O'Callaghan (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Subrata Banik, Angel Pons, Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69129 ) Change subject: writeprotect,ichspi,spi25: handle register access constraints ...................................................................... Patch Set 7: (1 comment) File spi25_statusreg.c: https://review.coreboot.org/c/flashrom/+/69129/comment/e8b15a48_3a693a7b PS7, Line 133: /* Some SPI masters like ICH7/ICH9 block opcodes for writing SR2/SR3/etc. */ Do they? I thought they are swseq and this was only a limitation of hwseq? -- To view, visit https://review.coreboot.org/c/flashrom/+/69129 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2145749dcc51f4556550650dab5aa1049f879c45 Gerrit-Change-Number: 69129 Gerrit-PatchSet: 7 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 29 Nov 2022 00:12:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi.c: Read chip ID and use it to populate `flash->chip`
by Edward O'Callaghan (Code Review) 29 Nov '22

29 Nov '22

Attention is currently required from: Subrata Banik, Angel Pons, Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69195 ) Change subject: ichspi.c: Read chip ID and use it to populate `flash->chip` ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/69195/comment/e1c22adb_bd3df837 PS3, Line 21: FIX: w,E downstream also has https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/refs/he… > Looks like it's trying to do SPI erase instead of hwseq now how did you surmise this? What is the output with `-VV` ? File ichspi.c: https://review.coreboot.org/c/flashrom/+/69195/comment/37c2b09b_b76e2a12 PS2, Line 1486: flash->chip->tested = entry->tested; > Wouldn't it be better to keep this and delete the line that overwrites it? […] Well as long as CB:69842 isn't re-introduced. -- To view, visit https://review.coreboot.org/c/flashrom/+/69195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia408e1e45dc6f53c0934afd6558e301abfa48ee6 Gerrit-Change-Number: 69195 Gerrit-PatchSet: 3 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 29 Nov 2022 00:09:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tree/: Make probe_opcode() flashctx argument const
by Edward O'Callaghan (Code Review) 28 Nov '22

28 Nov '22

Attention is currently required from: Nikolai Artemiev. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70030 ) Change subject: tree/: Make probe_opcode() flashctx argument const ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70030 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I19e98be50d682de2d2715417f8b7b8c62b871617 Gerrit-Change-Number: 70030 Gerrit-PatchSet: 2 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 28 Nov 2022 22:58:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: bindings: Add FLASHROM_WP_ERR_UNSUPPORTED_STATE
by Evan Benn (Code Review) 28 Nov '22

28 Nov '22

Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69864 ) Change subject: bindings: Add FLASHROM_WP_ERR_UNSUPPORTED_STATE ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: > It works CB:70099 \o/ 👍 -- To view, visit https://review.coreboot.org/c/flashrom/+/69864 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2adaad6552a601147e779c85a2a51d2e39b91c28 Gerrit-Change-Number: 69864 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: 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-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Mon, 28 Nov 2022 22:32:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: libflashrom: Define macro to detect v1.3 header
by Evan Benn (Code Review) 28 Nov '22

28 Nov '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70027 ) Change subject: libflashrom: Define macro to detect v1.3 header ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > I'm not against doing it this way, but I'm wondering how this gets used. […] Heres what SDL does, https://github.com/libsdl-org/SDL/blob/main/include/SDL3/SDL_version.h#L64 `#define SDL_MAJOR_VERSION 3` and as a struct: `typedef struct SDL_version` and as a runtime function `extern DECLSPEC void SDLCALL SDL_GetVersion(SDL_version * ver);` -- To view, visit https://review.coreboot.org/c/flashrom/+/70027 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iac35ff167272e1bc3d5434f7b47099f53bfceab7 Gerrit-Change-Number: 70027 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 28 Nov 2022 22:25:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022