Change in coreboot[master]: x86: Implement a romstage-only version of RESET_VECTOR_IN_RAM

[Marshall Dawson (Code Review)]

Marshall Dawson has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37491 )

Change subject: x86: Implement a romstage-only version of RESET_VECTOR_IN_RAM
......................................................................

x86: Implement a romstage-only version of RESET_VECTOR_IN_RAM

In continuing the flexibility of RESET_VECTOR_IN_RAM, add Kconfig options
for declaring whether the first stage of execution is bootblock or
romstage.  For the romstage version, rely on the existing x86 reset
vector handling and initialization, as well as assembly_entry.S
(previously intended only to reinitialize the x86's basic settings in
early stages).

Add a new function to be called earlyram_stage_entry() that transmits
the BIST information to be checked.

Adjust linker scripts accordingly.

Note that flipping the default build to a hybrid romstage arrangement
is intentional.  Implementations intending to use a traditional
bootblock should select RESET_VECTOR_IN_RAM_BOOTBLOCK.

Sample build using amd/mandolin (currently WIP):
  Name                           Offset     Type           Size   Comp
  cbfs master header             0x0        cbfs header        32 none
  fallback/ramstage              0x80       stage           76100 none
  config                         0x12a00    raw               364 none
  revision                       0x12bc0    raw               674 none
  payload_config                 0x12ec0    raw              1593 none
  payload_revision               0x13540    raw               262 none
  (empty)                        0x136c0    null             1752 none
  fspm.bin                       0x13dc0    fsp            786432 none
  (empty)                        0xd3e00    null             3992 none
  fsps.bin                       0xd4dc0    fsp            262144 none
  pci1002,15d8.rom               0x114e00   optionrom       54272 none
  fallback/dsdt.aml              0x122280   raw              8365 none
  fallback/payload               0x124380   simple elf      68099 none
  (empty)                        0x134dc0   null          2928536 none
  bootblock                      0x3ffd80   bootblock          64 none

readelf of build/cbfs/fallback/romstage.elf:
Section Headers:
  [Nr] Name              Type      Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL      00000000 000000 000000 00      0   0  0
  [ 1] .text             PROGBITS  08000000 001000 007c20 00  AX  0   0 32
  [ 2] .rel.text         REL       00000000 0838f4 001ac0 08   I  9   1  4
  [ 3] .earlyram.data    NOBITS    0802d000 008c20 045ed8 00  WA  0   0 32
  [ 4] .near_reset_vecto PROGBITS  0807f000 080000 0000c8 00  AX  0   0 4096
  [ 5] .rel.near_reset_v REL       00000000 0853b4 000058 08   I  9   4  4
  [ 6] .reset            PROGBITS  0807fff0 080ff0 000010 00  AX  0   0  1
  [ 7] .rel.reset        REL       00000000 08540c 000008 08   I  9   6  4
  [ 8] .gnu_debuglink    PROGBITS  00000000 081000 000014 00      0   0  4
  [ 9] .symtab           SYMTAB    00000000 081014 001590 10     10 122  4
  [10] .strtab           STRTAB    00000000 0825a4 001350 00      0   0  1
  [11] .shstrtab         STRTAB    00000000 085414 000065 00      0   0  1

167: 0802d000     0 NOTYPE  GLOBAL DEFAULT    3 _earlyram_stack
134: 0802d800     0 NOTYPE  GLOBAL DEFAULT    3 _eearlyram_stack
274: 0802d800     0 NOTYPE  GLOBAL DEFAULT    3 _preram_cbmem_console
310: 0802ee00     0 NOTYPE  GLOBAL DEFAULT    3 _epreram_cbmem_console
202: 0802ee00     0 NOTYPE  GLOBAL DEFAULT    3 _timestamp
335: 0802f000     0 NOTYPE  GLOBAL DEFAULT    3 _etimestamp
244: 0802f000     0 NOTYPE  GLOBAL DEFAULT    3 _bss
216: 08072ed8     0 NOTYPE  GLOBAL DEFAULT    3 _ebss

