Skip to content

Commit

Permalink
LoongArch: Fix watchpoint setting error
Browse files Browse the repository at this point in the history
In the current code, when debugging the following code using gdb,
"invalid argument ..." message will be displayed.

lihui@bogon:~$ cat test.c
  #include <stdio.h>
  int a = 0;
  int main()
  {
	a = 1;
	return 0;
  }
lihui@bogon:~$ gcc -g test.c -o test
lihui@bogon:~$ gdb test
...
(gdb) watch a
Hardware watchpoint 1: a
(gdb) r
...
Invalid argument setting hardware debug registers

There are mainly two types of issues.

1. Some incorrect judgment condition existed in user_watch_state
   argument parsing, causing -EINVAL to be returned.

When setting up a watchpoint, gdb uses the ptrace interface,
ptrace(PTRACE_SETREGSET, tid, NT_LOONGARCH_HW_WATCH, (void *) &iov)).
Register values in user_watch_state as follows:

  addr[0] = 0x0, mask[0] = 0x0, ctrl[0] = 0x0
  addr[1] = 0x0, mask[1] = 0x0, ctrl[1] = 0x0
  addr[2] = 0x0, mask[2] = 0x0, ctrl[2] = 0x0
  addr[3] = 0x0, mask[3] = 0x0, ctrl[3] = 0x0
  addr[4] = 0x0, mask[4] = 0x0, ctrl[4] = 0x0
  addr[5] = 0x0, mask[5] = 0x0, ctrl[5] = 0x0
  addr[6] = 0x0, mask[6] = 0x0, ctrl[6] = 0x0
  addr[7] = 0x12000803c, mask[7] = 0x0, ctrl[7] = 0x610

In arch_bp_generic_fields(), return -EINVAL when ctrl.len is
LOONGARCH_BREAKPOINT_LEN_8(0b00). So delete the incorrect judgment here.

In ptrace_hbp_fill_attr_ctrl(), when note_type is NT_LOONGARCH_HW_WATCH
and ctrl[0] == 0x0, if ((type & HW_BREAKPOINT_RW) != type) will return
-EINVAL. Here ctrl.type should be set based on note_type, and unnecessary
judgments can be removed.

2. The watchpoint argument was not set correctly due to unnecessary
   offset and alignment_mask.

Modify ptrace_hbp_fill_attr_ctrl() and hw_breakpoint_arch_parse(), which
ensure the watchpont argument is set correctly.

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

Cc: [email protected]
Signed-off-by: Hui Li <[email protected]>
Signed-off-by: Huacai Chen <[email protected]>
  • Loading branch information
lhloongson authored and chenhuacai committed Jun 21, 2024
1 parent 120dd41 commit f63a47b
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 32 deletions.
2 changes: 1 addition & 1 deletion arch/loongarch/include/asm/hw_breakpoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ struct perf_event;
struct perf_event_attr;

extern int arch_bp_generic_fields(struct arch_hw_breakpoint_ctrl ctrl,
int *gen_len, int *gen_type, int *offset);
int *gen_len, int *gen_type);
extern int arch_check_bp_in_kernelspace(struct arch_hw_breakpoint *hw);
extern int hw_breakpoint_arch_parse(struct perf_event *bp,
const struct perf_event_attr *attr,
Expand Down
19 changes: 5 additions & 14 deletions arch/loongarch/kernel/hw_breakpoint.c
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ int arch_check_bp_in_kernelspace(struct arch_hw_breakpoint *hw)
* to generic breakpoint descriptions.
*/
int arch_bp_generic_fields(struct arch_hw_breakpoint_ctrl ctrl,
int *gen_len, int *gen_type, int *offset)
int *gen_len, int *gen_type)
{
/* Type */
switch (ctrl.type) {
Expand All @@ -303,11 +303,6 @@ int arch_bp_generic_fields(struct arch_hw_breakpoint_ctrl ctrl,
return -EINVAL;
}

if (!ctrl.len)
return -EINVAL;

*offset = __ffs(ctrl.len);

/* Len */
switch (ctrl.len) {
case LOONGARCH_BREAKPOINT_LEN_1:
Expand Down Expand Up @@ -386,21 +381,17 @@ int hw_breakpoint_arch_parse(struct perf_event *bp,
struct arch_hw_breakpoint *hw)
{
int ret;
u64 alignment_mask, offset;
u64 alignment_mask;

/* Build the arch_hw_breakpoint. */
ret = arch_build_bp_info(bp, attr, hw);
if (ret)
return ret;

if (hw->ctrl.type != LOONGARCH_BREAKPOINT_EXECUTE)
alignment_mask = 0x7;
else
if (hw->ctrl.type == LOONGARCH_BREAKPOINT_EXECUTE) {
alignment_mask = 0x3;
offset = hw->address & alignment_mask;

hw->address &= ~alignment_mask;
hw->ctrl.len <<= offset;
hw->address &= ~alignment_mask;
}

return 0;
}
Expand Down
32 changes: 15 additions & 17 deletions arch/loongarch/kernel/ptrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -494,28 +494,14 @@ static int ptrace_hbp_fill_attr_ctrl(unsigned int note_type,
struct arch_hw_breakpoint_ctrl ctrl,
struct perf_event_attr *attr)
{
int err, len, type, offset;
int err, len, type;

err = arch_bp_generic_fields(ctrl, &len, &type, &offset);
err = arch_bp_generic_fields(ctrl, &len, &type);
if (err)
return err;

switch (note_type) {
case NT_LOONGARCH_HW_BREAK:
if ((type & HW_BREAKPOINT_X) != type)
return -EINVAL;
break;
case NT_LOONGARCH_HW_WATCH:
if ((type & HW_BREAKPOINT_RW) != type)
return -EINVAL;
break;
default:
return -EINVAL;
}

attr->bp_len = len;
attr->bp_type = type;
attr->bp_addr += offset;

return 0;
}
Expand Down Expand Up @@ -609,7 +595,19 @@ static int ptrace_hbp_set_ctrl(unsigned int note_type,
return PTR_ERR(bp);

attr = bp->attr;
decode_ctrl_reg(uctrl, &ctrl);

switch (note_type) {
case NT_LOONGARCH_HW_BREAK:
ctrl.type = LOONGARCH_BREAKPOINT_EXECUTE;
ctrl.len = LOONGARCH_BREAKPOINT_LEN_4;
break;
case NT_LOONGARCH_HW_WATCH:
decode_ctrl_reg(uctrl, &ctrl);
break;
default:
return -EINVAL;
}

err = ptrace_hbp_fill_attr_ctrl(note_type, ctrl, &attr);
if (err)
return err;
Expand Down

0 comments on commit f63a47b

Please sign in to comment.