Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46603 )
Change subject: sec/intel/txt: Split MTRR setup ASM code into a macro ......................................................................
Patch Set 1: Code-Review+2
(3 comments)
https://review.coreboot.org/c/coreboot/+/46603/1/src/security/intel/txt/gets... File src/security/intel/txt/getsec_enteraccs.S:
https://review.coreboot.org/c/coreboot/+/46603/1/src/security/intel/txt/gets... PS1, Line 7: #include "getsec_mtrr_setup.S" It should probably be called getsec_mtrr_setup.inc then.
https://review.coreboot.org/c/coreboot/+/46603/1/src/security/intel/txt/gets... File src/security/intel/txt/getsec_mtrr_setup.S:
https://review.coreboot.org/c/coreboot/+/46603/1/src/security/intel/txt/gets... PS1, Line 6: #define MTRR_HIGH_MASK $((1 << (CONFIG_CPU_ADDR_BITS - 32)) - 1) Would be nice to do this dynamically with a cpuid call (see cpu/intel/car/p4-netburst/cache_as_ram.S L62)
https://review.coreboot.org/c/coreboot/+/46603/1/src/security/intel/txt/gets... PS1, Line 15: SSE nit: SSE2