Change-Id: Ida2bc90fdd3e953445065929124adaa168164250
Signed-off-by: Marshall Dawson 
---
M Makefile.inc
M src/arch/x86/Kconfig
M src/arch/x86/Makefile.inc
M src/arch/x86/assembly_entry.S
M src/arch/x86/include/arch/cpu.h
M src/arch/x86/memlayout.ld
M src/cpu/x86/16bit/entry16.inc
M src/cpu/x86/16bit/reset16.ld
M src/cpu/x86/32bit/entry32.inc
9 files changed, 57 insertions(+), 8 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37491/1

diff --git a/Makefile.inc b/Makefile.inc
index e4defc1..6a11ae8 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -1153,6 +1153,8 @@
    @printf "    CBFSPRINT  $(subst $(obj)/,,$(@))\n\n"
    $(CBFSTOOL) $@ print -r $(subst $(spc),$(comma),$(all-regions))

+ifneq ($(CONFIG_RESET_VECTOR_IN_RAM_ROMSTAGE),y)
+
 cbfs-files-y += $(CONFIG_CBFS_PREFIX)/romstage
 $(CONFIG_CBFS_PREFIX)/romstage-file := $(objcbfs)/romstage.elf
 $(CONFIG_CBFS_PREFIX)/romstage-type := stage
@@ -1192,6 +1194,7 @@
 ifeq ($(CONFIG_VBOOT_STARTS_IN_ROMSTAGE),y)
 $(CONFIG_CBFS_PREFIX)/romstage-options += $(TXTIBB)
 endif
+endif # !CONFIG_RESET_VECTOR_IN_RAM_ROMSTAGE

cbfs-files-$(CONFIG_HAVE_RAMSTAGE) += $(CONFIG_CBFS_PREFIX)/ramstage
 $(CONFIG_CBFS_PREFIX)/ramstage-file := $(RAMSTAGE)
diff --git a/src/arch/x86/Kconfig b/src/arch/x86/Kconfig
index 0547193..467dbf2 100644
--- a/src/arch/x86/Kconfig
+++ b/src/arch/x86/Kconfig
@@ -106,6 +106,23 @@
      The first stage executed may be a bootblock, or a hybrid romstage,
      combining certain elements of bootblock and romstage.

+config RESET_VECTOR_IN_RAM_BOOTBLOCK
+	bool
+	depends on RESET_VECTOR_IN_RAM
+	select HAVE_BOOTBLOCK
+	help
+	  Build a traditional bootblock that begins execution in RAM.  This
+	  implementation uses a C environment, and romcc is not supported.
+
+config RESET_VECTOR_IN_RAM_ROMSTAGE
+	bool
+	depends on RESET_VECTOR_IN_RAM
+	default !RESET_VECTOR_IN_RAM_BOOTBLOCK
+	help
+	  Build a hybrid romstage, combining the bootblock operations of
+	  bringing the x86 online and setting flat protected mode, with the
+	  romstage operations of determining memory setup and bring up cbmem.
+
 # Aligns 16bit entry code in bootblock so that hyper-threading CPUs
 # can boot AP CPUs to enable their shared caches.
 config SIPI_VECTOR_IN_ROM
diff --git a/src/arch/x86/Makefile.inc b/src/arch/x86/Makefile.inc
index 423c351..973ef65 100644
--- a/src/arch/x86/Makefile.inc
+++ b/src/arch/x86/Makefile.inc
@@ -224,8 +224,10 @@
 # gdt_init.S is included by entry32.inc when romstage is the first C
 # environment.
 ifneq ($(CONFIG_ROMCC_BOOTBLOCK),y)
+ifneq ($(CONFIG_RESET_VECTOR_IN_RAM_ROMSTAGE),y)
 romstage-y += gdt_init.S
 endif
