Linux Perf Users
 help / color / mirror / Atom feed
* [PATCH v7 0/8] riscv: Introduce support for hardware break/watchpoints
@ 2026-09-30  6:39 Himanshu Chauhan
  2026-09-30  6:39 ` [PATCH v7 1/8] " Himanshu Chauhan
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Himanshu Chauhan @ 2026-09-30  6:39 UTC (permalink / raw)
  To: linux-kernel, linux-riscv, linux-perf-users
  Cc: peterz, mingo, alex, aou, jtaubepe, palmer, pjw, qingfang.deng,
	shuah, thecharlesjenkins, cp0613, Himanshu Chauhan

This patchset adds support for hardware breakpoints and watchpoints in the
RISC-V architecture. The framework is built on top of the perf/ptrace subsystem
and the SBI debug trigger extension (Sdtrig).

Single stepping is ready with software support and mc6. Once, icount is fixed, the software
supported single stepping will be put under a Kconfig option. Until then its default.

Virtualization of debug triggers are pending. Patch set is ready but needs testing.

Changes from v6:
  - Rebased to v7.3-rc5
  - Added HIT0/1 bit check/clear in breakpoint handler with EPC/Bp address check
  - Removed dbtr_shmem null check as pointed by Qinfang Deng
  - Fixed MEM_HI/MEM_LO macros to work both on rv64/32
  - Changed unsigned long to __u64 from hwdebug state struct and dropped packing
  - Gated HAVE_MIXED_BREAKPOINT_REGS with PERF_EVENTS
  - Software base single stepping with mcontrol6 trigger type.
  - icount is still under development.
  - Moved and and reworked instruction decoding code to a common file.
  - perf test suite now supported.
	- Breakpoint overflow signal handler
	- Breakpoint overflow sampling
	- Breakpoint accounting (needs sudo)
	- Watchpoint
	  - Read Only Watchpoint
	  - Write Only Watchpoint
	  - Read / Write Watchpoint
	  - Modify Watchpoint
  - __test_function in "Breakpoint overflow signal handler" is name noinline
  - Fixed warnings from checkpatch.pl --strict

Changes from v5:
  - Rebased to v7.2-rc6
  - Simplified Macros in hw_breakpoint.h
  - Took care of the review comments
  - Added ptrace support for hardware break/watchpoints (new patch)
    - PTRACE_GETREGSET/SETREGSET via NT_RISCV_HW_BREAK / NT_RISCV_HW_WATCH
    - PTRACE_GETHBPREGS / SETHBPREGS for direct single-trigger access
    - HAVE_MIXED_BREAKPOINTS_REGS selected (break/watch share trigger pool)
    - flush_ptrace_hw_breakpoint / ptrace_hw_copy_thread wired up
  - Extended selftest to cover ptrace-based hw break/watchpoint paths (new patch)

Changes from v4:
  - Rebased to v7.2-rc4
  - Fixed rv32 build error
  - Added pr_fmt to print KBUILD_MODNAME
  - Changed type of shmem_pa to phys_addr_t
  - Use per_cpu_ptr_to_phys instead of __pa for per-cpu allocated memory
  - Print successful registration/unregistration message when no error
  - Added RISC-V DEBUGGING section in MAINTAINERS and added myself as maintainer
  - Fixed warnings from checkpatch.pl --strict run

Changes from v3:
  - Rebased to v7.1-rc3
  - For watchpoints, check tdata1.hit via SBI_EXT_DBTR_TRIG_READ and keep
    STVAL-based matching as fallback
  - Improved watchpoint matching when STVAL reports the lowest accessed address
    for wider memory accesses
  - Program execute breakpoints with SIZE=0 (match any size) to avoid misses
    with 16-bit/compressed instruction addresses
  - Updated selftest to avoid deadlock by replacing unbounded sem_wait() with
    sem_timedwait() timeout handling
  - Updated selftest breakpoint function so it cannot be inlined or optimized away

Changes from v2:
  - Rebased to v7.0-rc1
  - Fixed warnings from checkpatch.pl --strict run

Changes from v1:
  - The patch adding the SBI extension and function IDs is already merged; this
    series builds on top of that
  - Added breakpoint selftest in tools/testing/selftests/breakpoints/

Specifications:
~~~~~~~~~~~~~~
The SBI debug trigger extension is specified in Chapter 19 of the SBI
specification:
  https://github.com/riscv-non-isa/riscv-sbi-doc/releases/download/v3.0/riscv-sbi.pdf

The Sdtrig ISA is part of the RISC-V debug specification:
  https://github.com/riscv/riscv-debug-spec

How to use:
~~~~~~~~~~~
OpenSBI:
  https://github.com/riscv-software-src/opensbi.git

QEMU:
  https://github.com/qemu/qemu.git

Linux Kernel:
  Apply these patches on top of v7.3-rc5

How to test:
~~~~~~~~~~~
From the Linux kernel directory, first install the UAPI headers (required on a
fresh tree so the compiler can locate <asm/ptrace.h> and the new
NT_RISCV_HW_BREAK/WATCH definitions via KHDR_INCLUDES):

  make headers

Then build the selftest:

  make -C tools/testing/selftests/breakpoints/

This produces breakpoint_test_riscv under the same directory. Load it on the
target and run. Sample output:

  # /apps/breakpoint_test_riscv
  # [perf_event]: Breakpoint test passed!
  # [perf_event]: Watchpoint test passed!
  # [ptrace]: Breakpoint test passed!
  # ptrace(PTRACE_GETREGSET): Number of watchpoints: 2
  # ptrace(PTRACE_GETREGSet): addr: 0x82888 control: 0x8080
  # [ptrace]: Watchpoint test passed!
  # [hbpregs] breakpoint readback: addr=0x10472 type=4 len=4 ctrl=0
  # [hbpregs]: Breakpoint test passed!
  # [hbpregs] watchpoint readback: addr=0x82888 type=2 len=8 ctrl=0
  # [hbpregs]: Watchpoint test passed!

Perf Test Suite:
~~~~~~~~~~~~~~~
~ # /apps/perf test -v 16 17 18 19
 16: Breakpoint overflow signal handler           : Ok
 17: Breakpoint overflow sampling                 : Ok
 18: Breakpoint accounting                        : Ok
 19: Watchpoint                                   :
 19.1: Read Only Watchpoint                       : Ok
 19.2: Write Only Watchpoint                      : Ok
 19.3: Read / Write Watchpoint                    : Ok
 19.4: Modify Watchpoint                          : Ok

=== Test Summary ===
Passed main tests : 3
Passed subtests   : 4
Skipped tests     : 0
Failed tests      : 0

Himanshu Chauhan (8):
  riscv: Introduce support for hardware break/watchpoints
  riscv: Add breakpoint and watchpoint test for riscv
  riscv: ptrace support for hardware break/watchpoints
  selftests/breakpoints: extend riscv test for ptrace hw
    break/watchpoints
  RISC-V: Add fetch and decode helpers to a common file
  riscv: Add software supported single stepping with mc/mc6 triggers
  perf tests: add noinline to __test_function
  MAINTAINERS: Add entry for RISC-V Debugging

 MAINTAINERS                                   |  11 +
 arch/riscv/Kconfig                            |   3 +
 arch/riscv/include/asm/hw_breakpoint.h        | 331 +++++++
 arch/riscv/include/asm/insn.h                 |  15 +
 arch/riscv/include/asm/kdebug.h               |   3 +-
 arch/riscv/include/asm/processor.h            |  18 +
 arch/riscv/include/uapi/asm/ptrace.h          |  50 +
 arch/riscv/kernel/Makefile                    |   1 +
 arch/riscv/kernel/hw_breakpoint.c             | 879 ++++++++++++++++++
 arch/riscv/kernel/process.c                   |   5 +
 arch/riscv/kernel/ptrace.c                    | 507 ++++++++++
 arch/riscv/kernel/traps.c                     |   6 +
 arch/riscv/kernel/traps_misaligned.c          |  52 +-
 arch/riscv/lib/Makefile                       |   1 +
 arch/riscv/lib/insn.c                         | 263 ++++++
 include/uapi/linux/elf.h                      |   4 +
 tools/include/uapi/linux/elf.h                |   2 +
 tools/perf/tests/bp_signal.c                  |   2 +-
 tools/testing/selftests/breakpoints/Makefile  |   5 +
 .../breakpoints/breakpoint_test_riscv.c       | 769 +++++++++++++++
 20 files changed, 2874 insertions(+), 53 deletions(-)
 create mode 100644 arch/riscv/include/asm/hw_breakpoint.h
 create mode 100644 arch/riscv/kernel/hw_breakpoint.c
 create mode 100644 arch/riscv/lib/insn.c
 create mode 100644 tools/testing/selftests/breakpoints/breakpoint_test_riscv.c

-- 
2.43.0


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v7 1/8] riscv: Introduce support for hardware break/watchpoints
  2026-09-30  6:39 [PATCH v7 0/8] riscv: Introduce support for hardware break/watchpoints Himanshu Chauhan
@ 2026-09-30  6:39 ` Himanshu Chauhan
  2026-09-30  6:58   ` sashiko-bot
  2026-09-30  6:39 ` [PATCH v7 2/8] riscv: Add breakpoint and watchpoint test for riscv Himanshu Chauhan
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Himanshu Chauhan @ 2026-09-30  6:39 UTC (permalink / raw)
  To: linux-kernel, linux-riscv, linux-perf-users
  Cc: peterz, mingo, alex, aou, jtaubepe, palmer, pjw, qingfang.deng,
	shuah, thecharlesjenkins, cp0613, Himanshu Chauhan

RISC-V hardware breakpoint framework is built on top of perf subsystem
and uses SBI debug trigger extension to install/uninstall/update/enable/disable
hardware triggers as specified in Sdtrig ISA extension.

While handling breakpoints, trust the hardware record, (hit[0-1]) bits, when deciding
if a trigger fired, and by clearing it once it's been read.

Signed-off-by: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
---
 arch/riscv/Kconfig                     |   2 +
 arch/riscv/include/asm/hw_breakpoint.h | 312 +++++++++++
 arch/riscv/include/asm/kdebug.h        |   3 +-
 arch/riscv/kernel/Makefile             |   1 +
 arch/riscv/kernel/hw_breakpoint.c      | 746 +++++++++++++++++++++++++
 arch/riscv/kernel/traps.c              |   6 +
 6 files changed, 1069 insertions(+), 1 deletion(-)
 create mode 100644 arch/riscv/include/asm/hw_breakpoint.h
 create mode 100644 arch/riscv/kernel/hw_breakpoint.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index d6c2dbf8455c..17fa7d4f9729 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -174,6 +174,8 @@ config RISCV
 	select HAVE_FUNCTION_ARG_ACCESS_API
 	select HAVE_FUNCTION_ERROR_INJECTION
 	select HAVE_GCC_PLUGINS
+	select HAVE_GENERIC_VDSO if MMU
+	select HAVE_HW_BREAKPOINT if PERF_EVENTS
 	select HAVE_IRQ_TIME_ACCOUNTING
 	select HAVE_KERNEL_BZIP2 if !EFI_ZBOOT
 	select HAVE_KERNEL_GZIP if !EFI_ZBOOT
diff --git a/arch/riscv/include/asm/hw_breakpoint.h b/arch/riscv/include/asm/hw_breakpoint.h
new file mode 100644
index 000000000000..a165bb9636de
--- /dev/null
+++ b/arch/riscv/include/asm/hw_breakpoint.h
@@ -0,0 +1,312 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2026 Qualcomm Technologies, Inc.
+ */
+
+#ifndef __RISCV_HW_BREAKPOINT_H
+#define __RISCV_HW_BREAKPOINT_H
+
+struct task_struct;
+
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+
+#include <uapi/linux/hw_breakpoint.h>
+
+/* Maximum number of hardware breakpoints supported */
+#define RISCV_HW_BP_NUM_MAX 32
+
+#if __riscv_xlen == 64
+#define cpu_to_le cpu_to_le64
+#define le_to_cpu le64_to_cpu
+#elif __riscv_xlen == 32
+#define cpu_to_le cpu_to_le32
+#define le_to_cpu le32_to_cpu
+#else
+#error "Unexpected __riscv_xlen"
+#endif
+
+#define CLEAR_DBTR_BIT(_target, _bit)	((_target) &= ~BIT(_bit))
+#define SET_DBTR_BIT(_target, _bit)	((_target) |= BIT(_bit))
+
+#define RISCV_DBTR_EXEC		BIT(0)
+#define RISCV_DBTR_LOAD		BIT(1)
+#define RISCV_DBTR_STORE	BIT(2)
+#define RISCV_DBTR_LDST		(RISCV_DBTR_LOAD | RISCV_DBTR_STORE)
+
+enum {
+	RISCV_DBTR_TRIG_NONE = 0,
+	RISCV_DBTR_TRIG_LEGACY,
+	RISCV_DBTR_TRIG_MCONTROL,
+	RISCV_DBTR_TRIG_ICOUNT,
+	RISCV_DBTR_TRIG_ITRIGGER,
+	RISCV_DBTR_TRIG_ETRIGGER,
+	RISCV_DBTR_TRIG_MCONTROL6,
+};
+
+/* Trigger Data 1 */
+#define RISCV_DBTR_TDATA1_DATA_BIT	0
+#if __riscv_xlen == 64
+#define RISCV_DBTR_TDATA1_DMODE_BIT	59
+#define RISCV_DBTR_TDATA1_TYPE_BIT	60
+#elif __riscv_xlen == 32
+#define RISCV_DBTR_TDATA1_DMODE_BIT	27
+#define RISCV_DBTR_TDATA1_TYPE_BIT	28
+#else
+#error "Unknown __riscv_xlen"
+#endif
+
+#if __riscv_xlen == 64
+#define RISCV_DBTR_TDATA1_DATA_BIT_MASK		GENMASK(58, RISCV_DBTR_TDATA1_DATA_BIT)
+#elif __riscv_xlen == 32
+#define RISCV_DBTR_TDATA1_DATA_BIT_MASK		GENMASK(26, RISCV_DBTR_TDATA1_DATA_BIT)
+#else
+#error "Unknown __riscv_xlen"
+#endif
+#define RISCV_DBTR_TDATA1_DMODE_BIT_MASK	BIT(RISCV_DBTR_TDATA1_DMODE_BIT)
+#define RISCV_DBTR_TDATA1_TYPE_BIT_MASK		\
+	GENMASK(RISCV_DBTR_TDATA1_TYPE_BIT + 3, RISCV_DBTR_TDATA1_TYPE_BIT)
+
+/* MC - Match Control Type Register */
+#define RISCV_DBTR_MC_LOAD_BIT		0
+#define RISCV_DBTR_MC_STORE_BIT		1
+#define RISCV_DBTR_MC_EXEC_BIT		2
+#define RISCV_DBTR_MC_U_BIT		3
+#define RISCV_DBTR_MC_S_BIT		4
+#define RISCV_DBTR_MC_RES2_BIT		5
+#define RISCV_DBTR_MC_M_BIT		6
+#define RISCV_DBTR_MC_MATCH_BIT		7
+#define RISCV_DBTR_MC_CHAIN_BIT		11
+#define RISCV_DBTR_MC_ACTION_BIT	12
+#define RISCV_DBTR_MC_SIZELO_BIT	16
+#define RISCV_DBTR_MC_TIMING_BIT	18
+#define RISCV_DBTR_MC_SELECT_BIT	19
+#define RISCV_DBTR_MC_HIT_BIT		20
+#if __riscv_xlen >= 64
+#define RISCV_DBTR_MC_SIZEHI_BIT	21
+#endif
+#if __riscv_xlen == 64
+#define RISCV_DBTR_MC_MASKMAX_BIT	53
+#define RISCV_DBTR_MC_DMODE_BIT		59
+#define RISCV_DBTR_MC_TYPE_BIT		60
+#elif __riscv_xlen == 32
+#define RISCV_DBTR_MC_MASKMAX_BIT	21
+#define RISCV_DBTR_MC_DMODE_BIT		27
+#define RISCV_DBTR_MC_TYPE_BIT		28
+#else
+#error "Unknown riscv xlen"
+#endif
+
+#define RISCV_DBTR_MC_LOAD_BIT_MASK	BIT(RISCV_DBTR_MC_LOAD_BIT)
+#define RISCV_DBTR_MC_STORE_BIT_MASK	BIT(RISCV_DBTR_MC_STORE_BIT)
+#define RISCV_DBTR_MC_EXEC_BIT_MASK	BIT(RISCV_DBTR_MC_EXEC_BIT)
+#define RISCV_DBTR_MC_U_BIT_MASK	BIT(RISCV_DBTR_MC_U_BIT)
+#define RISCV_DBTR_MC_S_BIT_MASK	BIT(RISCV_DBTR_MC_S_BIT)
+#define RISCV_DBTR_MC_RES2_BIT_MASK	BIT(RISCV_DBTR_MC_RES2_BIT)
+#define RISCV_DBTR_MC_M_BIT_MASK	BIT(RISCV_DBTR_MC_M_BIT)
+#define RISCV_DBTR_MC_MATCH_BIT_MASK	\
+	GENMASK(RISCV_DBTR_MC_MATCH_BIT + 3, RISCV_DBTR_MC_MATCH_BIT)
+#define RISCV_DBTR_MC_CHAIN_BIT_MASK	BIT(RISCV_DBTR_MC_CHAIN_BIT)
+#define RISCV_DBTR_MC_ACTION_BIT_MASK	\
+	GENMASK(RISCV_DBTR_MC_ACTION_BIT + 3, RISCV_DBTR_MC_ACTION_BIT)
+#define RISCV_DBTR_MC_SIZELO_BIT_MASK	\
+	GENMASK(RISCV_DBTR_MC_SIZELO_BIT + 1, RISCV_DBTR_MC_SIZELO_BIT)
+#define RISCV_DBTR_MC_TIMING_BIT_MASK	BIT(RISCV_DBTR_MC_TIMING_BIT)
+#define RISCV_DBTR_MC_SELECT_BIT_MASK	BIT(RISCV_DBTR_MC_SELECT_BIT)
+#define RISCV_DBTR_MC_HIT_BIT_MASK	BIT(RISCV_DBTR_MC_HIT_BIT)
+#if __riscv_xlen >= 64
+#define RISCV_DBTR_MC_SIZEHI_BIT_MASK	\
+	GENMASK(RISCV_DBTR_MC_SIZEHI_BIT + 1, RISCV_DBTR_MC_SIZEHI_BIT)
+#endif
+#define RISCV_DBTR_MC_MASKMAX_BIT_MASK	\
+	GENMASK(RISCV_DBTR_MC_MASKMAX_BIT + 5, RISCV_DBTR_MC_MASKMAX_BIT)
+#define RISCV_DBTR_MC_DMODE_BIT_MASK	BIT(RISCV_DBTR_MC_DMODE_BIT)
+#define RISCV_DBTR_MC_TYPE_BIT_MASK	GENMASK(RISCV_DBTR_MC_TYPE_BIT + 3, RISCV_DBTR_MC_TYPE_BIT)
+
+/* MC6 - Match Control 6 Type Register */
+#define RISCV_DBTR_MC6_LOAD_BIT		0
+#define RISCV_DBTR_MC6_STORE_BIT	1
+#define RISCV_DBTR_MC6_EXEC_BIT		2
+#define RISCV_DBTR_MC6_U_BIT		3
+#define RISCV_DBTR_MC6_S_BIT		4
+#define RISCV_DBTR_MC6_RES2_BIT		5
+#define RISCV_DBTR_MC6_M_BIT		6
+#define RISCV_DBTR_MC6_MATCH_BIT	7
+#define RISCV_DBTR_MC6_CHAIN_BIT	11
+#define RISCV_DBTR_MC6_ACTION_BIT	12
+#define RISCV_DBTR_MC6_SIZE_BIT		16
+#define RISCV_DBTR_MC6_TIMING_BIT	20
+#define RISCV_DBTR_MC6_SELECT_BIT	21
+#define RISCV_DBTR_MC6_HIT_BIT		22
+#define RISCV_DBTR_MC6_VU_BIT		23
+#define RISCV_DBTR_MC6_VS_BIT		24
+#if __riscv_xlen == 64
+#define RISCV_DBTR_MC6_DMODE_BIT	59
+#define RISCV_DBTR_MC6_TYPE_BIT		60
+#elif __riscv_xlen == 32
+#define RISCV_DBTR_MC6_DMODE_BIT	27
+#define RISCV_DBTR_MC6_TYPE_BIT		28
+#else
+#error "Unknown riscv xlen"
+#endif
+
+#define RISCV_DBTR_MC6_LOAD_BIT_MASK	BIT(RISCV_DBTR_MC6_LOAD_BIT)
+#define RISCV_DBTR_MC6_STORE_BIT_MASK	BIT(RISCV_DBTR_MC6_STORE_BIT)
+#define RISCV_DBTR_MC6_EXEC_BIT_MASK	BIT(RISCV_DBTR_MC6_EXEC_BIT)
+#define RISCV_DBTR_MC6_U_BIT_MASK	BIT(RISCV_DBTR_MC6_U_BIT)
+#define RISCV_DBTR_MC6_S_BIT_MASK	BIT(RISCV_DBTR_MC6_S_BIT)
+#define RISCV_DBTR_MC6_RES2_BIT_MASK	BIT(RISCV_DBTR_MC6_RES2_BIT)
+#define RISCV_DBTR_MC6_M_BIT_MASK	BIT(RISCV_DBTR_MC6_M_BIT)
+#define RISCV_DBTR_MC6_MATCH_BIT_MASK	\
+	GENMASK(RISCV_DBTR_MC6_MATCH_BIT + 3, RISCV_DBTR_MC6_MATCH_BIT)
+#define RISCV_DBTR_MC6_CHAIN_BIT_MASK	BIT(RISCV_DBTR_MC6_CHAIN_BIT)
+#define RISCV_DBTR_MC6_ACTION_BIT_MASK	\
+	GENMASK(RISCV_DBTR_MC6_ACTION_BIT + 3, RISCV_DBTR_MC6_ACTION_BIT)
+#define RISCV_DBTR_MC6_SIZE_BIT_MASK	\
+	GENMASK(RISCV_DBTR_MC6_SIZE_BIT + 3, RISCV_DBTR_MC6_SIZE_BIT)
+#define RISCV_DBTR_MC6_TIMING_BIT_MASK	BIT(RISCV_DBTR_MC6_TIMING_BIT)
+#define RISCV_DBTR_MC6_SELECT_BIT_MASK	BIT(RISCV_DBTR_MC6_SELECT_BIT)
+#define RISCV_DBTR_MC6_HIT_BIT_MASK	BIT(RISCV_DBTR_MC6_HIT_BIT)
+#define RISCV_DBTR_MC6_VU_BIT_MASK	BIT(RISCV_DBTR_MC6_VU_BIT)
+#define RISCV_DBTR_MC6_VS_BIT_MASK	BIT(RISCV_DBTR_MC6_VS_BIT)
+#define RISCV_DBTR_MC6_DMODE_BIT_MASK	BIT(RISCV_DBTR_MC6_DMODE_BIT)
+#define RISCV_DBTR_MC6_TYPE_BIT_MASK	\
+	GENMASK(RISCV_DBTR_MC6_TYPE_BIT + 3, RISCV_DBTR_MC6_TYPE_BIT)
+
+/*
+ * mcontrol6 hit1:hit0 (ratified Debug spec v1.0). RISCV_DBTR_MC6_HIT_BIT
+ * above is hit0 (bit 22); hit1 (bit 25) is not adjacent to it but combines
+ * with it into a single 2-bit field, hit1 as MSB.
+ */
+#define RISCV_DBTR_MC6_HIT1_BIT		25
+#define RISCV_DBTR_MC6_HIT1_BIT_MASK		BIT(RISCV_DBTR_MC6_HIT1_BIT)
+
+#define RISCV_DBTR_MC6_HIT_FALSE	0	/* trigger did not fire */
+#define RISCV_DBTR_MC6_HIT_BEFORE	1	/* fired before the instruction */
+#define RISCV_DBTR_MC6_HIT_AFTER	2	/* fired after, epc has moved past */
+#define RISCV_DBTR_MC6_HIT_IMM_AFTER	3	/* fired immediately after */
+
+#define RISCV_DBTR_MC6_GET_HIT(_t1)					\
+	({								\
+		typeof(_t1) __t1 = (_t1);				\
+		(((__t1) & RISCV_DBTR_MC6_HIT1_BIT_MASK) ? 2 : 0) |	\
+			(((__t1) & RISCV_DBTR_MC6_HIT_BIT_MASK) ? 1 : 0); \
+	})
+
+#define RISCV_DBTR_MC6_CLEAR_HIT(_t1)					\
+	((_t1) &= ~(RISCV_DBTR_MC6_HIT1_BIT_MASK | RISCV_DBTR_MC6_HIT_BIT_MASK))
+
+#define RISCV_DBTR_SET_TDATA1_TYPE(_t1, _type)				\
+	({								\
+		typeof(_t1) (td1t1) = (_t1);				\
+		(td1t1) &= ~RISCV_DBTR_TDATA1_TYPE_BIT_MASK;		\
+		(td1t1) |= (((unsigned long)(_type)			\
+			     << RISCV_DBTR_TDATA1_TYPE_BIT)		\
+			    & RISCV_DBTR_TDATA1_TYPE_BIT_MASK);		\
+		(td1t1);						\
+	})
+
+#define RISCV_DBTR_SET_MC_TYPE(_t1, _type)				\
+	({								\
+		typeof(_t1) (mct1) = (_t1);				\
+		(mct1) &= ~RISCV_DBTR_MC_TYPE_BIT_MASK;			\
+		(mct1) |= (((unsigned long)(_type)			\
+			    << RISCV_DBTR_MC_TYPE_BIT)			\
+			   & RISCV_DBTR_MC_TYPE_BIT_MASK);		\
+		(mct1);							\
+	})
+
+#define RISCV_DBTR_SET_MC6_TYPE(_t1, _type)				\
+	({								\
+		typeof(_t1) (mc6t1) = (_t1);				\
+		(mc6t1) &= ~RISCV_DBTR_MC6_TYPE_BIT_MASK;		\
+		(mc6t1) |= (((unsigned long)(_type)			\
+			     << RISCV_DBTR_MC6_TYPE_BIT)		\
+			    & RISCV_DBTR_MC6_TYPE_BIT_MASK);		\
+		(mc6t1);						\
+	})
+
+#define RISCV_DBTR_SET_MC_EXEC_BIT(_t1)			\
+	SET_DBTR_BIT(_t1, RISCV_DBTR_MC_EXEC_BIT)
+
+#define RISCV_DBTR_SET_MC_LOAD_BIT(_t1)			\
+	SET_DBTR_BIT(_t1, RISCV_DBTR_MC_LOAD_BIT)
+
+#define RISCV_DBTR_SET_MC_STORE_BIT(_t1)		\
+	SET_DBTR_BIT(_t1, RISCV_DBTR_MC_STORE_BIT)
+
+#define RISCV_DBTR_SET_MC_SIZELO(_t1, _val)				\
+	({								\
+		typeof(_t1) (mcslt1) = (_t1);				\
+		mcslt1 &= ~RISCV_DBTR_MC_SIZELO_BIT_MASK;		\
+		mcslt1 |= (((_val) << RISCV_DBTR_MC_SIZELO_BIT)		\
+			   & RISCV_DBTR_MC_SIZELO_BIT_MASK);		\
+		(mcslt1);						\
+	})
+
+#if __riscv_xlen >= 64
+#define RISCV_DBTR_SET_MC_SIZEHI(_t1, _val)				\
+	({								\
+		typeof(_t1) (mcsht1) = (_t1);				\
+		mcsht1 &= ~RISCV_DBTR_MC_SIZEHI_BIT_MASK;		\
+		mcsht1 |= (((_val) << RISCV_DBTR_MC_SIZEHI_BIT)		\
+			   & RISCV_DBTR_MC_SIZEHI_BIT_MASK);		\
+		(mcsht1);						\
+	})
+#else
+/* SIZEHI does not exist in the rv32 mcontrol layout; nothing to set. */
+#define RISCV_DBTR_SET_MC_SIZEHI(_t1, _val) ((void)(_val), (_t1))
+#endif
+
+#define RISCV_DBTR_SET_MC6_EXEC_BIT(_t1)		\
+	SET_DBTR_BIT(_t1, RISCV_DBTR_MC6_EXEC_BIT)
+
+#define RISCV_DBTR_SET_MC6_LOAD_BIT(_t1)		\
+	SET_DBTR_BIT(_t1, RISCV_DBTR_MC6_LOAD_BIT)
+
+#define RISCV_DBTR_SET_MC6_STORE_BIT(_t1)		\
+	SET_DBTR_BIT(_t1, RISCV_DBTR_MC6_STORE_BIT)
+
+#define RISCV_DBTR_SET_MC6_SIZE(_t1, _val)				\
+	({								\
+		typeof(_t1) (mc6szt1) = (_t1);				\
+		(mc6szt1) &= ~RISCV_DBTR_MC6_SIZE_BIT_MASK;		\
+		(mc6szt1) |= (((_val) << RISCV_DBTR_MC6_SIZE_BIT)	\
+			      & RISCV_DBTR_MC6_SIZE_BIT_MASK);		\
+		(mc6szt1);						\
+	})
+
+struct arch_hw_breakpoint {
+	unsigned long address;
+	unsigned long len;
+	unsigned int type;
+
+	/* Trigger configuration data */
+	unsigned long tdata1;
+	unsigned long tdata2;
+	unsigned long tdata3;
+};
+
+struct perf_event_attr;
+struct notifier_block;
+struct perf_event;
+struct pt_regs;
+
+int hw_breakpoint_slots(int type);
+int arch_check_bp_in_kernelspace(struct arch_hw_breakpoint *hw);
+int hw_breakpoint_arch_parse(struct perf_event *bp,
+			     const struct perf_event_attr *attr,
+			     struct arch_hw_breakpoint *hw);
+int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+				    unsigned long val, void *data);
+
+void arch_enable_hw_breakpoint(struct perf_event *bp);
+void arch_update_hw_breakpoint(struct perf_event *bp);
+void arch_disable_hw_breakpoint(struct perf_event *bp);
+int arch_install_hw_breakpoint(struct perf_event *bp);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void hw_breakpoint_pmu_read(struct perf_event *bp);
+
+#else
+
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+#endif /* __RISCV_HW_BREAKPOINT_H */
diff --git a/arch/riscv/include/asm/kdebug.h b/arch/riscv/include/asm/kdebug.h
index 85ac00411f6e..53e989781aa1 100644
--- a/arch/riscv/include/asm/kdebug.h
+++ b/arch/riscv/include/asm/kdebug.h
@@ -6,7 +6,8 @@
 enum die_val {
 	DIE_UNUSED,
 	DIE_TRAP,
-	DIE_OOPS
+	DIE_OOPS,
+	DIE_DEBUG
 };
 
 #endif
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index ebe1c3588177..a4197c3c36e8 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -100,6 +100,7 @@ obj-$(CONFIG_DYNAMIC_FTRACE)	+= mcount-dyn.o
 
 obj-$(CONFIG_PERF_EVENTS)	+= perf_callchain.o
 obj-$(CONFIG_HAVE_PERF_REGS)	+= perf_regs.o
