summaryrefslogtreecommitdiff
path: root/util
AgeCommit message (Collapse)Author
2022-04-04crossgcc: Upgrade IASL from 20211217 to 20220331Elyes Haouas
"REDUNDANT_OFFSET_REMARK" to ignore redundant offset remarks is not needed any more as it’s included upstream. Changes: https://acpica.org/node/199 Signed-off-by: Elyes Haouas <ehaouas@noos.fr> Change-Id: Ice7f9a10051f7f62c53098161fd2f498d724c17d Reviewed-on: https://review.coreboot.org/c/coreboot/+/63279 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
2022-04-04crossgcc: Upgrade CMake from 3.22.2 to 3.23.0Elyes Haouas
Release Notes: https://cmake.org/cmake/help/v3.23/release/3.23.html Signed-off-by: Elyes Haouas <ehaouas@noos.fr> Change-Id: Ib31124baa3cae65211ad361a7d41c9504105be91 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63246 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
2022-04-03util/amdfwtool/data_parse: fix SPL table handling regressionFelix Held
Use the SPL table binary from the config file if no override is specified via the spl-table command line argument. This fixes a regression caused by commit 6c5ec8e31ccbe3d9bbf201c956fc3b54703a9767 (amdfwtool: Add options to support mainboard specific SPL table). Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I93419a878b41b1dfcbf58d930740aaae553120f6 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63314 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-04-02util/cbmem: add type castPaul Fagerburg
arch_convert_raw_ts_entry returns a uint64_t, which needs to be cast on ARM systems to avoid a type error. BUG=b/227871959 TEST=no build errors in downstream Signed-off-by: Paul Fagerburg <pfagerburg@google.com> Change-Id: I87a83758b7f122b77f9631c669c7cd8df66f8d1b Reviewed-on: https://review.coreboot.org/c/coreboot/+/63317 Reviewed-by: Rob Barnes <robbarnes@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-04-01util/ifittool: Fix clearing FIT when setting the pointerArthur Heymans
When setting the FIT pointer, the FIT table is only known later in the codeflow. Change-Id: I658f4fffa997d1f7beaf6d6ae37d2885ae602e5c Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/63035 Reviewed-by: Paul Menzel <paulepanter@mailbox.org> Reviewed-by: Sean Rhodes <sean@starlabs.systems> Reviewed-by: Michael Niewöhner <foss@mniewoehner.de> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-03-31util/cbmem: Add FlameGraph-compatible timestamps outputJakub Czapiga
Flame graphs are used to visualize hierarchical data, like call stacks. Timestamps collected by coreboot can be processed to resemble profiler-like output, and thus can be feed to flame graph generation tools. Generating flame graph using https://github.com/brendangregg/FlameGraph: cbmem -S > trace.txt FlameGraph/flamegraph.pl --flamechart trace.txt > output.svg TEST=Run on coreboot-enabled device and extract timestamps using -t/-T/-S options Signed-off-by: Jakub Czapiga <jacz@semihalf.com> Change-Id: I3a4e20a267e9e0fbc6b3a4d6a2409b32ce8fca33 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62474 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Julius Werner <jwerner@chromium.org>
2022-03-30util/amdfwtool: use ISH support for Sabrina SoCFelix Held
The PSP in the Sabrina SoC uses the image slot header to find the second level PSP directory table, so it needs the ISH to be generated. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I9e6308854147c9f6f72d722215c833ee86ee4f94 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63186 Reviewed-by: Raul Rangel <rrangel@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-03-30util/amdfwtool: add Sabrina SoC typeFelix Held
Add PLATFORM_SABRINA to the enum of supported platforms and integrate it into the existing code. Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ibe52b44395619f697686bd900a522562abbe7646 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63185 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-03-30util/amdfwtool: select A/B recovery when ISH is usedFelix Held
In newer AMD SoCs, the image slot header is used in the AMD A/B recovery scheme, so set recovery_ab to true when need_ish is true. Also move the block of code before the process_config call, since that call will already use the recovery_ab field of the cb_config struct. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I65903765514f215bf5cc9b949d0b95aff781eb34 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63184 Reviewed-by: Raul Rangel <rrangel@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-03-30util/amdfwtool: use table-relative addressing in ISH caseFelix Held
When the image slot header (ISH) is used, the addresses in the PSP and BIOS directory tables need to be relative to the beginning of the table. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ia61f7c8313d5a1af95c68b9177a53a2f5443552a Reviewed-on: https://review.coreboot.org/c/coreboot/+/63183 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-03-30util/genbuild_h: micro-adjust the regexp used to set COREBOOT_MAJOR_VERSIONIdwer Vollering
On FreeBSD, every build target would show warnings from its builtin printf(). Change the regexp to be compatible with BSD sed. This will avoid noise like "printf: 4.14-1278-g5d74ccf1c3: not completely converted". Signed-off-by: Idwer Vollering <vidwer@gmail.com> Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com> Change-Id: I1c0c260fd8d42e23a612a353a288e472cc068c8e Reviewed-on: https://review.coreboot.org/c/coreboot/+/56803 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-by: Nico Huber <nico.h@gmx.de>
2022-03-29amdfwtool: Clear the whole byte of EFS_GENZheng Bao
Change-Id: I434e031e906f73362b1e920e034fa15a8d078ab2 Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/63138 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-03-27amdfwtool: Add ISH header support for A/B recovery layoutZheng Bao
Image Slot Header (ISH) is a new feature. The rom layout for A/B recovery with ISH: EFS -> PSP L1 0x48 -> ISH A -> PSP L2 A -> BIOS L2 A 0x4A -> ISH B -> PSP L2 B -> BIOS L2 B The newer 55758 will updated about the boot priority and update retry in ISH header. Change-Id: Ib0690cde1dce949514c7aacebe13096b7814ceff Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/57747 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-03-27util/amdfwtool: add MSMU, SPIROM_CFG and DMCUB PSP FW typesFelix Held
Compared to Cezanne, the Sabrina SoC has a 3 additional PSP firmware table entries, so add those as a preparation for Sabrina support. Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Iaa5aacd53b3c7637f6d5e94b1a8d92bba57ddb9d Reviewed-on: https://review.coreboot.org/c/coreboot/+/63120 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-03-25util/lint/checkpatch: Update commit message & subject line limitsMartin Roth
The commit message has a (soft) line length limit of 72 characters and the subject has a (soft) line limit of 65 characters. This change updates checkpatch to warn at those limits. Note that neither of these are hard limits because git & gerrit can both handle longer lines, it just doesn't look good. Change-Id: I4ef131a65254e2b184b05e0215969aef97e12712 Signed-off-by: Martin Roth <martin@coreboot.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/63029 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@mailbox.org> Reviewed-by: Elyes Haouas <ehaouas@noos.fr> Reviewed-by: Felix Singer <felixsinger@posteo.net>
2022-03-23amdfwtool: Change the some FW's level for A/B recoveryZheng Bao
The Pubkey(0), PSP bootloader(1) and IKEK(0x21) should be put to level 2 only for A/B recovery for Sabrina, which is going to be the long term and A/B recovery layout only. So the amdfwtool should be changed for Sabrina. The old levels of these 3 FWs are for Cezanne, which doesn't use AB recovery now. Just set the specific field levels in generic Cezanne folder for demo. Leave the fw.cfg in Guybrush unchanged. Change-Id: I11092b52927b2c526a5be719104ba39a790b6fa8 Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62329 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com>
2022-03-22util/spd_tools: Add support for exclusive IDsRobert Zieba
Currently memory parts that use the same SPD are assigned the same ID by spd_tools. This commit adds support for exclusive IDs. When given an exclusive ID a memory part will not share its ID with other parts unless they also have the same exclusive ID. BUG=b:225161910 TEST=Ran part_id_gen and checked that exclusive IDs work correctly and that the current behavior still works in their abscence. Signed-off-by: Robert Zieba <robertzieba@google.com> Change-Id: Ife5afe32337f69bc06451ce16238c7a83bc983c8 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62905 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2022-03-21util/cbmem: Keep original Total Time calculation when no negative timestampsBora Guvendik
"Total time" calculation changed after CL 59555 to include "1st timestamp" value in the calculation. This patch restores original Total Time calculation where "1st timetamp" is subtracted from "jumping to kernel". If pre CPU reset timestamps are added (negative timestamps), "Total time" calculation still includes the pre-reset time as expected. 1) Before https://review.coreboot.org/c/coreboot/+/59555: 0:1st timestamp 225,897 1101:jumping to kernel 1,238,218 (16,316) Total Time: 1,012,281 2) After https://review.coreboot.org/c/coreboot/+/59555: 0:1st timestamp 225,897 1101:jumping to kernel 1,238,218 (16,316) Total Time: 1,238,178 3) After this patch: 0:1st timestamp 225,897 (0) 1101:jumping to kernel 1,238,218 (16,316) Total Time: 1,012,281 BUG=none TEST=Boot to OS, check cbmem -t on Redrix board Signed-off-by: Bora Guvendik <bora.guvendik@intel.com> Change-Id: I0442f796b03731df3b869aea32d40ed94cabdce0 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61839 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Julius Werner <jwerner@chromium.org> Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
2022-03-21amdfwtool: Check the length of matching string before accessingZheng Bao
If AB recovery is enabled and get a "Lx" in fw.cfg, wrong character is got or access violation happens. Change-Id: Ibd8ffe34fd44d860ec2115cd36117da7b02169cd Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62483 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com>
2022-03-17util/liveiso: Remove coreboot toolchain from todoFelix Singer
The coreboot toolchain is a huge blob and increases the size of the build a lot. If needed, the specific toolchain can be added before building the ISO or with `nix-shell` later in the live system, as shown below. $ nix-shell -p coreboot-toolchain.i386 Thus, remove this from the todo list. Change-Id: Ia24ceb84f202828f1c97d3ba5bafbf6af0361bdb Signed-off-by: Felix Singer <felixsinger@posteo.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62194 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Matt DeVillier <matt.devillier@gmail.com> Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
2022-03-15util/ifdtool: Add Meteor Lake platform support under IFDv2Subrata Banik
BUG=b:224325352 TEST=Able to build ifdtool. Signed-off-by: Subrata Banik <subratabanik@google.com> Change-Id: I3564efa27d0271286435284e745458aada987008 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61274 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec@chromium.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
2022-03-10cbmem: Fix console banner matchesJulius Werner
Since the new loglevel markers were added, there will now be a marker character at the beginning of the coreboot banner string, and this will make the existing regular expressions meant to find it fail to match. This patch fixes the problem by just allowing for a single extra character there (any character to avoid the hassle of having to match the marker explicitly). The extra character is optional so that we will still continue to match banners from older versions of coreboot as well. Since the `?` glyph is not available in basic POSIX regular expressions, we have to switch to REG_EXTENDED syntax (should otherwise make no difference). (Also, move side effects out of assert() while I'm here, that's not actually safe for the standard libc implementation.) Signed-off-by: Julius Werner <jwerner@chromium.org> Change-Id: I99fb347eb1cf7b043a2113dfda7c798d6ee38975 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62720 Reviewed-by: Yu-Ping Wu <yupingso@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-03-09util/futility: Don't echo the warning message unless it failsMartin Roth
Currently, all of the commands for building futility are printed as they are run. This change skips printing the check for libcrypto unless the check actually fails. This prevents the error from being displayed when there isn't actually a problem. Signed-off-by: Martin Roth <gaumless@gmail.com> Change-Id: I9ef36c0b64f7cd69d19b8faabd165ef6651c838e Reviewed-on: https://review.coreboot.org/c/coreboot/+/62322 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
2022-03-09cbfstool/linux_trampoline: Fill the ACPI RSDP entryArthur Heymans
With LinuxBoot Linux relied on the legacy method of fetching the RSDP pointer to get ACPI. This uses a more modern approach available since 2018 on the Linux kernel, which involves filling in the zero page. This method takes precedence over any other method of fetching the RSDP in Linux (UEFI, Kexec, Legacy/BIOS). Some UEFI zealots are threatening that legacy code will be removed from Linux so it's best to already adapt to that possibility. Tested on Qemu: - With qemu the RSDP is always in the EBDA, so checking if Linux uses the provided pointer is better done with a forced bad entry - With a fake bad pointer Linux correctly does not find RDSP Change-Id: I688b94608b03b0177c42d2834c7e3beb802ae686 Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62574 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org> Reviewed-by: Nico Huber <nico.h@gmx.de> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2022-03-09amdfwtool: Clear struct match before regular expression matchingZheng Bao
If it is not cleared and the number of strings is fewer than last iteration, the match[3] will keep the last value, which actually should be empty. Add assert to make sure the level is a legal value. BUG=b:222038278 Change-Id: If14e0923fbb1648d83784eb5dc1411c93227db5a Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62482 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-03-09commonlib/bsd: Remove cb_err_tJulius Werner
cb_err_t was meant to be used in place of `enum cb_err` in all situations, but the choice to use a typedef here seems to be controversial. We should not be arbitrarily using two different identifiers for the same thing across the codebase, so since there are no use cases for serializing enum cb_err at the moment (which would be the primary reason to typedef a fixed-width integer instead), remove cb_err_t again for now. Signed-off-by: Julius Werner <jwerner@chromium.org> Change-Id: Iaec36210d129db26d51f0a105d3de070c03b686b Reviewed-on: https://review.coreboot.org/c/coreboot/+/62600 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Yu-Ping Wu <yupingso@google.com> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
2022-03-08util/spd_tools: Encode SDRAM min cycle time (TCKMinPs)Karthikeyan Ramasubramanian
ADL encodes CK cycle time as tCKMin whereas Sabrina encodes WCK cycle time. Encode tCKMin as per the respective advisories. BUG=None TEST=Generate the SPD and ensure that tCKMin is encoded accordingly. Minimum CAS Latency time is also impacted and is encoded accordingly. Signed-off-by: Karthikeyan Ramasubramanian <kramasub@google.com> Change-Id: I99ada7ead3a75befb0f934af871eecc060adcb26 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62387 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Reka Norman <rekanorman@chromium.org>
2022-03-08util/ifdtool: Add support for Denverton SoCJeff Daly
Denverton is a special version of IFD2 flash layout. It defines 10GbE firmware regions (11/12) and the IE (10) region which other IFD2 platforms do not have. Denverton does not include the legacy GbE region (3) or the EC region (8) which other IFD2 platforms do have. TEST='ifdtool -p dnv coreboot.rom' and verify correct output Signed-off-by: Jeff Daly <jeffd@silicom-usa.com> Change-Id: I15939ce4672123f39a807d63c13ba7df98c57523 Reviewed-on: https://review.coreboot.org/c/coreboot/+/60830 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2022-03-02util/docker/coreboot-jenkins-node: Alphabetize installed toolsMartin Roth
It's easier to read and to add new packages when each package is on its own line and they're sorted alphabetically. Indenting them also makes it easier to see what's getting installed and what's a command. Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: Ibfe297bd408ed0783fcff09c1ecb5672fe785c48 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62446 Reviewed-by: Felix Singer <felixsinger@posteo.net> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-03-02util/docker/coreboot-jenkins-node: add linkcheckerMartin Roth
The linkchecker tool is now being used to find broken links in our websites. Since it's not needed for building anything, just add it to the jenkins-node Dockerfile instead. Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: Iac2246b5378e556b5cd9f2107fc5a7e51d583b5b Reviewed-on: https://review.coreboot.org/c/coreboot/+/62445 Reviewed-by: Felix Singer <felixsinger@posteo.net> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-27utils/cbfstool: Fix building with `make test-tools`Felix Singer
The variable `RM` is empty and thus set it to `rm`. While executing the `clean` rule, run each `rm` command with the -f flag to ignore non-existing files. Also, disable the objutil feature locally fixing another build issue. Change-Id: Icb17e2c924ef480f8ac6195f96cf495709a0a023 Signed-off-by: Felix Singer <felixsinger@posteo.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62415 Reviewed-by: Nico Huber <nico.h@gmx.de> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-27util/testing: Add cbfstool tools to tested utilsMartin Roth
Previously, cbfstool was tested as part of the coreboot build, but not tested individually. This let a change that broke elogtool slip through. Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: I9e7b7a01d4a77ffdac932ba5af12cbd1ba96628b Reviewed-on: https://review.coreboot.org/c/coreboot/+/62406 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Singer <felixsinger@posteo.net> Reviewed-by: Elyes Haouas <ehaouas@noos.fr>
2022-02-26Revert "util/cbfstool: Port elogtool to libflashrom"Martin Roth
This reverts commit d74b8d9c990780ba64515b36aaff79d719d71ead. This change breaks the 'make all' build of the cbfstool tools from the util/cbfstool directory unless libflashrom-dev is installed, complaining that flashrom is not installed. Even with libflashrom-dev installed, it breaks building elogtool with the public version of libflashrom-dev. Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: I572daa0c0f3998e20a8ed76df21228fdbb384baf Reviewed-on: https://review.coreboot.org/c/coreboot/+/62404 Reviewed-by: Nico Huber <nico.h@gmx.de> Reviewed-by: ron minnich <rminnich@gmail.com> Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org> Reviewed-by: Felix Singer <felixsinger@posteo.net> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-25crossgcc: Upgrade LLVM/clang from 12.0.0 to 13.0.1Elyes HAOUAS
Build/run not tested on board. Change-Id: I8c550d3528a5b1c891b318c08ecfba3a9255e69c Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59400 Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-24amdfwtool: Check the real length of PMU stringZheng Bao
The length should be checked before the PMU_STR_INS_INDEX(th) character is accessed, otherwise it is going to an access violation. Change-Id: I8b59eb34e1cb01fd6e2571fcebc28ef2084b6ec4 Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62249 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-02-22util/nixshell: Add a Nix shell for building documentationFelix Singer
Add a Nix shell config allowing to build the coreboot documentation. Change-Id: I1c9715c677342241b78fbdef0afeb4536f48d50f Signed-off-by: Felix Singer <felixsinger@posteo.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62203 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
2022-02-21amdfwtool: Add entries for PMUI & PMUD with instance 2Zheng Bao
Change-Id: I69c4b3cdd2473655064d1329d5319cffdba2425a Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62067 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Reviewed-by: Nikolai Vyssotski <nikolai.vyssotski@amd.corp-partner.google.com>
2022-02-21amdfwtool: Add support for AMD's BIOS A/B recovery featureZheng Bao
The rom layout for A/B recovery: EFS -> PSP L1 0x48 -> PSP L2 A -> BIOS L2 A 0x4A -> PSP L2 B -> BIOS L2 B The coreboot doesn't implement the AMD's A/B recovery. This is only for the ROM layout. To save some flash space, the entire B section can be eliminated. To enable A/B recovery in PSP layout, add "--recovery-ab" to amdfwtool. TEST=Majolica(Cezanne) Change-Id: I27f5d3476f648fcecafb8d258ccb6cfad4f50036 Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56773 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-02-21soc/amd/*/fw.cfg: Remove the misleading name for PMUI and PMUDZheng Bao
Add the information of substance and instance in the string for PMUI and PMUD. It is amdfwtool's job to extract the number from the string. Change-Id: I43235fefcbff5f730efaf0a8e70b906e62cee42e Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62066 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-02-21util/liveiso: Use programs.flashrom.enableFelix Singer
NixOS 21.11 introduced the option `programs.flashrom.enable`. The option allows installing flashrom and hooking up its udev rules. Thus, set it to `true` and add the user `user` to the `flashrom` group allowing it to use the programmers. Change-Id: I017ddb4314702a5252dfc0d05cd1e4961043d23b Signed-off-by: Felix Singer <felixsinger@posteo.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62193 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
2022-02-21crossgcc: Upgrade CMake to 3.22.2 versionElyes HAOUAS
Change-Id: I4272f72dd6ed686dbad5615a0ab44c8c632b5930 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59399 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2022-02-17util/spd_tools/spd_gen/lp5: Encode Bank ArchitectureKarthikeyan Ramasubramanian
ADL supports 8B Bank Architecture, whereas Sabrina supports either BG or 16B Bank Architectures depending on the speed. This influences SDRAM Density and Banks, SDRAM Addressing bytes in SPD. Encode them as per the individual SoC advisories. BUG=b:211510456 TEST=Generate SPDs for Sabrina. Signed-off-by: Karthikeyan Ramasubramanian <kramasub@google.com> Change-Id: Ic854ccccb2b301e75d0f28cd36daf87fd41e07e7 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61948 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Nick Vaccaro <nvaccaro@google.com>
2022-02-17util/spd_tools/spd_gen/lp5: Encode Optional SDRAM featuresKarthikeyan Ramasubramanian
ADL and Sabrina provide different advisories to encode Optional SDRAM features (byte indices 7 & 9). Encode those bytes as per the respective advisories. BUG=b:211510456 TEST=Generate the SPD binaries for Sabrina. Signed-off-by: Karthikeyan Ramasubramanian <kramasub@google.com> Change-Id: Icac8ae148458162768a919d9690d7bf96734e6c0 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61730 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Reka Norman <rekanorman@chromium.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org> Reviewed-by: Nick Vaccaro <nvaccaro@google.com>
2022-02-15util/chromeos/crosfirmware: format with shfmtMatt DeVillier
Clean up formatting using shfmt Change-Id: I46ce84668bfb4ea3df179317e2848b6bb75d8d5c Signed-off-by: Matt DeVillier <matt.devillier@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/61617 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Sean Rhodes <sean@starlabs.systems>
2022-02-15util/amdtools/README,description.md: add update_efs_spi_speed docsFelix Held
This change is mostly from CB:56644 patchset 3. Signed-off-by: Martin Roth <martin@coreboot.org> Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Idece950bab260a099c9790485805cbe8ea641666 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61895 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-15util/amdtools/description.md: add description for the different toolsFelix Held
This change is mostly from CB:56644 patchset 3. Signed-off-by: Martin Roth <martin@coreboot.org> Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I4cb9bbb3d7fd5d7c9e33fbf656301c0beb2f1b47 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61894 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@mailbox.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-15util/amdtools/README: convert to markdownFelix Held
This change is mostly from CB:56644 patchset 3. Signed-off-by: Martin Roth <martin@coreboot.org> Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Idcee9de9bc409a4dfe7d2f8c18ec5132f2747c33 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61893 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@mailbox.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-15util/inteltool: Add support for Tiger Lake chips detection and GPIOsMichał Żygowski
Add PCI IDs for Tiger Lake LP and Tiger Lake H devices and their GPIO tables. TEST: dump GPIOs on i5-1135G7, Tiger Lake H untested Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com> Change-Id: I6071a999be9e8a372997db0369218f297e579d08 Reviewed-on: https://review.coreboot.org/c/coreboot/+/56171 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
2022-02-12amdfwtool: Add options to support mainboard specific SPL tableZheng Bao
For the generic board which uses Cezanne, we use the generic SPL table. For the Guybrush Chromebook, we need to use a customized SPL file. BUG=b:216096562 Change-Id: I385b0fe13cb78a053c07127ec3ea1c61dc42c7e4 Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/61836 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-12util/inteltool: Actually read SATA init data from SIRDBenjamin Doron
Fix issue where registers always seem to contain their own offset. After writing the desired register into SIRI, the requested data is returned in SIRD. This register is 4 bytes after SIRI, commonly 0xA4. Tested on TGL-H (SATA SIR registers are common), genuine data is returned. Change-Id: I322b11d53178e5b64e353c1b4e576548592c16c3 Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/61737 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
2022-02-11util/chromeos/crosfirmware: Handle "broken" recovery imagesMatt DeVillier
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>
2022-02-11util/chromeos/crosfirmware: Fix handling of newer boardsMatt DeVillier
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>
2022-02-11util/cbmem: Add --loglevel option to restrict console printing by levelJulius Werner
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>
2022-02-09util/ifdtool: add generic `PLATFORM_IFD2` for early SoC developmentWonkyu Kim
`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>
2022-02-08util/spd_tools/spd_gen/lp5: Update BusWidth EncodingKarthikeyan Ramasubramanian
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>
2022-02-07console: Add loglevel marker codes to stored consolesJulius Werner
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>
2022-02-07amdfwtool: Add SPL supportZheng Bao
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>
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>