`mrc_cache_needs_update` is comparing the "new size" of the MRC data
(minus metadata size) to the size including the metadata, which causes
the driver to think the data has changed, and so it will rewrite the
MRC cache on every boot. This patch removes the metadata size from
the comparison.
BUG=b:171513942
BRANCH=volteer
TEST=1) Memory training data gets written the on a boot where the data
was wiped out.
2) Memory training data does not get written back on every subsequent
boot.
Signed-off-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Change-Id: I7280276f71fdaa492c327b2b7ade8e53e7c59f51
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46824
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Shelley Chen <shchen@google.com>
Reviewed-by: Furquan Shaikh <furquan@google.com>
When MRC_SAVE_HASH_IN_TPM is selected, we can just use the TPM hash to
verify the MRC_CACHE data. Thus, we don't need to calculate the
checksum anymore in this case.
BUG=b:150502246
BRANCH=None
TEST=make sure memory training still works on nami
Change-Id: I1db4469da49755805b541f50c7ef2f9cdb749425
Signed-off-by: Shelley Chen <shchen@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46515
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
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>
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>
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>
Modify mrc_cache_load current to return the size of the mrc_cache
entry so that caller will know what the actual size of the data
returned is. This is needed for ARM devices like trogdor, which need
to know the size of the training data when populating the QcLib
interface table.
BUG=b:150502246
BRANCH=None
TEST=./util/abuild/abuild -p none -t GOOGLE_NAMI -x -a
Change-Id: Ia314717ad2a7d5232b37a19951c1aecd7f843c27
Signed-off-by: Shelley Chen <shchen@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46110
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
Previously, we were writing to cbmem after memory training and then
writing the training data from cbmem to mrc_cache in ramstage. We
were doing this because we were unable to read/write to SPI
simultaneously on older x86 chips. Now that newer chips allow for
simultaneously reads and writes, we can move the mrc_cache update into
romstage. This is beneficial if there is a reboot for some reason
after memory training but before the previous mrc_cache_stash_data
call originally in ramstage. If this happens, we would lose all the
mrc_cache training data in the next boot even though we've already
performed the memory training.
Added new config BOOT_DEVICE_SPI_FLASH_NO_EARLY_WRITES to accomodate
older x86 platforms that don't do mmapping but still want to use the
cbmem to store the mrc_cache data in order to write the mrc_cache data
back at a later time. We are maintaining the use of cbmem for these
older platforms because we have no way of validating the earlier write
back to mrc_cache at this time.
BUG=b:150502246
BRANCH=None
TEST=reboot from ec console. Make sure memory training happens.
reboot from ec console. Make sure that we don't do training again.
Signed-off-by: Shelley Chen <shchen@google.com>
Change-Id: I3430bda45484cb8c2b01ab9614508039dfaac9a3
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44196
Reviewed-by: Furquan Shaikh <furquan@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
On autogenerated FMAPs, there's no `UNIFIED_MRC_CACHE` region. The
current code will print a spurious error message about it, though.
Reduce the log level to BIOS_INFO to avoid confusion.
Change-Id: I0961bb2a7d2d81dc5c0d28f6e6c29b320421fc3e
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45076
Reviewed-by: Furquan Shaikh <furquan@google.com>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Reviewed-by: Christian Walter <christian.walter@9elements.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
We need to pull update_mrc_cache into mrc_cache_stash_data, so moving
to end of the file to make sure update_mrc_cache is defined before.
BUG=b:150502246
BRANCH=None
TEST=Testing on a nami (x86) device:
reboot from ec console. Make sure memory training happens.
reboot from ec console. Make sure that we don't do training again.
Signed-off-by: Shelley Chen <shchen@google.com>
Change-Id: I9e14fec96e9dabceafc2f6f5663fc6f1023f0395
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44195
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
Create two new functions to fetch mrc_cache data (replacing
mrc_cache_get_current):
- mrc_cache_load_current: fetches the mrc_cache data and drops it into
the given buffer. This is useful for ARM platforms where the mmap
operation is very expensive.
- mrc_cache_mmap_leak: fetch the mrc_cache data and puts it into a
given buffer. This is useful for platforms where the mmap operation
is a no-op (like x86 platforms). As the name mentions, we are not
freeing the memory that we allocated with the mmap, so it is the
caller's responsibility to do so.
Additionally, we are replacing mrc_cache_latest with
mrc_cache_get_latest_slot_info, which does not check the validity of
the data when retrieving the current mrc_cache slot. This allows the
caller some flexibility in deciding where they want the mrc_cache data
stored (either in an mmaped region or at a given address).
BUG=b:150502246
BRANCH=None
TEST=Testing on a nami (x86) device:
reboot from ec console. Make sure memory training happens.
reboot from ec console. Make sure that we don't do training again.
Signed-off-by: Shelley Chen <shchen@google.com>
Change-Id: I259dd4f550719d821bbafa2d445cbae6ea22e988
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44006
Reviewed-by: Furquan Shaikh <furquan@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Fix the scan-build warning below:
CC romstage/drivers/mrc_cache/mrc_cache.o
src/drivers/mrc_cache/mrc_cache.c:450:26: warning: Value stored to 'flash' during its initialization is never read
const struct spi_flash *flash = boot_device_spi_flash();
^~~~~ ~~~~~~~~~~~~~~~~~~~~~~~
1 warning generated.
The function can return early before the value is read. Fix this, by
getting rid of the variable, as the value is only read once.
Change-Id: I3c94b123f4994eed9d7568b63971fd5b1d94bc09
Found-by: scan-build (clang-tools-9 1:9.0.1-12)
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42798
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
When MRC cached data update is performed, messages are written to
event log, which is flash based. For system that does not have flash
based event log, the messages are lost.
Added corresponding BIOS_DEBUG messages.
Signed-off-by: Jonathan Zhang <jonzhang@fb.com>
Change-Id: I1ef4794151fea7213c8317ddc898b0e37da280b5
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41981
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
Reviewed-by: Michael Niewöhner
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
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>
vboot_recovery_mode_enabled() was recently changed to assert() when it
is called before vboot logic has run, because we cannot determine
whether we're going to be in recovery mode at that point and we wanted
to flush out existing uses that pretended that we could. Turns out there
are a bunch of uses like that, and there is some code that is shared
across configurations that can and those that can't.
This patch cleans them up to either remove checks that cannot return
true, or add explicit Kconfig guards to clarify that the code is shared.
This means that using a separate recovery MRC cache is no longer
supported on boards that use VBOOT_STARTS_IN_ROMSTAGE (this has already
been broken with CB:38780, but with this patch those boards will boot
again using their normal MRC caches rather than just die). Skipping the
MRC cache and always regenerating from scratch in recovery mode is
likewise no longer supported for VBOOT_STARTS_IN_ROMSTAGE.
For FSP1.1 boards, none of them support VBOOT_STARTS_IN_BOOTBLOCK and
that is unlikely to change in the future so we will just hardcode that
fact in Kconfig (otherwise, fsp1.1 raminit would also have to be fixed
to work around this issue).
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I31bfc7663724fdacab9955224dcaf650d1ec1c3c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39221
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Done with sed and God Lines. Only done for C-like code for now.
Change-Id: I38eaffa391ed5971217ffad74a312b1641e431c9
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40051
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Indent continuation lines of an if test farther than its "true"
expression to be executed.
Change-Id: I3dfa4049761095dcbb6797f1533d6a513e3b503c
Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37720
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
As discussed on the mailing list and voted upon, the coreboot project
is going to move the majority of copyrights out of the headers and into
an AUTHORS file. This will happen a bit at a time, as we'll be unifying
license headers at the same time.
Updated Authors file is in a separate commit.
Signed-off-by: Martin Roth <martin@coreboot.org>
Change-Id: Ia0a07df6ca1fdaa2837ce8839057057cbd44d157
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36181
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
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>
Some SPI controllers support both READ and WRITE protection
add a variable to the protect API for the callers to specify
the kind of protection they want (Read/Write/Both).
Also, update the callers and protect API implementation.
BUG=None
BRANCH=None
TEST=test that the mrc cache is protected as expected on soraka.
Also tried if the read protection is applied correctly.
Change-Id: I093884c4768b08a378f21242ac82e430ac013d15
Signed-off-by: Rizwan Qureshi <rizwan.qureshi@intel.com>
Reviewed-on: https://review.coreboot.org/c/30559
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
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>
This automatically generates an FMAP region for the MRC_CACHE driver
which is easier to handle than a cbfsfile.
Adds some spaces and more comments to Makefile.inc to improve
readability.
Tested on Thinkpad x200 with some proof of concept patches.
Change-Id: Iaaca36b1123b094ec1bbe5df4fb25660919173ca
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/23150
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
The default time for writing MRC data to flash is at the exit of
BS_DEV_ENUMERATE but this is too early for some implementations.
Add an option to Kconfig for allowing a late option to be selected.
The timing of the late option is at the entry of BS_OS_RESUME_CHECK.
TEST=Select option and inspect console log on Kahlee
BUG=b:69614064
Change-Id: Ie7ba9070829d98414ee788e14d1a768145d742ea
Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com>
Reviewed-on: https://review.coreboot.org/22937
Reviewed-by: Martin Roth <martinroth@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
There's nothing intel-specific about the current mrc_cache support.
It's logic manages saving non-volatile areas into the boot media.
Therefore, expose it to the rest of the system for any and all to
use.
BUG=b:69614064
Change-Id: I3b331c82a102f88912a3e10507a70207fb20aecc
Signed-off-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-on: https://review.coreboot.org/22901
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
2017-12-17 18:29:41 +00:00
Renamed from src/soc/intel/common/mrc_cache.c (Browse further)
