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>
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>
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>
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>
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>
Implement x2apic mode as existing code only supports apic mode.
Use info from LAPIC_BASE_MSR (LAPIC_BASE_MSR_X2APIC_MODE) to check
if apic mode or x2apic mode and implement x2apic mode according to
x2apic specfication.
Reference:
https://software.intel.com/content/www/us/en/develop/download/intel-64-architecture-x2apic-specification.html
BUG=None
BRANCH=None
TEST=boot to OS and check apic mode
cat /proc/cpuinfo | grep "apicid"
ex) can see apicid bigger than 255
apicid : 256
apicid : 260
Signed-off-by: Wonkyu Kim <wonkyu.kim@intel.com>
Change-Id: I0bb729b0521fb9dc38b7981014755daeaf9ca817
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51723
Reviewed-by: Ravishankar Sarawadi <ravishankar.sarawadi@intel.com>
Reviewed-by: Jamie Ryu <jamie.m.ryu@intel.com>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Adapt the old lapic init code for x86_64.
Change-Id: I5128ed574323025e927137870fb10b23d06bc01d
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48221
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Clarify what the function does by renaming it from do_lapic_init() to
lapic_virtual_wire_mode_init().
Change-Id: Ie4430bf0f6c6bf0081b6aaeace351092bcf7f4ac
Signed-off-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47020
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>
The assumption up to this point was that if the system had an x86
processor, verstage would be running on the x86 processor. With running
verstage on the PSP, that assumption no longer holds true, so exclude
pieces of code that cause problems for verstage on the PSP.
This change will add these files to verstage only if the verstage
architecture is X86 - either 32 or 64 bit.
BUG=b:158124527
TEST=Build and boot on Trembyle
Signed-off-by: Martin Roth <martin@coreboot.org>
Change-Id: I797b67394825172bd44ad1ee693a0c509289486b
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42062
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Eric Peers <epeers@google.com>
Reviewed-by: Rob Barnes <robbarnes@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Most LAPIC registers are 32bit, and thus the use of long is valid on
x86_32, however it doesn't work on x86_64.
* Don't use long as it is 64bit on x86_64, which breaks interrupts
in QEMU and thus SeaBIOS wouldn't time out the boot menu
* Get rid of unused defines
* Get rid of unused atomic xchg code
Tested on QEMU Q35 with x86_64 enabled: Interrupts work again.
Tested on QEMU Q35 with x86_32 enabled: Interrupts are still working.
Tested on Lenovo T410 with x86_64 enabled.
Change-Id: Iaed1ad956d090625c7bb5cd9cf55cbae16dd82bd
Signed-off-by: Patrick Rudolph <siro@das-labor.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36777
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This reverts commit aac79e0b8f.
Reason for revert: This massively slows down the boot process because
the LAPIC delivery mode for the APs is not set anymore. Plus, not all
review comments were fully addressed, yet this got merged in anyway.
Change-Id: If9bae6aae0d4d1f21b067a7d970975193c2b16d5
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42166
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
When Linux is booted, the kernel reports
"do_IRQ: 1.55 No irq handler for vector"
So far it comes with payloads SeaBIOS and depthcharge, not with
Grub. We assume Grub does something to avoid this problem.
AMD bug tracker system (JIRA PLAT-21393) says the APs can not be set
EXTINT delivery mode.
In Intel 64 and IA-32 Architectures Software Developer’s Manual volume
3A, see chapter 10.5.1 Local Vector Table, it says:
"The APIC architecture supports only one ExtINT source in a system,
usually contained in the compatibility bridge. Only one processor in the
system should have an LVT entry configured to use the ExtINT delivery
mode."
Tested on mandolin (Picasso) board, the error in dmesg is gone.
The bug 153677727 has two parts.
1. Soft lockup
2. do_IRQ 1.55.
The soft lockup issued has been fixed by
https://review.coreboot.org/c/coreboot/+/41128
BUG=b:153677727
TEST=mandolin
Change-Id: I2956dcaad87cc1466deeca703748de33390b7603
Signed-off-by: Zheng Bao <zheng.bao@amd.com>
Signed-off-by: Zheng Bao <fishbaozi@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42219
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Stefan thinks they don't add value.
Command used:
sed -i -e '/file is part of /d' $(git grep "file is part of " |egrep ":( */\*.*\*/\$|#|;#|-- | *\* )" | cut -d: -f1 |grep -v crossgcc |grep -v gcov | grep -v /elf.h |grep -v nvramtool)
The exceptions are for:
- crossgcc (patch file)
- gcov (imported from gcc)
- elf.h (imported from GNU's libc)
- nvramtool (more complicated header)
The removed lines are:
- fmt.Fprintln(f, "/* This file is part of the coreboot project. */")
-# This file is part of a set of unofficial pre-commit hooks available
-/* This file is part of coreboot */
-# This file is part of msrtool.
-/* This file is part of msrtool. */
- * This file is part of ncurses, designed to be appended after curses.h.in
-/* This file is part of pgtblgen. */
- * This file is part of the coreboot project.
- /* This file is part of the coreboot project. */
-# This file is part of the coreboot project.
-# This file is part of the coreboot project.
-## This file is part of the coreboot project.
--- This file is part of the coreboot project.
-/* This file is part of the coreboot project */
-/* This file is part of the coreboot project. */
-;## This file is part of the coreboot project.
-# This file is part of the coreboot project. It originated in the
- * This file is part of the coreinfo project.
-## This file is part of the coreinfo project.
- * This file is part of the depthcharge project.
-/* This file is part of the depthcharge project. */
-/* This file is part of the ectool project. */
- * This file is part of the GNU C Library.
- * This file is part of the libpayload project.
-## This file is part of the libpayload project.
-/* This file is part of the Linux kernel. */
-## This file is part of the superiotool project.
-/* This file is part of the superiotool project */
-/* This file is part of uio_usbdebug */
Change-Id: I82d872b3b337388c93d5f5bf704e9ee9e53ab3a9
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41194
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This change moves all ACPI table support in coreboot currently living
under arch/x86 into common code to make it architecture
independent. ACPI table generation is not really tied to any
architecture and hence it makes sense to move this to its own
directory.
In order to make it easier to review, this change is being split into
multiple CLs. This is change 3/5 which basically is generated by
running the following command:
$ git grep -iIl "arch/acpi" | xargs sed -i 's/arch\/acpi/acpi\/acpi/g'
BUG=b:155428745
Change-Id: I16b1c45d954d6440fb9db1d3710063a47b582eae
Signed-off-by: Furquan Shaikh <furquan@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40938
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Done with sed and God Lines. Only done for C-like code for now.
Change-Id: I2adf28d805fe248d55a9514f74c38280c0ad9a78
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40049
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Modify CAR setup to work in bootblock. Provide bootblock C file with
necessary C bootblock functions. Additionally chache the ROM and set
the MMCONF base before jumping to bootblock main.
Change-Id: I29916a96f490ff717c69dc7cd565d74a83dbfb0d
Signed-off-by: Michał Żygowski <michal.zygowski@3mdeb.com>
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36914
Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
These were often used to distinguish CAR_GLOBAL variables that weren't
directly usable. Since we're getting rid of this special case, also get
rid of the marker.
This change was created using coccinelle and the following script:
@match@
type T;
identifier old =~ "^(g_.*|.*_g)$";
@@
old
@script:python global_marker@
old << match.old;
new;
@@
new = old
if old[0:2] == "g_":
new = new[2:]
if new[-2:] == "_g":
new = new[:-2]
coccinelle.new = new
@@
identifier match.old, global_marker.new;
@@
- old
+ new
@@
type T;
identifier match.old, global_marker.new;
@@
- T old;
+ T new;
@@
type T;
identifier match.old, global_marker.new;
@@
- T old
+ T new
= ...;
There were some manual fixups: Some code still uses the global/local
variable naming scheme, so keep g_* there, and some variable names
weren't completely rewritten.
Change-Id: I4936ff9780a0d3ed9b8b539772bc48887f8d5eed
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37358
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
As preparation for x86_64 clean the assembly code and introduce
arch/ram_segs.h similar to existing arch/rom_segs.h.
Replace open coded segment values with the defines from the new
header.
Change-Id: Ib006cd4df59951335506b8153e9347450ec3403e
Signed-off-by: Patrick Rudolph <siro@das-labor.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36321
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Also:
add some missing includes spotted by Jenkins.
Including <types.h>, is supposed to provide stdint and stddef.
Change-Id: I7bd999b59d1c0bdfa5999bf5805576f94c9a2390
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/33680
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
As discussed on the mailing list and voted upon, the coreboot project
is going to move the majority of copyrights out of the headers and into
an AUTHORS file. This will happen a bit at a time, as we'll be unifying
license headers at the same time.
Signed-off-by: Martin Roth <martin@coreboot.org>
Change-Id: Id6070fb586896653a1e44951a6af8f42f93b5a7b
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35184
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Added missing new line to Info Output.
Change-Id: Ic4cd63f231de918fad7cd34724651bf8eb1c8e62
Signed-off-by: Christian Walter <christian.walter@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/32987
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
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 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>
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>
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>
