Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34912 )
Change subject: rmodule: Add R_386_16 to valid reloc ......................................................................
Patch Set 3:
I was just thinking about this. We do need romstage in the image somewhere. vboot will steer psp to load from the correct location, but the multiple copies needs to reside somewhere. What were your plans on this front?
As I understand it in b:127795737, vboot will return a pointer to the PSP for a PSP Directory Table and not to a romstage image in cbfs. I infer that each table resides in a separate FMAP region. So the decision is made ahead of this code. However this seems like we have two holes to plug:
(1) An image that finds itself running in DRAM will have no idea which table was chosen by vboot, yet will need to load ramstage from the appropriate RO/A/B region in the flash. I'm not sure if there have been discussions for whether/how to get that knowledge from a PSP-based vboot.
(2) Rereading the precise language in the bug, a PSP Directory Table is not the same as the PSP's BIOS Directory Table. I will reopen that discussion, because the BIOS image is described by the BIOS Directory Table, as are microcode patches and PMU firmware. If I could have a separate hybrid-romstage for each RO/A/B, then #1 becomes easier. However if not, the hybrid-romstage should arguably be in RO and then not updateable. That feels like a problem without a current solution.
FWIW the PSP supports 2 (only 2?) directory table levels so that the secondary may be updated without risking bricking the system. But to use this I would still need to place the 2nd level inside an A/B to ensure rogue code can't be run. And I don't know whether it's possible to prevent the PSP from using a 2nd level if it's healthy, i.e. no way to force the primary RO table to be used.
BTW (3) Since we'll need three tables, amdfwtool may still require an update, maybe we want to call it multiple times, or we may be able to duplicate the amdfw.rom images and fixup pointers.