Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30855 )
Change subject: arch/x86: Align _start16bit with C_ENVIRONMENT_BOOBLOCK ......................................................................
Patch Set 2:
Patch Set 2:
So we could isolate top 4 KiB? Use it for entry16, entry32, walkcbfs, update_microcode?
I'll try to learn how to do that, there's no rush submitting this one.
Yes, we could do that. There's some gotchas there in how we add FIT entries and bizarre platform specifics such as apollolake. That said, I believe putting in a static assert for our current implementation of C_ENV_BOOTBLOCK_SIZE <= 64KiB would be sufficient. If we need to make it larger then we could look at fixing the location of those pieces of code. After working through the math, current implementation, and adding a static assert inherently documents and protects the current implementation. I'm fine with submitting this and following up.
This could be put in a better place, but we just need something to fail during compilation:
diff --git a/src/arch/x86/boot.c b/src/arch/x86/boot.c index 7819c0f2db..2b4a1c3c55 100644 --- a/src/arch/x86/boot.c +++ b/src/arch/x86/boot.c @@ -41,3 +41,11 @@ void arch_prog_run(struct prog *prog) :: "D"(prog_entry(prog)) ); } + +#if IS_ENABLED(CONFIG_C_ENV_BOOTBLOCK) +/* Please see src/cpu/x86/16bit/reset16.inc and the jump to _start16bit from the reset vector. + * The current implementation requires a max jump (currently includes math wrap around) size + * of 64 KiB. This check assumes the reset vector is at the end of bootblock and _start16bit + * symbol being farthest away at the beginning of bootblock. */ +_Static_assert(CONFIG_C_ENV_BOOTBLOCK_SIZE <= 64*KiB); +#endif