Perf list fix for *6i.16xl with patched excelize package

cmd/metrics/metadata.go

@@ -52,8 +52,69 @@ const (
 	scriptARMSlots               = "arm slots"
 	scriptARMCPUID               = "arm cpuid"
 	scriptPerfStatAMDUncoreProbe = "perf stat amd uncore probe"
+	scriptPMUEnumerationSafety   = "pmu enumeration safety"
 )
 
+// Checks whether fixed purpose counters are exposed and that the slots event is exposed
+// Prevents kernel faults when trying to access virtualized PMUs
+// So far only seen on AWS m6i.16xl instance
+var pmuEnumerationSafetyScript = script.ScriptDefinition{
+	Name: scriptPMUEnumerationSafety,
+	// Need both "fixed-purpose events" and "fixed-purpose counters" to be recognized.
+	// It changed in kernel 6.15.
+	ScriptTemplate: `fixed=$(dmesg 2>/dev/null | grep -oE 'fixed-purpose (events|counters):[[:space:]]*[0-9]+' | grep -oE '[0-9]+$' | tail -1)
+slots=no
+# Unmatched globs stay literal here, and [[ -e ]] on a literal is false, so a machine
+# with no such event correctly reports "no" rather than matching the pattern itself.
+for event in /sys/bus/event_source/devices/cpu*/events/slots \
+	/sys/bus/event_source/devices/cpu*/events/topdown-*; do
+	[[ -e "$event" ]] && slots=yes
+done
+echo "fixed_purpose_counters=${fixed:-unknown}"
+echo "slots_event_exposed=$slots"
+`,
+	Architectures: []string{cpus.X86Architecture},
+}
+
+// Event enumeration from sysfs, for targets where asking perf to do it would fault the
+// kernel (see pmuEnumerationSafetyScript). Reading sysfs opens no events, so it cannot
+// fault. Anything absent from it is then dropped from the metric definitions
+// by the existing IsCollectable checks, so metric collection cannot walk into the fault
+// later either. Output must match the shape 'perf list --json' is parsed into.
+const sysfsEventEnumerationPreamble = `# See pmuEnumerationSafetyScript in cmd/metrics/metadata.go for why this reads sysfs
+# rather than asking perf to enumerate events.
+for events_dir in /sys/bus/event_source/devices/*/events; do
+	[[ -d "$events_dir" ]] || continue
+	pmu=${events_dir%/events}
+	pmu=${pmu##*/}
+	for event_path in "$events_dir"/*; do
+		[[ -f "$event_path" ]] || continue
+		event=${event_path##*/}
+		# Sibling metadata files, not events in their own right.
+		case "$event" in
+		*.scale | *.unit | *.snapshot | *.per-pkg) continue ;;
+		esac
+`
+
+// Hardware events plus cstate and power events: the same selection the awk filter makes
+// from 'perf list --json' output.
+const sysfsSupportedEventsScript = sysfsEventEnumerationPreamble + `		case "$pmu" in
+		cpu | cpu_core | cpu_atom) echo "$event" ;;
+		cstate_core | cstate_pkg | power) echo "$pmu/$event/" ;;
+		esac
+	done
+done
+`
+
+// Every event the kernel publishes, from every PMU, unfiltered.
+const sysfsAllSupportedEventsScript = sysfsEventEnumerationPreamble + `		case "$pmu" in
+		cpu | cpu_core | cpu_atom) echo "$event" ;;
+		*) echo "$pmu/$event/" ;;
+		esac
+	done
+done
+`
+
 // CommonMetadata -- common to all architectures
 type CommonMetadata struct {
 	NumGeneralPurposeCounters int
@@ -302,14 +363,28 @@ BEGIN {
 
 // getMetadataScripts returns the list of scripts to run for metadata collection.
 // It copies the base definitions and applies template replacements and privilege settings.
-func getMetadataScripts(noRoot bool, noSystemSummary bool, numGPCounters int) ([]script.ScriptDefinition, error) {
+//
+// enumerateEventsFromSysfs replaces the two 'perf list --json' scripts with sysfs reads,
+// for targets where letting perf enumerate events would fault the kernel.
+func getMetadataScripts(noRoot bool, noSystemSummary bool, numGPCounters int, enumerateEventsFromSysfs bool) ([]script.ScriptDefinition, error) {
 	metadataScripts := make([]script.ScriptDefinition, 0, len(baseMetadataScripts))
 
 	// Copy base scripts and apply settings
 	for _, baseDef := range baseMetadataScripts {
 		scriptDef := baseDef
 		scriptDef.Superuser = !noRoot
 
+		if enumerateEventsFromSysfs {
+			switch scriptDef.Name {
+			case scriptPerfSupportedEvents:
+				scriptDef.ScriptTemplate = sysfsSupportedEventsScript
+				scriptDef.Depends = nil // sysfs enumeration does not need perf
+			case scriptPerfAllSupportedEvents:
+				scriptDef.ScriptTemplate = sysfsAllSupportedEventsScript
+				scriptDef.Depends = nil
+			}
+		}
+
 		// Apply template replacements for fixed counter scripts
 		switch scriptDef.Name {
 		case scriptPerfStatFixedInstr:
@@ -346,6 +421,61 @@ func getMetadataScripts(noRoot bool, noSystemSummary bool, numGPCounters int) ([
 	return metadataScripts, nil
 }
 
+// perfEnumerationFaultsKernel reports whether asking perf to enumerate events on this
+// target can be expected to fault the kernel, and if so a reason worth logging. It must
+// be called before the metadata scripts run, because the command that faults is one of
+// them. See pmuEnumerationSafetyScript for the fault itself.
+//
+// Fails open. When the answer cannot be established -- the probe did not run, dmesg is
+// restricted, the fields are missing -- this reports false and the caller keeps perf
+// enumeration, because wrongly reporting true would narrow a healthy target's event set
+// for no reason. The cost of failing open is that a target with restricted dmesg and this
+// specific broken PMU is still exposed, which is the narrower of the two risks.
+func perfEnumerationFaultsKernel(t target.Target, localTempDir string, noRoot bool) (bool, string) {
+	scriptDef := pmuEnumerationSafetyScript
+	scriptDef.Superuser = !noRoot
+	scriptOutput, err := script.RunScript(t, scriptDef, localTempDir)
+	if err != nil {
+		slog.Debug("could not check whether perf event enumeration is safe; assuming it is",
+			slog.String("error", err.Error()))
+		return false, ""
+	}
+	return perfEnumerationFaultsKernelFromOutput(scriptOutput.Stdout)
+}
+
+// perfEnumerationFaultsKernelFromOutput holds the decision itself, separately from
+// running the probe, so it can be tested against the capability data from real targets.
+func perfEnumerationFaultsKernelFromOutput(stdout string) (bool, string) {
+	var fixedCounters, slotsExposed string
+	for _, line := range strings.Split(stdout, "\n") {
+		key, value, found := strings.Cut(strings.TrimSpace(line), "=")
+		if !found {
+			continue
+		}
+		switch key {
+		case "fixed_purpose_counters":
+			fixedCounters = value
+		case "slots_event_exposed":
+			slotsExposed = value
+		}
+	}
+	numFixedCounters, err := strconv.Atoi(fixedCounters)
+	if err != nil {
+		slog.Debug("PMU fixed counter count unavailable; assuming perf event enumeration is safe",
+			slog.String("fixed_purpose_counters", fixedCounters))
+		return false, ""
+	}
+	// Counter 3 is TOPDOWN.SLOTS, so a count of 4 or more advertises it. The kernel
+	// exposing no slots or topdown event means it knows the counter is unusable here --
+	// while perf will still happily program it from its own event tables.
+	if numFixedCounters >= 4 && slotsExposed == "no" {
+		return true, fmt.Sprintf("PMU advertises %d fixed-purpose counters, including TOPDOWN.SLOTS, "+
+			"but the kernel exposes no slots or topdown event; enumerating events with perf would fault the kernel",
+			numFixedCounters)
+	}
+	return false, ""
+}
+
 // String provides a string representation of the Metadata structure.
 func (md Metadata) String() string {
 	// Create a copy without PerfSupportedEvents to reduce log size

cmd/metrics/metadata_aarch.go

@@ -93,7 +93,9 @@ func (c *ARMMetadataCollector) CollectMetadata(t target.Target, noRoot bool, noS
 	}
 
 	// Run metadata scripts concurrently
-	metadataScripts, err := getMetadataScripts(noRoot, noSystemSummary, metadata.NumGeneralPurposeCounters)
+	// The PMU incoherence that makes perf enumeration fault has only been observed on
+	// virtualized x86 guests, so ARM keeps perf enumeration unconditionally.
+	metadataScripts, err := getMetadataScripts(noRoot, noSystemSummary, metadata.NumGeneralPurposeCounters, false)
 	if err != nil {
 		return Metadata{}, fmt.Errorf("failed to get metadata scripts: %v", err)
 	}

cmd/metrics/metadata_pmu_safety_test.go

@@ -0,0 +1,65 @@
+// Copyright (C) 2021-2025 Intel Corporation
+// SPDX-License-Identifier: BSD-3-Clause
+
+package metrics
+
+import "testing"
+
+// The cases are the four targets the predicate was derived from, so a change that starts
+// treating a healthy PMU as broken -- or stops recognizing the broken one -- fails here.
+func TestPerfEnumerationFaultsKernelFromOutput(t *testing.T) {
+	tests := []struct {
+		name       string
+		stdout     string
+		wantFaults bool
+	}{
+		{
+			// The target this exists for: 4 fixed counters advertised, so counter 3
+			// (TOPDOWN.SLOTS) is claimed, while the kernel exposes no slots event.
+			name:       "m6i.16xlarge, Ice Lake guest with an unusable fixed counter 3",
+			stdout:     "fixed_purpose_counters=4\nslots_event_exposed=no\n",
+			wantFaults: true,
+		},
+		{
+			// Same silicon and kernel, bare metal: the counter is advertised and usable.
+			name:       "bare metal Ice Lake, counter 3 advertised and exposed",
+			stdout:     "fixed_purpose_counters=4\nslots_event_exposed=yes\n",
+			wantFaults: false,
+		},
+		{
+			// Sapphire Rapids guest: only 2 fixed counters, so counter 3 is never claimed
+			// and perf refuses the event outright rather than programming it.
+			name:       "m7i.24xlarge, counter 3 not advertised",
+			stdout:     "fixed_purpose_counters=2\nslots_event_exposed=no\n",
+			wantFaults: false,
+		},
+		{
+			// Skylake guest: no slots counter exists on that microarchitecture at all.
+			name:       "m5.24xlarge, Skylake with 3 fixed counters",
+			stdout:     "fixed_purpose_counters=3\nslots_event_exposed=no\n",
+			wantFaults: false,
+		},
+		{
+			// Fails open: an unreadable dmesg must not narrow a healthy target's events.
+			name:       "counter count unavailable",
+			stdout:     "fixed_purpose_counters=unknown\nslots_event_exposed=no\n",
+			wantFaults: false,
+		},
+		{
+			name:       "empty output",
+			stdout:     "",
+			wantFaults: false,
+		},
+	}
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			faults, reason := perfEnumerationFaultsKernelFromOutput(test.stdout)
+			if faults != test.wantFaults {
+				t.Errorf("perfEnumerationFaultsKernelFromOutput() = %v, want %v", faults, test.wantFaults)
+			}
+			if faults && reason == "" {
+				t.Error("expected a reason to log when reporting that enumeration faults")
+			}
+		})
+	}
+}

cmd/metrics/metadata_x86.go

@@ -91,8 +91,18 @@ func (c *X86MetadataCollector) CollectMetadata(t target.Target, noRoot bool, noS
 		return Metadata{}, fmt.Errorf("failed to get number of general purpose counters: %v", err)
 	}
 
+	// Whether perf can safely enumerate this target's events. Decided before the metadata
+	// scripts rather than from their output, because the command that faults the kernel on
+	// an affected target is one of those scripts.
+	enumerateEventsFromSysfs, reason := perfEnumerationFaultsKernel(t, localTempDir, noRoot)
+	if enumerateEventsFromSysfs {
+		slog.Warn("enumerating perf events from sysfs instead of with perf; the event set, and so the "+
+			"set of metrics that can be collected, will be narrower than usual on this target",
+			slog.String("reason", reason))
+	}
+
 	// Run metadata scripts concurrently
-	metadataScripts, err := getMetadataScripts(noRoot, noSystemSummary, metadata.NumGeneralPurposeCounters)
+	metadataScripts, err := getMetadataScripts(noRoot, noSystemSummary, metadata.NumGeneralPurposeCounters, enumerateEventsFromSysfs)
 	if err != nil {
 		return Metadata{}, fmt.Errorf("failed to get metadata scripts: %v", err)
 	}

go.mod

@@ -23,7 +23,7 @@ require (
 	github.com/spf13/cobra v1.10.2
 	github.com/spf13/pflag v1.0.10
 	github.com/stretchr/testify v1.12.1
-	github.com/xuri/excelize/v2 v2.11.0
+	github.com/xuri/excelize/v2 v2.11.0-0.20260915055537-22f76f9acb94
 	golang.org/x/term v0.47.0
 	golang.org/x/text v0.43.0
 	gopkg.in/yaml.v2 v2.4.0

internal/script/scripts.go

@@ -1749,8 +1749,11 @@ cleanup() {
 }
 trap cleanup INT TERM
 
-# Enumerate syscall enter events dynamically
-EVENTS=$(perf list | awk '/^[[:space:]]*syscalls:sys_enter_/ {print $1}' | paste -sd,)
+# Enumerate syscall enter events dynamically.
+#
+# Uses 'perf list tracepoint' to enumerate events safely. Bare 'perf list' will fault the
+# kernel on affected targets leaving it in D state and unreachable.
+EVENTS=$(perf list tracepoint | awk '/^[[:space:]]*syscalls:sys_enter_/ {print $1}' | paste -sd,)
 
 if [[ -z "$EVENTS" ]]; then
   echo "No syscalls:sys_enter_* events found" >&2