August 2021 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: Add support for Intel Emmitsburg PCH
by David Hendricks (Code Review) 31 Aug '21

31 Aug '21

Attention is currently required from: Arthur Heymans, Jonathan Zhang, Johnny Lin, Christian Walter, Edward O'Callaghan, Deomid "rojer" Ryabkov, Tim Chu. David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54965 ) Change subject: Add support for Intel Emmitsburg PCH ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: Nico did some nice refactoring work on the detection from IFD code a couple months ago. I've updated this patch, which is now much more straightforward than before, and tested it on my EBG system (with an em100). -- To view, visit https://review.coreboot.org/c/flashrom/+/54965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a1bb7467e693d1583aa885fa0e277075edd4a3e Gerrit-Change-Number: 54965 Gerrit-PatchSet: 2 Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks Gerrit-Reviewer: Deomid "rojer" Ryabkov <rojer9(a)fb.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Deomid "rojer" Ryabkov <rojer9(a)fb.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Tue, 31 Aug 2021 06:37:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Add support for Intel Emmitsburg PCH
by David Hendricks (Code Review) 31 Aug '21

31 Aug '21

Attention is currently required from: Arthur Heymans, Jonathan Zhang, Johnny Lin, Christian Walter, Edward O'Callaghan, David Hendricks, Deomid "rojer" Ryabkov, Tim Chu. David Hendricks has uploaded a new patch set (#2) to the change originally created by Jonathan Zhang. ( https://review.coreboot.org/c/flashrom/+/54965 ) Change subject: Add support for Intel Emmitsburg PCH ...................................................................... Add support for Intel Emmitsburg PCH This patch does the following: - Add PCIe ID for Intel Emmitsburg PCH - Based on ICH descriptor content, choose CHIPSET_C620_SERIES_LEWISBURG if ISL/PSL is 80. TESTED=tried on a server with Intel Emmitsburg PCH, flash update was successful. This server, however, does not have flash chip installed, it instead has em100 emulator connected. Change-Id: I2a1bb7467e693d1583aa885fa0e277075edd4a3e Signed-off-by: Jonathan Zhang <jonzhang(a)fb.com> Signed-off-by: David Hendricks <ddaveh(a)amazon.com> --- M chipset_enable.c M ich_descriptors.c 2 files changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/54965/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/54965 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2a1bb7467e693d1583aa885fa0e277075edd4a3e Gerrit-Change-Number: 54965 Gerrit-PatchSet: 2 Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks Gerrit-Reviewer: Deomid "rojer" Ryabkov <rojer9(a)fb.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: David Hendricks Gerrit-Attention: Deomid "rojer" Ryabkov <rojer9(a)fb.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: Accept shutdown function in register_master
by Anastasia Klimchuk (Code Review) 31 Aug '21

31 Aug '21

Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/51761 ) Change subject: Accept shutdown function in register_master ...................................................................... Abandoned The change has been merged in three parts as commit a69c5196d20d136b1de120f0fa5ea1e06c3776da , commit 9db8e12c169aa5a74154d9fb9993af8044afe39f , commit 31b283bd21ea1f0e9fc99f2ee17dfce64b3938e2 -- To view, visit https://review.coreboot.org/c/flashrom/+/51761 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1704ddafc56e1393613a7ea077e40c36911cfac8 Gerrit-Change-Number: 51761 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-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sam McNally <sammc(a)google.com> Gerrit-MessageType: abandon

1 0

Change in flashrom[master]: Accept shutdown function in register_master
by Anastasia Klimchuk (Code Review) 31 Aug '21

31 Aug '21

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/+/51761 ) Change subject: Accept shutdown function in register_master ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: This is done in many other commits, final ones are: commit a69c5196d20d136b1de120f0fa5ea1e06c3776da , commit 9db8e12c169aa5a74154d9fb9993af8044afe39f , commit 31b283bd21ea1f0e9fc99f2ee17dfce64b3938e2 . For reference, full work is https://review.coreboot.org/q/topic:%22register_master_api%22+(status:open%… together with chain CB:54067 I wanted to say THANK YOU to everyone who is here! This took lots of time from all of us, it was team work, I am really proud to be a part of such a cool team!! \m/ (I look forward to do more stuff :)) -- To view, visit https://review.coreboot.org/c/flashrom/+/51761 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1704ddafc56e1393613a7ea077e40c36911cfac8 Gerrit-Change-Number: 51761 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-CC: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sam McNally <sammc(a)google.com> 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: Tue, 31 Aug 2021 03:15:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Experimenting with register_x_master API change
by Anastasia Klimchuk (Code Review) 31 Aug '21

