summaryrefslogtreecommitdiff
path: root/src/security
AgeCommit message (Collapse)Author
2021-02-27vboot: update GBB flags to use altfw terminologyJoel Kitching
As per CL:2641346, update GBB flag names: GBB_FLAG_FORCE_DEV_BOOT_LEGACY -> GBB_FLAG_FORCE_DEV_BOOT_ALTFW GBB_FLAG_DEFAULT_DEV_BOOT_LEGACY -> GBB_FLAG_DEFAULT_DEV_BOOT_ALTFW BUG=b:179458327 TEST=make clean && make test-abuild BRANCH=none Signed-off-by: Joel Kitching <kitching@google.com> Change-Id: I0ac5c9fde5a175f8844e9006bb18f792923f4f6d Reviewed-on: https://review.coreboot.org/c/coreboot/+/50906 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2021-02-16src/{drivers,security}: Remove unused <string.h>Elyes HAOUAS
Found using: diff <(git grep -l '#include <string.h>' -- src/) <(git grep -l 'STRINGIFY\|memcpy\|memmove\|memset\|memcmp\|memchr\|strdup\|strconcat\|strnlen\|strlen\|strchr\|strncpy\|strcpy\|strcmp\|strncmp\|strspn\|strcspn\|strstr\|strtok_r\|strtok\|atol\|strrchr\|skip_atoi\|snprintf' -- src/)|grep '<' Change-Id: Ief86a596b036487a17f98469c04faa2f8f929cfc Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/50691 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2021-02-15src/security: Remove unused <console/console.h>Elyes HAOUAS
Change-Id: I2b81a57ded80ef9c5cbdff06d8ca9d6b4f599777 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/50526 Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-02-11security/intel/txt/Makefile.inc: Use tab for indentElyes HAOUAS
Change-Id: Ic85a3b6cfb462f335df99e7d6c6c7aa46dc094e7 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/50432 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Frans Hendriks <fhendriks@eltan.com> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2021-02-09security/vboot/bootmode: Add weak fill_lb_gpiosRaul E Rangel
This change allows VBOOT to build when the mainboard hasn't implemented any of the VBOOT functions yet. Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: I42ca8f0dba9fd4a868bc7b636e4ed04cbf8dfab0 Reviewed-on: https://review.coreboot.org/c/coreboot/+/50341 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-02-09src: Add missing <cbmem.h>Elyes HAOUAS
Change-Id: I75a816c594b326df8a4aa5458bb055fca35e1741 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/50379 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2021-02-09security/tpm/tss/vendor/cr50/cr50.c: Fix typoElyes HAOUAS
Change-Id: I71c0b3b28979053b73f22f280ff11ba19ee0eee2 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/50372 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2021-02-04coreboot_table: Move VBOOT_VBNV supportKyösti Mälkki
The guard changes from (CHROMEOS && PC80_SYSTEM) to VBOOT_VBNV_CMOS here. Change-Id: I653285c04e864aa6a3494ba1400787fa184ba187 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/50250 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-by: Furquan Shaikh <furquan@google.com>
2021-02-03src: Remove unused <cbmem.h>Elyes HAOUAS
Change-Id: I2279e2d7e6255a88953b2485c1f1a3b51a72c65e Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/50182 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com>
2021-02-01security/vboot: Add config for GBB_FLAG_ENABLE_UDCEric Lai
This change adds the missing `GBB_FLAG_ENABLE_UDC` as a config in vboot/Kconfig (just like the other GBB flags) and uses its value to configure GBB_FLAGS Makefile variable. This is done to allow the mainboard to configure GBB flags by selecting appropriate configs in Kconfig. Signed-off-by: Eric Lai <ericr_lai@compal.corp-partner.google.com> Change-Id: I6b397713d643cf9461294e6928596dc847ace6bd Reviewed-on: https://review.coreboot.org/c/coreboot/+/50110 Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-02-01drivers/security/cbnt: Fix bootblock sizeArthur Heymans
Change-Id: Ic5ad9d29f247b6f828501bfacc27a8af08761d55 Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/50082 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jonathan Zhang <jonzhang@fb.com> Reviewed-by: Patrick Rudolph <siro@das-labor.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
2021-01-30soc/intel: Replace `SA_PCIEX_LENGTH` Kconfig optionsAngel Pons
Use the existing `MMCONF_BUS_NUMBER` and `MMCONF_LENGTH` symbols. Change-Id: I88dcc0d5845198f668c6604c45fd869617168231 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/50113 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2021-01-19security/tpm/tss/tcg-1.2/tss.c: Use __func__Elyes HAOUAS
Change-Id: I51e7111b17274b8951925d1c13e2f1386778b93a Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/49557 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Christian Walter <christian.walter@9elements.com>
2021-01-18security/intel/stm/StmPlatformSmm.c: Remove repeated wordElyes HAOUAS
Change-Id: I45adc4622f2d3358c703259931bafc4511395a5a Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/49529 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com>
2021-01-18security/vboot/secdata_tpm.c: Remove repeated wordElyes HAOUAS
Change-Id: Idc17a4305398defd19e7f6ba2fc00bf69af34c4b Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/49526 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jacob Garber <jgarber1@ualberta.ca>
2021-01-15build system: Always add coreboot.pre dependency to intermediatesPatrick Georgi
They all operate on that file, so just add it globally. Change-Id: I953975a4078d0f4a5ec0b6248f0dcedada69afb2 Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/49380 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Martin Roth <martinroth@google.com> Reviewed-by: Julius Werner <jwerner@chromium.org>
2021-01-14build system: Structure and serialize INTERMEDIATEPatrick Georgi
Target added to INTERMEDIATE all operate on coreboot.pre, each modifying the file in some way. When running them in parallel, coreboot.pre can be read from and written to in parallel which can corrupt the result. Add a function to create those rules that also adds existing INTERMEDIATE targets to enforce an order (as established by evaluation order of Makefile.inc files). While at it, also add the addition to the PHONY target so we don't forget it. BUG=chromium:1154313, b:174585424 TEST=Built a configuration with SeaBIOS + SeaBIOS config files (ps2 timeout and sercon) and saw that they were executed. Change-Id: Ia5803806e6c33083dfe5dec8904a65c46436e756 Signed-off-by: Patrick Georgi <pgeorgi@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/49358 Reviewed-by: Martin Roth <martinroth@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-01-08*/Makefile.inc: Add some INTERMEDIATE targets to .PHONYArthur Heymans
Change-Id: I125e40204f3a9602ee5810d341ef40f9f50d045b Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/48897 Reviewed-by: Idwer Vollering <vidwer@gmail.com> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2021-01-07security/intel/txt: Don't run SCHECK on CBnTArthur Heymans
This functionality only exists on legacy TXT. Change-Id: I4206ba65fafbe3d4dda626a8807e415ce6d64633 Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/49164 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Christian Walter <christian.walter@9elements.com> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2021-01-04security/intel/txt/ramstage.c: Fix clearing secrets on CBNTArthur Heymans
intel_txt_memory_has_secret() checks for ESTS.TXT_ESTS_WAKE_ERROR_STS || E2STS.TXT_E2STS_SECRET_STS and it looks like with CBNT the E2STS bit can be set without the ESTS bit. Change-Id: Iff4436501b84f5c209add845b3cd3a62782d17e6 Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/47934 Reviewed-by: Jonathan Zhang <jonzhang@fb.com> Reviewed-by: Christian Walter <christian.walter@9elements.com> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-12-29sec/intel/txt/Kconfig: Make TXT HEAP and SINIT size configurableArthur Heymans
More recent platforms (Cooperlake) need bigger sizes. Change-Id: Ia3e81d051a03b54233eef6ccdc4740c1a709be40 Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46556 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-12-03cbfs: Add verification for RO CBFS metadata hashJulius Werner
This patch adds the first stage of the new CONFIG_CBFS_VERIFICATION feature. It's not useful to end-users in this stage so it cannot be selected in menuconfig (and should not be used other than for development) yet. With this patch coreboot can verify the metadata hash of the RO CBFS when it starts booting, but it does not verify individual files yet. Likewise, verifying RW CBFSes with vboot is not yet supported. Verification is bootstrapped from a "metadata hash anchor" structure that is embedded in the bootblock code and marked by a unique magic number. This anchor contains both the CBFS metadata hash and a separate hash for the FMAP which is required to find the primary CBFS. Both are verified on first use in the bootblock (and halt the system on failure). The CONFIG_TOCTOU_SAFETY option is also added for illustrative purposes to show some paths that need to be different when full protection against TOCTOU (time-of-check vs. time-of-use) attacks is desired. For normal verification it is sufficient to check the FMAP and the CBFS metadata hash only once in the bootblock -- for TOCTOU verification we do the same, but we need to be extra careful that we do not re-read the FMAP or any CBFS metadata in later stages. This is mostly achieved by depending on the CBFS metadata cache and FMAP cache features, but we allow for one edge case in case the RW CBFS metadata cache overflows (which may happen during an RW update and could otherwise no longer be fixed because mcache size is defined by RO code). This code is added to demonstrate design intent but won't really matter until RW CBFS verification can be supported. Signed-off-by: Julius Werner <jwerner@chromium.org> Change-Id: I8930434de55eb938b042fdada9aa90218c0b5a34 Reviewed-on: https://review.coreboot.org/c/coreboot/+/41120 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com>
2020-12-02cbfs: Simplify load/map API names, remove type argumentsJulius Werner
This patch renames cbfs_boot_map_with_leak() and cbfs_boot_load_file() to cbfs_map() and cbfs_load() respectively. This is supposed to be the start of a new, better organized CBFS API where the most common operations have the most simple and straight-forward names. Less commonly used variants of these operations (e.g. cbfs_ro_load() or cbfs_region_load()) can be introduced later. It seems unnecessary to keep carrying around "boot" in the names of most CBFS APIs if the vast majority of accesses go to the boot CBFS (instead, more unusual operations should have longer names that describe how they diverge from the common ones). cbfs_map() is paired with a new cbfs_unmap() to allow callers to cleanly reap mappings when desired. A few new cbfs_unmap() calls are added to generic code where it makes sense, but it seems unnecessary to introduce this everywhere in platform or architecture specific code where the boot medium is known to be memory-mapped anyway. In fact, even for non-memory-mapped platforms, sometimes leaking a mapping to the CBFS cache is a much cleaner solution than jumping through hoops to provide some other storage for some long-lived file object, and it shouldn't be outright forbidden when it makes sense. Additionally, remove the type arguments from these function signatures. The goal is to eventually remove type arguments for lookup from the whole CBFS API. Filenames already uniquely identify CBFS files. The type field is just informational, and there should be APIs to allow callers to check it when desired, but it's not clear what we gain from forcing this as a parameter into every single CBFS access when the vast majority of the time it provides no additional value and is just clutter. Signed-off-by: Julius Werner <jwerner@chromium.org> Change-Id: Ib24325400815a9c3d25f66c61829a24a239bb88e Reviewed-on: https://review.coreboot.org/c/coreboot/+/39304 Reviewed-by: Hung-Te Lin <hungte@chromium.org> Reviewed-by: Wim Vervoorn <wvervoorn@eltan.com> Reviewed-by: Mariusz Szafrański <mariuszx.szafranski@intel.com> Reviewed-by: Patrick Georgi <pgeorgi@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-12-02cbfs: Move more stuff into cbfs_boot_lookup()Julius Werner
cbfs_boot_locate() is supposed to be deprecated eventually, after slowly migrating all APIs to bypass it. That means common features (like RO-fallback or measurement) need to be moved to the new cbfs_boot_lookup(). Also export the function externally. Since it is a low-level API and most code should use the higher-level loading or mapping functions instead, put it into a new <cbfs_private.h> to raise the mental barrier for using this API (this will make more sense once cbfs_boot_locate() is removed from <cbfs.h>). Signed-off-by: Julius Werner <jwerner@chromium.org> Change-Id: I4bc9b7cbc42a4211d806a3e3389abab7f589a25a Reviewed-on: https://review.coreboot.org/c/coreboot/+/39327 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2020-11-21cbfs: Add metadata cacheJulius Werner
This patch adds a new CBFS "mcache" (metadata cache) -- a memory buffer that stores the headers of all CBFS files. Similar to the existing FMAP cache, this cache should reduce the amount of SPI accesses we need to do every boot: rather than having to re-read all CBFS headers from SPI flash every time we're looking for a file, we can just walk the same list in this in-memory copy and finally use it to directly access the flash at the right position for the file data. This patch adds the code to support the cache but doesn't enable it on any platform. The next one will turn it on by default. Change-Id: I5b1084bfdad1c6ab0ee1b143ed8dd796827f4c65 Signed-off-by: Julius Werner <jwerner@chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/38423 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2020-11-18vboot: stop implementing VbExDisplayScreenJoel Kitching
This function is no longer required to be implemented since EC/AUXFW sync was decoupled from vboot UI. (See CL:2087016.) BUG=b:172343019 TEST=Compile locally BRANCH=none Signed-off-by: Joel Kitching <kitching@google.com> Change-Id: I43e8160a4766a38c4fa14bcf4495fc719fbcd6c2 Reviewed-on: https://review.coreboot.org/c/coreboot/+/47233 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Yu-Ping Wu <yupingso@google.com>
2020-11-17src: Add missing 'include <console/console.h>'Elyes HAOUAS
"printk()" needs <console/console.h>. Change-Id: Iac6b7000bcd8b1335fa3a0ba462a63aed2dc85b8 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/45539 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2020-11-10sec/intel/cbnt: Stitch in ACMs in the coreboot imageArthur Heymans
Actual support CBnT will be added later on. Change-Id: Icc35c5e6c74d002efee43cc05ecc8023e00631e0 Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46456 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-11-06security/vboot: Add Kconfig symbol to set hashing block sizeMartin Roth
Generally, this size probably doesn't matter very much, but in the case of picasso's psp_verstage, the hash is being calculated by hardware using relatively expensive system calls. By increasing the block size, we can save roughly 140ms of boot and resume time. TEST=Build & boot see that boot time has decreased. BRANCH=Zork BUG=b:169217270 - Zork: SHA calculation in vboot takes too long Signed-off-by: Martin Roth <martinroth@chromium.org> Change-Id: I68eecbbdfadcbf14288dc6e849397724fb66e0b2 Reviewed-on: https://review.coreboot.org/c/coreboot/+/46901 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com> Reviewed-by: Kangheui Won <khwon@chromium.org>
2020-11-04haswell: Add Intel TXT support in romstageAngel Pons
Provide necessary romstage hooks to allow unblocking the memory with SCLEAN. Note that this is slow, and took four minutes with 4 GiB of RAM. Tested on Asrock B85M Pro4 with tboot. When Linux has tboot support compiled in, booting as well as S3 suspend and resume are functional. However, SINIT will TXT reset when the iGPU is enabled, and using a dGPU will result in DMAR-related problems as soon as the IOMMU is enabled. However, SCLEAN seems to hang sometimes. This may be because the AP initialization that reference code does before SCLEAN is missing, but the ACM is still able to unblock the memory. Considering that SCLEAN is critical to recover an otherwise-bricked platform but is hardly ever necessary, prefer having a partially-working solution over none at all. Change-Id: I60beb7d79a30f460bbd5d94e4cba0244318c124e Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46608 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2020-11-04sec/intel/txt: Add support for running SCLEAN in romstageAngel Pons
SCLEAN has specific requirements and needs to run in early romstage, since the DRAM would be locked when SCLEAN needs to be executed. Change-Id: I77b237342e0c98eda974f87944f1948d197714db Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46607 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2020-10-28sec/intel/txt/Kconfig: Remove the menu for including ACMsArthur Heymans
This is consistent with how other binaries (e.g. FSP) are added via Kconfig. This also makes it more visible that things need to be configured. Change-Id: I399de6270cc4c0ab3b8c8a9543aec0d68d3cfc03 Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46455 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-10-28sec/intel/txt/Makefile.inc: Include ACMs using Kconfig variablesArthur Heymans
The Kconfig variables are used in the C code for cbfs file names but not in the Makefiles adding them. Change-Id: Ie35508d54ae91292f06de9827f0fb543ad81734d Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46454 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-10-28security/vboot: fix policy digest for nvmem spacesAndrey Pronin
This CL fixes the policy digest that restricts deleting the nvmem spaces to specific PCR0 states. BRANCH=none BUG=b:140958855 TEST=verified that nvmem spaces created with this digest can be deleted in the intended states, and cannot be deleted in other states (test details for ChromeOS - in BUG comments). Change-Id: I3cb7d644fdebda71cec3ae36de1dc76387e61ea7 Signed-off-by: Andrey Pronin <apronin@chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46772 Reviewed-by: Julius Werner <jwerner@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-10-26vboot: Disable vboot functions in SMMJulius Werner
SMM does not have access to CBMEM and therefore cannot access any persistent state like the vboot context. This makes it impossible to query vboot state like the developer mode switch or the currently active RW CBFS. However some code (namely the PC80 option table) does CBFS accesses in SMM. This is currently worked around by directly using cbfs_locate_file_in_region() with the COREBOOT region. By disabling vboot functions explicitly in SMM, we can get rid of that and use normal CBFS APIs in this code. Signed-off-by: Julius Werner <jwerner@chromium.org> Change-Id: I4b1baa73681fc138771ad8384d12c0a04b605377 Reviewed-on: https://review.coreboot.org/c/coreboot/+/46645 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2020-10-26security/tpm/tspi/crtm: Add line break to debug messagesFrans Hendriks
Add line break at debug messages. Tested on Facebook FBG1701 Change-Id: Idbfcd6ce7139efcb79e2980b366937e9fdcb3a4e Signed-off-by: Frans Hendriks <fhendriks@eltan.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46659 Reviewed-by: Christian Walter <christian.walter@9elements.com> Reviewed-by: Wim Vervoorn <wvervoorn@eltan.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-10-22sec/intel/txt: Split MTRR setup ASM code into a macroAngel Pons
If necessary, SCLEAN needs to run in early romstage, where DRAM is not working yet. In fact, that the DRAM isn't working is the reason to run SCLEAN in the first place. Before running GETSEC, CAR needs to be torn down, as MTRRs have to be reprogrammed to cache the BIOS ACM. Further, running SCLEAN leaves the system in an undefined state, where the only sane thing to do is reset the platform. Thus, invoking SCLEAN requires specific assembly prologue and epilogue sections before and after MTRR setup, and neither DRAM nor CAR may be relied upon for the MTRR setup. In order to handle this without duplicating the MTRR setup code, place it in a macro on a separate file. This needs to be a macro because the call and return instructions rely on the stack being usable, and it is not the case for SCLEAN. The MTRR code clobbers many registers, but no other choice remains when the registers cannot be saved anywhere else. Tested on Asrock B85M Pro4, BIOS ACM can still be launched. Change-Id: I2f5e82f57b458ca1637790ddc1ddc14bba68ac49 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46603 Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-10-22sec/intel/txt: Add `enable_getsec_or_reset` functionAngel Pons
This can be used to enable GETSEC/SMX in the IA32_FEATURE_CONTROL MSR, and will be put to use on Haswell in subsequent commits. Change-Id: I5a82e515c6352b6ebbc361c6a53ff528c4b6cdba Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46606 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2020-10-22sec/intel/txt: Extract BIOS ACM loading into a functionAngel Pons
Tested on Asrock B85M Pro4, still boots with TXT enabled. Change-Id: I0b04955b341848ea8627a9c3ffd6a68cd49c3858 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46593 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2020-10-22sec/intel/txt: Only run LockConfig for LT-SXAngel Pons
LockConfig only exists on Intel TXT for Servers. Check whether this is supported using GETSEC[PARAMETERS]. This eliminates a spurious error for Client TXT platforms such as Haswell, and is a no-op on TXT for Servers. Change-Id: Ibb7b0eeba1489dc522d06ab27eafcaa0248b7083 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46498 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2020-10-22sec/intel/txt: Always run SCHECK on regular bootsAngel Pons
When Boot Guard is disabled or not available, the IBB might not even exist. This is the case on traditional (non-ULT) Haswell, for example. Leave the S3 resume check as-is for now. Skylake and newer may need to run SCHECK on resume as well, but I lack the hardware to test this on. Change-Id: I70231f60d4d4c5bc8ee0fcbb0651896256fdd391 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46497 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2020-10-22sec/intel/txt: Allow skipping ACM NOP functionAngel Pons
This is merely used to test whether the BIOS ACM calling code is working properly. There's no need to do this on production platforms. Testing on Haswell showed that running this NOP function breaks S3 resume with TXT. Add a Kconfig bool to control whether the NOP function is to be invoked. Change-Id: Ibf461c18a96f1add7867e1320726fadec65b7184 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46496 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2020-10-22sec/intel/txt/ramstage.c: Do not init the heap on S3 resumeAngel Pons
It causes problems on Haswell: SINIT detects that the heap tables differ in size, and then issues a Class Code 9, Major Error Code 1 TXT reset. Change-Id: I26f3d291abc7b2263e0b115e94426ac6ec8e5c48 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46495 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2020-10-22sec/intel/txt/ramstage.c: Extract heap init into a functionAngel Pons
Heap initialization is self-contained, so place it into a separate function. Also, do it after the MSEG registers have been written, so that all register writes are grouped together. This has no impact. Change-Id: Id108f4cfcd2896d881d9ba267888f7ed5dd984fa Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46494 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2020-10-22sec/intel/txt: Add and fill in BIOS Specification infoAngel Pons
This is not critical to function, but is nice to have. Change-Id: Ieb5f41f3e4c5644a31606434916c35542d35617a Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46493 Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-10-22sec/intel/txt/common.c: Only log ACM error on failureAngel Pons
The TXT_BIOSACM_ERRORCODE register is only valid if TXT_SPAD bit 62 is set, or if CBnT is supported and bit 61 is set. Moreover, this is only applicable to LT-SX (i.e. platforms supporting Intel TXT for Servers). This allows TXT to work on client platforms, where these registers are regular scratchpads and are not necessarily written to by the BIOS ACM. Change-Id: If047ad79f12de5e0f34227198ee742b9e2b5eb54 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46492 Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-10-22sec/intel/txt: Move DPR size to KconfigAngel Pons
Instead of hardcoding the size in code, expose it as a Kconfig symbol. This allows platform code to program the size in the MCH DPR register. Change-Id: I9b9bcfc7ceefea6882f8133a6c3755da2e64a80c Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46491 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2020-10-22security/vboot: Remove all tpm 1.2 functions for mrc hash in the tpmShelley Chen
Since MRC_SAVE_HASH_IN_TPM depends on TPM2, we can now remove the tpm 1.2 versions of functions that deal with mrc hash in the tpm as it will not be used by tpm 1.2 boards. Also move all antirollback functions that deal with mrc hash in the tpm under CONFIG(TPM2). BUG=b:150502246 BRANCH=None TEST=make sure boards are still compiling on coreboot Jenkins Change-Id: I446dde36ce2233fc40687892da1fb515ce35b82b Signed-off-by: Shelley Chen <shchen@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46615 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Julius Werner <jwerner@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-10-20mrc_cache: Add tpm_hash_index field to cache_region structShelley Chen
Pull selection of tpm hash index logic into cache_region struct. This CL also enables the storing of the MRC hash into the TPM NVRAM space for both recovery and non-recovery cases. This will affect all platforms with TPM2 enabled and use the MRC_CACHE driver. BUG=b:150502246 BRANCH=None TEST=make sure memory training still works on nami and lazor Change-Id: I1a744d6f40f062ca3aab6157b3747e6c1f6977f9 Signed-off-by: Shelley Chen <shchen@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46514 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-10-20security/vboot: Add new TPM NVRAM index MRC_RW_HASH_NV_INDEXShelley Chen
Add new index for MRC_CACHE data in RW. Also update antirollback functions to handle this new index where necessary. BUG=b:150502246 BRANCH=None TEST=make sure memory training still works on nami Change-Id: I2de3c23aa56d3b576ca54dbd85c75e5b80199560 Signed-off-by: Shelley Chen <shchen@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46511 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-10-20security/vboot: Make mrc_cache hash functions genericShelley Chen
We need to extend the functionality of the mrc_cache hash functions to work for both recovery and normal mrc_cache data. Updating the API of these functions to pass in an index to identify the hash indices for recovery and normal mode. BUG=b:150502246 BRANCH=None TEST=make sure memory training still works on nami Change-Id: I9c0bb25eafc731ca9c7a95113ab940f55997fc0f Signed-off-by: Shelley Chen <shchen@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46432 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-10-20mrc_cache: Move mrc_cache_*_hash functions into mrc_cache driverShelley Chen
This CL would remove these calls from fsp 2.0. Platforms that select MRC_STASH_TO_CBMEM, updating the TPM NVRAM space is moved from romstage (when data stashed to CBMEM) to ramstage (when data is written back to SPI flash. BUG=b:150502246 BRANCH=None TEST=make sure memory training still works on nami Change-Id: I3088ca6927c7dbc65386c13e868afa0462086937 Signed-off-by: Shelley Chen <shchen@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46510 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com> Reviewed-by: Julius Werner <jwerner@chromium.org>
2020-10-20mrc_cache: Add config MRC_SAVE_HASH_IN_TPMShelley Chen
Use this config to specify whether we want to save a hash of the MRC_CACHE in the TPM NVRAM space. Replace all uses of FSP2_0_USES_TPM_MRC_HASH with MRC_SAVE_HASH_IN_TPM and remove the FSP2_0_USES_TPM_MRC_HASH config. Note that TPM1 platforms will not select MRC_SAVE_HASH_IN_TPM as none of them use FSP2.0 and have recovery MRC_CACHE. BUG=b:150502246 BRANCH=None TEST=emerge-nami coreboot chromeos-bootimage Change-Id: Ic5ffcdba27cb1f09c39c3835029c8d9cc3453af1 Signed-off-by: Shelley Chen <shchen@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46509 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-10-19security/vboot: Rename mem_init.h to mrc_cache_hash_tpm.hShelley Chen
As ongoing work for generalizing mrc_cache to be used by all platforms, we are pulling it out from fsp 2.0 and renaming it as mrc_cache_hash_tpm.h in security/vboot. BUG=b:150502246 BRANCH=None TEST=emerge-nami coreboot chromeos-bootimage Change-Id: I5a204bc3342a3462f177c3ed6b8443e31816091c Signed-off-by: Shelley Chen <shchen@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46508 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-10-17intel/txt: Add `txt_get_chipset_dpr` functionAngel Pons
Due to platform-specific constraints, it is not possible to enable DPR by programming the MCH's DPR register in ramstage. Instead, assume it has been programmed earlier and check that its value is valid. If it is, then simply configure DPR in TXT public base with the same parameters. Note that some bits only exist on MCH DPR, and thus need to be cleared. Implement this function on most client platforms. For Skylake and newer, place it in common System Agent code. Also implement it for Haswell, for which the rest of Intel TXT support will be added in subsequent commits. Do not error out if DPR is larger than expected. On some platforms, such as Haswell, MRC decides the size of DPR, and cannot be changed easily. Reimplementing MRC is easier than working around its limitations anyway. Change-Id: I391383fb03bd6636063964ff249c75028e0644cf Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46490 Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-by: Patrick Rudolph <siro@das-labor.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-10-17security/intel/txt: Improve MTRR setup for GETSEC[ENTERACCS]Angel Pons
The BIOS ACM will check that enabled variable MTRRs do not cover more than the ACM's size, rounded up to 4 KiB. If that is not the case, launching the ACM will result in a lovely TXT reset. How boring. The new algorithm simply performs a reverse bit scan in a loop, and allocates one MTRR for each set bit in the rounded-up size to cache. Before allocating anything, it checks if there are enough variable MTRRs; if not, it will refuse to cache anything. This will result in another TXT reset, initiated by the processor, with error type 5: Load memory type error in Authenticated Code Execution Area. This can only happen if the ACM has specific caching requirements that the current code does not know about, or something has been compromised. Therefore, causing a TXT reset should be a reasonable enough approach. Also, disable all MTRRs before clearing the variable MTRRs and only enable them again once they have been set up with the new values. Tested on Asrock B85M Pro4 with a BIOS ACM whose size is 101504 bytes. Without this patch, launching the ACM would result in a TXT reset. This no longer happens when this patch is applied. Change-Id: I8d411f6450928357544be20250262c2005d1e75d Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44880 Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-by: Christian Walter <christian.walter@9elements.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-10-17sec/intel/txt: Bail if var MTRRs cannot snugly cache the BIOS ACMAngel Pons
When caching the BIOS ACM, one must cache less than a page (4 KiB) of unused memory past the end of the BIOS ACM. Failure to do so on Haswell will result in a lovely TXT reset with Class Code 5, Major Error Code 2. The current approach uses a single variable MTRR to cache the whole BIOS ACM. Before fighting with the variable MTRRs in assembly code, ensure that enough variable MTRRs exist to cache the BIOS ACM's size. Since the code checks that the ACM base is aligned to its size, each `one` bit in the ACM size will require one variable MTRR to properly cache the ACM. One of the several BIOS ACMs for Haswell has a size of 101504 bytes. This is 0x18c80 in hexadecimal, and 0001 1000 1100 1000 0000 in binary. After aligning up the BIOS ACM size to a page boundary, the resulting size is 0x19000 in hexadecimal, and 0001 1001 0000 0000 0000 in binary. To successfully invoke said ACM, its base must be a multiple of 0x20000 and three variable MTRRs must be used to cache the ACM. The MTRR ranges must be contiguous and cover 0x10000, 0x8000, 0x1000 bytes, in order. The assembly code is updated in a follow-up, and relies on these checks. Change-Id: I480dc3e4a9e4a59fbb73d571fd62b0257abc65b3 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46422 Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-10-15sec/intel/txt/getsec_enteraccs.S: Save and restore MTRR_DEF_TYPEArthur Heymans
This needs to be saved and restored, otherwise the BSP might have an inconsistent MTRR setup with regards to the AP's which results in weird errors and slowdowns in the operating system. TESTED: Fixes booting OCP/Deltalake with Linux 5.8. Change-Id: Iace636ec6fca3b4d7b2856f0f054947c5b3bc8de Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46375 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Christian Walter <christian.walter@9elements.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-10-15security/intel/txt: Use `smm_region()` to get TSEG baseAngel Pons
This function is available for all TXT-capable platforms. Use it. As it also provides the size of TSEG, display it when logging is on. Change-Id: I4b3dcbc61854fbdd42275bf9456eaa5ce783e8aa Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46055 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2020-10-12security/intel/txt: Add and use DPR register layoutAngel Pons
This simplifies operations with this register's bitfields, and can also be used by TXT-enabled platforms on the register in PCI config space. Change-Id: I10a26bc8f4457158dd09e91d666fb29ad16a2087 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46050 Reviewed-by: Patrick Rudolph <siro@das-labor.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-10-12security/intel/txt: Clean up includesAngel Pons
Sort them alphabetically, and use <types.h> everywhere. Drop unused <intelblocks/systemagent.h> header, too. Change-Id: Ib8f3339e5969cf8552984164fa7e08e070987a24 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/46049 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Rudolph <siro@das-labor.org>
2020-10-12security/intel/stm: Add options for STM buildEugene Myers
This patch adds options that support building the STM as a part of the coreboot build. The option defaults assume that these configuration options are set as follows: IED_REGION_SIZE = 0x400000 SMM_RESERVED_SIZE = 0x200000 SMM_TSEG_SIZE = 0x800000 Change-Id: I80ed7cbcb93468c5ff93d089d77742ce7b671a37 Signed-off-by: Eugene Myers <cedarhouse@comcast.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44686 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: ron minnich <rminnich@gmail.com>
2020-10-08security/intel/txt: Print chipset as hex valueChristian Walter
Print chipset as hex value in order to make it more readable. Change-Id: Ifafbe0a1161e9fe6e790692002375f45d813b723 Signed-off-by: Christian Walter <christian.walter@9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/45867 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-10-01security/intel/stm: Fix size_t printf format errorFelix Held
This sort-of reverts commit 075df92298fe3bb0ef04233395effe668c4a5550 and fixes the underlying issue. The printf format string type/length specifier for a size_t type is z. Change-Id: I897380060f7ea09700f77beb81d52c18a45326ad Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/45872 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Eugene Myers <cedarhouse1@comcast.net> Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
2020-09-30security/intel/stm: Fix size_t printf format errorEugene D Myers
Size_t seems to have a compiler dependency. When building on the Purism librem 15v4, size_t is 'unsigned long'. In this instance, the compiler is the coreboot configured cross-compiler. In another instance, size_t is defined as 'unsigned short'. To get around the formatting conflict caused by this, The variable of type size_t was cast as 'unsigned int' in the format. Change-Id: Id51730c883d8fb9e87183121deb49f5fdda0114e Signed-off-by: Eugene D Myers <cedarhouse@comcast.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/45181 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: ron minnich <rminnich@gmail.com>
2020-09-21src/security: Drop unneeded empty linesElyes HAOUAS
Change-Id: Icb6057ac73fcc038981ef95a648420ac00b3c106 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44808 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
2020-09-21security/tpm/tss/tcg-2.0: add const to marshalling functionsCaveh Jalali
This adds the const qualifier to inputs of marshalling functions as they are intended to be read-only. Change-Id: I099bf46c928733aff2c1d1c134deec35da6309ba Signed-off-by: Caveh Jalali <caveh@chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/45409 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org> Reviewed-by: Julius Werner <jwerner@chromium.org>
2020-08-30security/intel/txt/getsec.c: Do not check lock bitAngel Pons
This allows calling GETSEC[CAPABILITIES] during early init, when the MSR isn't locked yet. Change-Id: I2253b5f2c8401c9aed8e32671eef1727363d00cc Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44883 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Rudolph <siro@das-labor.org>
2020-08-30security/intel/txt: Add missing definitionsAngel Pons
Change-Id: I3ca585429df318c31c2ffd484ec91a7971f18f27 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44882 Reviewed-by: Michael Niewöhner Reviewed-by: Patrick Rudolph <siro@das-labor.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-08-20drivers/spi/tpm: Enable long cr50 ready pulses for Tiger Lake systemsJes Klinke
For Volteer (and future Tiger Lake boards) we can enable mode S0i3.4 only if we know that the Cr50 is generating 100us interrupt pulses. We have to do so, because the SoC is not guaranteed to detect pulses shorter than 100us in S0i3.4 substate. A new Kconfig setting CR50_USE_LONG_INTERRUPT_PULSES controls new code running in verstage, which will program a new Cr50 register, provided that Cr50 firmware is new enough to support the register. BUG=b:154333137 TEST=util/abuild/abuild -t GOOGLE_VOLTEER -c max -x Signed-off-by: Jes Bodi Klinke <jbk@chromium.org> Change-Id: If83188fd09fe69c2cda4ce1a8bf5b2efe1ca86da Reviewed-on: https://review.coreboot.org/c/coreboot/+/43741 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com> Reviewed-by: Julius Werner <jwerner@chromium.org>
2020-08-18src: Remove unused 'include <lib.h>'Elyes HAOUAS
Change-Id: Ic09fc4ff4ee5524d89366e28d1d22900dd0c5b4d Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44100 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com>
2020-08-18src: Remove unused 'include <boot_device.h>'Elyes HAOUAS
Change-Id: I5589fdeade7f69995adf1c983ced13773472be74 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/42349 Reviewed-by: Patrick Georgi <pgeorgi@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-08-13security/vboot/Makefile.inc: Update regions-for-file functionSridhar Siricilla
This patch updates regions-for-file function in the security/vboot/Makefile.inc to support adding a CBFS file into required FMAP REGIONs in a flexible manner. The file that needs to be added to specific REGIONs, those regions list should be specified in the regions-for-file-{CBFS_FILE_TO_BE_ADDED} variable. For example, if a file foo.bin needs to be added in FW_MAIN_B and COREBOOT, then below code needs to be added in a Makefile.inc. regions-for-file-foo := FW_MAIN_B,COREBOOT cbfs-file-y := foo foo-file := foo.bin foo-type := raw TEST=Verified on hatch Signed-off-by: Sridhar Siricilla <sridhar.siricilla@intel.com> Change-Id: I1f5c22b3d9558ee3c5daa2781a115964f8d2d83b Reviewed-on: https://review.coreboot.org/c/coreboot/+/43766 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
2020-08-07security/intel/txt: Fix variable MTRR handlingAngel Pons
The MSR macros were treated as memory addresses and the loops had off-by-one errors. This resulted in a CPU exception before GETSEC, and another exception after GETSEC (once the first exception was fixed). Tested on Asrock B85M Pro4, ACM complains about the missing TPM and resets the platform. When the `getsec` instruction is commented-out, the board is able to boot normally, without any exceptions nor corruption. Change-Id: Ib5d23cf9885401f3ec69b0f14cea7bad77eee19a Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44183 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
2020-08-07security/intel/txt: Allow using CF9 reset, tooAngel Pons
Soften the hard dependency on SOC_INTEL_COMMON_BLOCK_SA by allowing CF9 resets to be used in place of global resets. If both types of reset are available, prefer a global reset. This preserves current behavior, and allows more platforms to use the TXT support code, such as Haswell. Change-Id: I034fa0b342135e7101c21646be8fd6b5d3252d9e Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44181 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Philipp Deppenwiese <zaolin.daisuki@gmail.com>
2020-08-06security/intel/txt: Avoid shifting by a negative valueJohn Zhao
Coverity detects an integer handling issue with BAD_SHIFT. The inline function log2_ceil(u32 x) { return (x == 0) ? -1 : log2(x * 2 - 1); } could return -1, which causes shifting by a negative amount value and has undefined behavior. Add sanity check for the acm_header->size to avoid shifting negative value. Found-by: Coverity CID 1431124 TEST=None Signed-off-by: John Zhao <john.zhao@intel.com> Change-Id: Ic687349b14917e39d2a8186968037ca2521c7cdc Reviewed-on: https://review.coreboot.org/c/coreboot/+/44186 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-07-31security/intel/txt: Add Intel TXT supportPhilipp Deppenwiese
Add TXT ramstage driver: * Show startup errors * Check for TXT reset * Check for Secrets-in-memory * Add assembly for GETSEC instruction * Check platform state if GETSEC instruction is supported * Configure TXT memory regions * Lock TXT * Protect TSEG using DMA protected regions * Place SINIT ACM * Print information about ACMs Extend the `security_clear_dram_request()` function: * Clear all DRAM if secrets are in memory Add a config so that the code gets build-tested. Since BIOS and SINIT ACM binaries are not available, use the STM binary as a placeholder. Tested on OCP Wedge100s and Facebook Watson * Able to enter a Measured Launch Environment using SINIT ACM and TBOOT * Secrets in Memory bit is set on ungraceful shutdown * Memory is cleared after ungraceful shutdown Change-Id: Iaf4be7f016cc12d3971e1e1fe171e6665e44c284 Signed-off-by: Philipp Deppenwiese <zaolin@das-labor.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/37016 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Christian Walter <christian.walter@9elements.com>
2020-07-26src: Change BOOL CONFIG_ to CONFIG() in comments & stringsMartin Roth
The Kconfig lint tool checks for cases of the code using BOOL type Kconfig options directly instead of with CONFIG() and will print out warnings about it. It gets confused by these references in comments and strings. To fix it so that it can find the real issues, just update these as we would with real issues. Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: I5c37f0ee103721c97483d07a368c0b813e3f25c0 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43824 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-07-26src: Remove unused 'include <cbmem.h>'Elyes HAOUAS
Change-Id: Ib41341b42904dc3050a97b70966dde7e46057d6b Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43362 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-07-21security/intel/stm: Add missing <stdbool.h>Angel Pons
Jenkins does not build `config.stm` because the file name lacks the mainboard name. So, the code was not being build-tested, and it does not build because several files lacked the definition for `bool`. Add the missing #include directives. Renaming the config file so that Jenkins build-tests it is done in a follow-up. Change-Id: Idf012b7ace0648027ef6e901d821ca6682cee198 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43622 Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr> Reviewed-by: Eugene Myers <cedarhouse1@comcast.net> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-15security/vboot: ensure that NVMEM is saved on every kernel space writeJett Rink
If the AP actually needs to write to the TPM, then it is important and the TPM should commit those changes to NVMEM immediately in case there is an unexpected power loss (e.g. from a USB-C port partner reset upon cold reboot request). BRANCH=none BUG=b:160913048 TEST=Verify that puff will no longer reboot loop when coreboot writes a new Hmir (Hash mirror) in the TPM Change-Id: I9597a55891d11bdf040d70f38b4c5a59c7888b8a Signed-off-by: Jett Rink <jettrink@chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43414 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-14src: Remove unused 'include <cpu/x86/msr.h>'Elyes HAOUAS
Found using: diff <(git grep -l '#include <cpu/x86/msr.h>' -- src/) <(git grep -l 'IA32_EFER\|EFER_\|TSC_MSR\|IA32_\|FEATURE_CONTROL_LOCK_BIT\|FEATURE_ENABLE_VMX\|SMRR_ENABLE\|CPUID_\|SGX_GLOBAL_ENABLE\|PLATFORM_INFO_SET_TDP\|SMBASE_RO_MSR\|MCG_CTL_P\|MCA_BANKS_MASK\|FAST_STRINGS_ENABLE_BIT\|SPEED_STEP_ENABLE_BIT\|ENERGY_POLICY_\|SMRR_PHYSMASK_\|MCA_STATUS_\|VMX_BASIC_HI_DUAL_MONITOR\|MC0_ADDR\|MC0_MISC\|MC0_CTL_MASK\|msr_struct\|msrinit_struct\|soc_msr_read\|soc_msr_write\|rdmsr\|wrmsr\|mca_valid\|mca_over\|mca_uc\|mca_en\|mca_miscv\|mca_addrv\|mca_pcc\|mca_idv\|mca_cecc\|mca_uecc\|mca_defd\|mca_poison\|mca_sublink\|mca_err_code\|mca_err_extcode\|MCA_ERRCODE_\|MCA_BANK_\|MCA_ERRTYPE_\|mca_err_type\|msr_set_bit\|msr_t\|msrinit_t' -- src/) |grep '<' Change-Id: I45a41e77e5269969280e9f95cfc0effe7f117a40 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41969 Reviewed-by: Patrick Georgi <pgeorgi@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-14src: Remove unused 'include <stdint.h>Elyes HAOUAS
Found using: diff <(git grep -l '#include <stdint.h>' -- src/) <(git grep -l 'int8_t\|int16_t\|int32_t\|int64_t\|intptr_t\|intmax_t\|s8\|u8\|s16\|u16\|s32\|u32\|s64\|u64\|INT8_MIN\|INT8_MAX\|INT16_MIN\|INT16_MAX\|INT32_MIN\|INT32_MAX\|INT64_MIN\|INT64_MAX\|INTMAX_MIN\|INTMAX_MAX' -- src/) |grep -v vendorcode |grep '<' Change-Id: I5e14bf4887c7d2644a64f4d58c6d8763eb74d2ed Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41827 Reviewed-by: Patrick Georgi <pgeorgi@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-09security/vboot/secdata_tpm.c: Drop dead codeAngel Pons
This code is not even being build-tested. Drop it before it grows moss. Change-Id: Ie01d65f80caf32a8318d5109ad48321661c5a87b Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43213 Reviewed-by: Julius Werner <jwerner@chromium.org> Reviewed-by: Michael Niewöhner Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-09security/tpm/tss/tcg-1.2/tss.c: Drop dead codeAngel Pons
This code is not even being build-tested. Drop it before it grows moss. Change-Id: Ifda2bbd87cd8ef5ec8e449d2c4d303be37b4d7c7 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43212 Reviewed-by: Julius Werner <jwerner@chromium.org> Reviewed-by: Michael Niewöhner Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-08security/vboot: Allow files to go into only RW-A or RW-B regionMartin Roth
The AMD firmware package created by amdfwtool contains pointers to the various binaries and settings. This means that we need different copies of the package in each region. This change allows for the different files in each of the 3 vboot regions. BUG=b:158124527 TEST=Build trembyle; see the correct versions of the files getting built into the RW-A & RW-B regions. Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: I45ff69dbc2266a67e05597bbe721fbf95cf41777 Reviewed-on: https://review.coreboot.org/c/coreboot/+/42822 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2020-07-06arch/x86: Remove RELOCATABLE_RAMSTAGEKyösti Mälkki
We always have it, no need to support opting-out. For PLATFORM_HAS_DRAM_CLEAR there is a dependency of ramstage located inside CBMEM, which is only true with ARCH_X86. Change-Id: I5cbf4063c69571db92de2d321c14d30c272e8098 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43014 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Nico Huber <nico.h@gmx.de> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-04drivers/pc80/tpm: Remove LPC_TPMKyösti Mälkki
Replace uses with MAINBOARD_HAS_LPC_TPM, if drivers/pc80/tpm is present in devicetree.cb it is necessary to always include the driver in the build. Change-Id: I9ab921ab70f7b527a52fbf5f775aa063d9a706ce Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41872 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Michael Niewöhner
2020-06-22lockdown: Add hint for how to check for lockdown support in boot logDaniel Gröber
Change-Id: Iaceb2e82f900e52efcce702486e18d0483665640 Signed-off-by: Daniel Gröber <dxld@darkboxed.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41749 Reviewed-by: Patrick Rudolph <siro@das-labor.org> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-06-22lockdown: Add Kconfigs for SPI media protection modeDaniel Gröber
SPI_WRITE_PROTECTION_REBOOT seems to be a Winbond thing, other vendors such as Macronix only support permanent protection but conditional on the WP# pin state. Change-Id: Iba7c1229c82c86e1303d74c7bc8f89662b5bb58c Signed-off-by: Daniel Gröber <dxld@darkboxed.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41747 Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Patrick Rudolph <siro@das-labor.org> 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-06-17security/tpm: Change wording of variable namesDuncan Laurie
Change the name of these variables to 'allowlist'. Change-Id: I9d5553988a1c9972b8f1ebaeee20878b23a8aa9b Signed-off-by: Duncan Laurie <dlaurie@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/42316 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Patrick Georgi <pgeorgi@google.com> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-06-17treewide: Replace CONFIG(ARCH_xx) testsKyösti Mälkki
Once we support building stages for different architectures, such CONFIG(ARCH_xx) tests do not evaluate correctly anymore. Change-Id: I599995b3ed5c4dfd578c87067fe8bfc8c75b9d43 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/42183 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-06-15security/vboot: Add option to run verstage before bootblockMartin Roth
For AMD's family 17h, verstage can run as a userspace app in the PSP before the X86 is released. The flags for this have been made generic to support any other future systems that might run verstage before the main processor starts. Although an attempt has been made to make things somewhat generic, since this is the first and currently only chip to support verstage before bootblock, there are a number of options which might ultimately be needed which have currently been left out for simplicity. Examples of this are: - PCI is not currently supported - this is currently just a given instead of making a separate Kconfig option for it. - The PSP uses an ARM v7 processor, so that's the only processor that is getting updated for the verstage-before-bootblock option. BUG=b:158124527 TEST=Build with following patches Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: I4849777cb7ba9f90fe8428b82c21884d1e662b96 Reviewed-on: https://review.coreboot.org/c/coreboot/+/41814 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2020-06-02src: Remove unused 'include <fmap.h>'Elyes HAOUAS
Change-Id: I79eea0a00c2de54f82b372229381534707a295bb Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41681 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-06-02src: Remove redundant includesElyes HAOUAS
<types.h> is supposed to provide <commonlib/bsd/cb_err.h>, <stdbool.h>,<stdint.h> and <stddef.h>. So remove those includes each time when <types.h> is included. Change-Id: I886f02255099f3005852a2e6095b21ca86a940ed Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41817 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Julius Werner <jwerner@chromium.org> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
2020-06-02src: Remove unused 'include <symbols.h>'Elyes HAOUAS
Change-Id: Ica355292eeda9c386b49db97f021566d52943d40 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41673 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-06-02lockdown: Fix missing spaces in boot logDaniel Gröber
Change-Id: I414703c53d356c6a69be515596c178997eed82e3 Signed-off-by: Daniel Gröber <dxld@darkboxed.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41748 Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-05-26Remove MAYBE_STATIC_BSS and ENV_STAGE_HAS_BSS_SECTIONKyösti Mälkki
After removal of CAR_MIGRATION there are no more reasons to carry around ENV_STAGE_HAS_BSS_SECTION=n case. Replace 'MAYBE_STATIC_BSS' with 'static' and remove explicit zero-initializers. Change-Id: I14dd9f52da5b06f0116bd97496cf794e5e71bc37 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/40535 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-by: Duncan Laurie <dlaurie@chromium.org> Reviewed-by: Julius Werner <jwerner@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-05-25security/tpm: Use SPDX identifiersJacob Garber
Also adjust a few comments to follow the style guide. Change-Id: I22001320f2ce1f0db348e0f7fabc5a65b50ba53e Signed-off-by: Jacob Garber <jgarber1@ualberta.ca> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41600 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>