]> git.dujemihanovic.xyz Git - linux.git/commitdiff
LoongArch: Fix multiple hardware watchpoint issues
authorHui Li <lihui@loongson.cn>
Fri, 21 Jun 2024 02:18:40 +0000 (10:18 +0800)
committerHuacai Chen <chenhuacai@loongson.cn>
Fri, 21 Jun 2024 02:18:40 +0000 (10:18 +0800)
In the current code, if multiple hardware breakpoints/watchpoints in
a user-space thread, some of them will not be triggered.

When debugging the following code using gdb.

lihui@bogon:~$ cat test.c
  #include <stdio.h>
  int a = 0;
  int main()
  {
    printf("start test\n");
    a = 1;
    printf("a = %d\n", a);
    printf("end test\n");
    return 0;
  }
lihui@bogon:~$ gcc -g test.c -o test
lihui@bogon:~$ gdb test
...
(gdb) start
...
Temporary breakpoint 1, main () at test.c:5
5        printf("start test\n");
(gdb) watch a
Hardware watchpoint 2: a
(gdb) hbreak 8
Hardware assisted breakpoint 3 at 0x1200006ec: file test.c, line 8.
(gdb) c
Continuing.
start test
a = 1

Breakpoint 3, main () at test.c:8
8        printf("end test\n");
...

The first hardware watchpoint is not triggered, the root causes are:

1. In hw_breakpoint_control(), The FWPnCFG1.2.4/MWPnCFG1.2.4 register
   settings are not distinguished. They should be set based on hardware
   watchpoint functions (fetch or load/store operations).

2. In breakpoint_handler() and watchpoint_handler(), it doesn't identify
   which watchpoint is triggered. So, all watchpoint-related perf_event
   callbacks are called and siginfo is sent to the user space. This will
   cause user-space unable to determine which watchpoint is triggered.
   The kernel need to identity which watchpoint is triggered via MWPS/
   FWPS registers, and then call the corresponding perf event callbacks
   to report siginfo to the user-space.

Modify the relevant code to solve above issues.

All changes according to the LoongArch Reference Manual:
https://loongson.github.io/LoongArch-Documentation/LoongArch-Vol1-EN.html#control-and-status-registers-related-to-watchpoints

With this patch:

lihui@bogon:~$ gdb test
...
(gdb) start
...
Temporary breakpoint 1, main () at test.c:5
5        printf("start test\n");
(gdb) watch a
Hardware watchpoint 2: a
(gdb) hbreak 8
Hardware assisted breakpoint 3 at 0x1200006ec: file test.c, line 8.
(gdb) c
Continuing.
start test

Hardware watchpoint 2: a

Old value = 0
New value = 1
main () at test.c:7
7        printf("a = %d\n", a);
(gdb) c
Continuing.
a = 1

Breakpoint 3, main () at test.c:8
8        printf("end test\n");
(gdb) c
Continuing.
end test
[Inferior 1 (process 778) exited normally]

Cc: stable@vger.kernel.org
Signed-off-by: Hui Li <lihui@loongson.cn>
Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
arch/loongarch/kernel/hw_breakpoint.c

