Thread (36 messages) flat view 36 messages, 2 authors, 20h ago

Re: [PATCH v14 05/14] x86/hw_breakpoints: Add arch_modify_local_hw_breakpoint_addr() API

From: sashiko-bot@kernel.org
Date: 2026-08-30 14:38:49
Also in: linux-perf-users

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Modifying a debug address register without first disabling the corresponding breakpoint in DR7 violates the x86 hardware architecture specification and causes unpredictable behavior.
--

commit 35b07811f48c348c13ac1d9de7fa44d25fc7f36a
Author: Jinchao Wang [off-list ref]

x86/hw_breakpoints: Add arch_modify_local_hw_breakpoint_addr() API

This commit adds an architecture backend to modify the address of an active
per-CPU watchpoint without releasing its hardware slot. It validates the new
address, finds the installed local slot, and updates the hardware debug
register and per-CPU shadow state.

Link: https://lore.kernel.org/all/59637b96946653393a7ad3c7de094094796b39c2.1785067572.git.wangjinchao600@gmail.com/ (local)
quoted hunk ↗ jump to hunk
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 3204e75dc4546..2227918b0163c 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
[ ... ]
+int arch_modify_local_hw_breakpoint_addr(struct perf_event *bp,
+					 unsigned long addr)
+{
[ ... ]
+	do {
+		seq = this_cpu_inc_return(cpu_dr7_seq);
+		this_cpu_write(cpu_debugreg[i], addr);
+		barrier();
+		set_debugreg(addr, i);
[Severity: High]
Does this code risk violating the x86 hardware architecture specification?

Looking at arch_modify_local_hw_breakpoint_addr() in
arch/x86/kernel/hw_breakpoint.c, set_debugreg() directly updates the debug
address register (DR0-DR3) while its corresponding enable bit in DR7 might
still be set.

According to hardware specs, modifying a debug address register without first
disabling the breakpoint in DR7 can lead to spurious debug exceptions or
unpredictable processor behavior if an NMI occurs or if execution boundaries
trigger a match on the transient state.

Should the DR7 enable bit be cleared before updating the debug register?
+		barrier();
+	} while (seq != this_cpu_read(cpu_dr7_seq));
+
+	return 0;
+}
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178810001186.64882.2161016469449127450.stgit@devnote2?part=5
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help