Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48175 )
Change subject: [WIP]cpu/x86/64bit: Add code to call FSP in protected mode ......................................................................
[WIP]cpu/x86/64bit: Add code to call FSP in protected mode
Change-Id: I22af2d224b546c0be9e7295330b4b6602df106d6 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/cpu/x86/64bit/helper.S M src/cpu/x86/Makefile.inc M src/drivers/intel/fsp2_0/memory_init.c 3 files changed, 80 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/48175/1
diff --git a/src/cpu/x86/64bit/helper.S b/src/cpu/x86/64bit/helper.S new file mode 100644 index 0000000..dd8f1b5 --- /dev/null +++ b/src/cpu/x86/64bit/helper.S @@ -0,0 +1,72 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifdef ENV_X86_64 +.text +.code64 + .section ".text.protected_mode_call", "ax", @progbits + .globl protected_mode_call_2arg +protected_mode_call_2arg: + + push %rbp + mov %rsp, %rbp + /* Preserve registers */ + push %rbx + push %r12 + push %r13 + push %r14 + push %r15 + + /* Arguments to stack */ + push %rdi + push %rsi + push %rdx + push %rcx + + #include <cpu/x86/64bit/exit32.inc> + + movl -48(%ebp), %eax /* Argument count */ + movl -64(%ebp), %edx /* Argument 0 */ + movl -72(%ebp), %ecx /* Argument 1 */ + + /* Align the stack */ + andl $0xFFFFFFF0, %esp + test %eax, %eax + je 1f /* Zero arguments */ + + subl $1, %eax + test %eax, %eax + je 2f /* One argument */ + + /* Two arguments */ + subl $8, %esp + pushl %ecx /* Argument 1 */ + pushl %edx /* Argument 0 */ + jmp 1f +2: + subl $12, %esp + pushl %edx /* Argument 0 */ + +1: + movl -56(%ebp), %ebx /* Function to call */ + call *%ebx + movl %eax, %ebx + + /* Preserves ebx */ + #include <cpu/x86/64bit/entry64.inc> + + /* Place return value in rax */ + movl %ebx, %eax + + /* Restore registers */ + mov -40(%rbp), %r15 + mov -32(%rbp), %r14 + mov -24(%rbp), %r13 + mov -16(%rbp), %r12 + mov -8(%rbp), %rbx + + /* Restore stack pointer */ + mov %rbp, %rsp + pop %rbp + + ret +#endif diff --git a/src/cpu/x86/Makefile.inc b/src/cpu/x86/Makefile.inc index 2f789f7..6d0c4ac 100644 --- a/src/cpu/x86/Makefile.inc +++ b/src/cpu/x86/Makefile.inc @@ -1,4 +1,6 @@ subdirs-y += pae +all-y += 64bit/helper.S + subdirs-$(CONFIG_PARALLEL_MP) += name ramstage-$(CONFIG_PARALLEL_MP) += mp_init.c ramstage-y += backup_default_smm.c diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index d62888e..0f6c701 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -223,6 +223,8 @@ struct memranges memmap; };
+int protected_mode_call_2arg(size_t arg_count, uint64_t func, uint64_t arg1, uint64_t arg2); + static void do_fsp_memory_init(const struct fspm_context *context, bool s3wake) { uint32_t status; @@ -296,7 +298,11 @@
post_code(POST_FSP_MEMORY_INIT); timestamp_add_now(TS_FSP_MEMORY_INIT_START); +#if ENV_X86_64 + status = protected_mode_call_2arg(2, (uintptr_t)fsp_raminit, (uintptr_t)&fspm_upd, (uintptr_t)fsp_get_hob_list_ptr()); +#else status = fsp_raminit(&fspm_upd, fsp_get_hob_list_ptr()); +#endif post_code(POST_FSP_MEMORY_EXIT); timestamp_add_now(TS_FSP_MEMORY_INIT_END);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48175 )
Change subject: [WIP]cpu/x86/64bit: Add code to call FSP in protected mode ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48175/1/src/drivers/intel/fsp2_0/me... File src/drivers/intel/fsp2_0/memory_init.c:
https://review.coreboot.org/c/coreboot/+/48175/1/src/drivers/intel/fsp2_0/me... PS1, Line 302: status = protected_mode_call_2arg(2, (uintptr_t)fsp_raminit, (uintptr_t)&fspm_upd, (uintptr_t)fsp_get_hob_list_ptr()); line over 96 characters
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48175
to look at the new patch set (#2).
Change subject: cpu/x86/64bit: Add code to call function in protected mode ......................................................................
cpu/x86/64bit: Add code to call function in protected mode
This adds a helper function for long mode to call some code in protected mode and return back to long mode.
The primary use case is to run binaries that have been compiled for protected mode, like the FSP or MRC binaries.
Tested on Intel Skylake. The FSP-M runs and returns without error while coreboot runs in long mode.
Change-Id: I22af2d224b546c0be9e7295330b4b6602df106d6 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/arch/x86/include/mode_switch.h A src/cpu/x86/64bit/mode_switch.S M src/cpu/x86/Makefile.inc 3 files changed, 149 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/48175/2
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48175
to look at the new patch set (#3).
Change subject: cpu/x86/64bit: Add code to call function in protected mode ......................................................................
cpu/x86/64bit: Add code to call function in protected mode
This adds a helper function for long mode to call some code in protected mode and return back to long mode.
The primary use case is to run binaries that have been compiled for protected mode, like the FSP or MRC binaries.
Tested on Intel Skylake. The FSP-M runs and returns without error while coreboot runs in long mode.
Change-Id: I22af2d224b546c0be9e7295330b4b6602df106d6 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/arch/x86/include/mode_switch.h A src/cpu/x86/64bit/mode_switch.S M src/cpu/x86/Makefile.inc 3 files changed, 148 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/48175/3
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48175 )
Change subject: cpu/x86/64bit: Add code to call function in protected mode ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48175/3/src/cpu/x86/Makefile.inc File src/cpu/x86/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48175/3/src/cpu/x86/Makefile.inc@3 PS3, Line 3: ramstage Use the all target?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48175 )
Change subject: cpu/x86/64bit: Add code to call function in protected mode ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48175/4/src/cpu/x86/64bit/mode_swit... File src/cpu/x86/64bit/mode_switch.S:
https://review.coreboot.org/c/coreboot/+/48175/4/src/cpu/x86/64bit/mode_swit... PS4, Line 27: movl -64(%ebp), %edx /* Argument 0 */ : movl -72(%ebp), %ecx /* Argument 1 */ This truncates the 64bit arguments to 32bit.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48175 )
Change subject: cpu/x86/64bit: Add code to call function in protected mode ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48175/4/src/cpu/x86/64bit/mode_swit... File src/cpu/x86/64bit/mode_switch.S:
https://review.coreboot.org/c/coreboot/+/48175/4/src/cpu/x86/64bit/mode_swit... PS4, Line 27: movl -64(%ebp), %edx /* Argument 0 */ : movl -72(%ebp), %ecx /* Argument 1 */
This truncates the 64bit arguments to 32bit.
That's intended, see description in the headers. Maybe it even works with a changed function prototype. I'll do some tests.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48175 )
Change subject: cpu/x86/64bit: Add code to call function in protected mode ......................................................................
Patch Set 4: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/48175/4/src/cpu/x86/64bit/mode_swit... File src/cpu/x86/64bit/mode_switch.S:
https://review.coreboot.org/c/coreboot/+/48175/4/src/cpu/x86/64bit/mode_swit... PS4, Line 27: movl -64(%ebp), %edx /* Argument 0 */ : movl -72(%ebp), %ecx /* Argument 1 */
That's intended, see description in the headers. Maybe it even works with a changed function prototype. I'll do some tests.
ok. sounds good.
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Arthur Heymans, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48175
to look at the new patch set (#5).
Change subject: cpu/x86/64bit: Add code to call function in protected mode ......................................................................
cpu/x86/64bit: Add code to call function in protected mode
This adds a helper function for long mode to call some code in protected mode and return back to long mode.
The primary use case is to run binaries that have been compiled for protected mode, like the FSP or MRC binaries.
Tested on Intel Skylake. The FSP-M runs and returns without error while coreboot runs in long mode.
Change-Id: I22af2d224b546c0be9e7295330b4b6602df106d6 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/arch/x86/include/mode_switch.h A src/cpu/x86/64bit/mode_switch.S M src/cpu/x86/Makefile.inc 3 files changed, 148 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/48175/5
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48175 )
Change subject: cpu/x86/64bit: Add code to call function in protected mode ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48175/3/src/cpu/x86/Makefile.inc File src/cpu/x86/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48175/3/src/cpu/x86/Makefile.inc@3 PS3, Line 3: ramstage
Use the all target?
there's ramstage twice
Hello build bot (Jenkins), Patrick Georgi, Martin Roth, Arthur Heymans, Andrey Petrov, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48175
to look at the new patch set (#6).
Change subject: cpu/x86/64bit: Add code to call function in protected mode ......................................................................
cpu/x86/64bit: Add code to call function in protected mode
This adds a helper function for long mode to call some code in protected mode and return back to long mode.
The primary use case is to run binaries that have been compiled for protected mode, like the FSP or MRC binaries.
Tested on Intel Skylake. The FSP-M runs and returns without error while coreboot runs in long mode.
Change-Id: I22af2d224b546c0be9e7295330b4b6602df106d6 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- A src/arch/x86/include/mode_switch.h A src/cpu/x86/64bit/mode_switch.S M src/cpu/x86/Makefile.inc 3 files changed, 143 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/75/48175/6
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48175 )
Change subject: cpu/x86/64bit: Add code to call function in protected mode ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48175/3/src/cpu/x86/Makefile.inc File src/cpu/x86/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/48175/3/src/cpu/x86/Makefile.inc@3 PS3, Line 3: ramstage
there's ramstage twice
Done
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48175 )
Change subject: cpu/x86/64bit: Add code to call function in protected mode ......................................................................
Patch Set 6: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48175 )
Change subject: cpu/x86/64bit: Add code to call function in protected mode ......................................................................
cpu/x86/64bit: Add code to call function in protected mode
This adds a helper function for long mode to call some code in protected mode and return back to long mode.
The primary use case is to run binaries that have been compiled for protected mode, like the FSP or MRC binaries.
Tested on Intel Skylake. The FSP-M runs and returns without error while coreboot runs in long mode.
Change-Id: I22af2d224b546c0be9e7295330b4b6602df106d6 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/48175 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- A src/arch/x86/include/mode_switch.h A src/cpu/x86/64bit/mode_switch.S M src/cpu/x86/Makefile.inc 3 files changed, 143 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved
diff --git a/src/arch/x86/include/mode_switch.h b/src/arch/x86/include/mode_switch.h new file mode 100644 index 0000000..0c46da5 --- /dev/null +++ b/src/arch/x86/include/mode_switch.h @@ -0,0 +1,70 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <stddef.h> +#include <stdint.h> + +#if ENV_X86_64 +int protected_mode_call_narg(uint32_t arg_count, + uint32_t func_ptr, + uint32_t opt_arg1, + uint32_t opt_arg2); + +/* + * Drops into protected mode and calls the function, which must have been compiled for x86_32. + * After the function returns it enters long mode again. + * The function pointer destination must be below 4GiB in physical memory. + * + * The called function doesn't have arguments and returns an int. + */ +static inline int protected_mode_call(void *func) +{ + return protected_mode_call_narg(0, (uintptr_t)func, 0, 0); +} + +/* + * Drops into protected mode and calls the function, which must have been compiled for x86_32. + * After the function returns it enters long mode again. + * The function pointer destination must be below 4GiB in physical memory. + * Only the lower 32bits of the argument are passed to the called function. + * + * The called function have one argument and returns an int. + */ +static inline int protected_mode_call_1arg(void *func, uint32_t arg1) +{ + return protected_mode_call_narg(1, (uintptr_t)func, arg1, 0); +} + +/* + * Drops into protected mode and calls the function, which must have been compiled for x86_32. + * After the function returns it enters long mode again. + * The function pointer destination must be below 4GiB in physical memory. + * Only the lower 32bits of the argument are passed to the called function. + * + * The called function has two arguments and returns an int. + */ +static inline int protected_mode_call_2arg(void *func, uint32_t arg1, uint32_t arg2) +{ + return protected_mode_call_narg(2, (uintptr_t)func, arg1, arg2); +} +#else +static inline int protected_mode_call(void *func) +{ + int (*doit)(void) = func; + + return doit(); +} + +static inline int protected_mode_call_1arg(void *func, uint32_t arg1) +{ + int (*doit)(uint32_t arg1) = func; + + return doit(arg1); +} + +static inline int protected_mode_call_2arg(void *func, uint32_t arg1, uint32_t arg2) +{ + int (*doit)(uint32_t arg1, uint32_t arg2) = func; + + return doit(arg1, arg2); +} +#endif diff --git a/src/cpu/x86/64bit/mode_switch.S b/src/cpu/x86/64bit/mode_switch.S new file mode 100644 index 0000000..eea104b --- /dev/null +++ b/src/cpu/x86/64bit/mode_switch.S @@ -0,0 +1,70 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +.text +.code64 + .section ".text.protected_mode_call", "ax", @progbits + .globl protected_mode_call_narg +protected_mode_call_narg: + + push %rbp + mov %rsp, %rbp + /* Preserve registers */ + push %rbx + push %r12 + push %r13 + push %r14 + push %r15 + + /* Arguments to stack */ + push %rdi + push %rsi + push %rdx + push %rcx + + #include <cpu/x86/64bit/exit32.inc> + + movl -48(%ebp), %eax /* Argument count */ + movl -64(%ebp), %edx /* Argument 0 */ + movl -72(%ebp), %ecx /* Argument 1 */ + + /* Align the stack */ + andl $0xFFFFFFF0, %esp + test %eax, %eax + je 1f /* Zero arguments */ + + subl $1, %eax + test %eax, %eax + je 2f /* One argument */ + + /* Two arguments */ + subl $8, %esp + pushl %ecx /* Argument 1 */ + pushl %edx /* Argument 0 */ + jmp 1f +2: + subl $12, %esp + pushl %edx /* Argument 0 */ + +1: + movl -56(%ebp), %ebx /* Function to call */ + call *%ebx + movl %eax, %ebx + + /* Preserves ebx */ + #include <cpu/x86/64bit/entry64.inc> + + /* Place return value in rax */ + movl %ebx, %eax + + /* Restore registers */ + mov -40(%rbp), %r15 + mov -32(%rbp), %r14 + mov -24(%rbp), %r13 + mov -16(%rbp), %r12 + mov -8(%rbp), %rbx + + /* Restore stack pointer */ + mov %rbp, %rsp + pop %rbp + + ret diff --git a/src/cpu/x86/Makefile.inc b/src/cpu/x86/Makefile.inc index 2f789f7..cd73b72 100644 --- a/src/cpu/x86/Makefile.inc +++ b/src/cpu/x86/Makefile.inc @@ -1,4 +1,7 @@ subdirs-y += pae + +all-$(CONFIG_ARCH_ALL_STAGES_X86_64) += 64bit/mode_switch.S + subdirs-$(CONFIG_PARALLEL_MP) += name ramstage-$(CONFIG_PARALLEL_MP) += mp_init.c ramstage-y += backup_default_smm.c
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
build bot (Jenkins) (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)