Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30854
Change subject: arch/x86: Drop Kconfig AP_SIPI_VECTOR ......................................................................
arch/x86: Drop Kconfig AP_SIPI_VECTOR
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 --- M src/arch/x86/failover.ld M src/cpu/Kconfig M src/cpu/intel/car/p4-netburst/cache_as_ram.S M src/cpu/x86/16bit/entry16.inc M src/cpu/x86/16bit/entry16.ld 5 files changed, 15 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/30854/1
diff --git a/src/arch/x86/failover.ld b/src/arch/x86/failover.ld index e9613d9..b32aa29 100644 --- a/src/arch/x86/failover.ld +++ b/src/arch/x86/failover.ld @@ -35,7 +35,6 @@ /* This section might be better named .setup */ .rom ROMLOC : { _rom = .; - ap_sipi_vector = .; *(.rom.text); *(.rom.data); *(.rom.data.*); @@ -54,9 +53,7 @@ (CONFIG_SIPI_VECTOR_IN_ROM ? 4096 : 0);
/* Post-check proper SIPI vector. */ - _bogus = ASSERT(!CONFIG_SIPI_VECTOR_IN_ROM || ((ap_sipi_vector & 0x0fff) == 0x0), - "Bad SIPI vector alignment"); - _bogus = ASSERT(!CONFIG_SIPI_VECTOR_IN_ROM || (ap_sipi_vector == CONFIG_AP_SIPI_VECTOR), + _bogus = ASSERT(!CONFIG_SIPI_VECTOR_IN_ROM || (ap_sipi_vector_in_rom == 0xff), "Address mismatch on AP_SIPI_VECTOR");
/DISCARD/ : { diff --git a/src/cpu/Kconfig b/src/cpu/Kconfig index f98ce6d..6078022 100644 --- a/src/cpu/Kconfig +++ b/src/cpu/Kconfig @@ -42,12 +42,6 @@ This option is used to enable certain functions to make coreboot work correctly on symmetric multi processor (SMP) systems.
-config AP_SIPI_VECTOR - hex - default 0xfffff000 - help - This must equal address of ap_sipi_vector from bootblock build. - config MMX bool help diff --git a/src/cpu/intel/car/p4-netburst/cache_as_ram.S b/src/cpu/intel/car/p4-netburst/cache_as_ram.S index bdce514..fda572d 100644 --- a/src/cpu/intel/car/p4-netburst/cache_as_ram.S +++ b/src/cpu/intel/car/p4-netburst/cache_as_ram.S @@ -23,7 +23,10 @@
/* Macro to access Local APIC registers at default base. */ #define LAPIC(x) $(LAPIC_DEFAULT_BASE | LAPIC_ ## x) -#define START_IPI_VECTOR ((CONFIG_AP_SIPI_VECTOR >> 12) & 0xff) +#if !IS_ENABLED(CONFIG_C_ENVIRONMENT_BOOTBLOCK) +/* Fixed location, ASSERTED in failover.ld if it changes. */ +.set ap_sipi_vector_in_rom, 0xff +#endif
#define CACHE_AS_RAM_SIZE CONFIG_DCACHE_RAM_SIZE #define CACHE_AS_RAM_BASE CONFIG_DCACHE_RAM_BASE @@ -180,7 +183,8 @@
/* Send Start IPI to all excluding ourself. */ movl LAPIC(ICR), %edi - movl $(LAPIC_DEST_ALLBUT | LAPIC_DM_STARTUP | START_IPI_VECTOR), %eax + movl $(LAPIC_DEST_ALLBUT | LAPIC_DM_STARTUP), %eax + orl $ap_sipi_vector_in_rom, %eax 1: movl %eax, (%edi) movl $0x30, %ecx 2: pause diff --git a/src/cpu/x86/16bit/entry16.inc b/src/cpu/x86/16bit/entry16.inc index 5a9739c..55d9bd9 100644 --- a/src/cpu/x86/16bit/entry16.inc +++ b/src/cpu/x86/16bit/entry16.inc @@ -28,6 +28,13 @@ */
#include <arch/rom_segs.h> + +#if IS_ENABLED(CONFIG_SIPI_VECTOR_IN_ROM) +/* Symbol _start16bit must be aligned to 4kB to start AP CPUs with + * Startup IPI message without RAM. + */ +.align 4096 +#endif .code16 .globl _start16bit .type _start16bit, @function diff --git a/src/cpu/x86/16bit/entry16.ld b/src/cpu/x86/16bit/entry16.ld index 112d429..66bfbd5 100644 --- a/src/cpu/x86/16bit/entry16.ld +++ b/src/cpu/x86/16bit/entry16.ld @@ -1,2 +1,3 @@ gdtptr16_offset = gdtptr16 & 0xffff; nullidt_offset = nullidt & 0xffff; + ap_sipi_vector_in_rom = (_start16bit >> 12) & 0xff;
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30854 )
Change subject: arch/x86: Drop Kconfig AP_SIPI_VECTOR ......................................................................
Patch Set 1: Code-Review+2
Kyösti Mälkki has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30854 )
Change subject: arch/x86: Drop Kconfig AP_SIPI_VECTOR ......................................................................
arch/x86: Drop Kconfig AP_SIPI_VECTOR
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 --- M src/arch/x86/failover.ld M src/cpu/Kconfig M src/cpu/intel/car/p4-netburst/cache_as_ram.S M src/cpu/x86/16bit/entry16.inc M src/cpu/x86/16bit/entry16.ld 5 files changed, 15 insertions(+), 12 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved
diff --git a/src/arch/x86/failover.ld b/src/arch/x86/failover.ld index e9613d9..b32aa29 100644 --- a/src/arch/x86/failover.ld +++ b/src/arch/x86/failover.ld @@ -35,7 +35,6 @@ /* This section might be better named .setup */ .rom ROMLOC : { _rom = .; - ap_sipi_vector = .; *(.rom.text); *(.rom.data); *(.rom.data.*); @@ -54,9 +53,7 @@ (CONFIG_SIPI_VECTOR_IN_ROM ? 4096 : 0);
/* Post-check proper SIPI vector. */ - _bogus = ASSERT(!CONFIG_SIPI_VECTOR_IN_ROM || ((ap_sipi_vector & 0x0fff) == 0x0), - "Bad SIPI vector alignment"); - _bogus = ASSERT(!CONFIG_SIPI_VECTOR_IN_ROM || (ap_sipi_vector == CONFIG_AP_SIPI_VECTOR), + _bogus = ASSERT(!CONFIG_SIPI_VECTOR_IN_ROM || (ap_sipi_vector_in_rom == 0xff), "Address mismatch on AP_SIPI_VECTOR");
/DISCARD/ : { diff --git a/src/cpu/Kconfig b/src/cpu/Kconfig index f98ce6d..6078022 100644 --- a/src/cpu/Kconfig +++ b/src/cpu/Kconfig @@ -42,12 +42,6 @@ This option is used to enable certain functions to make coreboot work correctly on symmetric multi processor (SMP) systems.
-config AP_SIPI_VECTOR - hex - default 0xfffff000 - help - This must equal address of ap_sipi_vector from bootblock build. - config MMX bool help diff --git a/src/cpu/intel/car/p4-netburst/cache_as_ram.S b/src/cpu/intel/car/p4-netburst/cache_as_ram.S index bdce514..fda572d 100644 --- a/src/cpu/intel/car/p4-netburst/cache_as_ram.S +++ b/src/cpu/intel/car/p4-netburst/cache_as_ram.S @@ -23,7 +23,10 @@
/* Macro to access Local APIC registers at default base. */ #define LAPIC(x) $(LAPIC_DEFAULT_BASE | LAPIC_ ## x) -#define START_IPI_VECTOR ((CONFIG_AP_SIPI_VECTOR >> 12) & 0xff) +#if !IS_ENABLED(CONFIG_C_ENVIRONMENT_BOOTBLOCK) +/* Fixed location, ASSERTED in failover.ld if it changes. */ +.set ap_sipi_vector_in_rom, 0xff +#endif
#define CACHE_AS_RAM_SIZE CONFIG_DCACHE_RAM_SIZE #define CACHE_AS_RAM_BASE CONFIG_DCACHE_RAM_BASE @@ -180,7 +183,8 @@
/* Send Start IPI to all excluding ourself. */ movl LAPIC(ICR), %edi - movl $(LAPIC_DEST_ALLBUT | LAPIC_DM_STARTUP | START_IPI_VECTOR), %eax + movl $(LAPIC_DEST_ALLBUT | LAPIC_DM_STARTUP), %eax + orl $ap_sipi_vector_in_rom, %eax 1: movl %eax, (%edi) movl $0x30, %ecx 2: pause diff --git a/src/cpu/x86/16bit/entry16.inc b/src/cpu/x86/16bit/entry16.inc index 5a9739c..55d9bd9 100644 --- a/src/cpu/x86/16bit/entry16.inc +++ b/src/cpu/x86/16bit/entry16.inc @@ -28,6 +28,13 @@ */
#include <arch/rom_segs.h> + +#if IS_ENABLED(CONFIG_SIPI_VECTOR_IN_ROM) +/* Symbol _start16bit must be aligned to 4kB to start AP CPUs with + * Startup IPI message without RAM. + */ +.align 4096 +#endif .code16 .globl _start16bit .type _start16bit, @function diff --git a/src/cpu/x86/16bit/entry16.ld b/src/cpu/x86/16bit/entry16.ld index 112d429..66bfbd5 100644 --- a/src/cpu/x86/16bit/entry16.ld +++ b/src/cpu/x86/16bit/entry16.ld @@ -1,2 +1,3 @@ gdtptr16_offset = gdtptr16 & 0xffff; nullidt_offset = nullidt & 0xffff; + ap_sipi_vector_in_rom = (_start16bit >> 12) & 0xff;
-
Arthur Heymans (Code Review) -
Kyösti Mälkki (Code Review)