+endif
 romstage-y += cbmem.c
 romstage-$(CONFIG_IDT_IN_EVERY_STAGE) += exception.c
 romstage-$(CONFIG_IDT_IN_EVERY_STAGE) += idt.S
diff --git a/src/arch/x86/assembly_entry.S b/src/arch/x86/assembly_entry.S
index 273045a..56f18f8 100644
--- a/src/arch/x86/assembly_entry.S
+++ b/src/arch/x86/assembly_entry.S
@@ -28,12 +28,19 @@
    #define _STACK_TOP _ecar_stack
 #endif

-.section ".text._start", "ax", @progbits
-.global _start
-_start:
+#if CONFIG(RESET_VECTOR_IN_RAM_ROMSTAGE)
+	#include <arch/x86/prologue.inc>
+	#include <cpu/x86/16bit/entry16.inc>
+	#include <cpu/x86/16bit/reset16.inc> /* includes _start symbol */
+	#include <cpu/x86/32bit/entry32.inc> /* includes gdt_init */
+#else
+	.section ".text._start", "ax", @progbits
+	.global _start
+	_start:

/* Migrate GDT to this text segment */
    call	gdt_init
+#endif

/* reset stack pointer to CAR stack */
    mov	$_STACK_TOP, %esp
@@ -62,7 +69,14 @@
 #if CONFIG(IDT_IN_EVERY_STAGE)
    call	exception_init
 #endif
+
+#if CONFIG(RESET_VECTOR_IN_RAM_ROMSTAGE)
+	subl	$12, %esp
+	pushl	%ebp /* BIST value */
+	call	earlyram_stage_entry
+#else
    call	car_stage_entry
+#endif

/* Expect to never return. */
 1:
diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h
index 50d636b..22fcce8 100644
--- a/src/arch/x86/include/arch/cpu.h
+++ b/src/arch/x86/include/arch/cpu.h
@@ -293,6 +293,13 @@
  */
 asmlinkage void car_stage_entry(void);

+/*
+ * A system using RESET_VECTOR_IN_RAM_ROMSTAGE eliminates bootblock and calls
+ * earlyram_stage_entry() as the platform's entry point.
+ * entry point.
+ */
+asmlinkage void earlyram_stage_entry(uint32_t bist);
+
 #endif

/*
diff --git a/src/arch/x86/memlayout.ld b/src/arch/x86/memlayout.ld
index 978925f..b7a1891 100644
--- a/src/arch/x86/memlayout.ld
+++ b/src/arch/x86/memlayout.ld
@@ -60,12 +60,14 @@
 #endif
 }

-#if ENV_BOOTBLOCK
+#if (ENV_BOOTBLOCK || (CONFIG(RESET_VECTOR_IN_RAM_ROMSTAGE) && ENV_ROMSTAGE))
 /* Bootblock specific scripts which provide more SECTION directives. */
 #include <cpu/x86/16bit/entry16.ld>
 #include <cpu/x86/16bit/reset16.ld>
+#if ENV_BOOTBLOCK
 #include <arch/x86/id.ld>
+#endif
 #if CONFIG(CPU_INTEL_FIRMWARE_INTERFACE_TABLE)
 #include <cpu/intel/fit/fit.ld>
 #endif
-#endif  /* ENV_BOOTBLOCK */
+#endif  /* ENV_BOOTBLOCK || (CONFIG(RESET_VECTOR_IN_RAM_ROMSTAGE) && ENV_ROMSTAGE) */
diff --git a/src/cpu/x86/16bit/entry16.inc b/src/cpu/x86/16bit/entry16.inc
index 2aa8240..0dca67e 100644
--- a/src/cpu/x86/16bit/entry16.inc
+++ b/src/cpu/x86/16bit/entry16.inc
@@ -29,7 +29,8 @@

#include <arch/rom_segs.h>

