mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jesse Taube <jesse@rivosinc.com>
To: linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Cc: "Paul Walmsley" <paul.walmsley@sifive.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Alexandre Ghiti" <alex@ghiti.fr>,
	"Oleg Nesterov" <oleg@redhat.com>,
	"Jesse Taube" <jesse@rivosinc.com>,
	"Himanshu Chauhan" <hchauhan@ventanamicro.com>,
	"Charlie Jenkins" <charlie@rivosinc.com>,
	"Samuel Holland" <samuel.holland@sifive.com>,
	"Deepak Gupta" <debug@rivosinc.com>,
	"Andrew Jones" <ajones@ventanamicro.com>,
	"Atish Patra" <atishp@rivosinc.com>,
	"Anup Patel" <apatel@ventanamicro.com>,
	"Mayuresh Chitale" <mchitale@ventanamicro.com>,
	"Conor Dooley" <conor.dooley@microchip.com>,
	WangYuli <wangyuli@uniontech.com>,
	"Huacai Chen" <chenhuacai@kernel.org>,
	"Nam Cao" <namcao@linutronix.de>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Mike Rapoport (Microsoft)" <rppt@kernel.org>,
	"Luis Chamberlain" <mcgrof@kernel.org>,
	"Yunhui Cui" <cuiyunhui@bytedance.com>,
	"Joel Granados" <joel.granados@kernel.org>,
	"Clément Léger" <cleger@rivosinc.com>,
	"Celeste Liu" <coelacanthushex@gmail.com>,
	"Evan Green" <evan@rivosinc.com>,
	"Nylon Chen" <nylon.chen@sifive.com>
Subject: [RFC PATCH 5/6] riscv: hw_breakpoint: Use icount for single stepping
Date: Tue, 22 Jul 2025 10:38:28 -0700	[thread overview]
Message-ID: <20250722173829.984082-6-jesse@rivosinc.com> (raw)
In-Reply-To: <20250722173829.984082-1-jesse@rivosinc.com>

The Sdtrig RISC-V ISA extension does not have a resume flag for
returning to and executing the instruction at the breakpoint.
To avoid skipping the instruction or looping, it is necessary to remove
the hardware breakpoint and single step. Use the icount feature of
Sdtrig to accomplish this. Use icount as default with an option to allow
software-based single stepping when hardware or SBI does not have
icount functionality, as it may cause unwanted side effects when reading
the instruction from memory.

Signed-off-by: Jesse Taube <jesse@rivosinc.com>
---
 arch/riscv/Kconfig                | 11 +++++
 arch/riscv/kernel/hw_breakpoint.c | 81 +++++++++++++++++++++++++------
 2 files changed, 76 insertions(+), 16 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 95d3047cab10..bbde5e118470 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -1105,6 +1105,17 @@ config PARAVIRT_TIME_ACCOUNTING
 
 	  If in doubt, say N here.
 
+config HW_BREAKPOINT_COMPUTE_STEP
+	bool "Allow computing hardware breakpoint step address"
+	default n
+	depends on HAVE_HW_BREAKPOINT
+	help
+	  Select this option if hardware breakpoints are desired, but
+	  hardware or SBI does not have icount functionality. This may cause
+	  unwanted side affects when reading the instruction from memory.
+
+	  If unsure, say N.
+
 config RELOCATABLE
 	bool "Build a relocatable kernel"
 	depends on !XIP_KERNEL
diff --git a/arch/riscv/kernel/hw_breakpoint.c b/arch/riscv/kernel/hw_breakpoint.c
index 9e3a3b82d300..437fd82b9590 100644
--- a/arch/riscv/kernel/hw_breakpoint.c
+++ b/arch/riscv/kernel/hw_breakpoint.c
@@ -20,6 +20,7 @@
 #define DBTR_TDATA1_DMODE		BIT_UL(__riscv_xlen - 5)
 
 #define DBTR_TDATA1_TYPE_MCONTROL	(2UL << DBTR_TDATA1_TYPE_SHIFT)
+#define DBTR_TDATA1_TYPE_ICOUNT		(3UL << DBTR_TDATA1_TYPE_SHIFT)
 #define DBTR_TDATA1_TYPE_MCONTROL6	(6UL << DBTR_TDATA1_TYPE_SHIFT)
 
 #define DBTR_TDATA1_MCONTROL6_LOAD		BIT(0)
@@ -55,6 +56,14 @@
 #define DBTR_TDATA1_MCONTROL_SIZELO_64BIT	1
 #define DBTR_TDATA1_MCONTROL_SIZEHI_64BIT	1
 
