When trying to implement the arm64 kdb bph command, i.e. using hardware breakpoints, the regs are set incorrectly.

Alex Ivy whu2gh at gmail.com
Tue Jan 2 03:00:43 PST 2024


Dear all:

Kernel v6.10 in qemu. When implementing the Aarch64 set_hw_breakpoint
in kgdb, i.e. the "bph" commands, codes from arch/x86/kernel/kgdb.c
were referenced.

However, after the bph commands, I found that the watchpoint's reg
DBGWVR0_EL1, instead of the breakpoints reg changed, but an incorrect
value 0xffff80000802e6d0, rather than 0xffff80000812564c
(jiffies_read).

Here are the kgdb inputs/outputs.

root at hobot:~# echo g > /proc/sysrq-trigger
[  152.935280] sysrq: DEBUG
Entering kdb (current=0xffff0003855749c0, pid 492) on processor 3 due
to Keyboard Entry
[3]kdb> bph jiffies_read
Instruction(Register) BP #0 at 0xffff80000812564c (jiffies_read)
    is enabled   addr at ffff80000812564c, hardtype=1 installed=0

After the bph commands, I found that the watchpoint's reg DBGWVR0_EL1,
instead of the watchpoint reg changed, but an incorrect value
0xffff80000802e6d0, rather than 0xffff80000812564c (jiffies_read).
DBGBVR0_EL1    0x0                 0
DBGBCR0_EL1    0x0                 0
DBGWVR0_EL1    0xffff80000802e6d0  -140737353947440
DBGWCR0_EL1    0x2033              8243

Are there any processes that I ignored?

The main changes I've made:
1. the breakinfo struct of hw_breakpoint, copied from x86's. X86
supports 4 hw_breakpoints, where AArch64, using armv8 as example, 6
hw_breakpoints regs and 4 watchpoint regs. This struct maybe not
approriate.
2. The "const struct kgdb_arch arch_kgdb_ops" . especially the
"kgdb_set_hw_break"
3. The "kgdb_arch_late", which is called in start_kernel, used for
pre-allocating hw breakpoint instructions and the corresponding
pevents.

The patch is listed as follows. If any better ways to post it, please inform me.

diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
index 0a795ed4b080..cb8e800103ec 100644
--- a/arch/arm64/kernel/kgdb.c
+++ b/arch/arm64/kernel/kgdb.c
@@ -20,6 +20,8 @@
 #include <asm/nmi.h>
 #include <asm/patching.h>
 #include <asm/traps.h>
+#include <linux/hw_breakpoint.h>
+#include <linux/kdb.h>

 struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
  { "x0", 8, offsetof(struct pt_regs, regs[0])},
@@ -102,6 +104,14 @@ struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
  { "fpcr", 4, -1 },
 };

+static struct hw_breakpoint {
+ unsigned enabled;
+ unsigned long addr;
+ int len;
+ int type;
+ struct perf_event * __percpu *pev;
+} breakinfo[HBP_NUM];
+
 char *dbg_get_reg(int regno, void *mem, struct pt_regs *regs)
 {
  if (regno >= DBG_MAX_REG_NUM || regno < 0)
@@ -334,7 +344,232 @@ void kgdb_arch_exit(void)
  unregister_die_notifier(&kgdb_notifier);
 }