-#if (!CONFIG(ROMCC_BOOTBLOCK) && CONFIG_C_ENV_BOOTBLOCK_SIZE > 0x10000)
+#if ((!CONFIG(ROMCC_BOOTBLOCK) && (CONFIG_C_ENV_BOOTBLOCK_SIZE > 0x10000)) \
+		|| CONFIG(RESET_VECTOR_IN_RAM_ROMSTAGE))
 .section ".near_reset_vector", "ax", @progbits
 #endif

diff --git a/src/cpu/x86/16bit/reset16.ld b/src/cpu/x86/16bit/reset16.ld
index 28893c6..4c4bdcb 100644
--- a/src/cpu/x86/16bit/reset16.ld
+++ b/src/cpu/x86/16bit/reset16.ld
@@ -14,7 +14,8 @@
 /* _RESET_VECTOR: typically the top of the ROM */

SECTIONS {
-#if (!CONFIG(ROMCC_BOOTBLOCK) && CONFIG_C_ENV_BOOTBLOCK_SIZE > 0x10000)
+#if ((!CONFIG(ROMCC_BOOTBLOCK) && (CONFIG_C_ENV_BOOTBLOCK_SIZE > 0x10000)) \
+		|| CONFIG(RESET_VECTOR_IN_RAM_ROMSTAGE))
    /* Trigger an error if I have an unuseable start address */
    _TOO_LOW = CONFIG_X86_RESET_VECTOR - 0xfff0;
    _bogus = ASSERT(_start16bit >= _TOO_LOW, "_start16bit too low. Please report.");
diff --git a/src/cpu/x86/32bit/entry32.inc b/src/cpu/x86/32bit/entry32.inc
index 52c0768..7f60ca5 100644
--- a/src/cpu/x86/32bit/entry32.inc
+++ b/src/cpu/x86/32bit/entry32.inc
@@ -13,6 +13,8 @@

/* For starting coreboot in protected mode */

+#define _START_IN_ENTRY32 (!ENV_BOOTBLOCK && !CONFIG(RESET_VECTOR_IN_RAM))
+
 #include <arch/rom_segs.h>
 #include <cpu/x86/post_code.h>
 #include <arch/x86/gdt_init.S>
@@ -35,7 +37,7 @@
  *	expose the _start symbol for bootblock.
  */
    .align	4
-#if !ENV_BOOTBLOCK
+#if _START_IN_ENTRY32
 .globl _start
 _start:
 #endif

[Marshall Dawson (Code Review)]

Hello Julius Werner, build bot (Jenkins), Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37491

