x86: Add .data section support for pre-memory stages

x86 pre-memory stages do not support the `.data` section and as a
result developers are required to include runtime initialization code
instead of relying on C global variable definition.

To illustrate the impact of this lack of `.data` section support, here
are two limitations I personally ran into:

1. The inclusion of libgfxinit in romstage for Raptor Lake has
   required some changes in libgfxinit to ensure data is initialized at
   runtime. In addition, we had to manually map some `.data` symbols in
   the `_bss` region.

2. CBFS cache is currently not supported in pre-memory stages and
   enabling it would require to add an initialization function and
   find a generic spot to call it.

Other platforms do not have that limitation. Hence, resolving it would
help to align code and reduce compilation based restriction (cf. the
use of `ENV_HAS_DATA_SECTION` compilation flag in various places of
coreboot code).

We identified three cases to consider:

1. eXecute-In-Place pre-memory stages
   - code is in SPINOR
   - data is also stored in SPINOR but must be linked in Cache-As-RAM
     and copied there at runtime

2. `bootblock` stage is a bit different as it uses Cache-As-Ram but
   the memory mapping and its entry code different

3. pre-memory stages loaded in and executed from
   Cache-As-RAM (cf. `CONFIG_NO_XIP_EARLY_STAGES`).

eXecute-In-Place pre-memory stages (#1) require the creation of a new
ELF segment as the code segment Virtual Memory Address and Load Memory
Address are identical but the data needs to be linked in
cache-As-RAM (VMA) but to be stored right after the code (LMA).

Here is the output `readelf --segments` on a `romstage.debug` ELF
binary.

    Program Headers:
      Type    Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
      LOAD    0x000080 0x02000000 0x02000000 0x21960 0x21960 R E 0x20
      LOAD    0x0219e0 0xfefb1640 0x02021960 0x00018 0x00018 RW  0x4

     Section to Segment mapping:
      Segment Sections...
       00     .text
       01     .data

Segment 0 `VirtAddr` and `PhysAddr` are at the same address while they
are totally different for the Segment 1 holding the `.data`
section. Since we need the data section `VirtAddr` to be in the
Cache-As-Ram and its `PhysAddr` right after the `.text` section, the
use of a new segment is mandatory.

`bootblock` (#2) also uses this new segment to store the data right
after the code and load it to Cache-As-RAM at runtime. However, the
code involved is different.

Not eXecute-In-Place pre-memory stages (#3) do not really need any
special work other than enabling a data section as the code and data
VMA / LMA translation vector is the same.

TEST=#1 and #2 verified on rex and qemu 32 and 64 bits:
     - The `bootblock.debug`, `romstage.debug` and
       `verstage.debug` all have data stored at the end of the `.text`
       section and code to copy the data content to the Cache-As-RAM.
     - The CBFS stages included in the final image has not improperly
       relocated any of the `.data` section symbol.
     - Test purposes global data symbols we added in bootblock,
       romstage and verstage are properly accessible at runtime
     #3: for "Intel Apollolake DDR3 RVP1" board, we verified that the
     generated romstage ELF includes a .data section similarly to a
     regular memory enabled stage.

Change-Id: I030407fcc72776e59def476daa5b86ad0495debe
Signed-off-by: Jeremy Compostella <jeremy.compostella@intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/77289
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>

5 files changed, 51 insertions, 25 deletions

diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc
index e0e8a3b7b9..accb022a81 100644
--- a/src/arch/x86/Makefile.inc
+++ b/src/arch/x86/Makefile.inc
@@ -64,11 +64,6 @@ $(1)-S-ccopts += -I.
 $$(objcbfs)/$(1).debug: $$$$($(1)-libs) $$$$($(1)-objs)
 	@printf "    LINK       $$(subst $$(obj)/,,$$(@))\n"
 	$$(LD_$(1)) $$(LDFLAGS_$(1)) -o $$@ -L$$(obj) $$(COMPILER_RT_FLAGS_$(1)) --whole-archive --start-group $$(filter-out %.ld,$$($(1)-objs)) $$($(1)-libs) --no-whole-archive $$(COMPILER_RT_$(1)) --end-group -T $(call src-to-obj,$(1),$(CONFIG_MEMLAYOUT_LD_FILE)) --oformat $(2)
-	-LANG=C LC_ALL= $$(OBJCOPY_$(1)) --only-section .illegal_globals $$(@) $$(objcbfs)/$(1)_null.offenders >/dev/null 2>&1
-	if [ -z "$$$$($$(NM_$(1)) $$(objcbfs)/$(1)_null.offenders 2>&1 | grep 'no symbols')" ];then \
-		echo "Forbidden global variables in $(1):"; \
-		$$(NM_$(1)) $$(objcbfs)/$(1)_null.offenders; false; \
-	fi
 endef
 
 ###############################################################################
diff --git a/src/arch/x86/assembly_entry.S b/src/arch/x86/assembly_entry.S
index 869acc84e2..9a9a0465dc 100644
--- a/src/arch/x86/assembly_entry.S
+++ b/src/arch/x86/assembly_entry.S
@@ -33,8 +33,8 @@ _start:
 	/* reset stack pointer to CAR/EARLYRAM stack */
 	mov	$_STACK_TOP, %esp
 
+#if ENV_SEPARATE_DATA_AND_BSS
 	/* clear .bss section as it is not shared */
-#if ENV_SEPARATE_BSS
 	cld
 	xor	%eax, %eax
 	movl	$(_ebss), %ecx
@@ -42,6 +42,14 @@ _start:
 	sub	%edi, %ecx
 	shrl	$2, %ecx
 	rep	stosl
+
+	/* Copy .data section content to Cache-As-Ram */
+	movl	$(_edata), %ecx
+	movl	$(_data), %edi
+	sub	%edi, %ecx
+	shrl	$2, %ecx
+	movl	$(_data_load),%esi
+	rep	movsl
 #endif
 
 #if ((ENV_SEPARATE_VERSTAGE && CONFIG(VERSTAGE_DEBUG_SPINLOOP)) \
diff --git a/src/arch/x86/car.ld b/src/arch/x86/car.ld
index ab6c3b0df8..a1a782ffe0 100644
--- a/src/arch/x86/car.ld
+++ b/src/arch/x86/car.ld
@@ -59,7 +59,7 @@
 	 * cbmem console. This is useful for clearing this area on a per-stage
 	 * basis when more than one stage uses cache-as-ram. */
 
-#if ENV_SEPARATE_BSS
+#if ENV_SEPARATE_DATA_AND_BSS
 	. = ALIGN(ARCH_POINTER_ALIGN_SIZE);
 	_bss = .;
 	/* Allow global uninitialized variables for stages without CAR teardown. */
@@ -89,11 +89,32 @@
 	_shadow_size = (_ebss - _car_region_start) >> 3;
 	REGION(asan_shadow, ., _shadow_size, ARCH_POINTER_ALIGN_SIZE)
 #endif
-	_car_unallocated_start = .;
-	_car_region_end = . + CONFIG_DCACHE_RAM_SIZE - (. - _car_region_start)
-			  - CONFIG_FSP_T_RESERVED_SIZE;
 }
 
+#if ENV_SEPARATE_DATA_AND_BSS
+/* This symbol defines the load address of the Cache-As-RAM .data
+ * section. It should be right at the end of the .text section (_etext)
+ * and ARCH_POINTER_ALIGN_SIZE aligned. */
+_data_load = _etext;
+
+_bogus = ASSERT(_etext == ALIGN(_etext, ARCH_POINTER_ALIGN_SIZE), "Cache-As-RAM load address is improperly defined.");
+
+.data ALIGN(ARCH_POINTER_ALIGN_SIZE) : AT (_data_load) {
+	_data = .;
+	*(.data);
+	*(.data.*);
+	*(.sdata);
+	*(.sdata.*);
+	. = ALIGN(ARCH_POINTER_ALIGN_SIZE);
+	_edata = .;
+	RECORD_SIZE(data)
+} : data_segment
+#endif
+
+_car_unallocated_start = .;
+_car_region_end = . + CONFIG_DCACHE_RAM_SIZE - (. - _car_region_start)
+		  - CONFIG_FSP_T_RESERVED_SIZE;
+
 . = _car_region_start;
 .car.fspm_rc_heap . (NOLOAD) : {
 . += CONFIG_FSP_M_RC_HEAP_SIZE;
@@ -124,18 +145,11 @@ _rom_mtrr_mask = ~(CACHE_ROM_SIZE - 1);
 _rom_mtrr_base = _rom_mtrr_mask;
 #endif
 
-/* Global variables are not allowed in romstage
- * This section is checked during stage creation to ensure
- * that there are no global variables present
- */
-
-. = 0xffffff00;
-.illegal_globals . : {
-	*(.data)
-	*(.data.*)
-}
-
+#if ENV_SEPARATE_DATA_AND_BSS
+_bogus = ASSERT((CONFIG_DCACHE_RAM_SIZE == 0) || (SIZEOF(.car.data) + SIZEOF(.data) <= CONFIG_DCACHE_RAM_SIZE), "Cache as RAM area is too full");
+#else
 _bogus = ASSERT((CONFIG_DCACHE_RAM_SIZE == 0) || (SIZEOF(.car.data) <= CONFIG_DCACHE_RAM_SIZE), "Cache as RAM area is too full");
+#endif
 #if CONFIG(PAGING_IN_CACHE_AS_RAM)
 _bogus2 = ASSERT(_pagetables == ALIGN(_pagetables, 4096), "_pagetables aren't 4KiB aligned");
 #endif
diff --git a/src/arch/x86/include/arch/header.ld b/src/arch/x86/include/arch/header.ld
index 5b380faad5..2ee021226c 100644
--- a/src/arch/x86/include/arch/header.ld
+++ b/src/arch/x86/include/arch/header.ld
@@ -3,6 +3,9 @@
 PHDRS
 {
 	to_load PT_LOAD;
+#if ENV_SEPARATE_DATA_AND_BSS
+	data_segment PT_LOAD;
+#endif
 }
 
 ENTRY(_start)
diff --git a/src/arch/x86/memlayout.ld b/src/arch/x86/memlayout.ld
index 549c2f9041..f448bf89de 100644
--- a/src/arch/x86/memlayout.ld
+++ b/src/arch/x86/memlayout.ld
@@ -3,6 +3,16 @@
 #include <memlayout.h>
 #include <arch/header.ld>
 
+/*
+ * The bootblock linker script should be included before the Cache-As-RAM linker
+ * script. Indeed, if it is included after and Cache-As-RAM .data section
+ * support is enabled, the definition order of the sections makes the linker
+ * create an image with an almost 4 GB hole.
+ */
+#if ENV_BOOTBLOCK
+INCLUDE "bootblock/arch/x86/bootblock.ld"
+#endif /* ENV_BOOTBLOCK */
+
 SECTIONS
 {
 	/*
@@ -36,7 +46,3 @@ SECTIONS
 	POSTCAR(32M, 1M)
 #endif
 }
-
-#if ENV_BOOTBLOCK
-	INCLUDE "bootblock/arch/x86/bootblock.ld"
-#endif  /* ENV_BOOTBLOCK */