summaryrefslogtreecommitdiff
path: root/src/vendorcode/amd/pi
AgeCommit message (Collapse)Author
2023-01-17treewide: Fix old-style declarationsElyes Haouas
Replace old style declaration "const static" with "static const". This to enable "Wold-style-declaration" command option. Change-Id: I757632befed1854f422daaf4dfea58281b16e2f5 Signed-off-by: Elyes Haouas <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/71841 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Eric Lai <eric_lai@quanta.corp-partner.google.com>
2023-01-03vc/amd/pi/00670F00/Makefile.inc: Remove path to non-existent directoryElyes Haouas
Fix: CC romstage/mainboard/amd/pademelon/static.o cc1: error: src/vendorcode/amd/pi/00670F00/Lib: No such file or directory [-Werror=missing-include-dirs] CC romstage/mainboard/amd/gardenia/static.o cc1: error: src/vendorcode/amd/pi/00670F00/Lib: No such file or directory [-Werror=missing-include-dirs] CC romstage/mainboard/google/kahlee/static.o cc1: error: src/vendorcode/amd/pi/00670F00/Lib: No such file or directory [-Werror=missing-include-dirs] CC romstage/mainboard/google/kahlee/static.o cc1: error: src/vendorcode/amd/pi/00670F00/Lib: No such file or directory [-Werror=missing-include-dirs] Change-Id: I038f87f564ed0415035d92bf0d79a9f8ae2227a4 Signed-off-by: Elyes Haouas <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/71597 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin L Roth <gaumless@gmail.com>
2022-11-07cpu/amd/agesa: Remove leftover codeArthur Heymans
Now that all agesa CPUs are removed this code is unused. Change-Id: If0c082bbdb09457e3876962fa75725add11cb67c Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/69118 Reviewed-by: Elyes Haouas <ehaouas@noos.fr> Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2022-05-11amd/*/gcccar.inc: Replace local declarationsArthur Heymans
Although useful to declare local symbols inside macros clang does not support them. Using the \@ symbol which increments each time the macro is used we can do the same. With BUILD_TIMELESS=1 the binaries don't change and do build with GCC so nothing is lost here. Change-Id: I01054e2bdcb63810b21eb51b46bdc6e1bd999516 Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/63045 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2022-03-25vendorcode/amd/pi: Fix building with clangArthur Heymans
Change-Id: I82913de07acc13af2f5f2c67853e112fb3c66319 Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/63048 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2021-05-26vc/amd/pi/00630F01: Remove unused directory and codeMichał Żygowski
No board currently uses AMD PI 00630F01 so remove it. Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com> Change-Id: I3f990e44e0f769219a6f80cf1369f6a3c94b3509 Reviewed-on: https://review.coreboot.org/c/coreboot/+/53994 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Nico Huber <nico.h@gmx.de> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-05-12vc/amd/pi/00660F01: Remove unused code and directoryMichał Żygowski
This is some leftover omitted during 00660F01 removal, since corresponding CPU and northbridge code is not present in the tree already. Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com> Change-Id: Ib7ccbc088766b5a4f59c47bd48790c6a2af8ca61 Reviewed-on: https://review.coreboot.org/c/coreboot/+/54056 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2020-12-28sb/amd/pi/hudson: Enable use of common GPIO APIKyösti Mälkki
The code in soc/amd/common has an implementation of GPIO register space that is compatible with the hardware sb/amd/pi/hudson supports. Change-Id: I86ae40a3cdf335263d7e9e3dcfdd588947cdd9b1 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/42733 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2020-12-05amd_blobs: Always set default pathsNico Huber
Don't make the default paths to AMD blobs depend on USE_AMD_BLOBS. This way we get error messages about the missing files when the blobs repos aren't checked out. Change-Id: I754fdc5e1414c8a3dc88b364bcfbea9a26b59eb0 Signed-off-by: Nico Huber <nico.h@gmx.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/38044 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2020-11-22cpu/amd/pi: Remove unused cpu code 00660F01Martin Roth
Remove the processor directory and references to the Kconfig symbol. Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: I403a453362fd76d6ef2a5b75728a362efa4f2491 Reviewed-on: https://review.coreboot.org/c/coreboot/+/47652 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-11-22vc/amd/pi/00670F00: Add raw AGESA binary only to COREBOOT CBFSFurquan Shaikh
If AGESA is added as a raw binary (and not a stage), then cbfstool does not perform relocation. In this case, it should be added only to COREBOOT (i.e. default) CBFS since the binary needs to be present only in one specific location that is present in the default CBFS. Change-Id: I7a7edc217663f9d1d36b05308bbd35f56a28b9b1 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/47832 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2020-10-17vendorcode/amd: Fix typo in *is defined* in commentsPaul Menzel
The passive clause is constructed with the past participle, which is *defined* in this case. Fix all occurrences in AMD vendor code with the command below. $ git grep -l "is define at" src/vendorcode/amd/ | xargs sed -i 's/is define at/is defined at/' Change-Id: Ia26c87aecb484dcb55737e417367757d38ce3b56 Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46065 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-09-01{include,mb,soc,sb,vendorcode}: Make hexadecimal notation consistentSubrata Banik
Convert 0X -> 0x Signed-off-by: Subrata Banik <subrata.banik@intel.com> Change-Id: Iea3ca67908135d0e85083a05bad2ea176ca34095 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44926 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-07-10vc/amd/pi/00660F01: Drop dead codeAngel Pons
This code is not even being build-tested. Drop it before it grows moss. Change-Id: I14c575ac20cd94af1cfbb1204e2923149ef2920d Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43259 Reviewed-by: Michael Niewöhner Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-06-19Kconfig: Escape variable to accommodate new Kconfig versionsPatrick Georgi
Kconfig 4.17 started using the $(..) syntax for environment variable expansion while we want to keep expansion to the build system. Older Kconfig versions (like ours) simply drop the escapes, not changing the behavior. While we could let Kconfig expand some of the variables, that only splits the handling in two places, making debugging harder and potentially messing with reproducible builds (e.g. when paths end up in configs), so escape them all. Change-Id: Ibc4087fdd76089352bd8dd0edb1351ec79ea4faa Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/42481 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Werner Zeh <werner.zeh@siemens.com> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Frans Hendriks <fhendriks@eltan.com> Reviewed-by: Wim Vervoorn <wvervoorn@eltan.com>
2020-05-11treewide: Remove "this file is part of" linesPatrick Georgi
Stefan thinks they don't add value. Command used: sed -i -e '/file is part of /d' $(git grep "file is part of " |egrep ":( */\*.*\*/\$|#|;#|-- | *\* )" | cut -d: -f1 |grep -v crossgcc |grep -v gcov | grep -v /elf.h |grep -v nvramtool) The exceptions are for: - crossgcc (patch file) - gcov (imported from gcc) - elf.h (imported from GNU's libc) - nvramtool (more complicated header) The removed lines are: - fmt.Fprintln(f, "/* This file is part of the coreboot project. */") -# This file is part of a set of unofficial pre-commit hooks available -/* This file is part of coreboot */ -# This file is part of msrtool. -/* This file is part of msrtool. */ - * This file is part of ncurses, designed to be appended after curses.h.in -/* This file is part of pgtblgen. */ - * This file is part of the coreboot project. - /* This file is part of the coreboot project. */ -# This file is part of the coreboot project. -# This file is part of the coreboot project. -## This file is part of the coreboot project. --- This file is part of the coreboot project. -/* This file is part of the coreboot project */ -/* This file is part of the coreboot project. */ -;## This file is part of the coreboot project. -# This file is part of the coreboot project. It originated in the - * This file is part of the coreinfo project. -## This file is part of the coreinfo project. - * This file is part of the depthcharge project. -/* This file is part of the depthcharge project. */ -/* This file is part of the ectool project. */ - * This file is part of the GNU C Library. - * This file is part of the libpayload project. -## This file is part of the libpayload project. -/* This file is part of the Linux kernel. */ -## This file is part of the superiotool project. -/* This file is part of the superiotool project */ -/* This file is part of uio_usbdebug */ Change-Id: I82d872b3b337388c93d5f5bf704e9ee9e53ab3a9 Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41194 Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-04-04src/vendorcode: Use SPDX for GPL-2.0-only filesAngel Pons
Done with sed and God Lines. Only done for C-like code for now. Change-Id: I49dc615178aaef278d6445376842d45152759234 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/40060 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Frans Hendriks <fhendriks@eltan.com>
2020-03-17src (minus soc and mainboard): Remove copyright noticesPatrick Georgi
They're listed in AUTHORS and often incorrect anyway, for example: - What's a "Copyright $year-present"? - Which incarnation of Google (Inc, LLC, ...) is the current copyright holder? - People sometimes have their editor auto-add themselves to files even though they only deleted stuff - Or they let the editor automatically update the copyright year, because why not? - Who is the copyright holder "The coreboot project Authors"? - Or "Generated Code"? Sidestep all these issues by simply not putting these notices in individual files, let's list all copyright holders in AUTHORS instead and use the git history to deal with the rest. Change-Id: I89b10076e0f4a4b3acd59160fb7abe349b228321 Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/39611 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: David Hendricks <david.hendricks@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-02-06mb/pcengines/apu2: use AGESA 1.0.0.4 with adjusted AGESA headerPiotr Kleinschmidt
PC Engines apu2 platform uses AGESA 1.0.0.4, because upstream AGESA 1.0.0.A doesn't work on apu2 - the platform doesn't boot. To properly utilize AGESA 1.0.0.4 we need to adjust AGESA header to state, which is compatible with AGESA 1.0.0.4 version. Cut out the changes introduced in CB:11225 exclusively for apu2 board. TEST=boot PC Engines apu2 and launch Debian Linux Change-Id: I3d85ee14e35dae8079e8d552b6530a3867f65876 Signed-off-by: Piotr Kleinschmidt <piotr.kleins@gmail.com> Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/35906 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Nico Huber <nico.h@gmx.de>
2020-01-28commonlib: Add commonlib/bsdJulius Werner
This patch creates a new commonlib/bsd subdirectory with a similar purpose to the existing commonlib, with the difference that all files under this subdirectory shall be licensed under the BSD-3-Clause license (or compatible permissive license). The goal is to allow more code to be shared with libpayload in the future. Initially, I'm going to move a few files there that have already been BSD-licensed in the existing commonlib. I am also exracting most contents of the often-needed <commonlib/helpers.h> as long as they have either been written by me (and are hereby relicensed) or have an existing equivalent in BSD-licensed libpayload code. I am also relicensing <commonlib/compression.h> (written by me) and <commonlib/compiler.h> (same stuff exists in libpayload). Finally, I am extracting the cb_err error code definitions from <types.h> into a new BSD-licensed header so that future commonlib/bsd code can build upon a common set of error values. I am making the assumption here that the enum constants and the half-sentence fragments of documentation next to them by themselves do not meet the threshold of copyrightability. Change-Id: I316cea70930f131e8e93d4218542ddb5ae4b63a2 Signed-off-by: Julius Werner <jwerner@chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/38420 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Rudolph <siro@das-labor.org>
2020-01-05vc/amd/pi/00670F00: Fix typo in phony target declarationMarshall Dawson
Correct a copy/paste error for warn_no_agesa. Change-Id: Ife2cca47f1f816f99395b33976d08826c53e3c3e Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/38145 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Nico Huber <nico.h@gmx.de>
2019-12-24vendorcode/amd/pi/Kconfig: Add prompt to pre/post pi filesRaul E Rangel
This allows the values to be set in a .config BUG=none TEST=Was able to set the value from a .config and built careena firmware Change-Id: I757e4b9a0b80ff42c1f49143a44f15550366fd0b Signed-off-by: Raul E Rangel <rrangel@chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37879 Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-by: Michał Żygowski <michal.zygowski@3mdeb.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-12-12vc/amd/pi: Fix typoPatrick Georgi
Change-Id: Ic3d1b9f90c6ed3d85ff209f433de9ab939d760a6 Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37676 Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-12-11soc/amd/stoneyridge|mbs: Deprecate SOC_AMD_NAME_PKG and othersMarshall Dawson
Add package and APU selections to mainboards and remove symbols no longer used in soc//stoneyridge. Change-Id: I60214b6557bef50358f9ec8f9fcdb7265e04663b Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37225 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
2019-12-11soc/amd/stoneyridge|mbs: Define SOC_AMD_STONEYRIDGE symbolMarshall Dawson
Make a new Kconfig symbol for using soc//stoneyridge. This code also supports Prairie Falcon is backward-compatible with Carrizo and Merlin Falcon. Although Bettong uses Carrizo, it does not currently rely on stoneyridge source, so it is unaffected by this change. Change-Id: I786ca54b0444cbcf36dc428a193006797b01fc09 Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37224 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
2019-12-11soc/amd/stoneyridge|mb: Add Kconfig symbol for Prairie FalconMarshall Dawson
The stoneyridge code inferred that if Merlin Falcon was built but no Merlin Falcon binaries were present, the intent must be Prairie Falcon. The two falcons are Embedded variants, and Prairie Falcon falls within Family 15h Models 70h-7Fh. Add a Prairie Falcon symbol that can be used explicitely. Drop HAVE_MERLINFALCON_BINARIES. Change-Id: I0d3a1bc302760c18c8fe3d57c955e2bb3bd8153a Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37223 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
2019-12-11soc/amd/stoneyridge|vc: Change default locations for blobsMarshall Dawson
Set the default location strings to point to the 3rdparty/amd_blobs files. Change-Id: I5426b8de2501ba55843efc1cda4b03bc3768f8cb Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37222 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
2019-12-11vc/amd/pi: Allow 00670F00 to build with no binaryPIMarshall Dawson
Make the default binaryPI image strings for all stoneyridge-based APUs depend on USE_AMD_BLOBS. Ensure the build completes without names, and without images. Change-Id: I74a38efa2a4ad2f9f12a1f8e7fb8694d0ab9dd1e Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37228 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
2019-12-04binaryPI: Fix failing AP startupKyösti Mälkki
Fix regression with commit 5639736 binaryPI: Drop CAR teardown without POSTCAR_STAGE Occassionally (maybe 1 boot in 10) SMP lapic_cpu_init() fails with following errors in the logs of pcengines/apu2: CPU 0x03 would not start! CPU 0x03 did not initialize! The CPU number is sometimes 0x02, never seen 0x01. Work-around also suggests something to do with cache coherency and MTRRs that is really at fault. As a work-around return the BSP CAR teardown to use wbinvd instead of invd. These platforms do not support S3 resume so this is the easy work-around for the time being. Change-Id: I3dac8785aaf4af5c7c105ec9dd0b95156b7cca21 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37438 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Michał Żygowski <michal.zygowski@3mdeb.com> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2019-11-27soc/amd/stoneyridge: Add selectable packagesMarshall Dawson
The StoneyPI package supports Family 15h Models 60h-6Fh and 70h-7Fh in FT4 and FP4 packages. Add options for the packages. The existing convention of SOC_AMD_PRODUCTNAME_PKG will be phased out. Change-Id: I60232ca099b813640742868db08aa66b32265f3b Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37218 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
2019-11-27binaryPI: Drop CAR teardown without POSTCAR_STAGEKyösti Mälkki
The remaining (active) binaryPI boards moved away from BINARYPI_LEGACY_WRAPPER and have POSTCAR_STAGE now. As the cache_as_ram.S is also used with AGESA, this slightly reduces the codesize there for romstage and postcar as well. This commit is actually a revert for the vendorcode parts, AMD originally shipped the codes using 'invd' for the CAR teardown, but these were changed for coreboot due the convoluted teardown that used to happen with non-empty stack. Change-Id: I693c104c3aab3be537c00695cbd764a48bd603b0 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/18526 Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-11-27binaryPI: Drop BINARYPI_LEGACY_WRAPPER supportKyösti Mälkki
Drop all the sources that were guarded with this. Change-Id: I6c6fd19875cb57f0caf42a1a94f59efed83bfe0d Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/19275 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Michał Żygowski <michal.zygowski@3mdeb.com>
2019-11-25binaryPI: Remove FieldAccessors.[ch]Kyösti Mälkki
SAGE brought these in outside AGESA specifications and they had some ill semantics. They were already removed from StoneyRidge. Change-Id: I59d0c450583b2ff58031c127aae881d1f3799338 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37174 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Michał Żygowski <michal.zygowski@3mdeb.com> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2019-11-23vendorcode/amd/pi/Makefile.inc: remove -fno-zero-initialized-in-bssKrystian Hebel
This fixes issue that became visible after implementing post-CAR stage on top of `340e4b80904f lib/cbmem_top: Add a common cbmem_top implementation`. Compilation error was: Forbidden global variables in romstage: ffffff00 d top.2205 Signed-off-by: Krystian Hebel <krystian.hebel@3mdeb.com> Change-Id: I088ac824f9b66387843ae5810fd2c75a8b16d9db Reviewed-on: https://review.coreboot.org/c/coreboot/+/36976 Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-07-18vendorcode/amd/pi: Integrate Merlin Falcon as a build optionRichard Spiegel
Add changes needed to build a project using Merlin Falcon SOC using 00670F00 vendor code, which is backward compatible with Merlin Falcon. Only the AGESA binary image is different then the one used by 00670F00. BUG=none. TEST=Tested later with padmelon board. Change-Id: Id3341f6a1ef2561a6391d3db8c54f6bdd09b0c0e Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/33622 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2019-05-30src/vendorcode/amd/pi: Fix CONFIG() check issue in rules.hSubrata Banik
This patch fixes problem of adding CONFIG() check inside rules.h. Change-Id: Ifb6842d0efef3521642c5c399fdf2876f71b167a Signed-off-by: Subrata Banik <subrata.banik@intel.com> Signed-off-by: Ronald G. Minnich <rminnich@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/33105 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com> Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
2019-05-25AGESA: Move heap_status_name() implementationKyösti Mälkki
Place it within class libagesa to avoid including AGESA internal header heapManager.h in coreboot proper build CPPFLAGS. Change-Id: Iae86d6631d7a6ba6ea2588a53b292b435dfd7861 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/31511 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com>
2019-05-23AGESA binaryPI: Sync STRUCT_NAME definitionsKyösti Mälkki
While not implemented, copying the definitions from later AGESA/AMD.h to older helps us avoid lots of preprocessor directives. Change-Id: I34edc1ca23e9c063c4286273c53249ff0a953798 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/31510 Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-05-21binaryPI/00670F000: Remove AGESA.c fileKyösti Mälkki
Change-Id: Id48de8b2f6feb6c29d745140c872215faa32eb37 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/31487 Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-05-21soc/amd/common: Refactor AmdCreateStruct() useKyösti Mälkki
AmdCreateStruct() and AmdReleaseStruct() are equally bad when it comes to lack of correct function declarations for definitions found in vendorcode binaryPI/AGESA.c. Replace these with calls that go through the common module_dispatch() functions. Change-Id: I611bcbe2a71fb65c8eb759a9dc74cbd9cb74136e Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/31486 Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-05-06soc/amd/common: Introduce module_dispatch()Kyösti Mälkki
This change removes all the separate entrypoint dispatch functions as they all share the same pattern. Furthermore, none of the function definitions under vendorcode binaryPI/AGESA.c file have proper declarations, the ones compiler picks up from AGESA.h are for the internal implementations and with sanely organized headerfiles would not be exposed outside the build of AGESA at all. Change-Id: I0b72badc007565740c93b58743cfd048e8b42775 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/31485 Reviewed-by: Nico Huber <nico.h@gmx.de> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-04-23soc/amd/common: Remove AmdReadEventLog()Kyösti Mälkki
Parameter passing is incorrect here, it should pass complete StdHeader instead of attempting to fill in HeapStatus that should be treated as a field private to AGESA, based on where it is defined in the header files. Furthermore the while() loop did not evaluate the return value. Feature can be brought back at a later date after someone verifies it actually works correctly across different stages. Change-Id: Ib243b275f8700ecaeb330772c795d305c61899c5 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/31484 Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2019-03-08coreboot: Replace all IS_ENABLED(CONFIG_XXX) with CONFIG(XXX)Julius Werner
This patch is a raw application of find src/ -type f | xargs sed -i -e 's/IS_ENABLED\s*(CONFIG_/CONFIG(/g' Change-Id: I6262d6d5c23cabe23c242b4f38d446b74fe16b88 Signed-off-by: Julius Werner <jwerner@chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/31774 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com>
2019-02-14binaryPI: Fix cache coherency use for AP CPUsKyösti Mälkki
The memory between _car_region_start .. _car_region_end has to be set up as WB in MTRRs for all the cores executing through bootblock, verstage and romstage. Otherwise global variables may fail on AP CPUs. Fixes combination of CBMEM_CONSOLE=y with SQUELCH_EARLY_SMP=n, which previously did not boot at all for some cases. Change-Id: I4abcec90c03046e32dafcf97d2f7228ca93c5549 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-on: https://review.coreboot.org/c/26115 Reviewed-by: Michał Żygowski <michal.zygowski@3mdeb.com> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2018-10-08Move compiler.h to commonlibNico Huber
Its spreading copies got out of sync. And as it is not a standard header but used in commonlib code, it belongs into commonlib. While we are at it, always include it via GCC's `-include` switch. Some Windows and BSD quirk handling went into the util copies. We always guard from redefinitions now to prevent further issues. Change-Id: I850414e6db1d799dce71ff2dc044e6a000ad2552 Signed-off-by: Nico Huber <nico.h@gmx.de> Reviewed-on: https://review.coreboot.org/28927 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2018-09-28vendorcode/amd/pi/00670F00/Lib: Remove folderRichard Spiegel
Now that the last dependency was resolved, remove AmdLib folder. BUG=b:112525011 TEST=Build and boot grunt. Change-Id: Ibd9a20bc358742520138b9b01f76d7fd2fac92ab Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28742 Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-by: Charles Marslett <charles.marslett@amd.corp-partner.google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2018-09-26vendorcode/amd/pi/00670F00/Proc/PspBaseLib: Remove folderRichard Spiegel
Now that PspBaseLib is no longer used, fully remove the folder. BUG=b:116579642 TEST=Build grunt Change-Id: I441b3f46e2312c12771766f87b25d1dc15ff3af0 Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28733 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2018-09-24amd/common/psp: Remove use of PspBaseLibCharles Marslett
Eliminate the references to PspBaseLib.c and PspBaseLib.h in agesa_headers.h. Fix psp.c references to definitions in those files by adding them to include/amdblocks/psp.h. BUG=b:78514564 TEST=Build and boot grunt/ChromeOS and restore an image from the internet. Change-Id: I2740ceb945736c6e413f7d0bd0c41a19e19c7d5a Signed-off-by: Charles Marslett <charles.marslett@amd.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/27619 Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2018-08-28vendorcode/amd/pi/00670F00: Remove IDS headersRichard Spiegel
Only Ids.h had definitions still in use, and they were removed or moved to AGESA.h. Now Ids.h, IdsPerf.h and IdsLib.h can be safely removed. BUG=b:112885948 TEST=Build grunt Change-Id: I031ae8eb5f34fee801365fc89ea11a881211e726 Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28299 Reviewed-by: Raul Rangel <rrangel@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2018-08-28vendorcode/amd/pi/00670F00: Transfer TP_Perf_STRUCT to AGESA.hRichard Spiegel
Google is creating code to measure AGESA performance, which needs structure TP_Perf_STRUCT and associated definitions. In preparation to remove IDS headers, move the necessary definitions to AGESA.h. BUG=b:112885948 TEST=Build grunt Change-Id: I941a67a8889a9dbf35c9fd511c7f670623204134 Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28369 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-by: Martin Roth <martinroth@google.com>
2018-08-27vendorcode/amd/pi/00670F00: Transfer IDS_CALLOUT to AGESA.hRichard Spiegel
Currently, IDS_CALLOUT macros are only used in stoneyridge callout. In preparation to remove IDS headers, move the definitions to AGESA.h. BUG=b:112885948 TEST=Build grunt Change-Id: Ia9717eb68fed2e568eaf169157c2837bb8232b7e Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28297 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2018-08-27vendorcode/amd/pi/00670F00/Include/Ids.h: Remove IDS_ERROR_TRAPRichard Spiegel
The macro IDS_ERROR_TRAP is only defined, and never used. Also, IDSOPT_ERROR_TRAP_ENABLED is defined FALSE, so the macro would translate to nothing. Remove the macro and IDSOPT_ERROR_TRAP_ENABLED. BUG=b:112885948 TEST=Build grunt Change-Id: I2c3ca4b0a4a1f96f245ba2f4902fd0051dda77ef Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28296 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2018-08-27vendorcode/amd/pi/00670F00/Lib/AmdLib.c: Remove IdsErrorStopRichard Spiegel
Function IdsErrorStop() is only used within AmdLib.c function LibAmdMsrRead(), which in turn is only used once within PspBaseLib.c and three times inside AmdLib.c, all with well defined MSR addresses. IdsErrorStop() is used as a trap if MSR address is 0 or 0xFFFFFFFF, which clearly it's not. Therefore it can be safely removed from AmdLib.c. BUG=b:112885948 TEST=Build grunt Change-Id: I47ffcbd4fbae28b6d711a340f0ac3f3b007e8e4f Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28295 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2018-08-20vendorcode/amd/pi/00670F00/Lib: Remove unused functionsRichard Spiegel
The only code still used are LibAmdPciRead() and LibAmdPciWrite(). These functions are used by PspBaseLib. Remove all functions that are not used, directly or indirectly, by LibAmdPciRead() and LibAmdPciWrite(). BUG=b:112688270 TEST=Build grunt Change-Id: Iba5cfbeee8e83ca78279a1bc2a333370c04f55ed Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28194 Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Martin Roth <martinroth@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2018-08-17src/vendorcode/amd/pi/00670F00/Proc/Fch/Common: Remove unused headersRichard Spiegel
Header files AcpiLib.h, FchDef.h and FchBiosRamUsage.h became obsolete when VENDORCODE_FULL_SUPPORT was removed. Therefor they should be removed. BUG=b:112602580 TEST=Build grunt and gardenia. Change-Id: If4fdb9ae1e106ba15f2a073f592499e638e40c65 Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28093 Reviewed-by: Martin Roth <martinroth@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2018-08-15Stoneyridge: Remove VENDORCODE_FULL_SUPPORTRichard Spiegel
Remove VENDORCODE_FULL_SUPPORT from /soc/amd/stoneyridge/Kconfig and from vendorcode/amd/pi/00670F00/Makefile.inc, thus completing the removal of VENDORCODE_FULL_SUPPORT from coreboot. BUG=b:112578491 TEST=none, VENDORCODE_FULL_SUPPORT already not used. Change-Id: Idb5f6dc7add1617f7a97a97ae110901b2dec0996 Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28092 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2018-08-15vendorcode/amd/pi/00670F00/Lib/AmdLib.c: Remove VENDORCODE_FULL_SUPPORTRichard Spiegel
Remove VENDORCODE_FULL_SUPPORT from file above mentioned file, in preparation to full removal of VENDORCODE_FULL_SUPPORT functions. BUG=b:112578491 TEST=none, VENDORCODE_FULL_SUPPORT already not used. Change-Id: Ic23dcf245b2cee24f7363ca3bb9918eb2f11179c Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28091 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2018-08-15vendorcode/amd/pi/00670F00/binaryPI/AGESA.c: Remove VENDORCODE_FULL_SUPPORTRichard Spiegel
Remove VENDORCODE_FULL_SUPPORT from file above mentioned file, in preparation to full removal of VENDORCODE_FULL_SUPPORT functions. BUG=b:112578491 TEST=none, VENDORCODE_FULL_SUPPORT already not used. Change-Id: Id91e76282509743070e34c02082d3f3f46a14059 Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28090 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2018-08-15vendorcode/amd/pi/00670F00/Proc/Psp: Remove VENDORCODE_FULL_SUPPORTRichard Spiegel
Remove VENDORCODE_FULL_SUPPORT from file: vendorcode/amd/pi/00670F00/Proc/Psp/PspBaseLib/PspBaseLib.c BUG=b:112578491 TEST=none, VENDORCODE_FULL_SUPPORT already not used. Change-Id: I0d590b175a3cf0426580dc9ee5164b3cedc838e2 Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28089 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2018-08-15vendorcode/amd/pi/00670F00/Proc/Fch/Common: Remove VENDORCODE_FULL_SUPPORTRichard Spiegel
Remove VENDORCODE_FULL_SUPPORT from files FchLib.c and FchPeLib.c. BUG=b:112578491 TEST=none, VENDORCODE_FULL_SUPPORT already not used. Change-Id: If24eb7f005720a62a1280fe78ddb54c9d2690150 Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28088 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2018-08-15vendorcode/amd/pi/00670F00/Lib: Remove read modify write functionsRichard Spiegel
Now that the functions that used them were safely removed, remove LibAmdIoRMW(), LibAmdMemRMW() and LibAmdPciRMW(). BUG=b:112541697 TEST=Build grunt and gardenia Change-Id: I570bd91cd9eba7798ea39d9685e214fee10824be Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28083 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2018-08-15vendorcode/amd/pi/00670F00: Remove functions that use LibAmdPciRMW()Richard Spiegel
The functions that use LibAmdPciRMW() are not used by coreboot and can be safely removed in preparation to remove LibAmdPciRMW() itself. The functions to be removed are: From vendorcode/amd/pi/00670F00/Proc/Fch/Common/FchPeLib.c: ProgramPciByteTable(). From vendorcode/amd/pi/00670F00/Proc/Fch/Common/FchLib.c: RwXhciIndReg(), RwXhci0IndReg() and RwXhci1IndReg(). From vendorcode/amd/pi/00670F00/Proc/Fch/Common/PciLib.c: RwPci(). BUG=b:112541697 TEST=Build grunt and gardenia Change-Id: I0b96d3d6b98140ed8e9298817dbe29d55b9e22cb Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28082 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2018-08-15vendorcode/amd/pi/00670F00: Remove functions that use LibAmdMemRMW()Richard Spiegel
The functions that use LibAmdMemRMW() are not used by coreboot and can be safely removed in preparation to remove LibAmdMemRMW() itself. The functions to be removed are: ProgramFchAcpiMmioTbl() and GetEfuseStatus(), both from vendorcode/amd/pi/00670F00/Proc/Fch/Common/FchPeLib.c. BUG=b:112541697 TEST=Build grunt and gardenia Change-Id: Ib935b1797c4bf8b504fdda6f676fca369169a7f1 Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/28081 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2018-07-31src/vendorcode/amd/pi/00670F00: Remove IMC supportRichard Spiegel
Per AMD, the Integrated Micro Controller is not a supported feature of the Stoney Ridge APU. Systems are expected to implement an external EC for desired features. Remove all stoney IMC files and functions from vendor code. BUG=b:111780177 TEST=Build grunt and gardenia Change-Id: I06e993fa498cc0978c1d037bc6001682407f7fac Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/27652 Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2018-06-13vc/amd/00670F00: Sync AGESA.h with PI blobMarshall Dawson
Add a new callout definition for AgesaGetTempHeapBase and displace AgesaHeapRebase (which was merged too soon) in the ordering. Also add its structure. AGESA will be modified to ask coreboot for the location for temporary storage of heap data at the end of InitPost. The old methodology is to use 0xb0000 but the change will allow coreboot to determine the location. BUG=b:74518368 Change-Id: I0bc894d7842cf4b3eb728a90704277b17f4bf7be Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-on: https://review.coreboot.org/26145 Reviewed-by: Martin Roth <martinroth@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2018-05-22stonyridge: Add TP_Perf_STRUCT structRaul E Rangel
The TP_Perf_STRUCT was missing from pi/00670F00. So I copied the file from src/vendorcode/amd/pi/00630F01/Include/IdsPerf.h and removed everything that we don't need. I did have to change MAX_PERFORMANCE_UNIT_NUM so it matches the size used by pi/00670F00. This struct is used to extract the timestamps from AGESA. BUG=b:64549506 TEST=built on grunt Change-Id: I06ec82348e3d10f2430c1192a925a49389ae4414 Signed-off-by: Raul E Rangel <rrangel@chromium.org> Reviewed-on: https://review.coreboot.org/26235 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2018-05-09vendorcode/amd/pi/00670F00: Control which procedure buildsRichard Spiegel
Vendor code is compiled as a library, thus the whole library is included into the final image. However, not all procedures are required, they are there because original AGESA code had them. We cannot remove them, in order to facilitate porting of fixed AGESA code. Therefor add #if throughout the code to allow the control if unneeded procedures will be build. BUG=b:78610011 TEST=Build and boot grunt; build kahlee and gardenia. Change-Id: I68f9e359b2331f715a3b85486c4181866985afdf Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/26135 Reviewed-by: Martin Roth <martinroth@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2018-05-09vendorcode/amd/pi/00670F00: Remove unneeded includesRichard Spiegel
Vendor code has several headers included into source code that are not needed in order to build them. Remove unneeded #include. This is part of controlling the build of unneeded procedures within vendor code. BUG=b:78610011 TEST=Build grunt. Change-Id: Id7d451b6be564632836fc64fd343131edb85183a Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/26134 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2018-05-06AGESA: Run ar with DTKyösti Mälkki
Create libagesa as a thin and deterministic archive file, this could reduce build time and used space. Change-Id: Icfd1f3fbf54f7e61ab528fa7686331182959c7d5 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/22068 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2018-04-25vendorcode/amd/pi/00670F00: Remove include headerGarrett Kirkendall
Remove Fch.h from being included in src/vendorcode/amd/pi/00670F00/agesa_headers.h. It is not needed. BUG=b:69220826 BRANCH=master TEST=build Gardenia and Grunt systems. Change-Id: Ifde58421d20c813ae5708b1d9c6ec76433051d33 Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/25808 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
2018-04-16vendorcode/amd/pi/00670F00: Remove unused headersGarrett Kirkendall
Remove unused header files in src/vendorcode/amd/pi/00670F00/agesa_headers.h. This is a first clean up. Hopefully more headers will be removed in other commits. Header files cannot be removed at this time. They are used by files in vendorcode/amd/pi/00670F00/. BUG=b:77944801 BRANCH=none TEST=build Gardenia and Grunt Change-Id: I99b77f6ba41ded30122a01bbe709681312561436 Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/25644 Reviewed-by: Martin Roth <martinroth@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2018-04-16vendorcode/amd/pi/00670F00: Remove unused headersGarrett Kirkendall
Remove unused AGESA header files from vendorcode/amd/pi/00670F00/binaryPI/AGESA.c BUG=b:77905293 BRANCH=none TEST=build Gardenia. Change-Id: Ic38424d489dcc37a4074159e33fca0d49c71f701 Signed-off-by: Garrett Kirkendall <garrett.kirkendall@amd.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/25626 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
2018-04-11Correct "MTTR" to "MTRR"Jonathan Neuschäfer
The term MTRR has been misspelled in a few places. Change-Id: I3e3c11f80de331fa45ae89779f2b8a74a0097c74 Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> Reviewed-on: https://review.coreboot.org/25568 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2018-04-06vc/amd/stoneyridge: Add definition for AGESA heap rebaseMarshall Dawson
AgesaHeapRebase is an optional callout that allows AGESA to use a coreboot-managed heap base address. Its internal default location is determined by AMD_HEAP_START_ADDRESS which is defined as 4 MB. Add a #define that AGESA may use once the feature is available. BUG=b:74518368 Change-Id: Id23455779b1c8c4931ad1a3122587e09ad237ecc Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-on: https://review.coreboot.org/25456 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
2018-03-16stoneyridge: Update AGESA binary and AGESA.hRichard Spiegel
AGESA.bin was updated in the binary repo, so update the submodule pointer. Among other changes, this added a callback "AGESA_HALT_THIS_AP", which requires updated header files. BUG=b:70338633 TEST=build kahlee. Change-Id: I5a07f1c539d00aed34cfe45d6d7ef60c1dc56566 Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/25183 Reviewed-by: Martin Roth <martinroth@google.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2018-02-12vc/amd/00670F00: Introduce S3FinalRestore helperMarshall Dawson
The Arch2008 spec describes an AmdS3FinalRestore Entry Point that coreboot has been missing. Add the helper function that can call into the blob to execute this. BUG=b:69614064 Change-Id: Ic72feb0406cd1d0d5c23e391c2464e12c9e10007 Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-on: https://review.coreboot.org/23442 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-by: Martin Roth <martinroth@google.com>
2018-02-01vendorcode/amd/pi/00670f00: Update headers to AGESA 1.3.0.9Marc Jones
Update the shared AGESA headers to 1.3.0.9. This depends on 3rdparty/blobs/pi/amd/00670F00/ binaries updated to the same version. BUG=b:72679320 TEST=build and boot Grunt Change-Id: I783b7318e8273913f753b70f12bfe8b71274e27f Signed-off-by: Marc Jones <marcj303@gmail.com> Reviewed-on: https://review.coreboot.org/23547 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2018-01-23binaryPI vendorcode: Remove HeapXXBuffer functionsKyösti Mälkki
The HeapAllocateBuffer and HeapDeallocateBuffer functions are not used. Change-Id: I491a796d87afd0e37051f9caabfff3f70d4d803c Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/22069 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
2018-01-05soc/amd/common: load post-memory AGESA as rmoduleAaron Durbin
Now that the AGESA binary is split into two sections load the post-memory AGESA binary into ram. It needs to be an rmdoule so that it can be relocated into ram. agesawrapper_amdinitenv() entry CBFS: 'VBOOT' located CBFS at [10000:cfd40) CBFS: Locating 'AGESA_POST_MEM' CBFS: Found @ offset 875c0 size 11c5e Decompressing stage AGESA_POST_MEM @ 0xc757ffc0 (183452 bytes) Loading module at c7580000 with entry c7580000. filesize: 0x2bafc memsize: 0x2bb0d Processing 1112 relocs. Offset value of 0xc7780000 AGESA call 00020001 using c75818fe AGESA call 00020003 using c75818fe Fch OEM config in INIT ENV Done agesawrapper_amdinitenv() returned AGESA_SUCCESS BUG=b:68141063,b:70714803 TEST=Booted kahlee. Change-Id: Ic0454e0d6909cb34ae8be2f4f221152532754d61 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Reviewed-on: https://review.coreboot.org/22976 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2018-01-05soc/amd/common: Allow AGESA file split for pre- and post-memoryJustin TerAvest
By splitting the binary files for platform initialization, the post-memory code can be modified to stop executing in place (--xip). This change creates two separate sections in CBFS for AGESA and loads the appropriate file at the correct stage. BUG=b:68141063 TEST=Booted kahlee with split agesa enabled. Change-Id: I2fa423df164037bc3738476fd2a34522df279e34 Signed-off-by: Justin TerAvest <teravest@chromium.org> Signed-off-by: Aaron Durbin <adurbin@chromium.org> Reviewed-on: https://review.coreboot.org/22974 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2017-12-13vc/amd/pi/0067F00: add option to add AGESA binary PI as stageAaron Durbin
Stage addition to CBFS allows relocation to happen on the fly. Take advantage of that by adding AGESA binary PI as a stage file so that each instance will be relocated properly within CBFS. Without this patch Chrome OS having multiple CBFS instances just redirects the AGESA calls back into RO which is inappropriate. BUG=b:65442265,b:68141063 TEST=Enabled AGESA_BINARY_PI_AS_STAGE and used ELF file. Booted and noted each instance in Chrome OS build was relocated. Change-Id: Ic0141bc6436a30f855148ff205f28ac9bce30043 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Reviewed-on: https://review.coreboot.org/22833 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2017-12-12vc/amd/pi/00670F00: fix #include paths to only use <amdblocks/header.h>Aaron Durbin
Ensure that soc/amd/common/blocks/include is the only #include path for the AMD common code. This removes the duplicate soc/amd/common include as well using the correct #include header in AGESA.c. BUG=b:69262110 Change-Id: I50d85b28514fd905df415f0cc052b9924ee4e741 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Reviewed-on: https://review.coreboot.org/22828 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-by: Justin TerAvest <teravest@chromium.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2017-12-12soc/amd/common: Move Agesa related headersRichard Spiegel
Move AGESA related headers in soc/amd/common to soc/amd/common/block/include/amdblocks. BUG=b:69262110 TEST=Build with no error gardenia and kahlee (no code change, headers moved). Change-Id: I5d3064625ddf8caaf370aabaf93165c6817f1ca0 Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com> Reviewed-on: https://review.coreboot.org/22772 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>
2017-12-11vc/amd/pi/00670F00/binaryPI: cache the AGESA dispatcherAaron Durbin
Instead of repeatedly walking cbfs for the AGESA blob and parsing it cache the resulting dispatcher value. There's only one dispatcher table so use it. The resulting change is that this work is done one time per stage. BUG=b:70401101 TEST=Booted and noted only one lookup per stage. Change-Id: Iaa4aecc384108d66d7c68fc5fb9ac1c3f40da905 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Reviewed-on: https://review.coreboot.org/22789 Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com> Reviewed-by: Justin TerAvest <teravest@chromium.org>
2017-11-22vendorcode/amd/pi/00670F00: Halt build if headers aren't wrappedMartin Roth
Make sure that AGESA headers don't get pulled directly into coreboot files again. BUG=b:66818758 TEST=Build gardenia; Build & boot kahlee; Include AGESA.h into files verify that the build fails. Change-Id: I8d6d94872ebf76a9df2850ed0452cf6b1a446ffd Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/22500 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-11-22vendorcode/amd/pi/00670F00: Remove direct AGESA header includesMartin Roth
Update amdlib to pull in the AGESA headers through agesa_headers.h BUG=b:66818758 TEST=Build gardenia; Build & boot kahlee Change-Id: I3a2a2fde9738a9fe7a0b55cb91c29416cdc227a2 Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/22550 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-11-21vendorcode/amd/pi/00670F00: Clean up makefileMartin Roth
- Remove unnecessary cflags, exports, and variables - Don't include AGESA cflags in the entire build - Reformat build target BUG=b:69220826 TEST=Build Change-Id: I60cb20a3849439cb808f5d3919588853e9c8c734 Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/22499 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marc Jones <marc@marcjonesconsulting.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-11-19vendorcode/amd/pi/00670F00: Remove dependency on amd/include dirMartin Roth
Copy the two headers used by the Stoney BinaryPI implementation into the 00670F00 directory so that any changes that are made to them don't affect other platforms. BUG=b:67299330 TEST=Build Change-Id: I5d37fac72871f2617c4be45c151741436cbfce96 Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/22498 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-11-19vendorcode/amd/pi/00670F00: Remove cpuFamilyTranslation.cMartin Roth
The file Proc/CPU/cpuFamilyTranslation.c isn't being included into the build, so it's obviously not needed. BUG=b:69220826 TEST=Build Change-Id: Id244d110b4f15e1d6af6c701f62e2f05d7eb289a Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/22496 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-11-16vendorcode/amd/pi/00670F00: Get rid of filecodes, replace filecode.hMartin Roth
coreboot doesn't need AGESA's version of Filecode.h. Some of the files that have been copied from AGESA include the header, so we can't get rid of it completely yet. - Remove includes from files that weren't copied from the AGESA source. - Remove FILECODE definitions from coreboot source. BUG=B:69220826 TEST=Build Gardenia; Build & boot Kahlee. Change-Id: If16feafc12dedeb90363826b62ea7513e54277f4 Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/22438 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2017-11-16vendorcode/amd/pi: Create stoney version of amdlibMartin Roth
Copy the vendorcode/amd/pi/Lib directory into 00670F00 directory and update the 00670F00 Makefile to use it instead of using the common version. This allows changes to stoney without affecting the rest of the AMD binary PI platforms. BUG=b:67299330 TEST=Build Gardenia; Build & boot kahlee Change-Id: I2fe4303f882938e9d917a3001476213f49426455 Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/22437 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-11-16vendorcode/amd/pi: Split stoney PI into its own MakefileMartin Roth
- Copy vendorcode/amd/pi makefile to 00670F00 directory - Remove all stoney references from the vendorcode/amd/pi makefile - Remove all non-stoney references from 00670F00 Makefile - Remove directories that don't exist from 00670F00 Makefile -- Proc/CPU/Feature -- Proc/Fch/Kern -- Proc/Fch/Kern/KernImc BUG=b:67299330 TEST=Build Gardenia; Build & boot kahlee Change-Id: I34690cfc3b1c4508d25d7cf062fcb9aea5945634 Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/22436 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-11-14AMD Stoney Ridge: Add agesa_headers.hMartin Roth
- Create header files for the stoneyridge PI that pulls in AGESA pi headers and encloses them in #pragma pack push/pop to keep the '#pragma pack(1)' in Porting.h from leaking. - Add that header to agesawrapper.h, replacing AGESA.h and Porting.h Following patches will update the coreboot code to use only agesawrapper.h to pull in the AGESA headers. BUG=b:66818758 TEST=Build tested Change-Id: Ib7d76811c1270ec7ef71266d84f3960919b792d4 Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/21713 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-11-11vendorcode/amd/pi/00670f00: Set ModuleIdentifier to be 8 bytesMartin Roth
ModuleIdentifier must be 8 bytes. Every other location else that uses this value explicityly defines it as 8 bytes. If it's initialized here to less than 8 bytes, it gets passed to those other locations with garbage at the end and fails to load the AGESA binary. TEST=Build & boot Kahlee BUG=B:69165234 Change-Id: I11fc90748f49782e2b16ee5326aee17cfe92d0bc Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/22430 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Justin TerAvest <teravest@chromium.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-11-03vendorcode/amd/pi/00670F00: remove unused headersMartin Roth
These header files are not used, so remove them. BUG=b:68812513 TEST=Build Change-Id: Ib43fc544186f7b46ecf9b318b9edcf008f2d08dc Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/22298 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2017-10-20vendorcode/amd/pi/00670F00: Remove S3 restore functionsMartin Roth
These functions are not currently used, and were not in the original AGESA source code drop. The structs involved here were marked "private" in AGESA headerfiles and should not be exposed. They could be handled as anonymous structs and required allocation size is communicated by other means. BUG=b:64766233 TEST=Build in cros tree and upstream coreboot, with old headers and updated headers. Change-Id: Iec346205470150257fd9d09131d54231b321740b Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/22061 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
2017-10-19src/vendorcode/amd: Use AR variable in MakefilesMartin Roth
Change-Id: I5158f1bcc18eb5b15f310d0cf50fb787c12317c8 Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/21700 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
2017-10-03vc/amd/pi/00670F00: Remove HeapXXBuffer functionsMartin Roth
The HeapAllocateBuffer and HEAPDeallocateBuffer functions are not used in Stoney Ridge, so get rid of them. Change-Id: I716d5c8957ced52c25fd501697111b1b0b263467 Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/21848 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
2017-10-03vendorcode/amd/pi: Put libagesa build all in libagesa directoryMartin Roth
Half the files were being placed in build/agesa and half in build/libagesa. Change-Id: Ied69dafffe2eb3354bd430789e098a1cb1d40551 Signed-off-by: Martin Roth <martinroth@google.com> Reviewed-on: https://review.coreboot.org/21699 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
2017-09-28amd/stoneyridge: Drop FieldAcccessorsKyösti Mälkki
It was either SAGE or AMD AES who implemented these for binaryPI, and it is not part of the documented AGESA API. My conclusions of these are: AmdGetValue() returns values from build-time configuration, these may not reflect the actual run-time configuration as there are OEM customization hooks to implement overrides. AmdSetValue() in __PRE_RAM__ will fail, as configuration data is const. Also AmdSetValue() in ramstage may fail, if said configuration data has already been evaluated. Semamtics of these calls are unusable unless one also has access to PI source to make exact decision on when they can be called. Remove these now that stoneyridge does not actually require them. Change-Id: I3379a75ce3b9448c17ef00eb16d3193c296626cd Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/21666 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com>