Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47828 )
Change subject: soc/amd: move non-CAR linker scripts to common directory ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47828/2/src/soc/amd/common/block/cp... File src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld:
https://review.coreboot.org/c/coreboot/+/47828/2/src/soc/amd/common/block/cp... PS2, Line 32: * | |X86_RESET_VECTOR = ROMSTAGE_ADDR + ROMSTAGE_SIZE - 0x10 Is this correct?
There is assert on:
BOOTBLOCK_ADDR + C_ENV_BOOTBLOCK_SIZE - 0x10 == X86_RESET_VECTOR
https://review.coreboot.org/c/coreboot/+/47828/2/src/soc/amd/common/block/cp... PS2, Line 85: _ = ASSERT((CONFIG_BOOTBLOCK_ADDR + CONFIG_C_ENV_BOOTBLOCK_SIZE - 0x10) == CONFIG_X86_RESET_VECTOR, "Reset vector should be -0x10 from end of bootblock"); The assertion here reveals redundancy in the use of Kconfigs. You do not need to define X86_RESET_VECTOR separately. Also X86_RESET_VECTOR help text forgets to mention there is architectural requirement for the address to end with 0xfff0.