* [PATCH 1/1] perf/x86: Do not read fixed counter control register if there is none
@ 2026-07-17 15:03 Petr Tesarik
2026-07-17 15:18 ` sashiko-bot
0 siblings, 1 reply; 3+ messages in thread
From: Petr Tesarik @ 2026-07-17 15:03 UTC (permalink / raw)
To: Alexander Shishkin, Sandipan Das, Jiri Olsa, Ian Rogers,
Adrian Hunter, James Clark, Thomas Gleixner, Borislav Petkov,
Dave Hansen
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, H. Peter Anvin, x86, linux-perf-users,
linux-kernel, Petr Tesarik
When printing out CPU performance registers for debugging, skip
MSR_ARCH_PERFMON_FIXED_CTR_CTRL if fixed counters are not implemented.
Since AMD CPUs with PerMonV2 do not implement this register, attempts to
read it generate a #GP. In particular, SysRq-P logs something like this:
[ 14.748144] [ T1118] sysrq: Show Regs
[ 14.748896] [ T1118] unchecked MSR access error: RDMSR from 0x38d at rIP: 0xffffffff937154c6 (native_read_msr+0x6/0x40)
[ 14.750878] [ T1118] Call Trace:
[ 14.751505] [ T1118] <TASK>
[ 14.752057] [ T1118] paravirt_read_msr+0x7/0x10
[ 14.752940] [ T1118] perf_event_print_debug+0x135/0x240
[ 14.753931] [ T1118] __handle_sysrq.cold+0x9b/0xde
[ 14.754855] [ T1118] write_sysrq_trigger+0x59/0x80
[ 14.755779] [ T1118] proc_reg_write+0x5a/0xb0
[ 14.756631] [ T1118] vfs_write+0xce/0x430
[ 14.757406] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.758322] [ T1118] ? security_file_fcntl+0x5f/0x120
[ 14.759083] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.759754] [ T1118] ? __x64_sys_fcntl+0x80/0x110
[ 14.760334] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.761005] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.761676] [ T1118] ? __ct_user_enter+0x27/0xc0
[ 14.762270] [ T1118] ksys_write+0x6a/0xe0
[ 14.762776] [ T1118] do_syscall_64+0xe1/0x610
[ 14.763324] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.763995] [ T1118] ? __ct_user_enter+0x27/0xc0
[ 14.764564] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.765233] [ T1118] ? do_syscall_64+0x205/0x610
[ 14.765806] [ T1118] ? __x64_sys_ioctl+0x97/0xe0
[ 14.766375] [ T1118] ? __pfx_woken_wake_function+0x10/0x10
[ 14.767048] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.767717] [ T1118] ? get_close_on_exec+0x34/0x40
[ 14.768311] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.768985] [ T1118] ? do_fcntl+0x6da/0x8e0
[ 14.769502] [ T1118] ? do_syscall_64+0x205/0x610
[ 14.770083] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.772321] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.773166] [ T1118] ? tty_read+0x150/0x220
[ 14.773722] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.774423] [ T1118] ? filp_flush+0x5b/0x80
[ 14.774966] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.775660] [ T1118] ? filp_close+0x25/0x40
[ 14.776214] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.776936] [ T1118] ? do_dup2+0xae/0x140
[ 14.777486] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.778309] [ T1118] ? ksys_dup3+0x67/0xf0
[ 14.778870] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.779600] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.780310] [ T1118] ? __ct_user_enter+0x27/0xc0
[ 14.780917] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.781613] [ T1118] ? do_syscall_64+0x205/0x610
[ 14.782215] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.782907] [ T1118] ? ksys_read+0x6a/0xe0
[ 14.783429] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.784120] [ T1118] ? __ct_user_enter+0x27/0xc0
[ 14.784741] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.785431] [ T1118] ? __ct_user_exit+0x15/0xb0
[ 14.786014] [ T1118] ? srso_alias_return_thunk+0x5/0xfbef5
[ 14.786706] [ T1118] ? do_syscall_64+0x98/0x610
[ 14.787282] [ T1118] ? exc_page_fault+0x6d/0x180
[ 14.787873] [ T1118] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 14.788612] [ T1118] RIP: 0033:0x7f70d6497e0a
[ 14.789186] [ T1118] Code: 08 03 00 00 59 5e 48 83 f8 fc 75 1e 83 e2 39 83 fa 08 75 16 e8 05 ff ff ff 0f 1f 80 00 00 00 00 49 89 ca 48 8b 44 24 20 0f 05 <48> 83 c4 18 c3 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 f3 0f
[ 14.791587] [ T1118] RSP: 002b:00007ffc36a93580 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
[ 14.792617] [ T1118] RAX: ffffffffffffffda RBX: 00007f70d65f6580 RCX: 00007f70d6497e0a
[ 14.793594] [ T1118] RDX: 0000000000000002 RSI: 0000556f34e41c30 RDI: 0000000000000001
[ 14.794574] [ T1118] RBP: 0000000000000002 R08: 0000000000000000 R09: 0000000000000000
[ 14.795563] [ T1118] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000002
[ 14.796537] [ T1118] R13: 0000556f34e41c30 R14: 0000000000000001 R15: 0000000000000000
[ 14.797514] [ T1118] </TASK>
Signed-off-by: Petr Tesarik <ptesarik@suse.com>
---
arch/x86/events/core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index d1af33d96d0a3..8310bedfc364f 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -1581,13 +1581,16 @@ void perf_event_print_debug(void)
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 (x86_pmu.fixedctr) {
+ rdmsrq(MSR_ARCH_PERFMON_FIXED_CTR_CTRL, fixed);
+ 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);
--
2.54.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/1] perf/x86: Do not read fixed counter control register if there is none
2026-07-17 15:03 [PATCH 1/1] perf/x86: Do not read fixed counter control register if there is none Petr Tesarik
@ 2026-07-17 15:18 ` sashiko-bot
2026-07-17 15:51 ` Petr Tesarik
0 siblings, 1 reply; 3+ messages in thread
From: sashiko-bot @ 2026-07-17 15:18 UTC (permalink / raw)
To: Petr Tesarik; +Cc: linux-perf-users
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] Using `x86_pmu.fixedctr` as a capability flag is conceptually flawed and suppresses valid debug output on Zhaoxin.
Pre-existing issues:
- [High] Zhaoxin PMU fails to initialize `x86_pmu.fixedctr`, leading to broken fixed counters and unsafe RDMSR accesses.
- [High] Undefined behavior in `GENMASK_ULL` during Zhaoxin PMU initialization if fixed counters are missing.
--
--- Patch [1]: [PATCH 1/1] perf/x86: Do not read fixed counter control register if there is none ---
commit d0dd53a6709a65141e2d105b19abad4d851825d4
Author: Petr Tesarik <ptesarik@suse.com>
perf/x86: Do not read fixed counter control register if there is none
This patch skips reading the MSR_ARCH_PERFMON_FIXED_CTR_CTRL register
during perf debug output if fixed counters are not implemented. This avoids
a #GP on AMD CPUs with PerfMonV2 that do not implement this register.
> diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> index 576baa9a52c5b..3055d59e0e82f 100644
> --- a/arch/x86/events/core.c
> +++ b/arch/x86/events/core.c
> @@ -1572,13 +1572,16 @@ void perf_event_print_debug(void)
> 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 (x86_pmu.fixedctr) {
[Severity: Medium]
Is it safe to use x86_pmu.fixedctr as a capability flag here?
The x86_pmu.fixedctr field is an MSR address, not a boolean capability
flag. While this check works for most architectures, the Zhaoxin PMU driver
fails to initialize this field, leaving it as 0. Because of this, the check
evaluates to false and inadvertently suppresses the debug output of
MSR_ARCH_PERFMON_FIXED_CTR_CTRL on Zhaoxin systems.
> + rdmsrq(MSR_ARCH_PERFMON_FIXED_CTR_CTRL, fixed);
> + 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);
[ ... ]
[Severity: High]
This is a pre-existing issue and was not introduced by the patch under
review, but the lack of initialization for x86_pmu.fixedctr causes other
issues.
In the Zhaoxin PMU driver setup:
arch/x86/events/zhaoxin/core.c:zhaoxin_pmu
static const struct x86_pmu zhaoxin_pmu __initconst = {
...
/* missing .fixedctr = MSR_ARCH_PERFMON_FIXED_CTR0 */
};
Because this remains 0, x86_pmu_fixed_ctr_addr() returns 0 + idx instead of
the correct MSR. This causes fixed counter 0 to silently return without
updating in x86_perf_event_update():
arch/x86/events/core.c:x86_perf_event_update() {
...
if (unlikely(!hwc->event_base))
return 0;
...
}
It also leads to unsafe MSR reads when iterating over the fixed counter
mask in perf_event_print_debug():
arch/x86/events/core.c:perf_event_print_debug() {
...
for_each_set_bit(idx, fixed_cntr_mask, X86_PMC_IDX_MAX) {
...
rdmsrq(x86_pmu_fixed_ctr_addr(idx), pmc_count);
...
}
}
Could this lead to reads from arbitrary low MSRs like MSR 0, 1, or 2,
potentially triggering a system crash if a user programs fixed counter
events or triggers SysRq-P?
[Severity: High]
This is a pre-existing issue and was not introduced by the patch under
review, but does this bitmask generation in the Zhaoxin PMU initialization
risk undefined behavior?
arch/x86/events/zhaoxin/core.c:zhaoxin_pmu_init() {
...
x86_pmu.fixed_cntr_mask64 = GENMASK_ULL(edx.split.num_counters_fixed - 1, 0);
...
}
If a hypervisor configures the CPU to report 0 fixed counters,
edx.split.num_counters_fixed - 1 wraps to -1. Expanding GENMASK_ULL(-1, 0)
results in a right-shift by 64, which is undefined behavior in C. On x86,
this yields ~0ULL, making the kernel assume 64 fixed counters exist.
Could this cause the kernel to attempt reading 64 undefined MSRs during
debug output or event initialization, leading to #GP exceptions?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717150315.2868314-1-ptesarik@suse.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 1/1] perf/x86: Do not read fixed counter control register if there is none
2026-07-17 15:18 ` sashiko-bot
@ 2026-07-17 15:51 ` Petr Tesarik
0 siblings, 0 replies; 3+ messages in thread
From: Petr Tesarik @ 2026-07-17 15:51 UTC (permalink / raw)
To: Tony W Wang-oc; +Cc: sashiko-reviews, linux-perf-users, x86, linux-kernel
On Fri, 17 Jul 2026 15:18:58 +0000
sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [Medium] Using `x86_pmu.fixedctr` as a capability flag is conceptually flawed and suppresses valid debug output on Zhaoxin.
>
> Pre-existing issues:
> - [High] Zhaoxin PMU fails to initialize `x86_pmu.fixedctr`, leading to broken fixed counters and unsafe RDMSR accesses.
> - [High] Undefined behavior in `GENMASK_ULL` during Zhaoxin PMU initialization if fixed counters are missing.
> --
>
> --- Patch [1]: [PATCH 1/1] perf/x86: Do not read fixed counter control register if there is none ---
> commit d0dd53a6709a65141e2d105b19abad4d851825d4
> Author: Petr Tesarik <ptesarik@suse.com>
>
> perf/x86: Do not read fixed counter control register if there is none
>
> This patch skips reading the MSR_ARCH_PERFMON_FIXED_CTR_CTRL register
> during perf debug output if fixed counters are not implemented. This avoids
> a #GP on AMD CPUs with PerfMonV2 that do not implement this register.
>
> > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
> > index 576baa9a52c5b..3055d59e0e82f 100644
> > --- a/arch/x86/events/core.c
> > +++ b/arch/x86/events/core.c
> > @@ -1572,13 +1572,16 @@ void perf_event_print_debug(void)
> > 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 (x86_pmu.fixedctr) {
>
> [Severity: Medium]
> Is it safe to use x86_pmu.fixedctr as a capability flag here?
Good question. In fact, I thought about it, and yes, introducing a new
flag would be the cleanest solution, but it seemed overkill just for
this debugging output.
AFAIK the MSR offset of the fixed perf counters cannot be zero, because
those MSR addresses have been assigned to other architectural MSRs since
Pentium times...
> The x86_pmu.fixedctr field is an MSR address, not a boolean capability
> flag. While this check works for most architectures, the Zhaoxin PMU driver
> fails to initialize this field, leaving it as 0. Because of this, the check
> evaluates to false and inadvertently suppresses the debug output of
> MSR_ARCH_PERFMON_FIXED_CTR_CTRL on Zhaoxin systems.
I don't know about Zhaoxin. Do these syste have fixed performance
counters and a corresponding global control register?
Adding Tony W Wang to the recipient list...
Petr T
--
> > + rdmsrq(MSR_ARCH_PERFMON_FIXED_CTR_CTRL,
> > fixed);
> > + 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);
> [ ... ]
>
>
> [Severity: High]
> This is a pre-existing issue and was not introduced by the patch under
> review, but the lack of initialization for x86_pmu.fixedctr causes
> other issues.
>
> In the Zhaoxin PMU driver setup:
>
> arch/x86/events/zhaoxin/core.c:zhaoxin_pmu
> static const struct x86_pmu zhaoxin_pmu __initconst = {
> ...
> /* missing .fixedctr = MSR_ARCH_PERFMON_FIXED_CTR0 */
> };
>
> Because this remains 0, x86_pmu_fixed_ctr_addr() returns 0 + idx
> instead of the correct MSR. This causes fixed counter 0 to silently
> return without updating in x86_perf_event_update():
>
> arch/x86/events/core.c:x86_perf_event_update() {
> ...
> if (unlikely(!hwc->event_base))
> return 0;
> ...
> }
>
> It also leads to unsafe MSR reads when iterating over the fixed
> counter mask in perf_event_print_debug():
>
> arch/x86/events/core.c:perf_event_print_debug() {
> ...
> for_each_set_bit(idx, fixed_cntr_mask, X86_PMC_IDX_MAX) {
> ...
> rdmsrq(x86_pmu_fixed_ctr_addr(idx), pmc_count);
> ...
> }
> }
>
> Could this lead to reads from arbitrary low MSRs like MSR 0, 1, or 2,
> potentially triggering a system crash if a user programs fixed counter
> events or triggers SysRq-P?
>
>
> [Severity: High]
> This is a pre-existing issue and was not introduced by the patch under
> review, but does this bitmask generation in the Zhaoxin PMU
> initialization risk undefined behavior?
>
> arch/x86/events/zhaoxin/core.c:zhaoxin_pmu_init() {
> ...
> x86_pmu.fixed_cntr_mask64 =
> GENMASK_ULL(edx.split.num_counters_fixed - 1, 0); ...
> }
>
> If a hypervisor configures the CPU to report 0 fixed counters,
> edx.split.num_counters_fixed - 1 wraps to -1. Expanding
> GENMASK_ULL(-1, 0) results in a right-shift by 64, which is undefined
> behavior in C. On x86, this yields ~0ULL, making the kernel assume 64
> fixed counters exist.
>
> Could this cause the kernel to attempt reading 64 undefined MSRs
> during debug output or event initialization, leading to #GP
> exceptions?
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-17 15:51 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17 15:03 [PATCH 1/1] perf/x86: Do not read fixed counter control register if there is none Petr Tesarik
2026-07-17 15:18 ` sashiko-bot
2026-07-17 15:51 ` Petr Tesarik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox