Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33484 )
Change subject: mainboard/facebook/fbg1701: Add FMAP file ......................................................................
Patch Set 2:
(1 comment)
It's not needed and prevents the use of features that automatically update the generated a fmap.
Should renaming fmap.fmd to e.g. secure.fmd be better solution? (and not adding to Kconfig)
Why secure?
I don't see a reason to add a handcrafted FMD, either. The default `CBFS_SIZE` currently set is just wrong, which likely prevents the default FMAP from working. Please note, that the name `CBFS_SIZE` is historical, today it includes all default coreboot regions, e.g. `FMAP` and `RW_MRC_CACHE`, too.
A handcrafted FMD should only be added if it adds features that the default FMAP can't provide.
Clean. Size should be 0x60000.
IMO there should be fmap.fmd to describe the layout of the SPI including the descriptor and ME location.
Well, are these regions used anywhere in coreboot? If not, it would just add redundant information that already exists in the flash descriptor. Which generally makes maintenance harder.
Previous comment was that fmap.fmd must not be supplied. My idea was to have a fmd file available, but rename it to some 'none default' name. Since main features this project are verified boot and measured boot my suggestion is using secure.fmp
Well, if that file adds information necessary for verified/measured boot, that makes sense. But I don't see that yet.
https://review.coreboot.org/#/c/33484/2/src/mainboard/facebook/fbg1701/Kconf... File src/mainboard/facebook/fbg1701/Kconfig:
https://review.coreboot.org/#/c/33484/2/src/mainboard/facebook/fbg1701/Kconf... PS2, Line 57:
My assumption is tht the CBFS_SIZE is 0x5F700 which is available for coreboot. […]
Yes, 0x600000 seems correct in your case. You can also try it and inspect the generated FMAP for yourself, simply set this value to 0x600000 and FMDFILE to an empty string, then build. `build/fmap.fmd` should be auto- matically generated. It's not easy to read because of some decimal numbers, but should roughly reflect what you have in your handcrafted file (wrt. the BIOS region).