summaryrefslogtreecommitdiff
path: root/util
AgeCommit message (Collapse)Author
2022-02-07util/spd_tools/spd_gen: Add support for Sabrina SoCKarthikeyan Ramasubramanian
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>
2022-02-04util/ifdtool: Add additional regions for platforms that support themJeff Daly
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>
2022-02-03util/crossgcc: Update this for normailze_dirs()Martin Roth
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>
2022-02-01util/lint/checkpatch.pl: Use "git_command"Elyes HAOUAS
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>
2022-01-31util/lint/checkpatch.pl: Use "gitroot"Elyes HAOUAS
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>
2022-01-31util/lint/checkpatch: Update "check for missing blank lines after declarations"Elyes HAOUAS
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>
2022-01-31util/lint/checkpatch.pl: Update 'commit message line length limit'Elyes HAOUAS
Also add "coreboot" comment on our modification. Change-Id: Ida58a92457e25bac7fb89bb5882e7647f388ec01 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/61468 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2022-01-31util/lint/checkpatch.pl: Remove unneeded whitespaces and fix some typosElyes HAOUAS
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>
2022-01-31util/lint/checkpatch.pl: Use "perl_version_ok"Elyes HAOUAS
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>
2022-01-31util/lint/checkpatch.pl: Use "tabsize"Elyes HAOUAS
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>
2022-01-31util/lint: Remove SuperIO from checkpatch spellcheckMartin Roth
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>
2022-01-28util/lint/lint-stable-003-whitespace: add exception for gif filesFred Reitberger
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>
2022-01-28crossgcc: Update acpica from 20210331 to 20211217Elyes HAOUAS
Changes: Version 20211217: https://acpica.org/node/197 Version 20210930: https://acpica.org/node/196 Version 20210730: https://acpica.org/node/195 Version 20210604: https://acpica.org/node/193 Change-Id: I3a03b74e95f910b50aa2f7ce502b1e9ad5b6df37 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59174 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2022-01-27util/chromeos: Update extract_blobs scriptMatt DeVillier
- 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>
2022-01-27util/coreboot-configurator: Add contrib filesSean Rhodes
Add contrib files for: * debian (Tested on Ubuntu 20.04, 21.10, MX Linux 21 and Debian) * PKGBUILD (Tested on Manjaro 21) * flatpak (Untested) Signed-off-by: Sean Rhodes <sean@starlabs.systems> Change-Id: Ie9f0193ed28c0842661426204fc88ec00091fbae Reviewed-on: https://review.coreboot.org/c/coreboot/+/59257 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2022-01-27util: Add coreboot-configuratorSean Rhodes
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>
2022-01-26util/mb/google: add support for braskZhuohao Lee
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>
2022-01-21util/cbmem: Rebase to handle negative timestampsBora Guvendik
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>
2022-01-14util/cbfstool: Port elogtool to libflashromEdward O'Callaghan
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>
2022-01-12util/cbfstool: Do minor fixesAlex James
- Fix truncation of stage->loadaddr by replacing be32toh with be64toh - Remove some redundant htobe32 calls - Address checkpatch lints Change-Id: I81b8cfd9eb0b2feffefaa9338bac9ae209e39a3c Signed-off-by: Alex James <theracermaster@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/60933 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Julius Werner <jwerner@chromium.org>
2022-01-10util/cbfstool: Replace swab.h with commonlib/bsd/sysincludes.hAlex James
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>
2022-01-10util/apcb: Add apcb_v3_edit toolRob Barnes
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>
2022-01-04docker/Makefile: Use all instead of all_without_gdbHsuan Ting Chen
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>
2022-01-04cbfstool: Avoid defining _XOPEN_SOURCEAlex James
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>
2022-01-04util/cbfstool: Remove redundant endian.h includeAlex James
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>
2021-12-29crossgcc/buildgcc: Remove unused GCC_AUTOCONF_VERSIONElyes HAOUAS
Clean up leftovers from commit d0f83723 and drop unused GCC_AUTOCONF_VERSION. Change-Id: I7d293ae2c8663efdc9ad4146ff32671ffd3e176a Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59398 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Singer <felixsinger@posteo.net> Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
2021-12-29crossgcc/Makefile: Remove obsolete target build_makeElyes HAOUAS
coreboot does not build gnumake anymore since commit 91fb1399 Change-Id: I0f159fc912d09ebde6ac7ba5be83933aa251f1d5 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59397 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Singer <felixsinger@posteo.net> Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
2021-12-26util/liveiso: Install mtdutilsFelix Singer
Change-Id: I1416d8f783518eca0606efef4314a3d86837b016 Signed-off-by: Felix Singer <felixsinger@posteo.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/60376 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
2021-12-26util/liveiso: Disable write protection of the intel-spi driverFelix Singer
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>
2021-12-26util/liveiso: Ensure compatible NixOS channel is usedFelix Singer
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>
2021-12-26util/liveiso: Merge build scriptsFelix Singer
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>
2021-12-25util/futility: Ensure futility checks for flashrom as a depEdward O'Callaghan
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>
2021-12-16amdfwtool: Upgrade "relative address" to four address modesZheng Bao
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>
2021-12-16Spell *Boot Guard* with a space for official spellingPaul Menzel
See for example Intel document *Secure the Network Infrastructure – Secure Boot Methodologies* [1]. Change all occurrences with the command below: $ git grep -l BootGuard | xargs sed -i 's/BootGuard/Boot Guard/g' [1]: https://builders.intel.com/docs/networkbuilders/secure-the-network-infrastructure-secure-boot-methodologies.pdf Change-Id: I69fb64b525fb4799bcb9d75624003c0d59b885b5 Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/60136 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2021-12-14cbfstool: Clean up remnants of locate actionJulius Werner
`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>
2021-12-14cbfstool: Use converted buffer size for do_cbfs_locate()Julius Werner
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>
2021-12-14amdfwtool: Use relative address for EFS gen2Zheng Bao
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>
2021-12-13cbfstool: Do host space address conversion earlier when adding filesJulius Werner
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>
2021-12-10util/lint/checkpatch: Decrease commit message line length limit to 72Paul Menzel
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>
2021-12-08util/testing: combine code coverage dataPaul Fagerburg
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>
2021-12-07cbfstool: Fix offset calculation for aligned filesJulius Werner
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>
2021-12-06util/liveiso: Update to NixOS 21.11Felix Singer
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>
2021-12-06util/cbfstool: Ensure that htole32 et al are visible when buildingPatrick Georgi
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>
2021-11-30commonlib: Move commonlib/cbmem_id.h to commonlib/bsd/Jakub Czapiga
Libpayload requires cbmem_id.h file to support extracting values from CBMEM IMD entries of coreboot tables. Libpayload use BSD-3-Clause license, and all of its files used to compile a static library have to use it too. Change-Id: I97c080e34ebdbcdf14fe3a3c9515b1dea8ede179 Signed-off-by: Jakub Czapiga <jacz@semihalf.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59696 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <patrick@coreboot.org>
2021-11-29util/cbfstool/.gitignore: Add CSE tool executablesAngel Pons
Commit 796aeeba96fc (util/cse_fpt: Add a new tool for managing Intel CSE FPT binaries) and commit d7fb6a90e1d0 (util/cse_serger: Add a new tool for stitching CSE components) add two utilities, and building cbfstool also generates executables for them. When building cbfstool standalone, these executables are placed in `util/cbfstool/`, and Git should never track them. Specify these executables' file names in .gitignore in order to prevent unintentional inclusion of these files in commits, which is very likely to happen when using `git add` on directories. Change-Id: I285a4d7aeee642822eaae2eb69e5d52efb4bc8c0 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59670 Reviewed-by: Paul Menzel <paulepanter@mailbox.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-11-26util/ifdtool/Makefile: Derive from Makefile.incPatrick Georgi
Instead of maintaining two complete Makefiles, reuse the coreboot build system rules in the stand-alone Makefile. Change-Id: I5d894a1f079799478bce0bd200ac735097f3806b Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59669 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Paul Menzel <paulepanter@mailbox.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-11-26util/testing: Add ifdtool to tools to be testedPatrick Georgi
Ensure that the separate Makefile doesn't break. Change-Id: I0fbe37dc01e46022c5e6de5629eb99f6b86b0b14 Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59664 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
2021-11-26util/ifdtool/Makefile: Fix building as standalone toolAngel Pons
Commit f1e401c6cb70 (util/cbfstool/flashmap/fmap.c: fix fmaptool endianness bugs on BE) makes use of endianness conversion macros in cbfstool's FMAP code, which is also used by ifdtool. At least on Linux, the <endian.h> header provides these helpers, but only when `__USE_MISC` is defined, which is defined in the <ctypes.h> header when `_DEFAULT_SOURCE` is defined. This was accounted for in `Makefile.inc`, but not in `Makefile`. As a result, trying to build ifdtool as a standalone tool (i.e. not as part of building a coreboot image) results in build errors because the endianness conversion macros are not defined. Define `_DEFAULT_SOURCE` in `Makefile` to fix the build errors. Change-Id: I8c2bbc07ddd87d885e2d6f5c7f2bd501e5c4e3b0 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59663 Reviewed-by: Patrick Georgi <patrick@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Paul Menzel <paulepanter@mailbox.org> Reviewed-by: Krystian Hebel <krystian.hebel@3mdeb.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-11-24util/testing: Give meaningful error if intel-sec-tools aren't aroundPatrick Georgi
Without manual handling, when 3rdparty/intel-sec-tools isn't around, `make what-jenkins-does` reports only go: go.mod file not found in current directory or any parent directory; see 'go help modules' which isn't meaningful or actionable. Instead check that the go.mod file exists and bail out with a better error message before trying to run `go mod vendor`. Change-Id: I035747746ca5fd54841bd67352044dde12a28185 Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/57527 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
2021-11-22util/cbfstool/flashmap/fmap.c: fix fmaptool endianness bugs on BEMarek Kasiewicz
This patch makes all accesses to the FMAP fields explicitly little endian. It fixes issue where build on BE host produced different binary image than on LE. Signed-off-by: Marek Kasiewicz <marek.kasiewicz@3mdeb.com> Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com> Change-Id: Ia88c0625cefa1e594ac1849271a71c3aacc8ce78 Reviewed-on: https://review.coreboot.org/c/coreboot/+/55039 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Julius Werner <jwerner@chromium.org>
2021-11-22eventlog: Add a log type for Chrome OS diagnosticsHsuan Ting Chen
Add events for Chrome OS diagnostics in eventlog tool: * ELOG_TYPE_CROS_DIAGNOSTICS(0xb6): diagnostics-related events * ELOG_CROS_LAUNCH_DIAGNOSTICS(0x01): sub-type for diagnostics boot These events are not added anywhere currently. They will be added in another separate commit. Signed-off-by: Hsuan Ting Chen <roccochen@chromium.org> Change-Id: I1b67fdb46f64db33f581cfb5635103c9f5bbb302 Reviewed-on: https://review.coreboot.org/c/coreboot/+/58795 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jack Rosenthal <jrosenth@chromium.org> Reviewed-by: Yu-Ping Wu <yupingso@google.com>
2021-11-19util/inteltool/gpio.c: Correct register nameAngel Pons
Document 319973-003 (ICH10 datasheet) and document 324645-006 (6-series PCH datasheet) indicate that the name of this register is `GP_LVL3`, not `GPIO_LVL3`. Correct the name. Change-Id: I44cc41843c9f7cd0796bd198fb89447d787f155a Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59289 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Christian Walter <christian.walter@9elements.com> Reviewed-by: Felix Singer <felixsinger@posteo.net>
2021-11-19Documentation/lint: Use Super I/O instead of SuperIOPatrick Georgi
Change-Id: Idb16092b687ebffb319bc1908f08f350d612d36a Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/39451 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@mailbox.org> Reviewed-by: Martin Roth <martinroth@google.com>
2021-11-19util/inteltool: Add ICH10D PCI IDAngel Pons
Add the PCI device ID for the ICH10D southbridge. While we're at it, also fix up whitespace in inteltool.h of an adjacent definition. Change-Id: I98d88a9ce27d3ddaafd7123ee51b2111a8bef019 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59287 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Christian Walter <christian.walter@9elements.com> Reviewed-by: Felix Singer <felixsinger@posteo.net>
2021-11-19util/lint: Fix linters to work with coreboot-configuratorSean Rhodes
* Exclude .gif files from newline checking * Exclude coreboot-configurator from checkpatch checking Signed-off-by: Sean Rhodes <sean@starlabs.systems> Change-Id: I1b07b7b05340409e5c1695cc7bbdea68f8190097 Reviewed-on: https://review.coreboot.org/c/coreboot/+/59096 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <patrick@coreboot.org>
2021-11-18amdfwtool: Call the set_efs_table for StoneyridgeZheng Bao
Related to https://review.coreboot.org/c/coreboot/+/58555 commit-id: 35b7e0a2d82ac In 58555, we added the SOC ID for Stoneyridge in amdfwtool command line. But it raised building error because it then called "set_efs_table" without setting SPI mode. So we skipped calling that. But in set_efs_table, it has case for Stoneyridge. The boards also need to have this setting. So we remove the skipping and give the proper SPI mode in mainboard Kconfig. Change-Id: I24499ff6daf7878b12b6044496f53379116c598f Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58871 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2021-11-18amdfwtool: Set flag comboable as bool typeZheng Bao
Fix the CL: https://review.coreboot.org/c/coreboot/+/58942 The type comboable was int but set as true. Change-Id: Id2c43378735c089a27a5aa683b55a0f7ec3677de Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59093 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2021-11-18amdfwtool: Add a union for combo and psp directoryZheng Bao
For combo layout, this is for combo header. For non-combo layout, this is for PSP directory. Change-Id: Ie7b5aec6b511ad61972908d1d22a13aeb7dd73a9 Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58557 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2021-11-17util/mb/google: Change comments in memory Makefile.inc templatesReka Norman
Begin comments with # instead of ## to match the Makefile.inc generated by spd_tools. BUG=None TEST=None Signed-off-by: Reka Norman <rekanorman@google.com> Change-Id: If2d716a7338fd5af8216b2bcd894fc88a9df137e Reviewed-on: https://review.coreboot.org/c/coreboot/+/59297 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
2021-11-15amdfwtool: Set soc name for StoneyridgeZheng Bao
For the stoneyridge, soc_name is not set in Makefile, so set_efs_table is not called. Keep it unchanged. Change-Id: I0e82188ce64733420a578446e22a077ef789be92 Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58555 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2021-11-14util/docker/coreboot-sdk: Add bsdextrautils & lcovMartin Roth
Add lcov for coverage calculations. Add bsdextrautils for hexdump. Signed-off-by: Martin Roth <gaumless@gmail.com> Change-Id: I421c59ce2d0d08bf5142dbc378eeea45b8b1d5b5 Reviewed-on: https://review.coreboot.org/c/coreboot/+/59019 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <patrick@coreboot.org>
2021-11-11amdfwtool: Pack out-of-bounds check into a function and moveZheng Bao
Need to check the FWs number limit several times. So pack the duplicated steps into a function. And do it before access the new entry. Change-Id: I71117d1c817c0b6ddaea4ea47aea91672cc6d55a Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58764 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2021-11-09util/spd_tools: Document adding support for a new memory technologyReka Norman
Add documentation describing how to add support for a new memory technology to spd_tools: - Add a section to the README. - Document the memTech interface in spd_gen.go. BUG=b:191776301 TEST=None Signed-off-by: Reka Norman <rekanorman@google.com> Change-Id: Ie710c1c686ddf5288db35cf43e5f1ac9b1974305 Reviewed-on: https://review.coreboot.org/c/coreboot/+/59005 Reviewed-by: Karthik Ramasubramanian <kramasub@google.com> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-11-09util/lint/kconfig_lint: Fix off by one error that missed last lineMartin Roth
This error prevented the last line of the Kconfig tree from being printed or added to the output file. This is a significant problem if you try to use the generated file as the kconfig source, because it changes CONFIG_HAVE_RAMSTAGE from defaulting to yes to defaulting to NO. This causes the build to stop working. Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: I3ec11f1ac59533a078fd3bd4d0dbee9df825a97a Reviewed-on: https://review.coreboot.org/c/coreboot/+/58992 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Nico Huber <nico.h@gmx.de>
2021-11-05amdfwtool: Change the flag value to type boolZheng Bao
Change-Id: I8bb87e6b16b323b26dd5b411e0063e2e9e333d05 Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58942 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2021-11-05amdfwtool: Fix the parameter point to NULL instead of integerZheng Bao
Change-Id: Iaeeec7a7e2de7847bfcefa5b7ff3f259f86533d4 Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58941 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2021-11-05amdfwtool: Change the definition of level to a bitwise formZheng Bao
Change-Id: Icca393f0d69519cc1c3cb852a11dd7006cf72061 Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58765 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2021-11-04util/testing: add code coverage to jenkinsPaul Fagerburg
Add COV=1 and the `coverage-report` target to unit test build rules in `what-jenkins-does` so that we get code coverage data from the coreboot and libpayload unit tests. Signed-off-by: Paul Fagerburg <pfagerburg@google.com> Change-Id: I96669c47d1a48e9ab678a4b9cb1d0c8032d727f0 Reviewed-on: https://review.coreboot.org/c/coreboot/+/58634 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2021-11-04util/spd_tools: Add LP5 support for ADLReka Norman
Add LP5 support to spd_tools. Currently, only Intel Alder Lake (ADL) is supported. The SPDs are generated based on a combination of: - The LPDDR5 spec JESD209-5B. - The SPD spec SPD4.1.2.M-2 (the LPDDR3/4 spec is used since JEDEC has not released an SPD spec for LPDDR5). - Intel recommendations in advisory #616599. BUG=b:201234943, b:198704251 TEST=Generate the SPD and manifests for a test part, and check that the SPD matches Intel's expectation. More details in CB:58680. Change-Id: Ic1e68d44f7c0ad64aa9904b7e1297d24bd5db56e Signed-off-by: Reka Norman <rekanorman@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58679 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2021-11-04SMBIOS/SCONFIG: Allow devtree-defined Type 41 entriesAngel Pons
Introduce the `smbios_dev_info` devicetree keyword to specify the instance ID and RefDes (Reference Designation) of onboard devices. Example syntax: device pci 1c.0 on # PCIe Port #1 device pci 00.0 on smbios_dev_info 6 end end device pci 1c.1 on # PCIe Port #2 device pci 00.0 on smbios_dev_info 42 "PCIe-PCI Time Machine" end end The `SMBIOS_TYPE41_PROVIDED_BY_DEVTREE` Kconfig option enables using this syntax to control the generated Type 41 entries. When this option is enabled, Type 41 entries are only autogenerated for devices with a defined instance ID. This avoids having to keep track of which instance IDs have been used for every device class. Using `smbios_dev_info` when `SMBIOS_TYPE41_PROVIDED_BY_DEVTREE` is not enabled will result in a build-time error, as the syntax is meaningless in this case. This is done with preprocessor guards around the Type 41 members in `struct device` and the code which uses the guarded members. Although the preprocessor usage isn't particularly elegant, adjusting the devicetree syntax and/or grammar depending on a Kconfig option is probably even worse. Change-Id: Iecca9ada6ee1000674cb5dd7afd5c309d8e1a64b Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/57370 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org> Reviewed-by: Nico Huber <nico.h@gmx.de>
2021-11-02util/crossgcc/Makefile: Clean up .PHONY definitionsPatrick Georgi
Order functionally: * first "all" and build-$tools * followed by clean * followed by the architecture targets The order was chosen this way because the architecture targets are the mostly likely to continue to grow. While at it, also fix the build_nasm mention (it was build-nasm) and add build_make. Change-Id: Id58338a512d44111b41503d4c14c08be50d51cde Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58796 Reviewed-by: Martin Roth <martinroth@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-11-02util/kconfig: Uprev to Linux 5.15's kconfigPatrick Georgi
Upstream's changes only affect a script that we don't use. Still, this keeps us in sync with the official version. Change-Id: I39cbbfb8dc816b4f36f92e6bd53f40c733691242 Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58792 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2021-11-02util/kconfig: Uprev to Linux 5.14's kconfigPatrick Georgi
Upstream's changes have been minimal, to the perl script that we don't use and a constness change, so I expect no harm. Still, this keeps us in sync with the official version. Change-Id: I5e5a2400bc3323938da4b946930e2ec119819672 Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/57880 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2021-11-02util/kconfig: Rewrite patch in quilt's normal formPatrick Georgi
This is what quilt writes on `quilt refresh` and what it can apply and unapply cleanly. Change-Id: I8c8586da384b65fd5c21c1c1a093642534f83283 Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/57878 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2021-11-01buildgcc: Remove GDB from crossgccPatrick Georgi
It was added for a specific defunct project by a specific defunct company. Change-Id: Ib56ae0fdc1a50d24ff44c7879c43f8e94a5bfa95 Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58380 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Singer <felixsinger@posteo.net>
2021-10-29amdfwtool: Add PSP ID for combo and ISH header for A/B recoveryZheng Bao
Nobody calls the function until combo or A/B is added, so suppress the warning for now. Test=Majolica (Cezanne) Change-Id: I3082b850fb3fd2d7ae83a1c4dfd89eb7e1bd0f97 Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55551 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2021-10-25elogtool: add pytest for elogtoolRicardo Quesada
This CL adds a python test for elogtool. It tests the basic functionality of elogtool: list, clear and add. A future CL will include more complex tests. BUG=b:172210863 TEST=pytest elogtool_test.py Change-Id: If1241ad070d1c690c84f5ca61c0487ba27c2a287 Signed-off-by: Ricardo Quesada <ricardoq@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/57869 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jack Rosenthal <jrosenth@chromium.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com>
2021-10-24util/inteltool: Add PCH IDs for 200 series chipsetsTimofey Komarov
Signed-off-by: Timofey Komarov <happycorsair@yandex.ru> Change-Id: Iadad5e79aef9da3fac627adc135525a5001a72b6 Reviewed-on: https://review.coreboot.org/c/coreboot/+/55839 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Nico Huber <nico.h@gmx.de>
2021-10-21util/release/build-release: Create cross-toolchain version fileFelix Singer
Add cross-toolchain version file to the release tarball, which can be used for pre-setting the variables used in buildgcc. Change-Id: Iad1e0adaa95b71f161caf978276bfb0a63eac8f4 Signed-off-by: Felix Singer <felixsinger@posteo.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58399 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Nico Huber <nico.h@gmx.de>
2021-10-21util/crossgcc/buildgcc: Use pre-set CROSSGCC_VERSION if possibleFelix Singer
For reproducibility, a version string is appended to the version of the tools used in the cross-toolchain. Currently, git is used to determine that version string at runtime of this script. There are cases, where it's not possible to determine that version string, e.g. when a release tarball is used, and if so, the version string is just `v_`. Thus, allow pre-setting the variable `CROSSGCC_VERSION`. Change-Id: I888ccd877c93436b5e033528c43bd8667b8d2f10 Signed-off-by: Felix Singer <felixsinger@posteo.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58396 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Nico Huber <nico.h@gmx.de>
2021-10-20util/crossgcc/buildgcc: Allow printing only the versionFelix Singer
In preperation to CB:58396, add the parameter `-W|--print-version`, which allows printing the content of `CROSSGCC_VERSION`. In combination with CB:58396, this can be used to pre-set the variable in case of the git history is not accessible. Change-Id: I9a205ca0ecb0ece47eb5d8fa73706478354512ff Signed-off-by: Felix Singer <felixsinger@posteo.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58395 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Nico Huber <nico.h@gmx.de>
2021-10-20util/crossgcc/buildgcc: Remove CROSSGCC_COMMITFelix Singer
For reproducibility, the buildgcc script is copied to the destination folder of the toolchain. `CROSSGCC_COMMIT` is used as a file name extension for the script and was introduced when `CROSSGCC_VERSION` didn't contain the commit yet. Since this is not the case anymore, remove it. Change-Id: Id0a0b657eb828b2728ff787228eaa38be83d9517 Signed-off-by: Felix Singer <felixsinger@posteo.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58450 Reviewed-by: Nico Huber <nico.h@gmx.de> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-10-20util/cbfstool/rmodule: Omit undefined extern symbols from reloc tableRaul E Rangel
When using `DECLARE_OPTIONAL_REGION`, it is assumed that REGION_SIZE(name) == 0 if the region was not defined in the memlayout. When using non-rmodule stages (i.e., bootblock, romstage, etc), this assumption holds true, but breaks down in rmodule (i.e., ramstage) stages. The rmodule tool is not currently omitting undefined externals from the relocation table. e.g., extern u8 _##name##_size[]; This means that when the rmodule loader runs, it will rewrite the symbol from 0 (which is the default the linker assumed) to 0 + offset. This is wrong since the symbol doesn't actually exist. Instead we need to omit the relocation so it continues to keep the default value of 0. BUG=b:179699789 TEST=Print out REGION_SIZE(cbfs_cache) in ramstage and verify it is set to 0. I also see the following printed by the rmodtool now: DEBUG: Omitting relocation for undefined extern: _watchdog_tombstone_size DEBUG: Omitting relocation for undefined extern: _watchdog_tombstone DEBUG: Omitting relocation for undefined extern: _watchdog_tombstone DEBUG: Omitting relocation for absolute symbol: _stack_size DEBUG: Omitting relocation for absolute symbol: _program_size DEBUG: Omitting relocation for absolute symbol: _cbmem_init_hooks_size DEBUG: Omitting relocation for absolute symbol: _payload_preload_cache_size DEBUG: Omitting relocation for absolute symbol: _payload_preload_cache DEBUG: Omitting relocation for absolute symbol: _payload_preload_cache_size DEBUG: Omitting relocation for absolute symbol: _payload_preload_cache DEBUG: Omitting relocation for undefined extern: _cbfs_cache DEBUG: Omitting relocation for undefined extern: _cbfs_cache_size As you can see the _watchdog_tombstone will also be fixed by this CL. Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: Ib57e263fa9014da4f6854637000c1c8ad8eb351a Reviewed-on: https://review.coreboot.org/c/coreboot/+/58376 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Julius Werner <jwerner@chromium.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2021-10-20kconfig_lint: use just one variable for keeping track of choicesMichael Niewöhner
Instead of using two variables, one for the boolean value and one for the path, use just one with the path. Since an empty string evalutes to false, this simplification does not change behaviour. Signed-off-by: Michael Niewöhner <foss@mniewoehner.de> Change-Id: I2f1171789af6815094446f107f3c634332a3427e Reviewed-on: https://review.coreboot.org/c/coreboot/+/58401 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2021-10-20kconfig_lint: put $inside_choice together right in the first placeMichael Niewöhner
Instead of substituting the delimiter later, put $inside_choice together right in the first place. Signed-off-by: Michael Niewöhner <foss@mniewoehner.de> Change-Id: Ia713510a683101c48c86a1c3722ebb1607a29288 Reviewed-on: https://review.coreboot.org/c/coreboot/+/58400 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2021-10-19util/cse_serger: Add command `create-cse-region`Furquan Shaikh
This change adds a new command `create-cse-region` to cse_serger tool which takes as inputs offset:size and file for different CSE partitions and generates the entire CSE region image. BUG=b:189177186 Change-Id: Ib087f5516e5beb6390831ef4e34b0b067d3fbc8b Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58215 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
2021-10-19util/cse_serger: Replace cse_layout_regions with array of regionsFurquan Shaikh
This change replaces `struct cse_layout_regions` with an array of `struct region` and introduces enums for DP and BP[1-4]. This makes it easier to loop over the different regions in following changes. BUG=b:189177186 Change-Id: If3cced4506d26dc534047cb9c385aaa9418d8522 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58214 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
2021-10-19util/cse*: Add cse_helpers.{c,h}Furquan Shaikh
This change moves `read_member` and `write_member` helper functions out of cse_fpt.c and cse_serger.c into cse_helpers.c to avoid duplication. BUG=b:189177186,b:189167923 Change-Id: I7b646b29c9058d892bb0fc9824ef1b4340d2510c Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58201 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
2021-10-19util/cse_serger: Add a new tool for stitching CSE componentsFurquan Shaikh
This change adds a new tool `cse_serger` which can be used to print, dump and stitch together different components for the CSE region. BUG=b:189177186 Change-Id: I90dd809b47fd16afdc80e66431312721082496aa Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55503 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
2021-10-19util/cse_fpt: Add a new tool for managing Intel CSE FPT binariesFurquan Shaikh
This change adds a new tool `cse_fpt` which can be used to print and dump CSE partitions in Flash Partition Table (FPT) format. BUG=b:189167923 Change-Id: I93c8d33e9baa327cbdab918a14f2f7a039953be6 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55259 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
2021-10-19util/amdfwtool: rename PSP related embedded_firmware struct elementsFelix Held
The element at offset 0x14 in the embedded_firmware struct is the pointer to the combo PSP directory header, so rename it from comboable to combo_psp_directory to clarify that this is not a flag, but a pointer to a data structure. Also rename psp_entry to psp_directory since it points to the PSP directory table. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ic6149c17ae813f4dcea71c308054849a1a2e4394 Reviewed-on: https://review.coreboot.org/c/coreboot/+/58419 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Kangheui Won <khwon@chromium.org>
2021-10-18cbfstool: Add helper function `buffer_from_file_aligned_size`Furquan Shaikh
This change adds a helper function `buffer_from_file_aligned_size` that loads a file into memory buffer by creating a memory buffer of size rounded up to the provided `size_granularity` parameter. BUG=b:189177186,b:189167923 Change-Id: Iad3430d476abcdad850505ac50e36cd5d5deecb4 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55989 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
2021-10-18kconfig_lint: Drop overly restrictive rule about choice configsNico Huber
This rule was creating trouble: * A symbol may only be declared inside or outside a choice. The linter treats every occurence of a `config` entry as a symbol declaration, even when it's just setting a default or adding selects. This is not easy to fix as the symbol objects are not created first and then added to the $symbols array when we know what kind of decla- ration we have, but are created incrementally inside this global list. Change-Id: I48a17f6403470251be6b6d44bb82a8bdcbefe9f6 Signed-off-by: Nico Huber <nico.h@gmx.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56410 Reviewed-by: Michael Niewöhner <foss@mniewoehner.de> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Martin Roth <martinroth@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-10-18libpayload: Add unit-tests framework and first test caseJakub Czapiga
This commit adds a unit-tests framework ported from coreboot, and test for drivers/speaker. Usage of the unit-tests framework is same as for the coreboot one. Change-Id: Iaa94ee4dcdc3f74af830113813df0e8fb0b31e4f Signed-off-by: Jakub Czapiga <jacz@semihalf.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58242 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Fagerburg <pfagerburg@chromium.org> Reviewed-by: Yu-Ping Wu <yupingso@google.com>
2021-10-15mainboard: Drop invalid `VGA_BIOS_FILE` defaultsAngel Pons
If the VGA BIOS file path for `VGA_BIOS_FILE` in a mainboard's Kconfig does not exist in the coreboot tree (including submodules), drop it. These files should be stored in the `site-local` subdirectory and the paths specified for each board in `site-local/Kconfig`. For example: config VGA_BIOS_FILE default "site-local/x200_vbios.bin" if BOARD_LENOVO_X200 Note that this is just an example. There are better ways to structure one's `site-local` subfolder. Using the `CONFIG_MAINBOARD_DIR` option would be one of them, though variants may still need special handling. Also, update autoport to not generate `VGA_BIOS_FILE` defaults. Change-Id: I1b5dfba035a42d7943f270f95fb7d32b285584d2 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/51340 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Nico Huber <nico.h@gmx.de> Reviewed-by: Michael Niewöhner <foss@mniewoehner.de> Reviewed-by: Felix Singer <felixsinger@posteo.net>
2021-10-12util/autoport/bd82x6x.go: Fix includesNicholas Chin
Fix include of nvs.h to reflect commit 661ad4666c (ACPI: Select ACPI_SOC_NVS only where suitable); and re-add <device/pci_ops.h>, removed in commit 0aad0531dc (util/autoport/bd82x6x.go: Drop unused includes), as the generated early_init.c uses pci_write_config16(). Signed-off-by: Nicholas Chin <nic.c3.14@gmail.com> Change-Id: Ic1e97cfa7dce0e4d25f7a37c28d3635bdbf6c2a5 Reviewed-on: https://review.coreboot.org/c/coreboot/+/58210 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
2021-10-04util/cbfstool: Add support for ARM64 UefiPayloadRex-BC Chen
UEFI payload is supported on some ARM64 platforms, for example MT8195. As a result, add MACHINE_TYPE_ARM64 to support ARM SystemReady. Signed-off-by: Rex-BC Chen <rex-bc.chen@mediatek.com> Change-Id: I4c0c6e263bd2f518a62ff9db44d72dd31086756a Reviewed-on: https://review.coreboot.org/c/coreboot/+/58055 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Yu-Ping Wu <yupingso@google.com> Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org> Reviewed-by: Hung-Te Lin <hungte@chromium.org>
2021-10-01util/liveiso: Install nvme-cli toolFelix Singer
nvme-cli is used to manage NVMe devices and it supports many vendors. Also, it seems it's commonly used to do firmware updates. Change-Id: I26a78867b01d3af0441827c5b25343a46d7ddea1 Signed-off-by: Felix Singer <felixsinger@posteo.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58056 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
2021-09-26Revert "Revert "util/abuild: Regenerate xcompile on every abuild run""Reka Norman
This reverts commit d94f8bbe9da290e120df20bf244920436e9510e7. This is a reland of https://review.coreboot.org/57651. The original change broke parallel abuild runs since the xcompile file was deleted by every recursive call to abuild. This issue was fixed by rebasing on top of a change which only regenerates the xcompile on non-recursive calls. BUG=None TEST=Parallel abuild run succeeds. Change-Id: I086ba7b2ae1b8b14459838bd18ce962a84aa306d Signed-off-by: Reka Norman <rekanorman@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/57913 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com> Reviewed-by: Nico Huber <nico.h@gmx.de>
2021-09-24elogtool: Fix off-by-one error in month in timestampRicardo Quesada
elogtool was setting the timestamp with the wrong value in the month. This CL fixes that by incrementing the month by one. This is needed since gmtime() returns the month value starting at 0. TEST=pytest elogtool_test.py (see next CL in relation chain) Change-Id: I00f89ed99b049caafba2e47feae3c068245f9021 Signed-off-by: Ricardo Quesada <ricardoq@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/57868 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jack Rosenthal <jrosenth@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com> Reviewed-by: Paul Menzel <paulepanter@mailbox.org>