-const struct kgdb_arch arch_kgdb_ops;
+// void hw_breakpoint_restore(void)
+// {
+// set_debugreg(__this_cpu_read(cpu_debugreg[0]), 0);
+// set_debugreg(__this_cpu_read(cpu_debugreg[1]), 1);
+// set_debugreg(__this_cpu_read(cpu_debugreg[2]), 2);
+// set_debugreg(__this_cpu_read(cpu_debugreg[3]), 3);
+// set_debugreg(DR6_RESERVED, 6);
+// set_debugreg(__this_cpu_read(cpu_dr7), 7);
+// }
+// EXPORT_SYMBOL_GPL(hw_breakpoint_restore);
+
+static int hw_break_release_slot(int breakno)
+{
+ struct perf_event **pevent;
+ int cpu;
+
+ for_each_online_cpu(cpu) {
+ pevent = per_cpu_ptr(breakinfo[breakno].pev, cpu);
+ if (dbg_release_bp_slot(*pevent))
+ /*
+ * The debugger is responsible for handing the retry on
+ * remove failure.
+ */
+ return -1;
+ }
+ return 0;
+}
+
+static int hw_break_reserve_slot(int breakno)
+{
+ int cpu;
+ int cnt = 0;
+ struct perf_event **pevent;
+
+ for_each_online_cpu(cpu) {
+ cnt++;
+ pevent = per_cpu_ptr(breakinfo[breakno].pev, cpu);
+ kdb_printf("pevent, %lx \n",(unsigned long)pevent);
+ kdb_printf("*pevent, %lx \n",(unsigned long)*pevent);
+ if (dbg_reserve_bp_slot(*pevent)) /*TODO failed place*/
+ goto fail;
+ }
+
+ return 0;
+
+fail:
+ for_each_online_cpu(cpu) {
+ cnt--;
+ if (!cnt)
+ break;
+ pevent = per_cpu_ptr(breakinfo[breakno].pev, cpu);
+ dbg_release_bp_slot(*pevent);
+ }
+ return -1;
+}
+
+static int
+kgdb_set_hw_break(unsigned long addr, int len, enum kgdb_bptype bptype)
+{
+ int i;
+
+ for (i = 0; i < HBP_NUM; i++)
+ if (!breakinfo[i].enabled)
+ break;
+ if (i == HBP_NUM)
+ return -1;
+
+ switch (bptype) {
+ case BP_HARDWARE_BREAKPOINT:
+ len = 1;
+ breakinfo[i].type = ARM_BREAKPOINT_EXECUTE;
+ break;
+ case BP_WRITE_WATCHPOINT:
+ breakinfo[i].type = ARM_BREAKPOINT_STORE;
+ break;
+ case BP_ACCESS_WATCHPOINT:
+ breakinfo[i].type = ARM_BREAKPOINT_LOAD | ARM_BREAKPOINT_STORE;
+ break;
+ default:
+ return -1;
+ }
+ switch (len) {
+ case 1:
+ breakinfo[i].len = ARM_BREAKPOINT_LEN_1;
+ break;
+ case 2:
+ breakinfo[i].len = ARM_BREAKPOINT_LEN_2;
+ break;
+ case 4:
+ breakinfo[i].len = ARM_BREAKPOINT_LEN_4;
+ break;
+ case 8:
+ breakinfo[i].len = ARM_BREAKPOINT_LEN_8;
+ break;
+ default:
+ return -1;
+ }
+ breakinfo[i].addr = addr;
+ if (hw_break_reserve_slot(i)) {
+ breakinfo[i].addr = 0;
+ return -1;
+ }
+ breakinfo[i].enabled = 1;
+
+ return 0;
+}
+
+static int
+kgdb_remove_hw_break(unsigned long addr, int len, enum kgdb_bptype bptype)
+{
+ int i;
+
+ for (i = 0; i < HBP_NUM; i++)
+ if (breakinfo[i].addr == addr && breakinfo[i].enabled)
+ break;
+ if (i == HBP_NUM)
+ return -1;
+
+ if (hw_break_release_slot(i)) {
+ printk(KERN_ERR "Cannot remove hw breakpoint at %lx\n", addr);
+ return -1;
+ }
+ breakinfo[i].enabled = 0;
+
+ return 0;
+}
+
+static void kgdb_remove_all_hw_break(void)
+{
+ int i;
+ int cpu = raw_smp_processor_id();
+ struct perf_event *bp;
+
+ for (i = 0; i < HBP_NUM; i++) {
+ if (!breakinfo[i].enabled)
+ continue;
+ bp = *per_cpu_ptr(breakinfo[i].pev, cpu);
+ if (!bp->attr.disabled) {
+ arch_uninstall_hw_breakpoint(bp);
+ bp->attr.disabled = 1;
+ continue;
+ }
+ else if (hw_break_release_slot(i))
+ printk(KERN_ERR "KGDB: hw bpt remove failed %lx\n",
+        breakinfo[i].addr);
+ breakinfo[i].enabled = 0;
+ }
+}
+
+/**
+ * kgdb_disable_hw_debug - Disable hardware debugging while we in kgdb.
+ * @regs: Current &struct pt_regs.
+ *
+ * This function will be called if the particular architecture must
+ * disable hardware debugging while it is processing gdb packets or
+ * handling exception.
+ */
+static void kgdb_disable_hw_debug(struct pt_regs *regs)
+{
+ int i;
+ int cpu = raw_smp_processor_id();
+ struct perf_event *bp;
+
+ /* Disable hardware debugging while we are in kgdb: */
+ // set_debugreg(0UL, 7); /*TODO arm64 has no set_debug reg*/
+ for (i = 0; i < HBP_NUM; i++) {
+ if (!breakinfo[i].enabled)
+ continue;
+ bp = *per_cpu_ptr(breakinfo[i].pev, cpu);
+ if (bp->attr.disabled == 1)
+ continue;
+ arch_uninstall_hw_breakpoint(bp);
+ bp->attr.disabled = 1;
+ }
+}
+
+static void kgdb_correct_hw_break(void)
+{
+ int breakno;
+
+ for (breakno = 0; breakno < HBP_NUM; breakno++) {
+ struct perf_event *bp;
+ struct arch_hw_breakpoint *info;
+ int val;
+ int cpu = raw_smp_processor_id();
+ if (!breakinfo[breakno].enabled)
+ continue;
+ bp = *per_cpu_ptr(breakinfo[breakno].pev, cpu);
+ info = counter_arch_bp(bp);
+ if (bp->attr.disabled != 1)
+ continue;
+ bp->attr.bp_addr = breakinfo[breakno].addr;
+ bp->attr.bp_len = breakinfo[breakno].len;
+ bp->attr.bp_type = breakinfo[breakno].type;
+ // info->address = breakinfo[breakno].addr;
+ // info->len = breakinfo[breakno].len;
+ // info->type = breakinfo[breakno].type;
+ val = arch_install_hw_breakpoint(bp);
+ if (!val)
+ bp->attr.disabled = 0;
+ }
+ // hw_breakpoint_restore(); /* TODO set hw_breakpoint_restore*/
+ // for (slots = this_cpu_ptr(bp_on_reg), i = 0; i < core_num_brps; ++i) {
+ // if (slots[i]) {
+ // hw_breakpoint_control(slots[i], HW_BREAKPOINT_RESTORE);
+ // } else {
+ // write_wb_reg(AARCH64_DBG_REG_BCR, i, 0UL);
+ // write_wb_reg(AARCH64_DBG_REG_BVR, i, 0UL);
+ // }
+ // }
+}
+
+const struct kgdb_arch arch_kgdb_ops = {
+ /* Breakpoint instruction: */
+#ifndef __ARMEB__
+ .gdb_bpt_instr = {0x00, 0x00, 0x20, 0xd4},
+#else /* ! __ARMEB__ */
+ .gdb_bpt_instr = {0xd4, 0x20, 0x00, 0x00},
+#endif
+ .flags = KGDB_HW_BREAKPOINT,
+ .set_hw_breakpoint = kgdb_set_hw_break,
+ .remove_hw_breakpoint = kgdb_remove_hw_break,
+ .disable_hw_break = kgdb_disable_hw_debug,
+ .remove_all_hw_break = kgdb_remove_all_hw_break,
+ .correct_hw_break = kgdb_correct_hw_break,
+};

 int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
 {
@@ -389,3 +624,48 @@ bool kgdb_arch_roundup_cpus(void)
  arm64_send_nmi(&mask);
  return true;
 }
