November 2022 - flashrom-gerrit

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

30 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 13: Code-Review+2 (1 comment) File spi25_statusreg.c: https://review.coreboot.org/c/flashrom/+/69129/comment/17d43d8c_4572206b PS7, Line 133: /* Some SPI masters like ICH7/ICH9 block opcodes for writing SR2/SR3/etc. */ > I thought so too but when I was debugging cyan/wizpig failures in b/240229722 I found that it didn't […] Can you include that bug in the commit message to trace things back when required. -- 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: 13 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: Wed, 30 Nov 2022 10:20:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes 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]: flashrom: Skip read/write/erase/verify ops on inaccessable flash regions
by Nikolai Artemiev (Code Review) 30 Nov '22

30 Nov '22

Attention is currently required from: Edward O'Callaghan. Hello build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70128 to look at the new patch set (#6). Change subject: flashrom: Skip read/write/erase/verify ops on inaccessable flash regions ...................................................................... flashrom: Skip read/write/erase/verify ops on inaccessable flash regions Skip flash regions that get_region() indicates are inaccessable. This fixes transaction errors due to accessing invalid flash regions on Intel platforms with active an CSME coprocessor. BUG=b:260440773 BRANCH=none TEST=flashrom -{r,w,E,v} on dedede (JSL) Change-Id: I9b96fb04b863625d2c9f9a00b97c35b3ddb0871b Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c 1 file changed, 179 insertions(+), 20 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/28/70128/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/70128 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b96fb04b863625d2c9f9a00b97c35b3ddb0871b Gerrit-Change-Number: 70128 Gerrit-PatchSet: 6 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom: Skip read/write/erase/verify ops on inaccessable flash regions
by Nikolai Artemiev (Code Review) 30 Nov '22

30 Nov '22

Attention is currently required from: Edward O'Callaghan. Hello build bot (Jenkins), Edward O'Callaghan, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70128 to look at the new patch set (#5). Change subject: flashrom: Skip read/write/erase/verify ops on inaccessable flash regions ...................................................................... flashrom: Skip read/write/erase/verify ops on inaccessable flash regions Skip flash regions that get_region() indicates are inaccessable. This fixes transaction errors due to accessing invalid flash regions on Intel platforms with active an CSME coprocessor. BUG=b:260440773 BRANCH=none TEST=todo Change-Id: I9b96fb04b863625d2c9f9a00b97c35b3ddb0871b Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c 1 file changed, 175 insertions(+), 20 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/28/70128/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/70128 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b96fb04b863625d2c9f9a00b97c35b3ddb0871b Gerrit-Change-Number: 70128 Gerrit-PatchSet: 5 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tests: Mock the mode_t variant of open
by Evan Benn (Code Review) 30 Nov '22

30 Nov '22

Attention is currently required from: Anastasia Klimchuk. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69263 ) Change subject: tests: Mock the mode_t variant of open ...................................................................... Patch Set 8: (1 comment) File tests/io_mock.h: https://review.coreboot.org/c/flashrom/+/69263/comment/7facaecb_d2fb528f PS4, Line 37: #include <fcntl.h> > https://www.freebsd.org/cgi/man.cgi?query=fcntl&sektion=2 […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/69263 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8c134e6d36a248d0f51985e389085a9e585fb83d Gerrit-Change-Number: 69263 Gerrit-PatchSet: 8 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 30 Nov 2022 05:37:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: Redirect to real I/O to support coverage run
by Evan Benn (Code Review) 30 Nov '22

30 Nov '22

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69539 ) Change subject: tests: Redirect to real I/O to support coverage run ...................................................................... Patch Set 2: (1 comment) File tests/io_real.h: https://review.coreboot.org/c/flashrom/+/69539/comment/5dc00c66_795caff2 PS2, Line 16: /* Detect file io that should not be mocked, for example code coverage writing > add header guards. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/69539 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0817fce6ea0f53a4c127794a0d8246504675f805 Gerrit-Change-Number: 69539 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 30 Nov 2022 05:37:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: Redirect to real I/O to support coverage run
by Evan Benn (Code Review) 30 Nov '22

