Attention is currently required from: Martin L Roth, Julius Werner, Martin Roth.
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71673 )
Change subject: Kconfig: Add option to compress ramstage with LZ4 ......................................................................
Patch Set 1:
(3 comments)
Patchset:
PS1: Interesting that decompression is actually the bottleneck here.
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/71673/comment/19063588_1f2e8106 PS1, Line 173: config MB_COMPRESS_RAMSTAGE_LZ4
I wonder if this should be a variable affecting both payload compression and ramstage compression, because if LZ4 is better for one of them it would likely be better for the other as well? It could be a sort of generic PREFER_LZ4 option that affects all compression choices.
It's quite complicated on x86 as caching has a big impact on performance, which to complicate things is platform dependent. How caching is set changes a lot during boot / at the moment of decompression. For instance when loading ramstage the ROM is often cached, but sometimes not when loading the payload. This also depends on the algo used. For instance with lzma it looks like caching the source does not have a big impact but with zstd the performance changes a lot.
https://review.coreboot.org/c/coreboot/+/71673/comment/9de47621_92ed3df1 PS1, Line 192: Compress ramstage with LZ4 for faster decompression
It would be good to explain the trade-offs involved in this decision (e.g. flash reading speed vs CPU speed and flash space) in more detail in one of these help strings.
That's a pretty complicated to get right because it depends on a lot of things. It's a good idea to document a few things that could impact the decision, but in the end it's a trial and error process since most of the hardware is a black box to us (e.g. caching behavior).
-
Arthur Heymans (Code Review)