[PATCH 0/3] Cleanup for Intel PMU initialization

[PATCH 0/3] Cleanup for Intel PMU initialization

[kan.liang]

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

The patches are to clean up different features. They all touch the
initialization code. There are some dependencies. So I put them in
a patch set to facilitate the review and merge.

Patch 1 is to clear up the PEBS-via-PT on hybrid.
The original V1 patch can be found at
https://lore.kernel.org/lkml/20250124183432.3565061-1-kan.liang@linux.intel.com/
This is the V2, which only update the comments.

Patch 2 is to clean up the 023H enumeration.
To close the discussion at
https://lore.kernel.org/lkml/20250127212901.GB9557@noisy.programming.kicks-ass.net/

Patch 3 is to clean up the counter information update and check codes.
To close the discussion at
https://lore.kernel.org/lkml/20250127164406.GN16742@noisy.programming.kicks-ass.net/

Kan Liang (3):
  perf/x86/intel: Clean up PEBS-via-PT on hybrid
  perf/x86/intel: Fix ARCH_PERFMON_NUM_COUNTER_LEAF
  perf/x86/intel: Clean up counter information update and check

 arch/x86/events/core.c            |  10 +--
 arch/x86/events/intel/core.c      | 134 +++++++++++++++---------------
 arch/x86/events/intel/ds.c        |  10 ++-
 arch/x86/events/perf_event.h      |   1 +
 arch/x86/include/asm/perf_event.h |  28 ++++++-
 5 files changed, 106 insertions(+), 77 deletions(-)

-- 
2.38.1

[PATCH 1/3] perf/x86/intel: Clean up PEBS-via-PT on hybrid

[kan.liang]

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

The PEBS-via-PT feature is exposed for the e-core of some hybrid
platforms, e.g., ADL and MTL. But it never works.

$ dmesg | grep PEBS
[    1.793888] core: cpu_atom PMU driver: PEBS-via-PT

$ perf record -c 1000 -e '{intel_pt/branch=0/,
cpu_atom/cpu-cycles,aux-output/pp}' -C8
Error:
The sys_perf_event_open() syscall returned with 22 (Invalid argument)
for event (cpu_atom/cpu-cycles,aux-output/pp).
"dmesg | grep -i perf" may provide additional information.

The "PEBS-via-PT" is printed if the corresponding bit of per-PMU
capabilities is set. Since the feature is supported by the e-core HW,
perf sets the bit for e-core. However, for Intel PT, if a feature is not
supported on all CPUs, it is not supported at all. The PEBS-via-PT event
cannot be created successfully.

The PEBS-via-PT is no longer enumerated on the latest hybrid platform. It
will be deprecated on future platforms with Arch PEBS. Let's remove it
from the existing hybrid platforms.

Fixes: d9977c43bff8 ("perf/x86: Register hybrid PMUs")
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
---

The original V1 patch can be found at
https://lore.kernel.org/lkml/20250124183432.3565061-1-kan.liang@linux.intel.com/
The patch only updates the comments.

 arch/x86/events/intel/core.c | 10 ----------
 arch/x86/events/intel/ds.c   | 10 +++++++++-
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 7f1b6b90a5fb..0a1030eb6db8 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4981,11 +4981,6 @@ static void intel_pmu_check_hybrid_pmus(struct x86_hybrid_pmu *pmu)
 	else
 		pmu->intel_ctrl &= ~(1ULL << GLOBAL_CTRL_EN_PERF_METRICS);
 