30 Nov '22

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk, Evan Benn. Hello build bot (Jenkins), Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/69539 to look at the new patch set (#3). Change subject: tests: Redirect to real I/O to support coverage run ...................................................................... tests: Redirect to real I/O to support coverage run Implement a check that redirects mock io functions to the real implementations. Real I/O functions are needed for the coverage tool to be able to create and write files. BUG=None BRANCH=None TEST=None Change-Id: I0817fce6ea0f53a4c127794a0d8246504675f805 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- A tests/io_real.c A tests/io_real.h M tests/meson.build M tests/wraps.h 4 files changed, 109 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/69539/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/69539 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0817fce6ea0f53a4c127794a0d8246504675f805 Gerrit-Change-Number: 69539 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom: Use get_region skip reading/writing inaccessable flash regions
by Nikolai Artemiev (Code Review) 30 Nov '22

30 Nov '22

Attention is currently required from: Edward O'Callaghan. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70128 ) Change subject: flashrom: Use get_region skip reading/writing inaccessable flash regions ...................................................................... Patch Set 4: This change is ready for review. -- To view, visit https://review.coreboot.org/c/flashrom/+/70128 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9b96fb04b863625d2c9f9a00b97c35b3ddb0871b Gerrit-Change-Number: 70128 Gerrit-PatchSet: 4 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 30 Nov 2022 05:12:40 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi: Expose flash descriptor regions through get_region()
by Nikolai Artemiev (Code Review) 30 Nov '22

30 Nov '22

Attention is currently required from: Edward O'Callaghan. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70127 ) Change subject: ichspi: Expose flash descriptor regions through get_region() ...................................................................... Patch Set 3: (1 comment) This change is ready for review. File ichspi.c: https://review.coreboot.org/c/flashrom/+/70127/comment/2fa4e7b3_5eaadda4 PS2, Line 1830: : : #define EMBEDDED_CONTROLLER_REGION 8 : : static enum ich_access_protection ec_region_rwperms(unsigned int i) : { > I'll probably factor out of this patch. I'm not sure if we need this at all actually. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/70127 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I019f3f407f6a2a82f686a168457e0e32961ff483 Gerrit-Change-Number: 70127 Gerrit-PatchSet: 3 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Wed, 30 Nov 2022 05:11:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: ichspi: Handle EC region permissions
by Nikolai Artemiev (Code Review) 30 Nov '22

30 Nov '22

Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/70169 ) Change subject: ichspi: Handle EC region permissions ...................................................................... ichspi: Handle EC region permissions WIP. Not sure if we need this. BUG=b:260440773 BRANCH=none TEST=todo Change-Id: Ieaf2a960ec0c72d7b576dc3733260592c7c1f766 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M ichspi.c 1 file changed, 53 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/70169/1 diff --git a/ichspi.c b/ichspi.c index 06f94c7..5bf8790 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1806,6 +1806,40 @@ "read-write", "write-only", "read-only", "locked" }; +#define EMBEDDED_CONTROLLER_REGION 8 + +static enum ich_access_protection ec_region_rwperms(unsigned int i) +{ + /* + * Use Flash Descriptor Observe register to determine if + * the EC region can be written by the BIOS master. + */ + int rwperms = NO_PROT; + + struct ich_descriptors desc; + memset(&desc, 0, sizeof(desc)); + + /* Region is RW if flash descriptor override is set */ + uint16_t freg = REGREAD16(ICH9_REG_HSFS); + if ((freg & HSFS_FDV) && !(freg & HSFS_FDOPSS)) { + rwperms = NO_PROT; + } else if (read_ich_descriptors_via_fdo(ich_generation, ich_spibar, &desc) == ICH_RET_OK) { + const struct ich_desc_master *const mstr = &desc.master; + int bios_ec_r = mstr->mstr[i].read & BIT(16); /* BIOS_EC_r in PCH100+ */ + int bios_ec_w = mstr->mstr[i].write & BIT(28); /* BIOS_EC_w in PCH100+ */ + if (bios_ec_r && bios_ec_w) + rwperms = NO_PROT; + else if (bios_ec_r && !bios_ec_w) + rwperms = WRITE_PROT; + else if (!bios_ec_r && bios_ec_w) + rwperms = READ_PROT; + else + rwperms = LOCKED; + } + + return rwperms; +} + static enum ich_access_protection ich9_handle_frap(uint32_t frap, unsigned int i) { static const char *const region_names[] = { @@ -1833,10 +1867,12 @@ } msg_pdbg("0x%02X: 0x%08x ", offset, freg); - if (i < 8) { + if (i < EMBEDDED_CONTROLLER_REGION) { rwperms_idx = (((ICH_BRWA(frap) >> i) & 1) << 1) | (((ICH_BRRA(frap) >> i) & 1) << 0); rwperms = access_perms_to_protection[rwperms_idx]; + } else if (i == EMBEDDED_CONTROLLER_REGION && ich_generation >= CHIPSET_100_SERIES_SUNRISE_POINT) { + rwperms = ec_region_rwperms(i); } else { /* Datasheets don't define any access bits for regions > 7. We can't rely on the actual descriptor settings either as there -- To view, visit https://review.coreboot.org/c/flashrom/+/70169 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ieaf2a960ec0c72d7b576dc3733260592c7c1f766 Gerrit-Change-Number: 70169 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: cli, manpage: Clean up occurrences of old image argument
by Edward O'Callaghan (Code Review) 30 Nov '22

30 Nov '22

Attention is currently required from: Felix Singer, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70003 ) Change subject: cli, manpage: Clean up occurrences of old image argument ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/70003 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If457e2b8548034868105b515125d7e8b4d5f6134 Gerrit-Change-Number: 70003 Gerrit-PatchSet: 3 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: 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 30 Nov 2022 04:38:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022