Merge pull request #142 from intel/eppfix

consider pkg control value when getting and setting EPP

Author: harp-intel

cmd/config/config.go

@@ -700,8 +700,7 @@ func setEpb(epb int, myTarget target.Target, localTempDir string) {
 
 func setEpp(epp int, myTarget target.Target, localTempDir string) {
 	fmt.Printf("set energy performance profile (EPP) to %d on %s\n", epp, myTarget.GetName())
-	// Mark the per-processor EPP values as invalid, so that the
-	// package EPP value is used. Then set the package EPP value.
+	// Set both the per-core EPP value and the package EPP value
 	// Reference: 15.4.4 Managing HWP in the Intel SDM
 
 	// get the current value of the IAEW_HWP_REQUEST MSR that includes the current EPP valid value in bit 60
@@ -724,12 +723,14 @@ func setEpp(epp int, myTarget target.Target, localTempDir string) {
 		slog.Error("failed to parse msr value", slog.String("msr", stdout), slog.String("error", err.Error()))
 		return
 	}
-	// clear bit 60 in the IA32_HWP_REQUEST MSR value
-	maskedValue := msrValue & 0xEFFFFFFFFFFFFFFF
+	// mask out bits 24-31 IA32_HWP_REQUEST MSR value
+	maskedValue := msrValue & 0xFFFFFFFF00FFFFFF
+	// put the EPP value in bits 24-31
+	eppValue := maskedValue | uint64(epp)<<24
 	// write it back to the MSR
 	setScript := script.ScriptDefinition{
-		Name:          "set epp valid",
-		Script:        fmt.Sprintf("wrmsr -a 0x774 %d", maskedValue),
+		Name:          "set epp",
+		Script:        fmt.Sprintf("wrmsr -a 0x774 %d", eppValue),
 		Superuser:     true,
 		Architectures: []string{"x86_64"},
 		Families:      []string{"6"}, // Intel only
@@ -738,7 +739,7 @@ func setEpp(epp int, myTarget target.Target, localTempDir string) {
 	}
 	_, err = runScript(myTarget, setScript, localTempDir)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "Error: failed to set EPP valid: %v\n", err)
+		fmt.Fprintf(os.Stderr, "Error: failed to set EPP: %v\n", err)
 		return
 	}
 
@@ -754,7 +755,7 @@ func setEpp(epp int, myTarget target.Target, localTempDir string) {
 	}
 	stdout, err = runScript(myTarget, getScript, localTempDir)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "Error: failed to get EPP: %v\n", err)
+		fmt.Fprintf(os.Stderr, "Error: failed to get pkg EPP: %v\n", err)
 		return
 	}
 	msrValue, err = strconv.ParseUint(strings.TrimSpace(stdout), 16, 64)
@@ -766,7 +767,7 @@ func setEpp(epp int, myTarget target.Target, localTempDir string) {
 	// mask out bits 24-31 IA32_HWP_REQUEST_PKG MSR value
 	maskedValue = msrValue & 0xFFFFFFFF00FFFFFF
 	// put the EPP value in bits 24-31
-	eppValue := maskedValue | uint64(epp)<<24
+	eppValue = maskedValue | uint64(epp)<<24
 	// write it back to the MSR
 	setScript = script.ScriptDefinition{
 		Name:          "set epp",
@@ -779,7 +780,7 @@ func setEpp(epp int, myTarget target.Target, localTempDir string) {
 	}
 	_, err = runScript(myTarget, setScript, localTempDir)
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "Error: failed to set EPP: %v\n", err)
+		fmt.Fprintf(os.Stderr, "Error: failed to set pkg EPP: %v\n", err)
 	}
 }
 

internal/report/table_defs.go

@@ -217,6 +217,7 @@ var tableDefinitions = map[string]TableDefinition{
 			script.EpbScriptName,
 			script.EppScriptName,
 			script.EppValidScriptName,
+			script.EppPackageControlScriptName,
 			script.EppPackageScriptName,
 			script.ScalingDriverScriptName,
 			script.ScalingGovernorScriptName},