+
+static void kgdb_hw_overflow_handler(struct perf_event *event,
+ struct perf_sample_data *data, struct pt_regs *regs)
+{
+
+}
+
+void kgdb_arch_late(void)
+{
+ int i, cpu;
+ struct perf_event_attr attr;
+ struct perf_event **pevent;
+
+ /*
+ * Pre-allocate the hw breakpoint instructions in the non-atomic
+ * portion of kgdb because this operation requires mutexs to
+ * complete.
+ */
+ hw_breakpoint_init(&attr);
+ attr.bp_addr = (unsigned long)kgdb_arch_init;
+ attr.bp_len = HW_BREAKPOINT_LEN_1;
+ attr.bp_type = HW_BREAKPOINT_W;
+ attr.disabled = 1;
+ for (i = 0; i < HBP_NUM; i++) {
+ if (breakinfo[i].pev)
+ continue;
+ breakinfo[i].pev = register_wide_hw_breakpoint(&attr, NULL, NULL);
+ if (IS_ERR((void * __force)breakinfo[i].pev)) {
+ printk(KERN_ERR "kgdb: Could not allocate hw"
+        "breakpoints\nDisabling the kernel debugger\n");
+ breakinfo[i].pev = NULL;
+ kgdb_arch_exit();
+ return;
+ }
+ for_each_online_cpu(cpu) {
+ pevent = per_cpu_ptr(breakinfo[i].pev, cpu);
+ pevent[0]->hw.sample_period = 1;
+ pevent[0]->overflow_handler = kgdb_hw_overflow_handler;
+ if (pevent[0]->destroy != NULL) {
+ pevent[0]->destroy = NULL;
+ release_bp_slot(*pevent);
+ }
+ }
+ }
+}
\ No newline at end of file

Best regards,
Alex



More information about the linux-arm-kernel mailing list