November 2021 - flashrom-gerrit

Change in flashrom[master]: chipset_enable.c: Mark Intel Q67 as DEP
by Angel Pons (Code Review) Nov. 1, 2021

Nov. 1, 2021

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/47797 ) Change subject: chipset_enable.c: Mark Intel Q67 as DEP ...................................................................... chipset_enable.c: Mark Intel Q67 as DEP Tested reading, writing and erasing the internal flash chip using an HP Elite 8200 mainboard with an Intel Q67 PCH. However, since ME-enabled chipsets are marked as DEP instead of OK, this one shall also be. Change-Id: I2bd431c5c72824654b6b5b840f9af55dfe9d3554 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M chipset_enable.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/47797/1 diff --git a/chipset_enable.c b/chipset_enable.c index 04ff9d8..5e4a547 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1829,7 +1829,7 @@ {0x8086, 0x1c4b, B_FS, NT, "Intel", "HM67", enable_flash_pch6}, {0x8086, 0x1c4c, B_FS, NT, "Intel", "Q65", enable_flash_pch6}, {0x8086, 0x1c4d, B_FS, NT, "Intel", "QS67", enable_flash_pch6}, - {0x8086, 0x1c4e, B_FS, NT, "Intel", "Q67", enable_flash_pch6}, + {0x8086, 0x1c4e, B_FS, DEP, "Intel", "Q67", enable_flash_pch6}, {0x8086, 0x1c4f, B_FS, DEP, "Intel", "QM67", enable_flash_pch6}, {0x8086, 0x1c50, B_FS, NT, "Intel", "B65", enable_flash_pch6}, {0x8086, 0x1c52, B_FS, NT, "Intel", "C202", enable_flash_pch6}, -- To view, visit https://review.coreboot.org/c/flashrom/+/47797 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2bd431c5c72824654b6b5b840f9af55dfe9d3554 Gerrit-Change-Number: 47797 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newchange

4 5

Change in flashrom[master]: [RFC] writeprotect: implement wp_{get,set}_range()
by Nikolai Artemiev (Code Review) Nov. 1, 2021

Nov. 1, 2021

Attention is currently required from: Nico Huber. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58482 ) Change subject: [RFC] writeprotect: implement wp_{get,set}_range() ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: > I guess a separate tool that handles only block protection (or write > protection or flash configuration in general) would be much easier to maintain. I think you mean building a separate binary from common sources right? Splitting off an entirely separate project would be a lot of extra work, especially when read/write/erase operations need some of the same functionality (i.e. disabling block protection). I'm fine with building a separate binary, though I think we would need to be careful about how we partition the functionality to avoid feature duplication. > I would much prefer to only call libflashrom functions from any CLI code. I agree, I definitely want to move the writeprotect interface into libflashrom. I thought it would be easier to call the functions directly for now, but it shouldn't take too much work to squash writeprotect.h into libflashrom.h. -- To view, visit https://review.coreboot.org/c/flashrom/+/58482 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7d26f43fb05c5828b9839bb57a28fa1088dcd9a0 Gerrit-Change-Number: 58482 Gerrit-PatchSet: 6 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Mon, 01 Nov 2021 10:24:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: chipset_enable.c: Mark Intel Z390 as DEP
by Angel Pons (Code Review) Nov. 1, 2021

Nov. 1, 2021

