Attention is currently required from: Furquan Shaikh, Subrata Banik, Arthur Heymans, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56717 )
Change subject: soc/intel/common: Calculate and configure SF Mask 2 ......................................................................
Patch Set 2:
(2 comments)
File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/56717/comment/89d2d578_111ddd00 PS2, Line 522: #if CONFIG(SF_MASK_2BITS_PER_WAY) : mov %edx, %ebx /* back up data_ways in ebx */ : #endif : #if CONFIG(CAR_HAS_SF_MASKS) : /* : * Calculate SF masks: : * if CONFIG_SF_MASK_2BITS_PER_WAY: data_ways = data_ways / 2 : * Program MSR 0x1892 Non-Eviction Mask #2 : * IA32_CR_SF_QOS_MASK_2 = ((1 << data_ways) - 1) : */ : #if CONFIG(SF_MASK_2BITS_PER_WAY) : movl $0x01, %ecx : shr %cl, %ebx : mov %ebx, %ecx : movl $0x01, %ebx : shl %cl, %ebx : subl $0x01, %ebx : #else : mov %esi, %ebx : #endif : mov %ebx, %eax /* restore data_ways in eax */ 1) What if %ebx (# of data ways to protect) is 1 on line 534?
2) I think I have a simplification (if we need to skip the division by 2 I referenced in #1 above, then there are some lines below which are marked as 'for #1') ``` set_eviction_mask: mov %ebx, %edi /* back up *TOTAL* number of ways */ mov %eax, %esi /* back up the non-eviction mask */ ...NEW CODE STARTS HERE mov %edx, %eax /* protected data ways --> eax */
#if CONFIG(CAR_HAS_SF_MASKS) #if CONFIG(SF_MASK_2BITS_PER_WAY) movl $0x01, %ecx #cmp $0x01, %eax // for #1 #jz program_sf2 // for #1 shr %cl, %eax #endif mov %eax, %ecx movl %0x01, %eax shl %cl, %eax subl %0x01, %eax program_sf2: // for #1 xorl %edx, %edx mov $IA32_CR_SF_QOS_MASK_2, %ecx wrmsr ```
https://review.coreboot.org/c/coreboot/+/56717/comment/934451c7_a1ad0ee1 PS2, Line 548: * SF mask is programmed with the double number of bits than : * the number of ways fix this comment?
-
Tim Wawrzynczak (Code Review)