Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36674 )
Change subject: arch/x86: Add option to compress postcar stage ......................................................................
Patch Set 1: Code-Review-2
Patch Set 1: Code-Review+1
I'm a bit confused about how the LZ4 compression works. Who is doing the decompression? romstage or is it sort of done in place? Then caching becomes something to think about (cbmem is uncached there).
romstage is doing the decompression. It is done in-place into the memory area that postcar is supposed to be loaded into (i.e. it loads the compressed file to the end of that area so that its end lands at _epostcar, and then it starts decompressing that front-to-back into _postcar so that it overwrites itself. If that memory area is not cached at the time you do this, you're gonna have a bad time.
It's not cached ATM. Something like CB:34992 could do it however. So the question is whether decompressing to uncached ram is worth the size reduction and therefore slow SPI reads. I guess this needs measuring.
I'm confused because with the work on trying to get non-XIP stages to work, it complained about romstage with the .bss and .data (not sure which one) being too small (probably .car.data discarted by cbfstool -S). But it still complained about .bss for compressing romstage.
Yes, you need a non-zero BSS for this in-place decompression, because the BSS is not part of the image that's being loaded. The BSS is an area of empty space between the end of the decompressed image (_postcar + uncompressed_size) and the end of the buffer (_epostcar). You need a small "gap" there (usually just a few bytes, but the worst-case bound is 8 + uncompressed_size/255) to make sure it doesn't trample over its own feet when overwriting itself. Normally, our BSSes are big enough that this is not a problem, but if you have no BSS at all you'll run into this (and cbfstool verifies that the image can be decompressed cleanly in the available space, so you get that error at build time).
Thanks for clarifying. This explains why I had to use the program.ld for x86 non XIP romstage where .bss is after the program instead of defined separately in car.ld (which gets discarted by cbfstool).