May 2024 - flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[main]: ch347_spi: Add driver support for CH347F packaging
by ZhiYuanNJ (Code Review) 21 May '24

21 May '24

Attention is currently required from: Nicholas Chin, Paul Menzel. ZhiYuanNJ has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email ) Change subject: ch347_spi: Add driver support for CH347F packaging ...................................................................... Patch Set 11: (1 comment) File ch347_spi.c: https://review.coreboot.org/c/flashrom/+/82193/comment/10c61734_264c794c?us… : PS10, Line 9: * (at your option) any later version. : * This program is distributed in the hope that it will be useful, > Seems like you removed the blank line between the "any later version" line and the "This program is. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/82193?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd Gerrit-Change-Number: 82193 Gerrit-PatchSet: 11 Gerrit-Owner: ZhiYuanNJ Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Tue, 21 May 2024 03:49:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[M] Change in flashrom[main]: ch347_spi: Add driver support for CH347F packaging
by Nicholas Chin (Code Review) 20 May '24

20 May '24

Attention is currently required from: Paul Menzel, ZhiYuanNJ. Nicholas Chin has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email ) Change subject: ch347_spi: Add driver support for CH347F packaging ...................................................................... Patch Set 10: Code-Review+1 (1 comment) File ch347_spi.c: https://review.coreboot.org/c/flashrom/+/82193/comment/002bcf5a_ef08a5b7?us… : PS10, Line 9: * (at your option) any later version. : * This program is distributed in the hope that it will be useful, Seems like you removed the blank line between the "any later version" line and the "This program is..." line, but it should be left in place (with no spaces after the *). Looks like Gerrit was updated and now support suggestions, so I think you can just click the "Show Edit" button, then "Apply Fix", and then "Publish Edit" ```suggestion * (at your option) any later version. * * This program is distributed in the hope that it will be useful, ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/82193?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd Gerrit-Change-Number: 82193 Gerrit-PatchSet: 10 Gerrit-Owner: ZhiYuanNJ Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: ZhiYuanNJ Gerrit-Comment-Date: Mon, 20 May 2024 19:56:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: Add documentation for pico-serprog
by Anastasia Klimchuk (Code Review) 20 May '24

20 May '24

Attention is currently required from: J. Neuschäfer, Riku Viitanen, Sydney, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change by Sydney. ( https://review.coreboot.org/c/flashrom/+/82229?usp=email ) Change subject: Add documentation for pico-serprog ...................................................................... Patch Set 4: Code-Review+2 (2 comments) Patchset: PS3: > Some comments on wording I read the latest version, it seems the comments are resolved? But if not, please tell. Thanks for reviewing! Patchset: PS4: I have a question for all three of you. You all have forks of flashrom, with some cool stuff which people use. What do you think about moving your development upstream? I don't know of the reasons you made forks, are there technical reasons why this can't be upstream? I am interested to discuss. For example, I learned about pico-serprog because from time to time people send patches to add support for a new chip, and they say "tested with pico-serprog". I have seen it several times so I remember. What do you think of adding pico-serprog upstream, can it be a programmer here? This was an example, but the question is to all of you. As for the technical aspects, if you add a new file(s), you add your copyright at the top of the file, also we have the MAINTAINERS file where you identify yourself as a maintainer for your programmer. More details here https://flashrom.org/about_flashrom/team.html This is something to think about, can be for later, not necessarily for right now. What do you think? Also just to be clear: the question does not affect this patch, the patch goes ahead independently of it. It's just convenient, all you are here! :) -- To view, visit https://review.coreboot.org/c/flashrom/+/82229?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I457dfec52f89997f64b6c276c50b329359d61b77 Gerrit-Change-Number: 82229 Gerrit-PatchSet: 4 Gerrit-Owner: Sydney <git(a)funkeleinhorn.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: J. Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Reviewer: Riku Viitanen <riku.viitanen(a)protonmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Riku Viitanen <riku.viitanen(a)protonmail.com> Gerrit-Attention: J. Neuschäfer <j.neuschaefer(a)gmx.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Sydney <git(a)funkeleinhorn.com> Gerrit-Comment-Date: Mon, 20 May 2024 10:20:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: J. Neuschäfer <j.neuschaefer(a)gmx.net>

1 0

[M] Change in flashrom[main]: ch347_spi: Add driver support for CH347F packaging
by ZhiYuanNJ (Code Review) 20 May '24

20 May '24

Attention is currently required from: Nicholas Chin, Paul Menzel. ZhiYuanNJ has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email ) Change subject: ch347_spi: Add driver support for CH347F packaging ...................................................................... Patch Set 10: (3 comments) Patchset: PS10: Are there any issues with this submission? File ch347_spi.c: https://review.coreboot.org/c/flashrom/+/82193/comment/d66c2532_0e34fa6b?us… : PS9, Line 10: > Remove extra spaces Done https://review.coreboot.org/c/flashrom/+/82193/comment/8f4a7d07_9444a82e?us… : PS9, Line 307: > Remove trailing space Done -- To view, visit https://review.coreboot.org/c/flashrom/+/82193?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd Gerrit-Change-Number: 82193 Gerrit-PatchSet: 10 Gerrit-Owner: ZhiYuanNJ Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Mon, 20 May 2024 10:02:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com>

