Attention is currently required from: Angel Pons, Arthur Heymans.
Jérémy Compostella has posted comments on this change by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/84052?usp=email )
Change subject: car.ld: Fix bug with LLD ......................................................................
Patch Set 9:
(2 comments)
File src/arch/x86/car.ld:
https://review.coreboot.org/c/coreboot/+/84052/comment/f278761b_847e907e?usp... : PS9, Line 7: .car.fspm_rc_heap (CONFIG_DCACHE_RAM_BASE) (NOLOAD) : { Wouldn't it make more sense to put that between line 13 and 14 ? ``` _car_region_start = CONFIG_DCACHE_RAM_BASE; . = CONFIG_DCACHE_RAM_BASE; .car.fspm_rc_heap (CONFIG_DCACHE_RAM_BASE) (NOLOAD) : { . += CONFIG_FSP_M_RC_HEAP_SIZE; } .car.data . (NOLOAD) : { [...] ```
https://review.coreboot.org/c/coreboot/+/84052/comment/89e6acf3_3ec5662a?usp... : PS9, Line 123: . += CONFIG_DCACHE_RAM_MRC_VAR_SIZE; It seems to be mostly used by sandybrige code with the following comment. ``` /* * These are the location and structure of MRC_VAR data in CAR. * The CAR region looks like this: * +------------------+ -> DCACHE_RAM_BASE * | | * | | * | COREBOOT STACK | * | | * | | * +------------------+ -> DCACHE_RAM_BASE + DCACHE_RAM_SIZE * | | * | MRC HEAP | * | size = 0x5000 | * | | * +------------------+ * | | * | MRC VAR | * | size = 0x4000 | * | | * +------------------+ -> DACHE_RAM_BASE + DACHE_RAM_SIZE * + DCACHE_RAM_MRC_VAR_SIZE */ #define DCACHE_RAM_MRC_VAR_BASE (CONFIG_DCACHE_RAM_BASE + CONFIG_DCACHE_RAM_SIZE \ + CONFIG_DCACHE_RAM_MRC_VAR_SIZE - 0x4000) ``` Since `. = _car_region_end;` is gone, how do we ensure we are at `DCACHE_RAM_BASE + DCACHE_RAM_SIZE` ?