+#define DBTR_TDATA1_ICOUNT_U			BIT(6)
+#define DBTR_TDATA1_ICOUNT_S			BIT(7)
+#define DBTR_TDATA1_ICOUNT_PENDING		BIT(8)
+#define DBTR_TDATA1_ICOUNT_M			BIT(9)
+#define DBTR_TDATA1_ICOUNT_COUNT_FIELD		GENMASK(23, 10)
+#define DBTR_TDATA1_ICOUNT_VU			BIT(25)
+#define DBTR_TDATA1_ICOUNT_VS			BIT(26)
+
 /* Registered per-cpu bp/wp */
 static DEFINE_PER_CPU(struct perf_event *, pcpu_hw_bp_events[RV_MAX_TRIGGERS]);
 static DEFINE_PER_CPU(unsigned long, ecall_lock_flags);
@@ -65,6 +74,7 @@ static DEFINE_PER_CPU(union sbi_dbtr_shmem_entry, sbi_dbtr_shmem);
 
 /* number of debug triggers on this cpu . */
 static int dbtr_total_num __ro_after_init;
+static bool have_icount __ro_after_init;
 static unsigned long dbtr_type __ro_after_init;
 static unsigned long dbtr_init __ro_after_init;
 
@@ -168,6 +178,7 @@ static int arch_smp_teardown_sbi_shmem(unsigned int cpu)
 static void init_sbi_dbtr(void)
 {
 	struct sbiret ret;
+	unsigned long dbtr_count = 0;
 
 	/*
 	 * Called by hw_breakpoint_slots and arch_hw_breakpoint_init.
@@ -182,6 +193,25 @@ static void init_sbi_dbtr(void)
 		return;
 	}
 
+	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_NUM_TRIGGERS,
+		DBTR_TDATA1_TYPE_ICOUNT, 0, 0, 0, 0, 0);
+	if (ret.error) {
+		pr_warn("%s: failed to detect icount triggers. error: %ld.\n",
+			__func__, ret.error);
+	} else if (!ret.value) {
+		if (IS_ENABLED(CONFIG_HW_BREAKPOINT_COMPUTE_STEP)) {
+			pr_warn("%s: No icount triggers available. "
+				"Falling-back to computing single step address.\n", __func__);
+		} else {
+			pr_err("%s: No icount triggers available.\n", __func__);
+			dbtr_total_num = 0;
+			return;
+		}
+	} else {
+		dbtr_count = ret.value;
+		have_icount = true;
+	}
+
 	ret = sbi_ecall(SBI_EXT_DBTR, SBI_EXT_DBTR_NUM_TRIGGERS,
 			DBTR_TDATA1_TYPE_MCONTROL6, 0, 0, 0, 0, 0);
 	if (ret.error) {
@@ -190,7 +220,7 @@ static void init_sbi_dbtr(void)
 	} else if (!ret.value) {
 		pr_warn("%s: No mcontrol6 triggers available.\n", __func__);
 	} else {
-		dbtr_total_num = ret.value;
+		dbtr_total_num = min_not_zero((unsigned long)ret.value, dbtr_count);
 		dbtr_type = DBTR_TDATA1_TYPE_MCONTROL6;
 		return;
 	}
@@ -205,7 +235,7 @@ static void init_sbi_dbtr(void)
 		pr_err("%s: No mcontrol triggers available.\n", __func__);
 		dbtr_total_num = 0;
 	} else {
-		dbtr_total_num = ret.value;
+		dbtr_total_num = min_not_zero((unsigned long)ret.value, dbtr_count);
 		dbtr_type = DBTR_TDATA1_TYPE_MCONTROL;
 	}
 }
@@ -344,6 +374,21 @@ static int rv_init_mcontrol6_trigger(const struct perf_event_attr *attr,
 	return 0;
 }
 
+static int rv_init_icount_trigger(struct arch_hw_breakpoint *hw)
+{
+	unsigned long tdata1 = DBTR_TDATA1_TYPE_ICOUNT;
+
+	/* Step one instruction */
+	tdata1 |= FIELD_PREP(DBTR_TDATA1_ICOUNT_COUNT_FIELD, 1);
+
+	tdata1 |= DBTR_TDATA1_ICOUNT_U;
+
+	hw->tdata1 = tdata1;
+	hw->tdata2 = 0;
+
+	return 0;
+}
+
 int hw_breakpoint_arch_parse(struct perf_event *bp,
 			     const struct perf_event_attr *attr,
 			     struct arch_hw_breakpoint *hw)