+obj-$(CONFIG_HAVE_HW_BREAKPOINT)	+= hw_breakpoint.o
 obj-$(CONFIG_RISCV_SBI)		+= sbi.o sbi_ecall.o
 ifeq ($(CONFIG_RISCV_SBI), y)
 obj-$(CONFIG_SMP)		+= sbi-ipi.o
diff --git a/arch/riscv/kernel/hw_breakpoint.c b/arch/riscv/kernel/hw_breakpoint.c
new file mode 100644
index 000000000000..c2ab5e6008e7
--- /dev/null
+++ b/arch/riscv/kernel/hw_breakpoint.c
@@ -0,0 +1,746 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2026 Qualcomm Technologies, Inc.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/hw_breakpoint.h>
+#include <linux/perf_event.h>
+#include <linux/spinlock.h>
+#include <linux/percpu.h>
+#include <linux/kdebug.h>
+#include <linux/bitops.h>
+#include <linux/cpu.h>
+#include <linux/cpuhotplug.h>
+
+#include <asm/sbi.h>
+
+/* Registered per-cpu bp/wp */
+static DEFINE_PER_CPU(struct perf_event *, pcpu_hw_bp_events[RISCV_HW_BP_NUM_MAX]);
+static DEFINE_PER_CPU(unsigned long, ecall_lock_flags);
+static DEFINE_PER_CPU(raw_spinlock_t, ecall_lock);
+
+/* Per-cpu shared memory between S and M mode */
+static union sbi_dbtr_shmem_entry __percpu *sbi_dbtr_shmem;
+
+/* number of debug triggers on this cpu . */
+static int dbtr_total_num __ro_after_init;
+static int dbtr_type __ro_after_init;
+static int dbtr_init __ro_after_init;
+
+#define MEM_HI(_m)		(0UL)
+#define MEM_LO(_m)		((unsigned long)(_m))
+
+static int arch_smp_setup_sbi_shmem(unsigned int cpu)
+{
+	union sbi_dbtr_shmem_entry *dbtr_shmem;
+	phys_addr_t shmem_pa;
+	struct sbiret ret;
+
+	dbtr_shmem = per_cpu_ptr(sbi_dbtr_shmem, cpu);
+
+	shmem_pa = per_cpu_ptr_to_phys(dbtr_shmem);
+
+	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
+			MEM_LO(shmem_pa), MEM_HI(shmem_pa), 0, 0, 0, 0);
+
+	if (ret.error) {
+		pr_warn("%s: failed to setup shared memory. error: %ld\n",
+			__func__, ret.error);
+		return sbi_err_map_linux_errno(ret.error);
+	}
+
+	pr_info("CPU %d: HW Breakpoint shared memory registered.\n", cpu);
+
+	return 0;
+}
+
+static int arch_smp_teardown_sbi_shmem(unsigned int cpu)
+{
+	struct sbiret ret;
+
+	/* Disable shared memory */
+	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_SETUP_SHMEM,
+			SBI_SHMEM_DISABLE, SBI_SHMEM_DISABLE, 0, 0, 0, 0);
+
+	if (ret.error)
+		pr_warn("%s: failed to disable shared memory. error: %ld\n",
+			__func__, ret.error);
+	else
+		pr_info("CPU %d: HW Breakpoint shared memory disabled.\n", cpu);
+
+	return 0;
+}
+
+static void init_sbi_dbtr(void)
+{
+	unsigned long tdata1;
+	struct sbiret ret;
+
+	if (sbi_probe_extension(SBI_EXT_DBTR) <= 0) {
+		pr_warn("SBI_EXT_DBTR is not supported\n");
+		dbtr_total_num = 0;
+		goto done;
+	}
+
+	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_NUM_TRIGGERS,
+			0, 0, 0, 0, 0, 0);
+	if (ret.error) {
+		pr_warn("Failed to detect triggers\n");
+		dbtr_total_num = 0;
+		goto done;
+	}
+
+	tdata1 = 0;
+	tdata1 = RISCV_DBTR_SET_TDATA1_TYPE(tdata1, RISCV_DBTR_TRIG_MCONTROL6);
+
+	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_NUM_TRIGGERS,
+			tdata1, 0, 0, 0, 0, 0);
+	if (ret.error) {
+		pr_warn("Failed to detect mcontrol6 triggers\n");
+	} else if (!ret.value) {
+		pr_warn("Type 6 triggers not available\n");
+	} else {
+		dbtr_total_num = min_t(unsigned long, ret.value,
+				       RISCV_HW_BP_NUM_MAX);
+		dbtr_type = RISCV_DBTR_TRIG_MCONTROL6;
+		pr_warn("Mcontrol6 trigger available.\n");
+		goto done;
+	}
+
+	/* fallback to type 2 triggers if type 6 is not available */
+
+	tdata1 = 0;
+	tdata1 = RISCV_DBTR_SET_TDATA1_TYPE(tdata1, RISCV_DBTR_TRIG_MCONTROL);
+
+	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_NUM_TRIGGERS,
+			tdata1, 0, 0, 0, 0, 0);
+	if (ret.error) {
+		pr_warn("Failed to detect mcontrol triggers\n");
+	} else if (!ret.value) {
+		pr_warn("Type 2 triggers not available\n");
+	} else {
+		dbtr_total_num = min_t(unsigned long, ret.value,
+				       RISCV_HW_BP_NUM_MAX);
+		dbtr_type = RISCV_DBTR_TRIG_MCONTROL;
+		goto done;
+	}
+
+done:
+	dbtr_init = 1;
+}
+
+int hw_breakpoint_slots(int type)
+{
+	/*
+	 * We can be called early, so don't rely on
+	 * static variables being initialised.
+	 */
+
+	if (!dbtr_init)
+		init_sbi_dbtr();
+
+	return dbtr_total_num;
+}
+
+int arch_check_bp_in_kernelspace(struct arch_hw_breakpoint *hw)
+{
+	unsigned int len;
+	unsigned long va;
+
+	va = hw->address;
+	len = hw->len;
+
+	return (va >= TASK_SIZE) && ((va + len - 1) >= TASK_SIZE);
+}
+
+static int rv_init_mcontrol_trigger(const struct perf_event_attr *attr,
+				    struct arch_hw_breakpoint *hw)
+{
+	switch (attr->bp_type) {
+	case HW_BREAKPOINT_X:
+		hw->type = RISCV_DBTR_EXEC;
+		RISCV_DBTR_SET_MC_EXEC_BIT(hw->tdata1);
+		break;
+	case HW_BREAKPOINT_R:
+		hw->type = RISCV_DBTR_LOAD;
+		RISCV_DBTR_SET_MC_LOAD_BIT(hw->tdata1);
+		break;
+	case HW_BREAKPOINT_W:
+		hw->type = RISCV_DBTR_STORE;
+		RISCV_DBTR_SET_MC_STORE_BIT(hw->tdata1);
+		break;
+	case HW_BREAKPOINT_RW:
+		hw->type = RISCV_DBTR_LDST;
+		RISCV_DBTR_SET_MC_LOAD_BIT(hw->tdata1);
+		RISCV_DBTR_SET_MC_STORE_BIT(hw->tdata1);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (attr->bp_type == HW_BREAKPOINT_X) {
+		/*
+		 * Userspace debuggers can request execute breakpoints with
+		 * bp_len == 2 for compressed/non-aligned instruction
+		 * addresses. Program execute triggers with "match any size"
+		 * to avoid missing valid instruction fetches.
+		 */
+		hw->len = 0;
+		hw->tdata1 = RISCV_DBTR_SET_MC_SIZELO(hw->tdata1, 0);
+		hw->tdata1 = RISCV_DBTR_SET_MC_SIZEHI(hw->tdata1, 0);
+	} else {
+		switch (attr->bp_len) {
+		case HW_BREAKPOINT_LEN_1:
+			hw->len = 1;
+			hw->tdata1 = RISCV_DBTR_SET_MC_SIZELO(hw->tdata1, 1);
+			break;
+		case HW_BREAKPOINT_LEN_2:
+			hw->len = 2;
+			hw->tdata1 = RISCV_DBTR_SET_MC_SIZELO(hw->tdata1, 2);
+			break;
+		case HW_BREAKPOINT_LEN_4:
+			hw->len = 4;
+			hw->tdata1 = RISCV_DBTR_SET_MC_SIZELO(hw->tdata1, 3);
+			break;
+#if __riscv_xlen >= 64
+		case HW_BREAKPOINT_LEN_8:
+			hw->len = 8;
+			hw->tdata1 = RISCV_DBTR_SET_MC_SIZELO(hw->tdata1, 1);
+			hw->tdata1 = RISCV_DBTR_SET_MC_SIZEHI(hw->tdata1, 1);
+			break;
+#endif
+		/* Set to match any size */
+		default:
+			hw->len = 0;
+			hw->tdata1 = RISCV_DBTR_SET_MC_SIZELO(hw->tdata1, 0);
+			hw->tdata1 = RISCV_DBTR_SET_MC_SIZEHI(hw->tdata1, 0);
+			break;
+		}
+	}
+
+	hw->tdata1 = RISCV_DBTR_SET_MC_TYPE(hw->tdata1, RISCV_DBTR_TRIG_MCONTROL);
+
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC_DMODE_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC_TIMING_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC_SELECT_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC_ACTION_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC_CHAIN_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC_MATCH_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC_M_BIT);
+
+	/*
+	 * Match only the trigger's actual privilege-mode target. Setting
+	 * both bits makes the kernel's own S-mode uaccess (e.g. SUM-enabled
+	 * loads in strncpy_from_user()) spuriously fire a user-space watch.
+	 */
+	if (arch_check_bp_in_kernelspace(hw))
+		SET_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC_S_BIT);
+	else
+		SET_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC_U_BIT);
+
+	return 0;
+}
+
+static int rv_init_mcontrol6_trigger(const struct perf_event_attr *attr,
+				     struct arch_hw_breakpoint *hw)
+{
+	switch (attr->bp_type) {
+	case HW_BREAKPOINT_X:
+		hw->type = RISCV_DBTR_EXEC;
+		RISCV_DBTR_SET_MC6_EXEC_BIT(hw->tdata1);
+		break;
+	case HW_BREAKPOINT_R:
+		hw->type = RISCV_DBTR_LOAD;
+		RISCV_DBTR_SET_MC6_LOAD_BIT(hw->tdata1);
+		break;
+	case HW_BREAKPOINT_W:
+		hw->type = RISCV_DBTR_STORE;
+		RISCV_DBTR_SET_MC6_STORE_BIT(hw->tdata1);
+		break;
+	case HW_BREAKPOINT_RW:
+		hw->type = RISCV_DBTR_LDST;
+		RISCV_DBTR_SET_MC6_STORE_BIT(hw->tdata1);
+		RISCV_DBTR_SET_MC6_LOAD_BIT(hw->tdata1);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (attr->bp_type == HW_BREAKPOINT_X) {
+		/* See rv_init_mcontrol_trigger() for rationale. */
+		hw->len = 0;
+		hw->tdata1 = RISCV_DBTR_SET_MC6_SIZE(hw->tdata1, 0);
+	} else {
+		switch (attr->bp_len) {
+		case HW_BREAKPOINT_LEN_1:
+			hw->len = 1;
+			hw->tdata1 = RISCV_DBTR_SET_MC6_SIZE(hw->tdata1, 1);
+			break;
+		case HW_BREAKPOINT_LEN_2:
+			hw->len = 2;
+			hw->tdata1 = RISCV_DBTR_SET_MC6_SIZE(hw->tdata1, 2);
+			break;
+		case HW_BREAKPOINT_LEN_4:
+			hw->len = 4;
+			hw->tdata1 = RISCV_DBTR_SET_MC6_SIZE(hw->tdata1, 3);
+			break;
+#if __riscv_xlen >= 64
+		case HW_BREAKPOINT_LEN_8:
+			hw->len = 8;
+			hw->tdata1 = RISCV_DBTR_SET_MC6_SIZE(hw->tdata1, 5);
+			break;
+#endif
+		/* Set to match any size */
+		default:
+			hw->len = 0;
+			hw->tdata1 = RISCV_DBTR_SET_MC6_SIZE(hw->tdata1, 0);
+		}
+	}
+
+	hw->tdata1 = RISCV_DBTR_SET_MC6_TYPE(hw->tdata1, RISCV_DBTR_TRIG_MCONTROL6);
+
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC6_DMODE_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC6_TIMING_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC6_SELECT_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC6_ACTION_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC6_CHAIN_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC6_MATCH_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC6_M_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC6_VS_BIT);
+	CLEAR_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC6_VU_BIT);
+
+	/* See rv_init_mcontrol_trigger() for rationale. */
+	if (arch_check_bp_in_kernelspace(hw))
+		SET_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC6_S_BIT);
+	else
+		SET_DBTR_BIT(hw->tdata1, RISCV_DBTR_MC6_U_BIT);
+
+	return 0;
+}
+
+int hw_breakpoint_arch_parse(struct perf_event *bp,
+			     const struct perf_event_attr *attr,
+			     struct arch_hw_breakpoint *hw)
+{
+	int ret;
+
+	/* Breakpoint address */
+	hw->address = attr->bp_addr;
+	hw->tdata2 = attr->bp_addr;
+	hw->tdata3 = 0x0;
+
+	switch (dbtr_type) {
+	case RISCV_DBTR_TRIG_MCONTROL:
+		ret = rv_init_mcontrol_trigger(attr, hw);
+		break;
+	case RISCV_DBTR_TRIG_MCONTROL6:
+		ret = rv_init_mcontrol6_trigger(attr, hw);
+		break;
+	default:
+		pr_warn("Unsupported trigger type\n");
+		ret = -EOPNOTSUPP;
+		break;
+	}
+
+	return ret;
+}
+
+/*
+ * 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
+ * this; hardware only ever sets it). Returns the RISCV_DBTR_MC6_HIT_*
+ * value read (RISCV_DBTR_MC6_HIT_FALSE if unimplemented or unfired).
+ */
+static unsigned int mc6_read_and_clear_hit(int idx)
+{
+	union sbi_dbtr_shmem_entry *shmem = this_cpu_ptr(sbi_dbtr_shmem);
+	struct sbi_dbtr_data_msg *xmit;
+	unsigned long tdata1, tdata2, tdata3;
+	struct sbiret ret;
+	unsigned int hit;
+
+	raw_spin_lock_irqsave(this_cpu_ptr(&ecall_lock),
+			      *this_cpu_ptr(&ecall_lock_flags));
+
+	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIG_READ, idx, 1, 0, 0, 0, 0);
+	if (ret.error) {
+		pr_warn("Failed to read trigger %d. error: %ld\n", idx, ret.error);
+		hit = RISCV_DBTR_MC6_HIT_FALSE;
+		goto out;
+	}
+
+	tdata1 = le_to_cpu(shmem->data.tdata1);
+	hit = RISCV_DBTR_MC6_GET_HIT(tdata1);
+	if (hit == RISCV_DBTR_MC6_HIT_FALSE)
+		goto out;
+
+	tdata2 = le_to_cpu(shmem->data.tdata2);
+	tdata3 = le_to_cpu(shmem->data.tdata3);
+	RISCV_DBTR_MC6_CLEAR_HIT(tdata1);
+
+	xmit = &shmem->data;
+	xmit->tdata1 = cpu_to_le(tdata1);
+	xmit->tdata2 = cpu_to_le(tdata2);
+	xmit->tdata3 = cpu_to_le(tdata3);
+
+	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIG_UPDATE, idx, 1, 0, 0, 0, 0);
+	if (ret.error)
+		pr_warn("Failed to clear hit bits on trigger %d. error: %ld\n",
+			idx, ret.error);
+
+out:
+	raw_spin_unlock_irqrestore(this_cpu_ptr(&ecall_lock),
+				   *this_cpu_ptr(&ecall_lock_flags));
+	return hit;
+}
+
+/*
+ * HW Breakpoint/watchpoint handler
+ */
+static int hw_breakpoint_handler(struct die_args *args)
+{
+	int ret = NOTIFY_DONE;
+	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]);
+		if (!event)
+			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 (hit) {
+				perf_bp_event(event, args->regs);
+				ret = NOTIFY_STOP;
+			}
+			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);
+				}
+				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;
+			}
+			break;
+		}
+
+		default:
+			pr_warn("Unknown type: %u\n", bp->type);
+			break;
+		}
+	}
+
+	return ret;
+}
+
+int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+				    unsigned long val, void *data)
+{
+	if (val != DIE_DEBUG)
+		return NOTIFY_DONE;
+
+	return hw_breakpoint_handler(data);
+}
+
+/* atomic: counter->ctx->lock is held */
+int arch_install_hw_breakpoint(struct perf_event *event)
+{
+	struct arch_hw_breakpoint *bp = counter_arch_bp(event);
+	union sbi_dbtr_shmem_entry *shmem = this_cpu_ptr(sbi_dbtr_shmem);
+	struct sbi_dbtr_data_msg *xmit;
+	struct sbi_dbtr_id_msg *recv;
+	struct perf_event **slot;
+	unsigned long idx;
+	struct sbiret ret;
+	int err = 0;
+
+	raw_spin_lock_irqsave(this_cpu_ptr(&ecall_lock),
+			      *this_cpu_ptr(&ecall_lock_flags));
+
+	xmit = &shmem->data;
+	recv = &shmem->id;
+	xmit->tdata1 = cpu_to_le(bp->tdata1);
+	xmit->tdata2 = cpu_to_le(bp->tdata2);
+	xmit->tdata3 = cpu_to_le(bp->tdata3);
+
+	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIG_INSTALL,
+			1, 0, 0, 0, 0, 0);
+
+	if (ret.error) {
+		pr_warn("Failed to install trigger\n");
+		err = sbi_err_map_linux_errno(ret.error);
+		goto done;
+	}
+
+	idx = le_to_cpu(recv->idx);
+	if (idx >= dbtr_total_num) {
+		pr_warn("Invalid trigger index %lu\n", idx);
+		err = -EINVAL;
+		goto done;
+	}
+
+	slot = this_cpu_ptr(&pcpu_hw_bp_events[idx]);
+	if (*slot) {
+		pr_warn("Slot %lu is in use\n", idx);
+		err = -EBUSY;
+		goto done;
+	}
+
+	/* Save the event - to be looked up in handler */
+	*slot = event;
+
+done:
+	raw_spin_unlock_irqrestore(this_cpu_ptr(&ecall_lock),
+				   *this_cpu_ptr(&ecall_lock_flags));
+	return err;
+}
+
+/* atomic: counter->ctx->lock is held */
+void arch_uninstall_hw_breakpoint(struct perf_event *event)
+{
+	struct sbiret ret;
+	int i;
+
+	raw_spin_lock_irqsave(this_cpu_ptr(&ecall_lock),
+			      *this_cpu_ptr(&ecall_lock_flags));
+
+	for (i = 0; i < dbtr_total_num; i++) {
+		struct perf_event **slot = this_cpu_ptr(&pcpu_hw_bp_events[i]);
+
+		if (*slot == event) {
+			*slot = NULL;
+			break;
+		}
+	}
+
+	if (i == dbtr_total_num) {
+		pr_warn("Breakpoint not installed.\n");
+		goto out;
+	}
+
+	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIG_UNINSTALL,
+			i, 1, 0, 0, 0, 0);
+
+	if (ret.error) {
+		pr_warn("Failed to uninstall trigger %d.\n", i);
+		goto out;
+	}
+
+ out:
+	raw_spin_unlock_irqrestore(this_cpu_ptr(&ecall_lock),
+				   *this_cpu_ptr(&ecall_lock_flags));
+}
+
+void arch_enable_hw_breakpoint(struct perf_event *event)
+{
+	struct sbiret ret;
+	int i;
+	struct perf_event **slot;
+
+	raw_spin_lock_irqsave(this_cpu_ptr(&ecall_lock),
+			      *this_cpu_ptr(&ecall_lock_flags));
+
+	for (i = 0; i < dbtr_total_num; i++) {
+		slot = this_cpu_ptr(&pcpu_hw_bp_events[i]);
+
+		if (*slot == event)
+			break;
+	}
+
+	if (i == dbtr_total_num) {
+		pr_warn("Breakpoint not installed.\n");
+		goto out;
+	}
+
+	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIG_ENABLE,
+			i, 1, 0, 0, 0, 0);
+
+	if (ret.error) {
+		pr_warn("Failed to install trigger %d\n", i);
+		goto out;
+	}
+
+ out:
+	raw_spin_unlock_irqrestore(this_cpu_ptr(&ecall_lock),
+				   *this_cpu_ptr(&ecall_lock_flags));
+}
+EXPORT_SYMBOL_GPL(arch_enable_hw_breakpoint);
+
+void arch_update_hw_breakpoint(struct perf_event *event)
+{
+	struct arch_hw_breakpoint *bp = counter_arch_bp(event);
+	union sbi_dbtr_shmem_entry *shmem = this_cpu_ptr(sbi_dbtr_shmem);
+	struct sbi_dbtr_data_msg *xmit;
+	struct sbi_dbtr_id_msg *id;
+	struct perf_event **slot;
+	struct sbiret ret;
+	int i;
+
+	for (i = 0; i < dbtr_total_num; i++) {
+		slot = this_cpu_ptr(&pcpu_hw_bp_events[i]);
+
+		if (*slot == event)
+			break;
+	}
+
+	if (i == dbtr_total_num) {
+		pr_warn("Breakpoint not installed.\n");
+		return;
+	}
+
+	raw_spin_lock_irqsave(this_cpu_ptr(&ecall_lock),
+			      *this_cpu_ptr(&ecall_lock_flags));
+
+	id = &shmem->id;
+	xmit = &shmem->data;
+	id->idx = cpu_to_le(i);
+	xmit->tdata1 = cpu_to_le(bp->tdata1);
+	xmit->tdata2 = cpu_to_le(bp->tdata2);
+	xmit->tdata3 = cpu_to_le(bp->tdata3);
+
+	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIG_UPDATE,
+			1, 0, 0, 0, 0, 0);
+	if (ret.error)
+		pr_warn("Failed to update trigger %d.\n", i);
+
+	raw_spin_unlock_irqrestore(this_cpu_ptr(&ecall_lock),
+				   *this_cpu_ptr(&ecall_lock_flags));
+}
+EXPORT_SYMBOL_GPL(arch_update_hw_breakpoint);
+
+void arch_disable_hw_breakpoint(struct perf_event *event)
+{
+	struct perf_event **slot;
+	struct sbiret ret;
+	int i;
+
+	for (i = 0; i < dbtr_total_num; i++) {
+		slot = this_cpu_ptr(&pcpu_hw_bp_events[i]);
+
+		if (*slot == event)
+			break;
+	}
+
+	if (i == dbtr_total_num) {
+		pr_warn("Breakpoint not installed.\n");
+		return;
+	}
+
+	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIG_DISABLE,
+			i, 1, 0, 0, 0, 0);
+
+	if (ret.error) {
+		pr_warn("Failed to uninstall trigger %d.\n", i);
+		return;
+	}
+}
+EXPORT_SYMBOL_GPL(arch_disable_hw_breakpoint);
+
+void hw_breakpoint_pmu_read(struct perf_event *bp) { }
+
+void flush_ptrace_hw_breakpoint(struct task_struct *tsk) { }
+
+static int __init arch_hw_breakpoint_init(void)
+{
+	unsigned int cpu;
+	int rc = 0;
+
+	for_each_possible_cpu(cpu)
+		raw_spin_lock_init(&per_cpu(ecall_lock, cpu));
+
+	if (!dbtr_init)
+		init_sbi_dbtr();
+
+	if (dbtr_total_num) {
+		pr_info("Total number of type %d triggers: %u\n",
+			dbtr_type, dbtr_total_num);
+	} else {
+		pr_info("No hardware triggers available\n");
+		goto out;
+	}
+
+	/* Allocate per-cpu shared memory */
+	sbi_dbtr_shmem = __alloc_percpu(sizeof(*sbi_dbtr_shmem) * dbtr_total_num,
+					PAGE_SIZE);
+
+	if (!sbi_dbtr_shmem) {
+		pr_warn("Failed to allocate shared memory.\n");
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	/* Hotplug handler to register/unregister shared memory with SBI */
+	rc = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
+			       "riscv/hw_breakpoint:prepare",
+			       arch_smp_setup_sbi_shmem,
+			       arch_smp_teardown_sbi_shmem);
+
+	if (rc < 0) {
+		pr_warn("Failed to setup CPU hotplug state\n");
+		free_percpu(sbi_dbtr_shmem);
+		return rc;
+	}
+ out:
+	return rc;
+}
+arch_initcall(arch_hw_breakpoint_init);
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index f8292adda099..1b9ccc0b6b8d 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -287,6 +287,12 @@ void handle_break(struct pt_regs *regs)
 	if (probe_breakpoint_handler(regs))
 		return;
 
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	if (notify_die(DIE_DEBUG, "EBREAK", regs, 0, regs->cause, SIGTRAP)
+	    == NOTIFY_STOP)
+		return;
+#endif
+
 	current->thread.bad_cause = regs->cause;
 
 	if (user_mode(regs))
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 2/8] riscv: Add breakpoint and watchpoint test for riscv
  2026-09-30  6:39 [PATCH v7 0/8] riscv: Introduce support for hardware break/watchpoints Himanshu Chauhan
  2026-09-30  6:39 ` [PATCH v7 1/8] " Himanshu Chauhan
@ 2026-09-30  6:39 ` Himanshu Chauhan
  2026-09-30  6:49   ` sashiko-bot
  2026-09-30  6:39 ` [PATCH v7 3/8] riscv: ptrace support for hardware break/watchpoints Himanshu Chauhan
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Himanshu Chauhan @ 2026-09-30  6:39 UTC (permalink / raw)
  To: linux-kernel, linux-riscv, linux-perf-users
  Cc: peterz, mingo, alex, aou, jtaubepe, palmer, pjw, qingfang.deng,
	shuah, thecharlesjenkins, cp0613, Himanshu Chauhan

