February 2022 - flashrom-gerrit

Change in flashrom[master]: Makefile: use libflashrom.a as input to build the flashrom executable
by Thomas Heijligen (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Felix Singer, Nico Huber. Hello Felix Singer, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62198 to look at the new patch set (#2). Change subject: Makefile: use libflashrom.a as input to build the flashrom executable ...................................................................... Makefile: use libflashrom.a as input to build the flashrom executable Change-Id: Ib0091a23611cd5a1d915e56c6d0f061d74198e88 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile 1 file changed, 5 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/62198/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/62198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib0091a23611cd5a1d915e56c6d0f061d74198e88 Gerrit-Change-Number: 62198 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: Makefile: use HAS_ USE_ pattern for serial support
by Thomas Heijligen (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Nico Huber, Thomas Heijligen. Hello Felix Singer, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62196 to look at the new patch set (#2). Change subject: Makefile: use HAS_ USE_ pattern for serial support ...................................................................... Makefile: use HAS_ USE_ pattern for serial support Align the usage of serial function with the selection of other dependencies. Change-Id: Ica951e76d6362b01f09d23a729a2a6049e7f0b66 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile 1 file changed, 7 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/62196/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/62196 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ica951e76d6362b01f09d23a729a2a6049e7f0b66 Gerrit-Change-Number: 62196 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: Makefile: Rework the EXEC_SUFFIX determination
by Thomas Heijligen (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Nico Huber, Thomas Heijligen. Hello Felix Singer, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62195 to look at the new patch set (#2). Change subject: Makefile: Rework the EXEC_SUFFIX determination ...................................................................... Makefile: Rework the EXEC_SUFFIX determination Use a conditional function for the statement. This limits the decision to one line instead of multiple places. Change-Id: Iee66dbc609bd5c6eb9d04b457f4508911b2e6560 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile 1 file changed, 1 insertion(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/95/62195/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/62195 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iee66dbc609bd5c6eb9d04b457f4508911b2e6560 Gerrit-Change-Number: 62195 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashrom.c: Implement file-based locking semantics
by Subrata Banik (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Nico Huber, Edward O'Callaghan. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62213 ) Change subject: flashrom.c: Implement file-based locking semantics ...................................................................... Patch Set 2: (1 comment) File big_lock.h: https://review.coreboot.org/c/flashrom/+/62213/comment/0f0e458b_7cbafeca PS2, Line 1: 2012 need update may be ? -- To view, visit https://review.coreboot.org/c/flashrom/+/62213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Gerrit-Change-Number: 62213 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 21 Feb 2022 06:40:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Implement file-based locking semantics
by Hung-Te Lin (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Nico Huber, Subrata Banik, Edward O'Callaghan. Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62213 ) Change subject: flashrom.c: Implement file-based locking semantics ...................................................................... Patch Set 2: (3 comments) File big_lock.c: https://review.coreboot.org/c/flashrom/+/62213/comment/1bda94c6_23a8a0ff PS1, Line 39: return acquire_lock(&big_lock, timeout_secs * 1000); > Seems unnecessary complexity to make it conditional since we would always want it to be the case tha […] I'm concerned the file-based locking may not work on all systems, for example Android or Windows, or any other systems that previously supports flashrom without problems. File big_lock.c: https://review.coreboot.org/c/flashrom/+/62213/comment/f8ca8977_704e1087 PS2, Line 37: firmware_utility_lock should this be flashrom_lock ? File flashrom.c: https://review.coreboot.org/c/flashrom/+/62213/comment/e2d2e50f_5c714c0f PS1, Line 142: programmer_entry > afaik, this is issue is specifically related to futility tests not yet running under platform2_wrapp […] Not just futility - this is more like "can you run flashrom without getting root permission (required by lock)". I think people will want to simulate a flashrom call using dummy programmer (to create an image file) without adding 'sudo'. But on the other hand, it's also probably not a bad idea if we always require 'sudo' - even for unit tests. I'm fine if you want to keep the biglock always there. One more thing to check: we won't use two programmers simultaneously right? Because currently the lock is a simple 1/0 state. We may need to consider a real singleton (e.g., add ref counter) if some programs calling libflashrom will try to get two programmers , for example read from one and write to the other. (flashrom doesn't do this but we don't know what libflashrom users will do) -- To view, visit https://review.coreboot.org/c/flashrom/+/62213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Gerrit-Change-Number: 62213 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 21 Feb 2022 05:29:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Implement file-based locking semantics
by Edward O'Callaghan (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Hung-Te Lin, Nico Huber, Subrata Banik. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62213 ) Change subject: flashrom.c: Implement file-based locking semantics ...................................................................... Patch Set 1: (5 comments) File big_lock.c: https://review.coreboot.org/c/flashrom/+/62213/comment/8d54b14c_fa792ee1 PS1, Line 39: return acquire_lock(&big_lock, timeout_secs * 1000); > should we check #if BIG_LOCK and decide if lock is required? Seems unnecessary complexity to make it conditional since we would always want it to be the case that two flashrom instances never run concurrently? File flashrom.c: https://review.coreboot.org/c/flashrom/+/62213/comment/dc7b4f24_14b85557 PS1, Line 51: #ifndef USE_BIG_LOCK : #define USE_BIG_LOCK 0 : #endif > Not really used? Done https://review.coreboot.org/c/flashrom/+/62213/comment/587389ae_175cce24 PS1, Line 142: programmer_entry > Is it possible to add an attribute to programmer_entry so we can know if big_lock is required? Then […] afaik, this is issue is specifically related to futility tests not yet running under platform2_wrapper so that is just a tmp workaround until we fix that. Therefore upstream shouldn't be burden with the issue that is our own. WDYT? https://review.coreboot.org/c/flashrom/+/62213/comment/7b97253e_de6983ed PS1, Line 151: /* Get big lock before doing any work that touches hardware. */ : msg_gdbg("Acquiring lock (timeout=%d sec)...\n", LOCK_TIMEOUT_SECS); : if (acquire_big_lock(LOCK_TIMEOUT_SECS) < 0) { : msg_gerr("Could not acquire lock.\n"); : return -1; : } : big_lock_acquired = true; : msg_gdbg("Lock acquired.\n"); : > If we're going to have a big_lock. […] Sounds good, Done. https://review.coreboot.org/c/flashrom/+/62213/comment/77b6a8f3_a57b7df2 PS1, Line 217: if (big_lock_acquired) { : release_big_lock(); : big_lock_acquired = false; : } > we should just call release_big_lock() and let it check/handle big_lock_acquired internally Done -- To view, visit https://review.coreboot.org/c/flashrom/+/62213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Gerrit-Change-Number: 62213 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Comment-Date: Mon, 21 Feb 2022 05:05:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Implement file-based locking semantics
by Edward O'Callaghan (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Nico Huber, Subrata Banik, Edward O'Callaghan. Hello Hung-Te Lin, build bot (Jenkins), Nico Huber, Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62213 to look at the new patch set (#2). Change subject: flashrom.c: Implement file-based locking semantics ...................................................................... flashrom.c: Implement file-based locking semantics This upstreams the ChromiumOS implementation of file-based locking for multiple instances of flashrom that could be spawned either from libflashrom (perhaps by fwupd for example) and user cli as another example. Since flashrom is programming singleton state of hardware from userspace there is no way to exclusively own the address space and therefore a file-based locking semantic is considered here. BUG=b:217629892,b:215255210 BRANCH=none TEST=nm -gD /build/brya/usr/lib64/libflashrom.so | grep "flock" Test futility update with multiple instances of flashrom running. Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M Makefile A big_lock.c A big_lock.h A file_lock.c M flashrom.c A ipc_lock.h M meson.build 7 files changed, 443 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/13/62213/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/62213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Gerrit-Change-Number: 62213 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashrom.c: Implement file-based locking semantics
by Hung-Te Lin (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Nico Huber, Subrata Banik, Edward O'Callaghan. Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62213 ) Change subject: flashrom.c: Implement file-based locking semantics ...................................................................... Patch Set 1: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/62213/comment/8c2ddf62_9b9f094a PS1, Line 142: programmer_entry Is it possible to add an attribute to programmer_entry so we can know if big_lock is required? Then set dummy programmer to false. -- To view, visit https://review.coreboot.org/c/flashrom/+/62213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Gerrit-Change-Number: 62213 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 21 Feb 2022 03:10:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashrom.c: Implement file-based locking semantics
by Hung-Te Lin (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Nico Huber, Subrata Banik, Edward O'Callaghan. Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62213 ) Change subject: flashrom.c: Implement file-based locking semantics ...................................................................... Patch Set 1: (4 comments) File big_lock.c: https://review.coreboot.org/c/flashrom/+/62213/comment/a75f89f5_7dccf3a7 PS1, Line 39: return acquire_lock(&big_lock, timeout_secs * 1000); should we check #if BIG_LOCK and decide if lock is required? File flashrom.c: https://review.coreboot.org/c/flashrom/+/62213/comment/1513440f_8be62fe3 PS1, Line 51: #ifndef USE_BIG_LOCK : #define USE_BIG_LOCK 0 : #endif Not really used? https://review.coreboot.org/c/flashrom/+/62213/comment/05e159b1_68263457 PS1, Line 151: /* Get big lock before doing any work that touches hardware. */ : msg_gdbg("Acquiring lock (timeout=%d sec)...\n", LOCK_TIMEOUT_SECS); : if (acquire_big_lock(LOCK_TIMEOUT_SECS) < 0) { : msg_gerr("Could not acquire lock.\n"); : return -1; : } : big_lock_acquired = true; : msg_gdbg("Lock acquired.\n"); : If we're going to have a big_lock.c, I think the implementation here should only contain the timeout, not the status and debug messages; e.g., if (acquire_big_lock(LOCK_TIMEOUT_SECS) < 0) return -1; And move all other logic into acquire_big_lock itself. WDYT? https://review.coreboot.org/c/flashrom/+/62213/comment/c9b83ce3_65a8c0ce PS1, Line 217: if (big_lock_acquired) { : release_big_lock(); : big_lock_acquired = false; : } we should just call release_big_lock() and let it check/handle big_lock_acquired internally -- To view, visit https://review.coreboot.org/c/flashrom/+/62213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Gerrit-Change-Number: 62213 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 21 Feb 2022 03:09:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips,writeprotect_ranges: add range decoding function
by Anastasia Klimchuk (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Nico Huber, Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58480 ) Change subject: flashchips,writeprotect_ranges: add range decoding function ...................................................................... Patch Set 39: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/58480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id163ed80938a946a502ed116e48e8236e36eb203 Gerrit-Change-Number: 58480 Gerrit-PatchSet: 39 Gerrit-Owner: Nikolai Artemiev <nartemiev(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: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Mon, 21 Feb 2022 01:50:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit February 2022