31 Aug '21

Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/51676 ) Change subject: Experimenting with register_x_master API change ...................................................................... Abandoned This patch was intended as a room for discussion, and it did its job. Some comments from here are good ideas that has been/will be implemented as separate patches. I am marking as Abandoned to avoid confusion. See also CB:51761 for more context. -- To view, visit https://review.coreboot.org/c/flashrom/+/51676 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia9ab858a8ce2cae78be32cc9a632eedd3b6ad7cd Gerrit-Change-Number: 51676 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: abandon

1 0

Change in flashrom[master]: dediprog.c: Refactor singleton states into reentrant pattern
by Anastasia Klimchuk (Code Review) 31 Aug '21

31 Aug '21

Attention is currently required from: Nico Huber, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56414 ) Change subject: dediprog.c: Refactor singleton states into reentrant pattern ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: There have been changes recently (including dediprog), so I rebased this -- To view, visit https://review.coreboot.org/c/flashrom/+/56414 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I72085e750af97b94dfa94f2ebf2a134e41a2ec8d Gerrit-Change-Number: 56414 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-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: Tue, 31 Aug 2021 01:22:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: dediprog.c: Refactor singleton states into reentrant pattern
by Anastasia Klimchuk (Code Review) 31 Aug '21

31 Aug '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. 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/+/56414 to look at the new patch set (#3). Change subject: dediprog.c: Refactor singleton states into reentrant pattern ...................................................................... dediprog.c: Refactor singleton states into reentrant pattern Move global singleton states into a struct and store within the spi_master data field for the life-time of the driver. This is one of the steps on the way to move spi_master data memory management behind the initialisation API, for more context see other patches under the same topic "register_master_api". BUG=b:185191942 TEST=builds and ninja test Change-Id: I72085e750af97b94dfa94f2ebf2a134e41a2ec8d Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M dediprog.c 1 file changed, 159 insertions(+), 114 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/14/56414/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/56414 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I72085e750af97b94dfa94f2ebf2a134e41a2ec8d Gerrit-Change-Number: 56414 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-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-MessageType: newpatchset

1 0

Change in flashrom[master]: dediprog.c: Drop dediprog_ prefix for spi data struct members
by Anastasia Klimchuk (Code Review) 31 Aug '21

31 Aug '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. 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/+/56415 to look at the new patch set (#3). Change subject: dediprog.c: Drop dediprog_ prefix for spi data struct members ...................................................................... dediprog.c: Drop dediprog_ prefix for spi data struct members The name of the struct type already contains dp_ prefix, so prefix doesn't need to be repeated in members name. BUG=b:185191942 TEST=builds and ninja test Change-Id: I688d50926b78a6c3f1c5a8ba4ef88a0d5b495bd0 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M dediprog.c 1 file changed, 56 insertions(+), 59 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/15/56415/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/56415 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I688d50926b78a6c3f1c5a8ba4ef88a0d5b495bd0 Gerrit-Change-Number: 56415 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-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-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Add NOT_ZERO and NOT_NULL macros to the header file
by Anastasia Klimchuk (Code Review) 31 Aug '21