Add self test for riscv architecture. It uses ptrace to ptrace framework
to set/unset break/watchpoint and uses signals to check triggers.

Also add $(KHDR_INCLUDES) to CFLAGS for the riscv test build so the
UAPI headers exercised by the test can be located during compilation.

Signed-off-by: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
---
 tools/testing/selftests/breakpoints/Makefile  |   5 +
 .../breakpoints/breakpoint_test_riscv.c       | 219 ++++++++++++++++++
 2 files changed, 224 insertions(+)
 create mode 100644 tools/testing/selftests/breakpoints/breakpoint_test_riscv.c

diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
index 0b8f5acf7c78..b32c56b3c1db 100644
--- a/tools/testing/selftests/breakpoints/Makefile
+++ b/tools/testing/selftests/breakpoints/Makefile
@@ -12,5 +12,10 @@ ifneq (,$(filter $(ARCH),aarch64 arm64))
 TEST_GEN_PROGS += breakpoint_test_arm64
 endif
 
+ifneq (,$(filter $(ARCH),riscv))
+CFLAGS += -static $(KHDR_INCLUDES)
+TEST_GEN_PROGS += breakpoint_test_riscv
+endif
+
 include ../lib.mk
 
diff --git a/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c b/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c
new file mode 100644
index 000000000000..0649940b709e
--- /dev/null
+++ b/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c
@@ -0,0 +1,219 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2026 Qualcomm Technologies, Inc.
+ *
+ * Author: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
+ */
+
+#define _GNU_SOURCE
+#include <linux/perf_event.h>    /* Definition of PERF_* constants */
+#include <linux/hw_breakpoint.h> /* Definition of HW_* constants */
+#include <sys/syscall.h>         /* Definition of SYS_* constants */
+#include <unistd.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/ioctl.h>
+#include <time.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <sys/mman.h>
+#include <string.h>
+#include <semaphore.h>
+#include <errno.h>
+
+#ifndef noinline
+#define noinline __attribute__((noinline))
+#endif
+
+static int gfd;
+sem_t ib_mtx, wp_mtx;
+static int bp_triggered, wp_triggered;
+static int test_func_sink;
+static const int wait_timeout_sec = 5;
+
+int setup_bp(bool is_x, void *addr, int sig)
+{
+	struct perf_event_attr pe;
+	int fd;
+
+	memset(&pe, 0, sizeof(struct perf_event_attr));
+	pe.type = PERF_TYPE_BREAKPOINT;
+	pe.size = sizeof(struct perf_event_attr);
+
+	pe.config = 0;
+	pe.bp_type = is_x ? HW_BREAKPOINT_X : HW_BREAKPOINT_W;
+	pe.bp_addr = (unsigned long)addr;
+	pe.bp_len = sizeof(long);
+
+	pe.sample_period = 1;
+	pe.sample_type = PERF_SAMPLE_IP;
+	pe.wakeup_events = 1;
+
+	pe.disabled = 1;
+	pe.exclude_kernel = 1;
+	pe.exclude_hv = 1;
+
+	fd = syscall(SYS_perf_event_open, &pe, 0, -1, -1, 0);
+	if (fd < 0) {
+		printf("Failed to open event: %llx\n", pe.config);
+		return -1;
+	}
+
+	fcntl(fd, F_SETFL, O_RDWR | O_NONBLOCK | O_ASYNC);
+	fcntl(fd, F_SETSIG, sig);
+	fcntl(fd, F_SETOWN, getpid());
+
+	ioctl(fd, PERF_EVENT_IOC_RESET, 0);
+
+	return fd;
+}
+
+static void sig_handler_bp(int signum, siginfo_t *oh, void *uc)
+{
+	int ret;
+
+	bp_triggered++;
+
+	printf("Breakpoint triggered!\n");
+	ioctl(gfd, PERF_EVENT_IOC_DISABLE, 0);
+	ret = sem_post(&ib_mtx);
+	if (ret) {
+		printf("Failed to report BP success\n");
+		return;
+	}
+}
+
+static void sig_handler_wp(int signum, siginfo_t *oh, void *uc)
+{
+	int ret;
+
+	printf("Watchpoint triggered!\n");
+	ioctl(gfd, PERF_EVENT_IOC_DISABLE, 0);
+	wp_triggered++;
+
+	ret = sem_post(&wp_mtx);
+
+	if (ret) {
+		printf("Failed to report WP success\n");
+		return;
+	}
+}
+
+/*
+ * Keep a real instruction address for HW execute breakpoints: prevent inlining
+ * and force a visible side effect so the function can't be optimized away.
+ */
+static noinline void test_func(void)
+{
+	test_func_sink++;
+	__asm__ __volatile__("" : : "g" (test_func_sink));
+}
+
+static int trigger_bp(void)
+{
+	struct sigaction sa;
+
+	memset(&sa, 0, sizeof(struct sigaction));
+	sa.sa_sigaction = (void *)sig_handler_bp;
+	sa.sa_flags = SA_SIGINFO;
+
+	if (sigaction(SIGIO, &sa, NULL) < 0) {
+		printf("Failed to setup signal handler\n");
+		return -1;
+	}
+
+	gfd = setup_bp(1, test_func, SIGIO);
+
+	if (gfd < 0) {
+		printf("Failed to setup breakpoint.\n");
+		return -1;
+	}
+
+	ioctl(gfd, PERF_EVENT_IOC_ENABLE, 0);
+
+	test_func();
+
+	ioctl(gfd, PERF_EVENT_IOC_DISABLE, 0);
+
+	close(gfd);
+
+	return 0;
+}
+
+static int trigger_wp(void)
+{
+	struct sigaction sa;
+	unsigned long test_data;
+
+	memset(&sa, 0, sizeof(struct sigaction));
+	sa.sa_sigaction = (void *)sig_handler_wp;
+	sa.sa_flags = SA_SIGINFO;
+
+	if (sigaction(SIGUSR1, &sa, NULL) < 0) {
+		printf("Failed to setup signal handler\n");
+		return -1;
+	}
+
+	gfd = setup_bp(0, &test_data, SIGUSR1);
+
+	if (gfd < 0) {
+		printf("Failed to setup watchpoint\n");
+		return -1;
+	}
+
+	ioctl(gfd, PERF_EVENT_IOC_ENABLE, 0);
+	test_data = 0xdeadbeef;
+	ioctl(gfd, PERF_EVENT_IOC_DISABLE, 0);
+
+	return 0;
+}
+
+static int wait_event(sem_t *sem, const char *name)
+{
+	struct timespec ts;
+
+	if (clock_gettime(CLOCK_REALTIME, &ts)) {
+		printf("%s: Failed to get current time\n", name);
+		return -1;
+	}
+
+	/*
+	 * Deadlock fix: avoid blocking forever on sem_wait() if the breakpoint/
+	 * watchpoint signal never arrives. Use a bounded wait and fail the test
+	 * on timeout instead.
+	 */
+	ts.tv_sec += wait_timeout_sec;
+	if (!sem_timedwait(sem, &ts))
+		return 0;
+
+	if (errno == ETIMEDOUT)
+		printf("%s: Timed out waiting for event\n", name);
+	else
+		printf("%s: sem_timedwait() failed with %d\n", name, errno);
+
+	return -1;
+}
+
+int main(int argc, char *argv[])
+{
+	sem_init(&ib_mtx, 0, 0);
+	if (trigger_bp() < 0)
+		return -1;
+	if (wait_event(&ib_mtx, "Breakpoint") < 0)
+		return -1;
+
+	if (bp_triggered)
+		printf("Breakpoint test passed!\n");
+
+	sem_init(&wp_mtx, 0, 0);
+	if (trigger_wp() < 0)
+		return -1;
+	if (wait_event(&wp_mtx, "Watchpoint") < 0)
+		return -1;
+
+	if (wp_triggered)
+		printf("Watchpoint test passed!\n");
+
+	return 0;
+}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 3/8] riscv: ptrace support for hardware break/watchpoints
  2026-09-30  6:39 [PATCH v7 0/8] riscv: Introduce support for hardware break/watchpoints Himanshu Chauhan
  2026-09-30  6:39 ` [PATCH v7 1/8] " Himanshu Chauhan
  2026-09-30  6:39 ` [PATCH v7 2/8] riscv: Add breakpoint and watchpoint test for riscv Himanshu Chauhan