index e882df1f72db81ba758ca722c6957629ee7978e8..621ad7634df718c3c4681a50e94bb852683d6f40 100644 (file)
@@ -207,15 +207,15 @@ static int hw_breakpoint_control(struct perf_event *bp,
        switch (ops) {
        case HW_BREAKPOINT_INSTALL:
                /* Set the FWPnCFG/MWPnCFG 1~4 register. */
-               write_wb_reg(CSR_CFG_ADDR, i, 0, info->address);
-               write_wb_reg(CSR_CFG_ADDR, i, 1, info->address);
-               write_wb_reg(CSR_CFG_MASK, i, 0, info->mask);
-               write_wb_reg(CSR_CFG_MASK, i, 1, info->mask);
-               write_wb_reg(CSR_CFG_ASID, i, 0, 0);
-               write_wb_reg(CSR_CFG_ASID, i, 1, 0);
                if (info->ctrl.type == LOONGARCH_BREAKPOINT_EXECUTE) {
+                       write_wb_reg(CSR_CFG_ADDR, i, 0, info->address);
+                       write_wb_reg(CSR_CFG_MASK, i, 0, info->mask);
+                       write_wb_reg(CSR_CFG_ASID, i, 0, 0);
                        write_wb_reg(CSR_CFG_CTRL, i, 0, privilege);
                } else {
+                       write_wb_reg(CSR_CFG_ADDR, i, 1, info->address);
+                       write_wb_reg(CSR_CFG_MASK, i, 1, info->mask);
+                       write_wb_reg(CSR_CFG_ASID, i, 1, 0);
                        ctrl = encode_ctrl_reg(info->ctrl);
                        write_wb_reg(CSR_CFG_CTRL, i, 1, ctrl | privilege);
                }
@@ -226,14 +226,17 @@ static int hw_breakpoint_control(struct perf_event *bp,
                break;
        case HW_BREAKPOINT_UNINSTALL:
                /* Reset the FWPnCFG/MWPnCFG 1~4 register. */
-               write_wb_reg(CSR_CFG_ADDR, i, 0, 0);
-               write_wb_reg(CSR_CFG_ADDR, i, 1, 0);
-               write_wb_reg(CSR_CFG_MASK, i, 0, 0);
-               write_wb_reg(CSR_CFG_MASK, i, 1, 0);
-               write_wb_reg(CSR_CFG_CTRL, i, 0, 0);
-               write_wb_reg(CSR_CFG_CTRL, i, 1, 0);
-               write_wb_reg(CSR_CFG_ASID, i, 0, 0);
-               write_wb_reg(CSR_CFG_ASID, i, 1, 0);
+               if (info->ctrl.type == LOONGARCH_BREAKPOINT_EXECUTE) {
+                       write_wb_reg(CSR_CFG_ADDR, i, 0, 0);
+                       write_wb_reg(CSR_CFG_MASK, i, 0, 0);
+                       write_wb_reg(CSR_CFG_CTRL, i, 0, 0);
+                       write_wb_reg(CSR_CFG_ASID, i, 0, 0);
+               } else {
+                       write_wb_reg(CSR_CFG_ADDR, i, 1, 0);
+                       write_wb_reg(CSR_CFG_MASK, i, 1, 0);
+                       write_wb_reg(CSR_CFG_CTRL, i, 1, 0);
+                       write_wb_reg(CSR_CFG_ASID, i, 1, 0);
+               }
                if (bp->hw.target)
                        regs->csr_prmd &= ~CSR_PRMD_PWE;
                break;
@@ -476,12 +479,15 @@ void breakpoint_handler(struct pt_regs *regs)
        slots = this_cpu_ptr(bp_on_reg);
 
        for (i = 0; i < boot_cpu_data.watch_ireg_count; ++i) {
-               bp = slots[i];
-               if (bp == NULL)
-                       continue;
-               perf_bp_event(bp, regs);
+               if ((csr_read32(LOONGARCH_CSR_FWPS) & (0x1 << i))) {
+                       bp = slots[i];
+                       if (bp == NULL)
+                               continue;
+                       perf_bp_event(bp, regs);
+                       csr_write32(0x1 << i, LOONGARCH_CSR_FWPS);
+                       update_bp_registers(regs, 0, 0);
+               }
        }
-       update_bp_registers(regs, 0, 0);
 }
 NOKPROBE_SYMBOL(breakpoint_handler);
 
@@ -493,12 +499,15 @@ void watchpoint_handler(struct pt_regs *regs)
        slots = this_cpu_ptr(wp_on_reg);
 
        for (i = 0; i < boot_cpu_data.watch_dreg_count; ++i) {
-               wp = slots[i];
-               if (wp == NULL)
-                       continue;
-               perf_bp_event(wp, regs);
+               if ((csr_read32(LOONGARCH_CSR_MWPS) & (0x1 << i))) {
+                       wp = slots[i];
+                       if (wp == NULL)
+                               continue;
+                       perf_bp_event(wp, regs);
+                       csr_write32(0x1 << i, LOONGARCH_CSR_MWPS);
+                       update_bp_registers(regs, 0, 1);
+               }
        }
-       update_bp_registers(regs, 0, 1);
 }
 NOKPROBE_SYMBOL(watchpoint_handler);