[PATCH v7 6/8] riscv: Add software supported single stepping with mc/mc6 triggers
Jesse Taube
jtaubepe at redhat.com
Tue Oct 27 12:46:18 PDT 2026
On Wed, Sep 30, 2026 at 2:40 AM Himanshu Chauhan
<himanshu.chauhan at oss.qualcomm.com> wrote:
>
> With mc/mc6 triggers, after handling breakpoint at current pc, there
> is no way to tell trigger to skip same breakpoint after the handler
> returns. This can cause loop of breakpoints at the same address until
> the trigger is disabled or uninstalled. Debugger may keep the breakpoints
> enabled and if kernel disables it, there is a loss of state coherency.
>
> To avoid the loop, software disables the current breakpoint, finds the
> next instruction that would be executed after pc and puts a breakpoint
> at that address. When this breakpoint hits, old breakpoint is reinstalled.
>
> Signed-off-by: Himanshu Chauhan <himanshu.chauhan at oss.qualcomm.com>
> ---
> arch/riscv/include/asm/hw_breakpoint.h | 9 +
> arch/riscv/kernel/hw_breakpoint.c | 256 +++++++++++++++++++------
> 2 files changed, 202 insertions(+), 63 deletions(-)
>
> diff --git a/arch/riscv/include/asm/hw_breakpoint.h b/arch/riscv/include/asm/hw_breakpoint.h
> index ca2ef93c020c..93cbf39f4b4a 100644
> --- a/arch/riscv/include/asm/hw_breakpoint.h
> +++ b/arch/riscv/include/asm/hw_breakpoint.h
> @@ -280,6 +280,10 @@ struct arch_hw_breakpoint {
> unsigned long address;
> unsigned long len;
> unsigned int type;
> +
> + /* Single-step callback info */
> + unsigned long next_addr;
> + bool in_callback;
> unsigned int match;
> unsigned int chain;
> unsigned int select;
> @@ -289,6 +293,11 @@ struct arch_hw_breakpoint {
> unsigned long tdata1;
> unsigned long tdata2;
> unsigned long tdata3;
> +
> + /* Saved trigger config for single-step restore */
> + unsigned long saved_tdata1;
> + unsigned long saved_tdata2;
> + unsigned long saved_tdata3;
> };
>
> struct perf_event_attr;
> diff --git a/arch/riscv/kernel/hw_breakpoint.c b/arch/riscv/kernel/hw_breakpoint.c
> index dd6f63953969..9597ed362537 100644
> --- a/arch/riscv/kernel/hw_breakpoint.c
> +++ b/arch/riscv/kernel/hw_breakpoint.c
> @@ -11,9 +11,12 @@
> #include <linux/percpu.h>
> #include <linux/kdebug.h>
> #include <linux/bitops.h>
> +#include <linux/bitfield.h>
> +#include <linux/math.h>
> #include <linux/cpu.h>
> #include <linux/cpuhotplug.h>
>
> +#include <asm/insn.h>
> #include <asm/sbi.h>
>
> /* Registered per-cpu bp/wp */
> @@ -328,8 +331,11 @@ int hw_breakpoint_arch_parse(struct perf_event *bp,
>
> /* Breakpoint address */
> hw->address = attr->bp_addr;
> + hw->tdata1 = 0;
> hw->tdata2 = attr->bp_addr;
> hw->tdata3 = 0x0;
> + hw->next_addr = 0x0;
> + hw->in_callback = false;
>
> switch (dbtr_type) {
> case RISCV_DBTR_TRIG_MCONTROL:
> @@ -347,6 +353,93 @@ int hw_breakpoint_arch_parse(struct perf_event *bp,
> return ret;
> }
>
> +static ulong get_step_address(struct pt_regs *regs, ulong insn)
> +{
> + return get_next_insn_address(regs, insn, regs->epc);
> +}
> +
> +/*
> + * setup_singlestep - Set the breakpoint to next instruction after current breakpoint.
> + */
> +static int setup_singlestep(struct perf_event *event, struct pt_regs *regs)
> +{
> + struct arch_hw_breakpoint *bp = counter_arch_bp(event);
> + unsigned long insn, next_addr = 0;
> + int ret;
> + struct arch_hw_breakpoint tmp = {};
> +
> + /*
> + * Save the original trigger configuration so we can restore it
> + * after the single-step fires.
> + */
> + bp->saved_tdata1 = bp->tdata1;
> + bp->saved_tdata2 = bp->tdata2;
> + bp->saved_tdata3 = bp->tdata3;
drop saved_tdata* as they are not used.
`hw_breakpoint_arch_parse(NULL, &event->attr, bp)` restores the
origonal state instead.
Thanks,
Jesse Taube
> +
> + ret = get_insn_safe(regs, regs->epc, &insn);
> + if (ret < 0)
> + return ret;
> +
> + next_addr = get_step_address(regs, insn);
> +
> + /*
> + * Software path: update the trigger in-place to an execute
> + * breakpoint at next_addr. Build the tdata directly without
> + * calling hw_breakpoint_arch_parse() so that bp->len, bp->type
> + * and bp->address are not overwritten and remain valid for the
> + * handler's matching logic after restore.
> + */
> + tmp.tdata1 = 0;
> + tmp.tdata2 = next_addr;
> + tmp.tdata3 = 0;
> + switch (dbtr_type) {
> + case RISCV_DBTR_TRIG_MCONTROL6:
> + RISCV_DBTR_SET_MC6_EXEC_BIT(tmp.tdata1);
> + tmp.tdata1 = RISCV_DBTR_SET_MC6_SIZE(tmp.tdata1, 0);
> + tmp.tdata1 = RISCV_DBTR_SET_MC6_TYPE(tmp.tdata1,
> + RISCV_DBTR_TRIG_MCONTROL6);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC6_DMODE_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC6_TIMING_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC6_SELECT_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC6_ACTION_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC6_CHAIN_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC6_MATCH_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC6_M_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC6_VS_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC6_VU_BIT);
> + SET_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC6_S_BIT);
> + SET_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC6_U_BIT);
> + break;
> + case RISCV_DBTR_TRIG_MCONTROL:
> + RISCV_DBTR_SET_MC_EXEC_BIT(tmp.tdata1);
> + tmp.tdata1 = RISCV_DBTR_SET_MC_SIZELO(tmp.tdata1, 0);
> + tmp.tdata1 = RISCV_DBTR_SET_MC_SIZEHI(tmp.tdata1, 0);
> + tmp.tdata1 = RISCV_DBTR_SET_MC_TYPE(tmp.tdata1,
> + RISCV_DBTR_TRIG_MCONTROL);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC_DMODE_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC_TIMING_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC_SELECT_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC_ACTION_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC_CHAIN_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC_MATCH_BIT);
> + CLEAR_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC_M_BIT);
> + SET_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC_S_BIT);
> + SET_DBTR_BIT(tmp.tdata1, RISCV_DBTR_MC_U_BIT);
> + break;
> + default:
> + return -EOPNOTSUPP;
> + }
> +
> + bp->tdata1 = tmp.tdata1;
> + bp->tdata2 = next_addr;
> + bp->tdata3 = 0;
> + arch_update_hw_breakpoint(event);
> +
> + bp->in_callback = true;
> + bp->next_addr = next_addr;
> + return 0;
> +}
> +
> /*
> * Read mcontrol6's hit1:hit0 field for trigger @idx. If either bit is set,
> * clear it in hardware (the Debug spec requires the trigger user to do
> @@ -401,10 +494,10 @@ static unsigned int mc6_read_and_clear_hit(int idx)
> */
> static int hw_breakpoint_handler(struct die_args *args)
> {
> - int ret = NOTIFY_DONE;
> + int i, ret = 0, bp_ret = NOTIFY_DONE;
> + bool expecting_callback = false;
> struct arch_hw_breakpoint *bp;
> struct perf_event *event;
> - int i;
>
> for (i = 0; i < dbtr_total_num; i++) {
> event = this_cpu_read(pcpu_hw_bp_events[i]);
> @@ -412,85 +505,122 @@ static int hw_breakpoint_handler(struct die_args *args)
> continue;
>
> bp = counter_arch_bp(event);
> - switch (bp->type) {
> - /* Breakpoint */
> - case HW_BREAKPOINT_X:
> - {
> - bool hit = bp->address == args->regs->epc;
>
> - if (!hit && dbtr_type == RISCV_DBTR_TRIG_MCONTROL6)
> - hit = mc6_read_and_clear_hit(i) != RISCV_DBTR_MC6_HIT_FALSE;
> + if (bp->in_callback) {
> + expecting_callback = true;
> + if (args->regs->epc != bp->next_addr)
> + continue;
> +
> + arch_uninstall_hw_breakpoint(event);
>
> - if (hit) {
> - perf_bp_event(event, args->regs);
> - ret = NOTIFY_STOP;
> + /* Restore original breakpoint */
> + if (hw_breakpoint_arch_parse(NULL, &event->attr, bp))
> + goto exit;
> +
> + if (arch_install_hw_breakpoint(event))
> + goto exit;
> +
> + bp->in_callback = false;
> + bp_ret = NOTIFY_STOP;
> + goto exit;
> + }
> +
> + switch (event->attr.bp_type) {
> + /* Breakpoint */
> + case HW_BREAKPOINT_X:
> + {
> + bool hit = bp->address == args->regs->epc;
> +
> + if (!hit && dbtr_type == RISCV_DBTR_TRIG_MCONTROL6)
> + hit = mc6_read_and_clear_hit(i) != RISCV_DBTR_MC6_HIT_FALSE;
> +
> + if (hit) {
> + perf_bp_event(event, args->regs);
> + ret = setup_singlestep(event, args->regs);
> + if (ret < 0) {
> + pr_err("Single step setup failed: %d.\n", ret);
> + goto exit;
> + }
> + bp_ret = NOTIFY_STOP;
> + goto exit;
> + }
> }
> break;
> - }
>
> /* Watchpoint */
> case HW_BREAKPOINT_W:
> case HW_BREAKPOINT_R:
> case HW_BREAKPOINT_RW:
> - {
> - unsigned long stval = args->regs->badaddr;
> - unsigned long bp_start = bp->address;
> - unsigned long bp_len = bp->len ?: 1;
> - unsigned long bp_end = bp_start + bp_len - 1;
> - unsigned long stval_end = stval + sizeof(long) - 1;
> - bool hit = false;
> -
> - if (bp_end < bp_start)
> - bp_end = ~0UL;
> - if (stval_end < stval)
> - stval_end = ~0UL;
> -
> - /*
> - * Prefer tdata1.hit from SBI trigger readout whenever
> - * possible. Fall back to address-based matching if HIT
> - * isn't observed/supported.
> - */
> - if (dbtr_type == RISCV_DBTR_TRIG_MCONTROL) {
> - unsigned long tdata1;
> - struct sbiret sret;
> - union sbi_dbtr_shmem_entry *shmem;
> -
> - raw_spin_lock_irqsave(this_cpu_ptr(&ecall_lock),
> - *this_cpu_ptr(&ecall_lock_flags));
> - shmem = this_cpu_ptr(sbi_dbtr_shmem);
> - sret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIG_READ,
> - i, 1, 0, 0, 0, 0);
> - if (!sret.error) {
> - tdata1 = le_to_cpu(shmem->data.tdata1);
> - hit = !!(tdata1 & RISCV_DBTR_MC_HIT_BIT_MASK);
> + {
> + unsigned long stval = args->regs->badaddr;
> + unsigned long bp_start = bp->address;
> + unsigned long bp_len = bp->len ?: 1;
> + unsigned long bp_end = bp_start + bp_len - 1;
> + unsigned long stval_end = stval + sizeof(long) - 1;
> + bool hit = false;
> +
> + if (bp_end < bp_start)
> + bp_end = ~0UL;
> + if (stval_end < stval)
> + stval_end = ~0UL;
> +
> + /*
> + * Prefer tdata1.hit from SBI trigger readout whenever
> + * possible. Fall back to address-based matching if HIT
> + * isn't observed/supported.
> + */
> + if (dbtr_type == RISCV_DBTR_TRIG_MCONTROL) {
> + unsigned long tdata1;
> + struct sbiret sret;
> + union sbi_dbtr_shmem_entry *shmem;
> +
> + raw_spin_lock_irqsave(this_cpu_ptr(&ecall_lock),
> + *this_cpu_ptr(&ecall_lock_flags));
> + shmem = this_cpu_ptr(sbi_dbtr_shmem);
> + sret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIG_READ,
> + i, 1, 0, 0, 0, 0);
> + if (!sret.error) {
> + tdata1 = le_to_cpu(shmem->data.tdata1);
> + hit = !!(tdata1 & RISCV_DBTR_MC_HIT_BIT_MASK);
> + }
> + raw_spin_unlock_irqrestore(this_cpu_ptr(&ecall_lock),
> + *this_cpu_ptr(&ecall_lock_flags));
> + } else if (dbtr_type == RISCV_DBTR_TRIG_MCONTROL6) {
> + hit = mc6_read_and_clear_hit(i) != RISCV_DBTR_MC6_HIT_FALSE;
> }
> - raw_spin_unlock_irqrestore(this_cpu_ptr(&ecall_lock),
> - *this_cpu_ptr(&ecall_lock_flags));
> - } else if (dbtr_type == RISCV_DBTR_TRIG_MCONTROL6) {
> - hit = mc6_read_and_clear_hit(i) != RISCV_DBTR_MC6_HIT_FALSE;
> - }
>
> - /*
> - * Sdtrig may report STVAL as the lowest accessed
> - * address while the watchpoint can match a higher byte
> - * in the same access.
> - */
> - if (hit ||
> - (stval >= bp_start && stval <= bp_end) ||
> - (bp_start >= stval && bp_start <= stval_end)) {
> - perf_bp_event(event, args->regs);
> - ret = NOTIFY_STOP;
> + /*
> + * Sdtrig may report STVAL as the lowest accessed
> + * address while the watchpoint can match a higher byte
> + * in the same access.
> + */
> + if (hit ||
> + (stval >= bp_start && stval <= bp_end) ||
> + (bp_start >= stval && bp_start <= stval_end)) {
> + perf_bp_event(event, args->regs);
> + ret = setup_singlestep(event, args->regs);
> + if (ret < 0) {
> + pr_err("setup_singlestep failed %d.\n", ret);
> + goto exit;
> + }
> + bp_ret = NOTIFY_STOP;
> + goto exit;
> + }
> }
> break;
> - }
>
> default:
> - pr_warn("Unknown type: %u\n", bp->type);
> - break;
> + pr_warn("Unknown type: %u\n", event->attr.bp_type);
> + goto exit;
> }
> }
>
> - return ret;
> + if (expecting_callback) {
> + pr_err("in_callback set but epc (%lx) not at next_addr (%lx).\n",
> + args->regs->epc, bp->next_addr);
> + }
> +exit:
> + return bp_ret;
> }
>
> int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
> --
> 2.43.0
>
More information about the linux-riscv
mailing list