Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44734 )
Change subject: Revert "cpu/x86/sipi: Add x86_64 support" ......................................................................
Revert "cpu/x86/sipi: Add x86_64 support"
This reverts commit 18ad7fa51f5c6560c9d7a9bcf68e9e277e37cd49. Breaks Mpinit. The log shows:
SIPI module has no parameters. MP initialization failure.
Change-Id: Ideed19437667124a02c0f03aa7be8dec042d0f44 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/cpu/x86/64bit/entry64.inc M src/cpu/x86/Makefile.inc M src/cpu/x86/sipi_vector.S 3 files changed, 0 insertions(+), 24 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/44734/1
diff --git a/src/cpu/x86/64bit/entry64.inc b/src/cpu/x86/64bit/entry64.inc index 7025517..65c0fdc 100644 --- a/src/cpu/x86/64bit/entry64.inc +++ b/src/cpu/x86/64bit/entry64.inc @@ -16,12 +16,7 @@ #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 */ @@ -47,12 +42,7 @@ 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/Makefile.inc b/src/cpu/x86/Makefile.inc index 0502c72..2f789f7 100644 --- a/src/cpu/x86/Makefile.inc +++ b/src/cpu/x86/Makefile.inc @@ -24,7 +24,6 @@ $(TARGET_STAGE)-srcs += $(SIPI_BIN).manual endif rmodules_$(ARCH-$(TARGET_STAGE)-y)-$(CONFIG_PARALLEL_MP) += sipi_vector.S -rmodules_$(ARCH-$(TARGET_STAGE)-y)-generic-ccopts += $($(TARGET_STAGE)-generic-ccopts)
$(SIPI_DOTO): $(call src-to-obj,rmodules_$(ARCH-$(TARGET_STAGE)-y),src/cpu/x86/sipi_vector.S) $(LD_rmodules_$(ARCH-$(TARGET_STAGE)-y)) -nostdlib -r -o $@ $^ diff --git a/src/cpu/x86/sipi_vector.S b/src/cpu/x86/sipi_vector.S index bda49cc..ba1ecb7 100644 --- a/src/cpu/x86/sipi_vector.S +++ b/src/cpu/x86/sipi_vector.S @@ -192,24 +192,11 @@ 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/+/44734 )
Change subject: Revert "cpu/x86/sipi: Add x86_64 support" ......................................................................
Patch Set 1: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/44734/1/src/cpu/x86/Makefile.inc File src/cpu/x86/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/44734/1/src/cpu/x86/Makefile.inc@a2... PS1, Line 27: rmodules_$(ARCH-$(TARGET_STAGE)-y)-generic-ccopts += $($(TARGET_STAGE)-generic-ccopts) Isn't this the only problematic thing?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44734 )
Change subject: Revert "cpu/x86/sipi: Add x86_64 support" ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44734/1/src/cpu/x86/Makefile.inc File src/cpu/x86/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/44734/1/src/cpu/x86/Makefile.inc@a2... PS1, Line 27: rmodules_$(ARCH-$(TARGET_STAGE)-y)-generic-ccopts += $($(TARGET_STAGE)-generic-ccopts)
Isn't this the only problematic thing?
Likely yes, but I don't know why. It compiles and linkes without problems...
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44734 )
Change subject: Revert "cpu/x86/sipi: Add x86_64 support" ......................................................................
Patch Set 1: Code-Review+2
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44734 )
Change subject: Revert "cpu/x86/sipi: Add x86_64 support" ......................................................................
Patch Set 1: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44734 )
Change subject: Revert "cpu/x86/sipi: Add x86_64 support" ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/coreboot/+/44734/1/src/cpu/x86/Makefile.inc File src/cpu/x86/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/44734/1/src/cpu/x86/Makefile.inc@a2... PS1, Line 27: rmodules_$(ARCH-$(TARGET_STAGE)-y)-generic-ccopts += $($(TARGET_STAGE)-generic-ccopts)
Likely yes, but I don't know why. It compiles and linkes without problems...
Ack
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44734 )
Change subject: Revert "cpu/x86/sipi: Add x86_64 support" ......................................................................
Patch Set 1: Code-Review+1
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/44734 )
Change subject: Revert "cpu/x86/sipi: Add x86_64 support" ......................................................................
Revert "cpu/x86/sipi: Add x86_64 support"
This reverts commit 18ad7fa51f5c6560c9d7a9bcf68e9e277e37cd49. Breaks Mpinit. The log shows:
SIPI module has no parameters. MP initialization failure.
Change-Id: Ideed19437667124a02c0f03aa7be8dec042d0f44 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/44734 Reviewed-by: Arthur Heymans arthur@aheymans.xyz Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/cpu/x86/64bit/entry64.inc M src/cpu/x86/Makefile.inc M src/cpu/x86/sipi_vector.S 3 files changed, 0 insertions(+), 24 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Paul Menzel: Looks good to me, but someone else must approve Arthur Heymans: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/src/cpu/x86/64bit/entry64.inc b/src/cpu/x86/64bit/entry64.inc index 7025517..65c0fdc 100644 --- a/src/cpu/x86/64bit/entry64.inc +++ b/src/cpu/x86/64bit/entry64.inc @@ -16,12 +16,7 @@ #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 */ @@ -47,12 +42,7 @@ 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/Makefile.inc b/src/cpu/x86/Makefile.inc index 0502c72..2f789f7 100644 --- a/src/cpu/x86/Makefile.inc +++ b/src/cpu/x86/Makefile.inc @@ -24,7 +24,6 @@ $(TARGET_STAGE)-srcs += $(SIPI_BIN).manual endif rmodules_$(ARCH-$(TARGET_STAGE)-y)-$(CONFIG_PARALLEL_MP) += sipi_vector.S -rmodules_$(ARCH-$(TARGET_STAGE)-y)-generic-ccopts += $($(TARGET_STAGE)-generic-ccopts)
$(SIPI_DOTO): $(call src-to-obj,rmodules_$(ARCH-$(TARGET_STAGE)-y),src/cpu/x86/sipi_vector.S) $(LD_rmodules_$(ARCH-$(TARGET_STAGE)-y)) -nostdlib -r -o $@ $^ diff --git a/src/cpu/x86/sipi_vector.S b/src/cpu/x86/sipi_vector.S index bda49cc..ba1ecb7 100644 --- a/src/cpu/x86/sipi_vector.S +++ b/src/cpu/x86/sipi_vector.S @@ -192,24 +192,11 @@ 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
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44734 )
Change subject: Revert "cpu/x86/sipi: Add x86_64 support" ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/44734/1/src/cpu/x86/Makefile.inc File src/cpu/x86/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/44734/1/src/cpu/x86/Makefile.inc@a2... PS1, Line 27: rmodules_$(ARCH-$(TARGET_STAGE)-y)-generic-ccopts += $($(TARGET_STAGE)-generic-ccopts)
Ack
I investigated the issue. This line adds -D__RAMSTAGE__ as intended. src/include/rules.h will then no longer set ENV_RMODULE 1 and remove the parameter section from the linker script.
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)