Intel CBnT (and Boot Guard) makes the chain of trust TOCTOU safe by
setting up NEM (non eviction mode) in the ACM. The CBnT IBB (Initial
BootBlock) therefore should not disable caching.
Sidenote: the MSR macros are taken from the slimbootloader project.
TESTED: ocp/Deltalake boot with and without CBnT and also a broken
CBnT setup.
Change-Id: Id2031e4e406655e14198e45f137ba152f8b6f567
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54010
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Christian Walter <christian.walter@9elements.com>
Over the last couple of years we have continuously added more and more
CBMEM init hooks related to different independent components. One
disadvantage of the API is that it can not model any dependencies
between the different hooks, and their order is essentially undefined
(based on link order). For most hooks this is not a problem, and in fact
it's probably not a bad thing to discourage implicit dependencies
between unrelated components like this... but one resource the
components obviously all share is CBMEM, and since many CBMEM init hooks
are used to create new CBMEM areas, the arbitrary order means that the
order of these areas becomes unpredictable.
Generally code using CBMEM should not care where exactly an area is
allocated, but one exception is the persistent CBMEM console which
relies (on a best effort basis) on always getting allocated at the same
address on every boot. This is, technically, a hack, but it's a pretty
harmless hack that has served us reasonably well so far and would be
difficult to realize in a more robust way (without adding a lot of new
infrastructure). Most of the time, coreboot will allocate the same CBMEM
areas in the same order with the same sizes on every boot, and this all
kinda works out (and since it's only a debug console, we don't need to
be afraid of the odd one-in-a-million edge case breaking it).
But one reproducible difference we can have between boots is the vboot
boot mode (e.g. normal vs. recovery boot), and we had just kinda gotten
lucky in the past that we didn't have differences in CBMEM allocations
in different boot modes. With the recent addition of the RW_MCACHE
(which does not get allocated in recovery mode), this is no longer true,
and as a result CBMEM consoles can no longer persist between normal and
recovery modes.
The somewhat kludgy but simple solution is to just create a new class of
specifically "early" CBMEM init hooks that will always run before all
the others. While arbitrarily partitioning hooks into "early" and "not
early" without any precise definition of what these things mean may seem
a bit haphazard, I think it will be good enough in practice for the very
few cases where this matters and beats building anything much more
complicated (FWIW Linux has been doing something similar for years with
device suspend/resume ordering). Since the current use case only relates
to CBMEM allocation ordering and you can only really be "first" if you
allocate in romstage, the "early" hook is only available in romstage for
now (could be expanded later if we find a use case for it).
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: If2c849a89f07a87d448ec1edbad4ce404afb0746
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54737
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
We would like to have an easy way to completely disable TPM support on a
board. For boards that don't pre-select a TPM protocol via the
MAINBOARD_HAS_TPMx options, this is already possible with the
USER_NO_TPM option. In order to make this available for all boards, this
patch just removes the whole USER_TPMx option group and directly makes
the TPM1 and TPM2 options visible to menuconfig. The MAINBOARD_HAS_TPMx
options can still be used to select defaults and to prevent selection of
a protocol that the TPM is known to not support, but the NO_TPM option
always remains available.
Also fix some mainboards that selected TPM2 directly, which they're not
supposed to do (that's what MAINBOARD_HAS_TPM2 is for), and add a
missing dependency to TPM_CR50 so it is set correctly for a NO_TPM
scenario.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Ib0a73da3c42fa4e8deffecb53f29ee38cbb51a93
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54641
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-by: Christian Walter <christian.walter@9elements.com>
Since the default for the corresponding UPD of the Picasso FSP is
DXIO_PSPP_POWERSAVE and the devicetree default is DXIO_PSPP_PERFORMANCE,
add a deviectree setting for each board that's using the Picasso SoC
code to not change the setting for the existing boards.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I0008ebb0c0f339ed3bdf24ab95a20aa83d5be2c9
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54934
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
hexdump and hexdump32 do similar things, but hexdump32 is mostly a
reimplementation that has additional support to configure the console
log level, but has a very unexpected len parameter that isn't in bytes,
but in DWORDs.
With the move to hexdump() the console log level for the hexdump is
changed to BIOS_DEBUG.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I6138d17f0ce8e4a14f22d132bf5c64d0c343b80d
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54925
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The only FSP 1.1 platform is Braswell, which has a non-weak definition
for the `soc_silicon_init_params` function. This changes the resulting
BUILD_TIMELESS=1 coreboot image for Facebook fbg1701, for some reason.
Change-Id: I2a1b51cda9eb21d7af8372c16a43195a4bdd9543
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54956
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
The only FSP 1.1 platform is Braswell. Drop unused weak definitions for
functions where a non-weak definition always exists.
Tested with BUILD_TIMELESS=1, Facebook fbg1701 remains identical.
Change-Id: Ifaf40a1cd661b123911fbeaafeb2b7002559a435
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54955
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
This allows boards to specify which PSPP policy (basically a dynamic
trade-off between power consumption and PCIe link speed) should be used
and also makes sure that the boards are using the expected PSPP policy
and not just the UPD default from the FSP binary that has already
changed once during the development.
BUG=b:188793754
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I1b6459b2984711e72b79f5d4d90e04cb4b78d512
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54930
Reviewed-by: Matt Papageorge <matthewpapa07@gmail.com>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
TBT PowerResource _ON/_OFF methods are currently invoked by _PS0 and
_PS3 respectively. It is defined for ACPI driver to call _ON and _OFF
methods. This change drops the _PS0 and _PS3 call for _ON/_OFF and
returns TBT PowerResource declaration in the _PR0 and _PR3, then ACPI
driver will call the TBT PowerResource _ON and _OFF methods.
BUG=b:188891878
TEST=Traced both of TBT _ON and _OFF methods invocation and execution
at run time. Verified TBT's power_state to be D3Cold.
Signed-off-by: John Zhao <john.zhao@intel.com>
Change-Id: I398b3f58ec89f98673cbbe633149d31188ec3351
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54812
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Generic Initiator Affinity structure is introdcued in ACPI spec 6.3.
This structure is used to define NUMA affinity domain which is
established by generic initiator (such as by CXL device).
Signed-off-by: Jonathan Zhang <jonzhang@fb.com>
Change-Id: Ic6ef01c59e02f30dc290f27e741027e16f5d8359
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52734
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Lance Zhao
No board uses AMD PI 00630F01, so drop it. And drop a single reference
to the now-removed `NORTHBRIDGE_AMD_PI_00630F01` Kconfig option inside
the `drivers/amd/agesa/acpi_tables.c` file.
Change-Id: Ibc45a4a6041220ed22273c1d41f9b796e1acb901
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54897
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Microcode header supports advertising support for only one CPU
signature and processor flags. If there are multiple processor
families supported by this microcode blob, they are mentioned in
the extended signature table.
Add support to parse the extended processor signature table to
determine if the microcode blob supports the currently running CPU.
BUG=b:182234962
TEST=Booted ADL brya system with a processor whose signature/pf are
in the extended signature table of a microcode patch. Was able to
match and load the patch appropriately.
Signed-off-by: Rizwan Qureshi <rizwan.qureshi@intel.com>
Change-Id: I1466caf4a4ba1f9a0214bdde19cce57dd65dacbd
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54734
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
After Thunderbolt firmware is downloaded to IMR, its authentication
validity needs to be checked. This change implements the valid_tbt_auth
function. Thunderbolt DSD and its corresponding IMR_VAID will be
present to kernel only if its authentication is successful.
BUG=b:188695995
TEST=Validated TGL TBT firmware authentication and its IMR_VALID
into SSDT which is properly present to kernel.
Signed-off-by: John Zhao <john.zhao@intel.com>
Change-Id: I3c9dda341ae6f19a2a8c85f92edda3dfa08c917a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54693
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
After Thunderbolt firmware is downloaded to IMR, its authentication
validity needs to be checked. This change adds the TBT firmware IMR
status register offset and its authentication valid bit for
valid_tbt_auth function usage.
BUG=b:188695995
TEST=Built coreboot image successfully.
Signed-off-by: John Zhao <john.zhao@intel.com>
Change-Id: I742a00b6b58c45c1261f06b06a94346ad0a74829
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54888
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
After Thunderbolt firmware is downloaded to IMR, its authentication
validity needs to be checked. This change adds the TBT firmware IMR
status register offset and its authentication valid bit for
valid_tbt_auth function usage.
BUG=b:188695995
TEST=Built Voxel coreboot image successfully.
Signed-off-by: John Zhao <john.zhao@intel.com>
Change-Id: Ia25827f18a10bf4d2dcabfe81565ac326851af3e
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54709
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Building google/sarien with a 64-bit compiler (x86_64-linux-gnu) fails
with the error below.
src/ec/google/wilco/mailbox.c: In function 'wilco_ec_transfer':
src/ec/google/wilco/mailbox.c:184:43: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
184 | printk(BIOS_ERR, "%s: data too short (%lu bytes, expected %zu)",
| ~~^
| |
| long unsigned int
| %u
185 | __func__, rs.data_size - skip_size, msg->response_size);
| ~~~~~~~~~~~~~~~~~~~~~~~~
| |
| size_t {aka unsigned int}
`data_size` has type `uint16_t`, and `skip_size` has type `size_t`,
whose size differs in 32-bit (unsigned int) and 64-bit (unsigned long).
So use the length modifier `z` for a `size_t` argument.
Found-by: x86_64-linux-gnu-gcc-10 (Debian 10.2.1-6) 10.2.1 20210110
Change-Id: Ida27323daeed9b8ff487302d0f3d6fcce0bbb705
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54786
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jacob Garber <jgarber1@ualberta.ca>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Furquan Shaikh <furquan@google.com>
Reviewed-by: Duncan Laurie
Coverity found resource leak in test setup function in error block.
Add malloc result check and free in error handling to silence Coverity.
Signed-off-by: Jakub Czapiga <jacz@semihalf.com>
Found-by: Coverity CID 1446760
Change-Id: Icf746df27167047fa3cf8f5df09fced20863f76d
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54874
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Fagerburg <pfagerburg@chromium.org>
Configure GPIO CAM_PDN5 (AP_XHCI_INIT_DONE) as output, so that
payloads (for example depthcharge) can assert it to notify EC to enable
USB VBUS.
BUG=b:187149602
TEST=emerge-asurada coreboot
BRANCH=asurada
Change-Id: I3bf63f91b8057e35be2780024a8b398c3044729b
Signed-off-by: Yu-Ping Wu <yupingso@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54902
Reviewed-by: Hung-Te Lin <hungte@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Incorrect size of the einj structure was being used, which created an
invalid checksum message by the OS. This patch fixes the issue.
Test=Booted to Linux on Deltalake mainboard and verified invalid checksum
message is not logged in syslog. Exact message -> 'ACPI BIOS Warning
(bug): Incorrect checksum in table [EINJ] - 0xDA, should be 0xD9'
Change-Id: I2b1722d6960d4a62d14fb02ac5e8838397e12f92
Signed-off-by: Rocky Phagura <rphagura@fb.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54787
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Reviewed-by: Lance Zhao
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
scan-build found a dead assignment, that the value stored to `res` is
never read. Use `pci_dev_read_resources()` instead, as done in
`sb/intel/common/smbus_ops.c` since commit 5f734327
(sb/intel/common/smbus_ops.c: Clean up read resources) avoiding the
assignment.
Change-Id: Ic59063b05a45dca411bf5b56c1abf3dd66ff0437
Found-by: scan-build (coreboot toolchain v0ad5fbd48d 2020-12-24 - clang version 11.0.0)
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54904
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
