Re: [RFC PATCH 2/2] riscv: Introduce support for hardware break/watchpoints
From: Himanshu Chauhan
Date: Thu May 08 2025 - 07:55:00 EST
On 5/8/25 03:02, Charlie Jenkins wrote:
On Wed, May 07, 2025 at 04:58:56PM +0530, Himanshu Chauhan wrote:
Hi Charlie,
On 5/6/25 07:30, Charlie Jenkins wrote:
On Thu, Feb 22, 2024 at 06:20:59PM +0530, Himanshu Chauhan wrote:
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.
Signed-off-by: Himanshu Chauhan <hchauhan@xxxxxxxxxxxxxxxx>
---
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/hw_breakpoint.h | 327 ++++++++++++
arch/riscv/include/asm/kdebug.h | 3 +-
arch/riscv/kernel/Makefile | 1 +
arch/riscv/kernel/hw_breakpoint.c | 659 +++++++++++++++++++++++++
arch/riscv/kernel/traps.c | 6 +
6 files changed, 996 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/kernel/hw_breakpoint.c b/arch/riscv/kernel/hw_breakpoint.c
new file mode 100644
index 000000000000..7787123c7180
--- /dev/null
+++ b/arch/riscv/kernel/hw_breakpoint.c
+
+void clear_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("%s: total number of type %d triggers: %u\n",
+ __func__, dbtr_type, dbtr_total_num);
+ } else {
+ pr_info("%s: No hardware triggers available\n", __func__);
+ 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("%s: Failed to allocate shared memory.\n", __func__);
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ /* Hotplug handler to register/unregister shared memory with SBI */
+ rc = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
When using this, only hart 0 is getting setup. I think instead we want
the following to have all harts get setup:
for_each_online_cpu(cpu)
arch_smp_setup_sbi_shmem(cpu);
/* Hotplug handler to register/unregister shared memory with SBI */
rc = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
cpuhp_setup_state() install the callbacks and invoke the @startup callback
(if not NULL) for all online CPUs. So there is no need to call
"arch_smp_setup_sbi_shmem" for each CPU and then install the hotplug
handler.
That's what I thought as well, but when testing that is not what was
happening.
I see the initialization happening on all the CPUs. I am using OpenSBI
Top-of-line.
Linux kernel is from https://github.com/ventanamicro/linux.git [branch:
dev-upstream]
Part of boot log:
[ 0.267250] arch_hw_breakpoint_init: total number of type 6 triggers: 2
[ 0.268108] CPU 0: HW Breakpoint shared memory registered.
[ 0.268835] CPU 1: HW Breakpoint shared memory registered.
[ 0.269932] CPU 2: HW Breakpoint shared memory registered.
[ 0.270468] CPU 3: HW Breakpoint shared memory registered.
[ 0.276554] sse: SBI SSE extension detected
[ 0.308172] HugeTLB: allocation took 0ms with
hugepage_allocation_threads=1
[ 0.309682] HugeTLB: registered 2.00 MiB page size, pre-allocated 0 pages
Can you please send me your config and bootlog?
Regards
Himanshu
If you are running this on QEMU, could you please share the qemu command you
are invoking? I will test at my end and update you.
This is my qemu command:
qemu-system-riscv64 -nographic -m 1G -machine virt -smp 4 \
-kernel arch/riscv/boot/Image -bios /home/charlie/opensbi/build/platform/generic/firmware/fw_dynamic.bin \
-append "root=/dev/vda rw earlycon console=ttyS0" \
-drive file=/home/charlie/buildroot/output/images/rootfs.ext2,format=raw,id=hd0,if=none \
-cpu rv64,zicond=true \
-device virtio-blk-device,drive=hd0 -gdb tcp::1234
- Charlie
Regards
Himanshu
However, I am testing against tip-of-tree opensbi and am hitting an
issue during the setup on all harts:
[ 0.202332] arch_smp_setup_sbi_shmem: Invalid address parameter (18446744073709551611)
[ 0.202794] CPU 0: HW Breakpoint shared memory registered.
Additionally, this seems like it should be a fatal error, but it
continues on to print that the shared memory is registered because there
is no check before printing this seemingly successful message.
I know I am reviving an old thread, but do you have any insight into
what might be happening?
- Charlie
+ "riscv/hw_breakpoint:prepare",
+ arch_smp_setup_sbi_shmem,
+ arch_smp_teardown_sbi_shmem);
+
+ if (rc < 0) {
+ pr_warn("%s: Failed to setup CPU hotplug state\n", __func__);
+ 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 a1b9be3c4332..53e1dfe5746b 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -277,6 +277,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.34.1
_______________________________________________
linux-riscv mailing list
linux-riscv@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-riscv