Attention is currently required from: Jakub Czapiga, Yu-Ping Wu.
Hello Jakub Czapiga, Yu-Ping Wu,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/78904?usp=email
to review the following change.
Change subject: fmap: Eliminate some impossible code paths ......................................................................
fmap: Eliminate some impossible code paths
When the FMAP cache is enabled, it cannot fail in pre-RAM stages unless flash I/O in general doesn't work. Therefore, it is unnecessary and a waste of binary size to also link a fallback path for this case. Similarly, once the cache is written to CAR/SRAM/CBMEM there should be no way for it to become magically corrupted between boot stages. Many other parts of coreboot blindly assume that persistent memory stays valid between stages so there is no reason why this code should link in extra fallback paths in case it doesn't.
This saves a little over 200 bytes per affected (uncompressed) stage on aarch64.
Change-Id: I7b8251dd6b34fe4f63865ebc44b9a8a103f32a57 Signed-off-by: Julius Werner jwerner@chromium.org --- M src/lib/fmap.c 1 file changed, 11 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/78904/1
diff --git a/src/lib/fmap.c b/src/lib/fmap.c index 62453ed..b63d5a5 100644 --- a/src/lib/fmap.c +++ b/src/lib/fmap.c @@ -31,8 +31,12 @@
static int verify_fmap(const struct fmap *fmap) { - if (memcmp(fmap->signature, FMAP_SIGNATURE, sizeof(fmap->signature))) + if (memcmp(fmap->signature, FMAP_SIGNATURE, sizeof(fmap->signature))) { + if (ENV_INITIAL_STAGE) + printk(BIOS_ERR, "FMAP missing or corrupted at offset 0x%x!\n", + FMAP_OFFSET); return -1; + }
static bool done = false; if (!CONFIG(CBFS_VERIFICATION) || !ENV_INITIAL_STAGE || done) @@ -82,9 +86,8 @@ if (!verify_fmap(fmap)) goto register_cache;
- printk(BIOS_ERR, "FMAP cache corrupted?!\n"); - if (CONFIG(TOCTOU_SAFETY)) - die("TOCTOU safety relies on FMAP cache"); + /* This shouldn't happen, so no point providing a fallback path here. */ + die("FMAP cache corrupted?!\n"); }
/* In case we fail below, make sure the cache is invalid. */ @@ -118,6 +121,10 @@ if (region_device_sz(&fmap_cache)) return rdev_chain_full(fmrd, &fmap_cache);
+ /* Cache setup in pre-RAM stages can't fail, unless flash I/O in general failed. */ + if (!CONFIG(NO_FMAP_CACHE) && ENV_ROMSTAGE_OR_BEFORE) + return -1; + boot_device_init(); boot = boot_device_ro();
@@ -130,8 +137,6 @@ return -1;
if (verify_fmap(fmap)) { - printk(BIOS_ERR, "FMAP missing or corrupted at offset 0x%zx!\n", - offset); rdev_munmap(boot, fmap); return -1; }