November 2022 - flashrom-gerrit

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

Nov. 30, 2022

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) Nov. 30, 2022

Nov. 30, 2022

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) Nov. 30, 2022

Nov. 30, 2022

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) Nov. 30, 2022

Nov. 30, 2022

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

[M] Change in flashrom[master]: libflashrom.c: Invert if conditions to improve the readability
by Edward O'Callaghan (Code Review) Nov. 30, 2022

Nov. 30, 2022

Attention is currently required from: Felix Singer. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68153 ) Change subject: libflashrom.c: Invert if conditions to improve the readability ...................................................................... Patch Set 3: Code-Review+2 (2 comments) File libflashrom.c: https://review.coreboot.org/c/flashrom/+/68153/comment/595b979b_644066f9 PS3, Line 103: (; optional: while we are here could we just scope the indexer into the loop construct. `for (unsigned int i = 0; ..` https://review.coreboot.org/c/flashrom/+/68153/comment/a4fba167_a89c6c3f PS3, Line 107: (enum flashrom_test_state) is this cast needed? -- To view, visit https://review.coreboot.org/c/flashrom/+/68153 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4021d8802cd041dcca29a226af0798ebd9c5a81b Gerrit-Change-Number: 68153 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Singer <felixsinger(a)posteo.net> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Wed, 30 Nov 2022 04:35:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: Redirect to real I/O to support coverage run
by Edward O'Callaghan (Code Review) Nov. 30, 2022

Nov. 30, 2022

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk, Evan Benn. Edward O'Callaghan 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: Code-Review+2 (1 comment) File tests/io_real.h: https://review.coreboot.org/c/flashrom/+/69539/comment/d1ebb546_ae46dd1a PS2, Line 16: /* Detect file io that should not be mocked, for example code coverage writing add header guards. -- 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-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Wed, 30 Nov 2022 04:13:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: Add fwrite and fdopen to io_mock
by Edward O'Callaghan (Code Review) Nov. 30, 2022

Nov. 30, 2022

Attention is currently required from: Anastasia Klimchuk, Evan Benn. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69538 ) Change subject: tests: Add fwrite and fdopen to io_mock ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69538 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4dff96c264b3ada354538b434b2808fb66c7ef59 Gerrit-Change-Number: 69538 Gerrit-PatchSet: 4 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: 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-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Wed, 30 Nov 2022 04:12:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: Mock the mode_t variant of open
by Edward O'Callaghan (Code Review) Nov. 30, 2022

Nov. 30, 2022

Attention is currently required from: Anastasia Klimchuk, Evan Benn. Edward O'Callaghan 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: Code-Review+2 (1 comment) File tests/io_mock.h: https://review.coreboot.org/c/flashrom/+/69263/comment/fbd8b185_e148a0f9 PS4, Line 37: #include <fcntl.h> > I will try to test https://www.freebsd.org/cgi/man.cgi?query=fcntl&sektion=2 https://docs.oracle.com/cd/E23824_01/html/821-1463/fcntl-2.html https://man.netbsd.org/fcntl.2 should be fine. -- 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-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Wed, 30 Nov 2022 04:11:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes 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

[S] Change in flashrom[master]: tests: Move test.h include
by Edward O'Callaghan (Code Review) Nov. 30, 2022

Nov. 30, 2022

Attention is currently required from: Anastasia Klimchuk, Evan Benn. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69537 ) Change subject: tests: Move test.h include ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69537 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8e0611c415c921f5b04b20270fb26e147fefd1b8 Gerrit-Change-Number: 69537 Gerrit-PatchSet: 4 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: 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-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Wed, 30 Nov 2022 04:07:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: Add llvm-cov option and run target for code coverage
by Edward O'Callaghan (Code Review) Nov. 30, 2022

Nov. 30, 2022

Attention is currently required from: Thomas Heijligen, Anastasia Klimchuk, Evan Benn. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69268 ) Change subject: tests: Add llvm-cov option and run target for code coverage ...................................................................... Patch Set 12: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69268 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id6c73bff46e7b88d425956a80def97082b201f56 Gerrit-Change-Number: 69268 Gerrit-PatchSet: 12 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-Comment-Date: Wed, 30 Nov 2022 04:06:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0