Attention is currently required from: Marshall Dawson, Julius Werner, Arthur Heymans, Kyösti Mälkki, Yu-Ping Wu, Felix Held. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56122 )
Change subject: Makefile.inc: Add the x86 bootblock as a regular cbfs file ......................................................................
Patch Set 7:
(2 comments)
Patchset:
PS7: Overall, I think it's looking good if flashrom isn't a problem.
Flashrom defaulting to looking for the platform name in the binary is a poor solution anyway. SMBIOS is probably a better, more universal method.
File Makefile.inc:
https://review.coreboot.org/c/coreboot/+/56122/comment/680a6299_38fa97f5 PS3, Line 840: This top aligns files of type bootblock, assuming that only x86 uses those
Instead of making this assumption, maybe add a Kconfig option that's selected by x86? Up to you […]
You're right, in the current state, the bootblock area is no longer needed on AMD Ryzen systems because we use the compressed version inside the AMD FW area.
I believe that we could actually still use it if we wanted to just by pointing the "BIOS" area in EFS to our uncompressed bootblock region.
As an aside, I'd actually really like to get away from the whole amdfwtool being used to create a bin binary. Except for the additional CBFS headers, I don't see any advantage to amdfwtool's method over just adding all of the various binaries separately in CBFS, then telling a new EFS generation tool where they are. I think that would allow for a lot more flexibility.
-
Martin Roth (Code Review)