1 0

[M] Change in flashrom[main]: flashchips: Add support for GigaDevice GD25LR256E, GD251R512ME
by Nikolai Artemiev (Code Review) 20 May '24

20 May '24

Attention is currently required from: Anastasia Klimchuk, Angel Pons, Martin L Roth, Martin Roth, Raj Astekar, Ravishankar Sarawadi, Wonkyu Kim. Nikolai Artemiev has posted comments on this change by Ravishankar Sarawadi. ( https://review.coreboot.org/c/flashrom/+/58025?usp=email ) Change subject: flashchips: Add support for GigaDevice GD25LR256E, GD251R512ME ...................................................................... Patch Set 10: Code-Review+2 (1 comment) Patchset: PS10: > I don't see copies of the datasheet on the list; it looks like they may have been sent to you privat […] I also don't have access to the datasheets on the mailing list thread. But I agree with Peter's reasoning about BP4/TB, I think we can submit these patches -- To view, visit https://review.coreboot.org/c/flashrom/+/58025?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I2fe6bc1219cd1ee19b93caabab69de938cfc44b0 Gerrit-Change-Number: 58025 Gerrit-PatchSet: 10 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.corp-partner.google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Comment-Date: Mon, 20 May 2024 06:42:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

[M] Change in flashrom[main]: flashchips: Add support for chip model GD25LF128E
by Nikolai Artemiev (Code Review) 20 May '24

20 May '24

Attention is currently required from: Anastasia Klimchuk, Stefan Reinauer, Victor Lim. Nikolai Artemiev has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/82332?usp=email ) Change subject: flashchips: Add support for chip model GD25LF128E ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82332?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I71fdc7ea1aea69d14db6af3bac2da3e7bee8abbe Gerrit-Change-Number: 82332 Gerrit-PatchSet: 2 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Victor Lim <vlim(a)gigadevice.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 20 May 2024 06:27:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in flashrom[main]: flashchips: Add support for GigaDevice GD25LR256E, GD251R512ME
by Peter Marheine (Code Review) 20 May '24

20 May '24