@ 2026-09-30  6:39 ` Himanshu Chauhan
  2026-09-30  6:56   ` sashiko-bot
  2026-09-30  6:39 ` [PATCH v7 4/8] selftests/breakpoints: extend riscv test for ptrace hw break/watchpoints Himanshu Chauhan
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Himanshu Chauhan @ 2026-09-30  6:39 UTC (permalink / raw)
  To: linux-kernel, linux-riscv, linux-perf-users
  Cc: peterz, mingo, alex, aou, jtaubepe, palmer, pjw, qingfang.deng,
	shuah, thecharlesjenkins, cp0613, Himanshu Chauhan

Add ptrace support for hardware breakpoints and watchpoints on
RISC-V. Debuggers can now set and query hardware debug triggers
through the standard PTRACE_GETREGSET/SETREGSET interface using new
NT_RISCV_HW_BREAK/WATCH note types, backed by
register_user_hw_breakpoint()/modify_user_hw_breakpoint() and
delivering SIGTRAP/TRAP_HWBKPT to the tracee when a trigger fires.

For convenience, also add a simpler PTRACE_GETHBPREGS/SETHBPREGS
request pair that lets a tracer read or write a single breakpoint or
watchpoint directly, without going through the regset machinery.
These request numbers live in the arch-specific ptrace range
(0x4210/0x4211) so they don't collide with the generic
PTRACE_PEEKDATA/PTRACE_PEEKUSR codes.

Breakpoints and watchpoints share the same trigger pool on this
architecture, so select HAVE_MIXED_BREAKPOINTS_REGS. Hook up
thread flush/copy so per-task breakpoints are cleaned up and cleared
across fork/exec.

Signed-off-by: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
---
 arch/riscv/Kconfig                     |   1 +
 arch/riscv/include/asm/hw_breakpoint.h |  10 +
 arch/riscv/include/asm/processor.h     |  18 +
 arch/riscv/include/uapi/asm/ptrace.h   |  50 +++
 arch/riscv/kernel/hw_breakpoint.c      |   1 -
 arch/riscv/kernel/process.c            |   5 +
 arch/riscv/kernel/ptrace.c             | 507 +++++++++++++++++++++++++
 include/uapi/linux/elf.h               |   4 +
 tools/include/uapi/linux/elf.h         |   2 +
 9 files changed, 597 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 17fa7d4f9729..d382f5d7b6a7 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -176,6 +176,7 @@ config RISCV
 	select HAVE_GCC_PLUGINS
 	select HAVE_GENERIC_VDSO if MMU
 	select HAVE_HW_BREAKPOINT if PERF_EVENTS
+	select HAVE_MIXED_BREAKPOINTS_REGS if PERF_EVENTS
 	select HAVE_IRQ_TIME_ACCOUNTING
 	select HAVE_KERNEL_BZIP2 if !EFI_ZBOOT
 	select HAVE_KERNEL_GZIP if !EFI_ZBOOT
diff --git a/arch/riscv/include/asm/hw_breakpoint.h b/arch/riscv/include/asm/hw_breakpoint.h
index a165bb9636de..ca2ef93c020c 100644
--- a/arch/riscv/include/asm/hw_breakpoint.h
+++ b/arch/riscv/include/asm/hw_breakpoint.h
@@ -14,6 +14,7 @@ struct task_struct;
 
 /* Maximum number of hardware breakpoints supported */
 #define RISCV_HW_BP_NUM_MAX 32
+#define RISCV_MAX_BP		16
 
 #if __riscv_xlen == 64
 #define cpu_to_le cpu_to_le64
@@ -279,6 +280,10 @@ struct arch_hw_breakpoint {
 	unsigned long address;
 	unsigned long len;
 	unsigned int type;
+	unsigned int match;
+	unsigned int chain;
+	unsigned int select;
+	unsigned int time;
 
 	/* Trigger configuration data */
 	unsigned long tdata1;
@@ -305,8 +310,13 @@ void arch_disable_hw_breakpoint(struct perf_event *bp);
 int arch_install_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 clear_ptrace_hw_breakpoint(struct task_struct *tsk);
+void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
+void ptrace_hw_copy_thread(struct task_struct *task);
 
 #else
 
+static inline void ptrace_hw_copy_thread(struct task_struct *task) { }
+
 #endif /* CONFIG_HAVE_HW_BREAKPOINT */
 #endif /* __RISCV_HW_BREAKPOINT_H */
diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index 815715c67f94..6f6186b9663e 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -15,6 +15,7 @@
 #include <asm/ptrace.h>
 #include <asm/insn-def.h>
 #include <asm/alternative-macros.h>
+#include <asm/hw_breakpoint.h>
 #include <asm/hwcap.h>
 #include <asm/usercfi.h>
 
@@ -110,6 +111,19 @@ struct pt_regs;
 #define RISCV_PREEMPT_V_NEED_RESTORE	0x40000000
 #define RISCV_PREEMPT_V_IN_SCHEDULE	0x20000000
 
+struct debug_info {
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	/* Have we suspended stepping by a debugger? */
+	int			suspended_step;
+	/* Allow breakpoints and watchpoints to be disabled for this thread. */
+	int			bp_disabled;
+	int			wp_disabled;
+	/* Hardware breakpoints pinned to this task. */
+	struct perf_event	*hbp_break[RISCV_MAX_BP];
+	struct perf_event	*hbp_watch[RISCV_MAX_BP];
+#endif
+};
+
 /* CPU-specific state of a task */
 struct thread_struct {
 	/* Callee-saved registers */
@@ -133,6 +147,10 @@ struct thread_struct {
 #endif
 #ifdef CONFIG_RISCV_ISA_SSQOSID
 	u32 srmcfg;
+#endif
+	struct debug_info debug;
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	struct perf_event	*ptrace_bps[RISCV_HW_BP_NUM_MAX];
 #endif
 };
 
diff --git a/arch/riscv/include/uapi/asm/ptrace.h b/arch/riscv/include/uapi/asm/ptrace.h
index 3de2b7124aff..83e073dbbf6d 100644
--- a/arch/riscv/include/uapi/asm/ptrace.h
+++ b/arch/riscv/include/uapi/asm/ptrace.h
@@ -10,11 +10,14 @@
 
 #include <linux/types.h>
 #include <linux/const.h>
+#include <linux/bits.h>
 
 #define PTRACE_GETFDPIC		33
 
 #define PTRACE_GETFDPIC_EXEC	0
 #define PTRACE_GETFDPIC_INTERP	1
+#define PTRACE_GETHBPREGS	0x4210
+#define PTRACE_SETHBPREGS	0x4211
 
 /*
  * User-mode register state for core dumps, ptrace, sigcontext
@@ -164,6 +167,53 @@ struct user_cfi_state {
 	__u64 shstk_ptr;
 };
 
+/*
+ * bit[3:0]        Match
+ * bit[8:4]        Size
+ * bit[11:9]       When
+ * bit[12]         Select
+ * bit[13]         Chain
+ * bit[16:14]      Type
+ * bit[XLEN-1:17]  Reserved
+ */
+#define HWDEBUG_MATCH_MASK	__GENMASK(3, 0)
+#define HWDEBUG_SIZE_MASK	__GENMASK(8, 4)
+#define HWDEBUG_WHEN_MASK	__GENMASK(11, 9)
+#define HWDEBUG_SELECT_MASK	_BITUL(12)
+#define HWDEBUG_CHAIN_MASK	_BITUL(13)
+#define HWDEBUG_TYPE_MASK	__GENMASK(16, 14)
+
+#define HWDEBUG_MATCH(_ctrl)	(((_ctrl) & HWDEBUG_MATCH_MASK) >> 0)
+#define HWDEBUG_SIZE(_ctrl)	(((_ctrl) & HWDEBUG_SIZE_MASK) >> 4)
+#define HWDEBUG_WHEN(_ctrl)	(((_ctrl) & HWDEBUG_WHEN_MASK) >> 9)
+#define HWDEBUG_SELECT(_ctrl)	(((_ctrl) & HWDEBUG_SELECT_MASK) >> 12)
+#define HWDEBUG_CHAIN(_ctrl)	(((_ctrl) & HWDEBUG_CHAIN_MASK) >> 13)
+#define HWDEBUG_TYPE(_ctrl)	(((_ctrl) & HWDEBUG_TYPE_MASK) >> 14)
+
+#define HWDEBUG_MK_MATCH(_match)	((_match << 0) & HWDEBUG_MATCH_MASK)
+#define HWDEBUG_MK_SIZE(_sz)		((_sz << 4) & HWDEBUG_SIZE_MASK)
+#define HWDEBUG_MK_WHEN(_when)		((_when << 9) & HWDEBUG_WHEN_MASK)
+#define HWDEBUG_MK_SELECT(_sel)		((_sel << 12) & HWDEBUG_SELECT_MASK)
+#define HWDEBUG_MK_CHAIN(_chain)	((_chain << 13) & HWDEBUG_CHAIN_MASK)
+#define HWDEBUG_MK_TYPE(_type)		((_type << 14) & HWDEBUG_TYPE_MASK)
+
+struct user_hwdebug_state {
+	__u32 info;
+	__u32 pad;
+	struct {
+		__u64 addr;
+		__u32 control;
+		__u32 pad;
+	} dbg_regs[16];
+};
+
+struct __riscv_hwdebug_state {
+	__u64 addr;
+	__u64 type;
+	__u64 len;
+	__u64 ctrl;
+};
+
 #endif /* __ASSEMBLER__ */
 
 #endif /* _UAPI_ASM_RISCV_PTRACE_H */
diff --git a/arch/riscv/kernel/hw_breakpoint.c b/arch/riscv/kernel/hw_breakpoint.c
index c2ab5e6008e7..dd6f63953969 100644
--- a/arch/riscv/kernel/hw_breakpoint.c
+++ b/arch/riscv/kernel/hw_breakpoint.c
@@ -698,7 +698,6 @@ EXPORT_SYMBOL_GPL(arch_disable_hw_breakpoint);
 
 void hw_breakpoint_pmu_read(struct perf_event *bp) { }
 
-void flush_ptrace_hw_breakpoint(struct task_struct *tsk) { }
 
 static int __init arch_hw_breakpoint_init(void)
 {
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index 7cc5a6a5c020..bdcb0386cd0d 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -204,6 +204,7 @@ void flush_thread(void)
 	if (riscv_has_extension_unlikely(RISCV_ISA_EXT_SUPM))
 		envcfg_update_bits(current, ENVCFG_PMM, ENVCFG_PMM_PMLEN_0);
 #endif
+	flush_ptrace_hw_breakpoint(current);
 }
 
 void arch_release_task_struct(struct task_struct *tsk)
@@ -283,6 +284,10 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 	p->thread.riscv_v_flags = 0;
 	if (has_vector() || has_xtheadvector())
 		riscv_v_thread_alloc(p);
+	ptrace_hw_copy_thread(p);
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	memset(p->thread.ptrace_bps, 0, sizeof(p->thread.ptrace_bps));
+#endif
 	p->thread.sp = (unsigned long)childregs; /* kernel sp */
 	return 0;
 }
diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
index f336a183667e..2d4ee51e6859 100644
--- a/arch/riscv/kernel/ptrace.c
+++ b/arch/riscv/kernel/ptrace.c
@@ -18,8 +18,10 @@
 #include <linux/elf.h>
 #include <linux/regset.h>
 #include <linux/sched.h>
+#include <linux/sched/signal.h>
 #include <linux/sched/task_stack.h>
 #include <asm/usercfi.h>
+#include <linux/hw_breakpoint.h>
 
 enum riscv_regset {
 	REGSET_X,
@@ -35,6 +37,10 @@ enum riscv_regset {
 #ifdef CONFIG_RISCV_USER_CFI
 	REGSET_CFI,
 #endif
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	REGSET_RISCV_HW_BREAK,
+	REGSET_RISCV_HW_WATCH,
+#endif
 };
 
 static int riscv_gpr_get(struct task_struct *target,
@@ -372,6 +378,397 @@ static int riscv_cfi_set(struct task_struct *target,
 }
 #endif
 
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+/*
+ * Handle hitting a HW-breakpoint.
+ */
+static void riscv_ptrace_hbptriggered(struct perf_event *bp,
+				struct perf_sample_data *data,
+				struct pt_regs *regs)
+{
+	struct arch_hw_breakpoint *bkpt = counter_arch_bp(bp);
+
+	force_sig_fault(SIGTRAP, TRAP_HWBKPT, (void __user *)bkpt->address);
+}
+
+/*
+ * Unregister breakpoints from this task and reset the pointers in
+ * the thread_struct.
+ */
+void flush_ptrace_hw_breakpoint(struct task_struct *tsk)
+{
+	int i;
+	struct thread_struct *t = &tsk->thread;
+
+	for (i = 0; i < RISCV_MAX_BP; i++) {
+		if (t->debug.hbp_break[i]) {
+			unregister_hw_breakpoint(t->debug.hbp_break[i]);
+			t->debug.hbp_break[i] = NULL;
+		}
+	}
+
+	for (i = 0; i < RISCV_MAX_BP; i++) {
+		if (t->debug.hbp_watch[i]) {
+			unregister_hw_breakpoint(t->debug.hbp_watch[i]);
+			t->debug.hbp_watch[i] = NULL;
+		}
+	}
+}
+
+void ptrace_hw_copy_thread(struct task_struct *tsk)
+{
+	memset(&tsk->thread.debug, 0, sizeof(struct debug_info));
+}
+
+static struct perf_event *ptrace_hbp_get_event(unsigned int note_type,
+					       struct task_struct *tsk,
+					       unsigned long idx)
+{
+	struct perf_event *bp = ERR_PTR(-EINVAL);
+
+	switch (note_type) {
+	case NT_RISCV_HW_BREAK:
+		if (idx >= RISCV_MAX_BP)
+			goto out;
+		idx = array_index_nospec(idx, RISCV_MAX_BP);
+		bp = tsk->thread.debug.hbp_break[idx];
+		break;
+	case NT_RISCV_HW_WATCH:
+		if (idx >= RISCV_MAX_BP)
+			goto out;
+		idx = array_index_nospec(idx, RISCV_MAX_BP);
+		bp = tsk->thread.debug.hbp_watch[idx];
+		break;
+	}
+
+out:
+	return bp;
+}
+
+static int ptrace_hbp_set_event(unsigned int note_type,
+				struct task_struct *tsk,
+				unsigned long idx,
+				struct perf_event *bp)
+{
+	int err = -EINVAL;
+
+	switch (note_type) {
+	case NT_RISCV_HW_BREAK:
+		if (idx >= RISCV_MAX_BP)
+			goto out;
+		idx = array_index_nospec(idx, RISCV_MAX_BP);
+		tsk->thread.debug.hbp_break[idx] = bp;
+		err = 0;
+		break;
+	case NT_RISCV_HW_WATCH:
+		if (idx >= RISCV_MAX_BP)
+			goto out;
+		idx = array_index_nospec(idx, RISCV_MAX_BP);
+		tsk->thread.debug.hbp_watch[idx] = bp;
+		err = 0;
+		break;
+	}
+
+out:
+	return err;
+}
+
+static struct perf_event *ptrace_hbp_create(unsigned int note_type,
+					    struct task_struct *tsk,
+					    unsigned long idx)
+{
+	struct perf_event *bp;
+	struct perf_event_attr attr;
+	int err, type;
+
+	switch (note_type) {
+	case NT_RISCV_HW_BREAK:
+		type = HW_BREAKPOINT_X;
+		break;
+	case NT_RISCV_HW_WATCH:
+		type = HW_BREAKPOINT_RW;
+		break;
+	default:
+		return ERR_PTR(-EINVAL);
+	}
+
+	ptrace_breakpoint_init(&attr);
+
+	/*
+	 * Initialise fields to sane defaults
+	 * (i.e. values that will pass validation).
+	 */
+	attr.bp_addr	= 0;
+	attr.bp_len	= HW_BREAKPOINT_LEN_4;
+	attr.bp_type	= type;
+	attr.disabled	= 1;
+
+	bp = register_user_hw_breakpoint(&attr, riscv_ptrace_hbptriggered, NULL, tsk);
+	if (IS_ERR(bp))
+		return bp;
+
+	err = ptrace_hbp_set_event(note_type, tsk, idx, bp);
+	if (err)
+		return ERR_PTR(err);
+
+	return bp;
+}
+
+static int ptrace_hbp_fill_attr_ctrl(unsigned int note_type,
+				     struct arch_hw_breakpoint *bpctrl,
+				     struct perf_event_attr *attr)
+{
+	int len, type;
+
+	attr->disabled = 0;
+	type = bpctrl->type;
+	len = bpctrl->len;
+
+	switch (note_type) {
+	case NT_RISCV_HW_BREAK:
+		if ((type & HW_BREAKPOINT_X) != type)
+			return -EINVAL;
+		break;
+	case NT_RISCV_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	= bpctrl->address;
+
+	return 0;
+}
+
+static int ptrace_hbp_get_resource_info(unsigned int note_type, u32 *info)
+{
+	u8 num;
+
+	switch (note_type) {
+	case NT_RISCV_HW_BREAK:
+		num = hw_breakpoint_slots(TYPE_INST);
+		break;
+	case NT_RISCV_HW_WATCH:
+		num = hw_breakpoint_slots(TYPE_DATA);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	*info = num;
+
+	return 0;
+}
+
+static u32 encode_ctrl_reg(struct perf_event *bp)
+{
+	struct arch_hw_breakpoint *bpctrl = counter_arch_bp(bp);
+	u32 ctrl = 0;
+
+	/* Expose the generic UAPI bp_type values in ptrace control bits. */
+	ctrl |= HWDEBUG_MK_TYPE(bp->attr.bp_type);
+	ctrl |= HWDEBUG_MK_MATCH(bpctrl->match);
+	ctrl |= HWDEBUG_MK_SELECT(bpctrl->select);
+	ctrl |= HWDEBUG_MK_WHEN(bpctrl->time);
+	ctrl |= HWDEBUG_MK_SIZE(bp->attr.bp_len);
+	ctrl |= HWDEBUG_MK_CHAIN(bpctrl->chain);
+
+	return ctrl;
+}
+
+static int ptrace_hbp_get_ctrl(unsigned int note_type,
+			       struct task_struct *tsk,
+			       unsigned long idx,
+			       u32 *ctrl)
+{
+	struct perf_event *bp = ptrace_hbp_get_event(note_type, tsk, idx);
+
+	if (IS_ERR(bp))
+		return PTR_ERR(bp);
+
+	*ctrl = bp ? encode_ctrl_reg(bp) : 0;
+	return 0;
+}
+
+static int ptrace_hbp_get_addr(unsigned int note_type,
+			       struct task_struct *tsk,
+			       unsigned long idx,
+			       u64 *addr)
+{
+	struct perf_event *bp = ptrace_hbp_get_event(note_type, tsk, idx);
+
+	if (IS_ERR(bp))
+		return PTR_ERR(bp);
+
+	*addr = bp ? counter_arch_bp(bp)->address : 0;
+	return 0;
+}
+
+static struct perf_event *ptrace_hbp_get_initialised_bp(unsigned int note_type,
+							struct task_struct *tsk,
+							unsigned long idx)
+{
+	struct perf_event *bp = ptrace_hbp_get_event(note_type, tsk, idx);
+
+	if (!bp)
+		bp = ptrace_hbp_create(note_type, tsk, idx);
+
+	return bp;
+}
+
+static void decode_ctrl_reg(u32 uctrl, struct arch_hw_breakpoint *bpctrl)
+{
+	bpctrl->type = HWDEBUG_TYPE(uctrl);
+	bpctrl->match = HWDEBUG_MATCH(uctrl);
+	bpctrl->select = HWDEBUG_SELECT(uctrl);
+	bpctrl->time = HWDEBUG_WHEN(uctrl);
+	bpctrl->len = HWDEBUG_SIZE(uctrl);
+	bpctrl->chain = HWDEBUG_CHAIN(uctrl);
+}
+
+static int ptrace_hbp_set_ctrl(unsigned int note_type,
+			       struct task_struct *tsk,
+			       unsigned long idx,
+			       u32 uctrl)
+{
+	int err;
+	struct perf_event *bp;
+	struct perf_event_attr attr;
+	struct arch_hw_breakpoint bpctrl;
+
+	bp = ptrace_hbp_get_initialised_bp(note_type, tsk, idx);
+	if (IS_ERR(bp)) {
+		err = PTR_ERR(bp);
+		return err;
+	}
+
+	attr = bp->attr;
+	decode_ctrl_reg(uctrl, &bpctrl);
+	bpctrl.address = attr.bp_addr;
+	err = ptrace_hbp_fill_attr_ctrl(note_type, &bpctrl, &attr);
+	if (err)
+		return err;
+
+	return modify_user_hw_breakpoint(bp, &attr);
+}
+
+static int ptrace_hbp_set_addr(unsigned int note_type,
+			       struct task_struct *tsk,
+			       unsigned long idx,
+			       u64 addr)
+{
+	int err;
+	struct perf_event *bp;
+	struct perf_event_attr attr;
+
+	bp = ptrace_hbp_get_initialised_bp(note_type, tsk, idx);
+	if (IS_ERR(bp)) {
+		err = PTR_ERR(bp);
+		return err;
+	}
+
+	attr = bp->attr;
+	attr.bp_addr = addr;
+	err = modify_user_hw_breakpoint(bp, &attr);
+	return err;
+}
+
+#define PTRACE_HBP_ADDR_SZ	sizeof(u64)
+#define PTRACE_HBP_CTRL_SZ	sizeof(u32)
+#define PTRACE_HBP_PAD_SZ	sizeof(u32)
+
+static int riscv_hw_break_get(struct task_struct *target,
+			const struct user_regset *regset,
+			struct membuf to)
+{
+	unsigned int note_type = regset->core_note_type;
+	int ret, idx, num_slots;
+	u32 info, ctrl;
+	u64 addr;
+
+	/* Resource info: number of available slots */
+	ret = ptrace_hbp_get_resource_info(note_type, &info);
+	if (ret)
+		return ret;
+
+	membuf_write(&to, &info, sizeof(info));
+	membuf_zero(&to, sizeof(u32));
+
+	/* Emit one (address, ctrl, pad) entry per available slot */
+	num_slots = (int)info;
+	for (idx = 0; idx < num_slots; idx++) {
+		ret = ptrace_hbp_get_addr(note_type, target, idx, &addr);
+		if (ret)
+			return ret;
+		ret = ptrace_hbp_get_ctrl(note_type, target, idx, &ctrl);
+		if (ret)
+			return ret;
+		membuf_store(&to, addr);
+		membuf_store(&to, ctrl);
+		membuf_zero(&to, sizeof(u32));
+	}
+	return 0;
+}
+
+static int riscv_hw_break_set(struct task_struct *target,
+			const struct user_regset *regset,
+			unsigned int pos, unsigned int count,
+			const void *kbuf, const void __user *ubuf)
+{
+	unsigned int note_type = regset->core_note_type;
+	int ret, idx = 0, offset, limit;
+	u32 ctrl;
+	u64 addr;
+
+	/* Resource info and pad */
+	offset = offsetof(struct user_hwdebug_state, dbg_regs);
+	user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf, 0, offset);
+
+	/* (address, ctrl) registers */
+	limit = regset->n * regset->size;
+	while (count && offset < limit) {
+		if (count < PTRACE_HBP_ADDR_SZ)
+			return -EINVAL;
+
+		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &addr,
+					 offset, offset + PTRACE_HBP_ADDR_SZ);
+		if (ret)
+			return ret;
+
+		ret = ptrace_hbp_set_addr(note_type, target, idx, addr);
+		if (ret)
+			return ret;
+
+		offset += PTRACE_HBP_ADDR_SZ;
+
+		if (!count)
+			break;
+
+		ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &ctrl,
+					 offset, offset + PTRACE_HBP_CTRL_SZ);
+		if (ret)
+			return ret;
+
+		ret = ptrace_hbp_set_ctrl(note_type, target, idx, ctrl);
+		if (ret)
+			return ret;
+
+		offset += PTRACE_HBP_CTRL_SZ;
+
+		user_regset_copyin_ignore(&pos, &count, &kbuf, &ubuf,
+					  offset, offset + PTRACE_HBP_PAD_SZ);
+		offset += PTRACE_HBP_PAD_SZ;
+		idx++;
+	}
+
+	return 0;
+}
+#endif	/* CONFIG_HAVE_HW_BREAKPOINT */
+
 static struct user_regset riscv_user_regset[] __ro_after_init = {
 	[REGSET_X] = {
 		USER_REGSET_NOTE_TYPE(PRSTATUS),
@@ -421,6 +818,24 @@ static struct user_regset riscv_user_regset[] __ro_after_init = {
 		.set = riscv_cfi_set,
 	},
 #endif
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	[REGSET_RISCV_HW_BREAK] = {
+		USER_REGSET_NOTE_TYPE(RISCV_HW_BREAK),
+		.n = sizeof(struct user_hwdebug_state) / sizeof(u32),
+		.size = sizeof(u32),
+		.align = sizeof(u32),
+		.regset_get = riscv_hw_break_get,
+		.set = riscv_hw_break_set,
+	},
+	[REGSET_RISCV_HW_WATCH] = {
+		USER_REGSET_NOTE_TYPE(RISCV_HW_WATCH),
+		.n = sizeof(struct user_hwdebug_state) / sizeof(u32),
+		.size = sizeof(u32),
+		.align = sizeof(u32),
+		.regset_get = riscv_hw_break_get,
+		.set = riscv_hw_break_set,
+	},
+#endif
 };
 
 static const struct user_regset_view riscv_user_native_view = {
@@ -541,12 +956,104 @@ void ptrace_disable(struct task_struct *child)
 {
 }
 
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+static int riscv_ptrace_bp_get(struct task_struct *child, unsigned long idx,
+			  struct __riscv_hwdebug_state *state)
+{
+	struct perf_event *bp;
+
+	if (idx >= RISCV_HW_BP_NUM_MAX)
+		return -EINVAL;
+
+	bp = child->thread.ptrace_bps[idx];
+	if (!bp)
+		return -ENOENT;
+
+	state->addr = bp->attr.bp_addr;
+	state->len  = bp->attr.bp_len;
+	state->type = bp->attr.bp_type;
+	state->ctrl = bp->attr.disabled == 1;
+
+	return 0;
+}
+
+static int riscv_ptrace_bp_set(struct task_struct *child, unsigned long idx,
+			  struct __riscv_hwdebug_state *state)
+{
+	struct perf_event *bp;
+	struct perf_event_attr attr;
+
+	if (idx >= RISCV_HW_BP_NUM_MAX)
+		return -EINVAL;
+
+	bp = child->thread.ptrace_bps[idx];
+	if (bp)
+		attr = bp->attr;
+	else
+		ptrace_breakpoint_init(&attr);
+
+	attr.bp_addr = state->addr;
+	attr.bp_len  = state->len;
+	attr.bp_type = state->type;
+	/* Always register disabled; enable below if requested */
+	attr.disabled = 1;
+
+	if (!bp) {
+		bp = register_user_hw_breakpoint(&attr, riscv_ptrace_hbptriggered, NULL, child);
+		if (IS_ERR(bp))
+			return PTR_ERR(bp);
+		child->thread.ptrace_bps[idx] = bp;
+	}
+
+	/* Enable or disable as requested by ctrl (0 = enabled, 1 = disabled) */
+	attr.disabled = state->ctrl == 1;
+	return modify_user_hw_breakpoint(bp, &attr);
+}
+
+static long riscv_ptrace_gethbpregs(struct task_struct *child, unsigned long idx,
+			      unsigned long __user *datap)
+{
+	struct __riscv_hwdebug_state state;
+	long ret;
+
+	ret = riscv_ptrace_bp_get(child, idx, &state);
+	if (ret)
+		return ret;
+	if (copy_to_user(datap, &state, sizeof(state)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static long riscv_ptrace_sethbpregs(struct task_struct *child, unsigned long idx,
+			      unsigned long __user *datap)
+{
+	struct __riscv_hwdebug_state state;
+
+	if (copy_from_user(&state, datap, sizeof(state)))
+		return -EFAULT;
+
+	return riscv_ptrace_bp_set(child, idx, &state);
+}
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+
 long arch_ptrace(struct task_struct *child, long request,
 		 unsigned long addr, unsigned long data)
 {
 	long ret = -EIO;
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	unsigned long __user *datap = (unsigned long __user *)data;
+#endif
 
 	switch (request) {
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	case PTRACE_GETHBPREGS:
+		ret = riscv_ptrace_gethbpregs(child, addr, datap);
+		break;
+	case PTRACE_SETHBPREGS:
+		ret = riscv_ptrace_sethbpregs(child, addr, datap);
+		break;
+#endif
 	default:
 		ret = ptrace_request(child, request, addr, data);
 		break;
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index ee30dcd80901..1315ac35157c 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -547,6 +547,10 @@ typedef struct elf64_shdr {
 #define NT_RISCV_TAGGED_ADDR_CTRL 0x902	/* RISC-V tagged address control (prctl()) */
 #define NN_RISCV_USER_CFI	"LINUX"
 #define NT_RISCV_USER_CFI	0x903		/* RISC-V shadow stack state */
+#define NN_RISCV_HW_BREAK	"LINUX"
+#define NT_RISCV_HW_BREAK	0x904		/* RISC-V hardware breakpoint registers */
+#define NN_RISCV_HW_WATCH	"LINUX"
+#define NT_RISCV_HW_WATCH	0x905		/* RISCV-V hardware watchpoint registers */
 #define NN_LOONGARCH_CPUCFG	"LINUX"
 #define NT_LOONGARCH_CPUCFG	0xa00	/* LoongArch CPU config registers */
 #define NN_LOONGARCH_CSR	"LINUX"
diff --git a/tools/include/uapi/linux/elf.h b/tools/include/uapi/linux/elf.h
index 5834b83d7f9a..21f225502051 100644
--- a/tools/include/uapi/linux/elf.h
+++ b/tools/include/uapi/linux/elf.h
@@ -460,6 +460,8 @@ typedef struct elf64_shdr {
 #define NT_RISCV_CSR	0x900		/* RISC-V Control and Status Registers */
 #define NT_RISCV_VECTOR	0x901		/* RISC-V vector registers */
 #define NT_RISCV_TAGGED_ADDR_CTRL 0x902	/* RISC-V tagged address control (prctl()) */
+#define NT_RISCV_HW_BREAK	0x904
+#define NT_RISCV_HW_WATCH	0x905
 #define NT_LOONGARCH_CPUCFG	0xa00	/* LoongArch CPU config registers */
 #define NT_LOONGARCH_CSR	0xa01	/* LoongArch control and status registers */
 #define NT_LOONGARCH_LSX	0xa02	/* LoongArch Loongson SIMD Extension registers */
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 4/8] selftests/breakpoints: extend riscv test for ptrace hw break/watchpoints
  2026-09-30  6:39 [PATCH v7 0/8] riscv: Introduce support for hardware break/watchpoints Himanshu Chauhan
                   ` (2 preceding siblings ...)
  2026-09-30  6:39 ` [PATCH v7 3/8] riscv: ptrace support for hardware break/watchpoints Himanshu Chauhan
@ 2026-09-30  6:39 ` Himanshu Chauhan
  2026-09-30  6:51   ` sashiko-bot
  2026-09-30  6:39 ` [PATCH v7 5/8] RISC-V: Add fetch and decode helpers to a common file Himanshu Chauhan
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Himanshu Chauhan @ 2026-09-30  6:39 UTC (permalink / raw)
  To: linux-kernel, linux-riscv, linux-perf-users
  Cc: peterz, mingo, alex, aou, jtaubepe, palmer, pjw, qingfang.deng,
	shuah, thecharlesjenkins, cp0613, Himanshu Chauhan

