Age | Commit message (Collapse) | Author |
|
Previously cpu_index() always succeeded, but since commit 095c931
(src/arch/x86: Use core apic id to get cpu_index()) it is now possible
for it to indicate an error by returning -1. This commit adds error
handling for all calls to cpu_index(), and restores several checks that
were removed in commit 7c712bb (Fix code that would trip -Wtype-limits)
but are now needed.
Signed-off-by: Jacob Garber <jgarber1@ualberta.ca>
Change-Id: I5436eed4cb5675f916924eb9670db04592a8b927
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32795
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
|
|
This patch ensures start_cpu() function to store default_apic_id using
common cpu_add_map_entry() function to make cpu_index() implementation
generic.
BRANCH=none
BUG=b:79562868
Change-Id: Iac4d6e9e6e6f9ba644335b4b70da8689c405f638
Signed-off-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32722
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
|
|
This patch renames mp_get_apic_id() to cpu_get_apic_id() and
add_cpu_map_entry() to cpu_add_map_entry() in order access it
outside CONFIG_PARALLEL_MP kconfig scope.
Also make below changes
- Make cpu_add_map_entry() function available externally to call
it from mp_init.c and lapic_cpu_init.c.
BRANCH=none
BUG=b:79562868
Change-Id: I6a6c85df055bc0b5fc8c850cfa04d50859067088
Signed-off-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32701
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
|
|
Checking for empty MTRR_DEF_TYPE_MSR as a proxy for proper CPU reset
is common across multiple platforms. Therefore place it in a common
location.
Change-Id: I81d82fb9fe27cd9de6085251fe1a5685cdd651fc
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32319
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-by: Furquan Shaikh <furquan@google.com>
|
|
Also, extra-lines added or removed and local includes moved down.
Change-Id: I5e739233f3742fd68d537f671642bb04886e3009
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32009
Reviewed-by: Nico Huber <nico.h@gmx.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Drop 'include <string.h>' when it is not used and
add it when it is missing.
Also extra lines removed, or added just before local includes.
Change-Id: Iccac4dbaa2dd4144fc347af36ecfc9747da3de20
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31966
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
|
|
This patch is a raw application of
find src/ -type f | xargs sed -i -e 's/IS_ENABLED\s*(CONFIG_/CONFIG(/g'
Change-Id: I6262d6d5c23cabe23c242b4f38d446b74fe16b88
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31774
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
|
|
Change-Id: I1f44ffeb54955ed660162a791c6281f292b1116a
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31715
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
|
|
This IS_ENABLED(XXX) line should've clearly been IS_ENABLED(CONFIG_XXX).
This patch fixes the issue. Not tested on a real board, but looking at
the affected code paths suggests that this will result in no effective
change anywhere (since CAR should already be torn down by the time this
is called on FSP1.0 boards, so do_car_migrate_variables() would have
immediately exited anyway).
Change-Id: I74e0ed4d04471ee521ff5c69a74a6f4c949e5847
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/31744
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
|
|
Change-Id: I91158452680586ac676ea11c8589062880a31f91
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/31692
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Static scan does not know the contents of the fixed MTRR descriptor, so
it has no way to eval the result for variable num_ranges. If num_ranges
is less or equal to 0, the for loop will not be entered, and the values
of fixed_msrs will not be set. Asserting that num_ranges is greater than
0 ensures the loop enters at least once.
BUG=b:112253891
TEST=build grunt
Change-Id: Ieec0ac432c745bde4b1700539c266625da6cfd77
Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
Reviewed-on: https://review.coreboot.org/c/31527
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
|
|
The SMI handler on qemu returned early, due to missing SMM-Revision
Level support.
Add the ID qemu uses, which is AMD64 compatible for qemu-system-x86_64.
Fixes booting tianocore payload with SMM variable store on qemu.
Change-Id: I978b94150cfc49a39c2a0818eb14a649850e451d
Signed-off-by: Patrick Rudolph <siro@das-labor.org>
Reviewed-on: https://review.coreboot.org/c/31594
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
|
|
When <symbols.h> was first introduced, it only declared a handful of
regions and we didn't expect that too many architectures and platforms
would need to add their own later. However, our amount of platforms has
greatly expanded since, and with them the need for more special memory
regions. The amount of code duplication is starting to get unsightly,
and platforms keep defining their own <soc/symbols.h> files that need
this as well.
This patch adds another macro to cut down the definition boilerplate.
Unfortunately, macros cannot define other macros when they're called, so
referring to region sizes as _name_size doesn't work anymore. This patch
replaces the scheme with REGION_SIZE(name).
Not touching the regions in the x86-specific <arch/symbols.h> yet since
they don't follow the standard _region/_eregion naming scheme. They can
be converted later if desired.
Change-Id: I44727d77d1de75882c72a94f29bd7e2c27741dd8
Signed-off-by: Julius Werner <jwerner@chromium.org>
Reviewed-on: https://review.coreboot.org/c/31539
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
|
|
Add get_ia32_fsb returns FSB values in MHz of intel's CPUs.
Also add get_ia32_fsb_x3 function. It returns round up 3 * get_ia32_fsb.
Change-Id: I232bf88de7ebba6ac5865db046ce79e9b2f3ed28
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/30103
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Followup removes SIPI_VECTOR_IN_ROM and it seems reasonable
enough to force the alignment unconditionally to page size.
Reason for the conditionals is the alignment is not possible
with romcc bootblocks having total size less than 4 kiB.
Change-Id: I0ff2786f80a319ebb3215d4fd696cda3e15c3012
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/30855
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Does not fix 3rdparty/, *.S or *.ld or yet.
Change-Id: I66b48013dd89540b35ab219d2b64bc13f5f19cda
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/17656
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
|
|
When rmodule is loaded CPU stack alignment is only guaranteed
to 4kiB. Implementation of cpu_info() requires that each
CPU sees its stack aligned to CONFIG_STACK_SIZE.
Add one spare CPU for the stack reserve, such that alignment can
be enforced runtime.
Change-Id: Ie04956c64df0dc7bb156002d3d4f2629f92b340e
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/26302
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
This was used to check romcc-built bootblock and romstage
agree about the location of 16-bit entrypoint. There was
no need to customize it as bootblock size requirement did
not grow. Just check for a fixed location at 4 GiB - 4 KiB.
With C_ENVIRONMENT_BOOTBLOCK we can have a proper symbol
for the purpose, since it appears in the same compilation
unit. It will adjust if C_ENV_BOOTBLOCK_SIZE changes.
Change-Id: I93f3c37e78ba587455c804de8c57e7e06832a81f
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/30854
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
|
|
It was already done once in c_start.S.
Change-Id: I1cb0ea25251644dbd1127d177247a02ba52bb550
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/30796
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
|
|
Change-Id: I7a10ddf79cf457b5dde21714b13890fc9510e7ce
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/30377
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tristan Corrick <tristan@corrick.kiwi>
|
|
This function returns APIC id for respective cpu core.
BUG=b:74436746
BRANCH=none
TEST=mp_get_apic_id() can be accessed in other files now.
Change-Id: I5c5eda8325f941ab84d8a3fe0dae64be71c44855
Signed-off-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-on: https://review.coreboot.org/c/25620
Reviewed-by: Aamir Bohra <aamir.bohra@intel.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Required to compile the code in x86_64, even though it's never used.
Change-Id: I2be8ad8805804e4da52bdb02ab43cb833402f999
Signed-off-by: Patrick Rudolph <siro@das-labor.org>
Reviewed-on: https://review.coreboot.org/c/29876
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-by: Justin TerAvest <teravest@chromium.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
|
|
All i82801gx targets now use SMM_TSEG.
Change-Id: Ib4e6974088a685290ed1dddf5228a99918744124
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/25599
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
|
|
This also caches the TSEG region and therefore increases MTRR usage
a little in some cases.
Currently SMRR msr's are not set on model_1067x and model_6fx since
this needs the MSRR enable bit and lock set in IA32_FEATURE_CONTROL.
This will be handled properly in the subsequent parallel mp init
patchset.
Tested on Intel DG41WV, resume from S3 still works fine.
Change-Id: I317c5ca34bd38c3d42bf0d4e929b2a225a8a82dc
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/25597
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
This removes CEIL_DIV and div_round_up() altogether and
replace it by DIV_ROUND_UP defined in commonlib/helpers.h.
Change-Id: I9aabc3fbe7834834c92d6ba59ff0005986622a34
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/29847
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
|
|
No need to hide prompts, it's a user choice anyway, they should know.
The help texts were just rephrasing the prompts or stating the obvious.
Change-Id: I5694a88f2da57af2a20357c4e22c7c648053cc26
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/29802
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Gather x86 specific debug options and deflate their code a little. We
keep their hiding rules and help texts, although they don't seem much
useful.
Change-Id: I3bb8e759fc6a4871d30fccff47babfb7a291b45c
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/29751
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-by: Julius Werner <jwerner@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Initially, I wanted to move only the Kconfig DISPLAY_MTRRS into the
"Debug" menu. It turned out, though, that the code looks rather generic.
No need to hide it in soc/intel/.
To not bloat src/Kconfig up any further, start a new `Kconfig.debug`
hierarchy just for debug options.
If somebody wants to review the code if it's 100% generic, we could
even get rid of HAVE_DISPLAY_MTRRS.
Change-Id: Ibd0a64121bd6e4ab5d7fd835f3ac25d3f5011f24
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/29684
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Change-Id: I801849fb31fe6958e3d9510da50e2e2dd351a98d
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/29304
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
|
|
Change-Id: I17c4fc4e3e2eeef7c720c6a020b37d8f7a0f57a4
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/29300
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
|
|
Change-Id: I9fba67be12483ea5e12ccd34c648735d409bc8b0
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/29252
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
|
|
A stack overrun has been observed on AMD64 CPUs during the SMM module
relocation process. Change the assumed required size from equaling the
save structure's size to a Kconfig symbol.
A value of 0x400 doubles the size used by AMD64 systems and maintains
the size used by EM64T.
BUG=b:118420852
TEST=S3 on Grunt and verify 0x2f000-0x30000 uncorrupted
Change-Id: Ib1d590ee4afb06ca649afd6ad253cdfd969ae777
Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com>
Reviewed-on: https://review.coreboot.org/29277
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
|
|
Change-Id: I15dfe0332fd87db61d692a94bf1fd5d00dfb83d4
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/29105
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
|
|
Use "cpu/x86/msr.h" for common IA-32 MSRs and correct IA-32 MSRs names.
Change-Id: Ida7f2d608c55796abf9452f190a58802e498302d
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/28752
Reviewed-by: Martin Roth <martinroth@google.com>
Reviewed-by: Lijian Zhao <lijian.zhao@intel.com>
Reviewed-by: Pratikkumar V Prajapati <pratikkumar.v.prajapati@intel.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
Its spreading copies got out of sync. And as it is not a standard header
but used in commonlib code, it belongs into commonlib. While we are at
it, always include it via GCC's `-include` switch.
Some Windows and BSD quirk handling went into the util copies. We always
guard from redefinitions now to prevent further issues.
Change-Id: I850414e6db1d799dce71ff2dc044e6a000ad2552
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/28927
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
|
|
As per internal discussion, there's no "ChromiumOS Authors" that's
meaningful outside the Chromium OS project, so change everything to the
contemporary "Google LLC."
While at it, also ensure consistency in the LLC variants (exactly one
trailing period).
"Google Inc" does not need to be touched, so leave them alone.
Change-Id: Ia0780e31cdab879d2aaef62a2f0403e3db0a4ac8
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-on: https://review.coreboot.org/28756
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Joel Kitching <kitching@google.com>
|
|
Add a __always_inline macro that wraps __attribute__((always_inline))
and replace current users with the macro, excluding files under
src/vendorcode.
Change-Id: Ic57e474c1d2ca7cc0405ac677869f78a28d3e529
Signed-off-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-on: https://review.coreboot.org/28587
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@google.com>
|
|
In procedure smm_load_module, variables fxsave_size and fxsave_area are set
to 0 and NULL, but if CONFIG_SSE is enabled, they are overwritten. Change
the code setting the value to an "else" of the "if" testing CONFIG_SSE, thus
avoiding static analysis error.
BUG=b:112253891
TEST=Build and boot grunt.
Change-Id: I3042b037017a082378b53ee419c769c535632038
Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
Reviewed-on: https://review.coreboot.org/28023
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
|
|
Within procedure save_bsp_msrs, the structure pointer "msr_entry" is updated
every time procedure save_msr() is called. However, after the last call of
save_msr(), "msr_entry" is not used, thus causing a static analysis error.
Add a "(void)msr_entry;" at the end to avoid the static analysis error.
BUG=b:112253891
TEST=Build and boot grunt.
Change-Id: If0fb336fbf49eec3da255fadbe38b3a38768d0cf
Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
Reviewed-on: https://review.coreboot.org/27956
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
|
|
Change-Id: I13dec72b2de2a525d45909e697c33fbdc31111cb
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/27918
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
|
|
In procedure allocate_cpu_devices(), if structure pointer new is null skip
using the pointer. Add a "continue;" to skip using the pointer.
The issue was found by static analysis tool.
BUG=b:112253891
TEST=Build and boot grunt.
Change-Id: I7011fbfa0725f22a6dfbca6752e668eddac3463c
Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com>
Reviewed-on: https://review.coreboot.org/27951
Reviewed-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-by: Martin Roth <martinroth@google.com>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
BUG=none
TEST=none
Change-Id: Ic2d7bf5f5335894ede98dc7e6cc6a65e4897e487
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Reviewed-on: https://review.coreboot.org/27811
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
|
|
timer_monotonic_get() was only compiled in a !__PRE_RAM__ environment.
Clean up the code paths by employing CAR_GLOBAL for the global state
which allows the same code to be used in all stages.
Change-Id: I08fd1795508f76abdab1618585366bf9d06482ff
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/27801
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
If CPU 0's stack grows to large, it will overflow into CPU 1's stack.
If CPU 0 is handling the interrupt then CPU 1 should be in an idle loop.
When the stack overflow occurs it will override the return pointer for
CPU 1, so when CPU 0 unlocks the SMI lock, CPU 1 will attempt to return
to a random address.
This method is not foolproof. If code allocates some stack variables
that overlap with the canary, and if the variables are never set, then
the canary will not be overwritten, but it will have been skipped. We
could mitigate this by adding a larger canary value if we wanted.
I chose to use the stack bottom pointer value as the canary value
because:
* It will change per CPU stack.
* Doesn't require hard coding a value that must be shared between the
.S and .c.
* Passing the expected canary value as a parameter felt like overkill.
We can explore adding other methods of signaling that a stack overflow
had occurred in a follow up. I limited die() to debug only because
otherwise it would be very hard to track down.
TEST=built on grunt with a small and large stack size. Then verified
that one causes a stack overflow and the other does not.
Stack overflow message:
canary 0x0 != 0xcdeafc00
SMM Handler caused a stack overflow
Change-Id: I0184de7e3bfb84e0f74e1fa6a307633541f55612
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Reviewed-on: https://review.coreboot.org/27229
Reviewed-by: Martin Roth <martinroth@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
%edx gets clobbered before the c handler is invoked. This is just a
cleanup cl to make the next cl look clean.
BUG=b:80539294
TEST=verified SMI still works on grunt.
Change-Id: I21bf41ed4fdeaaa8737c883f202a39cb57c2b517
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Reviewed-on: https://review.coreboot.org/27228
Reviewed-by: Martin Roth <martinroth@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
When generating a backtrace we need an indicator when we have hit the
beginning of the stack. The i386 ABI states that %ebp points to the next
stack frame. NULL can be used to indicate the end of the stack.
We could add a NULL return pointer at %ebp+4, but I decided to omit it
since a NULL stack pointer can be used as an indicator that there is no
return pointer.
BUG=b:80539294
TEST=built and tested on grunt
Change-Id: I8a48114d31a5c716335d264fa4fe4da41dc5bf11
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Reviewed-on: https://review.coreboot.org/27226
Reviewed-by: Martin Roth <martinroth@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
This reduces the cognitive overhead of referencing locals via esp since
it changes with every push.
BUG=b:80539294
TEST=built and booted on grunt.
Change-Id: Ib7eb98ce3483d4fc803696c1b2496d8384317536
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Reviewed-on: https://review.coreboot.org/27225
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
|
|
This makes it easier to spot unused RAM.
BUG=b:80539294
TEST=built and tested on grunt. Verified unused memory was set to 0xcdcdcdcd
Change-Id: I335eaf642bd8526f31819eaac95ce80c2df3c300
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Reviewed-on: https://review.coreboot.org/27227
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
|
|
Stoneyridge is running into a stack overflow in the SMM handler.
BUG=b:80539294
TEST=built on grunt
Change-Id: I94e385497bd93c3638c69fb08d9b843c3bbb55ce
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Reviewed-on: https://review.coreboot.org/27034
Reviewed-by: Martin Roth <martinroth@google.com>
Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
|
|
All boards except those with NO_RELOCATABLE_RAMSTAGE
or explicit select already had this feature built.
Change-Id: I838e12141243ec49c2555c09269e07476eb0cfad
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/26816
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
|