31 Aug '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/57269 ) Change subject: tests: Add NOT_ZERO and NOT_NULL macros to the header file ...................................................................... tests: Add NOT_ZERO and NOT_NULL macros to the header file This patch adds two macros into io_mock.h, so that they can be used anywhere in tests. Common usage is to indicate a valid pointer, where it doesn't matter what the pointer is, only matters it is not null. Another case is to indicate a valid file descriptor, where it only matters the descriptor is not zero integer. New macros replace existing MOCK_HANDLE. This patch corrects return value from __wrap_ioctl to be successful by default. It used to be MOCK_HANDLE, but should be 0. BUG=b:181803212 TEST=builds and ninja test Change-Id: I5ad6ee4aa9091447c6c9108c92bf7f6e755fca48 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/init_shutdown.c M tests/io_mock.h M tests/tests.c 3 files changed, 11 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/69/57269/1 diff --git a/tests/init_shutdown.c b/tests/init_shutdown.c index 3236b22..d1e99cb 100644 --- a/tests/init_shutdown.c +++ b/tests/init_shutdown.c @@ -19,8 +19,6 @@ #include "io_mock.h" #include "programmer.h" -#define NOT_NULL ((void *)0xf000baaa) - static void run_lifecycle(void **state, const struct programmer_entry *prog, const char *param) { (void) state; /* unused */ diff --git a/tests/io_mock.h b/tests/io_mock.h index adb5f3b..18eaac8 100644 --- a/tests/io_mock.h +++ b/tests/io_mock.h @@ -34,6 +34,9 @@ /* Required for `FILE *` */ #include <stdio.h> +#define NOT_ZERO (0xf000baaa) +#define NOT_NULL ((void *)NOT_ZERO) + /* Define libusb symbols to avoid dependency on libusb.h */ struct libusb_device_handle; typedef struct libusb_device_handle libusb_device_handle; diff --git a/tests/tests.c b/tests/tests.c index 4965fe1..cd50bdf 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -23,7 +23,6 @@ /* redefinitions/wrapping */ #define LOG_ME printf("%s is called\n", __func__) -#define MOCK_HANDLE 2021 static const struct io_mock *current_io = NULL; @@ -54,7 +53,7 @@ } struct pci_dev mock_pci_dev = { - .device_id = MOCK_HANDLE, + .device_id = NOT_ZERO, }; struct pci_dev *__wrap_pcidev_init(void *devs, int bar) @@ -66,7 +65,7 @@ uintptr_t __wrap_pcidev_readbar(void *dev, int bar) { LOG_ME; - return MOCK_HANDLE; + return NOT_ZERO; } void __wrap_sio_write(uint16_t port, uint8_t reg, uint8_t data) @@ -85,7 +84,7 @@ LOG_ME; if (current_io && current_io->open) return current_io->open(current_io->state, pathname, flags); - return MOCK_HANDLE; + return NOT_ZERO; } int __wrap_open64(const char *pathname, int flags) @@ -93,7 +92,7 @@ LOG_ME; if (current_io && current_io->open) return current_io->open(current_io->state, pathname, flags); - return MOCK_HANDLE; + return NOT_ZERO; } int __wrap_ioctl(int fd, unsigned long int request, ...) @@ -107,7 +106,7 @@ va_end(args); return out; } - return MOCK_HANDLE; + return 0; } int __wrap_write(int fd, const void *buf, size_t sz) @@ -131,7 +130,7 @@ LOG_ME; if (current_io && current_io->fopen) return current_io->fopen(current_io->state, pathname, mode); - return (void *)MOCK_HANDLE; + return NOT_NULL; } FILE *__wrap_fopen64(const char *pathname, const char *mode) @@ -139,7 +138,7 @@ LOG_ME; if (current_io && current_io->fopen) return current_io->fopen(current_io->state, pathname, mode); - return (void *)MOCK_HANDLE; + return NOT_NULL; } int __wrap_stat(const char *path, void *buf) @@ -256,7 +255,7 @@ libusb_context *usb_ctx, uint16_t vid, uint16_t pid, unsigned int num) { LOG_ME; - return (void *)MOCK_HANDLE; + return NOT_NULL; } int __wrap_libusb_set_configuration(libusb_device_handle *devh, int config) -- To view, visit https://review.coreboot.org/c/flashrom/+/57269 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5ad6ee4aa9091447c6c9108c92bf7f6e755fca48 Gerrit-Change-Number: 57269 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: layout/fmap: reimplement for endian independence
by Thomas Heijligen (Code Review) 30 Aug '21

