Attention is currently required from: Furquan Shaikh, Rizwan Qureshi, Subrata Banik, Angel Pons. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48344 )
Change subject: soc/intel/common/block/cpu: Introduce CAR_HAS_L3_PROTECTED_WAYS Kconfig ......................................................................
Patch Set 12:
(3 comments)
File src/soc/intel/common/block/cpu/car/cache_as_ram.S:
https://review.coreboot.org/c/coreboot/+/48344/comment/a7752376_ae9707b0 PS12, Line 517: mov %ebx, %ecx /* back up number of ways */ : mov %eax, %ebx /* back up the non-eviction mask*/ Thinking out loud (for a later change): Now that there are `wrmsr` and `rdmsr` calls below, maybe we should not use `ecx` for a backup location, since it is required to be used for the msr instructions (obv. not eax or edx either) and thus has to be backed-up several times. edi and esi seem like good backup locations (the only `rep` prefix is in the clear_car macro)
https://review.coreboot.org/c/coreboot/+/48344/comment/f6d26406_d33b074a PS12, Line 558: %bl, shouldn't this be %edi (data_ways) ? ebx contains the non-eviction mask at this point
https://review.coreboot.org/c/coreboot/+/48344/comment/f95d25ae_de641bf4 PS12, Line 557: mov $0x01, %eax : mov %bl, %cl : shl %cl, %eax : subl $0x01, %eax doesn't `ebx` already hold the correct value (the data ways non-eviction mask) to program into the MSR at this point? `(1 << data_ways) - 1` ?