Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35140 )
Change subject: [WIP] arch/x86: Refactor CAR_GLOBAL quirk for FSP1.0 ......................................................................
Patch Set 5:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35140/2/src/cpu/x86/car.c File src/cpu/x86/car.c:
https://review.coreboot.org/c/coreboot/+/35140/2/src/cpu/x86/car.c@97 PS2, Line 97: uintptr_t *mig_var = car_get_var_ptr(var);
Could you add some comments here? […]
Ack
https://review.coreboot.org/c/coreboot/+/35140/4/src/cpu/x86/car.c File src/cpu/x86/car.c:
https://review.coreboot.org/c/coreboot/+/35140/4/src/cpu/x86/car.c@82 PS4, Line 82: Pointers to CAR_GLOBALs use relative addressing
What ensures dst is a location in CAR_GLOBALs region? […]
Changed the text. After CBMEM comes online dst will be outside _car_region. We could try add some assert() here, but this code is doomed with next release and there has been no FSP1.0 development for some 5 years now.
https://review.coreboot.org/c/coreboot/+/35140/4/src/cpu/x86/car.c@86 PS4, Line 86: dst
This would be clearer if it was 'val' or something like that.
Also s/mig_var/offset, not sure if it made this better or worse.