Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60110 )
Change subject: hwaccess: move x86 port I/O related code into own files
......................................................................
Patch Set 5:
(1 comment)
File Makefile:
https://review.coreboot.org/c/flashrom/+/60110/comment/1565d87a_103a872c
PS4, Line 783: ifneq ($(NEED_RAW_ACCESS), )
: # Raw memory, MSR or PCI port I/O access.
: FEATURE_CFLAGS += -D'NEED_RAW_ACCESS=1' -D'__FLASHROM_HAVE_OUTB__=1'
: PROGRAMMER_OBJS += physmap.o hwaccess.o hwaccess_x86_io.o
> Yes, currently it's disabling all libpci programmer and rayer_spi.
is it realistic to check that such a case builds? does it matter? or maybe you did it already
> Building with `make NEED_RAW_ACCESS=` fails during linking.
Maybe I am missing something, why this is fine? was it always like this, building `make NEED_RAW_ACCESS=` never intent to work?
--
To view, visit https://review.coreboot.org/c/flashrom/+/60110
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I372b4a409f036da766c42bc406b596bc41b0f75a
Gerrit-Change-Number: 60110
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
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: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Fri, 17 Dec 2021 01:01:16 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58479 )
Change subject: libflashrom,writeprotect: add flashrom_wp_{read,write}_chip_config()
......................................................................
Patch Set 22:
(1 comment)
File writeprotect.c:
https://review.coreboot.org/c/flashrom/+/58479/comment/6957c4a1_1c2f2ed3
PS22, Line 56: struct wp_chip_config *cfg
Only realized this now: this requires the caller to know the size of
`struct wp_chip_config` which is currently not given by `libflashrom.h`.
IMO, we should use dynamic allocation instead, like in the rest of the
API. It will require yet another de-allocation function, but having a
clean, consistent API seems worth it.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58479
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad
Gerrit-Change-Number: 58479
Gerrit-PatchSet: 22
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: Edward O'Callaghan <quasisec(a)chromium.org>
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: Fri, 17 Dec 2021 00:56:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Sergii Dmytruk.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59183 )
Change subject: libflashrom,writeprotect: add flashrom_wp_get_range()
......................................................................
Patch Set 15:
(1 comment)
File writeprotect.c:
https://review.coreboot.org/c/flashrom/+/59183/comment/7fa716a4_c684d1ae
PS15, Line 189: decode_range
We should check here if the pointer is set.
--
To view, visit https://review.coreboot.org/c/flashrom/+/59183
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5a1dfcf384166b1bac319d286306747e1dcaa000
Gerrit-Change-Number: 59183
Gerrit-PatchSet: 15
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: Edward O'Callaghan <quasisec(a)chromium.org>
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: Fri, 17 Dec 2021 00:49:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk.
Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60110 )
Change subject: hwaccess: move x86 port I/O related code into own files
......................................................................
Patch Set 5:
(2 comments)
File Makefile:
https://review.coreboot.org/c/flashrom/+/60110/comment/c071fe9e_9a2c1f8f
PS4, Line 783: ifneq ($(NEED_RAW_ACCESS), )
: # Raw memory, MSR or PCI port I/O access.
: FEATURE_CFLAGS += -D'NEED_RAW_ACCESS=1' -D'__FLASHROM_HAVE_OUTB__=1'
: PROGRAMMER_OBJS += physmap.o hwaccess.o hwaccess_x86_io.o
> Mostly curiosity: what is the way to build with NEED_RAW_ACCESS empty? Would that be disabling all t […]
Yes, currently it's disabling all libpci programmer and rayer_spi. Building with `make NEED_RAW_ACCESS=` fails during linking.
On x86 Linux NEED_RAW_ACCESS is enabled by CONFIG_RAYER_SPI which builds by default and all libcpi programmer when libpci is found
File tests/hwaccess_x86_io_unittest.h:
https://review.coreboot.org/c/flashrom/+/60110/comment/f790396a_9fbacab0
PS4, Line 33: int rget_io_perms(void);
> If you could move it below all defines and includes (right above iopl)? thanks!
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/60110
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I372b4a409f036da766c42bc406b596bc41b0f75a
Gerrit-Change-Number: 60110
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 16 Dec 2021 20:39:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons.
Hello build bot (Jenkins), Nico Huber, Angel Pons, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/60110
to look at the new patch set (#5).
Change subject: hwaccess: move x86 port I/O related code into own files
......................................................................
hwaccess: move x86 port I/O related code into own files
Allow port I/O related code to be compiled independent from memory
mapping functionality. This enables for a better selection of needed
hardware access types.
Change-Id: I372b4a409f036da766c42bc406b596bc41b0f75a
Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com>
---
M Makefile
M amd_imc.c
M atahpt.c
M atapromise.c
M atavia.c
M board_enable.c
M chipset_enable.c
M drkaiser.c
M gfxnvidia.c
M hwaccess.c
M hwaccess.h
A hwaccess_x86_io.c
M hwaccess_x86_io.h
M internal.c
M it8212.c
M it85spi.c
M it87spi.c
M meson.build
M nic3com.c
M nicintel.c
M nicintel_eeprom.c
M nicintel_spi.c
M nicnatsemi.c
M nicrealtek.c
M ogp_spi.c
M programmer.h
M rayer_spi.c
M satamv.c
M satasii.c
M tests/hwaccess_x86_io_unittest.h
M wbsio_spi.c
31 files changed, 118 insertions(+), 69 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/10/60110/5
--
To view, visit https://review.coreboot.org/c/flashrom/+/60110
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I372b4a409f036da766c42bc406b596bc41b0f75a
Gerrit-Change-Number: 60110
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
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: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Thomas Heijligen, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60110 )
Change subject: hwaccess: move x86 port I/O related code into own files
......................................................................
Patch Set 4:
(2 comments)
File Makefile:
https://review.coreboot.org/c/flashrom/+/60110/comment/35ad35aa_e532052c
PS4, Line 783: ifneq ($(NEED_RAW_ACCESS), )
: # Raw memory, MSR or PCI port I/O access.
: FEATURE_CFLAGS += -D'NEED_RAW_ACCESS=1' -D'__FLASHROM_HAVE_OUTB__=1'
: PROGRAMMER_OBJS += physmap.o hwaccess.o hwaccess_x86_io.o
Mostly curiosity: what is the way to build with NEED_RAW_ACCESS empty? Would that be disabling all the drivers that need raw access?
Running `make` by default will have drivers that need raw access, which gives one (positive) branch of ifneq condition, that's what I am thinking.
File tests/hwaccess_x86_io_unittest.h:
https://review.coreboot.org/c/flashrom/+/60110/comment/6e74bef6_329c2b20
PS4, Line 33: int rget_io_perms(void);
If you could move it below all defines and includes (right above iopl)? thanks!
--
To view, visit https://review.coreboot.org/c/flashrom/+/60110
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I372b4a409f036da766c42bc406b596bc41b0f75a
Gerrit-Change-Number: 60110
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
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-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Thu, 16 Dec 2021 01:34:07 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk, Peter Marheine.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59275 )
Change subject: pcidev: Avoid internal programmer relying on pacc global
......................................................................
Patch Set 3:
(2 comments)
File internal.c:
https://review.coreboot.org/c/flashrom/+/59275/comment/c1ffc7c1_5b3bd2ca
PS3, Line 41: temp = pcidev_scandev(&filter, NULL);
: for (; temp; temp = pcidev_scandev(&filter, temp)) {
> I guess this could also be written as […]
That's a more natural construct, fixed the other loop accordingly too. Done.
https://review.coreboot.org/c/flashrom/+/59275/comment/c8e7630b_3d49bbc5
PS3, Line 43: if (pci_filter_match(&filter, temp)) {
> That's what pcidev_scandev() does, isn't it?
Yes, fat fingered the diff. Done.
--
To view, visit https://review.coreboot.org/c/flashrom/+/59275
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id83bfd41f785f907e52a65a6689e8c7016fc1b77
Gerrit-Change-Number: 59275
Gerrit-PatchSet: 3
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
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: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Thu, 16 Dec 2021 00:30:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Peter Marheine.
Hello build bot (Jenkins), Nico Huber, Angel Pons, Anastasia Klimchuk, Peter Marheine,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/59275
to look at the new patch set (#4).
Change subject: pcidev: Avoid internal programmer relying on pacc global
......................................................................
pcidev: Avoid internal programmer relying on pacc global
Make progress towards the goal of removing pacc from global
state as noted in the FIXME of programmer.h
BUG=none
TEST=builds
Change-Id: Id83bfd41f785f907e52a65a6689e8c7016fc1b77
Signed-off-by: Edward O'Callaghan <quasisec(a)google.com>
---
M internal.c
M pcidev.c
M programmer.h
3 files changed, 25 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/59275/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/59275
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id83bfd41f785f907e52a65a6689e8c7016fc1b77
Gerrit-Change-Number: 59275
Gerrit-PatchSet: 4
Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: newpatchset