Several recovery images for newer ChromeOS boards fail in
extract_partition() due to parted detecting that there are overlapping
partitions, and therefore failing to print the partition layout
(this is potentially a parted bug; requries further investigation).
To work around this, fall back to using fdisk, making the assumption
that ROOT-A is always partition #3, and calculate the partition
start and size using the sector size.
Test: successfully extract coreboot firmware images from recovery
images which previously failed to extract (fizz, octopus, volteer).
Change-Id: I03234170ba0544af9eb0879253f0a8e0e7bf33f5
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61616
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
Wile historically there was a unique recovery image for each Chrome OS
board/HWID (with matching names), this is no longer the case. Now,
multiple boards share a single recovery image, so adjust how the proper
recovery image is determined, and how the coreboot image is extracted from it.
Test: successfully extract coreboot images for older 1:1 boards (e.g. CAVE)
and newer 1:N boards (e.g. DROBIT)
Change-Id: If478aa6eadea3acf3ee9d4c5fa266acd72c99b7a
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61615
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
This patch adds a new --loglevel option to the CBMEM utility which can
be used either numerically (e.g. `cbmem -1 --loglevel 6`) or by name
(e.g. `cbmem -c --loglevel INFO`) to restrict the lines that will be
printed from the CBMEM console log to a maximum loglevel. By default,
using this option means that lines without a loglevel (which usually
happens when payloads or other non-coreboot components add their own
logs to the CBMEM console) will not be printed. Prefixing a `+`
character to the option value (e.g. `--loglevel +6` or
`--loglevel +INFO`) can be used to change that behavior.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I8458027083246df5637dffd3ebfeb4d0a78deadf
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61779
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
`PLATFORM_IFD2` macro is more generic tag that can be associated with
early next SoC platform development which using IFDv2.
The current assumption is that newer SoC platform still uses the same
SPI/eSPI frequency definition being used for latest platform(TGL, ADL)
and if the frequency definition is updated later, `PLATFORM_IFD2' will
use latest frequency definition for early next SoC development.
And once upstream is allowed for new platform, platform name will be
added in tool later.
Signed-off-by: Wonkyu Kim <wonkyu.kim@intel.com>
Change-Id: I14a71a58c7d51b9c8b92e013b5637c6b35005f22
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61576
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Subrata Banik <subratabanik@google.com>
ADL and Sabrina have different advisory regarding encoding the bus
width. Encode the bus width as per the respective advisories.
BUG=b:211510456
TEST=Build spd_gen and ensure that the bus width is encoded as expected.
Signed-off-by: Karthikeyan Ramasubramanian <kramasub@google.com>
Change-Id: Ia12a5bd8f70a70ca8a510ecf00f6268c6904ec25
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61639
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
In order to provide the same loglevel prefixes and highlighting that
were recently introduced for "interactive" consoles (e.g. UART) to
"stored" consoles (e.g. CBMEM) but minimize the amont of extra storage
space wasted on this info, this patch will write a 1-byte control
character marker indicating the loglevel to the start of every line
logged in those consoles. The `cbmem` utility will then interpret those
markers and translate them back into loglevel prefixes and escape
sequences as needed.
Since coreboot and userspace log readers aren't always in sync,
occasionally an older reader may come across these markers and not know
how to interpret them... but that should usually be fine, as the range
chosen contains non-printable ASCII characters that normally have no
effect on the terminal. At worst the outdated reader would display one
garbled character at the start of every line which isn't that bad.
(Older versions of the `cbmem` utility will translate non-printable
characters into `?` question marks.)
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I86073f48aaf1e0a58e97676fb80e2475ec418ffc
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61308
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
SPL: Security Patch Level
The data in SPL is used for FW anti-rollback, preventing rollback of
platform level firmware to older version that are deemed vulnerable
from a security point of view.
BUG=b:216096562
Change-Id: I4665f2372ccd599ab835c8784da08cde5558a795
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61426
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Add support to generate SPD binary for Sabrina SoC. Mainboards using
Sabrina SoC are planning to use LP5 memory technology. Some of the SPD
bytes expected by Sabrina differ from the existing ADL. To start with,
memory training code for Sabrina expects SPD Revision 1.1. More patches
will follow to accommodate additional differences.
BUG=b:211510456
TEST=make -C util/spd_tools.
Generate SPD binaries for the existing memory parts in
lp5/memory_parts.json and observe that SPDs for Sabrina is generated as
a separate set without impacting the ADL mainboards.
Signed-off-by: Karthikeyan Ramasubramanian <kramasub@google.com>
Change-Id: I2a2c0d0e8c8cbebf3937a99df8f170ae8afc75df
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61542
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Reka Norman <rekanorman@chromium.org>
Reviewed-by: Nick Vaccaro <nvaccaro@google.com>
Some Intel SoCs such as Denverton support additional SPI regions for
things like Innovation Engine firmware or 10GbE LAN firmwares
Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
Change-Id: Ia5a450e5002e9f8edee76ca7c2eede9906df36c5
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60829
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
Reviewed-by: Subrata Banik <subratabanik@google.com>
Currently, the function normalize_dirs() fails if the directories lib32
and lib64 don't exist. That can be fixed by using an rm -rf on it
instead of rmdir.
The cmake build doesn't create those directories, so was showing a
failure message after the build was already completed. That's fixed by
removing normailze_dirs() from the build_CMAKE() function.
Signed-off-by: Martin Roth <gaumless@gmail.com>
Change-Id: Iea6e3ca57fb91ff1234be875861b27a78972d9ca
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61515
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
This is to reduce difference with linux v5.16.
Change-Id: I7abd4d8eed856eee841422515db2ff7f50ecd0a4
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61471
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
This is to reduce difference with linux v5.16.
Change-Id: I3bdf880c8b6068467665865b7cf1249d1047e833
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61470
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
This is to reduce difference with linux v5.16.
Change-Id: I1b7bc2b4ec832f0abeda215c381856a5ec153883
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61469
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
This is to reduce difference with linux v5.16.
Change-Id: I4aa7abce83b41ccd5129717cd3bf85be19ec4807
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61467
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
Also use '$minimum_perl_version'.
This is to reduce difference with linux v5.16.
Change-Id: I7c2f5d5c9853dc8ddc8f89a5e2edd6c8613ba790
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61466
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
This is to reduce difference with linux v5.16.
Change-Id: Ifeb9c4406737fa24f9bd803af48d8b8d17654940
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60874
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
Patch 423e9e0fc0: Documentation/lint: Use Super I/O instead of SuperIO
added the word SuperIO to the checkpatch spelling list.
There were unfortunately some issues with this.
1) This introduced a problem because the comparison is used in different
cases in different places. The misspelled word is compared ignoring
the case, but when looking for the correct word, it looks through the
list for the misspelling in all lowercase. When it couldn't find the
word "superio" in the list, the variable came back uninitialized.
2) The spellcheck feature isn't enabled in checkpatch unless the option
--strict is enabled, so this wasn't getting reported anyway.
3) SuperIO (or superio) will match the KCONFIG options such as
CONFIG_SUPERIO_NUVOTON_NCT5104D, and suggest "Super I/O" which doesn't
make any sense.
Signed-off-by: Martin Roth <gaumless@gmail.com>
Change-Id: I464305af539926ac8a45c9c0d59eeb2c78dea17a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61434
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Adding gif files to the whitespace exclude list, to prevent issue where
commits were failing due to binary files.
Change-Id: I56679780348579d01c81c6f1677e4ea456315c9e
Signed-off-by: Fred Reitberger <reitbergerfred@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61460
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Matt DeVillier <matt.devillier@gmail.com>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
- Handle older CrOS firmware which lacks a COREBOOT FMAP region
- Add support for all blobs used in CrOS firmware 2013 to current
- Put extracted blobs in their own directory
Change-Id: Idaa39eca3be68a9327cead9b21c35a6c7a3a8166
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59266
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
A simple GUI to change settings in coreboot's CBFS, via the nvramtool utility.
Test on the StarBook Mk IV running coreboot 4.15 with:
* Ubuntu 20.04
* Ubuntu 21.10
* MX Linux 21
* elementary OS 6
* Manjaro 21
Signed-off-by: Sean Rhodes <sean@starlabs.systems>
Change-Id: I491922bf55ed87c2339897099634a38f8d055876
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59256
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
Add the file templates for creating a new variant of Brask.
BUG=b:215091592
TEST=new_variant.py and build coreboot pass for the new variant.
Change-Id: I67e4ed450d6033fed7419bd7c76c127ecd942fe8
Signed-off-by: Zhuohao Lee <zhuohao@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61184
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Rebase all of the timestamps to the lowest (potentially negative) value
in the list when displaying them. Also drop the extra
`timestamp_print_*_entry` calls for time 0 and instead inserted a
"dummy" timestamp entry of time 0 into the table.
TEST=Boot to OS after adding negative timestamps, cbmem -t
Signed-off-by: Bora Guvendik <bora.guvendik@intel.com>
Change-Id: I7eb519c360e066d48dde205401e4ccd3b0b3d8a5
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59555
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Reviewed-by: Subrata Banik <subratabanik@google.com>
This also uncouples cbfstool from being overly Chromium
specific. However the main objective is to not subprocess
flashrom any more and instead use the programmatic API.
BUG=b:207808292
TEST=built and ran `elogtool (list|clear|add 0x16 C0FFEE)`.
Change-Id: I79df2934b9b0492a554a4fecdd533a0abe1df231
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59714
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Sam McNally <sammc@google.com>
Instead of maintaining another set of byteswapping functions in
cbfstool, this change removes swab.h and replaces it with
bsd/sysincludes.h from commonlib. Callers have been updated to use
be32toh/be64toh/htobe32/htobe64 instead of ntohl/ntohll/htonl/htonll
respectively.
Change-Id: I54195865ab4042fcf83609fcf67ef8f33994d68e
Signed-off-by: Alex James <theracermaster@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60233
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Julius Werner <jwerner@chromium.org>
apcb_v3_edit.py tool edits APCB V3 binaries. Specifically it will inject
up to 16 SPDs into an existing APCB. The APCB must have a magic number
at the top of each SPD slot.
BUG=b:209486191
BRANCH=None
TEST=Inject 4 SPDs into magic APCB, boot guybrush with modified APCB
Change-Id: I9148977c415df41210a3a13a1cd9b3bc1504a480
Signed-off-by: Rob Barnes <robbarnes@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60281
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
After removing GDB from crossgcc in commit f32eed16 (buildgcc: Remove
GDB from crossgcc), there is no target named all_without_gdb anymore
and we should always build crossgcc with target all.
But in util/docker/Makefile, we still try to build crossgcc with
target all_without_gdb as default and will cause a build failure.
Set CROSSGCC_PARAM from all_without_gdb to all to fix this issue.
Signed-off-by: Hsuan Ting Chen <roccochen@chromium.org>
Change-Id: I06c6d8e36dfd4e6a00ddec8b640b608ab1ba614c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60268
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Reviewed-by: Martin Roth <martinroth@google.com>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
This restricts availability of non-standard functions (such as memmem)
on FreeBSD and macOS. It also isn't necessary on glibc.
Change-Id: Iaee1ce7304c89f128a35a385032fce16a2772b13
Signed-off-by: Alex James <theracermaster@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60232
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Julius Werner <jwerner@chromium.org>
flashmap/fmap.c includes commonlib/bsd/sysincludes.h, which already
includes the necessary header for endian(3) functions (endian.h on
Linux and sys/endian.h on FreeBSD). This also resolves a compilation
error on macOS (tested on 10.5.7), as macOS does not provide endian.h.
Change-Id: I0cb17eacd253605b75db8cf734e71ca3fe24ad6c
Signed-off-by: Alex James <theracermaster@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60228
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
The intel-spi driver maps the BIOS region of the flash as an mtd device
at /dev/mtdX. Since this system is intended for development purposes,
disable its write protection.
Change-Id: Ib73d14eb4e7df6e29433b8dfbeb77dbab4a85f08
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60375
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
Config options and package names might change from channel to channel.
Thus, don't let nix-build depend on the locally configured NixOS
channel, but instead let `nixpkgs` point to a specific channel to ensure
that always a compatible channel is used.
For now, let `nixpkgs` point to NixOS 21.11, which is currently the
latest stable release. This needs to be updated after a new release.
Change-Id: Ia77c34f93f0e2c3d351ae229830adfce75a56ae4
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60373
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
Merge build scripts to `build.sh`. The new one takes the desired NixOS
config as an argument.
Example:
$ build.sh console.nix
Change-Id: I49360a5c57954a205c697a4ae07361779db2aa83
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60372
Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
futility actually depends on flashrom. Previously it
was of the form of subprocess and now uses the libflashrom
API directly. Due to the previous subprocess decoupling it
was not obvious that the dependency existed however not
the runtime requirement is also a strict buildtime requirement.
Therefore update the Makefile accordingly.
BUG=b:203715651,b:209702505
TEST=builds
Change-Id: Id9744424f75299eb8335c1c0c2aca2808bde829d
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60236
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
Reviewed-by: Hsuan-ting Chen <roccochen@google.com>
Address Mode 0: Physical Address, bit 63~56: 0x00
Address Mode 1: Relative Address to entire BIOS image, bit 63~56: 0x40
Address Mode 2: Relative Address to PSP/BIOS directory, bit 63~56: 0x80
Address Mode 3: Relative Address to slot N, bit 63~56: 0xC0
It is the expanding mode for simple relative address mode, for which
address_mode equals 1.
Only mode 2 is added. We need to record current table base address and
calculate the offset. The ctx.current_table is zero outside the
table. When it goes into the function to integrate the table, it
should backup the old value and get current table base. Before it goes
out the function, it should restore the value.
If the table address mode is 2, the address in each entry should be
also add address mode information. If not, the address mode in entry
is meanless.
The old mode 0,1 should be back compatible.
Change-Id: I29a03f4381cd0507e2b2e3b359111e3375a73de1
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59308
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
`cbfstool locate` and the associated -T switch were removed a looong
time ago (2015 in CB:11671). However, getopt and the help text weren't
cleaned up correctly. Fix that.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Ib098278d68df65d348528fbfd2496b5737ca6246
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60085
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
The whole point of moving do_cbfs_locate() later (CB:59877) was that it
could use the file size that is actually going to be inserted into CBFS,
rather than the on-disk file size. Unfortunately, after all that work I
forgot to actually make it do that. This patch fixes that.
Since there is no more use case for do_cbfs_locate() having to figure
out the file size on its own, and that generally seems to be a bad idea
(as the original issue shows), also remove that part of it completely
and make the data_size parameter mandatory.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I1af35e8e388f78aae3593c029afcfb4e510d2b8f
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60084
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
The second generation EFS (offset 0x24[0]=0) uses "binary relative"
offsets and not "x86 physical MMIO address" like gen1.
The field additional_info in table header can tell if the absolute or
relative address is used.
Chips like Cezanne can run in both cases, so no problem
comes up so far.
The related change in psp_verstage has been uploaded.
https://review.coreboot.org/c/coreboot/+/58316
The relative mode is the mode 1 of four address modes. The absolute
mode is the mode 0. Later we will implement mode 2. Not sure if mode 3
is needed.
It needs to be simple to work with psp_verstage change to make SOC
Cezanne work quickly. This patch is defacto a subset of
https://review.coreboot.org/c/coreboot/+/59308
which implements the framework of address mode and covers mode
0,1,2. Some hardcode value like 29 can be removed in 59308.
BUG=b:188754219
Test=Majolica (Cezanne)
Change-Id: I7701c7819f03586d4ecab3d744056c8c902b630f
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56438
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Kangheui Won <khwon@chromium.org>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
In cbfs_add_component(), the |offset| variable confusingly jumps back
and forth between host address space and flash address space in some
cases. This patch tries to clean that logic up a bit by converting it
to flash address space very early in the function, and then keeping it
that way afterwards. convert() implementations that need the host
address space value should store it in a different variable to reduce
the risk of confusion. This should also fix a tiny issue where
--gen-attribute might have previously encoded the base address as given
in CBFS -- it probably makes more sense to always have it store a
consistent format (i.e. always flash address).
Also revert the unnecessary check for --base-address in
add_topswap_bootblock() that was added in CB:59877. On closer
inspection, the function actually doesn't use the passed in *offset at
all and uses it purely as an out-parameter. So while our current
Makefile does pass --base-address when adding the bootblock, it actually
has no effect and is redundant for the topswap case.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Idf4721c5b0700789ddb81c1618d740b3e7f486cb
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60018
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Currently, `checkpatch.pl`, imported from the Linux project, checks for
75 characters per line [2]:
> Suggest line wrapping at 75 columns so the default git commit log
> indentation of 4 plus the commit message text still fits on an 80
> column screen.
But Gerrit’s Web interface and its commit hooks use with 72 characters
per line [2]:
remote: commit 35bb56d: warning: too many message lines longer than 72 characters; manually wrap lines
remote:
remote: SUCCESS
remote:
remote: https://review.coreboot.org/c/coreboot/+/60004 [DO NOT SUBMIT] Gerrit commit msg line length test [NEW]
So, decrease the suggested length from 75 to 72 characters per line.
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2a076f40d8c9be95bee7bcf18436655e1140447f
[2]: https://review.coreboot.org/60004
Change-Id: Ic9c686cb1a902259b18377b76b5c999e94660fed
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60006
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
As part of the `what-jenkins-does` target, combine the code coverage
data from all unit tests (currently just coreboot and libpayload).
BUG=b:203800199
TEST=`make what-jenkins-does && ls -l coreboot-builds/coverage.info`
Signed-off-by: Paul Fagerburg <pfagerburg@google.com>
Change-Id: Id99615ca8279f80a402d5371221b8fd36fb91d55
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59959
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <patrick@coreboot.org>
The placement calculation logic in cbfs_add_component() has become quite
a mess, and this patch can only fix that to a limited degree. The
interaction between all the different pathways of how the `offset`
variable can be set and at what point exactly the final placement offset
is decided can get quite convoluted. In particular, one existing problem
is that the offset for a file added with the --align flag is decided
before the convert() function is called, which may change the form (and
thereby the size) of the file again after its location was found --
resulting in a location that ends up being too small, or being unable to
find a location for a file that should fit. This used to be okay under
the assumption that forced alignment should really only be necessary for
use cases like XIP where the file is directly "used" straight from its
location on flash in some way, and those cases can never be compressed
-- however, recent AMD platforms have started using the --align flag to
meet the requirements of their SPI DMA controller and broken this
assumption.
This patch fixes that particular problem and hopefully eliminates a bit
of the convolution by moving the offset decision point in the --align
case after the convert() step. This is safe when the steps in-between
(add_topswap_bootblock() and convert() itself) do not rely on the
location having already been decided by --align before that point. For
the topswap case this is easy, because in practice we always call it
with --base-address (and as far as I can tell that's the only way it was
ever meant to work?) -- so codify that assumption in the function. For
convert() this mostly means that the implementations that do touch the
offset variable (mkstage and FSP) need to ensure they take care of the
alignment themselves. The FSP case is particularly complex so I tried to
rewrite the code in a slightly more straight-forward way and clearly
document the supported cases, which should hopefully make it easier to
see that the offset variable is handled correctly in all of them. For
mkstage the best solution seems to be to only have it touch the offset
variable in the XIP case (where we know compression must be disabled, so
we can rely on it not changing the file size later), and have the extra
space for the stage header directly taken care of by do_cbfs_locate() so
that can happen after convert().
NOTE: This is changing the behavior of `cbfstool add -t fsp` when
neither --base-address nor --xip are passed (e.g. FSP-S). Previously,
cbfstool would implicitly force an alignment of 4K. As far as I can tell
from the comments, this is unnecessary because this binary is loaded
into RAM and CBFS placement does not matter, so I assume this is an
oversight caused by accidentally reusing code that was only meant for
the XIP case.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Ia49a585988f7a74944a6630b77b3ebd79b3a9897
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59877
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <patrick@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Update configs so that they work with NixOS 21.11. Drop `iasl` package
since it was replaced with `acpica-tools`.
Change-Id: Icb9a382b83b3b3e55126bb0bb508659d11497a05
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59881
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
endian.h wasn't included (although it probably came in as an indirect
include) but in some header sets _XOPEN_SOURCE overrides _DEFAULT_SOURCE
whereas the latter is a super set of the former:
We should get the same things as with _XOPEN_SOURCE (such as memccpy for
which it has been defined) but also extra features like htole32.
Change-Id: Iaee7495b2ae64fdc719ae0879ea95fe7df286212
Signed-off-by: Patrick Georgi <patrick@coreboot.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59891
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Rob Barnes <robbarnes@google.com>
Matt DeVillier