Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32414 )
Change subject: arch/x86: Add option for running romstage in DRAM ......................................................................
Patch Set 6:
(4 comments)
https://review.coreboot.org/c/coreboot/+/32414/6/src/Kconfig File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/32414/6/src/Kconfig@161 PS6, Line 161: XIP XIP from boot media.
https://review.coreboot.org/c/coreboot/+/32414/6/src/arch/x86/assembly_entry... File src/arch/x86/assembly_entry.S:
https://review.coreboot.org/c/coreboot/+/32414/6/src/arch/x86/assembly_entry... PS6, Line 43: call gdt_init Wouldn't we want gdt init as well? Or a way to supply a different set of descriptors? I'd expect them to be consistent.
https://review.coreboot.org/c/coreboot/+/32414/6/src/arch/x86/assembly_entry... PS6, Line 49: /* clear CAR_GLOBAL area as it is not shared */
Completely agree. See non-inline comments.
In this case it's effectively .bss which we can update the comments to reflect that. How is romstage being built and loaded? Is it only PROGBITS being loaded?
https://review.coreboot.org/c/coreboot/+/32414/6/src/arch/x86/memlayout.ld File src/arch/x86/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/32414/6/src/arch/x86/memlayout.ld@6... PS6, Line 60: #if ENV_ROMSTAGE && CONFIG(ROMSTAGE_IN_RAM) Why isn't this line up near line 37?