Move out smm_create_map as this was not run if concurrent_save_states
is 1. The cpus struct array is used in the smm_get_cpu_smbase()
callback so it is necessary to create this.
TEST: run qemu/q35 with -smp 1 (or no -smp argument)
Change-Id: I07a98bbc9ff6dce548171ee6cd0c303db94087aa
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50783
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The parameters that the permanent handler requires are pushed directly
to the permanent handlers relocatable module params.
The paremeters that the relocation handler requires are not passed on
via arguments but are copied inside the ramstage. This is ok as the
relocation handler calls into ramstage.
Change-Id: Ice311d05e2eb0e95122312511d83683d7f0dee58
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50767
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
struct smm_loader_params is a struct that is passed around in the
ramstage code to set up either the relocation handler or the permanent
handler. At the moment no parameters in the stub 'smm_runtime' are
referenced so it can be dropped. The purpose is to drop the
smm_runtime struct from the stub as it is already located in the
permanent handler.
Change-Id: I09c1b649b5991f55b5ccf57f22e4a3ad4c9e4f03
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50766
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Keep a copy of start32_offset into ramstage to avoid needing to pass
arguments, calling from assembly. Doing this in C code is better than
assembly.
Change-Id: Iac04358e377026f45293bbee03e30d792df407fd
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50765
Reviewed-by: Eugene Myers <cedarhouse1@comcast.net>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Instead of passing on parameters from the stub to the permanent
handler, add them directly to the permanent handler.
The parameters in the stub will be removed in a later patch.
Change-Id: Ib3bde78dd9e0c02dd1d86e03665fa9c65e3d07eb
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50764
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
No need to do this assembly anymore.
Change-Id: I69b42c31e495530fe96030a5a25209775f9d4dca
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51533
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
With CBnT a digest needs to be made of the IBB, Initial BootBlock, in
this case the bootblock. After that a pointer to the BPM, Boot Policy
Manifest, containing the IBB digest needs to be added to the FIT
table.
If the fit table is inside the IBB, updating it with a pointer to the
BPM, would make the digest invalid.
The proper solution is to move the FIT table out of the bootblock.
The FIT table itself does not need to be covered by the digest as it
just contains pointers to structures that can by verified by the
hardware itself, such as microcode and ACMs (Authenticated Code
Modules).
Change-Id: I352e11d5f7717147a877be16a87e9ae35ae14856
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50926
Reviewed-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-by: Christian Walter <christian.walter@9elements.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The data needed to compute the permanent smbase for a core, when
relocating, is present in the ramstage data which the stub located at
DEFAULT_SMBASE (0x30000) calls back to. There is no need to fetch this
from via the stub params.
Change-Id: I3894c39ec8cae3ecc46b469a0fdddcad2a8f26c4
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50763
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This is only consumed by the stub and not by the relocation handler or
the permanent handler, so move it out of the runtime struct.
Change-Id: I01ed0a412c23c8a82d88408be058a27e55d0dc4d
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50762
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
These stub params need to be synced with the code in smm_stub.S and
are consumed by both the smmloader and smmloader_v2. So it is better
to have the definition located in one place.
Change-Id: Ide3e0cb6dea3359fa9ae660eab627499832817c9
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50761
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The idea is to get rid of having 2 different smmloaders so add this
option only to qemu/q35 to get it buildtested.
Change-Id: Id4901784c4044e945b7f258b3acdc8d549665f3a
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51525
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Tested with and without -enable-kvm, with -smp 1 2 and 32.
Change-Id: I612cebcd2ddef809434eb9bfae9d8681cda112ef
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48262
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
In pursuit of the eventual goal of removing cbfs_boot_locate() (and
direct rdev access) from CBFS APIs, this patch replaces all remaining
"simple" uses of the function call that can easily be replaced by the
newer APIs (like cbfs_load() or cbfs_map()). Some cases of
cbfs_boot_locate() remain that will be more complicated to solve.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Icd0f21e2fa49c7cc834523578b7b45b5482cb1a8
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50348
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Since prog_locate() was eliminated, prog_rdev() only ever represents the
loaded program in memory now. Using the rdev API for this is unnecessary
if we know that the "device" is always just memory. This patch changes
it to be represented by a simple pointer and size. Since some code still
really wants this to be an rdev, introduce a prog_chain_rdev() helper to
translate back to that if necessary.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: If7c0f1c5698fa0c326e23c553ea0fe928b25d202
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46483
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
CB:49896 added support in `intel_microcode_find()` to cache the found
microcode for faster subsequent accesses. This works okay when the
function succeeds in finding the microcode on BSP. However, if for any
reason, `cpu_microcode_blob.bin` does not contain a valid microcode
for the given processor, then the logic ends up attempting to find
microcode again and again every time it is called (because
`ucode_updates` is set to NULL on failed find, thus retriggering the
whole find sequence every time). This leads to a weird race condition
when multiple APs are running in parallel and executing this
function.
A snippet of the issues observed in the scenario described above:
```
...
microcode: Update skipped, already up-to-date
...
Microcode header corrupted!
...
```
1. AP reports that microcode update is being skipped since the current
version matches the version in CBFS (even though there is no matching
microcode update in CBFS).
2. AP reports microcode header is corrupted because it thinks that the
data size reported in the microcode is larger than the file read from
CBFS.
Above issues occur because each time an AP calls
`intel_microcode_find()`, it might end up seeing some intermittent
state of `ucode_updates` and taking incorrect action.
This change fixes this race condition by separating the logic for
finding microcode into an internal function `find_cbfs_microcode()`
and maintaining the caching logic in `intel_microcode_find()` using a
boolean flag `microcode_checked`.
BUG=b:182232187
TEST=Verified that `intel_microcode_find()` no longer makes repeated
attempts to find microcode from CBFS if it failed the first time.
Change-Id: I8600c830ba029e5cb9c0d7e0f1af18d87c61ad3a
Signed-off-by: Furquan Shaikh <furquan@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51371
Reviewed-by: Patrick Rudolph
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Add a helper function mp_run_on_all_aps, it allows running a given
func on all APs excluding the BSP, with an added provision to run
func in serial manner per AP.
BUG=b:169114674
Signed-off-by: Aamir Bohra <aamir.bohra@intel.com>
Change-Id: I74ee8168eb6380e346590f2575350e0a6b73856e
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51271
Reviewed-by: Furquan Shaikh <furquan@google.com>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Leverage the existing `acpigen_write_CST_package` function.
Yes, bad devicetree values can trigger undefined behavior. The old code
already had this issue, and will be addressed in subsequent commits.
Change-Id: Icec5431987d91242930efcea0c8ea4e3df3182fd
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49093
Reviewed-by: Nico Huber <nico.h@gmx.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
CPUID result does not change when HyperThreading is disabled on
HT-enabled CPUs, which breaks `generate_cpu_entries`. Use MSR 0x35
instead, which returns the currently-enabled core and thread count.
Also rename the function to `get_logical_cores_per_package, which is
more accurate. Based on commit 920d2b77f2 (cpu/intel/206ax/acpi.c: Fix
get_cores_per_package). The MSR definition is the same for Sandy Bridge
and Haswell.
Change-Id: I5e1789d3037780b4285c9e367ff0e2b0d4365b39
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49099
Reviewed-by: Nico Huber <nico.h@gmx.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Fix booting with SMP enabled, when specifying more CPUs than supported
by the code.
Change-Id: Ib3d7c1a1a7a8633d4d434ccbd46cf92b0074b724
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50235
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
It's a static value that is neither referenced from SMI handler
nor needs to be updated on S3 resume path.
Change-Id: I3928e5973fe65d9a4fe7975e5d5584efe6e5f2f8
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50120
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
It is useful to know if MCU have been applied successfully.
On the start of MP init lines similar to:
"AP: slot 1 apic_id 1, MCU rev: 0x0700001d" will be printed.
The example is taken from the log of an ocp/deltalake.
Change-Id: Ia0a6428b41d07f87943f3aa7736b8cb457fdd15a
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49840
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Get rid of custom microcode caching in MPinit and SGX code and
use the caching introduced in intel_microcode_find() instead.
Change-Id: If3ccd4dcff221c88839ffeafa812f4c38cede63f
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49897
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
This fixes a regression introduced by
Commit 985821c (cpu/intel/socket_LGA775: Increase DCACHE_RAM_SIZE)
where the CAR base is not aligned to its size.
Change-Id: If54cb178e86426e1491dda4047302632d876a8f0
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50029
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
lint report warning
Solve the RETURN_VOID
BUG = N/A
TEST = N/A
Change-Id: I3b8088494049b5c3244531a4a77af4153edbdff4
Signed-off-by: Frans Hendriks <fhendriks@eltan.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49970
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Erik van den Bogaert <ebogaert@eltan.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
With top-aligned bootblock this is no longer globally needed.
The default maximum is now a generous 256 KiB with couple
platforms having lower limits of 32 KiB and 64 KiB.
Change-Id: Ib1aee44908c0dcbc17978d3ee53bd05a6200410c
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47600
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Link .init section near the end of bootblock program.
It contains _start16bit, gdtptr and gdt that must be
addressable from realmode, thus within top 64 KiB.
Change-Id: If7b9737650362ac7cd82685cfdfaf18bd2429238
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47970
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
It was confusing to have this defined while there was another
symbol bootblock_protected_mode_entry that was not really used
as an entry point.
Change-Id: I3da07ba9c0a9fc15b1515452adfb27f963659951
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48404
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Like Haswell, Broadwell has a "FSB" speed of 100 MHz. Add the IDs for
both the traditional and ULT variants of Broadwell, because the CPU
driver for Haswell already contains CPUIDs for both Broadwell types.
Without this patch, Broadwell CPUs would hang when trying to print the
first console log message, but only if flashconsole was not enabled.
This was missed in commit f542b7bcef (cpu/intel/haswell: Add Broadwell
CPUIDs and microcode) and went unnoticed until now because the tests
were done with flashconsole enabled, which somehow boots properly even
though the console time tracking would not work (depends on TSC).
Tested on out-of-tree Acer E5-573, fixes booting without flashconsole.
Change-Id: I78a1696771d4d6d2138ec432dc0d8e030f14293b
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49939
Reviewed-by: Matt DeVillier <matt.devillier@gmail.com>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
None of the mainboards have the magic SpeedStep device, so the C-state
generation function bails out without doing anything. Moreover, this
code is broken and was copied from Sandy Bridge. Thus, drop it.
Change-Id: I580157ee33c599af5fc48b06eeb39cb32c9831ec
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49806
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Now that the boards use Haswell's CPU code, Broadwell can be updated.
Change-Id: If07e5272f07edb59bb18eef1f80d7d5807b26e66
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46949
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
