Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35035 )
Change subject: arch/x86: Implement RESET_VECTOR_IN_RAM ......................................................................
Patch Set 21:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35035/20/src/arch/x86/assembly_entr... File src/arch/x86/assembly_entry.S:
https://review.coreboot.org/c/coreboot/+/35035/20/src/arch/x86/assembly_entr... PS20, Line 24: call gdt_init
I can clean it up later.
I left some comments about the stack and .earlyram.data.
Pretty much everything has changed from early April wrt executed x86 stages. You can, of course, evaluate once again if car.ld would be more suitable for you instead of the separate earlyram.ld approach. The isolation was suggested partly due the (scheduled) CAR_MIGRATION removal work that would likely have hit merge conflicts both ways.
https://review.coreboot.org/c/coreboot/+/35035/21/src/arch/x86/early_ram.ld File src/arch/x86/early_ram.ld:
https://review.coreboot.org/c/coreboot/+/35035/21/src/arch/x86/early_ram.ld@... PS21, Line 35: _bogus2 = ASSERT(. <= CONFIG_X86_RESET_VECTOR, "Earlyram data regions don't fit below the reset vector!"); This can probably wait for verstage development, but I'll comment early on.
a) You may need to lock the layout of .earlyram.data with first shipped bootblock. Depends of how your root-of-trust will look like. b) Stack requirement with Intel FSP2.1 has blown thru the roof to some 128KB. While that is mostly due to raminit, you may want to have some reserve there for the stack. c) Increasing stack size could make _start16bit too far away from reset vector. You may need to reconsider .near_reset_vector or complete CB:37895. d) The motivation to share stack for early stages was to spare cachelines. You don't have such restrictions. e) The section .earlyram.data appears inside bootblock stage. Traditionally next stage was allowed to discard previous stage program section. f) If verstage shall verify bootblock integrity (althought it was already executed), you only have the modified bootblock program region left in DRAM and the compressed one in SPI.
Just saying, I think .earlyram.data was better kept separate and outside bootblock program. I did not look for argumentation why and when this changed from some of the early patchsets.