Attention is currently required from: Subrata Banik, Kangheui Won, Nick Vaccaro, Julius Werner, Angel Pons, Arthur Heymans, Karthikeyan Ramasubramanian.
Reka Norman has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68641 )
Change subject: arch/x86/postcar_loader: Don't add postcar to stage cache ......................................................................
Patch Set 3:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/68641/comment/8de71b9c_defd8c26 PS2, Line 40: treeya
Can you also test on Zork or MI devices since they have different flow due to psp-verstage?
I think because of PSP verstage they don't use postcar at all - see `depends on !RESET_VECTOR_IN_RAM` for `config POSTCAR_STAGE`.
Patchset:
PS2: Thanks for the info, I always thought it was purely a performance optimisation. Just for my own understanding, how does it work as a security feature since there are other things which are not added to the stage cache, e.g. romstage and FSP-M since they're XIP?
Also https://review.coreboot.org/c/coreboot/+/36674/22 does set up caching for the stage cache region (and cbmem). Maybe that will provide you some performance boosts?
I did try this out previously, see my comments here: https://review.coreboot.org/c/coreboot/+/67669/comments/3a97af53_86f5fe91 . The problem is that CLFLUSHing cbmem and stage cache at the end of romstage adds ~12 ms, mostly because the FSP reserved memory is so large (5M out of ~5.3M total cbmem size). So we waste time running CLFLUSH on it, even though it should be clean because I don't think coreboot writes to FSP reserved memory. I guess we could find a way to solve this, e.g. provide a way to skip flushing certain cbmem entries, but that might get messy.
Another option is to just select NO_STAGE_CACHE for nissa and other recent Intel Chrome OS devices, as Furquan suggested in b:192032803. Although we technically support S3, we only use S0ix on user devices. So we don't care about optimising S3 resume time, and I assume we're not using the stage cache as a security feature since we have vboot. Let me know if anyone sees an issue with doing this, otherwise I'll upload a CL.