30 Aug '21

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/57265 ) Change subject: layout/fmap: reimplement for endian independence ...................................................................... layout/fmap: reimplement for endian independence Change-Id: I6d38649327d2021a9917d1294f1541376ed6717d Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.de> --- M fmap.c M layout.h M libflashrom.c 3 files changed, 177 insertions(+), 375 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/57265/1 diff --git a/fmap.c b/fmap.c index b18cbf7..903ffc2 100644 --- a/fmap.c +++ b/fmap.c @@ -1,331 +1,158 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + /* - * Copyright 2010, Google LLC. - * Copyright 2018-present, Facebook Inc. - * All rights reserved. + * This file is part of flashrom project. For the full license text see LICENSE.txt. * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - * Alternatively, this software may be distributed under the terms of the - * GNU General Public License ("GPL") version 2 as published by the Free - * Software Foundation. + * Copyright (C) 2021, secunet Secure Networks AG, + * written by Thomas Heijligen <thomas.heijligen(a)secunet.com> */ -#include <ctype.h> -#include <stdlib.h> +/* + * This file contais functions to find and read layout information from a fmap structure. + */ + +#include <stddef.h> +#include <stdint.h> +#include <stdio.h> #include <string.h> -#include <sys/types.h> -#include "flash.h" -#include "fmap.h" +#include "layout.h" +#include "libflashrom.h" +#include "hwaccess.h" -static size_t fmap_size(const struct fmap *fmap) +#define FMAP_SIGNATURE "__FMAP__" +#define FMAP_NAME_SIZE 32 +#define FMAP_VERSION_MAJOR 1 +#define FMAP_VERSION_MINOR 1 + +#define FMAP_HEADER_SIZE 56 +#define FMAP_HEADER_OFFSET_SIGNATURE 0 +#define FMAP_HEADER_OFFSET_VER_MAJOR 8 +#define FMAP_HEADER_OFFSET_VER_MINOR 9 +#define FMAP_HEADER_OFFSET_BASE 10 +#define FMAP_HEADER_OFFSET_SIZE 18 +#define FMAP_HEADER_OFFSET_NAME 22 +#define FMAP_HEADER_OFFSET_NAREAS 54 + +#define FMAP_AREA_SIZE 42 +#define FMAP_AREA_OFFSET_OFFSET 0 +#define FMAP_AERA_OFFSET_SIZE 4 +#define FMAP_AREA_OFFSET_NAME 8 +#define FMAP_AREA_OFFSET_FLAGS 40 + +/** + * @brief find the signature of a FMAP in a buffer, uses linear search + * @param[in] buffer + * @param[in] buffer_size + * @param[out] fmap_offset, on SUCCESS the offset where to find the fmap, if NULL or FAILURE don't change + * @return 0 SUCCESS + * 1 UNEXPECTED_NULL_POINTER + * 2 NOT_FOUND - FMAP signature not found + */ +int find_fmap_in_buffer(const void *const buffer, const uint64_t buffer_size, uint64_t *const fmap_offset) { - return sizeof(*fmap) + (fmap->nareas * sizeof(struct fmap_area)); -} + /* test for unexpected NULL pointer */ + if (buffer == NULL) { + return 1; /* UNEXPECTED_NULL_POINTER */ + } -static int is_valid_fmap(const struct fmap *fmap) -{ - if (memcmp(fmap, FMAP_SIGNATURE, strlen(FMAP_SIGNATURE)) != 0) - return 0; - /* strings containing the magic tend to fail here */ - if (fmap->ver_major > FMAP_VER_MAJOR) - return 0; - if (fmap->ver_minor > FMAP_VER_MINOR) - return 0; - /* a basic consistency check: flash address space size should be larger - * than the size of the fmap data structure */ - if (fmap->size < fmap_size(fmap)) - return 0; - - /* fmap-alikes along binary data tend to fail on having a valid, - * null-terminated string in the name field.*/ - int i; - for (i = 0; i < FMAP_STRLEN; i++) { - if (fmap->name[i] == 0) - break; - if (!isgraph(fmap->name[i])) - return 0; - if (i == FMAP_STRLEN - 1) { - /* name is specified to be null terminated single-word string - * without spaces. We did not break in the 0 test, we know it - * is a printable spaceless string but we're seeing FMAP_STRLEN - * symbols, which is one too many. - */ - return 0; + for (uint64_t offset = 0; (offset + FMAP_HEADER_SIZE) < buffer_size; offset++) { + if (memcmp(buffer + offset + FMAP_HEADER_OFFSET_SIGNATURE, FMAP_SIGNATURE, strlen(FMAP_SIGNATURE)) == 0) { + if (fmap_offset != NULL) { + *fmap_offset = offset; + } + return 0; /* SUCCESS */ } } - return 1; - + return 2; /* NOT_FOUND */ } /** - * @brief Do a brute-force linear search for fmap in provided buffer + * TODO: implement * - * @param[in] buffer The buffer to search - * @param[in] len Length (in bytes) to search - * - * @return offset in buffer where fmap is found if successful - * -1 to indicate that fmap was not found - * -2 to indicate fmap is truncated or exceeds buffer + len + * @brief find the signature of a FMAP on the flash chip + * @param[in] flashctx + * @param[out] fmap_offset on SUCCESS offset of the FMAP, else not changed + * @param[out] fmap_size on SUCCESS size of the FMAP, else not changed + * @return 0 SUCESS + * 2 NOT_FOUND - FMAP signature not found */ -static off_t fmap_lsearch(const uint8_t *buf, size_t len) +int find_fmap_in_rom(const struct flashrom_flashctx *const flashctx, uint64_t *const fmap_offset, uint64_t *const fmap_size) { - off_t offset; - bool fmap_found = 0; + return 2; +} - for (offset = 0; offset <= (off_t)(len - sizeof(struct fmap)); offset++) { - if (is_valid_fmap((struct fmap *)&buf[offset])) { - fmap_found = 1; - break; + +/** + * @brief read out the layout information from a FMAP in buffer + * @param[out] layout + * @param[in] buffer + * @param[in] buffer_size + * @param[in] fmap_offset + * @return 0 SUCCESS + * 1 UNEXPECTED_NULL_POINTER + * 3 BUFFER_TO_SMALL + * 4 INVALID_SIGNATURE + * 5 INVALID_VERSION + * 6 MEMORY_ALLOCATION_FAILED + */ +int read_layout_from_fmap(struct flashrom_layout **layout, const void *const buffer, const uint64_t buffer_size, const uint64_t fmap_offset) +{ + /* buffer must not be NULL */ + if (buffer == NULL) { + return 1; /* FLASHROM_UNEXPECTED_NULL_POINTER */ + } + /* buffer_size must be larger than fmap_offset + FMAP_HEADER_SIZE */ + if (buffer_size < fmap_offset + FMAP_HEADER_SIZE) { + return 3; /* FLASHROM_BUFFER_TO_SMALL */ + } + /* fmap signature check */ + if (memcmp(buffer + fmap_offset + FMAP_HEADER_OFFSET_SIGNATURE, FMAP_SIGNATURE, strlen(FMAP_SIGNATURE)) != 0) { + return 4; /* FLASHROM_INVALID_SIGNATURE */ + } + /* fmap version check */ + if ((read_le8(buffer + fmap_offset + FMAP_HEADER_OFFSET_VER_MAJOR) != FMAP_VERSION_MAJOR) && + (read_le8(buffer + fmap_offset + FMAP_HEADER_OFFSET_VER_MINOR) != FMAP_VERSION_MINOR)) { + return 5; /* FLASHROM_INVALID_VERSION */ + } + + const void* const fmap = buffer + fmap_offset; + + struct flashrom_layout *l = NULL; + if (flashrom_layout_new(&l) != 0) { + return 6; /* FLASHROM_MEMORY_ALLOCATION_FAILED */ + } + uint64_t firmware_base = read_le64(fmap + FMAP_HEADER_OFFSET_BASE); + /* uint32_t firmware_size = read_le32(fmap + FMAP_HEADER_OFFSET_SIZE); TODO: mode checks */ + char firmware_name[FMAP_NAME_SIZE + 1] = {'\0'}; + memcpy(firmware_name, fmap + FMAP_HEADER_OFFSET_NAME, FMAP_NAME_SIZE); + uint16_t firmware_nareas = read_le16(fmap + FMAP_HEADER_OFFSET_NAREAS); + + /* check if buffer includes all fmap areas */ + if (buffer_size < (fmap_offset + FMAP_HEADER_SIZE + (firmware_nareas * FMAP_AREA_SIZE))) { + return 3; /* FLASHROM_BUFFER_TO_SMALL */ + } + + for (unsigned int i = 0; i < firmware_nareas; i++) { + const void* const region = fmap + FMAP_HEADER_SIZE + (i * FMAP_AREA_SIZE); + + uint32_t area_offset = read_le32(region + FMAP_AREA_OFFSET_OFFSET); + uint32_t area_size = read_le32(region + FMAP_AERA_OFFSET_SIZE); + char area_name[FMAP_NAME_SIZE + 1] = {'\0'}; + char combined_name[(2 * FMAP_NAME_SIZE) + 2]; + memcpy(area_name, region + FMAP_AREA_OFFSET_NAME, FMAP_NAME_SIZE); + snprintf(combined_name, sizeof(combined_name), "%s_%s", firmware_name, area_name); + + int ret = flashrom_layout_add_region(l, + firmware_base + area_offset, + firmware_base + area_offset + area_size, + combined_name ); + + if (ret != 0) { + flashrom_layout_release(l); + return 6; /* FLASHROM_MEMORY_ALLOCATION_FAILED */ } } - if (!fmap_found) - return -1; - - if (offset + fmap_size((struct fmap *)&buf[offset]) > len) { - msg_gerr("fmap size exceeds buffer boundary.\n"); - return -2; - } - - return offset; -} - -/** - * @brief Read fmap from provided buffer and copy it to fmap_out - * - * @param[out] fmap_out Double-pointer to location to store fmap contents. - * Caller must free allocated fmap contents. - * @param[in] buf Buffer to search - * @param[in] len Length (in bytes) to search - * - * @return 0 if successful - * 1 to indicate error - * 2 to indicate fmap is not found - */ -int fmap_read_from_buffer(struct fmap **fmap_out, const uint8_t *const buf, size_t len) -{ - off_t offset = fmap_lsearch(buf, len); - if (offset < 0) { - msg_gdbg("Unable to find fmap in provided buffer.\n"); - return 2; - } - msg_gdbg("Found fmap at offset 0x%06zx\n", (size_t)offset); - - const struct fmap *fmap = (const struct fmap *)(buf + offset); - *fmap_out = malloc(fmap_size(fmap)); - if (*fmap_out == NULL) { - msg_gerr("Out of memory.\n"); - return 1; - } - - memcpy(*fmap_out, fmap, fmap_size(fmap)); - return 0; -} - -static int fmap_lsearch_rom(struct fmap **fmap_out, - struct flashctx *const flashctx, size_t rom_offset, size_t len) -{ - int ret = -1; - uint8_t *buf; - - if (prepare_flash_access(flashctx, true, false, false, false)) - goto _finalize_ret; - - /* likely more memory than we need, but it simplifies handling and - * printing offsets to keep them uniform with what's on the ROM */ - buf = malloc(rom_offset + len); - if (!buf) { - msg_gerr("Out of memory.\n"); - goto _finalize_ret; - } - - ret = flashctx->chip->read(flashctx, buf + rom_offset, rom_offset, len); - if (ret) { - msg_pdbg("Cannot read ROM contents.\n"); - goto _free_ret; - } - - ret = fmap_read_from_buffer(fmap_out, buf + rom_offset, len); -_free_ret: - free(buf); -_finalize_ret: - finalize_flash_access(flashctx); - return ret; -} - -static int fmap_bsearch_rom(struct fmap **fmap_out, struct flashctx *const flashctx, - size_t rom_offset, size_t len, size_t min_stride) -{ - size_t stride, fmap_len = 0; - int ret = 1, fmap_found = 0, check_offset_0 = 1; - struct fmap *fmap; - const unsigned int chip_size = flashctx->chip->total_size * 1024; - const int sig_len = strlen(FMAP_SIGNATURE); - - if (rom_offset + len > flashctx->chip->total_size * 1024) - return 1; - - if (len < sizeof(*fmap)) - return 1; - - if (prepare_flash_access(flashctx, true, false, false, false)) - return 1; - - fmap = malloc(sizeof(*fmap)); - if (!fmap) { - msg_gerr("Out of memory.\n"); - goto _free_ret; - } - - /* - * For efficient operation, we start with the largest stride possible - * and then decrease the stride on each iteration. Also, check for a - * remainder when modding the offset with the previous stride. This - * makes it so that each offset is only checked once. - * - * Zero (rom_offset == 0) is a special case and is handled using a - * variable to track whether or not we've checked it. - */ - size_t offset; - for (stride = chip_size / 2; stride >= min_stride; stride /= 2) { - if (stride > len) - continue; - - for (offset = rom_offset; - offset <= rom_offset + len - sizeof(struct fmap); - offset += stride) { - if ((offset % (stride * 2) == 0) && (offset != 0)) - continue; - if (offset == 0 && !check_offset_0) - continue; - check_offset_0 = 0; - - /* Read errors are considered non-fatal since we may - * encounter locked regions and want to continue. */ - if (flashctx->chip->read(flashctx, (uint8_t *)fmap, offset, sig_len)) { - /* - * Print in verbose mode only to avoid excessive - * messages for benign errors. Subsequent error - * prints should be done as usual. - */ - msg_cdbg("Cannot read %d bytes at offset %zu\n", sig_len, offset); - continue; - } - - if (memcmp(fmap, FMAP_SIGNATURE, sig_len) != 0) - continue; - - if (flashctx->chip->read(flashctx, (uint8_t *)fmap + sig_len, - offset + sig_len, sizeof(*fmap) - sig_len)) { - msg_cerr("Cannot read %zu bytes at offset %06zx\n", - sizeof(*fmap) - sig_len, offset + sig_len); - continue; - } - - if (is_valid_fmap(fmap)) { - msg_gdbg("fmap found at offset 0x%06zx\n", offset); - fmap_found = 1; - break; - } - msg_gerr("fmap signature found at %zu but header is invalid.\n", offset); - ret = 2; - } - - if (fmap_found) - break; - } - - if (!fmap_found) - goto _free_ret; - - fmap_len = fmap_size(fmap); - struct fmap *tmp = fmap; - fmap = realloc(fmap, fmap_len); - if (!fmap) { - msg_gerr("Failed to realloc.\n"); - free(tmp); - goto _free_ret; - } - - if (flashctx->chip->read(flashctx, (uint8_t *)fmap + sizeof(*fmap), - offset + sizeof(*fmap), fmap_len - sizeof(*fmap))) { - msg_cerr("Cannot read %zu bytes at offset %06zx\n", - fmap_len - sizeof(*fmap), offset + sizeof(*fmap)); - /* Treat read failure to be fatal since this - * should be a valid, usable fmap. */ - ret = 2; - goto _free_ret; - } - - *fmap_out = fmap; - ret = 0; -_free_ret: - if (ret) - free(fmap); - finalize_flash_access(flashctx); - return ret; -} - -/** - * @brief Read fmap from ROM - * - * @param[out] fmap_out Double-pointer to location to store fmap contents. - * Caller must free allocated fmap contents. - * @param[in] flashctx Flash context - * @param[in] rom_offset Offset in ROM to begin search - * @param[in] len Length to search relative to rom_offset - * - * @return 0 on success, - * 2 if the fmap couldn't be read, - * 1 on any other error. - */ -int fmap_read_from_rom(struct fmap **fmap_out, - struct flashctx *const flashctx, size_t rom_offset, size_t len) -{ - int ret; - - if (!flashctx || !flashctx->chip) - return 1; - - /* - * Binary search is used at first to see if we can find an fmap quickly - * in a usual location (often at a power-of-2 offset). However, once we - * reach a small enough stride the transaction overhead will reverse the - * speed benefit of using bsearch at which point we need to use brute- - * force instead. - * - * TODO: Since flashrom is often used with high-latency external - * programmers we should not be overly aggressive with bsearch. - */ - ret = fmap_bsearch_rom(fmap_out, flashctx, rom_offset, len, 256); - if (ret) { - msg_gdbg("Binary search failed, trying linear search...\n"); - ret = fmap_lsearch_rom(fmap_out, flashctx, rom_offset, len); - } - - return ret; -} + *layout = l; + return 0; /* FLASHROM_SUCCESS */ +} \ No newline at end of file diff --git a/layout.h b/layout.h index 713241f..6e084ae 100644 --- a/layout.h +++ b/layout.h @@ -68,4 +68,8 @@ void prepare_layout_for_extraction(struct flashrom_flashctx *); int layout_sanity_checks(const struct flashrom_flashctx *); +int find_fmap_in_buffer(const void *const buffer, const uint64_t buffer_size, uint64_t *const fmap_offset); +int find_fmap_in_rom(const struct flashrom_flashctx *const flashctx, uint64_t *const fmap_offset, uint64_t *const fmap_size); +int read_layout_from_fmap(struct flashrom_layout **layout, const void *const buffer, const uint64_t buffer_size, const uint64_t fmap_offset); + #endif /* !__LAYOUT_H__ */ diff --git a/libflashrom.c b/libflashrom.c index 1ecf650..ae8cd21 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -495,31 +495,6 @@ #endif } -#ifdef __FLASHROM_LITTLE_ENDIAN__ -static int flashrom_layout_parse_fmap(struct flashrom_layout **layout, - struct flashctx *const flashctx, const struct fmap *const fmap) -{ - int i; - char name[FMAP_STRLEN + 1]; - const struct fmap_area *area; - struct flashrom_layout *l; - - if (!fmap || flashrom_layout_new(&l)) - return 1; - - for (i = 0, area = fmap->areas; i < fmap->nareas; i++, area++) { - snprintf(name, sizeof(name), "%s", area->name); - if (flashrom_layout_add_region(l, area->offset, area->offset + area->size - 1, name)) { - flashrom_layout_release(l); - return 1; - } - } - - *layout = l; - return 0; -} -#endif /* __FLASHROM_LITTLE_ENDIAN__ */ - /** * @brief Read a layout by searching the flash chip for fmap. * @@ -537,27 +512,35 @@ int flashrom_layout_read_fmap_from_rom(struct flashrom_layout **const layout, struct flashctx *const flashctx, off_t offset, size_t len) { -#ifndef __FLASHROM_LITTLE_ENDIAN__ - return 3; -#else - struct fmap *fmap = NULL; - int ret = 0; + (void)(offset); // TODO: remove unused parameter - msg_gdbg("Attempting to read fmap from ROM content.\n"); - if (fmap_read_from_rom(&fmap, flashctx, offset, len)) { - msg_gerr("Failed to read fmap from ROM.\n"); - return 1; + int ret; + uint64_t fmap_offset, fmap_size; + + if(flashctx == NULL || flashctx->chip == NULL || flashctx->chip->read == NULL) { + return 2; // UNEXPECTED_NULL_POINTER } - msg_gdbg("Adding fmap layout to global layout.\n"); - if (flashrom_layout_parse_fmap(layout, flashctx, fmap)) { - msg_gerr("Failed to add fmap regions to layout.\n"); - ret = 1; + ret = find_fmap_in_rom(flashctx, &fmap_offset, &fmap_size); + if (ret != 0) { + return ret; } + void *fmap_buffer = malloc(fmap_size); + if (fmap_buffer == NULL) { + return 1; // MEMORY_ALLOCATION_FAILED + } + // prepare flash access? + // use flashrom_image_read? + ret = flashctx->chip->read(flashctx, fmap_buffer, fmap_offset, fmap_size); + if (ret != 0) { + free(fmap_buffer); + return ret; + } + ret = read_layout_from_fmap(layout, fmap_buffer, fmap_size, 0); + free(fmap_buffer); - free(fmap); + // TODO add layout to flashctx? return ret; -#endif } /** @@ -577,33 +560,21 @@ int flashrom_layout_read_fmap_from_buffer(struct flashrom_layout **const layout, struct flashctx *const flashctx, const uint8_t *const buf, size_t size) { -#ifndef __FLASHROM_LITTLE_ENDIAN__ - return 3; -#else - struct fmap *fmap = NULL; - int ret = 1; + (void)(flashctx); // Unused + int ret; + uint64_t fmap_offset; - if (!buf || !size) - goto _ret; - - msg_gdbg("Attempting to read fmap from buffer.\n"); - if (fmap_read_from_buffer(&fmap, buf, size)) { - msg_gerr("Failed to read fmap from buffer.\n"); - goto _ret; + ret = find_fmap_in_buffer(buf, size, &fmap_offset); + if (ret != 0) { + return ret; + } + ret = read_layout_from_fmap(layout, buf, size, fmap_offset); + if( ret != 0) { + return ret; } - msg_gdbg("Adding fmap layout to global layout.\n"); - if (flashrom_layout_parse_fmap(layout, flashctx, fmap)) { - msg_gerr("Failed to add fmap regions to layout.\n"); - goto _free_ret; - } - - ret = 0; -_free_ret: - free(fmap); -_ret: - return ret; -#endif + // TODO add layout to flashctx? + return 0; } /** -- To view, visit https://review.coreboot.org/c/flashrom/+/57265 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6d38649327d2021a9917d1294f1541376ed6717d Gerrit-Change-Number: 57265 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit August 2021