Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35025 )
Change subject: soc/{amd, intel}: Make use of common enable_tseg_cache() API ......................................................................
Patch Set 1:
Patch Set 1:
Subrata, you realise this is conflicting CB:34893
CB:34893 is supposed to refactoring code to fit into common romstage.c file if i understood that correctly.
which you previously said is something Intel wants (and you still have not otherwise reviewed).
I mean to say that "Use common romstage code" i like and its too far to reach at that stage using this patch alone. i really wish to maintain exact call sequences common between small cores and big cores and then forward those call into common IP blocks, which might need deep study on entire code that touches some IP initialization sequence and define common path. Not by just making mainboard_romstage_entry()/fill_postcar_frame() name change alone. We are currently working on study path and hopefully get back as and when we have some data ready.
Also, all work on x86-smm-tseg smm_subregion() will just be slowed down if this is merged.
This CB:34995/CB:35025/CB:35026 supposed to address below comments isn't it?
Aaron Durbin Patch Set 4:
Patch Set 4:
Patch Set 4:
I like the idea of adding smm region as WB to the postar mtrr solution. A common API for doing that is helpful . To do it automatically we can add a Kconfig and do this conditional call in run_postcar_phase() or just make the appropriate call in the appropriate romstage.c file.
Got your point, we can create a common API and call that API from run_postcar_phase() or romstage.c but do you think we need any dedicated kconfig when we have TSEG_STAGE_CACHE. My concern is that how do SoC user know if they have to enable TSEG caching as well via another kconfig. I thought it should automatically cache TSEG region but there might be one catch by marking that region as WB as we already learn the WB implication. I guess you are advocating new kconfig just for WB purpose ?
That or just put in calls at the proper places. e.g. replace the smm_region()/postcar_frame_add_mtrr() sequence in apollolake with the correct call. We can follow up with patches which reduce the call sequences from the various code bases with more shared code that could be driven by Kconfigs.