Skip to contentSkip to conversation

perf/x86: Fix perf_event_print_debug() on non-Intel PMUs

PATCH In review Sandipan Das 4 participants 10 messages Aug 14 – Sep 15

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover perf/x86: Fix perf_event_print_debug() on non-Intel PMUs

Petr Tesarik reported that reading MSR_ARCH_PERFMON_FIXED_CTR_CTRL raises a #GP on AMD processors that support PerfMonV2, and proposed skipping that register when no fixed counters are implemented [1].

However, perf_event_print_debug() also dumps the global control and status registers whenever x86_pmu.version >= 2, and reads Intel Architectural PerfMon MSRs to do so. That condition used to be Intel-specific, but since commit 21d59e3e2c40 ("perf/x86/amd/core:
Detect PerfMonV2 support"), x86_pmu.version is also set to 2 on AMD processors that support PerfMonV2, where none of those registers are available.

The same reasoning therefore applies to the other registers read here, so rather than special-casing each one or reinstating a vendor check, this series makes the global register dump a vendor-specific operation.

[1] https://lore.kernel.org/all/[email protected]/

Previous versions can be found at
v1: https://lore.kernel.org/all/[email protected]/

Changes in v2:
* Address build issue reported by Sashiko.
* Use a static_call as suggested by Dapeng.
* Pick up Tested-by from Petr.

Sandipan Das (3):
perf/x86: Add x86_pmu::print_debug
perf/x86: Move MSR_CORE_PERF_GLOBAL_* dump to vendor code perf/x86/amd: Implement x86_pmu::print_debug

 arch/x86/events/amd/core.c     | 20 ++++++++++++
 arch/x86/events/core.c         | 60 ++++++++++++++++------------------
 arch/x86/events/intel/core.c   | 32 ++++++++++++++++++
 arch/x86/events/perf_event.h   |  4 +++
 arch/x86/events/zhaoxin/core.c | 18 ++++++++++
 5 files changed, 102 insertions(+), 32 deletions(-)
Signature · 1 line
2.53.0
#1 lore raw

PATCH v2 1/3 perf/x86: Add x86_pmu::print_debug

perf_event_print_debug() dumps the global control and status MSRs whenever x86_pmu.version >= 2, reading registers that exist only on Intel-compatible PMUs. This is not safe since x86_pmu.version is not Intel-specific and is now set by other vendors whose global registers use different addresses.

As a first step, split perf_event_print_debug() in two. The register dump moves into a new common helper, x86_pmu_print_debug(), leaving perf_event_print_debug() to handle the preamble and dispatch to an optional x86_pmu::print_debug method. This lets each vendor-specific PMU dump its own global state before chaining into the common helper. PMUs that do not implement the method, such as those with x86_pmu.version < 2, get the common helper alone.

No functional change intended.

---
 arch/x86/events/core.c       | 35 +++++++++++++++++++++++++++--------
 arch/x86/events/perf_event.h |  4 ++++
 2 files changed, 31 insertions(+), 8 deletions(-)
arch/x86/events/core.c +27 -8
index 8b3ea0adb965..364a4c83f677 100644
@@ -105,6 +105,8 @@ DEFINE_STATIC_CALL_NULL(x86_pmu_pebs_disable, *x86_pmu.pebs_disable);
 DEFINE_STATIC_CALL_NULL(x86_pmu_pebs_enable_all, *x86_pmu.pebs_enable_all);
 DEFINE_STATIC_CALL_NULL(x86_pmu_pebs_disable_all, *x86_pmu.pebs_disable_all);
 
+DEFINE_STATIC_CALL_NULL(x86_pmu_print_debug, *x86_pmu.print_debug);
+
 /*
  * This one is magic, it will get called even when PMU init fails (because
  * there is no PMU), in which case it should simply return NULL.
@@ -1571,26 +1573,20 @@ static void x86_pmu_start(struct perf_event *event, int flags)
 	perf_event_update_userpage(event);
 }
 
-void perf_event_print_debug(void)
+void x86_pmu_print_debug(int cpu)
 {
 	u64 ctrl, status, overflow, pmc_ctrl, pmc_count, prev_left, fixed;
 	unsigned long *cntr_mask, *fixed_cntr_mask;
 	struct event_constraint *pebs_constraints;
 	struct cpu_hw_events *cpuc;
 	u64 pebs, debugctl;
-	int cpu, idx;
-
-	guard(irqsave)();
+	int idx;
 
-	cpu = smp_processor_id();
 	cpuc = &per_cpu(cpu_hw_events, cpu);
 	cntr_mask = hybrid(cpuc->pmu, cntr_mask);
 	fixed_cntr_mask = hybrid(cpuc->pmu, fixed_cntr_mask);
 	pebs_constraints = hybrid(cpuc->pmu, pebs_constraints);
 
-	if (!*(u64 *)cntr_mask)
-		return;
-
 	if (x86_pmu.version >= 2) {
 		rdmsrq(MSR_CORE_PERF_GLOBAL_CTRL, ctrl);
 		rdmsrq(MSR_CORE_PERF_GLOBAL_STATUS, status);
@@ -1634,6 +1630,24 @@ void perf_event_print_debug(void)
 	}
 }
 
+void perf_event_print_debug(void)
+{
+	struct cpu_hw_events *cpuc;
+	unsigned long *cntr_mask;
+	int cpu;
+
+	guard(irqsave)();
+
+	cpu = smp_processor_id();
+	cpuc = &per_cpu(cpu_hw_events, cpu);
+	cntr_mask = hybrid(cpuc->pmu, cntr_mask);
+
+	if (!*(u64 *)cntr_mask)
+		return;
+
+	static_call(x86_pmu_print_debug)(cpu);
+}
+
 void x86_pmu_stop(struct perf_event *event, int flags)
 {
 	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);
@@ -2122,6 +2136,8 @@ static void x86_pmu_static_call_update(void)
 	static_call_update(x86_pmu_pebs_disable, x86_pmu.pebs_disable);
 	static_call_update(x86_pmu_pebs_enable_all, x86_pmu.pebs_enable_all);
 	static_call_update(x86_pmu_pebs_disable_all, x86_pmu.pebs_disable_all);
+
+	static_call_update(x86_pmu_print_debug, x86_pmu.print_debug);
 }
 
 static void _x86_pmu_read(struct perf_event *event)
@@ -2233,6 +2249,9 @@ static int __init init_hw_perf_events(void)
 	if (!x86_pmu.update)
 		x86_pmu.update = x86_perf_event_update;
 
+	if (!x86_pmu.print_debug)
+		x86_pmu.print_debug = x86_pmu_print_debug;
+
 	x86_pmu_static_call_update();
 
 	/*
arch/x86/events/perf_event.h +4 -0
index fa381110f7a7..a5d15ff50a2d 100644
@@ -1052,6 +1052,8 @@ struct x86_pmu {
 	int				num_hybrid_pmus;
 	struct x86_hybrid_pmu		*hybrid_pmu;
 	enum intel_cpu_type (*get_hybrid_cpu_type)	(void);
+
+	void (*print_debug)(int cpu);
 };
 
 struct x86_perf_task_context_opt {
@@ -1317,6 +1319,8 @@ int x86_pmu_handle_irq(struct pt_regs *regs);
 
 void x86_pmu_show_pmu_cap(struct pmu *pmu);
 
+void x86_pmu_print_debug(int cpu);
+
 static inline int x86_pmu_num_counters(struct pmu *pmu)
 {
 	return hweight64(hybrid(pmu, cntr_mask64));
Signature · 1 line
2.53.0
#2 lore raw

Re: [PATCH v2 1/3] perf/x86: Add x86_pmu::print_debug

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 14 lines

perf_event_print_debug() dumps the global control and status MSRs whenever x86_pmu.version >= 2, reading registers that exist only on Intel-compatible PMUs. This is not safe since x86_pmu.version is not Intel-specific and is now set by other vendors whose global registers use different addresses.

As a first step, split perf_event_print_debug() in two. The register dump moves into a new common helper, x86_pmu_print_debug(), leaving perf_event_print_debug() to handle the preamble and dispatch to an optional x86_pmu::print_debug method. This lets each vendor-specific PMU dump its own global state before chaining into the common helper. PMUs that do not implement the method, such as those with x86_pmu.version < 2, get the common helper alone.

Quoted 3 lines

No functional change intended.

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#3 lore raw

PATCH v2 2/3 perf/x86: Move MSR_CORE_PERF_GLOBAL_* dump to vendor code

In reply to Sandipan Das

MSR_CORE_PERF_GLOBAL_{CTRL,STATUS} and MSR_ARCH_PERFMON_FIXED_CTR_CTRL are Intel Architectural PerfMon registers that are not available on AMD processors.

Since commit 21d59e3e2c40 ("perf/x86/amd/core: Detect PerfMonV2 support"), x86_pmu.version is also set to 2 on AMD processors that support PerfMonV2, which makes perf_event_print_debug() attempt to read these non-existent MSRs. The resulting #GP is caught by the exception fixup, but as the reads are unchecked, SysRq-P logs an "unchecked MSR access error" for each one and then prints zeroes as if they were valid register contents.

Implement x86_pmu::print_debug for the Intel and Zhaoxin PMUs, which do have these registers, and drop the reads from common code. Both are wired up through PMU descriptors that are selected only when the PMU version is at least 2, so the version check is no longer needed.

MSR_CORE_PERF_GLOBAL_OVF_CTRL is also dropped from the dump since it is write-to-clear and does not read back the overflow state.

Fixes 21d59e3e2c40 ("perf/x86/amd/core: Detect PerfMonV2 support")
---
 arch/x86/events/core.c         | 27 ++-------------------------
 arch/x86/events/intel/core.c   | 32 ++++++++++++++++++++++++++++++++
 arch/x86/events/zhaoxin/core.c | 18 ++++++++++++++++++
 3 files changed, 52 insertions(+), 25 deletions(-)
arch/x86/events/core.c +2 -25
index 364a4c83f677..1b5af204cf64 100644
@@ -1575,38 +1575,15 @@ static void x86_pmu_start(struct perf_event *event, int flags)
 
 void x86_pmu_print_debug(int cpu)
 {
-	u64 ctrl, status, overflow, pmc_ctrl, pmc_count, prev_left, fixed;
 	unsigned long *cntr_mask, *fixed_cntr_mask;
-	struct event_constraint *pebs_constraints;
+	u64 pmc_ctrl, pmc_count, prev_left;
 	struct cpu_hw_events *cpuc;
-	u64 pebs, debugctl;
 	int idx;
 
 	cpuc = &per_cpu(cpu_hw_events, cpu);
 	cntr_mask = hybrid(cpuc->pmu, cntr_mask);
 	fixed_cntr_mask = hybrid(cpuc->pmu, fixed_cntr_mask);
-	pebs_constraints = hybrid(cpuc->pmu, pebs_constraints);
-
-	if (x86_pmu.version >= 2) {
-		rdmsrq(MSR_CORE_PERF_GLOBAL_CTRL, ctrl);
-		rdmsrq(MSR_CORE_PERF_GLOBAL_STATUS, status);
-		rdmsrq(MSR_CORE_PERF_GLOBAL_OVF_CTRL, overflow);
-		rdmsrq(MSR_ARCH_PERFMON_FIXED_CTR_CTRL, fixed);
-
-		pr_info("\n");
-		pr_info("CPU#%d: ctrl:       %016llx\n", cpu, ctrl);
-		pr_info("CPU#%d: status:     %016llx\n", cpu, status);
-		pr_info("CPU#%d: overflow:   %016llx\n", cpu, overflow);
-		pr_info("CPU#%d: fixed:      %016llx\n", cpu, fixed);
-		if (pebs_constraints) {
-			rdmsrq(MSR_IA32_PEBS_ENABLE, pebs);
-			pr_info("CPU#%d: pebs:       %016llx\n", cpu, pebs);
-		}
-		if (x86_pmu.lbr_nr) {
-			rdmsrq(MSR_IA32_DEBUGCTLMSR, debugctl);
-			pr_info("CPU#%d: debugctl:   %016llx\n", cpu, debugctl);
-		}
-	}
+
 	pr_info("CPU#%d: active:     %016llx\n", cpu, *(u64 *)cpuc->active_mask);
 
 	for_each_set_bit(idx, cntr_mask, X86_PMC_IDX_MAX) {
arch/x86/events/intel/core.c +32 -0
index f6ee1819864e..10f2f8f49340 100644
@@ -3734,6 +3734,36 @@ static void intel_pmu_reset(void)
 	local_irq_restore(flags);
 }
 
+static void intel_pmu_print_debug(int cpu)
+{
+	struct event_constraint *pebs_constraints;
+	struct cpu_hw_events *cpuc;
+	u64 ctrl, status, fixed;
+	u64 pebs, debugctl;
+
+	cpuc = &per_cpu(cpu_hw_events, cpu);
+	pebs_constraints = hybrid(cpuc->pmu, pebs_constraints);
+
+	rdmsrq(MSR_CORE_PERF_GLOBAL_CTRL, ctrl);
+	rdmsrq(MSR_CORE_PERF_GLOBAL_STATUS, status);
+	rdmsrq(MSR_ARCH_PERFMON_FIXED_CTR_CTRL, fixed);
+
+	pr_info("\n");
+	pr_info("CPU#%d: ctrl:       %016llx\n", cpu, ctrl);
+	pr_info("CPU#%d: status:     %016llx\n", cpu, status);
+	pr_info("CPU#%d: fixed:      %016llx\n", cpu, fixed);
+	if (pebs_constraints) {
+		rdmsrq(MSR_IA32_PEBS_ENABLE, pebs);
+		pr_info("CPU#%d: pebs:       %016llx\n", cpu, pebs);
+	}
+	if (x86_pmu.lbr_nr) {
+		rdmsrq(MSR_IA32_DEBUGCTLMSR, debugctl);
+		pr_info("CPU#%d: debugctl:   %016llx\n", cpu, debugctl);
+	}
+
+	x86_pmu_print_debug(cpu);
+}
+
 /*
  * We may be running with guest PEBS events created by KVM, and the
  * PEBS records are logged into the guest's DS and invisible to host.
@@ -6706,6 +6736,8 @@ static __initconst const struct x86_pmu intel_pmu = {
 	 * counting SMM by default.
 	 */
 	.attr_freeze_on_smi	= 1,
+
+	.print_debug		= intel_pmu_print_debug,
 };
 
 static __init void intel_clovertown_quirk(void)
