On Wed, Feb 27, 2019 at 8:27 AM SteveMooney@sysproconsulting.com wrote:
I noticed in your patch you are calling this host CPU based clearing after the return from FSP memory initialization. If you are calling this in a system with ECC memory, this step is redundant. Part of the initialization of ECC enabled memory is to write a pattern to all of memory to setup the ECC code.
This is a bad idea for many people for a simple reason: sometimes you don't want memory cleared. We deliberately did not clear memory in coreboot for many years, starting in 1999, following the practice of unix: memory setup was the kernel's job and could even be done lazily or on demand. This allows you, even after a power cycle, to do some post mortem in the event of a failure, or even upload the contents to a server for later analysis. This is useful in many places, from supercomputer to data center to laptop, especially in development. Hardwiring memory clearing behavior as part of ECC setup is a poor decision IMHO. It's like doing a lobotomy every time you boot, and after a failure, you REALLY want that memory data. We should at least have a choice.
The preferred way to do this is to read the memory and write it back, with ECC traps disabled, to get the ECC bits set up. Memory contents are preserved and ECC is set up correctly. Especially in this case, the speed tradeoff is not worth it.
The engine is interesting but it's another case of proprietary firmware designers (as in FSP) implementing a neat idea without thinking of user consequences. This is disappointing. Were FSP open source, we could explore the option space, but as it is, we're just twiddling the knobs on a black box. The fact that someone as knowledgeable as Patrick might not have known this tells you how obscure the information is. That's not good.
ron