Kyösti Mälkki submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Marshall Dawson: Looks good to me, approved
arch/x86: Restrict use of _car_global[start|end]

Restrict the use of symbol names _car_global_[start|end]
to be used exclusively with CAR_GLOBAL_MIGRATION=y.
They just alias the start and end of .bss section in CAR.

Change-Id: I36c858a4f181516d4c61f9fd1d5005c7d2c06057
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/35034
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
---
M src/arch/x86/assembly_entry.S
M src/arch/x86/car.ld
M src/arch/x86/include/arch/early_variables.h
M src/arch/x86/include/arch/symbols.h
M src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S
M src/soc/intel/quark/romstage/fsp2_0.c
6 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/src/arch/x86/assembly_entry.S b/src/arch/x86/assembly_entry.S
index 5e84af2..c36dc1c 100644
--- a/src/arch/x86/assembly_entry.S
+++ b/src/arch/x86/assembly_entry.S
@@ -32,11 +32,11 @@
/* reset stack pointer to CAR stack */
mov $_car_stack_end, %esp

- /* clear CAR_GLOBAL area as it is not shared */
+ /* clear .bss section as it is not shared */
cld
xor %eax, %eax
- movl $(_car_global_end), %ecx
- movl $(_car_global_start), %edi
+ movl $(_ebss), %ecx
+ movl $(_bss), %edi
sub %edi, %ecx
shrl $2, %ecx
rep stosl
diff --git a/src/arch/x86/car.ld b/src/arch/x86/car.ld
index a09150f..6ccbd8c 100644
--- a/src/arch/x86/car.ld
+++ b/src/arch/x86/car.ld
@@ -64,11 +64,13 @@
. += 80;
_car_ehci_dbg_info_end = .;

- /* _car_global_start and _car_global_end provide symbols to per-stage
+ /* _bss and _ebss provide symbols to per-stage
* variables that are not shared like the timestamp and the pre-ram
* cbmem console. This is useful for clearing this area on a per-stage
* basis when more than one stage uses cache-as-ram for CAR_GLOBALs. */
- _car_global_start = .;
+
+ . = ALIGN(ARCH_POINTER_ALIGN_SIZE);
+ _bss = .;
#if ENV_STAGE_HAS_BSS_SECTION
/* Allow global uninitialized variables for stages without CAR teardown. */
*(.bss)
@@ -76,10 +78,12 @@
*(.sbss)
*(.sbss.*)
#else
+ _car_global_start = .;
*(.car.global_data);
+ _car_global_end = .;
#endif
. = ALIGN(ARCH_POINTER_ALIGN_SIZE);
- _car_global_end = .;
+ _ebss = .;
_car_unallocated_start = .;

#if !CONFIG(C_ENVIRONMENT_BOOTBLOCK)
diff --git a/src/arch/x86/include/arch/early_variables.h b/src/arch/x86/include/arch/early_variables.h
index 57f4306..b88495c 100644
--- a/src/arch/x86/include/arch/early_variables.h
+++ b/src/arch/x86/include/arch/early_variables.h
@@ -20,6 +20,15 @@

#if ENV_ROMSTAGE && CONFIG(CAR_GLOBAL_MIGRATION)

+/*
+ * The _car_global_[start|end]symbols cover CAR data which is relocatable
+ * once memory comes online. Variables with CAR_GLOBAL decoration
+ * reside within this region.
+ */
+extern char _car_global_start[];
+extern char _car_global_end[];
+#define _car_global_size (_car_global_end - _car_global_start)
+
asm(".section .car.global_data,\"w\",@nobits");
asm(".previous");
#ifdef __clang__
diff --git a/src/arch/x86/include/arch/symbols.h b/src/arch/x86/include/arch/symbols.h
index a516155..f715e0a 100644
--- a/src/arch/x86/include/arch/symbols.h
+++ b/src/arch/x86/include/arch/symbols.h
@@ -24,8 +24,8 @@
#define _car_region_size (_car_region_end - _car_region_start)

/*
- * This is the stack used under CONFIG_C_ENVIRONMENT_BOOTBLOCK for
- * all stages that execute when cache-as-ram is up.
+ * This is the stack area used for all stages that execute when cache-as-ram
+ * is up. Area is not cleared in between stages.
*/
extern char _car_stack_start[];
extern char _car_stack_end[];
@@ -38,13 +38,4 @@
#define _car_ehci_dbg_info_size \
(_car_ehci_dbg_info_end - _car_ehci_dbg_info_start)

-/*
- * The _car_global_[start|end]symbols cover CAR data which is relocatable
- * once memory comes online. Variables with CAR_GLOBAL decoration
- * reside within this region.
- */
-extern char _car_global_start[];
-extern char _car_global_end[];
-#define _car_global_size (_car_global_end - _car_global_start)
-
#endif
diff --git a/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S b/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S
index 9a8ab5b..091fc4a 100644
--- a/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S
+++ b/src/soc/intel/common/block/cpu/car/cache_as_ram_fsp.S
@@ -89,11 +89,11 @@
/* Setup bootblock stack */
mov %edx, %esp

- /* clear CAR_GLOBAL area as it is not shared */
+ /* clear .bss section as it is not shared */
cld
xor %eax, %eax
- movl $(_car_global_end), %ecx
- movl $(_car_global_start), %edi
+ movl $(_ebss), %ecx
+ movl $(_bss), %edi
sub %edi, %ecx
shrl $2, %ecx
rep stosl
diff --git a/src/soc/intel/quark/romstage/fsp2_0.c b/src/soc/intel/quark/romstage/fsp2_0.c
index c999bce..a64fed4 100644
--- a/src/soc/intel/quark/romstage/fsp2_0.c
+++ b/src/soc/intel/quark/romstage/fsp2_0.c
@@ -115,7 +115,7 @@
aupd->StackBase);
printk(BIOS_SPEW, "| |\n");
printk(BIOS_SPEW, "+-------------------+ 0x%p\n",
- _car_global_end);
+ _car_unallocated_start);
printk(BIOS_SPEW, "| coreboot data |\n");
printk(BIOS_SPEW, "+-------------------+ 0x%p\n",
_car_stack_end);

To view, visit change 35034. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I36c858a4f181516d4c61f9fd1d5005c7d2c06057
Gerrit-Change-Number: 35034
Gerrit-PatchSet: 8
Gerrit-Owner: Kyösti Mälkki <kyosti.malkki@gmail.com>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki@gmail.com>
Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd@gmail.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-MessageType: merged