Attention is currently required from: Anastasia Klimchuk, Angel Pons, Martin L Roth, Martin Roth, Nikolai Artemiev, Raj Astekar, Ravishankar Sarawadi, Wonkyu Kim. Peter Marheine has posted comments on this change by Ravishankar Sarawadi. ( https://review.coreboot.org/c/flashrom/+/58025?usp=email ) Change subject: flashchips: Add support for GigaDevice GD25LR256E, GD251R512ME ...................................................................... Patch Set 10: Code-Review+2 (1 comment) Patchset: PS10: I don't see copies of the datasheet on the list; it looks like they may have been sent to you privately, since I see a message in reply blocks that looks like it had them attached? I was able to find a copy of the -512ME datasheet to look at, though (and assume the -256E is basically just a 256Mb version). > Especially whether my comment is correct, that BP4 in datasheets works as TB. Looks correct to me, since table 3 of the -512ME datasheet illustrates that pretty clearly (and I compared against EN25QH32 to verify the polarity was interpreted the same). -- To view, visit https://review.coreboot.org/c/flashrom/+/58025?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I2fe6bc1219cd1ee19b93caabab69de938cfc44b0 Gerrit-Change-Number: 58025 Gerrit-PatchSet: 10 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-CC: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.corp-partner.google.com> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Comment-Date: Mon, 20 May 2024 00:00:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: flashchips: Add support for Boya B25Q64AS
by Malcolm Boyes (Code Review) 19 May '24

19 May '24

Attention is currently required from: Nikolai Artemiev, Stefan Reinauer. Malcolm Boyes has posted comments on this change by Malcolm Boyes. ( https://review.coreboot.org/c/flashrom/+/82259?usp=email ) Change subject: flashchips: Add support for Boya B25Q64AS ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/82259/comment/127332cb_e667dede?us… : PS2, Line 14: boyesm > Sorry I missed this: I think it's more standard to put your name here (Malcolm Boyes). […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/82259?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I05ecf2b118902db974544d86e023a348912371dd Gerrit-Change-Number: 82259 Gerrit-PatchSet: 3 Gerrit-Owner: Malcolm Boyes <boyesmalcolm(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sun, 19 May 2024 15:24:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: flashchips: Add support for Boya B25Q64AS
by Malcolm Boyes (Code Review) 19 May '24

19 May '24

Attention is currently required from: Malcolm Boyes, Nikolai Artemiev, Stefan Reinauer. Hello Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/82259?usp=email to look at the new patch set (#3). Change subject: flashchips: Add support for Boya B25Q64AS ...................................................................... flashchips: Add support for Boya B25Q64AS The B25Q64AS has been tested by ch341a programmer: read, write, erase Datasheet: https://archive.org/details/1912111437-boyamicro-by-25-q-64-assig-c-383793 Change-Id: I05ecf2b118902db974544d86e023a348912371dd Signed-off-by: Malcolm Boyes <boyesmalcolm(a)gmail.com> --- M flashchips.c M include/flashchips.h 2 files changed, 39 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/82259/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/82259?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I05ecf2b118902db974544d86e023a348912371dd Gerrit-Change-Number: 82259 Gerrit-PatchSet: 3 Gerrit-Owner: Malcolm Boyes <boyesmalcolm(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Malcolm Boyes <boyesmalcolm(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>

1 0

[M] Change in flashrom[main]: ichspi.c: Add support for region 9 and beyond in Meteor Lake
by Anastasia Klimchuk (Code Review) 19 May '24

19 May '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/81357?usp=email ) Change subject: ichspi.c: Add support for region 9 and beyond in Meteor Lake ...................................................................... ichspi.c: Add support for region 9 and beyond in Meteor Lake Since Meteor Lake, configuring region access for FREG9 and higher is necessary. This configuration is determined using BIOS_BM registers: BIOS_BM_RAP (Offset 0x118): BIOS Master Read Access Permissions. Each bit [15:0] corresponds to a region [15:0]. A set bit grants BIOS master read access. BIOS_BM_WAP (Offset 0x11c): BIOS Master Write Access Permissions. Each bit [15:0] corresponds to a region [15:0]. A set bit grants BIOS master write/erase access. Move CHIPSET_METEOR_LAKE to the bottom of the ich_chipset list to ensure that all the newer chipsets in the future will use BIOS_BM check by default. BUG=b:319773700, b:304439294 BUG=b:319336080 TEST=On MTL, use flashrom -VV to see correct FREG9 access TEST=On ADL, use flashrom -VV to see not break anything TEST=On APL, use flashrom -VV to see not break anything Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9 Signed-off-by: Hsuan Ting Chen <roccochen(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/81357 Reviewed-by: Jamie Ryu <jamie.m.ryu(a)intel.com> Reviewed-by: Nikolai Artemiev <nartemiev(a)google.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M ichspi.c M include/programmer.h 2 files changed, 67 insertions(+), 20 deletions(-) Approvals: build bot (Jenkins): Verified Jamie Ryu: Looks good to me, but someone else must approve Nikolai Artemiev: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/ichspi.c b/ichspi.c index bee5ec9..bef43b9 100644 --- a/ichspi.c +++ b/ichspi.c @@ -153,6 +153,9 @@ #define ICH9_FADDR_FLA 0x01ffffff #define ICH9_REG_FDATA0 0x10 /* 64 Bytes */ +#define ICH_REG_BIOS_BM_RAP 0x118 /* 16 Bits BIOS Master Read Access Permissions */ +#define ICH_REG_BIOS_BM_WAP 0x11c /* 16 Bits BIOS Master Write Access Permissions */ + #define ICH9_REG_FRAP 0x50 /* 32 Bytes Flash Region Access Permissions */ #define ICH9_REG_FREG0 0x54 /* 32 Bytes Flash Region 0 */ @@ -1834,8 +1837,52 @@ "read-write", "write-only", "read-only", "locked" }; -static enum ich_access_protection ich9_handle_frap(struct fd_region *fd_regions, - uint32_t frap, unsigned int i) +static void ich_get_bios_region_access(uint16_t *region_read_access, + uint16_t *region_write_access) +{ + uint32_t tmp; + *region_read_access = 0; + *region_write_access = 0; + + if (ich_generation >= CHIPSET_METEOR_LAKE) { + /* + * Starting from Meteor Lake, we need to fetch the region + * read/write access permissions from the BIOS_BM registers + * because we need to support FREG9 or above. + */ + *region_read_access = mmio_readw(ich_spibar + ICH_REG_BIOS_BM_RAP); + *region_write_access = mmio_readw(ich_spibar + ICH_REG_BIOS_BM_WAP); + msg_pdbg("0x118: 0x%04"PRIx16" (BIOS_BM_RAP)\n", *region_read_access); + msg_pdbg("0x11a: 0x%04"PRIx16" (BIOS_BM_WAP)\n", *region_write_access); + } else { + /* + * FRAP - Flash Regions Access Permissions Register + * Bit Descriptions: + * 31:24 BIOS Master Write Access Grant (BMWAG) + * 23:16 BIOS Master Read Access Grant (BMRAG) + * 15:8 BIOS Region Write Access (BRWA) + * 7:0 BIOS Region Read Access (BRRA) + */ + tmp = mmio_readl(ich_spibar + ICH9_REG_FRAP); + msg_pdbg("0x50: 0x%08"PRIx32" (FRAP)\n", tmp); + msg_pdbg("BMWAG 0x%02"PRIx32", ", ICH_BMWAG(tmp)); + msg_pdbg("BMRAG 0x%02"PRIx32", ", ICH_BMRAG(tmp)); + msg_pdbg("BRWA 0x%02"PRIx32", ", ICH_BRWA(tmp)); + msg_pdbg("BRRA 0x%02"PRIx32"\n", ICH_BRRA(tmp)); + + *region_read_access = (uint16_t)ICH_BRRA(tmp); + *region_write_access = (uint16_t)ICH_BRWA(tmp); + } +} + +static unsigned int ich_get_defined_region_count(void) { + return (ich_generation >= CHIPSET_METEOR_LAKE) ? 16 : 8; +} + +static enum ich_access_protection ich9_handle_region_access(struct fd_region *fd_regions, + uint16_t region_read_access, + uint16_t region_write_access, + unsigned int i) { static const char *const region_names[] = { "Flash Descriptor", "BIOS", "Management Engine", @@ -1863,12 +1910,12 @@ } msg_pdbg("0x%02X: 0x%08"PRIx32" ", offset, freg); - if (i < 8) { - rwperms_idx = (((ICH_BRWA(frap) >> i) & 1) << 1) | - (((ICH_BRRA(frap) >> i) & 1) << 0); + if (i < ich_get_defined_region_count()) { + rwperms_idx = (((region_write_access >> i) & 1) << 1) | + (((region_read_access >> i) & 1) << 0); rwperms = access_perms_to_protection[rwperms_idx]; } else { - /* Datasheets don't define any access bits for regions > 7. We + /* Datasheets might not define all the access bits for regions. We can't rely on the actual descriptor settings either as there are several overrides for them (those by other masters are not even readable by us, *shrug*). */ @@ -2059,11 +2106,11 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: - case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_JASPER_LAKE: case CHIPSET_ELKHART_LAKE: + case CHIPSET_METEOR_LAKE: *num_pr = 6; /* Includes GPR0 */ *reg_pr0 = PCH100_REG_FPR0; swseq->reg_ssfsc = PCH100_REG_SSFSC; @@ -2099,11 +2146,11 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: - case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_JASPER_LAKE: case CHIPSET_ELKHART_LAKE: + case CHIPSET_METEOR_LAKE: *num_freg = 16; break; default: @@ -2161,11 +2208,11 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: - case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_JASPER_LAKE: case CHIPSET_ELKHART_LAKE: + case CHIPSET_METEOR_LAKE: tmp = mmio_readl(spibar + PCH100_REG_DLOCK); msg_pdbg("0x0c: 0x%08"PRIx32" (DLOCK)\n", tmp); prettyprint_pch100_reg_dlock(tmp); @@ -2175,16 +2222,15 @@ } if (desc_valid) { - tmp = mmio_readl(spibar + ICH9_REG_FRAP); - msg_pdbg("0x50: 0x%08"PRIx32" (FRAP)\n", tmp); - msg_pdbg("BMWAG 0x%02"PRIx32", ", ICH_BMWAG(tmp)); - msg_pdbg("BMRAG 0x%02"PRIx32", ", ICH_BMRAG(tmp)); - msg_pdbg("BRWA 0x%02"PRIx32", ", ICH_BRWA(tmp)); - msg_pdbg("BRRA 0x%02"PRIx32"\n", ICH_BRRA(tmp)); + /* Get the region access data from FRAP/BIOS_BM */ + uint16_t region_read_access, region_write_access; + ich_get_bios_region_access(&region_read_access, &region_write_access); - /* Handle FREGx and FRAP registers */ + /* Handle FREGx and region access registers */ for (i = 0; i < num_freg; i++) - ich_spi_rw_restricted |= ich9_handle_frap(hwseq_data.fd_regions, tmp, i); + ich_spi_rw_restricted |= ich9_handle_region_access(hwseq_data.fd_regions, + region_read_access, + region_write_access, i); if (ich_spi_rw_restricted) msg_pinfo("Not all flash regions are freely accessible by flashrom. This is " "most likely\ndue to an active ME. Please see " @@ -2242,12 +2288,12 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: - case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_JASPER_LAKE: case CHIPSET_BAYTRAIL: case CHIPSET_ELKHART_LAKE: + case CHIPSET_METEOR_LAKE: break; default: ichspi_bbar = mmio_readl(spibar + ICH9_REG_BBAR); @@ -2282,11 +2328,11 @@ case CHIPSET_400_SERIES_COMET_POINT: case CHIPSET_500_SERIES_TIGER_POINT: case CHIPSET_600_SERIES_ALDER_POINT: - case CHIPSET_METEOR_LAKE: case CHIPSET_APOLLO_LAKE: case CHIPSET_GEMINI_LAKE: case CHIPSET_JASPER_LAKE: case CHIPSET_ELKHART_LAKE: + case CHIPSET_METEOR_LAKE: break; default: tmp = mmio_readl(spibar + ICH9_REG_FPB); diff --git a/include/programmer.h b/include/programmer.h index 939c8de..8ef6ffd 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -357,11 +357,12 @@ CHIPSET_400_SERIES_COMET_POINT, CHIPSET_500_SERIES_TIGER_POINT, CHIPSET_600_SERIES_ALDER_POINT, - CHIPSET_METEOR_LAKE, CHIPSET_APOLLO_LAKE, CHIPSET_GEMINI_LAKE, CHIPSET_JASPER_LAKE, CHIPSET_ELKHART_LAKE, + /* All chipsets after METEOR_LAKE should support checking BIOS_BM to get read/write access to of FREG0~15 */ + CHIPSET_METEOR_LAKE, }; /* ichspi.c */ -- To view, visit https://review.coreboot.org/c/flashrom/+/81357?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9 Gerrit-Change-Number: 81357 Gerrit-PatchSet: 13 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2024