Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36657 )
Change subject: lib/fmap: Add optional pre-RAM cache ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/c/coreboot/+/36657/5/src/arch/x86/car.ld File src/arch/x86/car.ld:
https://review.coreboot.org/c/coreboot/+/36657/5/src/arch/x86/car.ld@37 PS5, Line 37: . = ALIGN(32);
The FMAP size isn't necessarily a multiple of 32, so if I don't put this here the location counter may be misaligned afterwards. This can cause all sorts of issues down below. I just noticed the memlayout alignment assertions for the PRERAM_CBMEM_CONSOLE and TIMESTAMP sections failing, but I could imagine that a misaligned car_stack is probably also not that great.
Ideally, every section in here would take care of aligning itself (like it's done for PDPT below). But right now most of them don't and it seems that there's rather some assumption that the location counter will always maintain a somewhat sane alignment, so putting this in here seems like the safest solution for now. If someone later wants to refactor the whole file and figure out the precise alignments every individual section needs, we could take it out again.
_car_stack_start is unlikely to have alignment requirements but _car_stack_end does (well the code aligns stack anyway...). Maybe move it a little lower (in the file, address) where we care little about alignment? e.g. below (in the file) _car_ehci_dbg
This file is due for a cleanup using the region macros.