[coreboot-gerrit] [XS] Change in coreboot[main]: soc/intel/common/block/power_limit: Disable RAPL via MSR completely

[Werner Zeh (Code Review)]

Werner Zeh has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85606?usp=email )

Change subject: soc/intel/common/block/power_limit: Disable RAPL via MSR completely
......................................................................

soc/intel/common/block/power_limit: Disable RAPL via MSR completely

Disabling RAPL via Kconfig switch SOC_INTEL_DISABLE_POWER_LIMITS does
not turn off RAPL completely (i.d. MMIO & MSR).
In the past it was assumed disabling RAPL via MCHBAR is sufficient and
the corresponding changes are also reflected in the related
MSR (0x610-PACKAGE_POWER_LIMIT). This is not the case for
Power Limit 2 (PL2) because Bit[47]-PKG_PWR_LIM_2_EN is still set
although PL1 and PL2 were disabled through MCHBAR.

Thus Bit[10]-POWER_LIMITATION_STATUS flag can be set in
MSR 0x19C (THERM_STATUS) when the power limit of the SKU exceeds.
This may lead to a throttling of the domain level frequency.
Moreover related parameters within the same
MSR (0x610-PACKAGE_POWER_LIMIT) like PKG_PWR_LIM_TIME, PKG_CLMP_LIM,
PKG_PWR_LIM have to be cleared as well for both Power Limits
(PL1 & PL2). This is due to the fact that these parameters stray in to
the system and may effect different system settings.

With this commit the PACKAGE_POWER_LIMIT MSR is cleared additionally to
the MCHBAR setting when build for ElkhartLake.

TEST=Verify MSR(0x610-PACKAGE_POWER_LIMIT) is set to zero during OS
runtime except Bit[15]-PKG_PWR_LIM_1_EN (it is known as a bug that this
bit will be set to 1 anyway).
Moreover using a system stress test tool (e.g. Passmark's BurnInTest)
and stressing the system hard should not lead to
Bit[10]-POWER_LIMITATION_STATUS flag being set. This is the case when
MSR (0x610-PACKAGE_POWER_LIMIT) is not cleared completely and the
system is stressed intensively.

Change-Id: I8272339a991667d5ba177f4755ec40e1961d729e
Signed-off-by: Johannes Hahn 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/85606
Reviewed-by: Mario Scheithauer 
Reviewed-by: Werner Zeh 
Tested-by: build bot (Jenkins) 
---
M src/soc/intel/common/block/power_limit/power_limit.c
1 file changed, 8 insertions(+), 0 deletions(-)

Approvals:
  Mario Scheithauer: Looks good to me, approved
  Werner Zeh: Looks good to me, approved
  build bot (Jenkins): Verified

diff --git a/src/soc/intel/common/block/power_limit/power_limit.c b/src/soc/intel/common/block/power_limit/power_limit.c
index b5fbe89..5f71d2e 100644
--- a/src/soc/intel/common/block/power_limit/power_limit.c
+++ b/src/soc/intel/common/block/power_limit/power_limit.c
@@ -91,6 +91,14 @@
    		MCHBAR32(MCH_PKG_POWER_LIMIT_LO) = value & ~(PKG_POWER_LIMIT_EN);
    		value = MCHBAR32(MCH_PKG_POWER_LIMIT_HI);
    		MCHBAR32(MCH_PKG_POWER_LIMIT_HI) = value & ~(PKG_POWER_LIMIT_EN);
+			/* Elkhartlake SoC does not shadow PKG_POWER_LIMIT MCHBAR settings
+			   to MSR correctly. */
+			if (CONFIG(SOC_INTEL_ELKHARTLAKE)) {
+				msr = rdmsr(MSR_PKG_POWER_LIMIT);
+				msr.hi = 0;
+				msr.lo = 0;
+				wrmsr(MSR_PKG_POWER_LIMIT, msr);
+			}
    	} else {
    		msr = rdmsr(MSR_PKG_POWER_LIMIT);
    		msr.lo &= ~PKG_POWER_LIMIT_EN;