Attention is currently required from: Christian Walter, Arthur Heymans, Kyösti Mälkki, Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54010 )
Change subject: cpu/x86/entry16.S: Make Intel CBnT TOCTOU safe ......................................................................
Patch Set 2: Code-Review+1
(4 comments)
File src/cpu/x86/entry16.S:
https://review.coreboot.org/c/coreboot/+/54010/comment/fa71be8d_690edda2 PS2, Line 135: 2: Since %ebx is not used, how about:
movl $0x60000001, %ebx /* CD, NW, PE = 1 */ #if CONFIG(INTEL_CBNT_SUPPORT) #include <cpu/intel/msr.h> /* Do not disable caching if the BootGuard ACM has set up CAR */ movl $MSR_BOOT_GUARD_SACM_INFO, %ecx rdmsr test $B_BOOT_GUARD_SACM_INFO_NEM_ENABLED, %eax cmovne $0x01, %ebx /* PE = 1 */ #endif movl %cr0, %eax andl $0x7FFAFFD1, %eax /* PG,AM,WP,NE,TS,EM,MP = 0 */ orl %ebx, %eax movl %eax, %cr0
I didn't test this
File src/include/cpu/intel/msr.h:
https://review.coreboot.org/c/coreboot/+/54010/comment/c657f11a_6e7afbf1 PS2, Line 17: For consistency with the other definitions, could you please indent these with tabs?
https://review.coreboot.org/c/coreboot/+/54010/comment/4bebe61c_9a6ea39b PS2, Line 18: nit: the bitfield macros for the other MSRs have an additional space after `#define`
https://review.coreboot.org/c/coreboot/+/54010/comment/d247cf13_b70ab4e1 PS2, Line 23: (1 << 32) Isn't this shift undefined behavior?
-
Angel Pons (Code Review)