@@ -461,6 +462,7 @@ var tableDefinitions = map[string]TableDefinition{
 			script.EpbScriptName,
 			script.EppScriptName,
 			script.EppValidScriptName,
+			script.EppPackageControlScriptName,
 			script.EppPackageScriptName,
 			script.ScalingGovernorScriptName,
 			script.UncoreMaxFromMSRScriptName,

internal/report/table_helpers.go

@@ -871,10 +871,16 @@ func eppValToLabel(msr int) string {
 	return fmt.Sprintf("%s (%d)", val, msr)
 }
 
+// eppFromOutput gets EPP value from script outputs
+// IF 0x774[42] is '1' AND 0x774[60] is '0'
+// THEN
+//       get EPP from 0x772 (package)
+// ELSE
+//       get EPP from 0x774 (per core)
 func eppFromOutput(outputs map[string]script.ScriptOutput) string {
-	eppValidConsistent := true
+	// check if the epp valid bit is set and consistent across all cores
 	var eppValid string
-	for i, line := range strings.Split(outputs[script.EppValidScriptName].Stdout, "\n") {
+	for i, line := range strings.Split(outputs[script.EppValidScriptName].Stdout, "\n") { // MSR 0x774, bit 60
 		if line == "" {
 			continue
 		}
@@ -884,14 +890,36 @@ func eppFromOutput(outputs map[string]script.ScriptOutput) string {
 			continue
 		}
 		if currentEpbValid != eppValid {
-			eppValidConsistent = false
-			break
+			slog.Warn("EPP valid bit is inconsistent across cores")
+			return "inconsistent"
 		}
 	}
-	if eppValidConsistent && eppValid == "1" {
-		eppConsistent := true
+	// check if epp package control bit is set and consistent across all cores
+	var eppPkgCtrl string
+	for i, line := range strings.Split(outputs[script.EppPackageControlScriptName].Stdout, "\n") { // MSR 0x774, bit 42
+		if line == "" {
+			continue
+		}
+		currentEppPkgCtrl := strings.TrimSpace(strings.Split(line, ":")[1])
+		if i == 0 {
+			eppPkgCtrl = currentEppPkgCtrl
+			continue
+		}
+		if currentEppPkgCtrl != eppPkgCtrl {
+			slog.Warn("EPP package control bit is inconsistent across cores")
+			return "inconsistent"
+		}
+	}
+	if eppPkgCtrl == "1" && eppValid == "0" {
+		eppPackage := strings.TrimSpace(outputs[script.EppPackageScriptName].Stdout) // MSR 0x772, bits 24-31  (package)
+		msr, err := strconv.ParseInt(eppPackage, 16, 0)
+		if err != nil {
+			return "EPP pkg parse error"
+		}
+		return eppValToLabel(int(msr))
+	} else {
 		var epp string
-		for i, line := range strings.Split(outputs[script.EppScriptName].Stdout, "\n") {
+		for i, line := range strings.Split(outputs[script.EppScriptName].Stdout, "\n") { // MSR 0x774, bits 24-31 (per-core)
 			if line == "" {
 				continue
 			}
@@ -901,30 +929,16 @@ func eppFromOutput(outputs map[string]script.ScriptOutput) string {
 				continue
 			}
 			if currentEpp != epp {
-				eppConsistent = false
-				break
+				slog.Warn("EPP is inconsistent across cores")
+				return "inconsistent"
 			}
 		}
-		if eppConsistent {
-			msr, err := strconv.ParseInt(epp, 16, 0)
-			if err != nil {
-				return "epp parse error"
-			}
-			return eppValToLabel(int(msr))
-		} else {
-			return "Varied"
-		}
-	} else if eppValidConsistent && eppValid == "0" {
-		eppPackage := strings.TrimSpace(outputs[script.EppPackageScriptName].Stdout)
-		msr, err := strconv.ParseInt(eppPackage, 16, 0)
+		msr, err := strconv.ParseInt(epp, 16, 0)
 		if err != nil {
-			return "epp pkg parse error"
+			return "EPP parse error"
 		}
 		return eppValToLabel(int(msr))
-	} else if eppValid != "" {
-		return "Varied"
 	}
-	return ""
 }
 
 func operatingSystemFromOutput(outputs map[string]script.ScriptOutput) string {

internal/script/script_defs.go

@@ -59,6 +59,7 @@ const (
 	EpbScriptName                               = "energy performance bias"
 	EppScriptName                               = "energy performance preference"
 	EppValidScriptName                          = "epp valid"
+	EppPackageControlScriptName                 = "epp package control"
 	EppPackageScriptName                        = "energy performance preference package"
 	IaaDevicesScriptName                        = "iaa devices"
 	DsaDevicesScriptName                        = "dsa devices"
@@ -337,6 +338,15 @@ fi`,
 			Depends:       []string{"rdmsr"},
 			Superuser:     true,
 		},
+		{
+			Name:          EppPackageControlScriptName,
+			Script:        "rdmsr -a -f 42:42 0x774", // IA32_HWP_REQUEST: Energy Performance Preference, bit 42 indicates if package control is enabled
+			Architectures: []string{x86_64},
+			Families:      []string{"6"}, // Intel
+			Lkms:          []string{"msr"},
+			Depends:       []string{"rdmsr"},
+			Superuser:     true,
+		},
 		{
 			Name:          EppScriptName,
 			Script:        "rdmsr -a -f 31:24 0x774", // IA32_HWP_REQUEST: Energy Performance Preference, bits 24-31 (0 is highest perf, 255 is highest energy saving)