to look at the new patch set (#3).

Change subject: x86: Implement a romstage-only version of RESET_VECTOR_IN_RAM
......................................................................

x86: Implement a romstage-only version of RESET_VECTOR_IN_RAM

In continuing the flexibility of RESET_VECTOR_IN_RAM, add Kconfig options
for declaring whether the first stage of execution is bootblock or
romstage.  For the romstage version, rely on the existing x86 reset
vector handling and initialization, as well as assembly_entry.S
(previously intended only to reinitialize the x86's basic settings in
early stages).

Add a new function to be called earlyram_stage_entry() that transmits
the BIST information to be checked.

Adjust linker scripts accordingly.

Note that flipping the default build to a hybrid romstage arrangement
is intentional.  Implementations intending to use a traditional
bootblock should select RESET_VECTOR_IN_RAM_BOOTBLOCK.

Sample build using amd/mandolin (currently WIP):
  Name                           Offset     Type           Size   Comp
  cbfs master header             0x0        cbfs header        32 none
  fallback/ramstage              0x80       stage           76253 none
  config                         0x12ac0    raw               300 none
  revision                       0x12c40    raw               674 none
  payload_config                 0x12f40    raw              1621 none
  payload_revision               0x13600    raw               264 none
  (empty)                        0x13780    null             1560 none
  fspm.bin                       0x13dc0    fsp            720896 none
  (empty)                        0xc3e00    null             3992 none
  fsps.bin                       0xc4dc0    fsp            327680 none
  pci1002,15d8.rom               0x114e00   optionrom       54272 none
  fallback/dsdt.aml              0x122280   raw              8372 none
  fallback/payload               0x124380   simple elf      69495 none
  (empty)                        0x135340   null          2927128 none
  bootblock                      0x3ffd80   bootblock          64 none

readelf of build/cbfs/fallback/romstage.elf:
Section Headers:
  [Nr] Name              Type      Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL      00000000 000000 000000 00      0   0  0
  [ 1] .text             PROGBITS  08000000 001000 007ba0 00  AX  0   0 32
  [ 2] .rel.text         REL       00000000 083938 001ab0 08   I  9   1  4
  [ 3] .earlyram.data    NOBITS    0802cf4a 008ba0 045f8e 00  WA  0   0 32
  [ 4] .near_reset_vecto PROGBITS  0807f000 080000 0000c8 00  AX  0   0 4096
  [ 5] .rel.near_reset_v REL       00000000 0853e8 000058 08   I  9   4  4
  [ 6] .reset            PROGBITS  0807fff0 080ff0 000010 00  AX  0   0  1
  [ 7] .rel.reset        REL       00000000 085440 000008 08   I  9   6  4
  [ 8] .gnu_debuglink    PROGBITS  00000000 081000 000014 00      0   0  4
  [ 9] .symtab           SYMTAB    00000000 081014 0015b0 10     10 123  4
  [10] .strtab           STRTAB    00000000 0825c4 001373 00      0   0  1
  [11] .shstrtab         STRTAB    00000000 085448 000065 00      0   0  1

169: 0802cf50     0 NOTYPE  GLOBAL DEFAULT    3 _earlyram_stack
135: 0802d750     0 NOTYPE  GLOBAL DEFAULT    3 _eearlyram_stack
276: 0802d750     0 NOTYPE  GLOBAL DEFAULT    3 _preram_cbmem_console
312: 0802ed50     0 NOTYPE  GLOBAL DEFAULT    3 _epreram_cbmem_console
204: 0802ed50     0 NOTYPE  GLOBAL DEFAULT    3 _timestamp
337: 0802ef50     0 NOTYPE  GLOBAL DEFAULT    3 _etimestamp
159: 0802ef50     0 NOTYPE  GLOBAL DEFAULT    3 _fmap_cache
201: 0802f006     0 NOTYPE  GLOBAL DEFAULT    3 _efmap_cache
246: 0802f008     0 NOTYPE  GLOBAL DEFAULT    3 _bss
218: 08072ed8     0 NOTYPE  GLOBAL DEFAULT    3 _ebss

Change-Id: Ida2bc90fdd3e953445065929124adaa168164250
Signed-off-by: Marshall Dawson 
---
M Makefile.inc
M src/arch/x86/Kconfig
M src/arch/x86/Makefile.inc
M src/arch/x86/assembly_entry.S
M src/arch/x86/include/arch/cpu.h
M src/arch/x86/memlayout.ld
M src/cpu/x86/32bit/entry32.inc
7 files changed, 50 insertions(+), 6 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/37491/3

[Marshall Dawson (Code Review)]

Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37491 )

Change subject: x86: Implement a romstage-only version of RESET_VECTOR_IN_RAM
......................................................................

Patch Set 4:

@Kyösti, @Aaron, Given our change of heart on the hybrid romstage, this change and the next one (CB:33759) will go away as soon as I can get it reworked.  This should simplify a few things and will also allow some of the earlier changes to be cleaner.

The new direction is to always run bootblock, <verstage>, romstage, ramstage just like all other x86 systems.  And, when multiple eligible regions are available, the plan is to always execute an RO bootblock image.  (The RO bootblock is the main driving reason behind a psp_biosimg.rom that sits outside of the typical amdfw.rom.)

[Marshall Dawson (Code Review)]

Marshall Dawson has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/37491 )

Change subject: x86: Implement a romstage-only version of RESET_VECTOR_IN_RAM
......................................................................

Abandoned

We're no longer planning to begin in romstage.  Solution will only begin with bootblock.