Attention is currently required from: melvyn2. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58774 ) Change subject: chipset_enable.c: Mark Intel Z390 as DEP ...................................................................... Patch Set 3: Code-Review+1 (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/58774/comment/e0771009_10192c29 PS3, Line 9: me_cleaner Minor: Our guidelines state that commit message lines wrap at 72 characters [1]. `me_cleaner` goes past the limit, so it should go in a new line. [1]: https://flashrom.org/Development_Guidelines#Commit_message https://review.coreboot.org/c/flashrom/+/58774/comment/9611dc8c_d84dcd52 PS3, Line 12: melvyn2 As per our Sign-off procedure [1], I'm afraid you have to use your real name here. [1]: https://flashrom.org/Development_Guidelines#Sign-off_Procedure Patchset: PS3: Welcome! The change itself looks good, but the sign-off needs to be addressed. -- To view, visit https://review.coreboot.org/c/flashrom/+/58774 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If14d45c144bb32a1d1046185d4476ea29e4d0912 Gerrit-Change-Number: 58774 Gerrit-PatchSet: 3 Gerrit-Owner: melvyn2 <melvyn2(a)brcok.tk> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: melvyn2 <melvyn2(a)brcok.tk> Gerrit-Comment-Date: Mon, 01 Nov 2021 09:27:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add wraps for __xstat variants of stat
by Anastasia Klimchuk (Code Review) Nov. 1, 2021

Nov. 1, 2021

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58103 to look at the new patch set (#4). Change subject: tests: Add wraps for __xstat variants of stat ...................................................................... tests: Add wraps for __xstat variants of stat __xstat variants of stat are invoked under chromium chroot. For all existing tests it is sufficient for stat to "do nothing, return 0", so new wraps do just that. Without this patch linux_mtd test fails under chromium chroot. BUG=b:181803212 TEST=running tests on three different environments, 1) stat64 (ninja tests in upstream tree) 2) stat64 (ninja tests in chromium tree) 2) __xstat64 (emerge with tests in chromium tree) Change-Id: I4c5c243acde09dc5bb6b2a14042fcd23a49707db Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/meson.build M tests/tests.c 2 files changed, 14 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/58103/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/58103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c5c243acde09dc5bb6b2a14042fcd23a49707db Gerrit-Change-Number: 58103 Gerrit-PatchSet: 4 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Add wraps for all variants of stat
by Anastasia Klimchuk (Code Review) Nov. 1, 2021

Nov. 1, 2021

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58103 ) Change subject: tests: Add wraps for all variants of stat ...................................................................... Patch Set 3: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/58103/comment/a917cdb8_c9b64ed7 PS2, Line 9: all > Yes you are right, I will do this. […] Environment this applies to: __xstat64 wrap is needed to run unit tests with emerge under chromium chroot. When I run same tests, in the same chromium tree, with meson, then stat64 is called. There was, as I understand, some change in chromium chroot recently, which caused the switch from stat64 to __xstat64, however I couldn't find what exactly has changed. Good news: "some flashrom developer" won't have a failing test locally, because this only affects people who are running tests with emerge under chromium chroot. Bad news: this affects me :( Partially, because I can work on upstream tree as usual. I will re-write commit message later today, I agree it is too vague. Did I answer your question about environment? Let me know if not! https://review.coreboot.org/c/flashrom/+/58103/comment/7e03e1f7_6d2987c4 PS2, Line 17: TEST=running tests on two different environments, : with 1) stat64 and 2) __xstat64 invoked > Yes sure, I will do this. Let's keep this comment unresolved. I looked into all potentially relevant minijail policies, could not find any recent policy change that would look like it caused switching from stat64 to __xstat64. Maybe it's not minijail. > before being sure we need this wrap Currently, anyone who wants to run unit tests with emerge under chroot, needs this wrap. I think we need it... Patchset: PS3: Sorry for delay, I was gathering info. I am back. -- To view, visit https://review.coreboot.org/c/flashrom/+/58103 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I4c5c243acde09dc5bb6b2a14042fcd23a49707db Gerrit-Change-Number: 58103 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 01 Nov 2021 06:16:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: [RFC] writeprotect: implement wp_get_ranges()
by Nikolai Artemiev (Code Review) Nov. 1, 2021

Nov. 1, 2021

Attention is currently required from: Sergii Dmytruk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58481 ) Change subject: [RFC] writeprotect: implement wp_get_ranges() ...................................................................... Patch Set 6: (1 comment) File writeprotect.h: https://review.coreboot.org/c/flashrom/+/58481/comment/4e9ef98b_36c4fa28 PS6, Line 57: wp_get_ranges > How about renaming it to, say, `wp_list_ranges` to make it less confusing? This will separate it fro […] I previously had wp_get_available_ranges() but thought it was a bit too long. Maybe we should call it wp_get_range_list() since it's returning the list rather than printing it. -- To view, visit https://review.coreboot.org/c/flashrom/+/58481 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id51f038f03305c8536d80313e52f77d27835f34d Gerrit-Change-Number: 58481 Gerrit-PatchSet: 6 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 01 Nov 2021 04:44:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0