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 20:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35035/15/src/arch/x86/memlayout.ld File src/arch/x86/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/35035/15/src/arch/x86/memlayout.ld@... PS15, Line 66: #if !CONFIG(RESET_VECTOR_IN_RAM)
removing the guard results in the build process failing with a segfault. […]
nit: I would prefer guards, if necessary, inside id.ld with some commentary.
Looks like assumed location near 4G to me:
. = (0xffffffff - CONFIG_ID_SECTION_OFFSET) - (__id_end - __id_start) + 1; vs . = CONFIG_X86_RESET_VECTOR - CONFIG_ID_SECTION_OFFSET + 0x10 - (__id_end - __id_start);
While this would not make it appear at the end of coreboot.rom image it would still be in the intermediate bootblock.bin just below reset vector. Although that would not flashrom or FILO at all.
Related CB:37895 and CB:37955 discussion, id.ld and fit.ld considered somewhat broken and true fix is out of the scope.
https://review.coreboot.org/c/coreboot/+/35035/17/src/arch/x86/memlayout.ld File src/arch/x86/memlayout.ld:
https://review.coreboot.org/c/coreboot/+/35035/17/src/arch/x86/memlayout.ld@... PS17, Line 57: #include <arch/x86/id.ld>
(marking as resolved because any solution wouldn't belong into this change)
Since it is far from obvious, I would like to see the lack of ID field and expected flashrom usage complications (-p internal will need some force?) mentioned in the commit message. Preferably this would also be filed as a bug, if you agree it breaks existing expectations on what coreboot.rom files look like and you have no absolute need to leave the ID out on these platforms.