Message-Id: <20250129154820.3755948-4-kan.liang@linux.intel.com>
Date: Wed, 29 Jan 2025 07:48:20 -0800
From: kan.liang@...ux.intel.com
To: peterz@...radead.org,
	mingo@...hat.com,
	acme@...nel.org,
	namhyung@...nel.org,
	irogers@...gle.com,
	adrian.hunter@...el.com,
	alexander.shishkin@...ux.intel.com,
	ak@...ux.intel.com,
	linux-kernel@...r.kernel.org
Cc: dapeng1.mi@...ux.intel.com,
	Kan Liang <kan.liang@...ux.intel.com>
Subject: [PATCH 3/3] perf/x86/intel: Clean up counter information update and check

From: Kan Liang <kan.liang@...ux.intel.com>

The counter-related PMU information may be updated in different places
on different platforms. For hybrid machines, the accurate counter
information can only be retrieved when the specific CPU is online.
The information is updated in
intel_pmu_cpu_starting()->init_hybrid_pmu().
For non-hybrid machines, the information is initialized in the
intel_pmu_init() when booting CPU0.

The counter information doesn't impact the PMU registration and should
not necessarily be in intel_pmu_init().

Setup/update the counter-related PMU information in the CPU starting
stage for all Intel platforms. The x86_pmu_show_pmu_cap() has to be
delayed until all the information is updated. Add a PMU_FL_LATE_SETUP
flag to indicate the late setup/update case.

Signed-off-by: Kan Liang <kan.liang@...ux.intel.com>
---
 arch/x86/events/core.c       |  10 ++--
 arch/x86/events/intel/core.c | 112 ++++++++++++++++++-----------------
 arch/x86/events/perf_event.h |   1 +
 3 files changed, 65 insertions(+), 58 deletions(-)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index 7b6430e5a77b..e9e31dc49749 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2119,10 +2119,6 @@ static int __init init_hw_perf_events(void)
 	perf_events_lapic_init();
 	register_nmi_handler(NMI_LOCAL, perf_event_nmi_handler, 0, "PMI");
 
-	unconstrained = (struct event_constraint)
-		__EVENT_CONSTRAINT(0, x86_pmu.cntr_mask64,
-				   0, x86_pmu_num_counters(NULL), 0, 0);
-
 	x86_pmu_format_group.attrs = x86_pmu.format_attrs;
 
 	if (!x86_pmu.events_sysfs_show)
@@ -2130,8 +2126,12 @@ static int __init init_hw_perf_events(void)
 
 	pmu.attr_update = x86_pmu.attr_update;
 
-	if (!is_hybrid())
+	if (!(x86_pmu.flags & PMU_FL_LATE_SETUP)) {
+		unconstrained = (struct event_constraint)
+			__EVENT_CONSTRAINT(0, x86_pmu.cntr_mask64,
+					   0, x86_pmu_num_counters(NULL), 0, 0);
 		x86_pmu_show_pmu_cap(NULL);
+	}
 
 	if (!x86_pmu.read)
 		x86_pmu.read = _x86_pmu_read;
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 120df9620951..2a16bc94345c 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4943,8 +4943,9 @@ static inline bool intel_pmu_broken_perf_cap(void)
 	return false;
 }
 
-static void update_pmu_cap(struct x86_hybrid_pmu *pmu)
+static void update_pmu_cap(struct x86_hybrid_pmu *h_pmu)
 {
+	struct pmu *pmu = h_pmu ? &h_pmu->pmu : NULL;
 	unsigned int cntr, fixed_cntr, ecx, edx;
 	union cpuid35_eax eax;
 	union cpuid35_ebx ebx;
@@ -4952,43 +4953,46 @@ static void update_pmu_cap(struct x86_hybrid_pmu *pmu)
 	cpuid(ARCH_PERFMON_EXT_LEAF, &eax.full, &ebx.full, &ecx, &edx);
 
 	if (ebx.split.umask2)
-		pmu->config_mask |= ARCH_PERFMON_EVENTSEL_UMASK2;
+		hybrid(pmu, config_mask) |= ARCH_PERFMON_EVENTSEL_UMASK2;
 	if (ebx.split.eq)
-		pmu->config_mask |= ARCH_PERFMON_EVENTSEL_EQ;
+		hybrid(pmu, config_mask) |= ARCH_PERFMON_EVENTSEL_EQ;
 
 	if (eax.split.cntr_subleaf) {
 		cpuid_count(ARCH_PERFMON_EXT_LEAF, ARCH_PERFMON_NUM_COUNTER_LEAF,
 			    &cntr, &fixed_cntr, &ecx, &edx);
-		pmu->cntr_mask64 = cntr;
-		pmu->fixed_cntr_mask64 = fixed_cntr;
+		hybrid(pmu, cntr_mask64) = cntr;
+		hybrid(pmu, fixed_cntr_mask64) = fixed_cntr;
 	}
 
 	if (!intel_pmu_broken_perf_cap()) {
 		/* Perf Metric (Bit 15) and PEBS via PT (Bit 16) are hybrid enumeration */
-		rdmsrl(MSR_IA32_PERF_CAPABILITIES, pmu->intel_cap.capabilities);
+		rdmsrl(MSR_IA32_PERF_CAPABILITIES, hybrid(pmu, intel_cap).capabilities);
 	}
 }
 
