From 245d81561b521bb94e3aa88fb704b967b023b0f1 Mon Sep 17 00:00:00 2001
From: Michael Pratt <mpratt@google.com>
Date: Tue, 30 Oct 2018 15:55:22 -0700
Subject: [PATCH] Clean up cpuid_parse_test

Actually parse flags from cpuinfo to avoid mistakenly matching
substrings in cpuinfo that happen to match a flags.

Some features were only exposed in recent versions of Linux. Don't
require them to appear in cpuinfo on old versions of Linux.

Move PREFETCHWT1 back to parse only features. It isn't actually exposed
in Linux yet. Move SDBG to shown features. It has been visible since
Linux 4.3.

PiperOrigin-RevId: 219381731
Change-Id: Ied7c0ee7c8a9879683e81933de56c9074b01108f
---
 pkg/cpuid/cpuid.go            | 12 +++--
 pkg/cpuid/cpuid_parse_test.go | 99 +++++++++++++++++++++++++++++++++--
 2 files changed, 101 insertions(+), 10 deletions(-)

diff --git a/pkg/cpuid/cpuid.go b/pkg/cpuid/cpuid.go
index f4b1db896..c19606898 100644
--- a/pkg/cpuid/cpuid.go
+++ b/pkg/cpuid/cpuid.go
@@ -274,6 +274,7 @@ var x86FeatureStrings = map[Feature]string{
 	X86FeatureTM2:      "tm2",
 	X86FeatureSSSE3:    "ssse3",
 	X86FeatureCNXTID:   "cid",
+	X86FeatureSDBG:     "sdbg",
 	X86FeatureFMA:      "fma",
 	X86FeatureCX16:     "cx16",
 	X86FeatureXTPR:     "xtpr",
@@ -352,10 +353,9 @@ var x86FeatureStrings = map[Feature]string{
 	X86FeatureAVX512VL:   "avx512vl",
 
 	// Block 3.
-	X86FeaturePREFETCHWT1: "prefetchwt1",
-	X86FeatureAVX512VBMI:  "avx512vbmi",
-	X86FeatureUMIP:        "umip",
-	X86FeaturePKU:         "pku",
+	X86FeatureAVX512VBMI: "avx512vbmi",
+	X86FeatureUMIP:       "umip",
+	X86FeaturePKU:        "pku",
 
 	// Block 4.
 	X86FeatureXSAVEOPT: "xsaveopt",
@@ -405,7 +405,6 @@ var x86FeatureStrings = map[Feature]string{
 // flags, but will not get printed out in /proc/cpuinfo.
 var x86FeatureParseOnlyStrings = map[Feature]string{
 	// Block 0.
-	X86FeatureSDBG:    "sdbg",
 	X86FeatureOSXSAVE: "osxsave",
 
 	// Block 2.
@@ -414,6 +413,9 @@ var x86FeatureParseOnlyStrings = map[Feature]string{
 	X86FeatureIPT:             "pt",
 	X86FeatureCLFLUSHOPT:      "clfushopt",
 
+	// Block 3.
+	X86FeaturePREFETCHWT1: "prefetchwt1",
+
 	// Block 4.
 	X86FeatureXSAVES: "xsaves",
 }
diff --git a/pkg/cpuid/cpuid_parse_test.go b/pkg/cpuid/cpuid_parse_test.go
index 81b06f48c..954ed53b6 100644
--- a/pkg/cpuid/cpuid_parse_test.go
+++ b/pkg/cpuid/cpuid_parse_test.go
@@ -15,23 +15,112 @@
 package cpuid
 
 import (
+	"fmt"
 	"io/ioutil"
+	"regexp"
+	"strconv"
 	"strings"
+	"syscall"
 	"testing"
 )
 
-// TestHostFeatureFlags ensures that package cpuid recognizes all features
-// present on this host.
+func kernelVersion() (int, int, error) {
+	var u syscall.Utsname
+	if err := syscall.Uname(&u); err != nil {
+		return 0, 0, err
+	}
+
+	var r string
+	for _, b := range u.Release {
+		if b == 0 {
+			break
+		}
+		r += string(b)
+	}
+
+	s := strings.Split(r, ".")
+	if len(s) < 2 {
+		return 0, 0, fmt.Errorf("kernel release missing major and minor component: %s", r)
+	}
+
+	major, err := strconv.Atoi(s[0])
+	if err != nil {
+		return 0, 0, fmt.Errorf("error parsing major version %q in %q: %v", s[0], r, err)
+	}
+
+	minor, err := strconv.Atoi(s[1])
+	if err != nil {
+		return 0, 0, fmt.Errorf("error parsing minor version %q in %q: %v", s[1], r, err)
+	}
+
+	return major, minor, nil
+}
+
+// TestHostFeatureFlags tests that all features detected by HostFeatureSet are
+// on the host.
+//
+// It does *not* verify that all features reported by the host are detected by
+// HostFeatureSet.
+//
+// i.e., test that HostFeatureSet is a subset of the host features.
 func TestHostFeatureFlags(t *testing.T) {
 	cpuinfoBytes, _ := ioutil.ReadFile("/proc/cpuinfo")
 	cpuinfo := string(cpuinfoBytes)
 	t.Logf("Host cpu info:\n%s", cpuinfo)
 
-	for f := range HostFeatureSet().Set {
-		if f.flagString(false) == "" {
+	major, minor, err := kernelVersion()
+	if err != nil {
+		t.Fatalf("Unable to parse kernel version: %v", err)
+	}
+
+	re := regexp.MustCompile(`(?m)^flags\s+: (.*)$`)
+	m := re.FindStringSubmatch(cpuinfo)
+	if len(m) != 2 {
+		t.Fatalf("Unable to extract flags from %q", cpuinfo)
+	}
+
+	cpuinfoFlags := make(map[string]struct{})
+	for _, f := range strings.Split(m[1], " ") {
+		cpuinfoFlags[f] = struct{}{}
+	}
+
+	fs := HostFeatureSet()
+
+	// All features have a string and appear in host cpuinfo.
+	for f := range fs.Set {
+		name := f.flagString(false)
+		if name == "" {
 			t.Errorf("Non-parsable feature: %v", f)
 		}
-		if s := f.flagString(true); !strings.Contains(cpuinfo, s) {
+
+		// Special cases not consistently visible. We don't mind if
+		// they are exposed in earlier versions.
+		switch {
+		// Block 0.
+		case f == X86FeatureSDBG && (major < 4 || major == 4 && minor < 3):
+			// SDBG only exposed in
+			// b1c599b8ff80ea79b9f8277a3f9f36a7b0cfedce (4.3).
+			continue
+		// Block 3.
+		case f == X86FeatureAVX512VBMI && (major < 4 || major == 4 && minor < 10):
+			// AVX512VBMI only exposed in
+			// a8d9df5a509a232a959e4ef2e281f7ecd77810d6 (4.10).
+			continue
+		case f == X86FeatureUMIP && (major < 4 || major == 4 && minor < 15):
+			// UMIP only exposed in
+			// 3522c2a6a4f341058b8291326a945e2a2d2aaf55 (4.15).
+			continue
+		case f == X86FeaturePKU && (major < 4 || major == 4 && minor < 9):
+			// PKU only exposed in dfb4a70f20c5b3880da56ee4c9484bdb4e8f1e65
+			// (4.9).
+			continue
+		}
+
+		hidden := f.flagString(true) == ""
+		_, ok := cpuinfoFlags[name]
+		if hidden && ok {
+			t.Errorf("Unexpectedly hidden flag: %v", f)
+		} else if !hidden && !ok {
 			t.Errorf("Non-native flag: %v", f)
 		}
 	}