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 17:
(4 comments)
https://review.coreboot.org/c/coreboot/+/48286/17/src/cpu/Kconfig File src/cpu/Kconfig:
https://review.coreboot.org/c/coreboot/+/48286/17/src/cpu/Kconfig@24 PS17, Line 24: hex Can you please add help text that outlines what this is and what it is used for?
https://review.coreboot.org/c/coreboot/+/48286/17/src/soc/intel/common/block... File src/soc/intel/common/block/cpu/car/exit_car.S:
https://review.coreboot.org/c/coreboot/+/48286/17/src/soc/intel/common/block... PS17, Line 100: IA32_L3_MASK_1 Questions: 1. CLOS selector is set to 0 above. Once that is done, do the L3_MASK and SF_QOS_MASK have any impact? Or are they effectively disabled?
2. If we don't ever configure these bits here, wouldn't it mean that the entity that uses it next is responsible for setting up the L3_MASK and SF_QOS_MASK before enabling CLOS selector again? Is that a dangerous assumption to make?
https://review.coreboot.org/c/coreboot/+/48286/17/src/soc/intel/common/block... PS17, Line 101: CONFIG_IA32_L3_MASK_1_DEFAULT Since this is used by all soc/intel/ platforms, what is getting programmed here for platforms other than TGL?
https://review.coreboot.org/c/coreboot/+/48286/17/src/soc/intel/tigerlake/Kc... File src/soc/intel/tigerlake/Kconfig:
https://review.coreboot.org/c/coreboot/+/48286/17/src/soc/intel/tigerlake/Kc... PS17, Line 255: 0xffff Isn't the default mask dependent on number of ways?