Cover the new ptrace-based hardware breakpoint/watchpoint support
added for riscv: exercise the PTRACE_GETREGSET/SETREGSET regset path
as well as the raw PTRACE_GETHBPREGS/SETHBPREGS interface, alongside
the existing perf_event-based tests.

Also drop a leftover #if 0 block duplicating the HWDEBUG_* control
field macros already defined in uapi/asm/ptrace.h.

Signed-off-by: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
---
 .../breakpoints/breakpoint_test_riscv.c       | 594 +++++++++++++++++-
 1 file changed, 572 insertions(+), 22 deletions(-)

diff --git a/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c b/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c
index 0649940b709e..13d6b3ff1b6e 100644
--- a/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c
+++ b/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c
@@ -9,6 +9,7 @@
 #include <linux/perf_event.h>    /* Definition of PERF_* constants */
 #include <linux/hw_breakpoint.h> /* Definition of HW_* constants */
 #include <sys/syscall.h>         /* Definition of SYS_* constants */
+#include <asm/ptrace.h>
 #include <unistd.h>
 #include <stdbool.h>
 #include <stdio.h>
@@ -18,18 +19,576 @@
 #include <fcntl.h>
 #include <signal.h>
 #include <sys/mman.h>
+#include <sys/ptrace.h>
+#include <sys/wait.h>
 #include <string.h>
 #include <semaphore.h>
 #include <errno.h>
+#include <stdint.h>
+#include <stddef.h>
+#include <linux/elf.h>
 
 #ifndef noinline
 #define noinline __attribute__((noinline))
 #endif
 
+#include "kselftest.h"
+
+static int test_func_sink;
+
+/*
+ * Keep a real instruction address for HW execute breakpoints: prevent inlining
+ * and force a visible side effect so the function can't be optimized away.
+ */
+static noinline void test_func(void)
+{
+	test_func_sink++;
+	__asm__ __volatile__("" : : "g" (test_func_sink));
+}
+
+/*
+ * BREAKPOINT TEST USING PTRACE
+ */
+static int do_wp_child(void *addr, size_t size)
+{
+	if (ptrace(PTRACE_TRACEME, 0, NULL, NULL) != 0) {
+		ksft_print_msg(
+			       "ptrace(PTRACE_TRACEME) failed: %s\n",
+			       strerror(errno));
+		_exit(1);
+	}
+
+	if (raise(SIGSTOP) != 0) {
+		ksft_print_msg(
+			       "raise(SIGSTOP) failed: %s\n", strerror(errno));
+		_exit(1);
+	}
+
+	sleep(1);
+	switch (size) {
+	case 1:
+		*(uint8_t *)addr = 47;
+		break;
+	case 2:
+		*(uint16_t *)addr = 47;
+		break;
+	case 4:
+		*(uint32_t *)addr = 47;
+		break;
+	case 8:
+		*(uint64_t *)addr = 47;
+		break;
+	default:
+		ksft_print_msg("Unknown watchpoint access size %u\n", size);
+		break;
+	}
+
+	_exit(0);
+}
+
+static int do_bp_child(void (*bp_func)(void))
+{
+	if (ptrace(PTRACE_TRACEME, 0, NULL, NULL) != 0) {
+		ksft_print_msg(
+			       "ptrace(PTRACE_TRACEME) failed: %s\n",
+			       strerror(errno));
+		_exit(1);
+	}
+
+	if (raise(SIGSTOP) != 0) {
+		ksft_print_msg(
+			       "raise(SIGSTOP) failed: %s\n", strerror(errno));
+		_exit(1);
+	}
+
+	bp_func();
+}
+
+unsigned long var;
+
+static bool set_watchpoint(pid_t pid, int size)
+{
+	uint8_t *addr = (uint8_t *)&var;
+	unsigned int control = 0;
+	struct user_hwdebug_state dreg_state;
+	struct iovec iov;
+
+	/* Write watchpoint */
+	control = (HW_BREAKPOINT_W << 14) & ((0x7 << 14));
+	control |= (HW_BREAKPOINT_LEN_8 << 4) & ((0x1f << 4));
+	memset(&dreg_state, 0, sizeof(dreg_state));
+	dreg_state.dbg_regs[0].addr = (uintptr_t)(addr);
+	dreg_state.dbg_regs[0].control = control;
+	iov.iov_base = &dreg_state;
+	iov.iov_len = offsetof(struct user_hwdebug_state, dbg_regs) +
+				sizeof(dreg_state.dbg_regs[0]);
+
+	if (ptrace(PTRACE_SETREGSET, pid, NT_RISCV_HW_WATCH, &iov) == 0) {
+		memset(&iov, 0, sizeof(iov));
+		memset(&dreg_state, 0, sizeof(dreg_state));
+
+		iov.iov_base = &dreg_state;
+		iov.iov_len = offsetof(struct user_hwdebug_state, dbg_regs) +
+			sizeof(dreg_state.dbg_regs[0]);
+
+		if (ptrace(PTRACE_GETREGSET, pid, NT_RISCV_HW_WATCH, &iov) == 0) {
+			ksft_print_msg(
+				       "ptrace(PTRACE_GETREGSET): Number of watchpoints: %u\n",
+				       dreg_state.info);
+			ksft_print_msg(
+				       "ptrace(PTRACE_GETREGSet): addr: 0x%lx control: 0x%x\n",
+				       dreg_state.dbg_regs[0].addr, dreg_state.dbg_regs[0].control);
+		} else {
+			ksft_print_msg(
+				       "ptrace(PTRACE_GETREGSET): Failed\n");
+			return false;
+		}
+
+		return true;
+	}
+
+	if (errno == EIO)
+		ksft_print_msg(
+			       "ptrace(PTRACE_SETREGSET, NT_RISCV_HW_WATCH) not supported: %s\n",
+			       strerror(errno));
+	else
+		ksft_print_msg(
+			       "ptrace(PTRACE_SETREGSET, NT_RISCV_HW_WATCH) failed: %s\n",
+			       strerror(errno));
+	return false;
+}
+
+static bool set_breakpoint(pid_t pid, void (*bp_func)(void))
+{
+	struct user_hwdebug_state dreg_state;
+	struct iovec iov;
+	unsigned int control = 0;
+
+	control = (HW_BREAKPOINT_X << 14) & ((0x7 << 14));
+	control |= (HW_BREAKPOINT_LEN_8 << 4) & ((0x1f << 4));
+	memset(&dreg_state, 0, sizeof(dreg_state));
+	dreg_state.dbg_regs[0].addr = (uintptr_t)bp_func;
+	dreg_state.dbg_regs[0].control = control;
+	iov.iov_base = &dreg_state;
+	iov.iov_len = offsetof(struct user_hwdebug_state, dbg_regs) + sizeof(dreg_state.dbg_regs[0]);
+
+	if (ptrace(PTRACE_SETREGSET, pid, NT_RISCV_HW_BREAK, &iov) == 0)
+		return true;
+
+	if (errno == EIO)
+		ksft_print_msg(
+			       "ptrace(PTRACE_SETREGSET, NT_RISCV_HW_BREAK) not supported: %s\n",
+			       strerror(errno));
+	else
+		ksft_print_msg(
+			       "ptrace(PTRACE_SETREGSET, NT_RISCV_HW_BREAK) failed: %s\n",
+			       strerror(errno));
+
+	return false;
+}
+
+static int run_ptrace_wp_test(void)
+{
+	pid_t pid = fork();
+	pid_t wpid;
+	siginfo_t siginfo;
+	int status;
+
+	if (pid == 0)
+		do_wp_child(&var, 8);
+
+	wpid = waitpid(pid, &status, __WALL);
+	if (wpid != pid) {
+		ksft_print_msg(
+			"waitpid() failed: %s\n", strerror(errno));
+		return false;
+	}
+	if (!WIFSTOPPED(status)) {
+		ksft_print_msg(
+			"child did not stop: %s\n", strerror(errno));
+		return false;
+	}
+	if (WSTOPSIG(status) != SIGSTOP) {
+		ksft_print_msg("child did not stop with SIGSTOP\n");
+		return false;
+	}
+
+	if (!set_watchpoint(pid, 8))
+		return false;
+
+	if (ptrace(PTRACE_CONT, pid, NULL, NULL) < 0) {
+		ksft_print_msg(
+			"ptrace(PTRACE_CONT) failed: %s\n",
+			strerror(errno));
+		return false;
+	}
+
+	alarm(3);
+	wpid = waitpid(pid, &status, __WALL);
+	if (wpid != pid) {
+		ksft_print_msg(
+			"waitpid() failed: %s\n", strerror(errno));
+		return false;
+	}
+	alarm(0);
+	if (WIFEXITED(status)) {
+		ksft_print_msg("child exited prematurely\n");
+		return false;
+	}
+	if (!WIFSTOPPED(status)) {
+		ksft_print_msg("child did not stop\n");
+		return false;
+	}
+	if (WSTOPSIG(status) != SIGTRAP) {
+		ksft_print_msg("child did not stop with SIGTRAP\n");
+		return false;
+	}
+	if (ptrace(PTRACE_GETSIGINFO, pid, NULL, &siginfo) != 0) {
+		ksft_print_msg(
+			"ptrace(PTRACE_GETSIGINFO): %s\n",
+			strerror(errno));
+		return false;
+	}
+	if (siginfo.si_code != TRAP_HWBKPT) {
+		ksft_print_msg(
+			"Unexpected si_code %d\n", siginfo.si_code);
+		return false;
+	}
+
+	kill(pid, SIGKILL);
+	wpid = waitpid(pid, &status, 0);
+	if (wpid != pid) {
+		ksft_print_msg(
+			"waitpid() failed: %s\n", strerror(errno));
+		return false;
+	}
+
+	ksft_print_msg("[ptrace]: Watchpoint test passed!\n");
+
+	return true;
+}
+
+static int run_ptrace_bp_test(void)
+{
+	pid_t pid = fork();
+	pid_t wpid;
+	siginfo_t siginfo;
+	int status;
+
+	if (pid == 0)
+		do_bp_child(test_func);
+
+	wpid = waitpid(pid, &status, __WALL);
+	if (wpid != pid) {
+		ksft_print_msg(
+			"waitpid() failed: %s\n", strerror(errno));
+		return false;
+	}
+	if (!WIFSTOPPED(status)) {
+		ksft_print_msg(
+			"child did not stop: %s\n", strerror(errno));
+		return false;
+	}
+	if (WSTOPSIG(status) != SIGSTOP) {
+		ksft_print_msg("child did not stop with SIGSTOP\n");
+		return false;
+	}
+
+	if (!set_breakpoint(pid, test_func))
+		return false;
+
+	if (ptrace(PTRACE_CONT, pid, NULL, NULL) < 0) {
+		ksft_print_msg(
+			"ptrace(PTRACE_CONT) failed: %s\n",
+			strerror(errno));
+		return false;
+	}
+
+	alarm(3);
+	wpid = waitpid(pid, &status, __WALL);
+	if (wpid != pid) {
+		ksft_print_msg(
+			"waitpid() failed: %s\n", strerror(errno));
+		return false;
+	}
+	alarm(0);
+	if (WIFEXITED(status)) {
+		ksft_print_msg("child exited prematurely\n");
+		return false;
+	}
+	if (!WIFSTOPPED(status)) {
+		ksft_print_msg("child did not stop\n");
+		return false;
+	}
+	if (WSTOPSIG(status) != SIGTRAP) {
+		ksft_print_msg("child did not stop with SIGTRAP\n");
+		return false;
+	}
+	if (ptrace(PTRACE_GETSIGINFO, pid, NULL, &siginfo) != 0) {
+		ksft_print_msg(
+			"ptrace(PTRACE_GETSIGINFO): %s\n",
+			strerror(errno));
+		return false;
+	}
+	if (siginfo.si_code != TRAP_HWBKPT) {
+		ksft_print_msg(
+			"Unexpected si_code %d\n", siginfo.si_code);
+		return false;
+	}
+
+	kill(pid, SIGKILL);
+	wpid = waitpid(pid, &status, 0);
+	if (wpid != pid) {
+		ksft_print_msg(
+			"waitpid() failed: %s\n", strerror(errno));
+		return false;
+	}
+
+	ksft_print_msg("[ptrace]: Breakpoint test passed!\n");
+
+	return true;
+}
+
+/*
+ * BREAKPOINT TEST USING PTRACE_SETHBPREGS / PTRACE_GETHBPREGS
+ */
+static bool set_hbpregs_watchpoint(pid_t pid)
+{
+	struct __riscv_hwdebug_state state;
+
+	memset(&state, 0, sizeof(state));
+	state.addr = (unsigned long)&var;
+	state.len  = HW_BREAKPOINT_LEN_8;
+	state.type = HW_BREAKPOINT_W;
+	state.ctrl = 0; /* enabled */
+
+	if (ptrace(PTRACE_SETHBPREGS, pid, 0, &state) != 0) {
+		ksft_print_msg(
+			"ptrace(PTRACE_SETHBPREGS) failed: %s\n",
+			strerror(errno));
+		return false;
+	}
+
+	/* Read back and verify */
+	memset(&state, 0, sizeof(state));
+	if (ptrace(PTRACE_GETHBPREGS, pid, 0, &state) != 0) {
+		ksft_print_msg(
+			"ptrace(PTRACE_GETHBPREGS) failed: %s\n",
+			strerror(errno));
+		return false;
+	}
+
+	ksft_print_msg(
+		"[hbpregs] watchpoint readback: addr=0x%lx type=%lu len=%lu ctrl=%lu\n",
+		state.addr, state.type, state.len, state.ctrl);
+
+	return true;
+}
+
+static bool set_hbpregs_breakpoint(pid_t pid, void (*bp_func)(void))
+{
+	struct __riscv_hwdebug_state state;
+
+	memset(&state, 0, sizeof(state));
+	state.addr = (unsigned long)bp_func;
+	state.len  = HW_BREAKPOINT_LEN_4;
+	state.type = HW_BREAKPOINT_X;
+	state.ctrl = 0; /* enabled */
+
+	if (ptrace(PTRACE_SETHBPREGS, pid, 0, &state) != 0) {
+		ksft_print_msg(
+			"ptrace(PTRACE_SETHBPREGS) failed: %s\n",
+			strerror(errno));
+		return false;
+	}
+
+	/* Read back and verify */
+	memset(&state, 0, sizeof(state));
+	if (ptrace(PTRACE_GETHBPREGS, pid, 0, &state) != 0) {
+		ksft_print_msg(
+			"ptrace(PTRACE_GETHBPREGS) failed: %s\n",
+			strerror(errno));
+		return false;
+	}
+
+	ksft_print_msg(
+		"[hbpregs] breakpoint readback: addr=0x%lx type=%lu len=%lu ctrl=%lu\n",
+		state.addr, state.type, state.len, state.ctrl);
+
+	return true;
+}
+
+static int run_hbpregs_wp_test(void)
+{
+	pid_t pid = fork();
+	pid_t wpid;
+	siginfo_t siginfo;
+	int status;
+
+	if (pid == 0)
+		do_wp_child(&var, 8);
+
+	wpid = waitpid(pid, &status, __WALL);
+	if (wpid != pid) {
+		ksft_print_msg("waitpid() failed: %s\n", strerror(errno));
+		return false;
+	}
+	if (!WIFSTOPPED(status)) {
+		ksft_print_msg("child did not stop: %s\n", strerror(errno));
+		return false;
+	}
+	if (WSTOPSIG(status) != SIGSTOP) {
+		ksft_print_msg("child did not stop with SIGSTOP\n");
+		return false;
+	}
+
+	if (!set_hbpregs_watchpoint(pid))
+		return false;
+
+	if (ptrace(PTRACE_CONT, pid, NULL, NULL) < 0) {
+		ksft_print_msg("ptrace(PTRACE_CONT) failed: %s\n",
+			strerror(errno));
+		return false;
+	}
+
+	alarm(3);
+	wpid = waitpid(pid, &status, __WALL);
+	if (wpid != pid) {
+		ksft_print_msg("waitpid() failed: %s\n", strerror(errno));
+		return false;
+	}
+	alarm(0);
+	if (WIFEXITED(status)) {
+		ksft_print_msg("child exited prematurely\n");
+		return false;
+	}
+	if (!WIFSTOPPED(status)) {
+		ksft_print_msg("child did not stop\n");
+		return false;
+	}
+	if (WSTOPSIG(status) != SIGTRAP) {
+		ksft_print_msg("child did not stop with SIGTRAP\n");
+		return false;
+	}
+	if (ptrace(PTRACE_GETSIGINFO, pid, NULL, &siginfo) != 0) {
+		ksft_print_msg("ptrace(PTRACE_GETSIGINFO): %s\n",
+			strerror(errno));
+		return false;
+	}
+	if (siginfo.si_code != TRAP_HWBKPT) {
+		ksft_print_msg("Unexpected si_code %d\n", siginfo.si_code);
+		return false;
+	}
+
+	kill(pid, SIGKILL);
+	wpid = waitpid(pid, &status, 0);
+	if (wpid != pid) {
+		ksft_print_msg("waitpid() failed: %s\n", strerror(errno));
+		return false;
+	}
+
+	ksft_print_msg("[hbpregs]: Watchpoint test passed!\n");
+	return true;
+}
+
+static int run_hbpregs_bp_test(void)
+{
+	pid_t pid = fork();
+	pid_t wpid;
+	siginfo_t siginfo;
+	int status;
+
+	if (pid == 0)
+		do_bp_child(test_func);
+
+	wpid = waitpid(pid, &status, __WALL);
+	if (wpid != pid) {
+		ksft_print_msg("waitpid() failed: %s\n", strerror(errno));
+		return false;
+	}
+	if (!WIFSTOPPED(status)) {
+		ksft_print_msg("child did not stop: %s\n", strerror(errno));
+		return false;
+	}
+	if (WSTOPSIG(status) != SIGSTOP) {
+		ksft_print_msg("child did not stop with SIGSTOP\n");
+		return false;
+	}
+
+	if (!set_hbpregs_breakpoint(pid, test_func))
+		return false;
+
+	if (ptrace(PTRACE_CONT, pid, NULL, NULL) < 0) {
+		ksft_print_msg("ptrace(PTRACE_CONT) failed: %s\n",
+			strerror(errno));
+		return false;
+	}
+
+	alarm(3);
+	wpid = waitpid(pid, &status, __WALL);
+	if (wpid != pid) {
+		ksft_print_msg("waitpid() failed: %s\n", strerror(errno));
+		return false;
+	}
+	alarm(0);
+	if (WIFEXITED(status)) {
+		ksft_print_msg("child exited prematurely\n");
+		return false;
+	}
+	if (!WIFSTOPPED(status)) {
+		ksft_print_msg("child did not stop\n");
+		return false;
+	}
+	if (WSTOPSIG(status) != SIGTRAP) {
+		ksft_print_msg("child did not stop with SIGTRAP\n");
+		return false;
+	}
+	if (ptrace(PTRACE_GETSIGINFO, pid, NULL, &siginfo) != 0) {
+		ksft_print_msg("ptrace(PTRACE_GETSIGINFO): %s\n",
+			strerror(errno));
+		return false;
+	}
+	if (siginfo.si_code != TRAP_HWBKPT) {
+		ksft_print_msg("Unexpected si_code %d\n", siginfo.si_code);
+		return false;
+	}
+
+	kill(pid, SIGKILL);
+	wpid = waitpid(pid, &status, 0);
+	if (wpid != pid) {
+		ksft_print_msg("waitpid() failed: %s\n", strerror(errno));
+		return false;
+	}
+
+	ksft_print_msg("[hbpregs]: Breakpoint test passed!\n");
+	return true;
+}
+
+static void run_hbpregs_tests(void)
+{
+	run_hbpregs_bp_test();
+	run_hbpregs_wp_test();
+}
+
+/*
+ * BREAKPOINT TEST USING PTRACE_SETHBPREGS / PTRACE_GETHBPREGS - END
+ */
+static void run_ptrace_tests(void)
+{
+	run_ptrace_bp_test();
+	run_ptrace_wp_test();
+}
+
+/*
+ * BREAKPOINT TEST USING PTRACE - END
+ */
+
+/*
+ * BREAKPOINT TEST USING perf events
+ */
 static int gfd;
 sem_t ib_mtx, wp_mtx;
 static int bp_triggered, wp_triggered;
