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 18:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48286/18/src/soc/intel/common/block... File src/soc/intel/common/block/cpu/car/exit_car.S:
https://review.coreboot.org/c/coreboot/+/48286/18/src/soc/intel/common/block... PS18, Line 94: /* Reset CLOS selector to 0 */ : mov $IA32_PQR_ASSOC, %ecx : rdmsr : and $~IA32_PQR_ASSOC_MASK, %edx : wrmsr
If we make following change, COS_MAPPED_TO_MSB has to be selected for all the programs. Right? […]
There is definitely some inconsistency here. I started looking up the old documents to understand why we had bits 33:32 being cleared on older platforms (KBL, CML, APL, GLK, etc.). Here is what I found:
Doc#564550 (SKL eNEM): This refers to 0xc8f as LLC_WAY_MASK_CONTROL and talks about setting lower 2 bits for way selection.
Doc#567985 (APL eNEM): This refers to 0xc8f as IA32_PQR_ASSOC and talks about setting bits 33:32.
Looking at Intel SDM, IA32_PQR_ASSOC says that bits 33:32 represent class of service (CLOS).
Based on the above details, I wonder if bits 1:0 were used for way selection only on older platforms like SKL and newer platforms have actually switched to using bits 33:32 as per SDM. However, it is difficult to get a complete picture here since none of the Processor EDS have IA32_PQR_ASSOC(0xc8f) defined.
Given this inconsistency, I think we should: 1. Take this change for TGL to configure SF mask forward without any other change in exit_car.S 2. Let's track the definition of IA32_PQR_ASSOC for each generation and ensure we set up things correctly for all platforms. It might be the case that COS_MAPPED_TO_MSB would have to be inverted COS_MAPPED_TO_LSB and set only on older platforms like SKL. But, this requires more details from Intel.