Furquan Shaikh 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/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 19: C_ENVIRONMENT_BOOTBLOCK I know this is for stages post bootblock where the platform uses C environment for bootblock. Just thinking out loud here. It looks like the platform would still have to select this config even though it does not have a real bootblock.
https://review.coreboot.org/c/coreboot/+/32414/6/src/arch/x86/assembly_entry... PS6, Line 23: The gdt is reloaded to accommodate : * platforms that are executing out of CAR. Comments in this file need to be updated in general to account for the changes being made.
https://review.coreboot.org/c/coreboot/+/32414/6/src/arch/x86/assembly_entry... PS6, Line 55: rep stosl
Isn't this now redundant with wiping _car_region_start .. _end in reset_vector. […]
Yes, looks like this is being done twice for this platform.
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?
Probably because romstage.ld in the follow-up patch provides .reset which was being done by reset16.ld here?