On 9/11/25 08:46, Masami Hiramatsu (Google) wrote:
Hi Jinchao,
On Wed, 10 Sep 2025 13:23:10 +0800
Jinchao Wang <[email protected]> wrote:
Introduce arch_reinstall_hw_breakpoint() to update hardware breakpoint
parameters (address, length, type) without freeing and reallocating the
debug register slot.
This allows atomic updates in contexts where memory allocation is not
permitted, such as kprobe handlers.
Signed-off-by: Jinchao Wang <[email protected]>
---
arch/x86/include/asm/hw_breakpoint.h | 1 +
arch/x86/kernel/hw_breakpoint.c | 50 ++++++++++++++++++++++++++++
2 files changed, 51 insertions(+)
diff --git a/arch/x86/include/asm/hw_breakpoint.h
b/arch/x86/include/asm/hw_breakpoint.h
index 0bc931cd0698..bb7c70ad22fe 100644
--- a/arch/x86/include/asm/hw_breakpoint.h
+++ b/arch/x86/include/asm/hw_breakpoint.h
@@ -59,6 +59,7 @@ extern int hw_breakpoint_exceptions_notify(struct
notifier_block *unused,
int arch_install_hw_breakpoint(struct perf_event *bp);
+int arch_reinstall_hw_breakpoint(struct perf_event *bp);
void arch_uninstall_hw_breakpoint(struct perf_event *bp);
void hw_breakpoint_pmu_read(struct perf_event *bp);
void hw_breakpoint_pmu_unthrottle(struct perf_event *bp);
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index b01644c949b2..89135229ed21 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -132,6 +132,56 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
return 0;
}
+/*
+ * Reinstall a hardware breakpoint on the current CPU.
+ *
+ * This function is used to re-establish a perf counter hardware breakpoint.
+ * It finds the debug address register slot previously allocated for the
+ * breakpoint and re-enables it by writing the address to the debug register
+ * and setting the corresponding bits in the debug control register (DR7).
+ *
+ * It is expected that the breakpoint's event context lock is already held
+ * and interrupts are disabled, ensuring atomicity and safety from other
+ * event handlers.
+ */
+int arch_reinstall_hw_breakpoint(struct perf_event *bp)
+{
+ struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+ unsigned long *dr7;
+ int i;
+
+ lockdep_assert_irqs_disabled();
+
+ for (i = 0; i < HBP_NUM; i++) {
+ struct perf_event **slot = this_cpu_ptr(&bp_per_reg[i]);
+
+ if (*slot == bp)
+ break;
+ }
+
+ if (WARN_ONCE(i == HBP_NUM, "Can't find a matching breakpoint slot"))
+ return -EINVAL;
+
+ set_debugreg(info->address, i);
+ __this_cpu_write(cpu_debugreg[i], info->address);
+
+ dr7 = this_cpu_ptr(&cpu_dr7);
+ *dr7 |= encode_dr7(i, info->len, info->type);
+
+ /*
+ * Ensure we first write cpu_dr7 before we set the DR7 register.
+ * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
+ */
+ barrier();
+
+ set_debugreg(*dr7, 7);
+ if (info->mask)
+ amd_set_dr_addr_mask(info->mask, i);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(arch_reinstall_hw_breakpoint);
Please do not expose the arch dependent symbol. Instead, you should
expose an arch independent wrapper.
Anyway, you also need to share the same code with arch_install_hw_breakpoint()
like below;
Thanks,
You are right. The arch-dependent symbol has been removed and the code
is shared
with arch_install_hw_breakpoint() in the next version of the patch.
https://lore.kernel.org/lkml/[email protected]
diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
index 89135229ed21..2f3c5406999e 100644
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -84,6 +84,28 @@ int decode_dr7(unsigned long dr7, int bpnum, unsigned *len,
unsigned *type)
return (dr7 >> (bpnum * DR_ENABLE_SIZE)) & 0x3;
}
+static void __arch_install_hw_breakpoint(struct perf_event *bp, int regno)
+{
+ struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+ unsigned long *dr7;
+
+ set_debugreg(info->address, regno);
+ __this_cpu_write(cpu_debugreg[i], info->address);
+
+ dr7 = this_cpu_ptr(&cpu_dr7);
+ *dr7 |= encode_dr7(i, info->len, info->type);
+
+ /*
+ * Ensure we first write cpu_dr7 before we set the DR7 register.
+ * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
+ */
+ barrier();
+
+ set_debugreg(*dr7, 7);
+ if (info->mask)
+ amd_set_dr_addr_mask(info->mask, i);
+}
+
/*
* Install a perf counter breakpoint.
*
@@ -95,8 +117,6 @@ int decode_dr7(unsigned long dr7, int bpnum, unsigned *len,
unsigned *type)
*/
int arch_install_hw_breakpoint(struct perf_event *bp)
{
- struct arch_hw_breakpoint *info = counter_arch_bp(bp);
- unsigned long *dr7;
int i;
lockdep_assert_irqs_disabled();
@@ -113,22 +133,7 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
if (WARN_ONCE(i == HBP_NUM, "Can't find any breakpoint slot"))
return -EBUSY;
- set_debugreg(info->address, i);
- __this_cpu_write(cpu_debugreg[i], info->address);
-
- dr7 = this_cpu_ptr(&cpu_dr7);
- *dr7 |= encode_dr7(i, info->len, info->type);
-
- /*
- * Ensure we first write cpu_dr7 before we set the DR7 register.
- * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
- */
- barrier();
-
- set_debugreg(*dr7, 7);
- if (info->mask)
- amd_set_dr_addr_mask(info->mask, i);
-
+ __arch_install_hw_breakpoint(bp, i);
return 0;
}
@@ -146,8 +151,6 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
*/
int arch_reinstall_hw_breakpoint(struct perf_event *bp)
{
- struct arch_hw_breakpoint *info = counter_arch_bp(bp);
- unsigned long *dr7;
int i;
lockdep_assert_irqs_disabled();
@@ -162,22 +165,7 @@ int arch_reinstall_hw_breakpoint(struct perf_event *bp)
if (WARN_ONCE(i == HBP_NUM, "Can't find a matching breakpoint slot"))
return -EINVAL;
- set_debugreg(info->address, i);
- __this_cpu_write(cpu_debugreg[i], info->address);
-
- dr7 = this_cpu_ptr(&cpu_dr7);
- *dr7 |= encode_dr7(i, info->len, info->type);
-
- /*
- * Ensure we first write cpu_dr7 before we set the DR7 register.
- * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
- */
- barrier();
-
- set_debugreg(*dr7, 7);
- if (info->mask)
- amd_set_dr_addr_mask(info->mask, i);
-
+ __arch_install_hw_breakpoint(bp, i);
return 0;
}
EXPORT_SYMBOL_GPL(arch_reinstall_hw_breakpoint);
--
Thanks,
Jinchao