-static void intel_pmu_check_hybrid_pmus(struct x86_hybrid_pmu *pmu)
+static void intel_pmu_check_pmus(struct x86_hybrid_pmu *h_pmu)
 {
-	intel_pmu_check_counters_mask(&pmu->cntr_mask64, &pmu->fixed_cntr_mask64,
-				      &pmu->intel_ctrl);
-	pmu->pebs_events_mask = intel_pmu_pebs_mask(pmu->cntr_mask64);
-	pmu->unconstrained = (struct event_constraint)
-			     __EVENT_CONSTRAINT(0, pmu->cntr_mask64,
-						0, x86_pmu_num_counters(&pmu->pmu), 0, 0);
+	struct pmu *pmu = h_pmu ? &h_pmu->pmu : NULL;
 
-	if (pmu->intel_cap.perf_metrics)
-		pmu->intel_ctrl |= 1ULL << GLOBAL_CTRL_EN_PERF_METRICS;
-	else
-		pmu->intel_ctrl &= ~(1ULL << GLOBAL_CTRL_EN_PERF_METRICS);
+	intel_pmu_check_counters_mask(&hybrid(pmu, cntr_mask64),
+				      &hybrid(pmu, fixed_cntr_mask64),
+				      &hybrid(pmu, intel_ctrl));
+	hybrid(pmu, pebs_events_mask) = intel_pmu_pebs_mask(hybrid(pmu, cntr_mask64));
+	hybrid_var(pmu, unconstrained) = (struct event_constraint)
+					 __EVENT_CONSTRAINT(0, hybrid(pmu, cntr_mask64),
+							    0, x86_pmu_num_counters(pmu),
+							    0, 0);
 
-	intel_pmu_check_event_constraints(pmu->event_constraints,
-					  pmu->cntr_mask64,
-					  pmu->fixed_cntr_mask64,
-					  pmu->intel_ctrl);
+	if (hybrid(pmu, intel_cap).perf_metrics)
+		hybrid(pmu, intel_ctrl) |= 1ULL << GLOBAL_CTRL_EN_PERF_METRICS;
+	else
+		hybrid(pmu, intel_ctrl) &= ~(1ULL << GLOBAL_CTRL_EN_PERF_METRICS);
 
-	intel_pmu_check_extra_regs(pmu->extra_regs);
+	intel_pmu_check_event_constraints(hybrid(pmu, event_constraints),
+					  hybrid(pmu, cntr_mask64),
+					  hybrid(pmu, fixed_cntr_mask64),
+					  hybrid(pmu, intel_ctrl));
+	intel_pmu_check_extra_regs(hybrid(pmu, extra_regs));
 }
 
 static struct x86_hybrid_pmu *find_hybrid_pmu_for_cpu(void)
@@ -5036,37 +5040,50 @@ static struct x86_hybrid_pmu *find_hybrid_pmu_for_cpu(void)
 	return NULL;
 }
 
