Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30500 )
Change subject: [WIP]arch/x86/postcar: Add x86_64 support ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/#/c/30500/4/src/arch/x86/exit_car.S File src/arch/x86/exit_car.S:
https://review.coreboot.org/#/c/30500/4/src/arch/x86/exit_car.S@32 PS4, Line 32: .macro pop_mtrr Maybe pop_edx_eax ?
https://review.coreboot.org/#/c/30500/4/src/arch/x86/exit_car.S@50 PS4, Line 50: /* Migrate GDT to this text segment */ Keep empty line above.
https://review.coreboot.org/#/c/30500/4/src/arch/x86/exit_car.S@118 PS4, Line 118: pop %ebx /* Number to clear */ It's fine like this.
We could easily reduce the remaining #ifdef here though, if we used %bh/%bl for these loop counts (MTRR counts are < 255). But that's just my opinion of what makes asm less readable.
https://review.coreboot.org/#/c/30500/4/src/arch/x86/exit_car.S@165 PS4, Line 165: /* Align stack to 16 bytes at call instruction. */ Keep empty line?
-
Kyösti Mälkki (Code Review)