March 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: hwaccess: replace macros by C code
by Felix Singer (Code Review) 19 Mar '22

19 Mar '22

Attention is currently required from: Nico Huber, Thomas Heijligen, Edward O'Callaghan, Angel Pons, Nikolai Artemiev, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62898 ) Change subject: hwaccess: replace macros by C code ...................................................................... Patch Set 6: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/62898 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I86d38d816b37c283279c485fac8027f8fb94364a Gerrit-Change-Number: 62898 Gerrit-PatchSet: 6 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: 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-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 20 Mar 2022 01:02:48 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess_x86_msr: rename rdmsr, wrmsr to read_cpu_msr, write_cpu_msr
by Thomas Heijligen (Code Review) 19 Mar '22

19 Mar '22

Attention is currently required from: Nico Huber, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62851 ) Change subject: hwaccess_x86_msr: rename rdmsr, wrmsr to read_cpu_msr, write_cpu_msr ...................................................................... Patch Set 2: (2 comments) Patchset: PS1: > Needs build testing on all operating systems. Tested on Linux, FreeBSD, OpenBSD i386, MacOS compiles, linking fails due to missing directhw object on my system. File hwaccess_x86_msr.c: https://review.coreboot.org/c/flashrom/+/62851/comment/9c87d949_352eaa5e PS1, Line 296: /* rdmsr() and wrmsr() are provided by DirectHW which needs neither setup nor cleanup. */ > I don't know. I've never tried directwh. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/62851 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie5ad54d198312578e0a1ee719eec67b37d2bf6a4 Gerrit-Change-Number: 62851 Gerrit-PatchSet: 2 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: Sat, 19 Mar 2022 16:48:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess_x86_msr: rename rdmsr, wrmsr to read_cpu_msr, write_cpu_msr
by Thomas Heijligen (Code Review) 19 Mar '22

19 Mar '22

Attention is currently required from: Nico Huber, Angel Pons. Hello Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/62851 to look at the new patch set (#2). Change subject: hwaccess_x86_msr: rename rdmsr, wrmsr to read_cpu_msr, write_cpu_msr ...................................................................... hwaccess_x86_msr: rename rdmsr, wrmsr to read_cpu_msr, write_cpu_msr This eliminates the need to redefine the rdmsr and wrmsr symbols, resulting in more understanable code. Change-Id: Ie5ad54d198312578e0a1ee719eec67b37d2bf6a4 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M board_enable.c M chipset_enable.c M hwaccess_x86_msr.c M hwaccess_x86_msr.h 4 files changed, 46 insertions(+), 43 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/62851/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/62851 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie5ad54d198312578e0a1ee719eec67b37d2bf6a4 Gerrit-Change-Number: 62851 Gerrit-PatchSet: 2 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-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-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Add padding to pci_dev struct for ASAN
by Daniel Campello (Code Review) 19 Mar '22

19 Mar '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62845 ) Change subject: tests: Add padding to pci_dev struct for ASAN ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > Oh that's a very good point, and I should have thought about it myself! […] the three outstanding patches block the enablement of tests in the cq as each of them are addressing failures in different environments -- To view, visit https://review.coreboot.org/c/flashrom/+/62845 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I47943bf70181a9041f287df3ece0f7067a112de8 Gerrit-Change-Number: 62845 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> 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: 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 19 Mar 2022 14:14:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add padding to pci_dev struct for ASAN
by Anastasia Klimchuk (Code Review) 18 Mar '22

18 Mar '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Daniel Campello, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62845 ) Change subject: tests: Add padding to pci_dev struct for ASAN ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Oh that's a very good point, and I should have thought about it myself! Especially because this is not the first time, we had the same situation and the same two options for libusb symbols in tests. We had a discussion at that time, and decided to go with option #1, which is in this case > we make libpci a dependency for the tests, then we can include <pci/pci.h> in the test code. I think we can reuse the same decision? And that will also make tests consistent. Which means, specifically: 1) include pci/pci.h in test code 2) remove struct pci_dev from our tests/io_mock.h 3) declare in meson.build that libpci is a dependency for tests (I am not sure how to do this one :) One thing, Daniel you mentioned "unblock the execution of the tests" but does this patch actually block anything? I understand CB:62844 blocks tests (and it's ready and has no comments). But this one, I don't think so? -- To view, visit https://review.coreboot.org/c/flashrom/+/62845 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I47943bf70181a9041f287df3ece0f7067a112de8 Gerrit-Change-Number: 62845 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> 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: 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 19 Mar 2022 00:21:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add padding to pci_dev struct for ASAN
by Daniel Campello (Code Review) 18 Mar '22

18 Mar '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62845 ) Change subject: tests: Add padding to pci_dev struct for ASAN ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > So the test code and the programmer driver used different definitions […] I agree that having two separate structs is not ideal. I propose that we land this patch to unblock the execution of the tests and we address the migration in a follow up patch. -- To view, visit https://review.coreboot.org/c/flashrom/+/62845 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I47943bf70181a9041f287df3ece0f7067a112de8 Gerrit-Change-Number: 62845 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> 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: 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 18 Mar 2022 20:30:32 +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_physmap: add missing DirectHW include
by Thomas Heijligen (Code Review) 18 Mar '22

18 Mar '22

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62929 ) Change subject: hwaccess_physmap: add missing DirectHW include ...................................................................... hwaccess_physmap: add missing DirectHW include For MACH / APPLE map_physical is defined in DirectHW.h TEST: run `make` compiles, linking fails due to misses directhw object in my setup, don't know how to fix this Change-Id: I0e0f3fd587ae46e6f73418f2c83641cb1202478c Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M hwaccess_physmap.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/29/62929/1 diff --git a/hwaccess_physmap.c b/hwaccess_physmap.c index b1b9c64..7afe60d 100644 --- a/hwaccess_physmap.c +++ b/hwaccess_physmap.c @@ -133,6 +133,7 @@ { } #elif defined(__MACH__) && defined(__APPLE__) +#include <DirectHW/DirectHW.h> #define MEM_DEV "DirectHW" -- To view, visit https://review.coreboot.org/c/flashrom/+/62929 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0e0f3fd587ae46e6f73418f2c83641cb1202478c Gerrit-Change-Number: 62929 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: hwaccess_x86_msr: rename rdmsr, wrmsr to read_cpu_msr, write_cpu_msr
by Thomas Heijligen (Code Review) 18 Mar '22

