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

[S] Change in flashrom[main]: flashchips: Add support for MXIC MX25L12850F
by DZ (Code Review) 20 Mar '24

20 Mar '24

DZ has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/81350?usp=email ) Change subject: flashchips: Add support for MXIC MX25L12850F ...................................................................... flashchips: Add support for MXIC MX25L12850F The MX25L12850F has been tested by ch341a programmer : read, write, erase and wp. MX25L12850F datasheet is available at the following URL: https://www.macronix.com/Lists/Datasheet/Attachments/8632/MX25L12850F,%203V… Change-Id: I71ac70d273904f94d015401f9d8df587084efad0 Signed-off-by: DanielZhang <danielzhang(a)mxic.com.cn> --- M flashchips.c 1 file changed, 46 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/50/81350/1 diff --git a/flashchips.c b/flashchips.c index 204f34f..0c4bc39 100644 --- a/flashchips.c +++ b/flashchips.c @@ -9045,6 +9045,52 @@ { .vendor = "Macronix", + .name = "MX25L12850F", + .bustype = BUS_SPI, + .manufacture_id = MACRONIX_ID, + .model_id = MACRONIX_MX25L12805D, + .total_size = 16384, + .page_size = 256, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_CFGR, + .tested = TEST_OK_PREWB, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 4096} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 512} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 256} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {16 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {16 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, /* bit6 is quad enable */ + .unlock = SPI_DISABLE_BLOCKPROTECT_BP3_SRWD, + .write = SPI_CHIP_WRITE256, + .read = SPI_CHIP_READ, /* Fast read (0x0B) supported */ + .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {CONFIG, 3, OTP} + }, + + .decode_range = DECODE_RANGE_SPI25, + }, + + { + .vendor = "Macronix", .name = "MX25L1605", .bustype = BUS_SPI, .manufacture_id = MACRONIX_ID, -- To view, visit https://review.coreboot.org/c/flashrom/+/81350?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I71ac70d273904f94d015401f9d8df587084efad0 Gerrit-Change-Number: 81350 Gerrit-PatchSet: 1 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[main]: ichspi.c: Add support for region 9 and beyond in Meteor Lake
by Subrata Banik (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Hsuan-ting Chen. Subrata Banik has posted comments on 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 ...................................................................... Patch Set 2: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/81357/comment/82abe3d6_838d31df : PS1, Line 1848: case CHIPSET_METEOR_LAKE: > It was your previous CL that put MTL in that position For sure that is my CL which added MTL after ADL but there is no such restriction to maintain the order. You can move the macro at the end of the list. > (https://review.coreboot.org/c/flashrom/+/62783) Could you share if (1) We could move it to the very end Please move MTL as the end so we can add LNL and PTL after MTL. > (2) Will there be any other chipset name which will be moved to make the order makes sense? Looking at ur current logic, i felt the MTL is the first SOC where we would like to enable this support hence, moving MTL is okay and new Intel SOC can be added after MTL so, the suggested logic (by me) works w/o any maintenance burden. -- 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 Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9 Gerrit-Change-Number: 81357 Gerrit-PatchSet: 2 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Comment-Date: Wed, 20 Mar 2024 03:15:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: doc/dev_guide: Add section about Jenkins build, and scan-build
by Peter Marheine (Code Review) 19 Mar '24

19 Mar '24

Attention is currently required from: Anastasia Klimchuk, Thomas Heijligen. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81261?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: doc/dev_guide: Add section about Jenkins build, and scan-build ...................................................................... Patch Set 1: (1 comment) File doc/dev_guide/development_guide.rst: https://review.coreboot.org/c/flashrom/+/81261/comment/c9bbd0f9_62450e2c : PS1, Line 226: need to "should" seems better, since you're saying authors should review the report but aren't required to. -- To view, visit https://review.coreboot.org/c/flashrom/+/81261?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I416b632c55d1ceb925456ac8c8947dfbcef2e888 Gerrit-Change-Number: 81261 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 19 Mar 2024 22:54:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: cli_common: Add link to the documentation how to mark chip tested
by Peter Marheine (Code Review) 19 Mar '24

19 Mar '24

Attention is currently required from: Anastasia Klimchuk. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81266?usp=email ) Change subject: cli_common: Add link to the documentation how to mark chip tested ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/81266?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I36105725058f2fecb82408c369b70b3324502ece Gerrit-Change-Number: 81266 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 19 Mar 2024 22:47:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: ichspi.c: Add support for region 9 and beyond in Meteor Lake
by Hsuan-ting Chen (Code Review) 19 Mar '24

19 Mar '24

Attention is currently required from: Subrata Banik. Hsuan-ting Chen has posted comments on 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 ...................................................................... Patch Set 2: (2 comments) File ichspi.c: https://review.coreboot.org/c/flashrom/+/81357/comment/7402d0f1_974aeb6e : PS1, Line 1848: case CHIPSET_METEOR_LAKE: > this might increase the maintenance burden going forward ? for example: we need to add PTL, WCL etc. […] It was your previous CL that put MTL in that position (https://review.coreboot.org/c/flashrom/+/62783) Could you share if (1) We could move it to the very end (2) Will there be any other chipset name which will be moved to make the order makes sense? https://review.coreboot.org/c/flashrom/+/81357/comment/358a4f26_5b8731d3 : PS1, Line 2236: ich9_handle_frap > isn't `ich9_handle_frap` function takes only takes 3 arguments ? In line#1889, I changed it to ``` static enum ich_access_protection ich9_handle_frap(struct fd_region *fd_regions, uint32_t region_read_access, uint32_t region_write_access, unsigned int i) ``` Since if we use `BIOS_BM`, we will derive two seperated bitmask, and I don't think we need to merge them. Maybe `ich9_handle_frap` should be renamed to something like `ich9_handle_region_access` ? -- 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 Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9 Gerrit-Change-Number: 81357 Gerrit-PatchSet: 2 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Comment-Date: Tue, 19 Mar 2024 22:38:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: ichspi.c: Add support for region 9 and beyond in Meteor Lake
by Hsuan-ting Chen (Code Review) 19 Mar '24

19 Mar '24

Attention is currently required from: Hsuan-ting Chen. Hello Hsuan Ting Chen, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/81357?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) 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. 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 Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9 Signed-off-by: roccochen(a)chromium.com <roccochen(a)chromium.org> --- M ichspi.c 1 file changed, 67 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/57/81357/2 -- 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 Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9 Gerrit-Change-Number: 81357 Gerrit-PatchSet: 2 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-MessageType: newpatchset

