Change in coreboot[master]: Makefile.inc: Generate master header and pointer as C structs
Attention is currently required from: Martin L Roth, Julius Werner, Arthur Heymans, Yu-Ping Wu. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59132 )
Change subject: Makefile.inc: Generate master header and pointer as C structs ......................................................................
Patch Set 9:
(5 comments)
File src/lib/cbfs_master_header.c:
https://review.coreboot.org/c/coreboot/+/59132/comment/6d738dd9_29bcc494 PS9, Line 7: __attribute((used))
Same problem here as the other one.
Done
https://review.coreboot.org/c/coreboot/+/59132/comment/d2292ee2_3cb975ab PS9, Line 10: /* The offset and romsize fields within the master header are absolute
nit: https://doc.coreboot.org/contributing/coding_style. […]
Done
https://review.coreboot.org/c/coreboot/+/59132/comment/214fe68f_7ffd2de3 PS9, Line 11: relfect
typo
Done
https://review.coreboot.org/c/coreboot/+/59132/comment/9fb49e41_adba7e70 PS9, Line 16: FMAP_SECTION_COREBOOT_START + FMAP_SECTION_COREBOOT_SIZE
Okay, I figured out how that memory-mapped offset thing in these headers works now (and it's honestly pretty horrible, we should try getting rid of that. It seems completely arbitrary which .fmd files define a FLASH@xxx and which don't. Most of our APIs that take a flash offset (e.g. boot_device_ro()) would not know how to deal with a memory-mapped offset like that. I think we only get away with that because we barely use these header constants anymore anyway). But then this line is a problem, because if it's using the memory-mapped offsets this would be a huge number. So you also need to subtract FMAP_SECTION_FLASH_START here again.
agreed.
File src/lib/master_header_pointer.c:
https://review.coreboot.org/c/coreboot/+/59132/comment/bd726dd9_d3bcc034 PS7, Line 13: ATTRIBUTES uint32_t header_pointer =
I think an easier way to write this is […]
Done
-
Arthur Heymans (Code Review)