-	if (pmu->intel_cap.pebs_output_pt_available)
-		pmu->pmu.capabilities |= PERF_PMU_CAP_AUX_OUTPUT;
-	else
-		pmu->pmu.capabilities &= ~PERF_PMU_CAP_AUX_OUTPUT;
-
 	intel_pmu_check_event_constraints(pmu->event_constraints,
 					  pmu->cntr_mask64,
 					  pmu->fixed_cntr_mask64,
@@ -5063,9 +5058,6 @@ static bool init_hybrid_pmu(int cpu)
 
 	pr_info("%s PMU driver: ", pmu->name);
 
-	if (pmu->intel_cap.pebs_output_pt_available)
-		pr_cont("PEBS-via-PT ");
-
 	pr_cont("\n");
 
 	x86_pmu_show_pmu_cap(&pmu->pmu);
@@ -6420,11 +6412,9 @@ static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
 		pmu->intel_cap.capabilities = x86_pmu.intel_cap.capabilities;
 		if (pmu->pmu_type & hybrid_small_tiny) {
 			pmu->intel_cap.perf_metrics = 0;
-			pmu->intel_cap.pebs_output_pt_available = 1;
 			pmu->mid_ack = true;
 		} else if (pmu->pmu_type & hybrid_big) {
 			pmu->intel_cap.perf_metrics = 1;
-			pmu->intel_cap.pebs_output_pt_available = 0;
 			pmu->late_ack = true;
 		}
 	}
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index 13a78a8a2780..46aaaeae0c8d 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -2742,7 +2742,15 @@ void __init intel_ds_init(void)
 			}
 			pr_cont("PEBS fmt%d%c%s, ", format, pebs_type, pebs_qual);
 
