linux-kernel - [PATCH v2 2/2] perf/x86/amd: Stop calling amd_pmu_cpu_reset() from amd_pmu_cpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 26 Oct 2023 12:03:30 -0500
From:   Mario Limonciello <mario.limonciello@....com>
To:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...en8.de>,
        "maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)" <x86@...nel.org>
CC:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...nel.org>,
        "Namhyung Kim" <namhyung@...nel.org>,
        Ian Rogers <irogers@...gle.com>,
        Adrian Hunter <adrian.hunter@...el.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "H . Peter Anvin" <hpa@...or.com>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        "Len Brown" <len.brown@...el.com>, Pavel Machek <pavel@....cz>,
        David Woodhouse <dwmw@...zon.co.uk>,
        Sandipan Das <sandipan.das@....com>,
        "open list:PERFORMANCE EVENTS SUBSYSTEM" 
        <linux-perf-users@...r.kernel.org>,
        "open list:PERFORMANCE EVENTS SUBSYSTEM" 
        <linux-kernel@...r.kernel.org>,
        "open list:SUSPEND TO RAM" <linux-pm@...r.kernel.org>,
        "open list:ACPI" <linux-acpi@...r.kernel.org>,
        Mario Limonciello <mario.limonciello@....com>
Subject: [PATCH v2 2/2] perf/x86/amd: Stop calling amd_pmu_cpu_reset() from amd_pmu_cpu_dead()

During suspend testing on a workstation CPU a preemption BUG was reported.

```
BUG: using smp_processor_id() in preemptible [00000000] code: rtcwake/2960
caller is amd_pmu_lbr_reset+0x19/0xc0
CPU: 104 PID: 2960 Comm: rtcwake Not tainted 6.6.0-rc6-00002-g3e2c7f3ac51f
Call Trace:
 <TASK>
 dump_stack_lvl+0x44/0x60
 check_preemption_disabled+0xce/0xf0
 ? __pfx_x86_pmu_dead_cpu+0x10/0x10
 amd_pmu_lbr_reset+0x19/0xc0
 ? __pfx_x86_pmu_dead_cpu+0x10/0x10
 amd_pmu_cpu_reset.constprop.0+0x51/0x60
 amd_pmu_cpu_dead+0x3e/0x90
 x86_pmu_dead_cpu+0x13/0x20
 cpuhp_invoke_callback+0x169/0x4b0
 ? __pfx_virtnet_cpu_dead+0x10/0x10
 __cpuhp_invoke_callback_range+0x76/0xe0
 _cpu_down+0x112/0x270
 freeze_secondary_cpus+0x8e/0x280
 suspend_devices_and_enter+0x342/0x900
 pm_suspend+0x2fd/0x690
 state_store+0x71/0xd0
 kernfs_fop_write_iter+0x128/0x1c0
 vfs_write+0x2db/0x400
 ksys_write+0x5f/0xe0
 do_syscall_64+0x59/0x90
 ? srso_alias_return_thunk+0x5/0x7f
 ? count_memcg_events.constprop.0+0x1a/0x30
 ? srso_alias_return_thunk+0x5/0x7f
 ? handle_mm_fault+0x1e9/0x340
 ? srso_alias_return_thunk+0x5/0x7f
 ? preempt_count_add+0x4d/0xa0
 ? srso_alias_return_thunk+0x5/0x7f
 ? up_read+0x38/0x70
 ? srso_alias_return_thunk+0x5/0x7f
 ? do_user_addr_fault+0x343/0x6b0
 ? srso_alias_return_thunk+0x5/0x7f
 ? exc_page_fault+0x74/0x170
 entry_SYSCALL_64_after_hwframe+0x6e/0xd8
RIP: 0033:0x7f32f8d14a77
Code: 10 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa
64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff
77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
RSP: 002b:00007ffdc648de18 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000004 RCX: 00007f32f8d14a77
RDX: 0000000000000004 RSI: 000055b2fc2a5670 RDI: 0000000000000004
RBP: 000055b2fc2a5670 R08: 0000000000000000 R09: 000055b2fc2a5670
R10: 00007f32f8e1a2f0 R11: 0000000000000246 R12: 0000000000000004
R13: 000055b2fc2a2480 R14: 00007f32f8e16600 R15: 00007f32f8e15a00
 </TASK>
```

This bug shows that there is a mistake with the flow used for offlining
a CPU.  Calling amd_pmu_cpu_reset() from the dead callback is problematic
because this doesn't run on the actual CPU being offlined.  The intent of
the function is to reset MSRs local to that CPU.

Move the call into the dying callback which is actually run on the local
CPU.

Cc: stable@...r.kernel.org # 6.1+
Fixes: ca5b7c0d9621 ("perf/x86/amd/lbr: Add LbrExtV2 branch record support")
Suggested-by: Sandipan Das <sandipan.das@....com>
Signed-off-by: Mario Limonciello <mario.limonciello@....com>
---
v1->v2:
 * Add more of trace
 * Explain root cause better
 * Adjust solution to fix real root cause
---
 arch/x86/events/amd/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/events/amd/core.c b/arch/x86/events/amd/core.c
index e24976593a29..4ec6d3ece07d 100644
--- a/arch/x86/events/amd/core.c
+++ b/arch/x86/events/amd/core.c
@@ -598,13 +598,17 @@ static void amd_pmu_cpu_starting(int cpu)
 	cpuc->amd_nb->refcnt++;
 }
 
+static void amd_pmu_cpu_dying(int cpu)
+{
+	amd_pmu_cpu_reset(cpu);
+}
+
 static void amd_pmu_cpu_dead(int cpu)
 {
 	struct cpu_hw_events *cpuhw = &per_cpu(cpu_hw_events, cpu);
 
 	kfree(cpuhw->lbr_sel);
 	cpuhw->lbr_sel = NULL;
-	amd_pmu_cpu_reset(cpu);
 
 	if (!x86_pmu.amd_nb_constraints)
 		return;
@@ -1270,6 +1274,7 @@ static __initconst const struct x86_pmu amd_pmu = {
 
 	.cpu_prepare		= amd_pmu_cpu_prepare,
 	.cpu_starting		= amd_pmu_cpu_starting,
+	.cpu_dying		= amd_pmu_cpu_dying,
 	.cpu_dead		= amd_pmu_cpu_dead,
 
 	.amd_nb_constraints	= 1,
-- 
2.34.1