Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35377 )
Change subject: lib/fmap: Cache FMAP in cbmem ......................................................................
Patch Set 7:
(5 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@32 PS6, Line 32: static const struct cbmem_entry *fmap_cache CAR_GLOBAL;
I think it's cleaner to have struct mem_region_device as your cache. […]
Done
https://review.coreboot.org/c/coreboot/+/35377/6/src/lib/fmap.c@159 PS6, Line 159: }
If the global to store the cache is a struct mem_region_device, we shouldn't even need a helper... […]
Done
https://review.coreboot.org/c/coreboot/+/35377/6/src/lib/fmap.c@221 PS6, Line 221:
Can't you check fmap_cache value and return early here?
Done
https://review.coreboot.org/c/coreboot/+/35377/6/src/lib/fmap.c@223 PS6, Line 223: if (!e) {
... […]
Done
https://review.coreboot.org/c/coreboot/+/35377/6/src/lib/fmap.c@236 PS6, Line 236: printk(BIOS_ERR, "Failed to read FMAP into CBMEM\n");
This should probably die() because otherwise you'd be running around with a cache full of invalid da […]
Done