arch/x86/events/zhaoxin/core.c +18 -0
index e506f677db57..cc447e228efb 100644
@@ -288,6 +288,22 @@ static inline void zxc_pmu_ack_status(u64 ack)
 	zhaoxin_pmu_disable_all();
 }
 
+static void zhaoxin_pmu_print_debug(int cpu)
+{
+	u64 ctrl, status, fixed;
+
+	rdmsrq(MSR_CORE_PERF_GLOBAL_CTRL, ctrl);
+	rdmsrq(MSR_CORE_PERF_GLOBAL_STATUS, status);
+	rdmsrq(MSR_ARCH_PERFMON_FIXED_CTR_CTRL, fixed);
+
+	pr_info("\n");
+	pr_info("CPU#%d: ctrl:       %016llx\n", cpu, ctrl);
+	pr_info("CPU#%d: status:     %016llx\n", cpu, status);
+	pr_info("CPU#%d: fixed:      %016llx\n", cpu, fixed);
+
+	x86_pmu_print_debug(cpu);
+}
+
 static void zhaoxin_pmu_disable_fixed(struct hw_perf_event *hwc)
 {
 	int idx = hwc->idx - INTEL_PMC_IDX_FIXED;
@@ -479,6 +495,8 @@ static const struct x86_pmu zhaoxin_pmu __initconst = {
 
 	.format_attrs		= zx_arch_formats_attr,
 	.events_sysfs_show	= zhaoxin_event_sysfs_show,
+
+	.print_debug		= zhaoxin_pmu_print_debug,
 };
 
 static const struct { int id; char *name; } zx_arch_events_map[] __initconst = {
Signature · 1 line
2.53.0
#4 lore raw

Re: [PATCH v2 2/3] perf/x86: Move MSR_CORE_PERF_GLOBAL_* dump to vendor code

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Unintended removal of MSR_IA32_DEBUGCTLMSR from AMD PMU debug dump. --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unintended removal of MSR_IA32_DEBUGCTLMSR from AMD PMU debug dump. --

--- Patch [2]: [PATCH v2 2/3] perf/x86: Move MSR_CORE_PERF_GLOBAL_* dump to vendor code --- commit 9c3227849508af744eb4359d394119d9f9ee749a
Author: Sandipan Das <[email protected]>

perf/x86: Move MSR_CORE_PERF_GLOBAL_* dump to vendor code

This commit moves the dumping of MSR_CORE_PERF_GLOBAL_* and MSR_ARCH_PERFMON_FIXED_CTR_CTRL registers from common code to vendor-specific code. This prevents unchecked MSR access errors on AMD processors that report PerfMonV2 support but lack these Intel Architectural PerfMon registers.

Quoted 4 lines
arch/x86/events/core.c +0 -0
index ae773f8d55c39..4c70cb0148716 100644

[ ... ]

Quoted 26 lines
arch/x86/events/core.c +1 -14
@@ -1561,38 +1561,15 @@ static void x86_pmu_start(struct perf_event *event, int flags)
 
 void x86_pmu_print_debug(int cpu)
 {
-	u64 ctrl, status, overflow, pmc_ctrl, pmc_count, prev_left, fixed;
 	unsigned long *cntr_mask, *fixed_cntr_mask;
-	struct event_constraint *pebs_constraints;
+	u64 pmc_ctrl, pmc_count, prev_left;
 	struct cpu_hw_events *cpuc;
-	u64 pebs, debugctl;
 	int idx;
 
 	cpuc = &per_cpu(cpu_hw_events, cpu);
 	cntr_mask = hybrid(cpuc->pmu, cntr_mask);
 	fixed_cntr_mask = hybrid(cpuc->pmu, fixed_cntr_mask);
-	pebs_constraints = hybrid(cpuc->pmu, pebs_constraints);
-
-	if (x86_pmu.version >= 2) {
-		rdmsrq(MSR_CORE_PERF_GLOBAL_CTRL, ctrl);
-		rdmsrq(MSR_CORE_PERF_GLOBAL_STATUS, status);
-		rdmsrq(MSR_CORE_PERF_GLOBAL_OVF_CTRL, overflow);
-		rdmsrq(MSR_ARCH_PERFMON_FIXED_CTR_CTRL, fixed);
-
-		pr_info("\n");
-		pr_info("CPU#%d: ctrl:       %016llx\n", cpu, ctrl);
-		pr_info("CPU#%d: status:     %016llx\n", cpu, status);

arch/x86/events/core.c static void x86_pmu_start(struct perf_event *event, int flags) · line 1594

Quoted 10 lines
arch/x86/events/core.c +0 -10
-		pr_info("CPU#%d: overflow:   %016llx\n", cpu, overflow);
-		pr_info("CPU#%d: fixed:      %016llx\n", cpu, fixed);
-		if (pebs_constraints) {
-			rdmsrq(MSR_IA32_PEBS_ENABLE, pebs);
-			pr_info("CPU#%d: pebs:       %016llx\n", cpu, pebs);
-		}
-		if (x86_pmu.lbr_nr) {
-			rdmsrq(MSR_IA32_DEBUGCTLMSR, debugctl);
-			pr_info("CPU#%d: debugctl:   %016llx\n", cpu, debugctl);
-		}

[Severity: Medium]
Will this unintentionally drop the MSR_IA32_DEBUGCTLMSR dump for AMD CPUs?

AMD processors with LBR support use MSR_IA32_DEBUGCTLMSR for LBR freezing, but since this read was moved entirely to the Intel-specific intel_pmu_print_debug(), SysRq-P will no longer dump this register for AMD processors relying on the common x86_pmu_print_debug() function.

Should the debugctl dump be preserved in common code, or added to an AMD-specific print_debug function?

Quoted 5 lines
arch/x86/events/core.c +1 -1
-	}
+
 	pr_info("CPU#%d: active:     %016llx\n", cpu, *(u64 *)cpuc->active_mask);
 
 	for_each_set_bit(idx, cntr_mask, X86_PMC_IDX_MAX) {
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
#5 lore raw

Re: [PATCH v2 2/3] perf/x86: Move MSR_CORE_PERF_GLOBAL_* dump to vendor code

Quoted 64 lines · On 14-08-2026 23:31, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unintended removal of MSR_IA32_DEBUGCTLMSR from AMD PMU debug dump. --

--- Patch [2]: [PATCH v2 2/3] perf/x86: Move MSR_CORE_PERF_GLOBAL_* dump to vendor code --- commit 9c3227849508af744eb4359d394119d9f9ee749a
Author: Sandipan Das <[email protected]>

perf/x86: Move MSR_CORE_PERF_GLOBAL_* dump to vendor code

This commit moves the dumping of MSR_CORE_PERF_GLOBAL_* and MSR_ARCH_PERFMON_FIXED_CTR_CTRL registers from common code to vendor-specific code. This prevents unchecked MSR access errors on AMD processors that report PerfMonV2 support but lack these Intel Architectural PerfMon registers.

Quoted 4 lines
arch/x86/events/core.c +0 -0
index ae773f8d55c39..4c70cb0148716 100644

[ ... ]

Quoted 26 lines
arch/x86/events/core.c +1 -14
@@ -1561,38 +1561,15 @@ static void x86_pmu_start(struct perf_event *event, int flags)
 
 void x86_pmu_print_debug(int cpu)
 {
-	u64 ctrl, status, overflow, pmc_ctrl, pmc_count, prev_left, fixed;
 	unsigned long *cntr_mask, *fixed_cntr_mask;
-	struct event_constraint *pebs_constraints;
+	u64 pmc_ctrl, pmc_count, prev_left;
 	struct cpu_hw_events *cpuc;
-	u64 pebs, debugctl;
 	int idx;
 
 	cpuc = &per_cpu(cpu_hw_events, cpu);
 	cntr_mask = hybrid(cpuc->pmu, cntr_mask);
 	fixed_cntr_mask = hybrid(cpuc->pmu, fixed_cntr_mask);
-	pebs_constraints = hybrid(cpuc->pmu, pebs_constraints);
-
-	if (x86_pmu.version >= 2) {
-		rdmsrq(MSR_CORE_PERF_GLOBAL_CTRL, ctrl);
-		rdmsrq(MSR_CORE_PERF_GLOBAL_STATUS, status);
-		rdmsrq(MSR_CORE_PERF_GLOBAL_OVF_CTRL, overflow);
-		rdmsrq(MSR_ARCH_PERFMON_FIXED_CTR_CTRL, fixed);
-
-		pr_info("\n");
-		pr_info("CPU#%d: ctrl:       %016llx\n", cpu, ctrl);
-		pr_info("CPU#%d: status:     %016llx\n", cpu, status);

arch/x86/events/core.c static void x86_pmu_start(struct perf_event *event, int flags) · line 1594

Quoted 10 lines
arch/x86/events/core.c +0 -10
-		pr_info("CPU#%d: overflow:   %016llx\n", cpu, overflow);
-		pr_info("CPU#%d: fixed:      %016llx\n", cpu, fixed);
-		if (pebs_constraints) {
-			rdmsrq(MSR_IA32_PEBS_ENABLE, pebs);
-			pr_info("CPU#%d: pebs:       %016llx\n", cpu, pebs);
-		}
-		if (x86_pmu.lbr_nr) {
-			rdmsrq(MSR_IA32_DEBUGCTLMSR, debugctl);
-			pr_info("CPU#%d: debugctl:   %016llx\n", cpu, debugctl);
-		}

[Severity: Medium]
Will this unintentionally drop the MSR_IA32_DEBUGCTLMSR dump for AMD CPUs?

Quoted 8 lines

AMD processors with LBR support use MSR_IA32_DEBUGCTLMSR for LBR freezing, but since this read was moved entirely to the Intel-specific intel_pmu_print_debug(), SysRq-P will no longer dump this register for AMD processors relying on the common x86_pmu_print_debug() function.

Should the debugctl dump be preserved in common code, or added to an AMD-specific print_debug function?

If the intent is to just see if LBR is enabled, then amd_pmu_v2_print_debug() already does that by dumping MSR_AMD_DBG_EXTN_CFG. I am not sure it matters here whether freeze is enabled as well.

Quoted 6 lines
Quoted 5 lines
arch/x86/events/core.c +1 -1
-	}
+
 	pr_info("CPU#%d: active:     %016llx\n", cpu, *(u64 *)cpuc->active_mask);
 
 	for_each_set_bit(idx, cntr_mask, X86_PMC_IDX_MAX) {
#6 lore raw

PATCH v2 3/3 perf/x86/amd: Implement x86_pmu::print_debug

In reply to Sandipan Das

Implement x86_pmu::print_debug for PerfMonV2 capable processors to dump MSR_AMD64_PERF_CNTR_GLOBAL_{CTL,STATUS}, plus MSR_AMD_DBG_EXTN_CFG if LBR Extension Version 2 is available. Chain into x86_pmu_print_debug() afterwards to dump the event selectors and counters.

MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_{CLR,SET} are left out since they are write-only.

---
 arch/x86/events/amd/core.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)
arch/x86/events/amd/core.c +20 -0
index a787409f5a62..ecd648af81f7 100644
@@ -841,6 +841,25 @@ static void amd_pmu_v2_disable_all(void)
 	amd_pmu_check_overflow();
 }
 
+static void amd_pmu_v2_print_debug(int cpu)
+{
+	u64 ctrl, status, debugextn;
+
+	rdmsrq(MSR_AMD64_PERF_CNTR_GLOBAL_CTL, ctrl);
+	rdmsrq(MSR_AMD64_PERF_CNTR_GLOBAL_STATUS, status);
+
+	pr_info("\n");
+	pr_info("CPU#%d: ctrl:       %016llx\n", cpu, ctrl);
+	pr_info("CPU#%d: status:     %016llx\n", cpu, status);
+
+	if (cpu_feature_enabled(X86_FEATURE_AMD_LBR_V2)) {
+		rdmsrq(MSR_AMD_DBG_EXTN_CFG, debugextn);
+		pr_info("CPU#%d: debugextn:  %016llx\n", cpu, debugextn);
+	}
+
+	x86_pmu_print_debug(cpu);
+}
+
 DEFINE_STATIC_CALL_NULL(amd_pmu_branch_add, *x86_pmu.add);
 
 static void amd_pmu_add_event(struct perf_event *event)
@@ -1452,6 +1471,7 @@ static int __init amd_core_pmu_init(void)
 		x86_pmu.disable_all = amd_pmu_v2_disable_all;
 		x86_pmu.enable = amd_pmu_v2_enable_event;
 		x86_pmu.handle_irq = amd_pmu_v2_handle_irq;
+		x86_pmu.print_debug = amd_pmu_v2_print_debug;
 		static_call_update(amd_pmu_test_overflow, amd_pmu_test_overflow_status);
 	}
 
Signature · 1 line
2.53.0
#7 lore raw

Re: [PATCH v2 3/3] perf/x86/amd: Implement x86_pmu::print_debug

Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 8 lines

Implement x86_pmu::print_debug for PerfMonV2 capable processors to dump MSR_AMD64_PERF_CNTR_GLOBAL_{CTL,STATUS}, plus MSR_AMD_DBG_EXTN_CFG if LBR Extension Version 2 is available. Chain into x86_pmu_print_debug() afterwards to dump the event selectors and counters.

MSR_AMD64_PERF_CNTR_GLOBAL_STATUS_{CLR,SET} are left out since they are write-only.

Quoted 6 lines

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3
Reviewed-by on the v2 series
#8 lore raw
In reply to Sandipan Das

The whole patch series looks good to me. Thanks.

Quoted 38 lines · Sandipan Das

Petr Tesarik reported that reading MSR_ARCH_PERFMON_FIXED_CTR_CTRL raises a #GP on AMD processors that support PerfMonV2, and proposed skipping that register when no fixed counters are implemented [1].

However, perf_event_print_debug() also dumps the global control and status registers whenever x86_pmu.version >= 2, and reads Intel Architectural PerfMon MSRs to do so. That condition used to be Intel-specific, but since commit 21d59e3e2c40 ("perf/x86/amd/core:
Detect PerfMonV2 support"), x86_pmu.version is also set to 2 on AMD processors that support PerfMonV2, where none of those registers are available.

The same reasoning therefore applies to the other registers read here, so rather than special-casing each one or reinstating a vendor check, this series makes the global register dump a vendor-specific operation.

[1] https://lore.kernel.org/all/[email protected]/

Previous versions can be found at
v1: https://lore.kernel.org/all/[email protected]/

Changes in v2:
* Address build issue reported by Sashiko.
* Use a static_call as suggested by Dapeng.
* Pick up Tested-by from Petr.

Sandipan Das (3):
perf/x86: Add x86_pmu::print_debug
perf/x86: Move MSR_CORE_PERF_GLOBAL_* dump to vendor code perf/x86/amd: Implement x86_pmu::print_debug

 arch/x86/events/amd/core.c     | 20 ++++++++++++
 arch/x86/events/core.c         | 60 ++++++++++++++++------------------
 arch/x86/events/intel/core.c   | 32 ++++++++++++++++++
 arch/x86/events/perf_event.h   |  4 +++
 arch/x86/events/zhaoxin/core.c | 18 ++++++++++
 5 files changed, 102 insertions(+), 32 deletions(-)
reviewer
#9 lore raw
Quoted 4 lines · Dapeng

The whole patch series looks good to me. Thanks.

Ping.

Thanks,
Ian

Quoted 39 lines · Sandipan Das
Quoted 38 lines

Petr Tesarik reported that reading MSR_ARCH_PERFMON_FIXED_CTR_CTRL raises a #GP on AMD processors that support PerfMonV2, and proposed skipping that register when no fixed counters are implemented [1].

However, perf_event_print_debug() also dumps the global control and status registers whenever x86_pmu.version >= 2, and reads Intel Architectural PerfMon MSRs to do so. That condition used to be Intel-specific, but since commit 21d59e3e2c40 ("perf/x86/amd/core:
Detect PerfMonV2 support"), x86_pmu.version is also set to 2 on AMD processors that support PerfMonV2, where none of those registers are available.

The same reasoning therefore applies to the other registers read here, so rather than special-casing each one or reinstating a vendor check, this series makes the global register dump a vendor-specific operation.

[1] https://lore.kernel.org/all/[email protected]/

Previous versions can be found at
v1: https://lore.kernel.org/all/[email protected]/

Changes in v2:
* Address build issue reported by Sashiko.
* Use a static_call as suggested by Dapeng.
* Pick up Tested-by from Petr.

Sandipan Das (3):
perf/x86: Add x86_pmu::print_debug
perf/x86: Move MSR_CORE_PERF_GLOBAL_* dump to vendor code perf/x86/amd: Implement x86_pmu::print_debug

 arch/x86/events/amd/core.c     | 20 ++++++++++++
 arch/x86/events/core.c         | 60 ++++++++++++++++------------------
 arch/x86/events/intel/core.c   | 32 ++++++++++++++++++
 arch/x86/events/perf_event.h   |  4 +++
 arch/x86/events/zhaoxin/core.c | 18 ++++++++++
 5 files changed, 102 insertions(+), 32 deletions(-)