-static bool init_hybrid_pmu(int cpu)
+static bool update_intel_pmu(int cpu)
 {
 	struct cpu_hw_events *cpuc = &per_cpu(cpu_hw_events, cpu);
-	struct x86_hybrid_pmu *pmu = find_hybrid_pmu_for_cpu();
+	struct x86_hybrid_pmu *h_pmu = find_hybrid_pmu_for_cpu();
+	struct pmu *pmu = h_pmu ? &h_pmu->pmu : NULL;
 
-	if (WARN_ON_ONCE(!pmu || (pmu->pmu.type == -1))) {
+	if (WARN_ON_ONCE(is_hybrid() && (!h_pmu || h_pmu->pmu.type == -1))) {
 		cpuc->pmu = NULL;
 		return false;
 	}
 
-	/* Only check and dump the PMU information for the first CPU */
-	if (!cpumask_empty(&pmu->supported_cpus))
-		goto end;
+	/*
+	 * Only need to update and check for the first CPU.
+	 * For non-bybird, it's always CPU 0.
+	 */
+	if (h_pmu) {
+		cpuc->pmu = &h_pmu->pmu;
+		if (!cpumask_empty(&h_pmu->supported_cpus)) {
+			cpumask_set_cpu(cpu, &h_pmu->supported_cpus);
+			return true;
+		}
+	} else if (cpu)
+		return true;
 
 	if (this_cpu_has(X86_FEATURE_ARCH_PERFMON_EXT))
-		update_pmu_cap(pmu);
-
-	intel_pmu_check_hybrid_pmus(pmu);
+		update_pmu_cap(h_pmu);
 
-	if (!check_hw_exists(&pmu->pmu, pmu->cntr_mask, pmu->fixed_cntr_mask))
-		return false;
+	intel_pmu_check_pmus(h_pmu);
 
-	pr_info("%s PMU driver: ", pmu->name);
+	/* Check and dump the PMU for each PMU on hybrid. */
+	if (h_pmu) {
+		if (!check_hw_exists(pmu, hybrid(pmu, cntr_mask),
+				     hybrid(pmu, fixed_cntr_mask))) {
+			cpuc->pmu = NULL;
+			return false;
+		}
+		cpumask_set_cpu(cpu, &h_pmu->supported_cpus);
 
-	pr_cont("\n");
+		pr_info("%s PMU driver: ", h_pmu->name);
 
-	x86_pmu_show_pmu_cap(&pmu->pmu);
+		pr_cont("\n");
+	}
 
-end:
-	cpumask_set_cpu(cpu, &pmu->supported_cpus);
-	cpuc->pmu = &pmu->pmu;
+	x86_pmu_show_pmu_cap(pmu);
 
 	return true;
 }
@@ -5077,7 +5094,7 @@ static void intel_pmu_cpu_starting(int cpu)
 	int core_id = topology_core_id(cpu);
 	int i;
 
-	if (is_hybrid() && !init_hybrid_pmu(cpu))
+	if (!update_intel_pmu(cpu))
 		return;
 
 	init_debug_store_on_cpu(cpu);
@@ -6545,6 +6562,8 @@ __init int intel_pmu_init(void)
 	x86_pmu.pebs_events_mask	= intel_pmu_pebs_mask(x86_pmu.cntr_mask64);
 	x86_pmu.pebs_capable		= PEBS_COUNTER_MASK;
 
+	x86_pmu.flags			|= PMU_FL_LATE_SETUP;
+
 	/*
 	 * Quirk: v2 perfmon does not report fixed-purpose events, so
 	 * assume at least 3 events, when not running in a hypervisor:
@@ -7349,18 +7368,10 @@ __init int intel_pmu_init(void)
 		x86_pmu.attr_update = hybrid_attr_update;
 	}
 
-	intel_pmu_check_counters_mask(&x86_pmu.cntr_mask64,
-				      &x86_pmu.fixed_cntr_mask64,
-				      &x86_pmu.intel_ctrl);
-
 	/* AnyThread may be deprecated on arch perfmon v5 or later */
 	if (x86_pmu.intel_cap.anythread_deprecated)
 		x86_pmu.format_attrs = intel_arch_formats_attr;
 
-	intel_pmu_check_event_constraints(x86_pmu.event_constraints,
-					  x86_pmu.cntr_mask64,
-					  x86_pmu.fixed_cntr_mask64,
-					  x86_pmu.intel_ctrl);
 	/*
 	 * Access LBR MSR may cause #GP under certain circumstances.
 	 * Check all LBR MSR here.
@@ -7391,8 +7402,6 @@ __init int intel_pmu_init(void)
 		}
 	}
 
-	intel_pmu_check_extra_regs(x86_pmu.extra_regs);
-
 	/* Support full width counters using alternative MSR range */
 	if (x86_pmu.intel_cap.full_width_write) {
 		x86_pmu.max_period = x86_pmu.cntval_mask >> 1;
@@ -7408,9 +7417,6 @@ __init int intel_pmu_init(void)
 		x86_pmu.addr_offset = intel_pmu_v6_addr_offset;
 	}
 
-	if (!is_hybrid() && x86_pmu.intel_cap.perf_metrics)
-		x86_pmu.intel_ctrl |= 1ULL << GLOBAL_CTRL_EN_PERF_METRICS;
-
 	if (x86_pmu.intel_cap.pebs_timing_info)
 		x86_pmu.flags |= PMU_FL_RETIRE_LATENCY;
 
diff --git a/arch/x86/events/perf_event.h b/arch/x86/events/perf_event.h
index a698e6484b3b..cece99165cfb 100644
--- a/arch/x86/events/perf_event.h
+++ b/arch/x86/events/perf_event.h
@@ -1066,6 +1066,7 @@ do {									\
 #define PMU_FL_MEM_LOADS_AUX	0x100 /* Require an auxiliary event for the complete memory info */
 #define PMU_FL_RETIRE_LATENCY	0x200 /* Support Retire Latency in PEBS */
 #define PMU_FL_BR_CNTR		0x400 /* Support branch counter logging */
+#define PMU_FL_LATE_SETUP	0x800 /* Setup/Update PMU info when CPU starts */
 
 #define EVENT_VAR(_id)  event_attr_##_id
 #define EVENT_PTR(_id) &event_attr_##_id.attr.attr
-- 
2.38.1

Powered by blists - more mailing lists