Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
cpu/x86/sipi: Add x86_64 support
Enter long mode on secondary APs.
Tested on Lenovo T410 with additional x86_64 patches. Tested on HP Z220 with additional x86_64 patches.
Still boots on x86_32.
Change-Id: I53eae082123d1a12cfa97ead1d87d84db4a334c0 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/x86/64bit/entry64.inc M src/cpu/x86/sipi_vector.S 2 files changed, 25 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/87/45187/1
diff --git a/src/cpu/x86/64bit/entry64.inc b/src/cpu/x86/64bit/entry64.inc index 65c0fdc..7025517 100644 --- a/src/cpu/x86/64bit/entry64.inc +++ b/src/cpu/x86/64bit/entry64.inc @@ -16,7 +16,12 @@ #endif
#include <cpu/x86/msr.h> +#if defined(__RAMSTAGE__) +#include <arch/ram_segs.h> +#else #include <arch/rom_segs.h> +#endif +
setup_longmode: /* Get page table address */ @@ -42,7 +47,12 @@ movl %eax, %cr0
/* use long jump to switch to 64-bit code segment */ +#if defined(__RAMSTAGE__) + ljmp $RAM_CODE_SEG64, $__longmode_start +#else ljmp $ROM_CODE_SEG64, $__longmode_start + +#endif .code64 __longmode_start:
diff --git a/src/cpu/x86/sipi_vector.S b/src/cpu/x86/sipi_vector.S index ba1ecb7..cde5532 100644 --- a/src/cpu/x86/sipi_vector.S +++ b/src/cpu/x86/sipi_vector.S @@ -5,6 +5,8 @@ #include <cpu/x86/msr.h> #include <arch/ram_segs.h>
+#define __RAMSTAGE__ + /* The SIPI vector is responsible for initializing the APs in the system. It * loads microcode, sets up MSRs, and enables caching before calling into * C code. */ @@ -192,11 +194,24 @@ mov %eax, %cr4 #endif
+#ifdef __x86_64__ + /* entry64.inc preserves ebx. */ +#include <cpu/x86/64bit/entry64.inc> + + mov %rsi, %rdi /* cpu_num */ + + movl c_handler, %eax + call *%rax +#else /* c_handler(cpu_num), preserve proper stack alignment */ sub $12, %esp push %esi /* cpu_num */ + mov c_handler, %eax call *%eax +#endif + + halt_jump: hlt jmp halt_jump
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
Patch Set 1: Code-Review+2
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S File src/cpu/x86/sipi_vector.S:
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S@2... PS1, Line 203: eax rax ?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S File src/cpu/x86/sipi_vector.S:
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S@2... PS1, Line 203: eax
rax ?
c_handler is a long
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S File src/cpu/x86/sipi_vector.S:
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S@2... PS1, Line 203: eax
c_handler is a long
If this is compiled as 64 bits, shouldn't a long/int be 64 bits?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S File src/cpu/x86/sipi_vector.S:
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S@2... PS1, Line 203: eax
If this is compiled as 64 bits, shouldn't a long/int be 64 bits?
nm that's .quad I guess
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S File src/cpu/x86/sipi_vector.S:
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S@2... PS1, Line 203: eax
nm that's . […]
It might be a good idea to have some sort of macro to use the correct pointer size for relocatable module parameters. Maybe the.section ".module_parameters" in C code is also an option?
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S File src/cpu/x86/sipi_vector.S:
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S@2... PS1, Line 203: eax
It might be a good idea to have some sort of macro to use the correct pointer size for relocatable module parameters. Maybe the.section ".module_parameters" in C code is also an option?
I kind of like moving .module_parameters into C, where stdint.h makes this trivial
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S File src/cpu/x86/sipi_vector.S:
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S@2... PS1, Line 203: eax
It might be a good idea to have some sort of macro to use the correct pointer size for relocatable […]
Not sure what you mean. The sipi module only contains one source file, that is sipi_vector.S. There's no C code here.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S File src/cpu/x86/sipi_vector.S:
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S@2... PS1, Line 203: eax
Not sure what you mean. The sipi module only contains one source file, that is sipi_vector.S. […]
It would be possible to define the .module_parameters section (the ap_start_params structure) in C instead of ASM, which would allow you to use uintptr_t for the pointer values, instead of having two definitions (32 & 64 bit), or a macro to choose the correct pointer size. That's all I meant 😊 not req'd, just possible.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S File src/cpu/x86/sipi_vector.S:
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S@2... PS1, Line 203: eax
It would be possible to define the . […]
I've run some tests and it's not possible to access C structs from assembly direclty. You have to "duplicate" the layout somehow, either by using .struct, hardcoded offsets to sipi_param or a section as it's currently being used.
Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S File src/cpu/x86/sipi_vector.S:
https://review.coreboot.org/c/coreboot/+/45187/1/src/cpu/x86/sipi_vector.S@2... PS1, Line 203: eax
I've run some tests and it's not possible to access C structs from assembly direclty. […]
The parts of the SIPI vector code that access it could be written using inline ASM, which could then access parts of the struct. Not a big deal.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
Patch Set 1:
Also works fine on qemu with CB:48210
Arthur Heymans has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45187 )
Change subject: cpu/x86/sipi: Add x86_64 support ......................................................................
cpu/x86/sipi: Add x86_64 support
Enter long mode on secondary APs.
Tested on Lenovo T410 with additional x86_64 patches. Tested on HP Z220 with additional x86_64 patches.
Still boots on x86_32.
Change-Id: I53eae082123d1a12cfa97ead1d87d84db4a334c0 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/45187 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Arthur Heymans arthur@aheymans.xyz Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/cpu/x86/64bit/entry64.inc M src/cpu/x86/sipi_vector.S 2 files changed, 25 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Angel Pons: Looks good to me, approved Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/cpu/x86/64bit/entry64.inc b/src/cpu/x86/64bit/entry64.inc index 65c0fdc..7025517 100644 --- a/src/cpu/x86/64bit/entry64.inc +++ b/src/cpu/x86/64bit/entry64.inc @@ -16,7 +16,12 @@ #endif
#include <cpu/x86/msr.h> +#if defined(__RAMSTAGE__) +#include <arch/ram_segs.h> +#else #include <arch/rom_segs.h> +#endif +
setup_longmode: /* Get page table address */ @@ -42,7 +47,12 @@ movl %eax, %cr0
/* use long jump to switch to 64-bit code segment */ +#if defined(__RAMSTAGE__) + ljmp $RAM_CODE_SEG64, $__longmode_start +#else ljmp $ROM_CODE_SEG64, $__longmode_start + +#endif .code64 __longmode_start:
diff --git a/src/cpu/x86/sipi_vector.S b/src/cpu/x86/sipi_vector.S index 054f30d..61d9e34 100644 --- a/src/cpu/x86/sipi_vector.S +++ b/src/cpu/x86/sipi_vector.S @@ -5,6 +5,8 @@ #include <cpu/x86/msr.h> #include <arch/ram_segs.h>
+#define __RAMSTAGE__ + /* The SIPI vector is responsible for initializing the APs in the system. It * loads microcode, sets up MSRs, and enables caching before calling into * C code. */ @@ -192,11 +194,24 @@ mov %eax, %cr4 #endif
+#ifdef __x86_64__ + /* entry64.inc preserves ebx. */ +#include <cpu/x86/64bit/entry64.inc> + + mov %rsi, %rdi /* cpu_num */ + + movl c_handler, %eax + call *%rax +#else /* c_handler(cpu_num), preserve proper stack alignment */ sub $12, %esp push %esi /* cpu_num */ + mov c_handler, %eax call *%eax +#endif + + halt_jump: hlt jmp halt_jump
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Patrick Rudolph (Code Review)
-
Tim Wawrzynczak (Code Review)