-static int test_func_sink;
 static const int wait_timeout_sec = 5;
 
 int setup_bp(bool is_x, void *addr, int sig)
@@ -56,7 +615,7 @@ int setup_bp(bool is_x, void *addr, int sig)
 
 	fd = syscall(SYS_perf_event_open, &pe, 0, -1, -1, 0);
 	if (fd < 0) {
-		printf("Failed to open event: %llx\n", pe.config);
+		ksft_print_msg("Failed to open event: %llx\n", pe.config);
 		return -1;
 	}
 
@@ -75,11 +634,10 @@ static void sig_handler_bp(int signum, siginfo_t *oh, void *uc)
 
 	bp_triggered++;
 
-	printf("Breakpoint triggered!\n");
 	ioctl(gfd, PERF_EVENT_IOC_DISABLE, 0);
 	ret = sem_post(&ib_mtx);
 	if (ret) {
-		printf("Failed to report BP success\n");
+		ksft_print_msg("Failed to report BP success\n");
 		return;
 	}
 }
@@ -88,28 +646,17 @@ static void sig_handler_wp(int signum, siginfo_t *oh, void *uc)
 {
 	int ret;
 
-	printf("Watchpoint triggered!\n");
 	ioctl(gfd, PERF_EVENT_IOC_DISABLE, 0);
 	wp_triggered++;
 
 	ret = sem_post(&wp_mtx);
 
 	if (ret) {
-		printf("Failed to report WP success\n");
+		ksft_print_msg("Failed to report WP success\n");
 		return;
 	}
 }
 
-/*
- * Keep a real instruction address for HW execute breakpoints: prevent inlining
- * and force a visible side effect so the function can't be optimized away.
- */
-static noinline void test_func(void)
-{
-	test_func_sink++;
-	__asm__ __volatile__("" : : "g" (test_func_sink));
-}
-
 static int trigger_bp(void)
 {
 	struct sigaction sa;
@@ -119,14 +666,14 @@ static int trigger_bp(void)
 	sa.sa_flags = SA_SIGINFO;
 
 	if (sigaction(SIGIO, &sa, NULL) < 0) {
-		printf("Failed to setup signal handler\n");
+		ksft_print_msg("Failed to setup signal handler\n");
 		return -1;
 	}
 
 	gfd = setup_bp(1, test_func, SIGIO);
 
 	if (gfd < 0) {
-		printf("Failed to setup breakpoint.\n");
+		ksft_print_msg("Failed to setup breakpoint.\n");
 		return -1;
 	}
 
@@ -151,14 +698,14 @@ static int trigger_wp(void)
 	sa.sa_flags = SA_SIGINFO;
 
 	if (sigaction(SIGUSR1, &sa, NULL) < 0) {
-		printf("Failed to setup signal handler\n");
+		ksft_print_msg("Failed to setup signal handler\n");
 		return -1;
 	}
 
 	gfd = setup_bp(0, &test_data, SIGUSR1);
 
 	if (gfd < 0) {
-		printf("Failed to setup watchpoint\n");
+		ksft_print_msg("Failed to setup watchpoint\n");
 		return -1;
 	}
 
@@ -204,7 +751,7 @@ int main(int argc, char *argv[])
 		return -1;
 
 	if (bp_triggered)
-		printf("Breakpoint test passed!\n");
+		ksft_print_msg("[perf_event]: Breakpoint test passed!\n");
 
 	sem_init(&wp_mtx, 0, 0);
 	if (trigger_wp() < 0)
@@ -213,7 +760,10 @@ int main(int argc, char *argv[])
 		return -1;
 
 	if (wp_triggered)
-		printf("Watchpoint test passed!\n");
+		ksft_print_msg("[perf_event]: Watchpoint test passed!\n");
+
+	run_ptrace_tests();
+	run_hbpregs_tests();
 
 	return 0;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 5/8] RISC-V: Add fetch and decode helpers to a common file
  2026-09-30  6:39 [PATCH v7 0/8] riscv: Introduce support for hardware break/watchpoints Himanshu Chauhan
                   ` (3 preceding siblings ...)
  2026-09-30  6:39 ` [PATCH v7 4/8] selftests/breakpoints: extend riscv test for ptrace hw break/watchpoints Himanshu Chauhan
@ 2026-09-30  6:39 ` Himanshu Chauhan
  2026-09-30  6:50   ` sashiko-bot
  2026-09-30  6:39 ` [PATCH v7 6/8] riscv: Add software supported single stepping with mc/mc6 triggers Himanshu Chauhan
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Himanshu Chauhan @ 2026-09-30  6:39 UTC (permalink / raw)
  To: linux-kernel, linux-riscv, linux-perf-users
  Cc: peterz, mingo, alex, aou, jtaubepe, palmer, pjw, qingfang.deng,
	shuah, thecharlesjenkins, cp0613, Himanshu Chauhan

Move get_insn() and __read_insn() from traps_misaligned.c

Add helpers for RISC-V register access and control-flow emulation

Add helpers to retrieve RISC-V register values, calcuate the next
execution address based on current pc, register states and by evaluating
conditional branch instructions. the helpers accommodate both compressed
and standard instructions. Add helper to determine is brach will be taken
by current pc. It supports register-based jumps, immediate jumps, BEQZ/BNEZ
conditional branches, and signed and unsigned comparisons for standard
conditional branch instructions.

Signed-off-by: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
Co-developed-by: Jesse Taube <jtaubepe@redhat.com>
---
 arch/riscv/include/asm/insn.h        |  15 ++
 arch/riscv/kernel/traps_misaligned.c |  52 +-----
 arch/riscv/lib/Makefile              |   1 +
 arch/riscv/lib/insn.c                | 263 +++++++++++++++++++++++++++
 4 files changed, 280 insertions(+), 51 deletions(-)
 create mode 100644 arch/riscv/lib/insn.c

diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
index c3005573e8c9..b2125728d4e9 100644
--- a/arch/riscv/include/asm/insn.h
+++ b/arch/riscv/include/asm/insn.h
@@ -227,9 +227,16 @@
 #define RVC_MASK_C_EBREAK	0xffff
 #define RVG_MASK_EBREAK		0xffffffff
 #define RVG_MASK_SRET		0xffffffff
+#define RVC_MASK_INSN		GENMASK(15, 0)
 
 #define __INSN_LENGTH_MASK	_UL(0x3)
 #define __INSN_LENGTH_GE_32	_UL(0x3)
+
+static __always_inline bool riscv_insn_is_compressed(u32 code)
+{
+	return (code & (__INSN_LENGTH_MASK)) != (__INSN_LENGTH_GE_32);
+}
+
 #define __INSN_OPCODE_MASK	_UL(0x7F)
 #define __INSN_BRANCH_OPCODE	_UL(RVG_OPCODE_BRANCH)
 
@@ -600,4 +607,12 @@ static inline void riscv_insn_insert_utype_itype_imm(u32 *utype_insn, u32 *itype
 	*utype_insn |= (imm & RV_U_IMM_31_12_MASK) + ((imm & BIT(11)) << 1);
 	*itype_insn |= ((imm & RV_I_IMM_11_0_MASK) << RV_I_IMM_11_0_OPOFF);
 }
+#ifndef __ASSEMBLY__
+#include <asm/ptrace.h>
+
+int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn);
+int get_insn_safe(struct pt_regs *regs, ulong epc, ulong *r_insn);
+unsigned long get_next_insn_address(struct pt_regs *regs, ulong insn, ulong pc);
+#endif /* __ASSEMBLY__ */
+
 #endif /* _ASM_RISCV_INSN_H */
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
index 6e8ae6c66322..541e6a5b8039 100644
--- a/arch/riscv/kernel/traps_misaligned.c
+++ b/arch/riscv/kernel/traps_misaligned.c
@@ -10,6 +10,7 @@
 #include <linux/irq.h>
 #include <linux/stringify.h>
 
+#include <asm/insn.h>
 #include <asm/processor.h>
 #include <asm/ptrace.h>
 #include <asm/csr.h>
@@ -129,57 +130,6 @@ static unsigned long get_f32_rs(unsigned long insn, u8 fp_reg_offset,
 #define GET_F32_RS2C(insn, regs) (get_f32_rs(insn, 2, regs))
 #define GET_F32_RS2S(insn, regs) (get_f32_rs(RVC_RS2S(insn), 0, regs))
 
-#define __read_insn(regs, insn, insn_addr, type)	\
-({							\
-	int __ret;					\
-							\
-	if (user_mode(regs)) {				\
-		__ret = get_user(insn, (type __user *) insn_addr); \
-	} else {					\
-		insn = *(type *)insn_addr;		\
-		__ret = 0;				\
-	}						\
-							\
-	__ret;						\
-})
-
-static inline int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn)
-{
-	ulong insn = 0;
-
-	if (epc & 0x2) {
-		ulong tmp = 0;
-
-		if (__read_insn(regs, insn, epc, u16))
-			return -EFAULT;
-		/* __get_user() uses regular "lw" which sign extend the loaded
-		 * value make sure to clear higher order bits in case we "or" it
-		 * below with the upper 16 bits half.
-		 */
-		insn &= GENMASK(15, 0);
-		if ((insn & __INSN_LENGTH_MASK) != __INSN_LENGTH_32) {
-			*r_insn = insn;
-			return 0;
-		}
-		epc += sizeof(u16);
-		if (__read_insn(regs, tmp, epc, u16))
-			return -EFAULT;
-		*r_insn = (tmp << 16) | insn;
-
-		return 0;
-	} else {
-		if (__read_insn(regs, insn, epc, u32))
-			return -EFAULT;
-		if ((insn & __INSN_LENGTH_MASK) == __INSN_LENGTH_32) {
-			*r_insn = insn;
-			return 0;
-		}
-		insn &= GENMASK(15, 0);
-		*r_insn = insn;
-
-		return 0;
-	}
-}
 
 union reg_data {
 	u8 data_bytes[8];
diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
index f668b98970bd..84cdd35afa42 100644
--- a/arch/riscv/lib/Makefile
+++ b/arch/riscv/lib/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 lib-y			+= delay.o
+lib-y			+= insn.o
 lib-y			+= memcpy.o
 lib-y			+= memset.o
 lib-y			+= memmove.o
diff --git a/arch/riscv/lib/insn.c b/arch/riscv/lib/insn.c
new file mode 100644
index 000000000000..361cac7abe10
--- /dev/null
+++ b/arch/riscv/lib/insn.c
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2026 Qualcomm Technoloies, Inc.
+ */
+
+#include <linux/uaccess.h>
+
+#include <asm/insn.h>
+#include <asm/ptrace.h>
+#include <asm/uaccess.h>
+
+/**
+ * __fetch_insn() - Fetch a RISC-V instruction parcel from memory
+ * @regs: Register state used to determine the access context
+ * @insn: Variable receiving the instruction value
+ * @insn_addr: Address from which to read the instruction parcel
+ * @type: Instruction parcel type, either @u16 or @u32
+ *
+ * Fetches an instruction parcel from user or kernel memory, depending on the
+ * execution context indicated by @regs. RISC-V instruction parcels are
+ * stored in little-endian byte order, so the fetched value is converted from
+ * little-endian to the native CPU representation before being assigned to
+ * @insn.
+ *
+ * The conversion is a no-op on little-endian targets and performs the
+ * required byte swap on big-endian targets.
+ *
+ * Return: 0 on success, or a negative error code if reading user memory
+ *         fails.
+ */
+#define __fetch_insn(regs, insn, insn_addr, type)		\
+({								\
+	type __val;						\
+	int __ret;						\
+								\
+	if (user_mode(regs))					\
+		__ret = get_user(__val,				\
+				 (type __user *)(insn_addr));	\
+	else {							\
+		__val = *(type *)(insn_addr);			\
+		__ret = 0;					\
+	}							\
+								\
+	if (!__ret) {						\
+		if (sizeof(type) == sizeof(u16))			\
+			(insn) = le16_to_cpu((__force __le16)__val);	\
+		else							\
+			(insn) = le32_to_cpu((__force __le32)__val);	\
+	}							\
+								\
+	__ret;							\
+})
+
+/**
+ * get_insn() - Fetch and decode a RISC-V instruction
+ * @regs: Register state used for instruction access
+ * @epc: Address of the instruction to fetch
+ * @r_insn: Pointer to store the fetched instruction encoding
+ *
+ * Fetches the instruction at @epc and stores its encoding in @r_insn.
+ * Both standard 32-bit instructions and compressed 16-bit instructions are
+ * supported. If a 32-bit instruction is split across two 16-bit instruction
+ * accesses, the halfwords are combined into a single instruction encoding.
+ *
+ * Return: 0 on success, or %-EFAULT if instruction access fails.
+ */
+int get_insn(struct pt_regs *regs, ulong epc, ulong *r_insn)
+{
+	ulong insn, tmp;
+
+	if (!(epc & 0x2)) {
+		if (__fetch_insn(regs, insn, epc, u32))
+			return -EFAULT;
+
+		if (riscv_insn_is_compressed(insn))
+			insn &= RVC_MASK_INSN;
+
+		*r_insn = insn;
+		return 0;
+	}
+
+	if (__fetch_insn(regs, insn, epc, u16))
+		return -EFAULT;
+
+	insn &= RVC_MASK_INSN;
+	if (riscv_insn_is_compressed(insn)) {
+		*r_insn = insn;
+		return 0;
+	}
+
+	if (__fetch_insn(regs, tmp, epc + sizeof(u16), u16))
+		return -EFAULT;
+
+	*r_insn = (tmp << 16) | insn;
+
+	return 0;
+}
+
+int get_insn_safe(struct pt_regs *regs, ulong epc, ulong *r_insn)
+{
+	int ret;
+
+	pagefault_disable();
+	ret = get_insn(regs, epc, r_insn);
+	pagefault_enable();
+
+	return ret;
+}
+
+/**
+ * riscv_get_reg_value() - Get the value stored the given RISC-V register number
+ * @regs: Register state containing the saved register values
+ * @regno: RISC-V register number
+ *
+ * Returns the value of the RISC-V register identified by @regno. Register
+ * x0 always returns zero, as required by the RISC-V ISA.
+ *
+ * Return: The register value, or zero if @regno is zero.
+ */
+static unsigned long riscv_get_reg_value(struct pt_regs *regs, unsigned int regno)
+{
+	return regno ? regs_get_register(regs, regno * sizeof(unsigned long)) : 0;
+}
+
+/**
+ * get_next_insn_address_compressed() - Calculate the next address for a compressed
+ * RISC-V instruction
+ * @regs: Register state used to evaluate indirect jumps and conditional
+ *        branches
+ * @insn: Compressed RISC-V instruction encoding
+ * @pc: Current program counter
+ *
+ * Determines the address at which execution should continue after processing
+ * the compressed instruction in @insn. Indirect jumps use the value of the
+ * instruction's source register, unconditional jumps use the encoded
+ * immediate, and conditional branches evaluate the relevant register value.
+ *
+ * For a branch that is not taken, or for an unsupported compressed
+ * instruction, the address of the following 16-bit instruction is returned.
+ *
+ * Return: The next instruction address.
+ */
+static unsigned long get_next_insn_address_compressed(struct pt_regs *regs, u32 insn,
+						      unsigned long pc)
+{
+	unsigned int rs1_num;
+
+	if (riscv_insn_is_c_jalr(insn) || riscv_insn_is_c_jr(insn)) {
+		rs1_num = RV_X(insn, RVC_C2_RS1_OPOFF, 5);
+		return regs_get_register(regs, rs1_num * sizeof(unsigned long));
+	}
+
+	if (riscv_insn_is_c_j(insn) || riscv_insn_is_c_jal(insn))
+		return RVC_EXTRACT_JTYPE_IMM(insn) + pc;
+
+	if (riscv_insn_is_c_beqz(insn)) {
+		rs1_num = RV_X(insn, RVC_C1_RS1_OPOFF, 3) + 8;
+		if (!rs1_num || riscv_get_reg_value(regs, rs1_num) == 0)
+			return RVC_EXTRACT_BTYPE_IMM(insn) + pc;
+		return pc + 2;
+	}
+
+	if (riscv_insn_is_c_bnez(insn)) {
+		rs1_num = RV_X(insn, RVC_C1_RS1_OPOFF, 3) + 8;
+		if (rs1_num && riscv_get_reg_value(regs, rs1_num) != 0)
+			return RVC_EXTRACT_BTYPE_IMM(insn) + pc;
+		return pc + 2;
+	}
+
+	return pc + 2;
+}
+
+/**
+ * riscv_branch_taken() - Determine whether a RISC-V conditional branch is taken
+ * @regs: Register state used to obtain the source operand values
+ * @insn: RISC-V branch instruction encoding
+ *
+ * Evaluates the branch condition encoded in @insn using the values of its
+ * source registers from @regs. Signed comparisons are used for BLT and BGE,
+ * while unsigned comparisons are used for BLTU and BGEU.
+ *
+ * Return: %true if the branch condition is satisfied, or %false otherwise.
+ *         Unsupported branch instructions also return %false.
+ */
+static bool riscv_branch_taken(struct pt_regs *regs, u32 insn)
+{
+	unsigned int rs1_num = RV_X(insn, RVG_RS1_OPOFF, 5);
+	unsigned int rs2_num = RV_X(insn, RVG_RS2_OPOFF, 5);
+	unsigned long rs1_val = riscv_get_reg_value(regs, rs1_num);
+	unsigned long rs2_val = riscv_get_reg_value(regs, rs2_num);
+
+	if (riscv_insn_is_beq(insn))
+		return rs1_val == rs2_val;
+	if (riscv_insn_is_bne(insn))
+		return rs1_val != rs2_val;
+	if (riscv_insn_is_blt(insn))
+		return (long)rs1_val < (long)rs2_val;
+	if (riscv_insn_is_bge(insn))
+		return (long)rs1_val >= (long)rs2_val;
+	if (riscv_insn_is_bltu(insn))
+		return rs1_val < rs2_val;
+	if (riscv_insn_is_bgeu(insn))
+		return rs1_val >= rs2_val;
+
+	return false;
+}
+
+/**
+* get_next_insn_address_standard() - Compute the next PC for standard RISC-V
+* control-flow instructions
+*
+* @regs: Register state of the current context.
+* @insn: Instruction located at @pc.
+* @pc: Address of the current instruction.
+*
+* Determine the address of the next instruction to be executed after @insn.
+* The function evaluates control-flow instructions whose target cannot be
+* obtained by simply advancing the program counter:
+*
+* - Conditional branches: returns either the branch target or @pc + 4
+* depending on the branch outcome.
+* - JAL: returns the jump target encoded in the instruction.
+* - JALR: returns the computed indirect jump target using the instruction
+* immediate and the value of the source register.
+* - SRET: returns @pc, as control transfer is handled by the trap return
+* mechanism.
+*
+* For all other instructions, execution is assumed to continue at the next
+* sequential 32-bit instruction and @pc + 4 is returned.
+*
+* Return: Address of the next instruction to be executed.
+*/
+static unsigned long get_next_insn_address_standard(struct pt_regs *regs, u32 insn,
+						    unsigned long pc)
+{
+	unsigned int rs1_num;
+
+	if ((insn & __INSN_OPCODE_MASK) == __INSN_BRANCH_OPCODE)
+		return riscv_branch_taken(regs, insn) ?
+			RV_EXTRACT_BTYPE_IMM(insn) + pc : pc + 4;
+
+	if (riscv_insn_is_jal(insn))
+		return RV_EXTRACT_JTYPE_IMM(insn) + pc;
+
+	if (riscv_insn_is_jalr(insn)) {
+		rs1_num = RV_X(insn, RVG_RS1_OPOFF, 5);
+		return RV_EXTRACT_ITYPE_IMM(insn) + riscv_get_reg_value(regs, rs1_num);
+	}
+
+	if (riscv_insn_is_sret(insn))
+		return pc;
+
+	return pc + 4;
+}
+
+/* Calculate the new address for after a step */
+unsigned long get_next_insn_address(struct pt_regs *regs, ulong insn, ulong pc)
+{
+	if (riscv_insn_is_compressed(insn))
+		return get_next_insn_address_compressed(regs, insn, pc);
+
+	return get_next_insn_address_standard(regs, insn, pc);
+}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 6/8] riscv: Add software supported single stepping with mc/mc6 triggers
  2026-09-30  6:39 [PATCH v7 0/8] riscv: Introduce support for hardware break/watchpoints Himanshu Chauhan
                   ` (4 preceding siblings ...)
  2026-09-30  6:39 ` [PATCH v7 5/8] RISC-V: Add fetch and decode helpers to a common file Himanshu Chauhan
@ 2026-09-30  6:39 ` Himanshu Chauhan
  2026-09-30  6:56   ` sashiko-bot
  2026-09-30  6:39 ` [PATCH v7 7/8] perf tests: add noinline to __test_function Himanshu Chauhan
  2026-09-30  6:39 ` [PATCH v7 8/8] MAINTAINERS: Add entry for RISC-V Debugging Himanshu Chauhan
  7 siblings, 1 reply; 16+ messages in thread
From: Himanshu Chauhan @ 2026-09-30  6:39 UTC (permalink / raw)
  To: linux-kernel, linux-riscv, linux-perf-users
  Cc: peterz, mingo, alex, aou, jtaubepe, palmer, pjw, qingfang.deng,
	shuah, thecharlesjenkins, cp0613, Himanshu Chauhan

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@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;
+
+	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


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 7/8] perf tests: add noinline to __test_function
  2026-09-30  6:39 [PATCH v7 0/8] riscv: Introduce support for hardware break/watchpoints Himanshu Chauhan
                   ` (5 preceding siblings ...)
  2026-09-30  6:39 ` [PATCH v7 6/8] riscv: Add software supported single stepping with mc/mc6 triggers Himanshu Chauhan
@ 2026-09-30  6:39 ` Himanshu Chauhan
  2026-09-30  6:45   ` sashiko-bot
  2026-09-30  6:39 ` [PATCH v7 8/8] MAINTAINERS: Add entry for RISC-V Debugging Himanshu Chauhan
  7 siblings, 1 reply; 16+ messages in thread
