Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36136 )
Change subject: soc/intel: skl,cnl,icl: rely on TOLUM as cbmem_top returned by FSP ......................................................................
Patch Set 12:
Patch Set 12:
Patch Set 12:
Patch Set 12:
Patch Set 12:
please don't make this change, this is exactly opposite the direction what we decided in past that we shouldn't relying much on FSP in memory snap shot creation and make design where FSP and bootloader have transparent memory design. please look at original CL when it got merged.
What is trying to be achieved here is reducing the coupling between coreboot and FSP for the memory layout. The original patches were attempting to closely track FSP's behavior.
I think below assumption doesn't hold good for CNL onwards platform, thats the reason we had decided to drop the FSP dependency in memory layout design
/* Lookup the FSP_BOOTLOADER_TOLUM_HOB */ if (fsp_find_bootloader_tolum(&tolum)) die("9.3: FSP_BOOTLOADER_TOLUM_HOB missing!\n");
Could you please elaborate on what doesn't hold? Why does this not work?
its 3 year old discussion when we had seen issue with consuming FSP hob to cbmem memory top. Because FSP TOLUM base calculation was different in SKL and CNL (BOOTLOADER TOLUM base and FSP Reserve memory base order) if i'm not wrong.
Top of that Aaron, its you who took stand that why do i relying on FSP to tell me what is my memory start and CB is master not FSP. Then we started looking at transparent memory layout design and come up with this CLs. Now if you are okay with original idea then from SOC side we don't have much objection except a request to don't break other platform with just SKL assumptions.