@@ -389,24 +434,28 @@ static int setup_singlestep(struct perf_event *event, struct pt_regs *regs)
 	/* Remove breakpoint even if return error as not to loop */
 	arch_uninstall_hw_breakpoint(event);
 
-	ret = get_insn(regs, regs->epc, &insn);
-	if (ret < 0)
-		return ret;
+	if (have_icount) {
+		rv_init_icount_trigger(bp);
+	} else {
+		ret = get_insn(regs, regs->epc, &insn);
+		if (ret < 0)
+			return ret;
 
-	next_addr = get_step_address(regs, insn);
+		next_addr = get_step_address(regs, insn);
 
-	ret = get_insn(regs, next_addr, &insn);
-	if (ret < 0)
-		return ret;
+		ret = get_insn(regs, next_addr, &insn);
+		if (ret < 0)
+			return ret;
 
-	bp_insn.bp_type = HW_BREAKPOINT_X;
-	bp_insn.bp_addr = next_addr;
-	/* Get the size of the intruction */
-	bp_insn.bp_len = GET_INSN_LENGTH(insn);
+		bp_insn.bp_type = HW_BREAKPOINT_X;
+		bp_insn.bp_addr = next_addr;
+		/* Get the size of the intruction */
+		bp_insn.bp_len = GET_INSN_LENGTH(insn);
 
-	ret = hw_breakpoint_arch_parse(NULL, &bp_insn, bp);
-	if (ret)
-		return ret;
+		ret = hw_breakpoint_arch_parse(NULL, &bp_insn, bp);
+		if (ret)
+			return ret;
+	}
 
 	ret = arch_install_hw_breakpoint(event);
 	if (ret)
-- 
2.43.0


  parent reply	other threads:[~2025-07-22 17:38 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-22 17:38 [RFC PATCH 0/6] riscv: add initial support for hardware breakpoints Jesse Taube
2025-07-22 17:38 ` [RFC PATCH 1/6] riscv: Add insn.c, consolidate instruction decoding Jesse Taube
2025-07-22 17:38 ` [RFC PATCH 2/6] riscv: Add SBI debug trigger extension and function ids Jesse Taube
2025-07-22 17:38 ` [RFC PATCH 3/6] riscv: insn: __read_insn use copy_from_X_nofault Jesse Taube
2025-08-04  7:44   ` Clément Léger
2025-07-22 17:38 ` [RFC PATCH 4/6] riscv: Introduce support for hardware break/watchpoints Jesse Taube
2025-07-23  2:49   ` Deepak Gupta
2025-07-23 17:02     ` Jesse Taube
2025-07-23 17:25       ` Deepak Gupta
2025-08-04  8:01   ` Clément Léger
2025-08-04 15:03     ` Jesse Taube
2025-07-22 17:38 ` Jesse Taube [this message]
2025-08-04  8:15   ` [RFC PATCH 5/6] riscv: hw_breakpoint: Use icount for single stepping Clément Léger
2025-07-22 17:38 ` [RFC PATCH 6/6] riscv: ptrace: Add hw breakpoint support Jesse Taube
2025-07-23  4:18   ` Deepak Gupta
2025-07-23 16:55     ` Jesse Taube
2025-07-23 17:23       ` Deepak Gupta
2025-08-04  8:16   ` Clément Léger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250722173829.984082-6-jesse@rivosinc.com \
    --to=jesse@rivosinc.com \
    --cc=ajones@ventanamicro.com \
    --cc=akpm@linux-foundation.org \
    --cc=alex@ghiti.fr \
    --cc=aou@eecs.berkeley.edu \
    --cc=apatel@ventanamicro.com \
    --cc=atishp@rivosinc.com \
    --cc=charlie@rivosinc.com \
    --cc=chenhuacai@kernel.org \
    --cc=cleger@rivosinc.com \
    --cc=coelacanthushex@gmail.com \
    --cc=conor.dooley@microchip.com \
    --cc=cuiyunhui@bytedance.com \
    --cc=debug@rivosinc.com \
    --cc=evan@rivosinc.com \
    --cc=hchauhan@ventanamicro.com \
    --cc=joel.granados@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=mcgrof@kernel.org \
    --cc=mchitale@ventanamicro.com \
    --cc=namcao@linutronix.de \
    --cc=nylon.chen@sifive.com \
    --cc=oleg@redhat.com \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=rppt@kernel.org \
    --cc=samuel.holland@sifive.com \
    --cc=wangyuli@uniontech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®