[XS] Change in coreboot[main]: soc/amd/common/block/noncar: Merge romstage into bootblock
Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Marshall Dawson, Marvin Drees, Matt DeVillier, Raul Rangel.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64950?usp=email )
Change subject: soc/amd/common/block/noncar: Merge romstage into bootblock ......................................................................
Patch Set 27:
(1 comment)
File src/soc/amd/common/block/cpu/noncar/memlayout_x86.ld:
https://review.coreboot.org/c/coreboot/+/64950/comment/e975e5c3_e6611dd1 : PS27, Line 102: BOOTBLOCK(CONFIG_ROMSTAGE_ADDR, CONFIG_ROMSTAGE_SIZE)
hmm, this ends up doing the right thing, but i found it confusing when looking at it at first. maybe a comment would help? would also be good if that was somehow integrated into the diagram above to have that cover both cases. might also be good to add a comment to the ROMSTAGE_SIZE Kconfig that this will be used as bootblock size in the combined bootblock and romstage case.
a probably better option might be to have different C_ENV_BOOTBLOCK_SIZE sizes dependent on whether SEPARATE_ROMSTAGE is selected in the Kconfig and make ROMSTAGE_SIZE 0 in the !SEPARATE_ROMSTAGE case. not sure if we'd need to put the ROMSATGE macro in here in a #if CONFIG(SEPARATE_ROMSTAGE) block. the calculation of BOOTBLOCK_END and BOOTBLOCK_ADDR would also need to be reworked for that
Currently the base of bootblock gets computed from ROMSTAGE_BASE. If one just increase C_ENV_BOOTBLOCK_SIZE it might overextend downwards overlapping the PSP_SHAREDMEM_SIZE. It does reduce the amount of the amount of DRAM used as ROMSTAGE has an arbitrarily large value which you could tune if using C_ENV_BOOTBLOCK_SIZE. A better way would be to set BOOTBLOCK_BASE in Kconfig and compute romstage base based on that.
-
Arthur Heymans (Code Review)