Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43678 )
Change subject: soc/amd/picasso: Don't run dcache_clean_all() in psp_verstage ......................................................................
Patch Set 4:
Patch Set 4: Code-Review+1
I considered another option over chat with Martin. Why not make the dcache_clean_all the weak function?
The benefit: One less layer The cost: PSP has it permanently defined to do nothing. We might actually want such a function to invalidate a PSP cache. Name of function also doesn't follow actual implementation which is confusing.
The biggest issue I see with making dcache_clean_all the weak function is that it's the main function for each architecture that would have to be made weak. There's a definite preference for weak functions to be empty or minimal as we have here with just the return of a value. While it would be one less layer, it would also have to touch every other implementation of dcache_clean_all, so would end up touching more files.
As far as wanting to invalidate the PSP cache, we can't actually do that because the psp_verstage code is running in userspace. If we needed to do it, we'd have to get AMD to add an SVC call, but I don't think it's even an issue for the PSP. The only reason for wanting to clear the cache before a reboot that I can think of is for security, so there's no data with privileged information left in the cache that could be retrieved after a reboot, and since nothing can run in the PSP before the PSP bootloader which must be signed by AMD, I don't think that's a problem.