Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34882 )
Change subject: arch/x86: Move stack with CAR_GLOBAL_MIGRATION ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34882/1/src/include/stddef.h File src/include/stddef.h:
https://review.coreboot.org/c/coreboot/+/34882/1/src/include/stddef.h@39 PS1, Line 39: #if defined(__PRE_RAM__) && CONFIG(ARCH_X86) Why aren't we just modifying these conditions to handle the lzma case (large buffers in early stages w/ CAR)? I believe that's the one object that matters. Everything else should continue to work.
It seems cleaner to:
1. allow static (thus .bss for 0 values) in the !CAR_GLOBAL_MIGRATION cases. 2. Purposefully use stack allocation for the lzma buffer specifically in that compilation unit for CAR_GLOBAL_MIGRATION cases.
Wouldn't that suggestion just be this?
#if defined(__PRE_RAM__) && CONFIG(CAR_GLOBAL_MIGRATION) #define MAYBE_STATIC #else #define MAYBE_STATIC static #endif #define ALLOC_LARGE_BUFFER MAYBE_STATIC
That handles case 1 correctly everywhere, ignoring lzma buffer which would use ALLOC_LARGE_BUFFER.
https://review.coreboot.org/c/coreboot/+/34882/1/src/include/stddef.h@47 PS1, Line 47: #define DECLARE_MAYBE_STATIC_ZERO(x) x = 0
Macros with complex values should be enclosed in parentheses
I'm not a big fan of initializing pointers to 0-value ints.