Attention is currently required from: Nico Huber, Angel Pons, Patrick Rudolph. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37190 )
Change subject: soc/intel/common/cache_as_ram.S: Add macro to find a free MTRR ......................................................................
Patch Set 8:
(7 comments)
File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/37190/comment/5dac7898_44cd87e5 PS4, Line 27: * macro: find_free_mtrr : * Clobbers %eax, %ebx, %ecx, %edx. : * If not found %ebx is 0. : * If found MTRR_BASE is at %ecx.
Note: the MTRR MSRs are contiguous, and alternating between BASE and MASK: […]
Done
https://review.coreboot.org/c/coreboot/+/37190/comment/7afebbd9_202e0554 PS4, Line 32: .macro find_free_mtrr
nit: Maybe don't indent this?
Both seem to be present in the codebase. Looks a bit better if not indented.
https://review.coreboot.org/c/coreboot/+/37190/comment/dc1c3412_9fba33b1 PS4, Line 43: 2f
right. Personally, I'd prefer names so that code self-documents
I'm not sure if that is a good idea in a macro.
https://review.coreboot.org/c/coreboot/+/37190/comment/0764f059_8d8489a7 PS4, Line 123: jne 1f
Or even jz .halt_forever
je is the same as jz.
File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/37190/comment/3a266b16_7b7f61de PS6, Line 26: /*
The comment looks rather weird now that the macro declaration isn't indented.
Done
https://review.coreboot.org/c/coreboot/+/37190/comment/35c7e5ea_4b44566e PS6, Line 35: put
out
Done
File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/37190/comment/1effd5a5_94f4cdf6 PS7, Line 152: cmp $0, %ebx : jne 1f : jmp .halt_forever
Simpler: […]
Done