Attention is currently required from: Julius Werner, Arthur Heymans, Martin Roth, Fred Reitberger.
Martin L Roth 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 2:
(5 comments)
File src/Kconfig:
https://review.coreboot.org/c/coreboot/+/71673/comment/3db89bdf_60d840f1 PS1, Line 173: config MB_COMPRESS_RAMSTAGE_LZ4
I wonder if this should be a variable affecting both payload compression and ramstage compression, […]
Changing FSP-M to LZ4 instead of LZMA slowed things down significantly, so I think it's worth having the options for various different areas.
I do agree that the selection process is somewhat kludgy right now, and maybe we should look at updating the entire compression configuration, but I'd rather do that separately than this patch.
Also, because ChromeOS adds the payload separately from the coreboot build, we need to modify a USE flag to change the payload compression.
https://review.coreboot.org/c/coreboot/+/71673/comment/4127c06f_1b25039e PS1, Line 180: depends on HAVE_RAMSTAGE && !UNCOMPRESSED_RAMSTAGE
I forget, what actually happens when the depends on in a choice is not satisfied? Does that mean bot […]
The option is set to false and hidden. This depends on was previously just lower in the Kconfig, so nothing is actually changed because of this.
https://review.coreboot.org/c/coreboot/+/71673/comment/50145b44_36a4a151 PS1, Line 181: default COMPRESS_RAMSTAGE_LZ4 if MB_COMPRESS_RAMSTAGE_LZ4
Don't you still need a `default COMPRESS_RAMSTAGE_LZMA` after this as well?
No, the first option will be the default if there's no other default specified. If you'd like it specified, I can add the default, but it's not actually needed.
https://review.coreboot.org/c/coreboot/+/71673/comment/87571d69_447ecde5 PS1, Line 185: # Default value set at the end of the file
This comment no longer seems accurate
Thanks. Updated.
https://review.coreboot.org/c/coreboot/+/71673/comment/f8aa7a86_c841030d 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. […]
Yeah, this was kind of a throw-away help string on my part. I don't think I can really offer general advice though, because as arthur says, it's complicated and the speed will depend on the platform.
I think we can definitely say that lZ4 offers faster decompression, but oviously that doesn't always mean a lower boot time.
Maybe
Compress ramstage with LZ4. This offers less compression but better decompression speed - you will need to test the compression options to determine what's best for your platform.
-
Martin L Roth (Code Review)