Change in coreboot[master]: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87
Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Nick Vaccaro, EricR Lai. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51374 )
Change subject: soc/intel/common/../car: Calculate SF Mask#1 based on MSR 0xc87 ......................................................................
Patch Set 8:
(1 comment)
File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/51374/comment/860239ec_96049de5 PS8, Line 532: mov %al, %cl : mov $0x01, %eax : shl %cl, %eax : subl $0x01, %eax /* contains ((1 << SFWayCnt) - 1) */ : mov %bl, %cl : mov $0x01, %ebx : shl %cl, %ebx : subl $0x01, %ebx /* contains ((1 << data_ways) - 1) */ : sub %ebx, %eax /* contains SF mask */
`edx` is also free at this point, so I think that could be used instead of backing of `ebx` and usin […]
subl %ecx, %edi /* ecx == SFWayCnt - data_ways */
won't edi will get impacted here ? and then line 548 would have wrong restoration ?
Can you please take a look into below code, we don't need edx as well looks like and in this process edi also not getting tampered. Also like 523 and 549 also not needed may be.
ebx == non-eviction mask ecx == number of ways mov %ecx, %edi /* edi == data_ways */ mov $IA32_SF_QOS_INFO, %ecx /* ecx == IA32_SF_QOS_INFO */ rdmsr and $0x3f, %eax /* eax == SFWayCnt */ mov %eax, %ecx /* ecx == eax */ sub %edi, %ecx /* ecx == SFWayCnt - data_ways */ movl $0x01, %eax /* eax == 1 */ shl $cl, %eax /* eax == 1 << (SFWayCnt - data_ways) */ subl $0x01, %eax /* eax == (1 << (SFWayCnt - data_ways)) - 1) */ mov %edi, %ecx /* ecx == data_ways */ shl %cl, %eax /* eax == ((1 << (SFWayCnt - data_ways)) - 1)) << data_ways */
-
Subrata Banik (Code Review)