Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37262 )
Change subject: util: cbfstool: Check alignment at build time ......................................................................
Patch Set 11:
(2 comments)
https://review.coreboot.org/c/coreboot/+/37262/11//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37262/11//COMMIT_MSG@17 PS11, Line 17: don't need to : be aligned, add a '%' after section
What was the reasoning for choosing this vs explicitly noting aligned sections? Is the alignment mag […]
We just noted that so many sections needed to be aligned that we might as well mark the unaligned ones. I think it's a little safer because otherwise one might forget marking a section as aligned that needs it. Also, not all sections that don't need alignment actually need the '%'... only those that don't happen to be aligned already (which many sections are). I would see this more as a warning suppression annotation, you just write the file normally and then it will tell you which sections are unaligned and then you put the % on those after checking that it's safe.
Magnitude is always 4K because we thought that in practice that's the only one that matters (and that allows us to make it a single character, whereas specifying a magnitude explicitly for every section would make the .fmd files look a lot more busy in a way that I don't feel is justified for this).
https://review.coreboot.org/c/coreboot/+/37262/11/Documentation/lib/flashmap... File Documentation/lib/flashmap.md:
https://review.coreboot.org/c/coreboot/+/37262/11/Documentation/lib/flashmap... PS11, Line 114: must be aligned to erase block (e.g., 4k).
Yes, there are different erase sizes. […]
This tests both offset and size alignment, maybe we should make the documentation more explicit about that.
For erase block size... well... in practice I believe the vast majority of chips used on coreboot boards (and all in Chrome OS?) are 4K. Supporting this feature for other sizes gets complicated because the FMAP is fundamentally a compile-time thing, but the SPI code in coreboot currently only handles erase block size at runtime. If we wanted to tie this feature to the true erase block size, I think the only real way to do that would be to have a compile-time Kconfig for it (with the runtime code only checking that it matches the actual chip).
If we ever do that, we can update this stuff to support it (e.g. add a parameter to fmaptool so the Makefile can pass that size in). But until somebody actually comes along to care about a non-4K erase block chip, I don't think we should gate this feature on that, because it's just so uncommon in practice.