Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35035 )
Change subject: arch/x86: Implement RESET_VECTOR_IN_RAM ......................................................................
Patch Set 9:
(2 comments)
Patch Set 8:
My apologies for being extremely tardy on all of this. Would people appreciate/want me to take some time reviewing this patch more thoroughly?
Yes, please. It's at a point that I finally feel pretty good about it. I looked at your suggestion of reusing assembly_entry.S but felt the necessary changes were detrimental to that file.
Patch Set 8:
Marshall, take commit ownership (reset author) and remove my signed-off-by.
AMD/Google/Silverback have failed to answer questions on x86/PSP interactions and build environments and have not provided NDA paperwork for me to participate in the discussion.
Done. AFAIK it's not possible to change the Owner field within gerrit.
https://review.coreboot.org/c/coreboot/+/35035/8/src/arch/x86/early_dram.ld File src/arch/x86/early_dram.ld:
https://review.coreboot.org/c/coreboot/+/35035/8/src/arch/x86/early_dram.ld@... PS8, Line 27: /* : * .near_reset_vector is used to position code that must be reachable : * from the reset vector. For a program size <= 64KB, this happens by : * default, however with > 64KB it is artificially enforced. : */ : _NEAR_RESET_VECTOR = CONFIG_ROMSTAGE_ADDR + CONFIG_RAM_RESET_VECTOR_STAGE_SIZE - 0x100; : _RESET_VECTOR = CONFIG_ROMSTAGE_ADDR + CONFIG_RAM_RESET_VECTOR_STAGE_SIZE - 0x10; : : . = _NEAR_RESET_VECTOR; : .near_reset_vector . : { : *(.near_reset_vector); : }
Nice. […]
Hmm, sure. AFAIK there's never been a need to worry about it because bootblock is never more than 64K. Do you think that's worth considering (or in the future)? coreboot still essentially allows systems with two different types of bootblocks, and I'm not currently using reset16.ld at all. I'm just trying to picture how things would need to change to make it happen, and whether that ultimately makes things more confusing for the reader.
https://review.coreboot.org/c/coreboot/+/35035/8/src/arch/x86/reset_in_dram_... File src/arch/x86/reset_in_dram_crt0.S:
https://review.coreboot.org/c/coreboot/+/35035/8/src/arch/x86/reset_in_dram_... PS8, Line 33: $(_ebss)
drop the brackets around constants?
Sure. We have differing styles in our .S files. I picked the one I thought would go through without objection, but I prefer without.