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>
The set_ts_fit_ptr makefile target was never a dependency of another
target and therefore not used.
Change-Id: Ie6b20164fce0dc406a28b4c1b9f41a79c68c27d7
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54677
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Rizwan Qureshi <rizwan.qureshi@intel.com>
'-t' is not needed when setting the FIT pointer and breaks
it as '-t' needs an argument so the $(TS_OPTIONS) is not properly
decoded.
Change-Id: I61a3ac1eda42e04152a7d10953bfb8407813d0f3
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54679
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Rizwan Qureshi <rizwan.qureshi@intel.com>
Make sure the fit pointer is set up before entries are added.
Change-Id: I285fbb830a52e43cde5e8db9569a64dafb4408df
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54678
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Meera Ravindranath <meera.ravindranath@intel.com>
Reviewed-by: Rizwan Qureshi <rizwan.qureshi@intel.com>
This removes the need to include this code separately on each
platform.
Change-Id: I3d848b1adca4921d7ffa2203348073f0a11d090e
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46380
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Create the correct MTRR solution based on the physical address space
provided by RESOURCE_ALLOCATOR_V4. Previously CPU initialization did not
account for lost C6 DRAM storage MTRR during postcar frame creation.
The BSP on 2GB has been stripped from UC MTRR covering C6 DRAM and
overlapping with usable DRAM WB MTRR. However this UC MTRR remained on
APs which caused inconsistent MTRRs warning in Linux. Use generic MTRR
function to create correct MTRR solution that propagates to APs. This
also fixes the inconsistent MTRRs warning.
TEST=boot Debian with Linux 4.14 on apu2 4GB ECC and apu3 2GB no-ECC
Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>
Change-Id: Ie2d7a75affd7d3d3a1bc6327fb423e206b28562f
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52762
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Fix booting issues on google/kahlee introduced by CB:51723.
Update use inital apic id in smm_stub.S to support xapic mode error.
Check more bits(LAPIC_BASE_MSR BIT10 and BIT11) for x2apic mode.
TEST=Boot to OS and check apicid, debug log for CPUIDs
cpuid_ebx(1), cpuid_ext(0xb, 0), cpuid_edx(0xb) etc
Signed-off-by: Wonkyu Kim <wonkyu.kim@intel.com>
Change-Id: Ia28f60a077182c3753f6ba9fbdd141f951d39b37
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52696
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
The CMOS option system does not support negative integers. Thus, retype
and rename the option API functions to reflect this.
Change-Id: Id3480e5cfc0ec90674def7ef0919e0b7ac5b19b3
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52672
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Let the linker decide if this code is needed.
Change-Id: I26fb19d461db39ce554af7b948f0d10a12920299
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52940
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
This patch removes a call to console_init() and debug print message since
the code is not thread safe. This prevents system hangs (soft hangs)
while in SMM if user drops in a new SOC with more cores or another
socket or as a result of bad configuration. Console is already
initialized after the lock has been acquired so this does not affect any
other functionality.
Tested on DeltaLake mainboard with SMM enabled and 52 CPU threads.
Change-Id: I7e8af35d1cde78b327144b6a9da528ae7870e874
Signed-off-by: Rocky Phagura <rphagura@fb.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52518
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Some platforms which have large amounts of RAM and also write-combining
regions may decide to drop the WC regions in favor of the default when
preserving MTRRs for the OS. From a data safety perspective, this is
safe to do, but if, say, the graphics framebuffer is the region that is
changed from WC to UC/WB, then the performance of writing to the
framebuffer will decrease dramatically.
Modern OSes typically use Page Attribute Tables (PAT) to determine the
cacheability on a page level and usually do not touch the MTRRs. Thus,
it is believed to be safe to stop reserving MTRRs for the OS, in
general; PentiumII is the exception here in that OSes that still
support that may still require MTRRs to be available. In any case, if
the OS wants to reprogram all of the MTRRs, it is of course still free
to do so (after consulting the e820 table).
BUG=b:185452338
TEST=Verify MTRR programming on a brya (where `sa_add_dram_resources`
was faked to think it had 32 GiB of DRAM installed) and variable MTRR
map includes a WC entry for the framebuffer (and all the RAM):
MTRR: default type WB/UC MTRR counts: 13/9.
MTRR: UC selected as default type.
MTRR: 0 base 0x0000000000000000 mask 0x00003fff80000000 type 6
MTRR: 1 base 0x0000000077000000 mask 0x00003fffff000000 type 0
MTRR: 2 base 0x0000000078000000 mask 0x00003ffff8000000 type 0
MTRR: 3 base 0x0000000090000000 mask 0x00003ffff0000000 type 1
MTRR: 4 base 0x0000000100000000 mask 0x00003fff00000000 type 6
MTRR: 5 base 0x0000000200000000 mask 0x00003ffe00000000 type 6
MTRR: 6 base 0x0000000400000000 mask 0x00003ffc00000000 type 6
MTRR: 7 base 0x0000000800000000 mask 0x00003fff80000000 type 6
MTRR: 8 base 0x000000087fc00000 mask 0x00003fffffc00000 type 0
ADL has 9 variable-range MTRRs, previously 8 of them were used, and
there was no separate entry for the framebuffer, thus leaving the
default MTRR in place of uncached.
Change-Id: I2ae2851248c95fd516627b101ebcb36ec59c29c3
Signed-off-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52522
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Furquan Shaikh <furquan@google.com>
Coverity detects the control flow UNREACHABLE issue for the printk
usage. This change adds rc to keep the smm_module_setup_stub function
call and returns rc after printk usage.
Found-by: Coverity CID 1452602
TEST=None
Signed-off-by: John Zhao <john.zhao@intel.com>
Change-Id: Ie3b90a8197c3b84c5a1dbca8a9ef566bef35c9ab
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52574
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
With this change, the type-unsafe {get,set}_option() API functions are
no longer used directly. The old API gets dropped in a follow-up.
Change-Id: Id3f3e172c850d50a7d2f348b1c3736969c73837d
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52512
Reviewed-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Use the same stack location during relocation as for the permanent
handler.
When the number of CPUs is too large the stacks during relocation
don't fit inside the default SMRAM segment at 0x30000. Currently the
code would just let the CPU stack base grow downwards outside of the
default SMM segment which would corrupt lower memory if S3 is
implemented.
Also update the comment on smm_module_setup_stub().
Change-Id: I6a0a890e8b1c2408301564c22772032cfee4d296
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51186
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Use lapicid api to support both x2apic mode and apic mode
BUG=None
BRANCH=None
TEST=boot to OS and check apic mode
cat /proc/cpuinfo | grep "apicid"
Signed-off-by: Wonkyu Kim <wonkyu.kim@intel.com>
Change-Id: I5ca5b09ae67941adcc07dfafdfe4ba78b0f81009
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51725
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Implement x2apic mode as existing code only supports apic mode.
Use info from LAPIC_BASE_MSR (LAPIC_BASE_MSR_X2APIC_MODE) to check
if apic mode or x2apic mode and implement x2apic mode according to
x2apic specfication.
Reference:
https://software.intel.com/content/www/us/en/develop/download/intel-64-architecture-x2apic-specification.html
BUG=None
BRANCH=None
TEST=boot to OS and check apic mode
cat /proc/cpuinfo | grep "apicid"
ex) can see apicid bigger than 255
apicid : 256
apicid : 260
Signed-off-by: Wonkyu Kim <wonkyu.kim@intel.com>
Change-Id: I0bb729b0521fb9dc38b7981014755daeaf9ca817
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51723
Reviewed-by: Ravishankar Sarawadi <ravishankar.sarawadi@intel.com>
Reviewed-by: Jamie Ryu <jamie.m.ryu@intel.com>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This fixes an issue introduced in
commit ad0116c032
cpu/x86/smm_loaderv2: Remove unused variables
It removed one variable that was needed to set the SMM start address
that is used to set the SMM stack location.
Change-Id: Iddf9f204db54f0d97a90bb423b65db2f7625217f
Signed-off-by: Marc Jones <marcjones@sysproconsulting.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51721
Reviewed-by: Jay Talbott <JayTalbott@sysproconsulting.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Global variables are located in .bss and not on the CPU stack.
Overwriting them a per CPU case is bound to cause race conditions. In
this case it is even just plainly wrong.
Note: This variable is set up in the get_smm_info() function.
Change-Id: Iaef26fa996f7e30b6e4c4941683026b8a29a5fd1
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51184
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
The argument provided to the function was always the same as the one
computed inside the function so drop the argument.
Change-Id: I14abf400dce1bd9b03e401b6619a0500a650fa0e
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51527
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
The permanent handler module argument 'save_state_size' now holds the
meaning of the real save state size which is then substracted from the
CPUs save state 'top' to get the save state base.
TESTED with qemu Q35 on x86_64 where the stub size exceeds the AMD64
save state size.
Change-Id: I55d7611a17b6d0a39aee1c56318539232a9bb781
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50770
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Remove variables that are either constants or are just assigned but
not used.
Change-Id: I5d291a3464f30fc5d9f4b7233bde575010275973
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50784
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
With the smm_module_loaderv2 the save state map is not linear so copy
a map from ramstage into the smihandler.
TESTED on QEMU q35: Both SMMLOADER V1 and V2 handle save states properly.
Change-Id: I31c57b59559ad4ee98500d83969424e5345881ee
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50769
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
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>
Broadwell can now use the Haswell CPU driver.
Change-Id: I36138cab72b1e3ad0ff7f6434996f5ce00de9d0d
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46942
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Backport commit 55228ba4b4 (broadwell: Changes from 2.2.0 ref code) to
Haswell, to eventually migrate Broadwell to use the same Haswell code.
Change-Id: I03d9ff16bcaab9091bd723ce933aa3f2d71e29b9
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46921
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Broadwell code unconditionally enables timed MWAIT, but not all Haswell
steppings support it. In preparation for merging Haswell and Broadwell,
also enable timed MWAIT on Haswell code, but only if it is supported.
Change-Id: I1d11d62f1801d65ae4d5623994fd55fd35e8f34a
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46916
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The `mobile` suffix is misleading, since desktop CPUs share the same
CPUIDs. Remove unused stepping IDs and add the full CPUIDs instead.
Finally, add Broadwell CPUIDs in preparation for merging CPU code.
Note that steppings for Haswell in various comments are incorrect.
Tested with BUILD_TIMELESS=1, Asrock B85M Pro4 remains identical.
Change-Id: I19e56b8826b1514550ae95e6363b0df2d08e3cb7
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46915
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Backport Broadwell's s0ix support to Haswell in preparation to unify
both platforms' CPU code. Note that only ULT variants support s0ix.
This option is currently unused, but will be put to use in subsequent
commits, when switching Broadwell mainboards to use Haswell's CPU code.
Tested with BUILD_TIMELESS=1, Asrock B85M Pro4 remains identical.
Change-Id: I91c6f937c09c9254a6f698f3a6fb6366364e3b2b
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46924
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Increase DCACHE_RAM_SIZE to 32kB and remove "NO_CBFS_MCACHE".
It’s quite safe to increase DCACHE_RAM_SIZE. All LGA775 targets
should have at least 256K L2 cache. That is plenty for XIP RO cache of
bootblock + romstage and a 32K CAR.
Change-Id: I393b2727bd90a990c3108a4dbead62b17d7fc531
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49505
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Writing 0 to MSR IA32_BIOS_SIGN_ID before fetching this MSRs content
is required. This is how things are done in
cpu/intel/microcode/microcode.c.
The "Intel® 64 and IA-32 Architectures Software Developer’s Manual"
also recommends this: "It is recommended that this field be preloaded
with 0 prior to executing CPUID" (this field being %edx).
Change-Id: I24a87aff9a699ed8ab2598007c8b8562d0555ac5
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49670
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jonathan Zhang <jonzhang@fb.com>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
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>
There's no need to have them in the devicetree. ACPI generation can now
be simplified even further, and is done in subsequent commits.
Change-Id: I3a788423aee9be279797a1f7c60ab892a0af37e7
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46908
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The caches have already been enabled during MP-init,
so these function calls are redundant. Remove them.
Change-Id: Ia9be1a3388d8e7c73c35a1c68b3dd5bc488658c2
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49383
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Fix C code to match comment and assembly implementation.
Tested on Prodrive hermes:
The microcode spinlock is no longer used.
Change-Id: I21441299f538783551d4d5ba2b2e7567e152d718
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49304
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This change affects Intel CPUs only. As most platforms are doing
uCode update using FIT, they aren't affected by this code either.
Update microcode in MP-init using a single spinlock when running on
a Hyper-Threading enabled CPU on pre FIT platforms.
This will slow down the MP-init boot flow.
Intel SDM and various BWGs specify to use a semaphore to update
microcode on one thread per core on Hyper-Threading enabled CPUs.
Due to this complex code would be necessary to determine the core #ID,
initializing and picking the right semaphore out of CONFIG_MAX_CPUS / 2.
Instead use the existing global spinlock already present in MPinit code.
Assuming that only pre-FIT platforms with Hyper-Threading enabled and at
most 8 threads will ever run into this condition, the boot delay is
negligible.
This change is a counterproposal to the previous published patch series
being much more unsophisticated.
Change-Id: I27bf5177859c12e92d6ce7a2966c965d7262b472
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49303
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
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>
Get rid of using eax and reload counter on race condition.
Change-Id: Ie4b9957d8aa1f272ff1db5caf2c69d1e1f086a03
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47714
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Christian Walter <christian.walter@9elements.com>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Commit 542307b815 (broadwell: Add small delay before Flex Ratio reboot)
introduced a workaround for Broadwell. Implement it on Haswell as well.
Since this is only necessary when a TPM is present on a system, only do
the delay (which is not that small, to be honest) on TPM-enabled builds.
Change-Id: Id8b58e9fa2a1c81989305f5b4b765b82c01e1596
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46941
Reviewed-by: Nico Huber <nico.h@gmx.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
