On Thu, Nov 17, 2016 at 4:32 PM, Mason M 09mmanning@gmail.com wrote:
Hi coreboot people,
I noticed the option in soc/intel/skylake/Kconfig called SKIP_FSP_CAR, which based on the description, it sounds like the intention is to simply skip the TempRamInit portion of FSP. However, from looking at the code in cache_as_ram.inc, the jump to TempRamInit is still performed, but with an additional #include of a non-existent file called "soc/car_setup.S" beforehand. A little more confusingly, only if the option is set does the code bother to assign the UPD values FspCarBase and FspCarSize a meaningful value in skylake/romstage/romstage.c. Compounding on top of this, the option is enabled by default, seeming to imply it's the preferred way to go for Skylake FSP builds.
Skylake FSP 1.1 still required TempRamInit to be called for some reason. However It used a non-zero MTRR to determine it didn't need to perform CAR initialization, IIRC.
The car_setup.S was removed in https://review.coreboot.org/15784 as well as the selection of USE_GENERIC_FSP_CAR_INC which dictates if src/drivers/intel/fsp1_1/cache_as_ram.inc is employed. You can see how it was and ships on SKL chromebooks in the chromium branch: https://chromium.googlesource.com/chromiumos/third_party/coreboot/+/refs/hea...
The FspCarBase and FspCarSize is way to inform the MemoryInit piece of FSP where CAR actual resides. Now skylake has changed to doing CAR setup in bootblock and those flows aren't taken any more. The TempRamInit is still done, but it's called in bootblock_fsp_temp_ram_init() from bootblock_soc_init() when CONFIG_PLATFORM_USES_FSP1_1 is set.
Can anyone explain to me what the purpose of this option is? Is it simply old code that was accidentally left behind?
Much appreciated, Mason
-- coreboot mailing list: coreboot@coreboot.org https://www.coreboot.org/mailman/listinfo/coreboot