From: Himanshu Chauhan @ 2026-09-30  6:39 UTC (permalink / raw)
  To: linux-kernel, linux-riscv, linux-perf-users
  Cc: peterz, mingo, alex, aou, jtaubepe, palmer, pjw, qingfang.deng,
	shuah, thecharlesjenkins, cp0613, Himanshu Chauhan

In the breakpoint signal handler test, a breakpoint is set on
__test_function. Compiler may optimize and inline __test_function.
This causes failure in the test.

Add noinline to __test_function so that it is not inlined.

Signed-off-by: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
---
 tools/perf/tests/bp_signal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/bp_signal.c b/tools/perf/tests/bp_signal.c
index f580ba7486b1..cc4a27224884 100644
--- a/tools/perf/tests/bp_signal.c
+++ b/tools/perf/tests/bp_signal.c
@@ -54,7 +54,7 @@ asm (
 	"ret\n"
 	".popsection\n");
 #else
-static void __test_function(volatile long *ptr)
+static noinline void __test_function(volatile long *ptr)
 {
 	*ptr = 0x1234;
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v7 8/8] MAINTAINERS: Add entry for RISC-V Debugging
  2026-09-30  6:39 [PATCH v7 0/8] riscv: Introduce support for hardware break/watchpoints Himanshu Chauhan
                   ` (6 preceding siblings ...)
  2026-09-30  6:39 ` [PATCH v7 7/8] perf tests: add noinline to __test_function Himanshu Chauhan
@ 2026-09-30  6:39 ` Himanshu Chauhan
  7 siblings, 0 replies; 16+ messages in thread
From: Himanshu Chauhan @ 2026-09-30  6:39 UTC (permalink / raw)
  To: linux-kernel, linux-riscv, linux-perf-users
  Cc: peterz, mingo, alex, aou, jtaubepe, palmer, pjw, qingfang.deng,
	shuah, thecharlesjenkins, cp0613, Himanshu Chauhan

Added myself as maintainer for the RISC-V inline debugging
with sdtrig extension

Signed-off-by: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
---
 MAINTAINERS | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 72294ddfa5b7..01631359f0ee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -23586,6 +23586,17 @@ F:	drivers/irqchip/irq-riscv-rpmi-sysmsi.c
 F:	drivers/mailbox/riscv-sbi-mpxy-mbox.c
 F:	include/linux/mailbox/riscv-rpmi-message.h
 
+RISC-V DEBUGGING
+M:	Himanshu Chauhan <himanshu@thechauhan.dev>
+L:	linux-riscv@lists.infradead.org
+F:	arch/riscv/include/asm/hw_breakpoint.h
+F:	arch/riscv/include/asm/insn.h
+F:	arch/riscv/include/asm/perf_event.h
+F:	arch/riscv/kernel/hw_breakpoint.c
+F:	arch/riscv/kernel/perf_*
+F:	arch/riscv/lib/insn.c
+F:	drivers/perf/
+
 RISC-V SPACEMIT SoC Support
 M:	Yixun Lan <dlan@kernel.org>
 L:	linux-riscv@lists.infradead.org
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 7/8] perf tests: add noinline to __test_function
  2026-09-30  6:39 ` [PATCH v7 7/8] perf tests: add noinline to __test_function Himanshu Chauhan
@ 2026-09-30  6:45   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-09-30  6:45 UTC (permalink / raw)
  To: Himanshu Chauhan; +Cc: linux-perf-users

> In the breakpoint signal handler test, a breakpoint is set on
> __test_function. Compiler may optimize and inline __test_function.
> This causes failure in the test.
> 
> Add noinline to __test_function so that it is not inlined.
> 
> Signed-off-by: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930063919.608911-1-himanshu.chauhan@oss.qualcomm.com?part=7


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 2/8] riscv: Add breakpoint and watchpoint test for riscv
  2026-09-30  6:39 ` [PATCH v7 2/8] riscv: Add breakpoint and watchpoint test for riscv Himanshu Chauhan
@ 2026-09-30  6:49   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-09-30  6:49 UTC (permalink / raw)
  To: Himanshu Chauhan; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] selftests/breakpoints: Makefile fails to match riscv64 native architecture
- [Low] selftests/breakpoints: global CFLAGS += -static affects other tests
- [Low] selftests/breakpoints: file descriptor leak in trigger_wp()
- [Low] selftests/breakpoints: shared variables lack volatile sig_atomic_t qualifier
--

commit d5a08206a858d8d5760badeff5fe2c51c8a98f2b
Author: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
Subject: riscv: Add breakpoint and watchpoint test for riscv

This commit adds a selftest for the RISC-V architecture using the ptrace
framework. It sets and unsets breakpoints and watchpoints, and uses signals 
to verify the triggers.

> diff --git a/tools/testing/selftests/breakpoints/Makefile b/tools/testing/selftests/breakpoints/Makefile
> index 0b8f5acf7c78f..b32c56b3c1db2 100644
> --- a/tools/testing/selftests/breakpoints/Makefile
> +++ b/tools/testing/selftests/breakpoints/Makefile
> @@ -12,5 +12,10 @@ ifneq (,$(filter $(ARCH),aarch64 arm64))
>  TEST_GEN_PROGS += breakpoint_test_arm64
>  endif
>  
> +ifneq (,$(filter $(ARCH),riscv))

[Severity: Medium]
Will this test be silently skipped during a native build on a RISC-V machine?

The native architecture reported by uname -m is "riscv64", but this filter
expects exactly "riscv". Since this leaf Makefile doesn't include the
kernel's subarch.include to normalize the architecture name, it looks like it
might fail to match the native architecture.

> +CFLAGS += -static $(KHDR_INCLUDES)

[Severity: Low]
Does appending to the global CFLAGS here force other unrelated tests in this
directory to inadvertently build statically on RISC-V?

Using a target-specific flag just for breakpoint_test_riscv might prevent
impacting tests like step_after_suspend_test.

> +TEST_GEN_PROGS += breakpoint_test_riscv
> +endif
> +
>  include ../lib.mk

[ ... ]

> diff --git a/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c b/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c
> new file mode 100644
> index 0000000000000..0649940b709e0
> --- /dev/null
> +++ b/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c
> @@ -0,0 +1,219 @@

[ ... ]

> +#ifndef noinline
> +#define noinline __attribute__((noinline))
> +#endif
> +
> +static int gfd;
> +sem_t ib_mtx, wp_mtx;
> +static int bp_triggered, wp_triggered;

[Severity: Low]
Can the compiler optimize and incorrectly cache these variables since they
are modified asynchronously in a signal handler but lack the volatile
sig_atomic_t qualifiers?

They are incremented in sig_handler_bp() and sig_handler_wp(), so standard
POSIX signal safety might require the proper qualifiers to avoid potential
hangs or incorrect test failures.

> +static int test_func_sink;
> +static const int wait_timeout_sec = 5;

[ ... ]

> +static int trigger_wp(void)
> +{
> +	struct sigaction sa;
> +	unsigned long test_data;

[ ... ]

> +	if (gfd < 0) {
> +		printf("Failed to setup watchpoint\n");
> +		return -1;
> +	}
> +
> +	ioctl(gfd, PERF_EVENT_IOC_ENABLE, 0);
> +	test_data = 0xdeadbeef;
> +	ioctl(gfd, PERF_EVENT_IOC_DISABLE, 0);
> +
> +	return 0;
> +}

[Severity: Low]
Does this success path leak the file descriptor allocated by setup_bp()?

It looks like close(gfd) is correctly called on the success path in
trigger_bp(), but appears to be missing here before returning.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930063919.608911-1-himanshu.chauhan@oss.qualcomm.com?part=2

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 5/8] RISC-V: Add fetch and decode helpers to a common file
  2026-09-30  6:39 ` [PATCH v7 5/8] RISC-V: Add fetch and decode helpers to a common file Himanshu Chauhan
@ 2026-09-30  6:50   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-09-30  6:50 UTC (permalink / raw)
  To: Himanshu Chauhan; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] riscv: raw pointer dereference in get_insn_safe() causes Oops
- [Medium] riscv: missing LSB clearing in JALR target address calculation
--

commit b41a38b921ab3b79d54f05b386dc929fbff4659a
Author: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>

RISC-V: Add fetch and decode helpers to a common file

This commit adds helper functions for fetching and decoding RISC-V
instructions. It also introduces functions to retrieve register values and
calculate the next execution address based on the current PC, register states,
and control-flow instructions.

