Instead of defining NUM_FIXED_MTRRS in both cpu/x86/mp_init.h and
cpu/x86/mtrr/mtrr.c in two different ways that will evaluate to the same
value, define it once in include/cpu/x86/mtrr.h which is included in
both C files.
TEST=Timeless build for amd/mandolin results in identical firmware image
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I71cec61e22f5ce76baef21344c7427be29f193f8
Reviewed-on: https://review.coreboot.org/c/coreboot/+/67774
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Martin Roth <martin.roth@amd.corp-partner.google.com>
AMD CPUs have a convenient MSR that allows to set the SMBASE in the save
state without ever entering SMM (e.g. at the default 0x30000 address).
This has been a feature in all AMD CPUs since at least AMD K8. This
allows to do relocation in parallel in ramstage and without setting up a
relocation handler, which likely results in a speedup. The more cores
the higher the speedup as relocation was happening sequentially. On a 4
core AMD picasso system this results in 33ms boot speedup.
TESTED on google/vilboz (Picasso) with CONFIG_SMI_DEBUG: verify that SMM
is correctly relocated with the BSP correctly entering the smihandler.
Change-Id: I9729fb94ed5c18cfd57b8098c838c08a04490e4b
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/64872
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Since mono_time is now 64-bit, the utility functions interfacing with
mono_time should also be 64-bit so precision isn't lost.
Fixed build errors related to printing the now int64_t result of
stopwatch_duration_[m|u]secs in various places.
BUG=b:237082996
BRANCH=All
TEST=Boot dewatt
Change-Id: I169588f5e14285557f2d03270f58f4c07c0154d5
Signed-off-by: Rob Barnes <robbarnes@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/66170
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Yu-Ping Wu <yupingso@google.com>
The do_smm struct element in the mp_state struct was an int even though
it only had two possible states, so change it to bool to make this more
obvious. Also change the return type of is_smm_enabled from int to bool.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I8d2d95f0497649d67565243d14a5ab9c9cdda412
Reviewed-on: https://review.coreboot.org/c/coreboot/+/65776
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
MTRR is a core level register which means 2 threads in one core share
same MTRR. There is a race condition could happen that AP overrides
BSP MTRR unintentionally.
In order to prevent such race condition between BSP and APs, this
patch provides a function to let BSP assign tasks to all APs and wait
them to complete the assigned tasks.
BUG=b:225766934
Change-Id: I8d1d49bca410c821a3ad0347548afc42eb860594
Signed-off-by: Kane Chen <kane.chen@intel.corp-partner.google.com>
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/63566
Reviewed-by: Subrata Banik <subratabanik@google.com>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Implementation for setup_lapic() did two things -- call
enable_lapic() and virtual_wire_mode_init().
In PARALLEL_MP case enable_lapic() was redundant as it
was already executed prior to initialize_cpu() call.
For the !PARALLEL_MP case enable_lapic() is added to
AP CPUs.
Change-Id: I5caf94315776a499e9cf8f007251b61f51292dc5
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58387
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
The options X2APIC_ONLY and X2APIC_RUNTIME were already user-visible
choices in menuconfig, but the functionality was not actually provided
except for platforms where FSP presumably enabled X2APIC.
Add the logic and related logging for switching to X2APIC operation.
TEST: qemu-system-x86_64 -M Q35 -accel kvm -bios coreboot.rom -serial
stdio -smp 2
PARALLEL_MP, and either X2APIC_ONLY or X2APIC_RUNTIME, need to be
selected for the build of emulation/qemu-q35.
Change-Id: I19a990ba287d21ccddaa64601923f1c4830e95e9
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55262
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Wonkyu Kim <wonkyu.kim@intel.com>
Fixes commit 29c7622 ("cpu/x86/mp_init.c: Fix building with no
smihandler") broke SMM init because is_smm_enable() was called before
smm_enable.
Rework the code a little to make it clear what codepaths are used with
CONFIG_HAVE_SMI_HANDLER.
TESTED: now prodrive/hermes boots again.
Change-Id: If4ce0dca2f29754d131dacf2da63e946be9a7b6d
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59912
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Subrata Banik <subrata.banik@intel.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This patch renames X86_AMD_INIT_SIPI Kconfig to leverage
the same logic (to skip 2nd SIPI and reduce delay between
INIT and SIPI while perform AP initialization) even on
newer Intel platform.
Change-Id: I7a4e6a8b1edc6e8ba43597259bd8b2de697e4e62
Signed-off-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56651
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Each CPU/SoC checks the return value of the mp_init_with_smm and prints
the same error message if it wasn't successful, so move this check and
printk to mp_init_with_smm. For this the original mp_init_with_smm
function gets renamed to do_mp_init_with_smm and a new mp_init_with_smm
function is created which then calls do_mp_init_with_smm, prints the
error if it didn't return CB_SUCCESS and passes the return value of
do_mp_init_with_smm to its caller.
Since no CPU/SoC code handles a mp_init_with_smm failure apart from
printing a message, also add a comment at the mp_init_with_smm call
sites that the code might want to handle a failure.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I181602723c204f3e43eb43302921adf7a88c81ed
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58498
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Using cb_err as return type of mp_run_on_aps, mp_run_on_all_aps,
mp_run_on_all_cpus and mp_park_aps clarifies the meaning of the
different return values. This patch also adds the types.h include that
provides the definition of the cb_err enum and checks the return value
of all 4 functions listed above against the enum values instead of
either checking if it's non-zero or less than zero to handle the error
case.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I4b3f03415a041d3ec9cd0e102980e53868b004b0
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58494
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Using cb_err as return type clarifies the meaning of the different
return values. This patch also adds the types.h include that provides
the definition of the cb_err enum and checks the return value of
mp_init_with_smm against the enum values instead of either checking if
it's non-zero or less than zero to handle the error case.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: Ibcd4a9a63cc87fe176ba885ced0f00832587d492
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58491
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Using cb_err as return type clarifies the meaning of the different
return values. To not change the return types of mp_run_on_aps which is
exposed outside of this compilation unit to keep the scope of this patch
limited, the return value of run_ap_work gets translated to the int
values in mp_run_on_aps. This could also be done by a cast of the
run_ap_work return value to int, but an explicit translation of the
return values should be clearer about what it does there.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: Id346c8edf06229a929b4783498d8c6774f54a8b1
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58490
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Using cb_err as return type clarifies the meaning of the different
return values. To not change the return types of mp_init_with_smm which
is exposed outside of this compilation unit to keep the scope of this
patch limited, the return value of mp_init gets translated to the int
values in mp_init_with_smm. This could also be done by a cast of the
mp_init return value to int, but an explicit translation of the return
values should be clearer about what it does there.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I4129c1db06a877c47cca87782af965b62dcbbdc2
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58489
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
We now pre-populate cpu_info before jumping to the C handler. We no
longer need this parameter.
I moved the stack alignment closer to the actual invocation of the C
handler so it's easier to reason about.
BUG=b:194391185, b:179699789
TEST=Boot guybrush to OS and verify all CPUs still function
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: I8997683b6613b7031784cabf7039a400f0efdea1
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58147
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
This will reduce the number of AP init paths we need to support.
BUG=b:194391185, b:179699789
TEST=Boot guybrush to OS and see all CPUs initialized correctly
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: I05beb591bd7b3a26b6c51c10d4ffd6f8621c12eb
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58146
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
When CONFIG_X86_AMD_INIT_SIPI was set, the second/final SIPI that
afterwards checks if all APs have checked in was skipped and if it got
so far, start_aps returned CB_SUCCESS despite not having checked if all
APs had checked in after the SIPI. This patch makes start_aps skip the
first SIPI in the CONFIG_X86_AMD_INIT_SIPI case so we use the proper
timeouts and error handling for the final and this case only SIPI and
signal the caller an error when not all APs have checked in after the
SIPI.
A timeless build for lenovo/x230 which is a mainboard that doesn't
select X86_AMD_INIT_SIPI results in identical binary, so this doesn't
change the behavior of the !X86_AMD_INIT_SIPI case.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I39438229497c5d9c44dc7e247c7b2c81252b4bdb
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58456
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Since during AP startup it's not guaranteed that no AP console output
will be printed between consecutive printk calls in send_sipi_to_aps,
add a new line character to all printks to make sure to have the outputs
from the APs on separate lines. For consistency also add a final new
line character to the printk call in start_aps.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I3983b8a0e6b272ba5fb2a90a108d17a0c480c8b8
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58454
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
We only ever start and execute threads on the BSP. By explicitly
checking to see if the CPU is the BSP we can remove the dependency on
cpu_info. With this change we can in theory enable threads in all
stages.
BUG=b:194391185, b:179699789
TEST=Boot guybrush to OS and verify coop multithreading still works
Suggested-by: Julius Werner <jwerner@chromium.org>
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: Iea4622d52c36d529e100b7ea55f32c334acfdf3e
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58199
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
There is currently a fundamental flaw in the current cpu_info()
implementation. It assumes that current stack is CONFIG_STACK_SIZE
aligned. This assumption breaks down when performing SMM relocation.
The first step in performing SMM relocation is changing the SMBASE. This
is accomplished by installing the smmstub at 0x00038000, which is the
default SMM entry point. The stub is configured to set up a new stack
with the size of 1 KiB (CONFIG_SMM_STUB_STACK_SIZE), and an entry point
of smm_do_relocation located in RAMSTAGE RAM.
This means that when smm_do_relocation is executed, it is running in SMM
with a different sized stack. When cpu_info() gets called it will be
using CONFIG_STACK_SIZE to calculate the location of the cpu_info
struct. This results in reading random memory. Since cpu_info() has to
run in multiple environments, we can't use a compile time constant to
locate the cpu_info struct.
This CL introduces a new way of locating cpu_info. It uses a per-cpu
segment descriptor that points to a per-cpu segment that is allocated on
the stack. By using a segment descriptor to point to the per-cpu data,
we no longer need to calculate the location of the cpu_info struct. This
has the following advantages:
* Stacks no longer need to be CONFIG_STACK_SIZE aligned.
* Accessing an unconfigured segment will result in an exception. This
ensures no one can call cpu_info() from an unsupported environment.
* Segment selectors are cleared when entering SMM and restored when
leaving SMM.
* There is a 1:1 mapping between cpu and cpu_info. When using
COOP_MULTITASKING, a new cpu_info is currently allocated at the top of
each thread's stack. This no longer needs to happen.
This CL guards most of the code with CONFIG(CPU_INFO_V2). I did this so
reviewers can feel more comfortable knowing most of the CL is a no-op. I
would eventually like to remove most of the guards though.
This CL does not touch the LEGACY_SMP_INIT code path. I don't have any
way of testing it.
The %gs segment was chosen over the %fs segment because it's what the
linux kernel uses for per-cpu data in x86_64 mode.
BUG=b:194391185, b:179699789
TEST=Boot guybrush with CPU_INFO_V2 and verify BSP and APs have correct
%gs segment. Verify cpu_info looks sane. Verify booting to the OS
works correctly with COOP_MULTITASKING enabled.
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: I79dce9597cb784acb39a96897fb3c2f2973bfd98
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57627
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Eric Peers <epeers@google.com>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
Since current AMD SoCs don't need some wait time between INIT and SIPI,
we can skip the 10ms wait there, which improves the boot time a bit.
before: CPU_CLUSTER: 0 init finished in 632 msecs
after: CPU_CLUSTER: 0 init finished in 619 msecs
mpinit still works on Mandolin and all CPU cores show up and are usable.
This also doesn't change the binary in a timeless build for boards/SoCs
that don't select X86_AMD_INIT_SIPI which I verified for lenovo/x230.
BUG=b:193885336
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I1e044776f45021742a88a5e369a74383c1baaab6
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56533
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
The alignment for `struct cpu_info` is wrong on x86_64. c_start.S uses
the `push` instruction when setting up the cpu_info struct. This
instruction will push 8 bytes but `unsigned int` is 4 bytes. By making
it a `size_t` we get the correct size for both x86_32 and x86_64.
BUG=b:179699789
TEST=Boot guybrush to the OS
Suggested-by: Julius Werner <jwerner@chromium.org>
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: I8ef311aaa8333ccf8a5b3f1f0e852bb26777671c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56573
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Trigger mode LAPIC_INT_LEVELTRIG was only used with LAPIC_DM_INIT,
specifically for (obsolete) Init Level De-assert.
Level LAPIC_INT_ASSERT is required to be set for all other delivery
modes other than LAPIC_DM_INIT.
This reverts the two above changes that X2APIC mode support introduced
to the IPI for LAPIC_DM_SMI.
Change-Id: I7264f39143cc6edb7a9687d0bd763cb2703a8265
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55197
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>
