Frans Hendriks 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)
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.
The solution: abondon this patchset.
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:
Yes, 0x600000 seems correct in your case. You can also try it and inspect […]
Build without custom FMAP and CBFS_SIZE to 0x600000 and the sizes are equal. The generated ROM works fine.