Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34995 )
Change subject: arch/x86: Cache the TSEG region at the top of ram ......................................................................
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.
All that said, I don't think the top of ram nomenclature is helpful here any longer nor do I think we need an API that doesn't save us anything over postcar_frame_add_mtrr() for the top of ram WB case.
top_of_dram_usage name was given that because we were doing few operation there ins same function related to DRAM (setting up postcar frame with DRAM context, now caching TSEG region, enable intermediate caching for DRAM region WP)
Ya. I know where it started from, but in what it is in this patch it doesn't add anything. The automatic addition of TSEG for writeback would be the logical new thing that could be shared.
I know these patches have churned a bit, but that's ok. I think we've sorted out previous assumptions and learned what's currently problematic.
yes, agree, i have learned a lot in terms of those assumption made in past and with current situation with CAT enabling.