February 2022 - flashrom-gerrit

Change in flashrom[master]: hwaccess_x86_io: refactor rget_io_perms()
by Thomas Heijligen (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Nico Huber, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61275 ) Change subject: hwaccess_x86_io: refactor rget_io_perms() ...................................................................... Patch Set 7: (3 comments) File hwaccess_x86_io.c: https://review.coreboot.org/c/flashrom/+/61275/comment/461e6567_7c136594 PS4, Line 124: msg_perr("Make sure you are root.\n"); > If we change this already... There were some reports lately that the message […] Done https://review.coreboot.org/c/flashrom/+/61275/comment/62169873_92675f15 PS4, Line 126: "reboot, or reboot into single user mode.\n"); > would be nice to keep the alignment, i.e. […] Done https://review.coreboot.org/c/flashrom/+/61275/comment/4dcb6f46_40f5f5a5 PS4, Line 128: "that your kernel configuration has the option INSECURE enabled.\n"); > would be nice to keep the alignment, i.e. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/61275 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If4b2f8c2532f3732086ee1d479da6ae6693f9a42 Gerrit-Change-Number: 61275 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 21 Feb 2022 16:04:26 +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]: hwaccess_x86_io: clean header concept
by Thomas Heijligen (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Nico Huber, Angel Pons. Hello build bot (Jenkins), Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/61276 to look at the new patch set (#11). Change subject: hwaccess_x86_io: clean header concept ...................................................................... hwaccess_x86_io: clean header concept Move all function implementations into the .c file Change-Id: I1400704e9ac5fed00c096796536108d5bfb875e3 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile M hwaccess_x86_io.c M hwaccess_x86_io.h M meson.build 4 files changed, 298 insertions(+), 188 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/76/61276/11 -- To view, visit https://review.coreboot.org/c/flashrom/+/61276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1400704e9ac5fed00c096796536108d5bfb875e3 Gerrit-Change-Number: 61276 Gerrit-PatchSet: 11 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: hwaccess_x86_io: refactor rget_io_perms()
by Thomas Heijligen (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Thomas Heijligen, Angel Pons. Hello build bot (Jenkins), Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/61275 to look at the new patch set (#7). Change subject: hwaccess_x86_io: refactor rget_io_perms() ...................................................................... hwaccess_x86_io: refactor rget_io_perms() Abstract the different I/O Port permission methods in own functions. Change-Id: If4b2f8c2532f3732086ee1d479da6ae6693f9a42 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M hwaccess_x86_io.c 1 file changed, 98 insertions(+), 50 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/61275/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/61275 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If4b2f8c2532f3732086ee1d479da6ae6693f9a42 Gerrit-Change-Number: 61275 Gerrit-PatchSet: 7 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

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

21 Feb '22

Attention is currently required from: Hung-Te Lin, Subrata Banik, Edward O'Callaghan. Nico Huber 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/aea89654_e6e0be8c PS2, Line 1: 2012 > > Copyright lines are supposed to tell the date of the original publication. […] As the commit message suggests, it's not new, just new to this repository. -- 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: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 21 Feb 2022 15:51:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

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: Hung-Te Lin, 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/e509d8e8_f12599f6 PS2, Line 1: 2012 > Copyright lines are supposed to tell the date of the original publication. Exactly that is my point as well. This is new file right, at least git history shows the same. Is it reasonable that a new file has copyright year from past is my point. > There are two lines... Was there anything significant added in the meantime? oO -- 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: Hung-Te Lin <hungte(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 15:07:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

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

21 Feb '22

Attention is currently required from: Hung-Te Lin, Subrata Banik, Edward O'Callaghan. Nico Huber 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) Patchset: PS2: Do we need a "big lock"? libusb based drivers, for instance, they have to call libusb_claim_interface() (I'm not sure though, if that's enough on all platforms). So they might be covered already. Generally, I'd prefer to lock individual resources, not the whole program. Wasn't flashrom used in Chromebook manufacturing? Seems possible that they run only one instance per host, but I wouldn't have expected that. FWIW, a sane candidate for locking is our physmap infrastructure. We could actually lock individual resources by encoding the base address into the file name, e.g. `flashrom.physmap-0x12345678.lock`. Port-based I/O also comes to mind. File big_lock.h: https://review.coreboot.org/c/flashrom/+/62213/comment/2f76fcbf_426a5bff PS2, Line 1: 2012 > need update may be ? Copyright lines are supposed to tell the date of the original publication. There are two lines... Was there anything significant added in the meantime? oO File flashrom.c: https://review.coreboot.org/c/flashrom/+/62213/comment/a40bde94_ae1e2d6d PS1, Line 142: programmer_entry > 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'. Why would a lock need root permission? Can't we just place a file in `/tmp/`? > > 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. FWIW, it's generally considered a bad idea to require `sudo`. -- 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: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 21 Feb 2022 15:03:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hung-Te Lin <hungte(a)chromium.org> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: print version info as part of the config target
by Nico Huber (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Felix Singer, Thomas Heijligen. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62199 ) Change subject: Makefile: print version info as part of the config target ...................................................................... Patch Set 2: (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/62199/comment/39fd656d_d6402220 PS2, Line 899: PROGRAM What if somebody runs `make libflashrom.a`? I think we can just hard-code "Building flashrom ...". -- To view, visit https://review.coreboot.org/c/flashrom/+/62199 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1a846acfd8d2e0a9fc8b02c078b6ac0342438490 Gerrit-Change-Number: 62199 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Mon, 21 Feb 2022 14:40:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

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

21 Feb '22

Attention is currently required from: Felix Singer, Thomas Heijligen. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62198 ) Change subject: Makefile: use libflashrom.a as input to build the flashrom executable ...................................................................... Patch Set 2: Code-Review+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: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Mon, 21 Feb 2022 14:37:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: use the HAS_ USE_ scheme for linux i2c dependend programmer
by Nico Huber (Code Review) 21 Feb '22

21 Feb '22

Attention is currently required from: Felix Singer, Thomas Heijligen. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62197 ) Change subject: Makefile: use the HAS_ USE_ scheme for linux i2c dependend programmer ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: Wasn't there a new I2C driver? IIRC, it passed rather unreviewed, something mediatek. Should be added to DEPENDS_ON_LINUX_I2C. That deserves at separate commit, though. -- To view, visit https://review.coreboot.org/c/flashrom/+/62197 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I47acdf89a369441b9fc664352c27c43b461545b1 Gerrit-Change-Number: 62197 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-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Mon, 21 Feb 2022 14:35:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

21 Feb '22

Attention is currently required from: Felix Singer, Thomas Heijligen. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62196 ) Change subject: Makefile: use HAS_ USE_ pattern for serial support ...................................................................... Patch Set 2: (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/62196/comment/e5c800c6_82f3ad6e PS2, Line 807: ifneq ($(NEED_POSIX_SOCKETS), ) Not sure about this. It seems special to serprog. It looks like currently the set of OSes that don't support it matches the set with serial support. But that's coincidence, I guess. It definitely looks odd to place the LDFLAGS under USE_SERIAL. Maybe move it to the SERPROG check. Or even add a DEPENDS_ON_SOCKETS group? -- 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Mon, 21 Feb 2022 14:28:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit February 2022