Wim Vervoorn has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41694 )
Change subject: [NOTFORMERGE] mb/facebook/fbg1701: Remove C_ENV_BOOTBLOCK_SIZE ......................................................................
Patch Set 1:
Patch Set 1:
I can think of two solutions:
a) Get bootblock start and size from CBFS, something like:
VERIFY_FILE, "bootblock", CBFS_TYPE_BOOTBLOCK
b) Maintain C_ENV_BOOTBLOCK_SIZE under arch/x86 such that when it has a non-zero value, start of bootblock is extended to that size.
I have looked at it. I think the best solution is your 2nd option. This will work for us and it allows you to control bootblock size in case the top block of the flash can be swapped or if you want to permanently lock it for some reason.
Well my preference is option a) but someone else needs to evaluate the following argumentation:
- It is possible that bootblock and romstage come from different builds, i.e. not have exactly the same config.h, while matching C_ENV_BOOTBLOCK_SIZE is required for your implementation to work.
- Seems like VERIFY_BLOCK bypasses cbfs_boot_map_with_leak() call so there is an assumption that all of bootmedia is MMIO mapped.
- Absolute addressing of CBFS files does not play together with multiple FMAP regions.
Thanks for sharing your arguments. I had a look and I think you are right. We can use VERIFY_FILE instead of VERIFY_BLOCK. In fact the verify block was a legacy from our original implementation in Braswell where we treated the actual boot block and the public key as one block. We will have a look and do some testing on this to make sure the coreboot implementation matches the signing scripts.
Changing to file verification for the boot block should solve all of the issues you mention. The issue with a locked boot block can be solved in another way and is anyhow board specific.