Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36657 )
Change subject: lib/fmap: Add optional pre-RAM cache ......................................................................
Patch Set 3: Code-Review+1
(3 comments)
I'm starting to wonder if it's not getting easier to define a section for persistent preram objects that you use objcopy to make sure they are the same between early stages or just use the romstage one for all early stages.
https://review.coreboot.org/c/coreboot/+/36657/3/src/include/memlayout.h File src/include/memlayout.h:
https://review.coreboot.org/c/coreboot/+/36657/3/src/include/memlayout.h@79 PS3, Line 79: FMAP_CACHE(addr, sz) Any need to specify the size and not use the one specified by fmap_config.h to match it exactly? If it grows too large would the linker not warn about that too?
https://review.coreboot.org/c/coreboot/+/36657/3/src/lib/fmap.c File src/lib/fmap.c:
https://review.coreboot.org/c/coreboot/+/36657/3/src/lib/fmap.c@64 PS3, Line 64: print_once(BIOS_WARNING, : "WARNING: Post-RAM FMAP access too early for cache!\n"); I guess it could be helpful to add a comment that this code is reached before cbmem_init?
https://review.coreboot.org/c/coreboot/+/36657/3/src/lib/fmap.c@70 PS3, Line 70: BIOS_WARNING BIOS_NOTICE since running without pre-RAM cache is also possible?