Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47899 )
Change subject: mb/google/hatch: Drop use of SPD cache for puff-based variants ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/47899/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/47899/1//COMMIT_MSG@11 PS1, Line 11: it's actually slower than simply letting FSP (vs coreboot) : read the SPD data via smbus, so drop it.
but it doesn't - retraining works just fine either way
Isn't the serial number in SPD cache compared to decide whether memory should be retrained when a DIMM is added/removed: https://review.coreboot.org/cgit/coreboot.git/tree/src/lib/spd_cache.c?id=re...
That is where the original bug started i.e. on adding or removing a DIMM, memory retraining wasn't triggered and so the SPD cache was added.
https://review.coreboot.org/c/coreboot/+/47899/1//COMMIT_MSG@19 PS1, Line 19: Test: build/boot WYVERN variant, check boot times via cbmem: : w/SPD caching: ~722 ms : w/FSP reading: ~627 ms
I don't understand why there is a difference from original implementation. […]
Thanks for the numbers Matt. That is helpful.
I am just curious why SPD cache takes more time in practice. Looking at the code, it seems to be just passing in the memory-mapped address of the SPD cache region to FSP. So, ideally it should be just reading from that address, which I think would be faster than performing SMBUS operation. Did you check where exactly the boot time savings are? Is it all inside FSP-M?