Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35377 )
Change subject: lib/fmap: Cache FMAP in cbmem ......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/c/coreboot/+/35377/6/src/lib/fmap.c File src/lib/fmap.c:
https://review.coreboot.org/c/coreboot/+/35377/6/src/lib/fmap.c@223 PS6, Line 223: if (!e) {
I don't understand the loading from flash 3 times each boot. […]
Yeah, I thought that's what you mean by "always re-seed" here. Maybe I misunderstood.
https://review.coreboot.org/c/coreboot/+/35377/7/src/lib/fmap.c File src/lib/fmap.c:
https://review.coreboot.org/c/coreboot/+/35377/7/src/lib/fmap.c@42 PS7, Line 42: if (cbmem_possibly_online()) {
I don't think this check is necessary as the size check below should always work.
Saves 5 more instructions for pre-RAM stages, I guess?
https://review.coreboot.org/c/coreboot/+/35377/7/src/lib/fmap.c@47 PS7, Line 47: if (!rdev_chain(fmrd, &cache->rdev, 0, region_device_sz(&cache->rdev))) Is there a point in checking this rather than just 'return rdev_chain(...)'? I don't see how it could fail.
https://review.coreboot.org/c/coreboot/+/35377/7/src/lib/fmap.c@212 PS7, Line 212: fmap_cache_setup_late nit: I think register_cache() or find_cache() or something would be more clear than setup_late().