18 Mar '22

Attention is currently required from: Nico Huber, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62851 ) Change subject: hwaccess_x86_msr: rename rdmsr, wrmsr to read_cpu_msr, write_cpu_msr ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Needs build testing on all operating systems. -- To view, visit https://review.coreboot.org/c/flashrom/+/62851 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie5ad54d198312578e0a1ee719eec67b37d2bf6a4 Gerrit-Change-Number: 62851 Gerrit-PatchSet: 1 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-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: Fri, 18 Mar 2022 19:46:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: raiden_debug_spi: Add more informative error message when WP is enabled
by Robert Zieba (Code Review) 18 Mar '22

18 Mar '22

Attention is currently required from: Nico Huber, Diana Zigterman, Jon Murphy, Edward O'Callaghan, Anastasia Klimchuk, Karthik Ramasubramanian. Robert Zieba has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62909 ) Change subject: raiden_debug_spi: Add more informative error message when WP is enabled ...................................................................... Patch Set 3: (2 comments) File raiden_debug_spi.c: https://review.coreboot.org/c/flashrom/+/62909/comment/ab54ee01_c0fdd319 PS2, Line 989: if (rsp_config.packet_size == 4 && > is packet size in bytes or words? Can we use sizeof() instead of a magic number? Can you update th […] Done https://review.coreboot.org/c/flashrom/+/62909/comment/afb2d4e4_20eda381 PS2, Line 1002: return status; > Is it always wrong to retry in this case? The commit message only argues […] You're correct, one of the error codes is actually a busy indicator. I've updated the code to only handle the `USB_SPI_DISABLED` case and let it fall back to the current behavior for other errors. -- To view, visit https://review.coreboot.org/c/flashrom/+/62909 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib1e8383baa9c3ea41ab1079af12e3dc8cdff90ae Gerrit-Change-Number: 62909 Gerrit-PatchSet: 3 Gerrit-Owner: Robert Zieba <robertzieba(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Diana Zigterman <dzigterman(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Peers <epeers(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Diana Zigterman <dzigterman(a)google.com> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 18 Mar 2022 18:49:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Jon Murphy <jpmurphy(a)google.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add padding to pci_dev struct for ASAN
by Nico Huber (Code Review) 18 Mar '22

18 Mar '22

Attention is currently required from: Edward O'Callaghan, Angel Pons, Daniel Campello. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62845 ) Change subject: tests: Add padding to pci_dev struct for ASAN ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: So the test code and the programmer driver used different definitions of `struct pci_dev`? If that's the case, we should decide on one and use that IMHO :) Suggestion: Either we make libpci a dependency for the tests, then we can include <pci/pci.h> in the test code. Or not, then we should mock the file, i.e. move `struct pci_dev` into a `tests/include/pci/pci.h` and make sure that we look there first when compiling the driver code. Instead of wrapping the functions, we would provide them under their original names and wouldn't link against libpci at all. The latter seems more work right now, but would avoid further trouble with mixed mocks and actual libpci code. -- To view, visit https://review.coreboot.org/c/flashrom/+/62845 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I47943bf70181a9041f287df3ece0f7067a112de8 Gerrit-Change-Number: 62845 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> 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: Nico Huber <nico.h(a)gmx.de> 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: Daniel Campello <campello(a)chromium.org> Gerrit-Comment-Date: Fri, 18 Mar 2022 18:09:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022