1 0

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

19 Mar '24

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen. Subrata Banik has posted comments on 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 ...................................................................... Patch Set 1: (2 comments) File ichspi.c: https://review.coreboot.org/c/flashrom/+/81357/comment/5a064777_d8989c4c : PS1, Line 1848: case CHIPSET_METEOR_LAKE: this might increase the maintenance burden going forward ? for example: we need to add PTL, WCL etc. etc. platform going forward. can we add `CHIPSET_METEOR_LAKE` as last entry for now inside `ich_chipset` and then update this logic something like this: ``` 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); } 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 = ICH_BRRA(tmp); *region_write_access = ICH_BRWA(tmp); } ``` https://review.coreboot.org/c/flashrom/+/81357/comment/1cb978df_fa7bff41 : PS1, Line 2236: ich9_handle_frap isn't `ich9_handle_frap` function takes only takes 3 arguments ? -- 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 Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9 Gerrit-Change-Number: 81357 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Comment-Date: Tue, 19 Mar 2024 09:25:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: ich: Add names for region 5, 9, 10, 11, 12, 13, 15
by Subrata Banik (Code Review) 19 Mar '24

19 Mar '24

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81356?usp=email ) Change subject: ich: Add names for region 5, 9, 10, 11, 12, 13, 15 ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/81356?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I3b164ce4ae84bfd523fcd8be416c5d13183ed632 Gerrit-Change-Number: 81356 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Comment-Date: Tue, 19 Mar 2024 09:13:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: Add support for Idaville HCC platform and iRC regions
by Kevin Yu (Code Review) 19 Mar '24

19 Mar '24

Attention is currently required from: Paul Menzel. Kevin Yu has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81327?usp=email ) Change subject: Add support for Idaville HCC platform and iRC regions ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Hi Pual Can you help review this code change? Thanks Kevin Yu -- To view, visit https://review.coreboot.org/c/flashrom/+/81327?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I39e7fa56b1b1de429f413ce32e69c8d8c769b6a9 Gerrit-Change-Number: 81327 Gerrit-PatchSet: 1 Gerrit-Owner: Kevin Yu <ykevin(a)celestica.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Tue, 19 Mar 2024 07:08:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: ichspi.c: Add support for region 9 and beyond in Meteor Lake
by Hsuan-ting Chen (Code Review) 19 Mar '24

19 Mar '24

Attention is currently required from: Hsuan Ting Chen. Hello Hsuan Ting Chen, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/81357?usp=email to review the following change. 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. 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 Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9 Signed-off-by: roccochen(a)chromium.com <roccochen(a)chromium.org> --- M ichspi.c 1 file changed, 63 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/57/81357/1 diff --git a/ichspi.c b/ichspi.c index bee5ec9..8e98de5 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,58 @@ "read-write", "write-only", "read-only", "locked" }; +static void ich_get_bios_region_access(uint32_t *region_read_access, + uint32_t *region_write_access) +{ + uint32_t tmp; + *region_read_access = 0; + *region_write_access = 0; + + switch (ich_generation) { + case 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); + break; + + default: + /* + * 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 = ICH_BRRA(tmp); + *region_write_access = ICH_BRWA(tmp); + break; + } +} + +inline static unsigned int ich_get_defined_region_count(void) { + switch (ich_generation) { + case CHIPSET_METEOR_LAKE: + return 16; + default: + return 8; + } +} + static enum ich_access_protection ich9_handle_frap(struct fd_region *fd_regions, - uint32_t frap, unsigned int i) + uint32_t region_read_access, + uint32_t region_write_access, unsigned int i) { static const char *const region_names[] = { "Flash Descriptor", "BIOS", "Management Engine", @@ -1863,12 +1916,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*). */ @@ -2175,16 +2228,14 @@ } 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)); + unsigned int region_read_access, region_write_access; + ich_get_bios_region_access(&region_read_access, &region_write_access); /* Handle FREGx and FRAP 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_frap(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 " -- 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 Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9 Gerrit-Change-Number: 81357 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2024