-			if (!is_hybrid() && x86_pmu.intel_cap.pebs_output_pt_available) {
+			/*
+			 * The PEBS-via-PT is not supported on hybrid platforms,
+			 * because not all CPUs of a hybrid machine support it.
+			 * The global x86_pmu.intel_cap, which only contains the
+			 * common capabilities, is used to check the availability
+			 * of the feature. The per-PMU pebs_output_pt_available
+			 * in a hybrid machine should be ignored.
+			 */
+			if (x86_pmu.intel_cap.pebs_output_pt_available) {
 				pr_cont("PEBS-via-PT, ");
 				x86_get_pmu(smp_processor_id())->capabilities |= PERF_PMU_CAP_AUX_OUTPUT;
 			}
-- 
2.38.1

[PATCH 2/3] perf/x86/intel: Fix ARCH_PERFMON_NUM_COUNTER_LEAF

[kan.liang]

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

The EAX of the CPUID Leaf 023H enumerates the mask of valid sub-leaves.
To tell the availability of the sub-leaf 1 (enumerate the counter mask),
perf should check the bit 1 (0x2) of EAS, rather than bit 0 (0x1).

The error is not user-visible on bare metal. Because the sub-leaf 0 and
the sub-leaf 1 are always available. However, it may bring issues in a
virtualization environment when a VMM only enumerates the sub-leaf 0.

Introduce the cpuid35_e?x to replace the macros, which makes the
implementation style consistent.

Fixes: eb467aaac21e ("perf/x86/intel: Support Architectural PerfMon Extension leaf")
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Cc: stable@vger.kernel.org
---
 arch/x86/events/intel/core.c      | 18 ++++++++++--------
 arch/x86/include/asm/perf_event.h | 28 +++++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 0a1030eb6db8..120df9620951 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4945,20 +4945,22 @@ static inline bool intel_pmu_broken_perf_cap(void)
 
 static void update_pmu_cap(struct x86_hybrid_pmu *pmu)
 {
-	unsigned int sub_bitmaps, eax, ebx, ecx, edx;
+	unsigned int cntr, fixed_cntr, ecx, edx;
+	union cpuid35_eax eax;
+	union cpuid35_ebx ebx;
 
-	cpuid(ARCH_PERFMON_EXT_LEAF, &sub_bitmaps, &ebx, &ecx, &edx);
+	cpuid(ARCH_PERFMON_EXT_LEAF, &eax.full, &ebx.full, &ecx, &edx);
 
-	if (ebx & ARCH_PERFMON_EXT_UMASK2)
+	if (ebx.split.umask2)
 		pmu->config_mask |= ARCH_PERFMON_EVENTSEL_UMASK2;
-	if (ebx & ARCH_PERFMON_EXT_EQ)
+	if (ebx.split.eq)
 		pmu->config_mask |= ARCH_PERFMON_EVENTSEL_EQ;
 
-	if (sub_bitmaps & ARCH_PERFMON_NUM_COUNTER_LEAF_BIT) {
+	if (eax.split.cntr_subleaf) {
 		cpuid_count(ARCH_PERFMON_EXT_LEAF, ARCH_PERFMON_NUM_COUNTER_LEAF,
-			    &eax, &ebx, &ecx, &edx);
-		pmu->cntr_mask64 = eax;
-		pmu->fixed_cntr_mask64 = ebx;
+			    &cntr, &fixed_cntr, &ecx, &edx);
+		pmu->cntr_mask64 = cntr;
+		pmu->fixed_cntr_mask64 = fixed_cntr;
 	}
 
 	if (!intel_pmu_broken_perf_cap()) {
diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
index adaeb8ca3a8a..9aef5d044706 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -194,11 +194,33 @@ union cpuid10_edx {
  * detection/enumeration details:
  */
 #define ARCH_PERFMON_EXT_LEAF			0x00000023
-#define ARCH_PERFMON_EXT_UMASK2			0x1
-#define ARCH_PERFMON_EXT_EQ			0x2
-#define ARCH_PERFMON_NUM_COUNTER_LEAF_BIT	0x1
 #define ARCH_PERFMON_NUM_COUNTER_LEAF		0x1
 
+union cpuid35_eax {
+	struct {
+		unsigned int	leaf0:1;
+		/* Counters Sub-Leaf */
+		unsigned int    cntr_subleaf:1;
+		/* Auto Counter Reload Sub-Leaf */
+		unsigned int    acr_subleaf:1;
+		/* Events Sub-Leaf */
+		unsigned int    events_subleaf:1;
+		unsigned int	reserved:28;
+	} split;
+	unsigned int            full;
+};
+
+union cpuid35_ebx {
+	struct {
+		/* UnitMask2 Supported */
+		unsigned int    umask2:1;
+		/* EQ-bit Supported */
+		unsigned int    eq:1;
+		unsigned int	reserved:30;
+	} split;
+	unsigned int            full;
+};
+
 /*
  * Intel Architectural LBR CPUID detection/enumeration details:
  */
-- 
2.38.1

[PATCH 3/3] perf/x86/intel: Clean up counter information update and check

[kan.liang]

From: Kan Liang <kan.liang@linux.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@linux.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

Re: [PATCH 0/3] Cleanup for Intel PMU initialization

[Peter Zijlstra]

On Wed, Jan 29, 2025 at 07:48:17AM -0800, kan.liang@linux.intel.com wrote:
> From: Kan Liang <kan.liang@linux.intel.com>
> 
> The patches are to clean up different features. They all touch the
> initialization code. There are some dependencies. So I put them in
> a patch set to facilitate the review and merge.
> 
> Patch 1 is to clear up the PEBS-via-PT on hybrid.
> The original V1 patch can be found at
> https://lore.kernel.org/lkml/20250124183432.3565061-1-kan.liang@linux.intel.com/
> This is the V2, which only update the comments.
> 
> Patch 2 is to clean up the 023H enumeration.
> To close the discussion at
> https://lore.kernel.org/lkml/20250127212901.GB9557@noisy.programming.kicks-ass.net/
> 
> Patch 3 is to clean up the counter information update and check codes.
> To close the discussion at
> https://lore.kernel.org/lkml/20250127164406.GN16742@noisy.programming.kicks-ass.net/

So I can take 1 and 2, but 3 is part of that arch pebs things, and that
series is not in a shape I want to apply.

Also, I'm not at all sure 3 improves things, it might just make it
worse.

Re: [PATCH 0/3] Cleanup for Intel PMU initialization

[Liang, Kan]

On 2025-02-03 8:16 a.m., Peter Zijlstra wrote:
> On Wed, Jan 29, 2025 at 07:48:17AM -0800, kan.liang@linux.intel.com wrote:
>> From: Kan Liang <kan.liang@linux.intel.com>
>>
>> The patches are to clean up different features. They all touch the
>> initialization code. There are some dependencies. So I put them in
>> a patch set to facilitate the review and merge.
>>
>> Patch 1 is to clear up the PEBS-via-PT on hybrid.
>> The original V1 patch can be found at
>> https://lore.kernel.org/lkml/20250124183432.3565061-1-kan.liang@linux.intel.com/
>> This is the V2, which only update the comments.
>>
>> Patch 2 is to clean up the 023H enumeration.
>> To close the discussion at
>> https://lore.kernel.org/lkml/20250127212901.GB9557@noisy.programming.kicks-ass.net/
>>
>> Patch 3 is to clean up the counter information update and check codes.
>> To close the discussion at
>> https://lore.kernel.org/lkml/20250127164406.GN16742@noisy.programming.kicks-ass.net/
> 
> So I can take 1 and 2, 

Thanks.

> but 3 is part of that arch pebs things, and that
> series is not in a shape I want to apply.
> 
> Also, I'm not at all sure 3 improves things, it might just make it
> worse.

There should be some improvements for the initialization code. At least,
we can reduce the is_hybrid() check and move the 0x23 enumeration in one
place.

Anyway, I will think more and see if there is more that can be improved.
Then, put it as part of the arch pebs code. We may have further
discussions with the V2 arch PEBS code.

Thanks,
Kan

[tip: perf/urgent] perf/x86/intel: Fix ARCH_PERFMON_NUM_COUNTER_LEAF

[tip-bot2 for Kan Liang]

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     47a973fd75639fe80d59f9e1860113bb2a0b112b
Gitweb:        https://git.kernel.org/tip/47a973fd75639fe80d59f9e1860113bb2a0b112b
Author:        Kan Liang <kan.liang@linux.intel.com>
AuthorDate:    Wed, 29 Jan 2025 07:48:19 -08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Sat, 08 Feb 2025 15:47:25 +01:00

perf/x86/intel: Fix ARCH_PERFMON_NUM_COUNTER_LEAF

The EAX of the CPUID Leaf 023H enumerates the mask of valid sub-leaves.
To tell the availability of the sub-leaf 1 (enumerate the counter mask),
perf should check the bit 1 (0x2) of EAS, rather than bit 0 (0x1).

The error is not user-visible on bare metal. Because the sub-leaf 0 and
the sub-leaf 1 are always available. However, it may bring issues in a
virtualization environment when a VMM only enumerates the sub-leaf 0.

Introduce the cpuid35_e?x to replace the macros, which makes the
implementation style consistent.

Fixes: eb467aaac21e ("perf/x86/intel: Support Architectural PerfMon Extension leaf")
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: stable@vger.kernel.org
Link: https://lkml.kernel.org/r/20250129154820.3755948-3-kan.liang@linux.intel.com
---
 arch/x86/events/intel/core.c      | 18 ++++++++++--------
 arch/x86/include/asm/perf_event.h | 28 +++++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 966f783..f3d5b71 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4905,20 +4905,22 @@ static inline bool intel_pmu_broken_perf_cap(void)
 
 static void update_pmu_cap(struct x86_hybrid_pmu *pmu)
 {
-	unsigned int sub_bitmaps, eax, ebx, ecx, edx;
+	unsigned int cntr, fixed_cntr, ecx, edx;
+	union cpuid35_eax eax;
+	union cpuid35_ebx ebx;
 
-	cpuid(ARCH_PERFMON_EXT_LEAF, &sub_bitmaps, &ebx, &ecx, &edx);
+	cpuid(ARCH_PERFMON_EXT_LEAF, &eax.full, &ebx.full, &ecx, &edx);
 
-	if (ebx & ARCH_PERFMON_EXT_UMASK2)
+	if (ebx.split.umask2)
 		pmu->config_mask |= ARCH_PERFMON_EVENTSEL_UMASK2;
-	if (ebx & ARCH_PERFMON_EXT_EQ)
+	if (ebx.split.eq)
 		pmu->config_mask |= ARCH_PERFMON_EVENTSEL_EQ;
 
-	if (sub_bitmaps & ARCH_PERFMON_NUM_COUNTER_LEAF_BIT) {
+	if (eax.split.cntr_subleaf) {
 		cpuid_count(ARCH_PERFMON_EXT_LEAF, ARCH_PERFMON_NUM_COUNTER_LEAF,
-			    &eax, &ebx, &ecx, &edx);
-		pmu->cntr_mask64 = eax;
-		pmu->fixed_cntr_mask64 = ebx;
+			    &cntr, &fixed_cntr, &ecx, &edx);
+		pmu->cntr_mask64 = cntr;
+		pmu->fixed_cntr_mask64 = fixed_cntr;
 	}
 
 	if (!intel_pmu_broken_perf_cap()) {
diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
index 1ac79f3..0ba8d20 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -188,11 +188,33 @@ union cpuid10_edx {
  * detection/enumeration details:
  */
 #define ARCH_PERFMON_EXT_LEAF			0x00000023
-#define ARCH_PERFMON_EXT_UMASK2			0x1
-#define ARCH_PERFMON_EXT_EQ			0x2
-#define ARCH_PERFMON_NUM_COUNTER_LEAF_BIT	0x1
 #define ARCH_PERFMON_NUM_COUNTER_LEAF		0x1
 
+union cpuid35_eax {
+	struct {
+		unsigned int	leaf0:1;
+		/* Counters Sub-Leaf */
+		unsigned int    cntr_subleaf:1;
+		/* Auto Counter Reload Sub-Leaf */
+		unsigned int    acr_subleaf:1;
+		/* Events Sub-Leaf */
+		unsigned int    events_subleaf:1;
+		unsigned int	reserved:28;
+	} split;
+	unsigned int            full;
+};
+
+union cpuid35_ebx {
+	struct {
+		/* UnitMask2 Supported */
+		unsigned int    umask2:1;
+		/* EQ-bit Supported */
+		unsigned int    eq:1;
+		unsigned int	reserved:30;
+	} split;
+	unsigned int            full;
+};
+
 /*
  * Intel Architectural LBR CPUID detection/enumeration details:
  */

[tip: perf/urgent] perf/x86/intel: Clean up PEBS-via-PT on hybrid

[tip-bot2 for Kan Liang]

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     0a5561501397e2bbd0fb0e300eb489f72a90597a
Gitweb:        https://git.kernel.org/tip/0a5561501397e2bbd0fb0e300eb489f72a90597a
Author:        Kan Liang <kan.liang@linux.intel.com>
AuthorDate:    Wed, 29 Jan 2025 07:48:18 -08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Sat, 08 Feb 2025 15:47:25 +01:00

perf/x86/intel: Clean up PEBS-via-PT on hybrid

The PEBS-via-PT feature is exposed for the e-core of some hybrid
platforms, e.g., ADL and MTL. But it never works.

$ dmesg | grep PEBS
[    1.793888] core: cpu_atom PMU driver: PEBS-via-PT

$ perf record -c 1000 -e '{intel_pt/branch=0/,
cpu_atom/cpu-cycles,aux-output/pp}' -C8
Error:
The sys_perf_event_open() syscall returned with 22 (Invalid argument)
for event (cpu_atom/cpu-cycles,aux-output/pp).
"dmesg | grep -i perf" may provide additional information.

The "PEBS-via-PT" is printed if the corresponding bit of per-PMU
capabilities is set. Since the feature is supported by the e-core HW,
perf sets the bit for e-core. However, for Intel PT, if a feature is not
supported on all CPUs, it is not supported at all. The PEBS-via-PT event
cannot be created successfully.

The PEBS-via-PT is no longer enumerated on the latest hybrid platform. It
will be deprecated on future platforms with Arch PEBS. Let's remove it
from the existing hybrid platforms.

Fixes: d9977c43bff8 ("perf/x86: Register hybrid PMUs")
Signed-off-by: Kan Liang <kan.liang@linux.intel.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20250129154820.3755948-2-kan.liang@linux.intel.com
---
 arch/x86/events/intel/core.c | 10 ----------
 arch/x86/events/intel/ds.c   | 10 +++++++++-
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index 7601196..966f783 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -4941,11 +4941,6 @@ static void intel_pmu_check_hybrid_pmus(struct x86_hybrid_pmu *pmu)
 	else
 		pmu->intel_ctrl &= ~(1ULL << GLOBAL_CTRL_EN_PERF_METRICS);
 
-	if (pmu->intel_cap.pebs_output_pt_available)
-		pmu->pmu.capabilities |= PERF_PMU_CAP_AUX_OUTPUT;
-	else
-		pmu->pmu.capabilities &= ~PERF_PMU_CAP_AUX_OUTPUT;
-
 	intel_pmu_check_event_constraints(pmu->event_constraints,
 					  pmu->cntr_mask64,
 					  pmu->fixed_cntr_mask64,
@@ -5023,9 +5018,6 @@ static bool init_hybrid_pmu(int cpu)
 
 	pr_info("%s PMU driver: ", pmu->name);
 
-	if (pmu->intel_cap.pebs_output_pt_available)
-		pr_cont("PEBS-via-PT ");
-
 	pr_cont("\n");
 
 	x86_pmu_show_pmu_cap(&pmu->pmu);
@@ -6370,11 +6362,9 @@ static __always_inline int intel_pmu_init_hybrid(enum hybrid_pmu_type pmus)
 		pmu->intel_cap.capabilities = x86_pmu.intel_cap.capabilities;
 		if (pmu->pmu_type & hybrid_small_tiny) {
 			pmu->intel_cap.perf_metrics = 0;
-			pmu->intel_cap.pebs_output_pt_available = 1;
 			pmu->mid_ack = true;
 		} else if (pmu->pmu_type & hybrid_big) {
 			pmu->intel_cap.perf_metrics = 1;
-			pmu->intel_cap.pebs_output_pt_available = 0;
 			pmu->late_ack = true;
 		}
 	}
diff --git a/arch/x86/events/intel/ds.c b/arch/x86/events/intel/ds.c
index ba74e11..c2e2eae 100644
--- a/arch/x86/events/intel/ds.c
+++ b/arch/x86/events/intel/ds.c
@@ -2578,7 +2578,15 @@ void __init intel_ds_init(void)
 			}
 			pr_cont("PEBS fmt4%c%s, ", pebs_type, pebs_qual);
 
-			if (!is_hybrid() && x86_pmu.intel_cap.pebs_output_pt_available) {
+			/*
+			 * The PEBS-via-PT is not supported on hybrid platforms,
+			 * because not all CPUs of a hybrid machine support it.
+			 * The global x86_pmu.intel_cap, which only contains the
+			 * common capabilities, is used to check the availability
+			 * of the feature. The per-PMU pebs_output_pt_available
+			 * in a hybrid machine should be ignored.
+			 */
+			if (x86_pmu.intel_cap.pebs_output_pt_available) {
 				pr_cont("PEBS-via-PT, ");
 				x86_get_pmu(smp_processor_id())->capabilities |= PERF_PMU_CAP_AUX_OUTPUT;
 			}