> diff --git a/arch/riscv/lib/insn.c b/arch/riscv/lib/insn.c
> new file mode 100644
> index 0000000000000..361cac7abe108
> --- /dev/null
> +++ b/arch/riscv/lib/insn.c
> @@ -0,0 +1,263 @@
[ ... ]
> +#define __fetch_insn(regs, insn, insn_addr, type)		\
> +({								\
> +	type __val;						\
> +	int __ret;						\
> +								\
> +	if (user_mode(regs))					\
> +		__ret = get_user(__val,				\
> +				 (type __user *)(insn_addr));	\
> +	else {							\
> +		__val = *(type *)(insn_addr);			\
> +		__ret = 0;					\
> +	}							\
> +								\
[ ... ]
> +int get_insn_safe(struct pt_regs *regs, ulong epc, ulong *r_insn)
> +{
> +	int ret;
> +
> +	pagefault_disable();
> +	ret = get_insn(regs, epc, r_insn);
> +	pagefault_enable();
> +
> +	return ret;
> +}

[Severity: High]
Does this raw pointer dereference in __fetch_insn() actually safely handle
faults on kernel addresses?

While get_insn_safe() disables page faults, this does not make unmapped memory
accesses safe. It just forces the fault handler to directly look up exception
tables and panic if none are found.

Because __fetch_insn() performs a raw C pointer dereference *(type *)(insn_addr)
without an exception table entry, any invalid kernel address will cause an
immediate kernel panic (Oops) instead of safely returning an error.

Should this use get_kernel_nofault() for safely reading arbitrary kernel memory?

[ ... ]
> +static unsigned long get_next_insn_address_compressed(struct pt_regs *regs, u32 insn,
> +						      unsigned long pc)
> +{
> +	unsigned int rs1_num;
> +
> +	if (riscv_insn_is_c_jalr(insn) || riscv_insn_is_c_jr(insn)) {
> +		rs1_num = RV_X(insn, RVC_C2_RS1_OPOFF, 5);
> +		return regs_get_register(regs, rs1_num * sizeof(unsigned long));
> +	}

[Severity: Medium]
Will this fail to clear the least-significant bit (LSB) of the target address?

The RISC-V ISA specification mandates that the target address for C.JALR and
C.JR instructions has its LSB set to zero by hardware. Since this function
does not apply & ~1UL to the register value, if the source register has the LSB
set, it will incorrectly return an odd (unaligned) address.

Subsystems relying on this for single-stepping might set breakpoints at the odd
address, which would fail to trigger when the CPU actually executes the aligned
target.

[ ... ]
> +static unsigned long get_next_insn_address_standard(struct pt_regs *regs, u32 insn,
> +						    unsigned long pc)
> +{
> +	unsigned int rs1_num;
> +
> +	if ((insn & __INSN_OPCODE_MASK) == __INSN_BRANCH_OPCODE)
> +		return riscv_branch_taken(regs, insn) ?
> +			RV_EXTRACT_BTYPE_IMM(insn) + pc : pc + 4;
> +
> +	if (riscv_insn_is_jal(insn))
> +		return RV_EXTRACT_JTYPE_IMM(insn) + pc;
> +
> +	if (riscv_insn_is_jalr(insn)) {
> +		rs1_num = RV_X(insn, RVG_RS1_OPOFF, 5);
> +		return RV_EXTRACT_ITYPE_IMM(insn) + riscv_get_reg_value(regs, rs1_num);
> +	}

[Severity: Medium]
Does this calculation also need to clear the LSB?

Similar to the compressed JALR handling above, JALR requires its target address
LSB to be cleared. Without applying & ~1UL, calculating the target address here
could also return an unaligned address if the LSB is set, breaking debug
control.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930063919.608911-1-himanshu.chauhan@oss.qualcomm.com?part=5

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 4/8] selftests/breakpoints: extend riscv test for ptrace hw break/watchpoints
  2026-09-30  6:39 ` [PATCH v7 4/8] selftests/breakpoints: extend riscv test for ptrace hw break/watchpoints Himanshu Chauhan
@ 2026-09-30  6:51   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-09-30  6:51 UTC (permalink / raw)
  To: Himanshu Chauhan; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] selftests: missing _exit(0) causes child to execute parent test logic
- [High] selftests: test failures are silently ignored causing false positive passes
- [Medium] selftests: orphaned stopped child processes on test error paths
- [Medium] selftests: unhandled SIGALRM causes abrupt test termination leaving orphaned tracees
- [Low] selftests: hardware breakpoint readback lacks programmatic assertions
- [Low] selftests: commit message claims removal of code not present in diff
- [Low] selftests: size parameter in set_watchpoint is ignored
--

commit dad9f82e0eab9aaebe66d40b3104c52fdbe939dc
Author: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>

selftests/breakpoints: extend riscv test for ptrace hw break/watchpoints

This commit extends the RISC-V breakpoint selftests to cover the new
ptrace-based hardware breakpoint and watchpoint support. It exercises
both the PTRACE_GETREGSET/SETREGSET and PTRACE_GETHBPREGS/SETHBPREGS
interfaces.

> Also drop a leftover #if 0 block duplicating the HWDEBUG_* control
> field macros already defined in uapi/asm/ptrace.h.

[Severity: Low]
This isn't a bug in the code, but does the provided patch actually
include this removal? Looking at the diff, it doesn't seem to contain
any modifications removing an #if 0 block, so the patch contents might
have desynced from the commit message.

> diff --git a/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c b/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c
> index 0649940b709e0..13d6b3ff1b6ee 100644
> --- a/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c
> +++ b/tools/testing/selftests/breakpoints/breakpoint_test_riscv.c
[ ... ]
> +static int do_bp_child(void (*bp_func)(void))
> +{
> +	if (ptrace(PTRACE_TRACEME, 0, NULL, NULL) != 0) {
> +		ksft_print_msg(
> +			       "ptrace(PTRACE_TRACEME) failed: %s\n",
> +			       strerror(errno));
> +		_exit(1);
> +	}
> +
> +	if (raise(SIGSTOP) != 0) {
> +		ksft_print_msg(
> +			       "raise(SIGSTOP) failed: %s\n", strerror(errno));
> +		_exit(1);
> +	}
> +
> +	bp_func();
> +}

[Severity: High]
Does this child process need to terminate with _exit(0) here? Since it
doesn't exit, when the child returns from bp_func(), it will break out
of the child context and begin executing the parent's logic in
run_ptrace_bp_test(). This causes the child to wrongly issue waitpid()
and ptrace() commands on itself.

[ ... ]
> +static bool set_watchpoint(pid_t pid, int size)
> +{
> +	uint8_t *addr = (uint8_t *)&var;
> +	unsigned int control = 0;
> +	struct user_hwdebug_state dreg_state;
> +	struct iovec iov;
> +
> +	/* Write watchpoint */
> +	control = (HW_BREAKPOINT_W << 14) & ((0x7 << 14));
> +	control |= (HW_BREAKPOINT_LEN_8 << 4) & ((0x1f << 4));

[Severity: Low]
Is it intentional that the size parameter passed to set_watchpoint() is
ignored here? The code hardcodes HW_BREAKPOINT_LEN_8 into the control bits,
unconditionally forcing the watchpoint size to 8 bytes regardless of the
caller's request.

[ ... ]
> +static int run_ptrace_wp_test(void)
> +{
[ ... ]
> +	if (!set_watchpoint(pid, 8))
> +		return false;
> +
> +	if (ptrace(PTRACE_CONT, pid, NULL, NULL) < 0) {
> +		ksft_print_msg(
> +			"ptrace(PTRACE_CONT) failed: %s\n",
> +			strerror(errno));
> +		return false;
> +	}

[Severity: Medium]
If the test returns false early here, does it leak the stopped child
process? The child process is already blocked in raise(SIGSTOP), and by
returning early, the test runner bypasses the kill(pid, SIGKILL) cleanup
block at the end of the function, which could leave orphaned tracees
consuming system resources.

> +
> +	alarm(3);
> +	wpid = waitpid(pid, &status, __WALL);

[Severity: Medium]
If waitpid() blocks for 3 seconds (e.g., if the child never hits the
breakpoint), will the resulting SIGALRM abruptly terminate the test
suite? Because no signal handler is registered for SIGALRM, the default
action would kill the test process outright, bypassing test framework
reporting and leaving any traced children orphaned.

[ ... ]
> +static bool set_hbpregs_watchpoint(pid_t pid)
> +{
[ ... ]
> +	/* Read back and verify */
> +	memset(&state, 0, sizeof(state));
> +	if (ptrace(PTRACE_GETHBPREGS, pid, 0, &state) != 0) {
> +		ksft_print_msg(
> +			"ptrace(PTRACE_GETHBPREGS) failed: %s\n",
> +			strerror(errno));
> +		return false;
> +	}
> +
> +	ksft_print_msg(
> +		"[hbpregs] watchpoint readback: addr=0x%lx type=%lu len=%lu ctrl=%lu\n",
> +		state.addr, state.type, state.len, state.ctrl);
> +
> +	return true;
> +}

[Severity: Low]
Should there be a programmatic assertion to validate the readback state?
The code currently prints the state to stdout, but doesn't check if the
returned addr, type, len, and ctrl actually match what was written.
Without a check, regressions in breakpoint state persistence might go
unnoticed by automated tests.

[ ... ]
> +static void run_ptrace_tests(void)
> +{
> +	run_ptrace_bp_test();
> +	run_ptrace_wp_test();
> +}

[Severity: High]
Since run_ptrace_bp_test() and run_ptrace_wp_test() return boolean
success values, does run_ptrace_tests() need to propagate these
results? Dropping them here masks test failures.

> +
> +/*
> + * BREAKPOINT TEST USING PTRACE - END
> + */
> +
[ ... ]
> @@ -213,7 +760,10 @@ int main(int argc, char *argv[])
>  		return -1;
>  
>  	if (wp_triggered)
> -		printf("Watchpoint test passed!\n");
> +		ksft_print_msg("[perf_event]: Watchpoint test passed!\n");
> +
> +	run_ptrace_tests();
> +	run_hbpregs_tests();
>  
>  	return 0;
>  }

[Severity: High]
Does main() need to return a non-zero exit code if the new tests fail?
Currently it unconditionally returns 0 at the end of execution, which
means CI systems will report a successful pass even if the new ptrace or
hbpregs tests completely fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930063919.608911-1-himanshu.chauhan@oss.qualcomm.com?part=4

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 3/8] riscv: ptrace support for hardware break/watchpoints
  2026-09-30  6:39 ` [PATCH v7 3/8] riscv: ptrace support for hardware break/watchpoints Himanshu Chauhan
@ 2026-09-30  6:56   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-09-30  6:56 UTC (permalink / raw)
  To: Himanshu Chauhan; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] riscv/ptrace: HW breakpoints leaked across execve() via PTRACE_SETHBPREGS
- [High] riscv/ptrace: PTRACE_GETREGSET fails with -EINVAL when triggers > 16
- [High] riscv/ptrace: kernel stack leak via uninitialized memory in riscv_hw_break_set
- [Medium] riscv/ptrace: missing hardware breakpoint support for compat processes
- [Medium] riscv/ptrace: SETHBPREGS and SETREGSET operate on disjoint breakpoint arrays
- [Medium] riscv/ptrace: missing array_index_nospec in riscv_ptrace_bp_get/set
--

commit 62c2c75a3fb752fd74e38e9ad58187d943832421
Author: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>

riscv: ptrace support for hardware break/watchpoints

This commit adds ptrace support for hardware breakpoints and watchpoints
on RISC-V. It introduces NT_RISCV_HW_BREAK/WATCH for GETREGSET/SETREGSET
and a convenience request pair PTRACE_GETHBPREGS/SETHBPREGS.

> diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
> index f336a183667eb..2d4ee51e68597 100644
> --- a/arch/riscv/kernel/ptrace.c
> +++ b/arch/riscv/kernel/ptrace.c

[ ... ]

> @@ -372,6 +378,397 @@ static int riscv_cfi_set(struct task_struct *target,
> +void flush_ptrace_hw_breakpoint(struct task_struct *tsk)
> +{
> +	int i;
> +	struct thread_struct *t = &tsk->thread;
> +
> +	for (i = 0; i < RISCV_MAX_BP; i++) {
> +		if (t->debug.hbp_break[i]) {
> +			unregister_hw_breakpoint(t->debug.hbp_break[i]);
> +			t->debug.hbp_break[i] = NULL;
> +		}
> +	}

[Severity: High]
Does this code leak hardware breakpoints across execve()?

PTRACE_SETHBPREGS stores hardware breakpoints in child->thread.ptrace_bps,
but this function only unregisters breakpoints in debug.hbp_break and
completely ignores ptrace_bps. If the tracee executes a SUID binary, do the
hardware breakpoints remain active in the privileged context?

[ ... ]

> +static int ptrace_hbp_set_event(unsigned int note_type,
> +				struct task_struct *tsk,
> +				unsigned long idx,
> +				struct perf_event *bp)
> +{
> +	int err = -EINVAL;
> +
> +	switch (note_type) {
> +	case NT_RISCV_HW_BREAK:
> +		if (idx >= RISCV_MAX_BP)
> +			goto out;
> +		idx = array_index_nospec(idx, RISCV_MAX_BP);
> +		tsk->thread.debug.hbp_break[idx] = bp;

[Severity: Medium]
Are PTRACE_SETHBPREGS and PTRACE_SETREGSET operating on completely disjoint
memory locations?

PTRACE_SETREGSET populates tsk->thread.debug.hbp_break, while PTRACE_SETHBPREGS
populates tsk->thread.ptrace_bps. Can a debugger using both APIs independently
allocate up to 16 breakpoints in each array, potentially consuming the entire
hardware trigger pool and observing disconnected states?

[ ... ]

> +static int ptrace_hbp_get_resource_info(unsigned int note_type, u32 *info)
> +{
> +	u8 num;
> +
> +	switch (note_type) {
> +	case NT_RISCV_HW_BREAK:
> +		num = hw_breakpoint_slots(TYPE_INST);
> +		break;

[Severity: High]
Can hw_breakpoint_slots() return a value larger than 16 (RISCV_MAX_BP)?

If a platform has more than 16 triggers, riscv_hw_break_get() and
riscv_hw_break_set() will iterate up to that number. However, the accessor
ptrace_hbp_get_event() contains a hardcoded bounds check returning -EINVAL
if idx >= RISCV_MAX_BP.

Would this cause the entire PTRACE_GETREGSET/SETREGSET syscall to abort with
-EINVAL when hitting index 16 on such platforms?

[ ... ]

> +static int riscv_hw_break_set(struct task_struct *target,
> +			const struct user_regset *regset,
> +			unsigned int pos, unsigned int count,
> +			const void *kbuf, const void __user *ubuf)
> +{
> +	unsigned int note_type = regset->core_note_type;
> +	int ret, idx = 0, offset, limit;
> +	u32 ctrl;

[Severity: High]
Is ctrl properly initialized before use?

user_regset_copyin() allows partial copies if count is smaller than the target
size. If a tracer supplies an iov_len of 9 or 17, user_regset_copyin() will
copy only 1 byte into ctrl, leaving the remaining 3 bytes containing
uninitialized kernel stack data.

When this partially initialized ctrl is passed to ptrace_hbp_set_ctrl() and
later read out via PTRACE_GETREGSET, could it successfully leak uninitialized
kernel stack memory to userspace?

[ ... ]

> +static int riscv_ptrace_bp_get(struct task_struct *child, unsigned long idx,
> +			  struct __riscv_hwdebug_state *state)
> +{
> +	struct perf_event *bp;
> +
> +	if (idx >= RISCV_HW_BP_NUM_MAX)
> +		return -EINVAL;
> +
> +	bp = child->thread.ptrace_bps[idx];

[Severity: Medium]
Might an array_index_nospec() mitigation be needed here to prevent speculative
out-of-bounds reads?

The user-supplied index idx is bounds-checked, but without array_index_nospec(),
speculative execution can bypass the bounds check and read out-of-bounds from
the ptrace_bps array. The resulting pointer is immediately dereferenced, which
could provide an arbitrary speculative load gadget.

[ ... ]

Since compat_riscv_user_regset and compat_arch_ptrace() were not modified in
this patch:

arch/riscv/kernel/ptrace.c:compat_riscv_user_regset[] = {
...
arch/riscv/kernel/ptrace.c:compat_arch_ptrace() {

[Severity: Medium]
Will 32-bit processes running on a 64-bit kernel be able to use hardware
breakpoints?

These additions were omitted from compat_riscv_user_regset and
compat_arch_ptrace(), which seems to break feature parity for compat tracing.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930063919.608911-1-himanshu.chauhan@oss.qualcomm.com?part=3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 6/8] riscv: Add software supported single stepping with mc/mc6 triggers
  2026-09-30  6:39 ` [PATCH v7 6/8] riscv: Add software supported single stepping with mc/mc6 triggers Himanshu Chauhan
@ 2026-09-30  6:56   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-09-30  6:56 UTC (permalink / raw)
  To: Himanshu Chauhan; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [Critical] riscv/hw_breakpoint: Unprivileged user can arm HW breakpoints on kernel addresses (DoS)
- [High] riscv/hw_breakpoint: HW breakpoint permanently lost on control flow divergence
- [High] riscv/hw_breakpoint: infinite trap loop if get_insn_safe fails
- [High] riscv/hw_breakpoint: setup_singlestep overrides synchronous event disablement
- [High] riscv/hw_breakpoint: CPU-wide breakpoints permanently lost on task migration
- [Medium] riscv/hw_breakpoint: state machine corruption on install failure
- [Medium] riscv/hw_breakpoint: infinite loop when single-stepping sret
- [Low] riscv/hw_breakpoint: saved_tdata fields are completely unused
- [Low] riscv/hw_breakpoint: incorrect next_addr logged due to loop variable shadowing
- [Low] riscv/hw_breakpoint: redundant bit clearing on zero-initialized fields
--

commit 39e397c6cb04690cc3a8477d767b61ec32e69fda
Author: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
Subject: riscv: Add software supported single stepping with mc/mc6 triggers

This commit adds software-supported single-stepping for mc/mc6 triggers on 
RISC-V. It attempts to avoid loops by temporarily disabling the current 
breakpoint, setting a new execute breakpoint at the next instruction's 
address, and restoring the original breakpoint once the next instruction is hit.

> diff --git a/arch/riscv/kernel/hw_breakpoint.c b/arch/riscv/kernel/hw_breakpoint.c
> index dd6f639539691..9597ed3625378 100644
> --- a/arch/riscv/kernel/hw_breakpoint.c
> +++ b/arch/riscv/kernel/hw_breakpoint.c
[ ... ]
> +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;

[Severity: Low]
Are these saved values actually consumed? The restore logic in
hw_breakpoint_handler() seems to ignore them and completely recreates the
state from scratch via hw_breakpoint_arch_parse().

> +
> +	ret = get_insn_safe(regs, regs->epc, &insn);
> +	if (ret < 0)
> +		return ret;

[Severity: High]
What happens if get_insn_safe() fails (for example, if the user space
instruction page is swapped out, causing get_insn() to fail under
pagefault_disable())?

Since hw_breakpoint_handler() logs the error and returns NOTIFY_DONE without
advancing the PC or disabling the execute trigger, will the CPU immediately
re-trap on the exact same instruction upon resuming, leading to an infinite
loop?

> +
> +	next_addr = get_step_address(regs, insn);

[Severity: Medium]
When single-stepping an sret instruction, the underlying call to
get_next_insn_address_standard() calculates the step target to be the
current instruction itself. Since this configures an execute breakpoint
at the same address, does this cause an infinite loop where the CPU
repeatedly traps before executing the sret?

> +
> +	/*
> +	 * 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);

[Severity: Low]
Since tmp.tdata1 is explicitly initialized to 0 above, are these
CLEAR_DBTR_BIT operations necessary? 

Also, does clearing the ACTION_BIT this way correctly handle the multi-bit
nature of this field, or does it only clear the lowest 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);

[Severity: Critical]
Does this allow an unprivileged user to arm hardware breakpoints on kernel
addresses? 

Since this unconditionally sets both the Supervisor and User bits and
bypasses arch_check_bp_in_kernelspace(), could a user place a user-space
breakpoint on a jalr instruction and set the target register to a critical
kernel address (like the stvec trap vector)? If the hardware trigger fires
in Supervisor mode, could this instantly lock up the CPU in a recursive
breakpoint exception loop?

[ ... ]
> +	arch_update_hw_breakpoint(event);
> +
> +	bp->in_callback = true;
> +	bp->next_addr = next_addr;
> +	return 0;
> +}

[Severity: High]
How does this interact with CPU-wide hardware breakpoints (such as
perf record -a)? 

If the currently running task migrates to another CPU before reaching
next_addr, does the original CPU get permanently stuck waiting for an
instruction it will never execute, effectively losing the CPU-wide
breakpoint?

[ ... ]
>  static int hw_breakpoint_handler(struct die_args *args)
>  {
[ ... ]
> +		if (bp->in_callback) {
> +			expecting_callback = true;
> +			if (args->regs->epc != bp->next_addr)
> +				continue;

[Severity: High]
If execution diverges before next_addr is reached (for instance, due to an
interrupt, a signal handler, or task migration), args->regs->epc will not
match next_addr. Since the handler just bails out here, is the original
breakpoint permanently lost? Is there a fallback mechanism to recover the
lost trigger?

> +
> +			arch_uninstall_hw_breakpoint(event);
> +
> +			/* Restore original breakpoint */
> +			if (hw_breakpoint_arch_parse(NULL, &event->attr, bp))
> +				goto exit;
> +
> +			if (arch_install_hw_breakpoint(event))
> +				goto exit;

[Severity: Medium]
If arch_install_hw_breakpoint() fails here, we jump to exit without clearing
bp->in_callback = false. Will this leave the perf event alive in the software
state but corrupted for any future re-enabling?

> +
> +			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);

[Severity: High]
Can setup_singlestep() override a synchronous event disablement?

If perf_bp_event() triggers the perf PMU to synchronously disable the event
(e.g., if a sample limit is reached), setup_singlestep() executes immediately
afterwards. Since it unconditionally sets the enable bits and calls
arch_update_hw_breakpoint(), does this forcefully re-enable the breakpoint in
hardware in single-step mode, overriding the request to disable it?

[ ... ]
>  	}
>  
> -	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);
> +	}

[Severity: Low]
At this point outside the loop, doesn't 'bp' hold the value of the last
event processed in the array, rather than the event that actually had
in_callback == true? Will this output an unrelated next_addr value in the
error log?

> +exit:
> +	return bp_ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930063919.608911-1-himanshu.chauhan@oss.qualcomm.com?part=6

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v7 1/8] riscv: Introduce support for hardware break/watchpoints
  2026-09-30  6:39 ` [PATCH v7 1/8] " Himanshu Chauhan
@ 2026-09-30  6:58   ` sashiko-bot
  0 siblings, 0 replies; 16+ messages in thread
From: sashiko-bot @ 2026-09-30  6:58 UTC (permalink / raw)
  To: Himanshu Chauhan; +Cc: linux-perf-users

Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [High] perf/riscv: deadlock and IRQ state corruption via per-CPU lock flags
- [High] perf/riscv: hardware trigger hit bit is not cleared causing stuck triggers
- [High] perf/riscv: arch_update_hw_breakpoint passes zero count and corrupts memory
- [High] perf/riscv: use-after-free of sbi_dbtr_shmem on hotplug setup failure
- [Medium] perf/riscv: hardware triggers are leaked on software allocation failure
- [Medium] perf/riscv: integer underflow in arch_check_bp_in_kernelspace
- [Medium] perf/riscv: race condition in arch_uninstall_hw_breakpoint crashes kernel
- [Medium] perf/riscv: uninitialized tstate passed to hypervisor on trigger install
- [Low] perf/riscv: arch_hw_breakpoint_init returns positive integer on success
--

commit 43e04941130b3d79825bf1347e698fc0ed2e7a0d
Author: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>

riscv: Introduce support for hardware break/watchpoints

This commit introduces hardware breakpoint and watchpoint support for RISC-V.
It builds on the perf subsystem and uses the SBI debug trigger extension to
manage hardware triggers.

> diff --git a/arch/riscv/kernel/hw_breakpoint.c b/arch/riscv/kernel/hw_breakpoint.c
> --- /dev/null
> +++ b/arch/riscv/kernel/hw_breakpoint.c
> @@ -0,0 +1,746 @@
[ ... ]
> +/* Registered per-cpu bp/wp */
> +static DEFINE_PER_CPU(struct perf_event *, pcpu_hw_bp_events[RISCV_HW_BP_NUM_MAX]);
> +static DEFINE_PER_CPU(unsigned long, ecall_lock_flags);

[Severity: High]
Does defining ecall_lock_flags as a global per-CPU variable cause IRQ state
corruption? The raw_spin_lock_irqsave() macro expects a local stack variable
to save the IRQ state, but this variable is shared globally across the CPU.

> +static DEFINE_PER_CPU(raw_spinlock_t, ecall_lock);
[ ... ]
> +int arch_check_bp_in_kernelspace(struct arch_hw_breakpoint *hw)
> +{
> +	unsigned int len;
> +	unsigned long va;
> +
> +	va = hw->address;
> +	len = hw->len;
> +
> +	return (va >= TASK_SIZE) && ((va + len - 1) >= TASK_SIZE);

[Severity: Medium]
For execute breakpoints, len is 0. Does va + len - 1 underflow, causing the
bounds check to wrap around when added to a 64-bit kernel address? This
might evaluate to less than TASK_SIZE and erroneously return false.

> +}
[ ... ]
> +static int hw_breakpoint_handler(struct die_args *args)
> +{
> +	int ret = NOTIFY_DONE;
> +	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]);
> +		if (!event)
> +			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;

[Severity: High]
If the address matches, this short-circuit logic prevents calling
mc6_read_and_clear_hit(i). Does skipping this leave the hardware hit bit
stuck at 1, causing endless spurious traps on unrelated exceptions?

> +
> +			if (hit) {
> +				perf_bp_event(event, args->regs);
> +				ret = NOTIFY_STOP;
> +			}
> +			break;
> +		}
[ ... ]
> +			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));

[Severity: High]
If a synchronous exception like a hardware breakpoint occurs while ecall_lock
is already held, this trap handler will attempt to acquire the exact same
non-recursive local lock and overwrite ecall_lock_flags. Could this result
in an immediate inescapable deadlock?

> +				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);

[Severity: High]
For MCONTROL triggers, the hit bit is read here, but it doesn't appear to be
cleared in tdata1 or updated via SBI_EXT_DBTR_TRIG_UPDATE. Does this leave
the hardware state corrupted with the hit bit permanently set?

> +				}
> +				raw_spin_unlock_irqrestore(this_cpu_ptr(&ecall_lock),
> +							   *this_cpu_ptr(&ecall_lock_flags));
> +			}
[ ... ]
> +/* atomic: counter->ctx->lock is held */
> +int arch_install_hw_breakpoint(struct perf_event *event)
> +{
> +	struct arch_hw_breakpoint *bp = counter_arch_bp(event);
> +	union sbi_dbtr_shmem_entry *shmem = this_cpu_ptr(sbi_dbtr_shmem);
> +	struct sbi_dbtr_data_msg *xmit;
> +	struct sbi_dbtr_id_msg *recv;
> +	struct perf_event **slot;
> +	unsigned long idx;
> +	struct sbiret ret;
> +	int err = 0;
> +
> +	raw_spin_lock_irqsave(this_cpu_ptr(&ecall_lock),
> +			      *this_cpu_ptr(&ecall_lock_flags));
> +
> +	xmit = &shmem->data;
> +	recv = &shmem->id;
> +	xmit->tdata1 = cpu_to_le(bp->tdata1);
> +	xmit->tdata2 = cpu_to_le(bp->tdata2);
> +	xmit->tdata3 = cpu_to_le(bp->tdata3);

[Severity: Medium]
Because shmem->id and shmem->data share the same memory via a union, does
xmit->tstate need to be initialized here? Otherwise, it might retain the
trigger index from previous recv->idx calls and pass uninitialized garbage
to the hypervisor as the trigger configuration state.

> +
> +	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIG_INSTALL,
> +			1, 0, 0, 0, 0, 0);
> +
> +	if (ret.error) {
> +		pr_warn("Failed to install trigger\n");
> +		err = sbi_err_map_linux_errno(ret.error);
> +		goto done;
> +	}
> +
> +	idx = le_to_cpu(recv->idx);
> +	if (idx >= dbtr_total_num) {
> +		pr_warn("Invalid trigger index %lu\n", idx);
> +		err = -EINVAL;
> +		goto done;

[Severity: Medium]
If this software validation fails, the error path jumps to done without
issuing an SBI_EXT_DBTR_TRIG_UNINSTALL. Does this permanently leak the
successfully allocated hardware trigger?

> +	}
> +
> +	slot = this_cpu_ptr(&pcpu_hw_bp_events[idx]);
> +	if (*slot) {
> +		pr_warn("Slot %lu is in use\n", idx);
> +		err = -EBUSY;
> +		goto done;

[Severity: Medium]
Similarly, does this slot check also bail out without cleaning up the
hardware state, causing a trigger leak?

> +	}
[ ... ]
> +/* atomic: counter->ctx->lock is held */
> +void arch_uninstall_hw_breakpoint(struct perf_event *event)
> +{
> +	struct sbiret ret;
> +	int i;
> +
> +	raw_spin_lock_irqsave(this_cpu_ptr(&ecall_lock),
> +			      *this_cpu_ptr(&ecall_lock_flags));
> +
> +	for (i = 0; i < dbtr_total_num; i++) {
> +		struct perf_event **slot = this_cpu_ptr(&pcpu_hw_bp_events[i]);
> +
> +		if (*slot == event) {
> +			*slot = NULL;
> +			break;
> +		}
> +	}

[Severity: Medium]
Does clearing the software slot before uninstalling the hardware trigger
open a dangerous race window? If a kernel watchpoint fires right here before
SBI_EXT_DBTR_TRIG_UNINSTALL completes, the handler will observe a NULL slot
and return NOTIFY_DONE, causing handle_break() to crash the system with an
unhandled DIE_DEBUG trap.

> +
> +	if (i == dbtr_total_num) {
> +		pr_warn("Breakpoint not installed.\n");
> +		goto out;
> +	}
> +
> +	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIG_UNINSTALL,
> +			i, 1, 0, 0, 0, 0);
[ ... ]
> +void arch_update_hw_breakpoint(struct perf_event *event)
> +{
> +	struct arch_hw_breakpoint *bp = counter_arch_bp(event);
> +	union sbi_dbtr_shmem_entry *shmem = this_cpu_ptr(sbi_dbtr_shmem);
> +	struct sbi_dbtr_data_msg *xmit;
> +	struct sbi_dbtr_id_msg *id;
> +	struct perf_event **slot;
> +	struct sbiret ret;
> +	int i;
[ ... ]
> +	raw_spin_lock_irqsave(this_cpu_ptr(&ecall_lock),
> +			      *this_cpu_ptr(&ecall_lock_flags));
> +
> +	id = &shmem->id;
> +	xmit = &shmem->data;
> +	id->idx = cpu_to_le(i);
> +	xmit->tdata1 = cpu_to_le(bp->tdata1);

[Severity: High]
Writing to id->idx and xmit->tdata1 here accesses the same shared memory union.
Does writing to id->idx corrupt the aliased xmit->tstate before it is sent to
the hypervisor?

> +	xmit->tdata2 = cpu_to_le(bp->tdata2);
> +	xmit->tdata3 = cpu_to_le(bp->tdata3);
> +
> +	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_TRIG_UPDATE,
> +			1, 0, 0, 0, 0, 0);

[Severity: High]
Are we invoking the update call with a hardcoded zero count in a1? This seems
like it would make the update a hardware no-op and cause breakpoint updates
to silently fail.

> +	if (ret.error)
> +		pr_warn("Failed to update trigger %d.\n", i);
[ ... ]
> +static int __init arch_hw_breakpoint_init(void)
> +{
[ ... ]
> +	/* Allocate per-cpu shared memory */
> +	sbi_dbtr_shmem = __alloc_percpu(sizeof(*sbi_dbtr_shmem) * dbtr_total_num,
> +					PAGE_SIZE);
> +
> +	if (!sbi_dbtr_shmem) {
> +		pr_warn("Failed to allocate shared memory.\n");
> +		rc = -ENOMEM;
> +		goto out;
> +	}
> +
> +	/* Hotplug handler to register/unregister shared memory with SBI */
> +	rc = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> +			       "riscv/hw_breakpoint:prepare",
> +			       arch_smp_setup_sbi_shmem,
> +			       arch_smp_teardown_sbi_shmem);
> +
> +	if (rc < 0) {
> +		pr_warn("Failed to setup CPU hotplug state\n");
> +		free_percpu(sbi_dbtr_shmem);
> +		return rc;
> +	}

[Severity: High]
If cpuhp_setup_state() fails, we free sbi_dbtr_shmem. However, since the PMU
has already been registered earlier in the generic subsystem, can users still
create hardware breakpoints and cause a use-after-free when they access the
freed sbi_dbtr_shmem?

> + out:
> +	return rc;

[Severity: Low]
When cpuhp_setup_state() succeeds, it returns a positive state ID which is
stored in rc. Should this initcall return 0 on success to avoid the initcall
core logging an erroneous failure message?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930063919.608911-1-himanshu.chauhan@oss.qualcomm.com?part=1

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2026-09-30  6:58 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-30  6:39 [PATCH v7 0/8] riscv: Introduce support for hardware break/watchpoints Himanshu Chauhan
2026-09-30  6:39 ` [PATCH v7 1/8] " Himanshu Chauhan
2026-09-30  6:58   ` sashiko-bot
2026-09-30  6:39 ` [PATCH v7 2/8] riscv: Add breakpoint and watchpoint test for riscv Himanshu Chauhan
2026-09-30  6:49   ` sashiko-bot
2026-09-30  6:39 ` [PATCH v7 3/8] riscv: ptrace support for hardware break/watchpoints Himanshu Chauhan
2026-09-30  6:56   ` sashiko-bot
2026-09-30  6:39 ` [PATCH v7 4/8] selftests/breakpoints: extend riscv test for ptrace hw break/watchpoints Himanshu Chauhan
2026-09-30  6:51   ` sashiko-bot
2026-09-30  6:39 ` [PATCH v7 5/8] RISC-V: Add fetch and decode helpers to a common file Himanshu Chauhan
2026-09-30  6:50   ` sashiko-bot
2026-09-30  6:39 ` [PATCH v7 6/8] riscv: Add software supported single stepping with mc/mc6 triggers Himanshu Chauhan
2026-09-30  6:56   ` sashiko-bot
2026-09-30  6:39 ` [PATCH v7 7/8] perf tests: add noinline to __test_function Himanshu Chauhan
2026-09-30  6:45   ` sashiko-bot
2026-09-30  6:39 ` [PATCH v7 8/8] MAINTAINERS: Add entry for RISC-V Debugging Himanshu Chauhan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox