Re: [coreboot] Skip CAR setup in FSP?

[Aaron Durbin]

On Thu, Nov 17, 2016 at 4:32 PM, Mason M  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