Attention is currently required from: Raul Rangel, Furquan Shaikh, Philipp Hug, ron minnich, Subrata Banik, Julius Werner, Arthur Heymans, Andrey Petrov, Aaron Durbin, Patrick Rudolph, Piotr Król, Jason Glenesk, Michał Żygowski, Martin Roth, Marshall Dawson, Ron Minnich, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55068 )
Change subject: [WIP]Allow to build romstage sources inside the bootblock ......................................................................
Patch Set 1:
(8 comments)
File src/drivers/amd/agesa/def_callouts.c:
https://review.coreboot.org/c/coreboot/+/55068/comment/11ebbdb0_e870654a PS1, Line 132: if (!(ENV_ROMSTAGE || (ENV_BOOTBLOCK && !CONFIG(SEPARATE_ROMSTAGE))) Would defining both `ENV_BOOTBLOCK` and `ENV_ROMSTAGE` for the combined stage work? Or does existing code rely on the assumption that they're mutually exclusive?
File src/lib/imd_cbmem.c:
https://review.coreboot.org/c/coreboot/+/55068/comment/3e93de02_a22724c1 PS1, Line 210: EARLY_CBMEM_LIST Option seems to be dead.
File src/lib/prog_loaders.c:
https://review.coreboot.org/c/coreboot/+/55068/comment/9ea27456_d99e524a PS1, Line 4: #include "commonlib/timestamp_serialized.h" Shouldn't this #include use `<>` instead of double quotes?
https://review.coreboot.org/c/coreboot/+/55068/comment/74838706_b93693bb PS1, Line 19: #include <romstage_common.h> x86-specific, apparently.
File src/soc/amd/common/block/pi/def_callouts.c:
https://review.coreboot.org/c/coreboot/+/55068/comment/762c4c23_fe0c651e PS1, Line 25: #if ENV_ROMSTAGE || (ENV_BOOTBLOCK && !CONFIG(SEPARATE_ROMSTAGE))
braces {} are not necessary for single statement blocks
lolwat
File src/southbridge/intel/bd82x6x/early_pch.c:
https://review.coreboot.org/c/coreboot/+/55068/comment/818a0258_26eceaf7 PS1, Line 313: if (ENV_ROMSTAGE || (ENV_BOOTBLOCK && !CONFIG(SEPARATE_ROMSTAGE))) This guard exists because this function is called twice: once in bootblock, once in romstage. I had some patches to clean it up, but they're probably stale by now...
src/southbridge/intel/bd82x6x/bootblock.c: early_pch_init(); src/northbridge/intel/sandybridge/romstage.c: early_pch_init();
File src/southbridge/intel/i82801ix/early_init.c:
https://review.coreboot.org/c/coreboot/+/55068/comment/864dd6e2_f92b7f99 PS1, Line 50: if (ENV_ROMSTAGE || (ENV_BOOTBLOCK && !CONFIG(SEPARATE_ROMSTAGE))) Same as bd82x6x, this function is called twice: once in bootblock, once in romstage. This is the case for both users of this southbridge (gm45 and qemu-q35).
src/southbridge/intel/i82801ix/bootblock.c: i82801ix_early_init(); src/northbridge/intel/gm45/romstage.c: i82801ix_early_init(); src/mainboard/emulation/qemu-q35/romstage.c: i82801ix_early_init();
File src/southbridge/intel/i82801jx/early_init.c:
https://review.coreboot.org/c/coreboot/+/55068/comment/a09d0db0_51e4562f PS1, Line 72: if (ENV_ROMSTAGE || (ENV_BOOTBLOCK && !CONFIG(SEPARATE_ROMSTAGE))) This one is fine because `i82801jx_early_init` is only called from romstage. Still, the guard is pointless. What's called twice is `i82801jx_setup_bars`.
src/northbridge/intel/x4x/romstage.c: i82801jx_early_init();
src/southbridge/intel/i82801jx/early_init.c: i82801jx_setup_bars(); src/southbridge/intel/i82801jx/bootblock.c: i82801jx_setup_bars();