Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48286 )
Change subject: src/soc/intel: Add support for CAR_HAS_SF_MASKS and select for TGL ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/48286/2/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/48286/2/src/soc/intel/common/block/... PS2, Line 417: mov %ecx, %edi This seems weird. ebx was backed up to ecx and 2 lines below that ecx is backed up to edi. Why not directly back up ebx to edi?
BTW, Tim had another suggestion on your original CL (https://review.coreboot.org/c/coreboot/+/47983/5/src/soc/intel/common/block/...):
what about using %edx directly? ``` mov %eax, %edx mov $0x01, %eax shl %dl, %eax subl $0x01, %eax ```
https://review.coreboot.org/c/coreboot/+/48286/2/src/soc/intel/common/block/... PS2, Line 436: /* : * Program MSR 0x1892 IA32_CR_SF_QOS_MASK_2 with : * total number of LLC ways : */ : movl $IA32_CR_SF_QOS_MASK_2, %ecx : xorl %edx, %edx : wrmsr This can be dropped completely: https://review.coreboot.org/c/coreboot/+/47983/5/src/soc/intel/common/block/...