Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44673 )
Change subject: cpu/intel/car/non-evict/cache_as_ram.S: Add support for longmode ......................................................................
Patch Set 3: Code-Review+1
(3 comments)
https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... File src/cpu/intel/car/non-evict/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... PS3, Line 12: /* Trick the linker into supporting x86_64 relocations in 32bit code */ : #ifdef __x86_64__ : rom_mtrr_mask: : .quad _rom_mtrr_mask : : rom_mtrr_base: : .quad _rom_mtrr_base : : car_mtrr_mask: : .quad _car_mtrr_mask : #else : rom_mtrr_mask: : .long _rom_mtrr_mask : : rom_mtrr_base: : .long _rom_mtrr_base : : car_mtrr_mask: : .long _car_mtrr_mask : #endif I think it would be a good idea to put these in a separatly linked file (with .global), to reduce boilerplate in other CAR setup files.
https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... PS3, Line 110: _car_mtrr_start Maybe be consistent about this one?
https://review.coreboot.org/c/coreboot/+/44673/3/src/cpu/intel/car/non-evict... PS3, Line 236:
A single space should be enough
This is the string cpuid reports in 0x80000002-0x80000004.