| Age | Commit message (Collapse) | Author |
|---|
|
Currently no smihandler uses heap.
coreboot's heap manager also is quite limited in what it will
free (only the latest alloc). This makes it a bad idea to use it inside
the smihandler, as depending on the alloc usage the heap might actually
be full at some point, breaking the smihandler.
This also reduces the ramstage by 448 bytes on google/vilboz.
Change-Id: I70cd822be17c1efe13c94a9dbd2e1038808b9c56
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/64521
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Nico Huber <nico.h@gmx.de>
|
|
Setting up postcar MTRRs is done when invd is already called so there
is no reason to do this in assembly anymore.
This also drops the custom code for Quark to set up MTRRs.
TESTED on foxconn/g41m and hermes/prodrive that MTRR are properly set
in postcar & ramstage.
Change-Id: I5ec10e84118197a04de0a5194336ef8bb049bba4
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54299
Reviewed-by: Elyes Haouas <ehaouas@noos.fr>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
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>
|
|
This removes the need for a tool to generate simple identity pages.
Future patches will link this page table directly into the stages on
some platforms so having an assembly file makes a lot of sense.
This also optimizes the size of the page of each 4K page by placing
the PDPE_table below the PDE.
Change-Id: Ia1e31b701a2584268c85d327bf139953213899e3
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/63725
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
|
|
Follow-up patches will add more to this makefile.
Change-Id: I8da6265b4c810e39a67f5ec27e26eeb26e3679a4
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/63758
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Lean Sheng Tan <sheng.tan@9elements.com>
|
|
<arch/cpu.h> is chain included through <cpu/cpu.h>.
Change-Id: I54a837394f67ac2a759907c7212ab947d07338dc
Signed-off-by: Elyes Haouas <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60931
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin L Roth <martinroth@google.com>
|
|
Change-Id: I530bb217bb9a944990232dcf4e08f160b5267512
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55008
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
None of these options make sense on different ARCH.
Change-Id: Ie90ad24ff9013e38c42f10285cc3b546a3cc0571
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/63673
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
The printed address ranges in the tree (resource allocator and even
some MTRR code) usually shows the range inclusive (meaning from start
address to the real end address of the range). Though there is still
some code in the MTRR context which prints the ranges with an exclusive
end. This patch aligns the printing of ranges in the MTRR code to be
consistent among the tree so that the shown end addresses are now
inclusive.
Change-Id: I0ca292f9cf272564cb5ef1c4ea38f5c483605c94
Signed-off-by: Werner Zeh <werner.zeh@siemens.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/63541
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Subrata Banik <subratabanik@google.com>
Reviewed-by: Eric Lai <eric_lai@quanta.corp-partner.google.com>
Reviewed-by: Christian Walter <christian.walter@9elements.com>
|
|
This patch calls into need_restore_mtrr() from the mtrr_use_temp_range
function to set `put_back_original_solution` to discard any temporary
MTRR range prior to boot to payload.
BUG=b:225766934
TEST=Able to build and boot google/brya to verify that
`remove_temp_solution()` is able to discard any temporary MTRR range
before booting to payload.
Signed-off-by: Subrata Banik <subratabanik@google.com>
Change-Id: I2e00ec593847e1eb173d5ac77b15b50342860f89
Reviewed-on: https://review.coreboot.org/c/coreboot/+/63491
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
|
|
`put_back_original_solution` variable in mtrr.c is static, but there is
a need to set put_back_original_solution outside of mtrr.c in order to
let `remove_temp_solution` to drop any temporary MTRRs being set
outside `mtrr_use_temp_range()`, for example: `set_var_mtrr()` function
is used to set MTRRs for the ROM caching.
BUG=b:225766934
TEST=Able to build and boot google/redrix.
Change-Id: Ic6b5683b2aa7398a5e141f710394ab772e9775e7
Signed-off-by: Kane Chen <kane.chen@intel.corp-partner.google.com>
Signed-off-by: Subrata Banik <subratabanik@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/63485
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
|
|
The sinkhole exploit exists in placing the lapic base such that it
messes with GDT. This can be mitigated by checking the lapic MSR
against the current program counter.
Change-Id: I49927c4f4218552b732bac8aae551d845ad7f079
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37289
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
There is no reason to do this in a separate loop.
Change-Id: I7fe9f1004597602147aae72f4b754395b6b527cf
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/63473
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Elyes Haouas <ehaouas@noos.fr>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
This patch delays removing `temporary` MTRR snapshots to avoid conflicts
with other operations attached with same `BS_PAYLOAD_BOOT/BS_ON_EXIT`
boot state.
BUG=b:225766934
TEST=Having variable MTRR snapshot using display_mtrrs() is able to
list only the permanent MTRRs and all temporary MTRRs are removed
prior to boot to payload.
Signed-off-by: Subrata Banik <subratabanik@google.com>
Change-Id: I602dca989745159d013d6573191861b296f5d3ab
Reviewed-on: https://review.coreboot.org/c/coreboot/+/63220
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Eric Lai <eric_lai@quanta.corp-partner.google.com>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
|
|
This patch replaces the implementation that is used to get the number of
variable MTRRs with `get_var_mtrr_count()` function.
BUG=b:225766934
TEST=Able to build and boot google/redrix board to ChromeOS.
Signed-off-by: Subrata Banik <subratabanik@google.com>
Change-Id: I4751add9c45374e60b7a425df87d06f52e6fcb8c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/63219
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Eric Lai <eric_lai@quanta.corp-partner.google.com>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
|
|
This patch migrates a few useful MTRR functions as below from
`earlymtrr.c` file to newly created common stage file `mtrrlib.c`.
1. get_free_var_mtrr
2. set_var_mtrr
3. clear_all_var_mtrr
These functions can be used to perform the MTRR programming from IA
common code SPI driver as `fast_spi.c` without requiring two separate
implementations for early boot stage (till romstage) and for ramstage
onwards.
BUG=b:225766934
TEST=Able to build and boot google/redrix board to ChromeOS.
Signed-off-by: Subrata Banik <subratabanik@google.com>
Change-Id: I2c62a04a36d3169545c3128b4231992ad9b3699d
Reviewed-on: https://review.coreboot.org/c/coreboot/+/63218
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Eric Lai <eric_lai@quanta.corp-partner.google.com>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
|
|
This change provides hooks for the SoC so it can perform any
initialization and cleanup in the SMM handler.
For example, if we have a UART enabled firmware with DEBUG_SMI, the UART
controller could have been powered off by the OS. In this case we need
to power on the UART when entering SMM, and then power it off before we
exit. If the OS had the UART enabled when entering SMM, we should
snapshot the UART register state, and restore it on exit. Otherwise we
risk clearing some interrupt enable bits.
BUG=b:221231786, b:217968734
TEST=Build test guybrush
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: I946619cd62a974a98c575a92943b43ea639fc329
Reviewed-on: https://review.coreboot.org/c/coreboot/+/62500
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
|
|
This change will allow the SMI handler to write to the cbmem console
buffer. Normally SMIs can only be debugged using some kind of serial
port (UART). By storing the SMI logs into cbmem we can debug SMIs using
`cbmem -1`. Now that these logs are available to the OS we could also
verify there were no errors in the SMI handler.
Since SMM can write to all of DRAM, we can't trust any pointers
provided by cbmem after the OS has booted. For this reason we store the
cbmem console pointer as part of the SMM runtime parameters. The cbmem
console is implemented as a circular buffer so it will never write
outside of this area.
BUG=b:221231786
TEST=Boot non-serial FW with DEBUG_SMI and verified SMI messages are
visible when running `cbmem -1`. Perform a suspend/resume cycle and
verify new SMI events are written to the cbmem console log.
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: Ia1e310a12ca2f54210ccfaee58807cb808cfff79
Reviewed-on: https://review.coreboot.org/c/coreboot/+/62355
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
|
|
This will allow to migrate all platform to the parallel_mp init code
and drop the old lapic_init code.
Change-Id: If499e21a8dc7fca18bd5990f833170d0fc21e10c
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/+/58700
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Fix regression after commit 9ec7227c9b
cpu/x86/lapic: Move LAPIC configuration to MP init
The call to disable_lapic() got removed and with asus/p2b
SeaBIOS payload was unable to load kernel.
The combination of entering SeaBIOS payload with an
enabled lapic but not having programmed LAPIC_LVT0
for DELIVERY_MODE_EXTINT apparently disconnects i8259
PIC interrupt delivery pin.
Change-Id: If51e5d65153a02ac7af191e7897c04bd4e298006
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61793
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Elyes Haouas <ehaouas@noos.fr>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
|
|
This reverts commit ceaf959678905f44a54a116f37bd15acab5d4608.
The AMD Picasso SoC doesn't support x2APIC and neither advertises the
presence of its support via bit 21 in EAX of CPUID leaf 1 nor has the
bit 10 in the APIC base address MSR 0x1b set, but it does have 0xd CPUID
leaves, so just checking for the presence of that CPUID leaf isn't
sufficient to be sure that EDX of the CPUID leaf 0xb will contain a
valid APIC ID.
In the case of Picasso EDX of the CPUID leaf 0xb returns 0 for all cores
which causes coreboot to get stuck somewhere at the end of MP init.
I'm not 100% sure if we should additionally check bit 21 in EAX of CPUID
function 1 is set instead of adding back the is_x2apic_mode check.
TEST=Mandolin with a Picasso SoC boots again.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: If1e3c55ce2d048b14c08e06bb79810179a87993d
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61776
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Reviewed-by: Christian Walter <christian.walter@9elements.com>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
|
|
Now that the console system itself will clearly differentiate loglevels,
it is no longer necessary to explicitly add "ERROR: " in front of every
BIOS_ERR message to help it stand out more (and allow automated tooling
to grep for it). Removing all these extra .rodata characters should save
us a nice little amount of binary size.
This patch was created by running
find src/ -type f -exec perl -0777 -pi -e 's/printk\(\s*BIOS_ERR,\s*"ERROR: /printk\(BIOS_ERR, "/gi' '{}' ';'
and doing some cursory review/cleanup on the result. Then doing the same
thing for BIOS_WARN with
's/printk\(\s*BIOS_WARNING,\s*"WARN(ING)?: /printk\(BIOS_WARNING, "/gi'
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I3d0573acb23d2df53db6813cb1a5fc31b5357db8
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61309
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Reviewed-by: Lance Zhao
Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com>
|
|
A lot of soc code requires a definition of apm_control, which
smm/smi_trigger.c provided for !HAVE_SMI_HANDLER, but is not added as
a build target.
Fixes building Q35 without smihandler.
Change-Id: Ie57819b3d169311371a1caca83c9b0c796b46048
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59913
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
|
|
This is just the amount of cpus so rename it for simplicity.
Change-Id: Ib2156136894eeda4a29e8e694480abe06da62959
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58699
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
Both the relocation handler and the permanent handler use the same
stacks, so things can be simplified.
Change-Id: I7bdca775550e8280757a6c5a5150a0d638d5fc2d
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58698
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
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>
|
|
This avoids unnecessary passing of APIC ID parameter and
allows some minor optimisation for X2APIC mode.
Change-Id: I0b0c8c39ecd13858cffc91cc781bea52decf67c5
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/60713
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.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>
|
|
Even when we're not in X2APIC mode, the information in CPUID
leaf 0xb will be valid if that leaf is implemented on the CPU.
Change-Id: I0f1f46fe5091ebeab6dfb4c7e151150cf495d0cb
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58386
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Reviewed-by: Wonkyu Kim <wonkyu.kim@intel.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
Change-Id: I85750282ab274f52bc176a1ac151ef2f9e0dd15d
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58697
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
|
|
Followup will allow use of PARALLEL_MP with SMM_ASEG so
some guards need to be adjusted.
Change-Id: If032ce2be4749559db0d46ab5ae422afa7666785
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61480
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
It was only evaluated on LEGACY_SMP_INIT path while model_106cx
has used PARALLEL_MP for a long time.
Change-Id: I90ce838f1041d55a7c77ca80e563e413ef3ff88d
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61479
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
Change-Id: I7fa1f9402b177a036f08bf99c98a6191c35fa0b5
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61371
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
|
|
With very little changes this code can be used to initialize systems
without SMP. The linker will remove most of the code.
Change-Id: Ia0e8fdf8ed7bc2e0e4ff01be8d3e3c3cb837e6c7
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59692
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
With CPU_INFO_V2 enabled %gs holds the pointer to the cpu_info struct,
so don't clobber it. Backup and restore %gs where possible.
Fixes a crash in MPinit seen after calling FSP-S.
Change-Id: If9fc999b34530de5d8b6ad27b9af25fc552e9420
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59764
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
|
|
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>
|
|
The build fails because smm_stub_size() tries to find a symbol that
won't be present.
Change-Id: I73fee3cf26c0e37cca03299c6730f7b4f1ef6685
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54754
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
|
|
Tested on Qemu/i440fx. The follow-up commit adds a config file to
buildtest it.
Change-Id: Ieeaa85691e4c4516bb51df0e87c4ecaa940810f0
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59694
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
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>
|
|
Change-Id: I88f62c18b814ac0ddd356944359e727d6e3bba5a
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58688
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Mariusz Szafrański <mariuszx.szafranski@intel.com>
|
|
Selecting CPU_X86_CACHE_HELPER only added the x86_enable_cache wrapper
function around enable_cache which additionally wrote a POST code to
port 0x80 and printed a message to the console. This function was only
called during multi-processor initialization in ramstage via the init
function pointer in the CPU's device operations struct and was run on
all cores, so the message on the console was printed once per CPU core.
This patch replaces all x86_enable_cache calls by calls to enable_cache
and removes the wrapper function and the Kconfig symbol
CPU_X86_CACHE_HELPER which was used to only add this when the
corresponding CPUs used the x86_enable_cache wrapper function.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Suggested-by: Angel Pons <th3fanbus@gmail.com>
Change-Id: I5866b6bf014821ff9e3a48052a5eaf69319b003a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58579
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Since all x86 CPUs in tree have MTRR support, there is no need to guard
the DISPLAY_MTRRS option with HAVE_DISPLAY_MTRRS. Also all x86 CPUs/SoCs
have a display_mtrrs call at least somewhere in their code, so selecting
the DISPLAY_MTRRS option will always have an effect. All SoCs that don't
select RESET_VECTOR_IN_RAM have the postcar stage where it gets called.
The two AMD SoCs that select RESET_VECTOR_IN_RAM use the FSP2 driver
which contains plenty of display_mtrrs calls.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I2894689ce58e7404d9d5a894f3c288bc4016ea19
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51575
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
|
|
All x86 CPUs in the coreboot tree have a local APIC, so the
corresponding code can be unconditionally included in the build.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: Ifc354fb386977b0fca4caa72c03aa77a20bc348e
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58551
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
|
|
With using a Kconfig option to add the x86 LAPIC support code to the
build, there's no need for adding the corresponding directory to subdirs
in the CPU/SoC Makefile. Comparing which CPU/SoC Makefiles added
(cpu/)x86/mtrr and (cpu/)x86/lapic before this and the corresponding
MTRR code selection patch and having verified that all platforms
added the MTRR code on that patch shows that soc/example/min86 and
soc/intel/quark are the only platforms that don't end up selecting the
LAPIC code. So for now the default value of CPU_X86_LAPIC is chosen as y
which gets overridden to n in the Kconfig of the two SoCs mentioned
above.
Change-Id: I6f683ea7ba92c91117017ebc6ad063ec54902b0a
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44228
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
|
|
There's no need for relative paths with Kconfig options.
Change-Id: Ib9b9b29a158c34a30480aaabf6d0b23819d28427
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44226
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
|
|
All x86-based CPUs and SoCs in the coreboot tree end up including the
Makefile in cpu/x86/mtrr, so include this directly in the Makefile in
cpu/x86 to add it for all x86 CPUs/SoCs. In the unlikely case that a new
x86 CPU/SoC will be added, a CPU_X86_MTRR Kconfig option that is
selected be default could be added and the new CPU/SoC without MTRR
support can override this option that then will be used in the Makefile
to guard adding the Makefile from the cpu/x86/mtrr sub-directory.
In cpu/intel all models except model 2065X and 206AX are selcted by a
socket and rely on the socket's Makefile.inc to add x86/mtrr to the
subdirs, so those models don't add x86/mtrr themselves. The Intel
Broadwell SoC selects CPU_INTEL_HASWELL and which added x86/mtrr to the
subdirs. The Intel Xeon SP SoC directory contains two sub-folders for
different versions or generations which both add x86/mtrr to the subdirs
in their Makefiles.
Change-Id: I743eaac99a85a5c712241ba48a320243c5a51f76
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44230
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.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>
|
|
Using cb_err as return type clarifies the meaning of the different
return values. Also restructure the implementation of wait_for_aps to
not need a local timeout variable.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I86b8c8b0849ae130c78125b83d159147ce11914c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58488
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. Also restructure the implementation of apic_wait_timeout
to not need a local timeout variable.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I2fe32c761492d252b154d2f50f2a330cf4f412d5
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58487
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.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: Ifb64b5908b938bb162153433e5f744ab0b95c525
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58486
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.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I95f36ba628c7f3ce960a8f3bda730d1c720253cc
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58485
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>
|
|
There are two possible code sections where the cpu_info macros can be
included: .code32 and .code64
Doing a `push %eax` while in a .code64 section will result in a compiler
error. This macro manually pushes the 32-bit register onto the stack so
we can share the code between 32 and 64 bit builds.
We also can't implicitly dereference per_cpu_segment_selector because
it's a 32-bit address. Trying to do this results in the following:
E: Invalid reloc type: 11
E: Illegal use of 32bit sign extended addressing at offset 0x1b2
If we load the address first, then dereference it, we can work around
the limitation.
With these fixes, 64-bit builds can now use CPU_INFO_V2.
BUG=b:179699789
TEST=Boot qemu 64 bit build with CPU_INFO_V2 and 4 CPUs. See AP init
work as expected.
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: I4e72a808c9583bb2d0f697cbbd9cb9c0aa0ea2dc
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58232
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>
|
|
Using cb_err as return type clarifies the meaning of the different
return values.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: Icb96f28b4d59b3d00638a43c927df80f5d1643f9
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58455
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.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>
|
|
Apart from the SIPI number in the debug message the two instances of the
SIPI sending code in start_aps are identical, so factor it out into a
new send_sipi_to_aps function.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I6a921b81fce77fbf58c7ae3b50efd8c3e6e5aef3
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58453
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Using types.h as include instead of stddef.h and stdint.h will also
provide commonlib/bsd/cb_err.h which will be used in follow-up patches.
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: I08a68dc827d60c6c9a27b3ec8b74b9c8a2c96d12
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58452
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
The push/pop of %ebx was only added because smm_stub saves the canary
value in it. Now that we no longer use cpu_info in smm, we no longer
need to save the register.
BUG=b:179699789
TEST=Boot guybrush to the OS
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: I554dbe016db8b1c61246c8ffc7fa252b2542ba92
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58205
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
|
|
Now that cpu_info() is no longer used by COOP_MULTITASKING, we no
longer need to set up cpu_info in SMM. When using CPU_INFO_V2, if
something does manage to call cpu_info() while executing in SMM mode,
the %gs segment is disabled, so it will generate an exception.
BUG=b:179699789
TEST=Boot guybrush to OS with threads enabled
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: Id64f32cc63082880a92dab6deb473431b2238cd0
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58204
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
Reviewed-by: Paul Menzel <paulepanter@mailbox.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>
|
|
Remove code, which was only needed for B and C2 stepping
of P54C. The linux kernel source has commentary on X86_BUG_11AP:
* See if we have a good local APIC by checking for buggy Pentia,
* i.e. all B steppings and the C2 stepping of P54C when using their
* integrated APIC (see 11AP erratum in "Pentium Processor
* Specification Update")
Change-Id: Iec10335f603674bcef2e7494831cf11200795d38
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55199
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
ExtINT is related to external PIC mode i8259 interrupts,
they should be delivered to one CPU (BSP) only.
Change-Id: I78490d2cbe3d9f52e10ef2471508263fd6c146ba
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42434
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>
|
|
The bits REMOTE_IRR and SEND_PENDING are documented as read-only,
and reserved bits should not be modified either.
Change-Id: I6bcb9eb990debe169340a0bfe662158b62a8f4dc
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55700
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
The bit LAPIC_SPIV_ENABLE returns 0 after reset even though
LAPIC has not been temporarily disabled.
Change-Id: Id261bc68fe9d1b1b0e5a3ef599a8f33a686d283b
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55699
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>
|
|
Only the enable_lapic() part is required while doing
SMP init. Also disable_lapic() must not be called if
we rely on LAPIC for timer source.
Change-Id: Ib5e37c1a0a91fa4e9542141aa74f1c1876fee94e
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55261
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>
|
|
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>
|
|
These issues were found and fixed by codespell, a useful tool for
finding spelling errors.
Signed-off-by: Martin Roth <martin@coreboot.org>
Change-Id: I5b8ecdfe75d99028fee820a2034466a8ad1c5e63
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58080
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
The %fs and %gs segment are typically used to implement thread local
storage or cpu local storage. We don't currently use these in coreboot,
so there is no reason to map them. By setting the segment index to 0,
it disables the segment. If an instruction tries to read from one of
these segments an exception will be raised.
The end goal is to make cpu_info() use the %gs segment. This will remove
the stack alignment requirements and fix smm_do_relocation.
BUG=b:194391185, b:179699789
TEST=Boot guybrush to OS
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: Iaa376e562acc6bd1dfffb7a23bdec82aa474c1d5
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57860
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Eric Peers <epeers@google.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
|
|
This will help reduce duplication and make it easier to add new members
to the cpu_info struct.
BUG=b:194391185, b:179699789
TEST=Compare assembly of romstage and ramstage before and after
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: I31f264f4bb8b605fa3cb3bfff0d9bf79224072aa
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57859
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
|
|
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Change-Id: Ic182d7c551932ab6917a81568490ed18acdcd597
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57927
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
It took me a while to understand the SMM set up flow. This adds a
clarifying comment.
BUG=b:194391185, b:179699789
TEST=None
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: I9c73e416b8c583cf870e7a29b0bd7dcc99c2f5f4
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57858
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
|
|
The code under `cpu/x86/tsc` is only compiled in when its `Makefile.inc`
is included from platform (CPU/SoC) code and the `UDELAY_TSC` Kconfig
option is enabled.
Include `cpu/x86/tsc/Makefile.inc` once from `cpu/x86/Makefile.inc` and
drop the now-redundant inclusions from platform code. Also, deduplicate
the `UDELAY_TSC` guards.
Change-Id: I41e96026f37f19de954fd5985b92a08cb97876c1
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57456
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
This patch ensures mp_run_on_all_aps() is passing 'MP_RUN_ON_ALL_CPUS'
macro rather hardcoding `0` while running `func` on all APs.
Change-Id: Icd34371c0d4349e1eefe945958eda957c4794707
Signed-off-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57342
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
|
|
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>
|
|
Tested on Intel Sandybridge x86_64 and x86_32.
Change-Id: I152483d24af0512c0ee4fbbe8931b7312e487ac6
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/44867
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
Expand NO_EARLY_BOOTBLOCK_POSTCODES to all of the early assembly code in
bootblock.
BUG=b:191370340
TEST: Build with & without the option enabled
Signed-off-by: Martin Roth <martinroth@chromium.org>
Change-Id: Idb4a96820d5c391fc17a0f0dcccd519d4881b78c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55731
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
|
|
This commit adds a method called `mainboard_smi_finalize` which provides
a mechanism for a mainboard to execute some code as part of the finalize
method in the SMM stage before SoC does its finalization.
BUG=b:191189275
BRANCH=None
TEST=Implement `mainboard_smi_finalize` on lalala and verify that the
code executes in SMM.
Signed-off-by: Aseda Aboagye <aaboagye@google.com>
Change-Id: If1ee63431e3c2a5831a4656c3a361229acff3f42
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55649
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
|
|
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>
|
|
In x86_64 code every function call consumes 32byte of stack with
no stack local variables being used. That limits the function call depth
in SMM to 32 or less.
Double the stack size to prevent overwriting the stack canary as seen
on HP8200 and x86_64 enabled.
Change-Id: Iee202ba2ae609a474d0eb3b06f49690f33f4eda8
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55449
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
|
|
This fixes a hard to debug hang that could occur in any stage, but in
the end it follows simple rules and is easy to fix.
In long mode the 32bit displacement addressing used on 'mov' and 'lea'
instructions is sign-extended. Those instructions can be found using
readelf on the stage and searching for relocation type R_X86_64_32S.
The sign extension is no issue when either running in protected mode or
the code module and thus the address is below 2GiB. If the address is
greater than 2GiB, as usually the case for code in TSEG, the higher
address bits [64:32] are all set to 1 and the effective address is
pointing to memory not paged. Accessing this memory will cause a page
fault, which isn't handled either.
To prevent such problems
- disable R_AMD64_32S relocations in rmodtool
- add comment explaining why it's not allowed
- use the pseudo op movabs, which doesn't use 32bit displacement addressing
- Print a useful error message if such a reloc is present in the code
Fixes a crash in TSEG and when in long mode seen on Intel Sandybridge.
Change-Id: Ia5f5a9cde7c325f67b12e3a8e9a76283cc3870a3
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55448
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
|
|
Note that there are assumptions about LAPIC MMIO location
in both AMD and Intel sources in coreboot proper.
Change-Id: I2c668f5f9b93d170351c00d77d003c230900e0b4
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55194
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Change-Id: Ia3935524e57885ca79586f1f4612020bb05956ab
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55195
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
|
|
Function is needed with PARALLEL_MP and excluding guard will
be added to the source file.
The incompatibilities with X2APIC_SUPPORT have been fixed
so the exclusion is removed here too.
Change-Id: I5696da4dfe98579a3b37a027966b6758f22574aa
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55193
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
Change-Id: Ib9c404cb55b96dcc5639287c214c5c8f468c0529
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55192
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Change-Id: Ife127d6dc8241cccb9d52236a9152da707f0e261
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/+/55191
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Wonkyu Kim <wonkyu.kim@intel.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Change-Id: I7207a9aadd987b4307ce8b3dd8dbfd47d0a5768e
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/+/55190
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Wonkyu Kim <wonkyu.kim@intel.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
They are not __always_inline and specially enable_lapic()
will become more complex to support X2APIC state changes.
Change-Id: Ic180fa8b36e419aba07e1754d4bf48c9dfddb2f3
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55258
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Wonkyu Kim <wonkyu.kim@intel.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Change-Id: I57c5d85d3098f9d59f26f427fe16829e4e769194
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55187
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Wonkyu Kim <wonkyu.kim@intel.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Allows compile-time optimisation on platforms that do not wish
to enable runtime checking of X2APIC.
Legacy lapic_cpu_init() is incompatible so there is dependency
on PARALLEL_MP. Also stop_this_cpu() is incompatible, so there
is dependency on !AP_IN_SIPI_WAIT.
Since the code actually lacks enablement of X2APIC (apparently
assuming the blob has done it) and the other small flaws pointed
out in earlier reviews, X2APIC_RUNTIME is not selected per
default on any platform yet.
Change-Id: I8269f9639ee3e89a2c2b4178d266ba2dac46db3f
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55073
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Wonkyu Kim <wonkyu.kim@intel.com>
|
|
For the purpose of LAPIC IPI messaging it is not required to
evaluate if IOAPIC is enabled. The necessary enable_lapic()
will still be called as part of setup_lapic() within cpu init.
Change-Id: I8b6a34e39f755452f0af63ae0ced7279747c28fc
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55251
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
|
Change-Id: I7e42519d5bcee95970d366fd64923de874098172
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55189
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
This is for the !PARALLEL_MP paths.
Change-Id: If4b91834a1b6de2a902ab914610ab76c1423f1e9
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55188
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
It was not used, platforms should move away from LEGACY_SMP_INIT
instead of maintaining this.
Change-Id: Id89ec4bb0bdc056ac328f31397e4fab02742e444
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55204
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
|
