[PATCH v2 0/5] riscv: Add KGDB and KDB support

[PATCH v2 0/5] riscv: Add KGDB and KDB support

[Vincent Chen]

This patch set implements required ports to enable RISC-V kernel to support
KGDB and KDB features. Because there is no immediate value in the RISC-V
trap instruction, the kernel cannot identify the purpose of each trap
exception through the opcode. This makes the existing identification
schemes in other architecture unsuitable for the RISC-V kernel. In order
to solve this problem, this patch adds the kgdb_has_hit_break() to kgdb.c
to help the RISC-V kernel identify the KGDB trap exception. In addition,
the XML target description was introduced in this patch set to enable KGDB
to report the contents of the status, cause and steal registers.
 
This patchset has passed the kgdbts test suite provided by Linux kernel on
HiFive unleashed board and QEMU.

Changes since v1:
1. Replace the magic number with macro when filling the gdb_regs[].
2. Only support GDB XML packet instead of all query packets.
3. Move the macros used to parse instrcuton to parse_asm.h


Vincent Chen (5):
  kgdb: Add kgdb_has_hit_break function
  riscv: Add KGDB support
  kgdb: enable arch to support XML packet support.
  riscv: Use the XML target descriptions to report 3 system registers
  riscv: Add SW single-step support for KDB

 arch/riscv/Kconfig                 |   2 +
 arch/riscv/include/asm/Kbuild      |   1 -
 arch/riscv/include/asm/gdb_xml.h   | 117 ++++++++++++
 arch/riscv/include/asm/kdebug.h    |  12 ++
 arch/riscv/include/asm/kgdb.h      | 113 +++++++++++
 arch/riscv/include/asm/parse_asm.h | 214 +++++++++++++++++++++
 arch/riscv/kernel/Makefile         |   1 +
 arch/riscv/kernel/kgdb.c           | 382 +++++++++++++++++++++++++++++++++++++
 arch/riscv/kernel/traps.c          |   5 +
 include/linux/kgdb.h               |   9 +
 kernel/debug/debug_core.c          |  12 ++
 kernel/debug/gdbstub.c             |  13 ++
 lib/Kconfig.kgdb                   |   5 +
 13 files changed, 885 insertions(+), 1 deletion(-)
 create mode 100644 arch/riscv/include/asm/gdb_xml.h
 create mode 100644 arch/riscv/include/asm/kdebug.h
 create mode 100644 arch/riscv/include/asm/kgdb.h
 create mode 100644 arch/riscv/include/asm/parse_asm.h
 create mode 100644 arch/riscv/kernel/kgdb.c

-- 
2.7.4

[PATCH v2 1/5] kgdb: Add kgdb_has_hit_break function

[Vincent Chen]

The break instruction in RISC-V does not have an immediate value field, so
the kernel cannot identify the purpose of each trap exception through the
opcode. This makes the existing identification schemes in other
architecture unsuitable for the RISC-V kernel. To solve this problem, this
patch adds kgdb_has_hit_break(), which can help RISC-V kernel identify
the KGDB trap exception.

Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
---
 kernel/debug/debug_core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 2b7c9b67931d..01bc3eea3d4d 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -417,6 +417,18 @@ int kgdb_isremovedbreak(unsigned long addr)
 	return 0;
 }
 
+int kgdb_has_hit_break(unsigned long addr)
+{
+	int i;
+
+	for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
+		if (kgdb_break[i].state == BP_ACTIVE &&
+		    kgdb_break[i].bpt_addr == addr)
+			return 1;
+	}
+	return 0;
+}
+
 int dbg_remove_all_break(void)
 {
 	int error;
-- 
2.7.4

Re: [PATCH v2 1/5] kgdb: Add kgdb_has_hit_break function

[Daniel Thompson]

On Tue, Mar 31, 2020 at 11:23:07PM +0800, Vincent Chen wrote:
> The break instruction in RISC-V does not have an immediate value field, so
> the kernel cannot identify the purpose of each trap exception through the
> opcode. This makes the existing identification schemes in other
> architecture unsuitable for the RISC-V kernel. To solve this problem, this
> patch adds kgdb_has_hit_break(), which can help RISC-V kernel identify
> the KGDB trap exception.

I was just reflecting on this again.

The approach is fine from a kgdb point of view (where breakpoints are
expensive heavy weight operations) but it might be wise to share
how much implementing kgdb in this manner slows down kprobe tracing
since these are normally pretty light weight.

If the benchmarks do look bad I'd certainly entertain patches to
optimize kgdb_has_hit_break(). For example by tracking the highest
currently allocated breakpoint number would make a big difference
(since it would normally be zero or close to).


Daniel.

> 
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
> Acked-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
>  kernel/debug/debug_core.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
> index 2b7c9b67931d..01bc3eea3d4d 100644
> --- a/kernel/debug/debug_core.c
> +++ b/kernel/debug/debug_core.c
> @@ -417,6 +417,18 @@ int kgdb_isremovedbreak(unsigned long addr)
>  	return 0;
>  }
>  
> +int kgdb_has_hit_break(unsigned long addr)
> +{
> +	int i;
> +
> +	for (i = 0; i < KGDB_MAX_BREAKPOINTS; i++) {
> +		if (kgdb_break[i].state == BP_ACTIVE &&
> +		    kgdb_break[i].bpt_addr == addr)
> +			return 1;
> +	}
> +	return 0;
> +}
> +
>  int dbg_remove_all_break(void)
>  {
>  	int error;
> -- 
> 2.7.4
> 

[PATCH v2 2/5] riscv: Add KGDB support

[Vincent Chen]

The skeleton of RISC-V KGDB port.

Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>
---
 arch/riscv/Kconfig              |   1 +
 arch/riscv/include/asm/Kbuild   |   1 -
 arch/riscv/include/asm/kdebug.h |  12 +++
 arch/riscv/include/asm/kgdb.h   | 107 +++++++++++++++++++++
 arch/riscv/kernel/Makefile      |   1 +
 arch/riscv/kernel/kgdb.c        | 199 ++++++++++++++++++++++++++++++++++++++++
 arch/riscv/kernel/traps.c       |   5 +
 7 files changed, 325 insertions(+), 1 deletion(-)
 create mode 100644 arch/riscv/include/asm/kdebug.h
 create mode 100644 arch/riscv/include/asm/kgdb.h
 create mode 100644 arch/riscv/kernel/kgdb.c

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 73f029eae0cc..108794f4aa45 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -66,6 +66,7 @@ config RISCV
 	select ARCH_HAS_GCOV_PROFILE_ALL
 	select HAVE_COPY_THREAD_TLS
 	select HAVE_ARCH_KASAN if MMU && 64BIT
+	select HAVE_ARCH_KGDB
 
 config ARCH_MMAP_RND_BITS_MIN
 	default 18 if 64BIT
diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
index ec0ca8c6ab64..625e46cbdec8 100644
--- a/arch/riscv/include/asm/Kbuild
+++ b/arch/riscv/include/asm/Kbuild
@@ -15,7 +15,6 @@ generic-y += hardirq.h
 generic-y += hw_irq.h
 generic-y += irq_regs.h
 generic-y += irq_work.h
-generic-y += kdebug.h
 generic-y += kmap_types.h
 generic-y += kvm_para.h
 generic-y += local.h
diff --git a/arch/riscv/include/asm/kdebug.h b/arch/riscv/include/asm/kdebug.h
new file mode 100644
index 000000000000..85ac00411f6e
--- /dev/null
+++ b/arch/riscv/include/asm/kdebug.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _ASM_ARC_KDEBUG_H
+#define _ASM_ARC_KDEBUG_H
+
+enum die_val {
+	DIE_UNUSED,
+	DIE_TRAP,
+	DIE_OOPS
+};
+
+#endif
diff --git a/arch/riscv/include/asm/kgdb.h b/arch/riscv/include/asm/kgdb.h
new file mode 100644
index 000000000000..69bc6a03081d
--- /dev/null
+++ b/arch/riscv/include/asm/kgdb.h
@@ -0,0 +1,107 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __ASM_KGDB_H_
+#define __ASM_KGDB_H_
+
+#ifdef __KERNEL__
+
+#define GDB_SIZEOF_REG sizeof(unsigned long)
+
+#define DBG_MAX_REG_NUM (33)
+#define NUMREGBYTES ((DBG_MAX_REG_NUM) * GDB_SIZEOF_REG)
+#define CACHE_FLUSH_IS_SAFE     1
+#define BUFMAX                  2048
+#ifdef CONFIG_RISCV_ISA_C
+#define BREAK_INSTR_SIZE	2
+#else
+#define BREAK_INSTR_SIZE	4
+#endif
+#define CACHE_FLUSH_IS_SAFE 1
+
+#ifndef	__ASSEMBLY__
+
+extern int kgdb_has_hit_break(unsigned long addr);
+extern unsigned long kgdb_compiled_break;
+
+static inline void arch_kgdb_breakpoint(void)
+{
+	asm(".global kgdb_compiled_break\n"
+	    ".option norvc\n"
+	    "kgdb_compiled_break: ebreak\n"
+	    ".option rvc\n");
+}
+
+#endif /* !__ASSEMBLY__ */
+
+#define DBG_REG_ZERO "zero"
+#define DBG_REG_RA "ra"
+#define DBG_REG_SP "sp"
+#define DBG_REG_GP "gp"
+#define DBG_REG_TP "tp"
+#define DBG_REG_T0 "t0"
+#define DBG_REG_T1 "t1"
+#define DBG_REG_T2 "t2"
+#define DBG_REG_FP "fp"
+#define DBG_REG_S1 "s1"
+#define DBG_REG_A0 "a0"
+#define DBG_REG_A1 "a1"
+#define DBG_REG_A2 "a2"
+#define DBG_REG_A3 "a3"
+#define DBG_REG_A4 "a4"
+#define DBG_REG_A5 "a5"
+#define DBG_REG_A6 "a6"
+#define DBG_REG_A7 "a7"
+#define DBG_REG_S2 "s2"
+#define DBG_REG_S3 "s3"
+#define DBG_REG_S4 "s4"
+#define DBG_REG_S5 "s5"
+#define DBG_REG_S6 "s6"
+#define DBG_REG_S7 "s7"
+#define DBG_REG_S8 "s8"
+#define DBG_REG_S9 "s9"
+#define DBG_REG_S10 "s10"
+#define DBG_REG_S11 "s11"
+#define DBG_REG_T3 "t3"
+#define DBG_REG_T4 "t4"
+#define DBG_REG_T5 "t5"
+#define DBG_REG_T6 "t6"
+#define DBG_REG_EPC "pc"
+
+#define DBG_REG_ZERO_OFF 0
+#define DBG_REG_RA_OFF 1
+#define DBG_REG_SP_OFF 2
+#define DBG_REG_GP_OFF 3
+#define DBG_REG_TP_OFF 4
+#define DBG_REG_T0_OFF 5
+#define DBG_REG_T1_OFF 6
+#define DBG_REG_T2_OFF 7
+#define DBG_REG_FP_OFF 8
+#define DBG_REG_S1_OFF 9
+#define DBG_REG_A0_OFF 10
+#define DBG_REG_A1_OFF 11
+#define DBG_REG_A2_OFF 12
+#define DBG_REG_A3_OFF 13
+#define DBG_REG_A4_OFF 14
+#define DBG_REG_A5_OFF 15
+#define DBG_REG_A6_OFF 16
+#define DBG_REG_A7_OFF 17
+#define DBG_REG_S2_OFF 18
+#define DBG_REG_S3_OFF 19
+#define DBG_REG_S4_OFF 20
+#define DBG_REG_S5_OFF 21
+#define DBG_REG_S6_OFF 22
+#define DBG_REG_S7_OFF 23
+#define DBG_REG_S8_OFF 24
+#define DBG_REG_S9_OFF 25
+#define DBG_REG_S10_OFF 26
+#define DBG_REG_S11_OFF 27
+#define DBG_REG_T3_OFF 28
+#define DBG_REG_T4_OFF 29
+#define DBG_REG_T5_OFF 30
+#define DBG_REG_T6_OFF 31
+#define DBG_REG_EPC_OFF 32
+#define DBG_REG_STATUS_OFF 33
+#define DBG_REG_BADADDR_OFF 34
+#define DBG_REG_CAUSE_OFF 35
+#endif
+#endif
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index f40205cb9a22..8956543f7d6b 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -42,5 +42,6 @@ obj-$(CONFIG_PERF_EVENTS)	+= perf_event.o
 obj-$(CONFIG_PERF_EVENTS)	+= perf_callchain.o
 obj-$(CONFIG_HAVE_PERF_REGS)	+= perf_regs.o
 obj-$(CONFIG_RISCV_SBI)		+= sbi.o
+obj-$(CONFIG_KGDB)		+= kgdb.o
 
 clean:
diff --git a/arch/riscv/kernel/kgdb.c b/arch/riscv/kernel/kgdb.c
new file mode 100644
index 000000000000..e3b1075c3935
--- /dev/null
+++ b/arch/riscv/kernel/kgdb.c
@@ -0,0 +1,199 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/ptrace.h>
+#include <linux/kdebug.h>
+#include <linux/bug.h>
+#include <linux/kgdb.h>
+#include <linux/irqflags.h>
+#include <linux/string.h>
+#include <asm/cacheflush.h>
+
+enum {
+	NOT_KGDB_BREAK = 0,
+	KGDB_SW_BREAK,
+	KGDB_COMPILED_BREAK,
+};
+
+struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
+	{DBG_REG_ZERO, GDB_SIZEOF_REG, -1},
+	{DBG_REG_RA, GDB_SIZEOF_REG, offsetof(struct pt_regs, ra)},
+	{DBG_REG_SP, GDB_SIZEOF_REG, offsetof(struct pt_regs, sp)},
+	{DBG_REG_GP, GDB_SIZEOF_REG, offsetof(struct pt_regs, gp)},
+	{DBG_REG_TP, GDB_SIZEOF_REG, offsetof(struct pt_regs, tp)},
+	{DBG_REG_T0, GDB_SIZEOF_REG, offsetof(struct pt_regs, t0)},
+	{DBG_REG_T1, GDB_SIZEOF_REG, offsetof(struct pt_regs, t1)},
+	{DBG_REG_T2, GDB_SIZEOF_REG, offsetof(struct pt_regs, t2)},
+	{DBG_REG_FP, GDB_SIZEOF_REG, offsetof(struct pt_regs, s0)},
+	{DBG_REG_S1, GDB_SIZEOF_REG, offsetof(struct pt_regs, a1)},
+	{DBG_REG_A0, GDB_SIZEOF_REG, offsetof(struct pt_regs, a0)},
+	{DBG_REG_A1, GDB_SIZEOF_REG, offsetof(struct pt_regs, a1)},
+	{DBG_REG_A2, GDB_SIZEOF_REG, offsetof(struct pt_regs, a2)},
+	{DBG_REG_A3, GDB_SIZEOF_REG, offsetof(struct pt_regs, a3)},
+	{DBG_REG_A4, GDB_SIZEOF_REG, offsetof(struct pt_regs, a4)},
+	{DBG_REG_A5, GDB_SIZEOF_REG, offsetof(struct pt_regs, a5)},
+	{DBG_REG_A6, GDB_SIZEOF_REG, offsetof(struct pt_regs, a6)},
+	{DBG_REG_A7, GDB_SIZEOF_REG, offsetof(struct pt_regs, a7)},
+	{DBG_REG_S2, GDB_SIZEOF_REG, offsetof(struct pt_regs, s2)},
+	{DBG_REG_S3, GDB_SIZEOF_REG, offsetof(struct pt_regs, s3)},
+	{DBG_REG_S4, GDB_SIZEOF_REG, offsetof(struct pt_regs, s4)},
+	{DBG_REG_S5, GDB_SIZEOF_REG, offsetof(struct pt_regs, s5)},
+	{DBG_REG_S6, GDB_SIZEOF_REG, offsetof(struct pt_regs, s6)},
+	{DBG_REG_S7, GDB_SIZEOF_REG, offsetof(struct pt_regs, s7)},
+	{DBG_REG_S8, GDB_SIZEOF_REG, offsetof(struct pt_regs, s8)},
+	{DBG_REG_S9, GDB_SIZEOF_REG, offsetof(struct pt_regs, s9)},
+	{DBG_REG_S10, GDB_SIZEOF_REG, offsetof(struct pt_regs, s10)},
+	{DBG_REG_S11, GDB_SIZEOF_REG, offsetof(struct pt_regs, s11)},
+	{DBG_REG_T3, GDB_SIZEOF_REG, offsetof(struct pt_regs, t3)},
+	{DBG_REG_T4, GDB_SIZEOF_REG, offsetof(struct pt_regs, t4)},
+	{DBG_REG_T5, GDB_SIZEOF_REG, offsetof(struct pt_regs, t5)},
+	{DBG_REG_T6, GDB_SIZEOF_REG, offsetof(struct pt_regs, t6)},
+	{DBG_REG_EPC, GDB_SIZEOF_REG, offsetof(struct pt_regs, epc)},
+};
+
+char *dbg_get_reg(int regno, void *mem, struct pt_regs *regs)
+{
+	if (regno >= DBG_MAX_REG_NUM || regno < 0)
+		return NULL;
+
+	if (dbg_reg_def[regno].offset != -1)
+		memcpy(mem, (void *)regs + dbg_reg_def[regno].offset,
+		       dbg_reg_def[regno].size);
+	else
+		memset(mem, 0, dbg_reg_def[regno].size);
+	return dbg_reg_def[regno].name;
+}
+
+int dbg_set_reg(int regno, void *mem, struct pt_regs *regs)
+{
+	if (regno >= DBG_MAX_REG_NUM || regno < 0)
+		return -EINVAL;
+
+	if (dbg_reg_def[regno].offset != -1)
+		memcpy((void *)regs + dbg_reg_def[regno].offset, mem,
+		       dbg_reg_def[regno].size);
+	return 0;
+}
+
+void
+sleeping_thread_to_gdb_regs(unsigned long *gdb_regs, struct task_struct *task)
+{
+	/* Initialize to zero */
+	memset((char *)gdb_regs, 0, NUMREGBYTES);
+
+	gdb_regs[DBG_REG_SP_OFF] = task->thread.sp;
+	gdb_regs[DBG_REG_FP_OFF] = task->thread.s[0];
+	gdb_regs[DBG_REG_S1_OFF] = task->thread.s[1];
+	gdb_regs[DBG_REG_S2_OFF] = task->thread.s[2];
+	gdb_regs[DBG_REG_S3_OFF] = task->thread.s[3];
+	gdb_regs[DBG_REG_S4_OFF] = task->thread.s[4];
+	gdb_regs[DBG_REG_S5_OFF] = task->thread.s[5];
+	gdb_regs[DBG_REG_S6_OFF] = task->thread.s[6];
+	gdb_regs[DBG_REG_S7_OFF] = task->thread.s[7];
+	gdb_regs[DBG_REG_S8_OFF] = task->thread.s[8];
+	gdb_regs[DBG_REG_S9_OFF] = task->thread.s[10];
+	gdb_regs[DBG_REG_S10_OFF] = task->thread.s[11];
+	gdb_regs[DBG_REG_EPC_OFF] = task->thread.ra;
+}
+
+void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc)
+{
+	regs->epc = pc;
+}
+
+static inline void kgdb_arch_update_addr(struct pt_regs *regs,
+					 char *remcom_in_buffer)
+{
+	unsigned long addr;
+	char *ptr;
+
+	ptr = &remcom_in_buffer[1];
+	if (kgdb_hex2long(&ptr, &addr))
+		regs->epc = addr;
+}
+
+int kgdb_arch_handle_exception(int vector, int signo, int err_code,
+			       char *remcom_in_buffer, char *remcom_out_buffer,
+			       struct pt_regs *regs)
+{
+	int err = 0;
+
+	switch (remcom_in_buffer[0]) {
+	case 'c':
+	case 'D':
+	case 'k':
+		if (remcom_in_buffer[0] == 'c')
+			kgdb_arch_update_addr(regs, remcom_in_buffer);
+		atomic_set(&kgdb_cpu_doing_single_step, -1);
+		kgdb_single_step = 0;
+		break;
+	default:
+		err = -1;
+	}
+
+	return err;
+}
+
+int kgdb_riscv_kgdbbreak(unsigned long addr)
+{
+	if (atomic_read(&kgdb_setting_breakpoint))
+		if (addr == (unsigned long)&kgdb_compiled_break)
+			return KGDB_COMPILED_BREAK;
+
+	return kgdb_has_hit_break(addr);
+}
+
+static int kgdb_riscv_notify(struct notifier_block *self, unsigned long cmd,
+			     void *ptr)
+{
+	struct die_args *args = (struct die_args *)ptr;
+	struct pt_regs *regs = args->regs;
+	unsigned long flags;
+	int type;
+
+	if (user_mode(regs))
+		return NOTIFY_DONE;
+
+	type = kgdb_riscv_kgdbbreak(regs->epc);
+	if (type == NOT_KGDB_BREAK && cmd == DIE_TRAP)
+		return NOTIFY_DONE;
+
+	local_irq_save(flags);
+	if (kgdb_handle_exception(1, args->signr, cmd, regs))
+		return NOTIFY_DONE;
+
+	if (type == KGDB_COMPILED_BREAK)
+		regs->epc += 4;
+
+	local_irq_restore(flags);
+
+	return NOTIFY_STOP;
+}
+
+static struct notifier_block kgdb_notifier = {
+	.notifier_call = kgdb_riscv_notify,
+};
+
+int kgdb_arch_init(void)
+{
+	register_die_notifier(&kgdb_notifier);
+
+	return 0;
+}
+
+void kgdb_arch_exit(void)
+{
+	unregister_die_notifier(&kgdb_notifier);
+}
+
+/*
+ * Global data
+ */
+#ifdef CONFIG_RISCV_ISA_C
+const struct kgdb_arch arch_kgdb_ops = {
+	.gdb_bpt_instr = {0x02, 0x90},	/* c.ebreak */
+};
+#else
+const struct kgdb_arch arch_kgdb_ops = {
+	.gdb_bpt_instr = {0x73, 0x00, 0x10, 0x00},	/* ebreak */
+};
+#endif
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index f4cad5163bf2..7e017c2131cf 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -125,6 +125,11 @@ asmlinkage __visible void do_trap_break(struct pt_regs *regs)
 {
 	if (user_mode(regs))
 		force_sig_fault(SIGTRAP, TRAP_BRKPT, (void __user *)regs->epc);
+#ifdef CONFIG_KGDB
+	else if (notify_die(DIE_TRAP, "EBREAK", regs, 0, regs->cause, SIGTRAP)
+								== NOTIFY_STOP)
+		return;
+#endif
 	else if (report_bug(regs->epc, regs) == BUG_TRAP_TYPE_WARN)
 		regs->epc += get_break_insn_length(regs->epc);
 	else
-- 
2.7.4

[PATCH v2 3/5] kgdb: enable arch to support XML packet support.

[Vincent Chen]

The XML packet could be supported by required architecture if the
architecture defines CONFIG_ACRH_SUPPORTS_GDB_XML and implement its own
arch_handle_qxfer_pkt(). Except for the arch_handle_qxfer_pkt(), the
architecture also needs to record the feature supported by gdb stub into
the arch_gdb_stub_feature, and these features will be reported to host gdb
when gdb stub receives the qSupported packet.

Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
---
 include/linux/kgdb.h   |  9 +++++++++
 kernel/debug/gdbstub.c | 13 +++++++++++++
 lib/Kconfig.kgdb       |  5 +++++
 3 files changed, 27 insertions(+)

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index b072aeb1fd78..ee9109d2f056 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -177,6 +177,15 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code,
 			   struct pt_regs *regs);
 
 /**
+ *	arch_handle_qxfer_pkt - Handle architecture specific GDB XML packets.
+ *	@remcom_in_buffer: The buffer of the packet we have read.
+ *	@remcom_out_buffer: The buffer of %BUFMAX bytes to write a packet into.
+ */
+
+extern void
+arch_handle_qxfer_pkt(char *remcom_in_buffer, char *remcom_out_buffer);
+
+/**
  *	kgdb_call_nmi_hook - Call kgdb_nmicallback() on the current CPU
  *	@ignored: This parameter is only here to match the prototype.
  *
diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
index 4b280fc7dd67..d6b1b630a7e7 100644
--- a/kernel/debug/gdbstub.c
+++ b/kernel/debug/gdbstub.c
@@ -792,6 +792,19 @@ static void gdb_cmd_query(struct kgdb_state *ks)
 		}
 		break;
 #endif
+#ifdef CONFIG_ACRH_SUPPORTS_GDB_XML
+	case 'S':
+		if (!strncmp(remcom_in_buffer, "qSupported:", 11))
+			strcpy(remcom_out_buffer, arch_gdb_stub_feature);
+		break;
+	case 'X':
+		if (!strncmp(remcom_in_buffer, "qXfer:", 6))
+			arch_handle_qxfer_pkt(remcom_in_buffer,
+					      remcom_out_buffer);
+		break;
+#endif
+	default:
+		break;
 	}
 }
 
diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb
index 933680b59e2d..5b586a3bba90 100644
--- a/lib/Kconfig.kgdb
+++ b/lib/Kconfig.kgdb
@@ -3,6 +3,11 @@
 config HAVE_ARCH_KGDB
 	bool
 
+# set if architecture implemented the arch_handle_qxfer_pkt function
+# to enable gdb stub to address XML packet sent from GDB.
+config ARCH_SUPPORTS_GDB_XML
+	bool
+
 menuconfig KGDB
 	bool "KGDB: kernel debugger"
 	depends on HAVE_ARCH_KGDB
-- 
2.7.4

Re: [PATCH v2 3/5] kgdb: enable arch to support XML packet support.

[Daniel Thompson]

On Tue, Mar 31, 2020 at 11:23:09PM +0800, Vincent Chen wrote:
> The XML packet could be supported by required architecture if the
> architecture defines CONFIG_ACRH_SUPPORTS_GDB_XML and implement its own
> arch_handle_qxfer_pkt(). Except for the arch_handle_qxfer_pkt(), the
> architecture also needs to record the feature supported by gdb stub into
> the arch_gdb_stub_feature, and these features will be reported to host gdb
> when gdb stub receives the qSupported packet.
> 
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> ---
>  include/linux/kgdb.h   |  9 +++++++++
>  kernel/debug/gdbstub.c | 13 +++++++++++++
>  lib/Kconfig.kgdb       |  5 +++++
>  3 files changed, 27 insertions(+)
> 
> diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> index b072aeb1fd78..ee9109d2f056 100644
> --- a/include/linux/kgdb.h
> +++ b/include/linux/kgdb.h
> @@ -177,6 +177,15 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code,
>  			   struct pt_regs *regs);
>  
>  /**
> + *	arch_handle_qxfer_pkt - Handle architecture specific GDB XML packets.
> + *	@remcom_in_buffer: The buffer of the packet we have read.
> + *	@remcom_out_buffer: The buffer of %BUFMAX bytes to write a packet into.
> + */
> +
> +extern void
> +arch_handle_qxfer_pkt(char *remcom_in_buffer, char *remcom_out_buffer);

This should be prefixed kgdb_ like the other arch functions.


> +
> +/**
>   *	kgdb_call_nmi_hook - Call kgdb_nmicallback() on the current CPU
>   *	@ignored: This parameter is only here to match the prototype.
>   *
> diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
> index 4b280fc7dd67..d6b1b630a7e7 100644
> --- a/kernel/debug/gdbstub.c
> +++ b/kernel/debug/gdbstub.c
> @@ -792,6 +792,19 @@ static void gdb_cmd_query(struct kgdb_state *ks)
>  		}
>  		break;
>  #endif
> +#ifdef CONFIG_ACRH_SUPPORTS_GDB_XML

Typo (and perhaps insufficient testing ;-) ).

Additional the naming of the CONFIG option looks wrong because it
describes why you added it, not what it actually does. Something
like CONFIG_HAVE_ARCH_KGDB_QXFER_PKT is more descriptive.


> +	case 'S':
> +		if (!strncmp(remcom_in_buffer, "qSupported:", 11))
> +			strcpy(remcom_out_buffer, arch_gdb_stub_feature);

Has this been declared anywhere? I cannot find it.

This might also benefit from a kgdb_ prefix.


> +		break;
> +	case 'X':
> +		if (!strncmp(remcom_in_buffer, "qXfer:", 6))
> +			arch_handle_qxfer_pkt(remcom_in_buffer,
> +					      remcom_out_buffer);
> +		break;
> +#endif
> +	default:
> +		break;
>  	}
>  }
>  
> diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb
> index 933680b59e2d..5b586a3bba90 100644
> --- a/lib/Kconfig.kgdb
> +++ b/lib/Kconfig.kgdb
> @@ -3,6 +3,11 @@
>  config HAVE_ARCH_KGDB
>  	bool
>  
> +# set if architecture implemented the arch_handle_qxfer_pkt function
> +# to enable gdb stub to address XML packet sent from GDB.
> +config ARCH_SUPPORTS_GDB_XML
> +	bool
> +
>  menuconfig KGDB
>  	bool "KGDB: kernel debugger"
>  	depends on HAVE_ARCH_KGDB
> -- 
> 2.7.4
> 

Re: [PATCH v2 3/5] kgdb: enable arch to support XML packet support.

[Vincent Chen]

On Fri, Apr 3, 2020 at 6:03 PM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Tue, Mar 31, 2020 at 11:23:09PM +0800, Vincent Chen wrote:
> > The XML packet could be supported by required architecture if the
> > architecture defines CONFIG_ACRH_SUPPORTS_GDB_XML and implement its own
> > arch_handle_qxfer_pkt(). Except for the arch_handle_qxfer_pkt(), the
> > architecture also needs to record the feature supported by gdb stub into
> > the arch_gdb_stub_feature, and these features will be reported to host gdb
> > when gdb stub receives the qSupported packet.
> >
> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> > ---
> >  include/linux/kgdb.h   |  9 +++++++++
> >  kernel/debug/gdbstub.c | 13 +++++++++++++
> >  lib/Kconfig.kgdb       |  5 +++++
> >  3 files changed, 27 insertions(+)
> >
> > diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
> > index b072aeb1fd78..ee9109d2f056 100644
> > --- a/include/linux/kgdb.h
> > +++ b/include/linux/kgdb.h
> > @@ -177,6 +177,15 @@ kgdb_arch_handle_exception(int vector, int signo, int err_code,
> >                          struct pt_regs *regs);
> >
> >  /**
> > + *   arch_handle_qxfer_pkt - Handle architecture specific GDB XML packets.
> > + *   @remcom_in_buffer: The buffer of the packet we have read.
> > + *   @remcom_out_buffer: The buffer of %BUFMAX bytes to write a packet into.
> > + */
> > +
> > +extern void
> > +arch_handle_qxfer_pkt(char *remcom_in_buffer, char *remcom_out_buffer);
>
> This should be prefixed kgdb_ like the other arch functions.
>

Ok, I will add the prefixed kgdb_ to the arch_handle_qxfer_pkt().

>
> > +
> > +/**
> >   *   kgdb_call_nmi_hook - Call kgdb_nmicallback() on the current CPU
> >   *   @ignored: This parameter is only here to match the prototype.
> >   *
> > diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
> > index 4b280fc7dd67..d6b1b630a7e7 100644
> > --- a/kernel/debug/gdbstub.c
> > +++ b/kernel/debug/gdbstub.c
> > @@ -792,6 +792,19 @@ static void gdb_cmd_query(struct kgdb_state *ks)
> >               }
> >               break;
> >  #endif
> > +#ifdef CONFIG_ACRH_SUPPORTS_GDB_XML
>
> Typo (and perhaps insufficient testing ;-) ).
>
> Additional the naming of the CONFIG option looks wrong because it
> describes why you added it, not what it actually does. Something
> like CONFIG_HAVE_ARCH_KGDB_QXFER_PKT is more descriptive.
>

OK, I will modify it.

>
> > +     case 'S':
> > +             if (!strncmp(remcom_in_buffer, "qSupported:", 11))
> > +                     strcpy(remcom_out_buffer, arch_gdb_stub_feature);
>
> Has this been declared anywhere? I cannot find it.
>
> This might also benefit from a kgdb_ prefix.
>

I think the supported functions depend on the implementation of the
architectures.
Therefore, I define arch_gdb_stub_feature[] in the
arch/riscv/include/asm/gdb_xml.h.
OK, I will add the kgdb_ prefix to arch_gdb_stub_feature[].
Thanks

>
> > +             break;
> > +     case 'X':
> > +             if (!strncmp(remcom_in_buffer, "qXfer:", 6))
> > +                     arch_handle_qxfer_pkt(remcom_in_buffer,
> > +                                           remcom_out_buffer);
> > +             break;
> > +#endif
> > +     default:
> > +             break;
> >       }
> >  }
> >
> > diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb
> > index 933680b59e2d..5b586a3bba90 100644
> > --- a/lib/Kconfig.kgdb
> > +++ b/lib/Kconfig.kgdb
> > @@ -3,6 +3,11 @@
> >  config HAVE_ARCH_KGDB
> >       bool
> >
> > +# set if architecture implemented the arch_handle_qxfer_pkt function
> > +# to enable gdb stub to address XML packet sent from GDB.
> > +config ARCH_SUPPORTS_GDB_XML
> > +     bool
> > +
> >  menuconfig KGDB
> >       bool "KGDB: kernel debugger"
> >       depends on HAVE_ARCH_KGDB
> > --
> > 2.7.4
> >

[PATCH v2 4/5] riscv: Use the XML target descriptions to report 3 system registers

[Vincent Chen]

The $sstatus, $badaddr, and $scause registers belong to the thread context,
so KGDB can obtain their contents from pt_regs in each trap. However, the
sequential number of these registers in the gdb register list is far from
the general-purpose registers. If riscv port uses the existing method to
report these three registers, many trivial registers with sequence numbers
in the middle of them will also be packaged to the reply packets. To solve
this problem, the riscv port wants to introduce the GDB target description
mechanism to customize the reported register list. By the list, the KGDB
can ignore the intermediate registers and just reports the general-purpose
registers and these three system registers.

Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
---
 arch/riscv/Kconfig               |   1 +
 arch/riscv/include/asm/gdb_xml.h | 117 +++++++++++++++++++++++++++++++++++++++
 arch/riscv/include/asm/kgdb.h    |   8 ++-
 arch/riscv/kernel/kgdb.c         |  14 +++++
 4 files changed, 139 insertions(+), 1 deletion(-)
 create mode 100644 arch/riscv/include/asm/gdb_xml.h

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 108794f4aa45..94b6f301007c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -67,6 +67,7 @@ config RISCV
 	select HAVE_COPY_THREAD_TLS
 	select HAVE_ARCH_KASAN if MMU && 64BIT
 	select HAVE_ARCH_KGDB
+	select ARCH_SUPPORTS_GDB_XML
 
 config ARCH_MMAP_RND_BITS_MIN
 	default 18 if 64BIT
diff --git a/arch/riscv/include/asm/gdb_xml.h b/arch/riscv/include/asm/gdb_xml.h
new file mode 100644
index 000000000000..1d1459d06a1b
--- /dev/null
+++ b/arch/riscv/include/asm/gdb_xml.h
@@ -0,0 +1,117 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef __ASM_GDB_XML_H_
+#define __ASM_GDB_XML_H_
+
+#define arch_gdb_stub_feature riscv_gdb_stub_feature
+static const char riscv_gdb_stub_feature[64] =
+			"PacketSize=800;qXfer:features:read+;";
+
+static const char gdb_xfer_read_target[31] = "qXfer:features:read:target.xml:";
+
+#ifdef CONFIG_64BIT
+static const char gdb_xfer_read_cpuxml[39] =
+			"qXfer:features:read:riscv-64bit-cpu.xml";
+
+static const char riscv_gdb_stub_target_desc[256] =
+"l<?xml version=\"1.0\"?>"
+"<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
+"<target>"
+"<xi:include href=\"riscv-64bit-cpu.xml\"/>"
+"</target>";
+
+static const char riscv_gdb_stub_cpuxml[2048] =
+"l<?xml version=\"1.0\"?>"
+"<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">"
+"<feature name=\"org.gnu.gdb.riscv.cpu\">"
+"<reg name=\""DBG_REG_ZERO"\" bitsize=\"64\" type=\"int\" regnum=\"0\"/>"
+"<reg name=\""DBG_REG_RA"\" bitsize=\"64\" type=\"code_ptr\"/>"
+"<reg name=\""DBG_REG_SP"\" bitsize=\"64\" type=\"data_ptr\"/>"
+"<reg name=\""DBG_REG_GP"\" bitsize=\"64\" type=\"data_ptr\"/>"
+"<reg name=\""DBG_REG_TP"\" bitsize=\"64\" type=\"data_ptr\"/>"
+"<reg name=\""DBG_REG_T0"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_T1"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_T2"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_FP"\" bitsize=\"64\" type=\"data_ptr\"/>"
+"<reg name=\""DBG_REG_S1"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A0"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A1"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A2"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A3"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A4"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A5"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A6"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A7"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S2"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S3"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S4"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S5"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S6"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S7"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S8"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S9"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S10"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S11"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_T3"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_T4"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_T5"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_T6"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_EPC"\" bitsize=\"64\" type=\"code_ptr\"/>"
+"<reg name=\""DBG_REG_STATUS"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_BADADDR"\" bitsize=\"64\" type=\"int\"/>"
+"<reg name=\""DBG_REG_CAUSE"\" bitsize=\"64\" type=\"int\"/>"
+"</feature>";
+#else
+static const char gdb_xfer_read_cpuxml[39] =
+			"qXfer:features:read:riscv-32bit-cpu.xml";
+
+static const char riscv_gdb_stub_target_desc[256] =
+"l<?xml version=\"1.0\"?>"
+"<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
+"<target>"
+"<xi:include href=\"riscv-32bit-cpu.xml\"/>"
+"</target>";
+
+static const char riscv_gdb_stub_cpuxml[2048] =
+"l<?xml version=\"1.0\"?>"
+"<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">"
+"<feature name=\"org.gnu.gdb.riscv.cpu\">"
+"<reg name=\""DBG_REG_ZERO"\" bitsize=\"32\" type=\"int\" regnum=\"0\"/>"
+"<reg name=\""DBG_REG_RA"\" bitsize=\"32\" type=\"code_ptr\"/>"
+"<reg name=\""DBG_REG_SP"\" bitsize=\"32\" type=\"data_ptr\"/>"
+"<reg name=\""DBG_REG_GP"\" bitsize=\"32\" type=\"data_ptr\"/>"
+"<reg name=\""DBG_REG_TP"\" bitsize=\"32\" type=\"data_ptr\"/>"
+"<reg name=\""DBG_REG_T0"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_T1"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_T2"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_FP"\" bitsize=\"32\" type=\"data_ptr\"/>"
+"<reg name=\""DBG_REG_S1"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A0"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A1"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A2"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A3"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A4"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A5"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A6"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_A7"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S2"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S3"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S4"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S5"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S6"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S7"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S8"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S9"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S10"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_S11"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_T3"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_T4"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_T5"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_T6"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_EPC"\" bitsize=\"32\" type=\"code_ptr\"/>"
+"<reg name=\""DBG_REG_STATUS"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_BADADDR"\" bitsize=\"32\" type=\"int\"/>"
+"<reg name=\""DBG_REG_CAUSE"\" bitsize=\"32\" type=\"int\"/>"
+"</feature>";
+#endif
+#endif
diff --git a/arch/riscv/include/asm/kgdb.h b/arch/riscv/include/asm/kgdb.h
index 69bc6a03081d..6c35a853940d 100644
--- a/arch/riscv/include/asm/kgdb.h
+++ b/arch/riscv/include/asm/kgdb.h
@@ -7,7 +7,7 @@
 
 #define GDB_SIZEOF_REG sizeof(unsigned long)
 
-#define DBG_MAX_REG_NUM (33)
+#define DBG_MAX_REG_NUM (36)
 #define NUMREGBYTES ((DBG_MAX_REG_NUM) * GDB_SIZEOF_REG)
 #define CACHE_FLUSH_IS_SAFE     1
 #define BUFMAX                  2048
@@ -66,6 +66,9 @@ static inline void arch_kgdb_breakpoint(void)
 #define DBG_REG_T5 "t5"
 #define DBG_REG_T6 "t6"
 #define DBG_REG_EPC "pc"
+#define DBG_REG_STATUS "sstatus"
+#define DBG_REG_BADADDR "stval"
+#define DBG_REG_CAUSE "scause"
 
 #define DBG_REG_ZERO_OFF 0
 #define DBG_REG_RA_OFF 1
@@ -103,5 +106,8 @@ static inline void arch_kgdb_breakpoint(void)
 #define DBG_REG_STATUS_OFF 33
 #define DBG_REG_BADADDR_OFF 34
 #define DBG_REG_CAUSE_OFF 35
+
+#include <asm/gdb_xml.h>
+
 #endif
 #endif
diff --git a/arch/riscv/kernel/kgdb.c b/arch/riscv/kernel/kgdb.c
index e3b1075c3935..86d891b7ea2c 100644
--- a/arch/riscv/kernel/kgdb.c
+++ b/arch/riscv/kernel/kgdb.c
@@ -7,6 +7,7 @@
 #include <linux/irqflags.h>
 #include <linux/string.h>
 #include <asm/cacheflush.h>
+#include <asm/gdb_xml.h>
 
 enum {
 	NOT_KGDB_BREAK = 0,
@@ -48,6 +49,9 @@ struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
 	{DBG_REG_T5, GDB_SIZEOF_REG, offsetof(struct pt_regs, t5)},
 	{DBG_REG_T6, GDB_SIZEOF_REG, offsetof(struct pt_regs, t6)},
 	{DBG_REG_EPC, GDB_SIZEOF_REG, offsetof(struct pt_regs, epc)},
+	{DBG_REG_STATUS, GDB_SIZEOF_REG, offsetof(struct pt_regs, status)},
+	{DBG_REG_BADADDR, GDB_SIZEOF_REG, offsetof(struct pt_regs, badaddr)},
+	{DBG_REG_CAUSE, GDB_SIZEOF_REG, offsetof(struct pt_regs, cause)},
 };
 
 char *dbg_get_reg(int regno, void *mem, struct pt_regs *regs)
@@ -100,6 +104,16 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc)
 	regs->epc = pc;
 }
 
+void arch_handle_qxfer_pkt(char *remcom_in_buffer, char *remcom_out_buffer)
+{
+	if (!strncmp(remcom_in_buffer, gdb_xfer_read_target,
+		     sizeof(gdb_xfer_read_target)))
+		strcpy(remcom_out_buffer, riscv_gdb_stub_target_desc);
+	else if (!strncmp(remcom_in_buffer, gdb_xfer_read_cpuxml,
+			  sizeof(gdb_xfer_read_cpuxml)))
+		strcpy(remcom_out_buffer, riscv_gdb_stub_cpuxml);
+}
+
 static inline void kgdb_arch_update_addr(struct pt_regs *regs,
 					 char *remcom_in_buffer)
 {
-- 
2.7.4

Re: [PATCH v2 4/5] riscv: Use the XML target descriptions to report 3 system registers

[Palmer Dabbelt]

On Tue, 31 Mar 2020 08:23:10 PDT (-0700), vincent.chen@sifive.com wrote:
> The $sstatus, $badaddr, and $scause registers belong to the thread context,
> so KGDB can obtain their contents from pt_regs in each trap. However, the
> sequential number of these registers in the gdb register list is far from
> the general-purpose registers. If riscv port uses the existing method to
> report these three registers, many trivial registers with sequence numbers
> in the middle of them will also be packaged to the reply packets. To solve
> this problem, the riscv port wants to introduce the GDB target description
> mechanism to customize the reported register list. By the list, the KGDB
> can ignore the intermediate registers and just reports the general-purpose
> registers and these three system registers.
>
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> ---
>  arch/riscv/Kconfig               |   1 +
>  arch/riscv/include/asm/gdb_xml.h | 117 +++++++++++++++++++++++++++++++++++++++
>  arch/riscv/include/asm/kgdb.h    |   8 ++-
>  arch/riscv/kernel/kgdb.c         |  14 +++++
>  4 files changed, 139 insertions(+), 1 deletion(-)
>  create mode 100644 arch/riscv/include/asm/gdb_xml.h
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 108794f4aa45..94b6f301007c 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -67,6 +67,7 @@ config RISCV
>  	select HAVE_COPY_THREAD_TLS
>  	select HAVE_ARCH_KASAN if MMU && 64BIT
>  	select HAVE_ARCH_KGDB
> +	select ARCH_SUPPORTS_GDB_XML
>
>  config ARCH_MMAP_RND_BITS_MIN
>  	default 18 if 64BIT
> diff --git a/arch/riscv/include/asm/gdb_xml.h b/arch/riscv/include/asm/gdb_xml.h
> new file mode 100644
> index 000000000000..1d1459d06a1b
> --- /dev/null
> +++ b/arch/riscv/include/asm/gdb_xml.h
> @@ -0,0 +1,117 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef __ASM_GDB_XML_H_
> +#define __ASM_GDB_XML_H_
> +
> +#define arch_gdb_stub_feature riscv_gdb_stub_feature
> +static const char riscv_gdb_stub_feature[64] =
> +			"PacketSize=800;qXfer:features:read+;";
> +
> +static const char gdb_xfer_read_target[31] = "qXfer:features:read:target.xml:";
> +
> +#ifdef CONFIG_64BIT
> +static const char gdb_xfer_read_cpuxml[39] =
> +			"qXfer:features:read:riscv-64bit-cpu.xml";
> +
> +static const char riscv_gdb_stub_target_desc[256] =
> +"l<?xml version=\"1.0\"?>"
> +"<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
> +"<target>"
> +"<xi:include href=\"riscv-64bit-cpu.xml\"/>"
> +"</target>";
> +
> +static const char riscv_gdb_stub_cpuxml[2048] =
> +"l<?xml version=\"1.0\"?>"
> +"<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">"
> +"<feature name=\"org.gnu.gdb.riscv.cpu\">"
> +"<reg name=\""DBG_REG_ZERO"\" bitsize=\"64\" type=\"int\" regnum=\"0\"/>"
> +"<reg name=\""DBG_REG_RA"\" bitsize=\"64\" type=\"code_ptr\"/>"
> +"<reg name=\""DBG_REG_SP"\" bitsize=\"64\" type=\"data_ptr\"/>"
> +"<reg name=\""DBG_REG_GP"\" bitsize=\"64\" type=\"data_ptr\"/>"
> +"<reg name=\""DBG_REG_TP"\" bitsize=\"64\" type=\"data_ptr\"/>"
> +"<reg name=\""DBG_REG_T0"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_T1"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_T2"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_FP"\" bitsize=\"64\" type=\"data_ptr\"/>"
> +"<reg name=\""DBG_REG_S1"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A0"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A1"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A2"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A3"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A4"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A5"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A6"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A7"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S2"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S3"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S4"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S5"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S6"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S7"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S8"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S9"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S10"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S11"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_T3"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_T4"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_T5"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_T6"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_EPC"\" bitsize=\"64\" type=\"code_ptr\"/>"
> +"<reg name=\""DBG_REG_STATUS"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_BADADDR"\" bitsize=\"64\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_CAUSE"\" bitsize=\"64\" type=\"int\"/>"
> +"</feature>";
> +#else
> +static const char gdb_xfer_read_cpuxml[39] =
> +			"qXfer:features:read:riscv-32bit-cpu.xml";
> +
> +static const char riscv_gdb_stub_target_desc[256] =
> +"l<?xml version=\"1.0\"?>"
> +"<!DOCTYPE target SYSTEM \"gdb-target.dtd\">"
> +"<target>"
> +"<xi:include href=\"riscv-32bit-cpu.xml\"/>"
> +"</target>";
> +
> +static const char riscv_gdb_stub_cpuxml[2048] =
> +"l<?xml version=\"1.0\"?>"
> +"<!DOCTYPE feature SYSTEM \"gdb-target.dtd\">"
> +"<feature name=\"org.gnu.gdb.riscv.cpu\">"
> +"<reg name=\""DBG_REG_ZERO"\" bitsize=\"32\" type=\"int\" regnum=\"0\"/>"
> +"<reg name=\""DBG_REG_RA"\" bitsize=\"32\" type=\"code_ptr\"/>"
> +"<reg name=\""DBG_REG_SP"\" bitsize=\"32\" type=\"data_ptr\"/>"
> +"<reg name=\""DBG_REG_GP"\" bitsize=\"32\" type=\"data_ptr\"/>"
> +"<reg name=\""DBG_REG_TP"\" bitsize=\"32\" type=\"data_ptr\"/>"
> +"<reg name=\""DBG_REG_T0"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_T1"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_T2"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_FP"\" bitsize=\"32\" type=\"data_ptr\"/>"
> +"<reg name=\""DBG_REG_S1"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A0"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A1"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A2"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A3"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A4"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A5"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A6"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_A7"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S2"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S3"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S4"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S5"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S6"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S7"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S8"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S9"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S10"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_S11"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_T3"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_T4"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_T5"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_T6"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_EPC"\" bitsize=\"32\" type=\"code_ptr\"/>"
> +"<reg name=\""DBG_REG_STATUS"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_BADADDR"\" bitsize=\"32\" type=\"int\"/>"
> +"<reg name=\""DBG_REG_CAUSE"\" bitsize=\"32\" type=\"int\"/>"
> +"</feature>";
> +#endif
> +#endif
> diff --git a/arch/riscv/include/asm/kgdb.h b/arch/riscv/include/asm/kgdb.h
> index 69bc6a03081d..6c35a853940d 100644
> --- a/arch/riscv/include/asm/kgdb.h
> +++ b/arch/riscv/include/asm/kgdb.h
> @@ -7,7 +7,7 @@
>
>  #define GDB_SIZEOF_REG sizeof(unsigned long)
>
> -#define DBG_MAX_REG_NUM (33)
> +#define DBG_MAX_REG_NUM (36)
>  #define NUMREGBYTES ((DBG_MAX_REG_NUM) * GDB_SIZEOF_REG)
>  #define CACHE_FLUSH_IS_SAFE     1
>  #define BUFMAX                  2048
> @@ -66,6 +66,9 @@ static inline void arch_kgdb_breakpoint(void)
>  #define DBG_REG_T5 "t5"
>  #define DBG_REG_T6 "t6"
>  #define DBG_REG_EPC "pc"
> +#define DBG_REG_STATUS "sstatus"
> +#define DBG_REG_BADADDR "stval"
> +#define DBG_REG_CAUSE "scause"
>
>  #define DBG_REG_ZERO_OFF 0
>  #define DBG_REG_RA_OFF 1
> @@ -103,5 +106,8 @@ static inline void arch_kgdb_breakpoint(void)
>  #define DBG_REG_STATUS_OFF 33
>  #define DBG_REG_BADADDR_OFF 34
>  #define DBG_REG_CAUSE_OFF 35
> +
> +#include <asm/gdb_xml.h>
> +
>  #endif
>  #endif
> diff --git a/arch/riscv/kernel/kgdb.c b/arch/riscv/kernel/kgdb.c
> index e3b1075c3935..86d891b7ea2c 100644
> --- a/arch/riscv/kernel/kgdb.c
> +++ b/arch/riscv/kernel/kgdb.c
> @@ -7,6 +7,7 @@
>  #include <linux/irqflags.h>
>  #include <linux/string.h>
>  #include <asm/cacheflush.h>
> +#include <asm/gdb_xml.h>
>
>  enum {
>  	NOT_KGDB_BREAK = 0,
> @@ -48,6 +49,9 @@ struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
>  	{DBG_REG_T5, GDB_SIZEOF_REG, offsetof(struct pt_regs, t5)},
>  	{DBG_REG_T6, GDB_SIZEOF_REG, offsetof(struct pt_regs, t6)},
>  	{DBG_REG_EPC, GDB_SIZEOF_REG, offsetof(struct pt_regs, epc)},
> +	{DBG_REG_STATUS, GDB_SIZEOF_REG, offsetof(struct pt_regs, status)},
> +	{DBG_REG_BADADDR, GDB_SIZEOF_REG, offsetof(struct pt_regs, badaddr)},
> +	{DBG_REG_CAUSE, GDB_SIZEOF_REG, offsetof(struct pt_regs, cause)},
>  };
>
>  char *dbg_get_reg(int regno, void *mem, struct pt_regs *regs)
> @@ -100,6 +104,16 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long pc)
>  	regs->epc = pc;
>  }
>
> +void arch_handle_qxfer_pkt(char *remcom_in_buffer, char *remcom_out_buffer)
> +{
> +	if (!strncmp(remcom_in_buffer, gdb_xfer_read_target,
> +		     sizeof(gdb_xfer_read_target)))
> +		strcpy(remcom_out_buffer, riscv_gdb_stub_target_desc);
> +	else if (!strncmp(remcom_in_buffer, gdb_xfer_read_cpuxml,
> +			  sizeof(gdb_xfer_read_cpuxml)))
> +		strcpy(remcom_out_buffer, riscv_gdb_stub_cpuxml);
> +}
> +
>  static inline void kgdb_arch_update_addr(struct pt_regs *regs,
>  					 char *remcom_in_buffer)
>  {

Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>

[PATCH v2 5/5] riscv: Add SW single-step support for KDB

[Vincent Chen]

In KGDB, the GDB in the host is responsible for the single-step operation
of the software. In other words, KGDB does not need to derive the next pc
address when performing a software single-step operation. KGDB just inserts
the break instruction at the indicated address according to the GDB
instructions. This approach does not work in KDB because the GDB does not
involve the KDB process. Therefore, this patch provides KDB a software
single-step mechanism to use.

Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
---
 arch/riscv/include/asm/parse_asm.h | 214 +++++++++++++++++++++++++++++++++++++
 arch/riscv/kernel/kgdb.c           | 171 ++++++++++++++++++++++++++++-
 2 files changed, 384 insertions(+), 1 deletion(-)
 create mode 100644 arch/riscv/include/asm/parse_asm.h

diff --git a/arch/riscv/include/asm/parse_asm.h b/arch/riscv/include/asm/parse_asm.h
new file mode 100644
index 000000000000..7e3291fcc8b6
--- /dev/null
+++ b/arch/riscv/include/asm/parse_asm.h
@@ -0,0 +1,214 @@
+#include <linux/bits.h>
+
+/* The bit field of immediate value in I-type instruction */
+#define I_IMM_SIGN_OPOFF	31
+#define I_IMM_11_0_OPOFF	20
+#define I_IMM_SIGN_OFF		12
+#define I_IMM_11_0_OFF		0
+#define I_IMM_11_0_MASK		GENMASK(11, 0)
+
+/* The bit field of immediate value in J-type instruction */
+#define J_IMM_SIGN_OPOFF	31
+#define J_IMM_10_1_OPOFF	21
+#define J_IMM_11_OPOFF		20
+#define J_IMM_19_12_OPOFF	12
+#define J_IMM_SIGN_OFF		20
+#define J_IMM_10_1_OFF		1
+#define J_IMM_11_OFF		11
+#define J_IMM_19_12_OFF 	12
+#define J_IMM_10_1_MASK		GENMASK(9, 0)
+#define J_IMM_11_MASK		GENMASK(0, 0)
+#define J_IMM_19_12_MASK	GENMASK(7, 0)
+
+/* The bit field of immediate value in B-type instruction */
+#define B_IMM_SIGN_OPOFF	31
+#define B_IMM_10_5_OPOFF	25
+#define B_IMM_4_1_OPOFF		8
+#define B_IMM_11_OPOFF		7
+#define B_IMM_SIGN_OFF		12
+#define B_IMM_10_5_OFF		5
+#define B_IMM_4_1_OFF		1
+#define B_IMM_11_OFF		11
+#define B_IMM_10_5_MASK		GENMASK(5, 0)
+#define B_IMM_4_1_MASK		GENMASK(3, 0)
+#define B_IMM_11_MASK		GENMASK(0, 0)
+
+/* The register offset in RVG instruction */
+#define RVG_RS1_OPOFF		15
+#define RVG_RS2_OPOFF		20
+#define RVG_RD_OPOFF		7
+
+/* The bit field of immediate value in RVC J instruction */
+#define RVC_J_IMM_SIGN_OPOFF	12
+#define RVC_J_IMM_4_OPOFF	11
+#define RVC_J_IMM_9_8_OPOFF	9
+#define RVC_J_IMM_10_OPOFF	8
+#define RVC_J_IMM_6_OPOFF	7
+#define RVC_J_IMM_7_OPOFF	6
+#define RVC_J_IMM_3_1_OPOFF	3
+#define RVC_J_IMM_5_OPOFF	2
+#define RVC_J_IMM_SIGN_OFF	11
+#define RVC_J_IMM_4_OFF		4
+#define RVC_J_IMM_9_8_OFF	8
+#define RVC_J_IMM_10_OFF	10
+#define RVC_J_IMM_6_OFF		6
+#define RVC_J_IMM_7_OFF		7
+#define RVC_J_IMM_3_1_OFF	1
+#define RVC_J_IMM_5_OFF		5
+#define RVC_J_IMM_4_MASK	GENMASK(0, 0)
+#define RVC_J_IMM_9_8_MASK	GENMASK(1, 0)
+#define RVC_J_IMM_10_MASK	GENMASK(0, 0)
+#define RVC_J_IMM_6_MASK	GENMASK(0, 0)
+#define RVC_J_IMM_7_MASK	GENMASK(0, 0)
+#define RVC_J_IMM_3_1_MASK	GENMASK(2, 0)
+#define RVC_J_IMM_5_MASK	GENMASK(0, 0)
+
+/* The bit field of immediate value in RVC B instruction */
+#define RVC_B_IMM_SIGN_OPOFF	12
+#define RVC_B_IMM_4_3_OPOFF	10
+#define RVC_B_IMM_7_6_OPOFF	5
+#define RVC_B_IMM_2_1_OPOFF	3
+#define RVC_B_IMM_5_OPOFF	2
+#define RVC_B_IMM_SIGN_OFF	8
+#define RVC_B_IMM_4_3_OFF	3
+#define RVC_B_IMM_7_6_OFF	6
+#define RVC_B_IMM_2_1_OFF	1
+#define RVC_B_IMM_5_OFF		5
+#define RVC_B_IMM_4_3_MASK	GENMASK(1, 0)
+#define RVC_B_IMM_7_6_MASK	GENMASK(1, 0)
+#define RVC_B_IMM_2_1_MASK	GENMASK(1, 0)
+#define RVC_B_IMM_5_MASK	GENMASK(0, 0)
+
+/* The register offset in RVC op=C0 instruction */
+#define RVC_C0_RS1_OPOFF	7
+#define RVC_C0_RS2_OPOFF	2
+#define RVC_C0_RD_OPOFF		2
+
+/* The register offset in RVC op=C1 instruction */
+#define RVC_C1_RS1_OPOFF	7
+#define RVC_C1_RS2_OPOFF	2
+#define RVC_C1_RD_OPOFF		7
+
+/* The register offset in RVC op=C2 instruction */
+#define RVC_C2_RS1_OPOFF	7
+#define RVC_C2_RS2_OPOFF	2
+#define RVC_C2_RD_OPOFF		7
+
+/* parts of opcode for RVG*/
+#define OPCODE_BRANCH		0x63
+#define OPCODE_JALR		0x67
+#define OPCODE_JAL		0x6f
+#define OPCODE_SYSTEM		0x73
+
+/* parts of opcode for RVC*/
+#define OPCODE_C_0		0x0
+#define OPCODE_C_1		0x1
+#define OPCODE_C_2		0x2
+
+/* parts of funct3 code for I, M, A extension*/
+#define FUNCT3_JALR		0x0
+#define FUNCT3_BEQ		0x0
+#define FUNCT3_BNE		0x1000
+#define FUNCT3_BLT		0x4000
+#define FUNCT3_BGE		0x5000
+#define FUNCT3_BLTU		0x6000
+#define FUNCT3_BGEU		0x7000
+
+/* parts of funct3 code for C extension*/
+#define FUNCT3_C_BEQZ		0xc000
+#define FUNCT3_C_BNEZ		0xe000
+#define FUNCT3_C_J		0xa000
+#define FUNCT3_C_JAL		0x2000
+#define FUNCT4_C_JR		0x8000
+#define FUNCT4_C_JALR		0xf000
+
+#define FUNCT12_SRET		0x10200000
+
+#define MATCH_JALR		(FUNCT3_JALR | OPCODE_JALR)
+#define MATCH_JAL		(OPCODE_JAL)
+#define MATCH_BEQ		(FUNCT3_BEQ | OPCODE_BRANCH)
+#define MATCH_BNE		(FUNCT3_BNE | OPCODE_BRANCH)
+#define MATCH_BLT		(FUNCT3_BLT | OPCODE_BRANCH)
+#define MATCH_BGE		(FUNCT3_BGE | OPCODE_BRANCH)
+#define MATCH_BLTU		(FUNCT3_BLTU | OPCODE_BRANCH)
+#define MATCH_BGEU		(FUNCT3_BGEU | OPCODE_BRANCH)
+#define MATCH_SRET		(FUNCT12_SRET | OPCODE_SYSTEM)
+#define MATCH_C_BEQZ		(FUNCT3_C_BEQZ | OPCODE_C_1)
+#define MATCH_C_BNEZ		(FUNCT3_C_BNEZ | OPCODE_C_1)
+#define MATCH_C_J		(FUNCT3_C_J | OPCODE_C_1)
+#define MATCH_C_JAL		(FUNCT3_C_JAL | OPCODE_C_1)
+#define MATCH_C_JR		(FUNCT4_C_JR | OPCODE_C_2)
+#define MATCH_C_JALR		(FUNCT4_C_JALR | OPCODE_C_2)
+
+#define MASK_JALR		0x707f
+#define MASK_JAL		0x7f
+#define MASK_C_JALR		0xf07f
+#define MASK_C_JR		0xf07f
+#define MASK_C_JAL		0xe003
+#define MASK_C_J		0xe003
+#define MASK_BEQ		0x707f
+#define MASK_BNE		0x707f
+#define MASK_BLT		0x707f
+#define MASK_BGE		0x707f
+#define MASK_BLTU		0x707f
+#define MASK_BGEU		0x707f
+#define MASK_C_BEQZ		0xe003
+#define MASK_C_BNEZ		0xe003
+#define MASK_SRET		0xffffffff
+
+#define __INSN_LENGTH_MASK	_UL(0x3)
+#define __INSN_LENGTH_GE_32	_UL(0x3)
+#define __INSN_OPCODE_MASK	_UL(0x7F)
+#define __INSN_BRANCH_OPCODE	_UL(OPCODE_BRANCH)
+
+/* Define a series of is_XXX_insn functions to check if the value INSN
+ * is an instance of instruction XXX.
+ */
+#define DECLARE_INSN(INSN_NAME, INSN_MATCH, INSN_MASK) \
+static inline bool is_ ## INSN_NAME ## _insn(long insn) \
+{ \
+	return (insn & (INSN_MASK)) == (INSN_MATCH); \
+}
+
+#define RV_IMM_SIGN(x) (-(((x) >> 31) & 1))
+#define RVC_IMM_SIGN(x) (-(((x) >> 12) & 1))
+#define RV_X(X, s, mask)  (((X) >> (s)) & (mask))
+#define RVC_X(X, s, mask) RV_X(X, s, mask)
+
+#define EXTRACT_JTYPE_IMM(x) \
+	({typeof(x) x_ = (x); \
+	(RV_X(x_, J_IMM_10_1_OPOFF, J_IMM_10_1_MASK) << J_IMM_10_1_OFF) | \
+	(RV_X(x_, J_IMM_11_OPOFF, J_IMM_11_MASK) << J_IMM_11_OFF) | \
+	(RV_X(x_, J_IMM_19_12_OPOFF, J_IMM_19_12_MASK) << J_IMM_19_12_OFF) | \
+	(RV_IMM_SIGN(x_) << J_IMM_SIGN_OFF); })
+
+#define EXTRACT_ITYPE_IMM(x) \
+	({typeof(x) x_ = (x); \
+	(RV_X(x_, I_IMM_11_0_OPOFF, I_IMM_11_0_MASK)) | \
+	(RV_IMM_SIGN(x_) << I_IMM_SIGN_OFF); })
+
+#define EXTRACT_BTYPE_IMM(x) \
+	({typeof(x) x_ = (x); \
+	(RV_X(x_, B_IMM_4_1_OPOFF, B_IMM_4_1_MASK) << B_IMM_4_1_OFF) | \
+	(RV_X(x_, B_IMM_10_5_OPOFF, B_IMM_10_5_MASK) << B_IMM_10_5_OFF) | \
+	(RV_X(x_, B_IMM_11_OPOFF, B_IMM_11_MASK) << B_IMM_11_OFF) | \
+	(RV_IMM_SIGN(x_) << B_IMM_SIGN_OFF); })
+
+#define EXTRACT_RVC_J_IMM(x) \
+	({typeof(x) x_ = (x); \
+	(RVC_X(x_, RVC_J_IMM_3_1_OPOFF, RVC_J_IMM_3_1_MASK) << RVC_J_IMM_3_1_OFF) | \
+	(RVC_X(x_, RVC_J_IMM_4_OPOFF, RVC_J_IMM_4_MASK) << RVC_J_IMM_4_OFF) | \
+	(RVC_X(x_, RVC_J_IMM_5_OPOFF, RVC_J_IMM_5_MASK) << RVC_J_IMM_5_OFF) | \
+	(RVC_X(x_, RVC_J_IMM_6_OPOFF, RVC_J_IMM_6_MASK) << RVC_J_IMM_6_OFF) | \
+	(RVC_X(x_, RVC_J_IMM_7_OPOFF, RVC_J_IMM_7_MASK) << RVC_J_IMM_7_OFF) | \
+	(RVC_X(x_, RVC_J_IMM_9_8_OPOFF, RVC_J_IMM_9_8_MASK) << RVC_J_IMM_9_8_OFF) | \
+	(RVC_X(x_, RVC_J_IMM_10_OPOFF, RVC_J_IMM_10_MASK) << RVC_J_IMM_10_OFF) | \
+	(RVC_IMM_SIGN(x_) << RVC_J_IMM_SIGN_OFF); })
+
+#define EXTRACT_RVC_B_IMM(x) \
+	({typeof(x) x_ = (x); \
+	(RVC_X(x_, RVC_B_IMM_2_1_OPOFF, RVC_B_IMM_2_1_MASK) << RVC_B_IMM_2_1_OFF) | \
+	(RVC_X(x_, RVC_B_IMM_4_3_OPOFF, RVC_B_IMM_4_3_MASK) << RVC_B_IMM_4_3_OFF) | \
+	(RVC_X(x_, RVC_B_IMM_5_OPOFF, RVC_B_IMM_5_MASK) << RVC_B_IMM_5_OFF) | \
+	(RVC_X(x_, RVC_B_IMM_7_6_OPOFF, RVC_B_IMM_7_6_MASK) << RVC_B_IMM_7_6_OFF) | \
+	(RVC_IMM_SIGN(x_) << RVC_B_IMM_SIGN_OFF); })
diff --git a/arch/riscv/kernel/kgdb.c b/arch/riscv/kernel/kgdb.c
index 86d891b7ea2c..2debc88f0a8b 100644
--- a/arch/riscv/kernel/kgdb.c
+++ b/arch/riscv/kernel/kgdb.c
@@ -8,13 +8,168 @@
 #include <linux/string.h>
 #include <asm/cacheflush.h>
 #include <asm/gdb_xml.h>
+#include <asm/parse_asm.h>
 
 enum {
 	NOT_KGDB_BREAK = 0,
 	KGDB_SW_BREAK,
 	KGDB_COMPILED_BREAK,
+	KGDB_SW_SINGLE_STEP
 };
 
+static unsigned long stepped_address;
+static unsigned int stepped_opcode;
+
+#if __riscv_xlen == 32
+/* C.JAL is an RV32C-only instruction */
+DECLARE_INSN(c_jal, MATCH_C_JAL, MASK_C_JAL)
+#else
+#define is_c_jal_insn(opcode) 0
+#endif
+DECLARE_INSN(jalr, MATCH_JALR, MASK_JALR)
+DECLARE_INSN(jal, MATCH_JAL, MASK_JAL)
+DECLARE_INSN(c_jr, MATCH_C_JR, MASK_C_JR)
+DECLARE_INSN(c_jalr, MATCH_C_JALR, MASK_C_JALR)
+DECLARE_INSN(c_j, MATCH_C_J, MASK_C_J)
+DECLARE_INSN(beq, MATCH_BEQ, MASK_BEQ)
+DECLARE_INSN(bne, MATCH_BNE, MASK_BNE)
+DECLARE_INSN(blt, MATCH_BLT, MASK_BLT)
+DECLARE_INSN(bge, MATCH_BGE, MASK_BGE)
+DECLARE_INSN(bltu, MATCH_BLTU, MASK_BLTU)
+DECLARE_INSN(bgeu, MATCH_BGEU, MASK_BGEU)
+DECLARE_INSN(c_beqz, MATCH_C_BEQZ, MASK_C_BEQZ)
+DECLARE_INSN(c_bnez, MATCH_C_BNEZ, MASK_C_BNEZ)
+DECLARE_INSN(sret, MATCH_SRET, MASK_SRET)
+
+int decode_register_index(unsigned long opcode, int offset)
+{
+	return (opcode >> offset) & 0x1F;
+}
+
+int decode_register_index_short(unsigned long opcode, int offset)
+{
+	return ((opcode >> offset) & 0x7) + 8;
+}
+
+/* Calculate the new address for after a step */
+int get_step_address(struct pt_regs *regs, unsigned long *next_addr)
+{
+	unsigned long pc = regs->epc;
+	unsigned long *regs_ptr = (unsigned long *)regs;
+	unsigned int rs1_num, rs2_num;
+	int op_code;
+
+	if (probe_kernel_address((void *)pc, op_code))
+		return -EINVAL;
+	if ((op_code & __INSN_LENGTH_MASK) != __INSN_LENGTH_GE_32) {
+		if (is_c_jalr_insn(op_code) || is_c_jr_insn(op_code)) {
+			rs1_num = decode_register_index(op_code, RVC_C2_RS1_OPOFF);
+			*next_addr = regs_ptr[rs1_num];
+		} else if (is_c_j_insn(op_code) || is_c_jal_insn(op_code)) {
+			*next_addr = EXTRACT_RVC_J_IMM(op_code) + pc;
+		} else if (is_c_beqz_insn(op_code)) {
+			rs1_num = decode_register_index_short(op_code,
+							      RVC_C1_RS1_OPOFF);
+			if (!rs1_num || regs_ptr[rs1_num] == 0)
+				*next_addr = EXTRACT_RVC_B_IMM(op_code) + pc;
+			else
+				*next_addr = pc + 2;
+		} else if (is_c_bnez_insn(op_code)) {
+			rs1_num =
+			    decode_register_index_short(op_code, RVC_C1_RS1_OPOFF);
+			if (rs1_num && regs_ptr[rs1_num] != 0)
+				*next_addr = EXTRACT_RVC_B_IMM(op_code) + pc;
+			else
+				*next_addr = pc + 2;
+		} else {
+			*next_addr = pc + 2;
+		}
+	} else {
+		if ((op_code & __INSN_OPCODE_MASK) == __INSN_BRANCH_OPCODE) {
+			bool result = false;
+			long imm = EXTRACT_BTYPE_IMM(op_code);
+			unsigned long rs1_val = 0, rs2_val = 0;
+
+			rs1_num = decode_register_index(op_code, RVG_RS1_OPOFF);
+			rs2_num = decode_register_index(op_code, RVG_RS2_OPOFF);
+			if (rs1_num)
+				rs1_val = regs_ptr[rs1_num];
+			if (rs2_num)
+				rs2_val = regs_ptr[rs2_num];
+
+			if (is_beq_insn(op_code))
+				result = (rs1_val == rs2_val) ? true : false;
+			else if (is_bne_insn(op_code))
+				result = (rs1_val != rs2_val) ? true : false;
+			else if (is_blt_insn(op_code))
+				result =
+				    ((long)rs1_val <
+				     (long)rs2_val) ? true : false;
+			else if (is_bge_insn(op_code))
+				result =
+				    ((long)rs1_val >=
+				     (long)rs2_val) ? true : false;
+			else if (is_bltu_insn(op_code))
+				result = (rs1_val < rs2_val) ? true : false;
+			else if (is_bgeu_insn(op_code))
+				result = (rs1_val >= rs2_val) ? true : false;
+			if (result)
+				*next_addr = imm + pc;
+			else
+				*next_addr = pc + 4;
+		} else if (is_jal_insn(op_code)) {
+			*next_addr = EXTRACT_JTYPE_IMM(op_code) + pc;
+		} else if (is_jalr_insn(op_code)) {
+			rs1_num = decode_register_index(op_code, RVG_RS1_OPOFF);
+			if (rs1_num)
+				*next_addr = ((unsigned long *)regs)[rs1_num];
+			*next_addr += EXTRACT_ITYPE_IMM(op_code);
+		} else if (is_sret_insn(op_code)) {
+			*next_addr = pc;
+		} else {
+			*next_addr = pc + 4;
+		}
+	}
+	return 0;
+}
+
+int do_single_step(struct pt_regs *regs)
+{
+	/* Determine where the target instruction will send us to */
+	unsigned long addr = 0;
+	int error = get_step_address(regs, &addr);
+
+	if (error)
+		return error;
+
+	stepped_address = addr;
+
+	/* Store the op code in the stepped address */
+	probe_kernel_address((void *)addr, stepped_opcode);
+	/* Replace the op code with the break instruction */
+	error = probe_kernel_write((void *)addr,
+				   arch_kgdb_ops.gdb_bpt_instr,
+				   BREAK_INSTR_SIZE);
+	/* Flush and return */
+	if (!error)
+		flush_icache_range(addr, addr + BREAK_INSTR_SIZE);
+
+	return error;
+}
+
+/* Undo a single step */
+static void undo_single_step(struct pt_regs *regs)
+{
+	if (stepped_opcode != 0) {
+		probe_kernel_write((void *)stepped_address,
+				   (void *)&stepped_opcode, BREAK_INSTR_SIZE);
+		flush_icache_range(stepped_address,
+				   stepped_address + BREAK_INSTR_SIZE);
+	}
+	stepped_address = 0;
+	stepped_opcode = 0;
+}
+
 struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
 	{DBG_REG_ZERO, GDB_SIZEOF_REG, -1},
 	{DBG_REG_RA, GDB_SIZEOF_REG, offsetof(struct pt_regs, ra)},
@@ -131,6 +286,8 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
 {
 	int err = 0;
 
+	undo_single_step(regs);
+
 	switch (remcom_in_buffer[0]) {
 	case 'c':
 	case 'D':
@@ -140,6 +297,15 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
 		atomic_set(&kgdb_cpu_doing_single_step, -1);
 		kgdb_single_step = 0;
 		break;
+	case 's':
+		kgdb_arch_update_addr(regs, remcom_in_buffer);
+		err = do_single_step(regs);
+		if (!err) {
+			kgdb_single_step = 1;
+			atomic_set(&kgdb_cpu_doing_single_step,
+				   raw_smp_processor_id());
+		}
+		break;
 	default:
 		err = -1;
 	}
@@ -149,6 +315,8 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
 
 int kgdb_riscv_kgdbbreak(unsigned long addr)
 {
+	if (stepped_address == addr)
+		return KGDB_SW_SINGLE_STEP;
 	if (atomic_read(&kgdb_setting_breakpoint))
 		if (addr == (unsigned long)&kgdb_compiled_break)
 			return KGDB_COMPILED_BREAK;
@@ -172,7 +340,8 @@ static int kgdb_riscv_notify(struct notifier_block *self, unsigned long cmd,
 		return NOTIFY_DONE;
 
 	local_irq_save(flags);
-	if (kgdb_handle_exception(1, args->signr, cmd, regs))
+	if (kgdb_handle_exception(type == KGDB_SW_SINGLE_STEP ? 0 : 1,
+				  args->signr, cmd, regs))
 		return NOTIFY_DONE;
 
 	if (type == KGDB_COMPILED_BREAK)
-- 
2.7.4

Re: [PATCH v2 5/5] riscv: Add SW single-step support for KDB

[Palmer Dabbelt]

On Tue, 31 Mar 2020 08:23:11 PDT (-0700), vincent.chen@sifive.com wrote:
> In KGDB, the GDB in the host is responsible for the single-step operation
> of the software. In other words, KGDB does not need to derive the next pc
> address when performing a software single-step operation. KGDB just inserts
> the break instruction at the indicated address according to the GDB
> instructions. This approach does not work in KDB because the GDB does not
> involve the KDB process. Therefore, this patch provides KDB a software
> single-step mechanism to use.
>
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> ---
>  arch/riscv/include/asm/parse_asm.h | 214 +++++++++++++++++++++++++++++++++++++
>  arch/riscv/kernel/kgdb.c           | 171 ++++++++++++++++++++++++++++-
>  2 files changed, 384 insertions(+), 1 deletion(-)
>  create mode 100644 arch/riscv/include/asm/parse_asm.h
>
> diff --git a/arch/riscv/include/asm/parse_asm.h b/arch/riscv/include/asm/parse_asm.h
> new file mode 100644
> index 000000000000..7e3291fcc8b6
> --- /dev/null
> +++ b/arch/riscv/include/asm/parse_asm.h
> @@ -0,0 +1,214 @@
> +#include <linux/bits.h>
> +
> +/* The bit field of immediate value in I-type instruction */
> +#define I_IMM_SIGN_OPOFF	31
> +#define I_IMM_11_0_OPOFF	20
> +#define I_IMM_SIGN_OFF		12
> +#define I_IMM_11_0_OFF		0
> +#define I_IMM_11_0_MASK		GENMASK(11, 0)
> +
> +/* The bit field of immediate value in J-type instruction */
> +#define J_IMM_SIGN_OPOFF	31
> +#define J_IMM_10_1_OPOFF	21
> +#define J_IMM_11_OPOFF		20
> +#define J_IMM_19_12_OPOFF	12
> +#define J_IMM_SIGN_OFF		20
> +#define J_IMM_10_1_OFF		1
> +#define J_IMM_11_OFF		11
> +#define J_IMM_19_12_OFF 	12
> +#define J_IMM_10_1_MASK		GENMASK(9, 0)
> +#define J_IMM_11_MASK		GENMASK(0, 0)
> +#define J_IMM_19_12_MASK	GENMASK(7, 0)
> +
> +/* The bit field of immediate value in B-type instruction */
> +#define B_IMM_SIGN_OPOFF	31
> +#define B_IMM_10_5_OPOFF	25
> +#define B_IMM_4_1_OPOFF		8
> +#define B_IMM_11_OPOFF		7
> +#define B_IMM_SIGN_OFF		12
> +#define B_IMM_10_5_OFF		5
> +#define B_IMM_4_1_OFF		1
> +#define B_IMM_11_OFF		11
> +#define B_IMM_10_5_MASK		GENMASK(5, 0)
> +#define B_IMM_4_1_MASK		GENMASK(3, 0)
> +#define B_IMM_11_MASK		GENMASK(0, 0)
> +
> +/* The register offset in RVG instruction */
> +#define RVG_RS1_OPOFF		15
> +#define RVG_RS2_OPOFF		20
> +#define RVG_RD_OPOFF		7
> +
> +/* The bit field of immediate value in RVC J instruction */
> +#define RVC_J_IMM_SIGN_OPOFF	12
> +#define RVC_J_IMM_4_OPOFF	11
> +#define RVC_J_IMM_9_8_OPOFF	9
> +#define RVC_J_IMM_10_OPOFF	8
> +#define RVC_J_IMM_6_OPOFF	7
> +#define RVC_J_IMM_7_OPOFF	6
> +#define RVC_J_IMM_3_1_OPOFF	3
> +#define RVC_J_IMM_5_OPOFF	2
> +#define RVC_J_IMM_SIGN_OFF	11
> +#define RVC_J_IMM_4_OFF		4
> +#define RVC_J_IMM_9_8_OFF	8
> +#define RVC_J_IMM_10_OFF	10
> +#define RVC_J_IMM_6_OFF		6
> +#define RVC_J_IMM_7_OFF		7
> +#define RVC_J_IMM_3_1_OFF	1
> +#define RVC_J_IMM_5_OFF		5
> +#define RVC_J_IMM_4_MASK	GENMASK(0, 0)
> +#define RVC_J_IMM_9_8_MASK	GENMASK(1, 0)
> +#define RVC_J_IMM_10_MASK	GENMASK(0, 0)
> +#define RVC_J_IMM_6_MASK	GENMASK(0, 0)
> +#define RVC_J_IMM_7_MASK	GENMASK(0, 0)
> +#define RVC_J_IMM_3_1_MASK	GENMASK(2, 0)
> +#define RVC_J_IMM_5_MASK	GENMASK(0, 0)
> +
> +/* The bit field of immediate value in RVC B instruction */
> +#define RVC_B_IMM_SIGN_OPOFF	12
> +#define RVC_B_IMM_4_3_OPOFF	10
> +#define RVC_B_IMM_7_6_OPOFF	5
> +#define RVC_B_IMM_2_1_OPOFF	3
> +#define RVC_B_IMM_5_OPOFF	2
> +#define RVC_B_IMM_SIGN_OFF	8
> +#define RVC_B_IMM_4_3_OFF	3
> +#define RVC_B_IMM_7_6_OFF	6
> +#define RVC_B_IMM_2_1_OFF	1
> +#define RVC_B_IMM_5_OFF		5
> +#define RVC_B_IMM_4_3_MASK	GENMASK(1, 0)
> +#define RVC_B_IMM_7_6_MASK	GENMASK(1, 0)
> +#define RVC_B_IMM_2_1_MASK	GENMASK(1, 0)
> +#define RVC_B_IMM_5_MASK	GENMASK(0, 0)
> +
> +/* The register offset in RVC op=C0 instruction */
> +#define RVC_C0_RS1_OPOFF	7
> +#define RVC_C0_RS2_OPOFF	2
> +#define RVC_C0_RD_OPOFF		2
> +
> +/* The register offset in RVC op=C1 instruction */
> +#define RVC_C1_RS1_OPOFF	7
> +#define RVC_C1_RS2_OPOFF	2
> +#define RVC_C1_RD_OPOFF		7
> +
> +/* The register offset in RVC op=C2 instruction */
> +#define RVC_C2_RS1_OPOFF	7
> +#define RVC_C2_RS2_OPOFF	2
> +#define RVC_C2_RD_OPOFF		7
> +
> +/* parts of opcode for RVG*/
> +#define OPCODE_BRANCH		0x63
> +#define OPCODE_JALR		0x67
> +#define OPCODE_JAL		0x6f
> +#define OPCODE_SYSTEM		0x73
> +
> +/* parts of opcode for RVC*/
> +#define OPCODE_C_0		0x0
> +#define OPCODE_C_1		0x1
> +#define OPCODE_C_2		0x2
> +
> +/* parts of funct3 code for I, M, A extension*/
> +#define FUNCT3_JALR		0x0
> +#define FUNCT3_BEQ		0x0
> +#define FUNCT3_BNE		0x1000
> +#define FUNCT3_BLT		0x4000
> +#define FUNCT3_BGE		0x5000
> +#define FUNCT3_BLTU		0x6000
> +#define FUNCT3_BGEU		0x7000
> +
> +/* parts of funct3 code for C extension*/
> +#define FUNCT3_C_BEQZ		0xc000
> +#define FUNCT3_C_BNEZ		0xe000
> +#define FUNCT3_C_J		0xa000
> +#define FUNCT3_C_JAL		0x2000
> +#define FUNCT4_C_JR		0x8000
> +#define FUNCT4_C_JALR		0xf000
> +
> +#define FUNCT12_SRET		0x10200000
> +
> +#define MATCH_JALR		(FUNCT3_JALR | OPCODE_JALR)
> +#define MATCH_JAL		(OPCODE_JAL)
> +#define MATCH_BEQ		(FUNCT3_BEQ | OPCODE_BRANCH)
> +#define MATCH_BNE		(FUNCT3_BNE | OPCODE_BRANCH)
> +#define MATCH_BLT		(FUNCT3_BLT | OPCODE_BRANCH)
> +#define MATCH_BGE		(FUNCT3_BGE | OPCODE_BRANCH)
> +#define MATCH_BLTU		(FUNCT3_BLTU | OPCODE_BRANCH)
> +#define MATCH_BGEU		(FUNCT3_BGEU | OPCODE_BRANCH)
> +#define MATCH_SRET		(FUNCT12_SRET | OPCODE_SYSTEM)
> +#define MATCH_C_BEQZ		(FUNCT3_C_BEQZ | OPCODE_C_1)
> +#define MATCH_C_BNEZ		(FUNCT3_C_BNEZ | OPCODE_C_1)
> +#define MATCH_C_J		(FUNCT3_C_J | OPCODE_C_1)
> +#define MATCH_C_JAL		(FUNCT3_C_JAL | OPCODE_C_1)
> +#define MATCH_C_JR		(FUNCT4_C_JR | OPCODE_C_2)
> +#define MATCH_C_JALR		(FUNCT4_C_JALR | OPCODE_C_2)
> +
> +#define MASK_JALR		0x707f
> +#define MASK_JAL		0x7f
> +#define MASK_C_JALR		0xf07f
> +#define MASK_C_JR		0xf07f
> +#define MASK_C_JAL		0xe003
> +#define MASK_C_J		0xe003
> +#define MASK_BEQ		0x707f
> +#define MASK_BNE		0x707f
> +#define MASK_BLT		0x707f
> +#define MASK_BGE		0x707f
> +#define MASK_BLTU		0x707f
> +#define MASK_BGEU		0x707f
> +#define MASK_C_BEQZ		0xe003
> +#define MASK_C_BNEZ		0xe003
> +#define MASK_SRET		0xffffffff
> +
> +#define __INSN_LENGTH_MASK	_UL(0x3)
> +#define __INSN_LENGTH_GE_32	_UL(0x3)
> +#define __INSN_OPCODE_MASK	_UL(0x7F)
> +#define __INSN_BRANCH_OPCODE	_UL(OPCODE_BRANCH)
> +
> +/* Define a series of is_XXX_insn functions to check if the value INSN
> + * is an instance of instruction XXX.
> + */
> +#define DECLARE_INSN(INSN_NAME, INSN_MATCH, INSN_MASK) \
> +static inline bool is_ ## INSN_NAME ## _insn(long insn) \
> +{ \
> +	return (insn & (INSN_MASK)) == (INSN_MATCH); \
> +}
> +
> +#define RV_IMM_SIGN(x) (-(((x) >> 31) & 1))
> +#define RVC_IMM_SIGN(x) (-(((x) >> 12) & 1))
> +#define RV_X(X, s, mask)  (((X) >> (s)) & (mask))
> +#define RVC_X(X, s, mask) RV_X(X, s, mask)
> +
> +#define EXTRACT_JTYPE_IMM(x) \
> +	({typeof(x) x_ = (x); \
> +	(RV_X(x_, J_IMM_10_1_OPOFF, J_IMM_10_1_MASK) << J_IMM_10_1_OFF) | \
> +	(RV_X(x_, J_IMM_11_OPOFF, J_IMM_11_MASK) << J_IMM_11_OFF) | \
> +	(RV_X(x_, J_IMM_19_12_OPOFF, J_IMM_19_12_MASK) << J_IMM_19_12_OFF) | \
> +	(RV_IMM_SIGN(x_) << J_IMM_SIGN_OFF); })
> +
> +#define EXTRACT_ITYPE_IMM(x) \
> +	({typeof(x) x_ = (x); \
> +	(RV_X(x_, I_IMM_11_0_OPOFF, I_IMM_11_0_MASK)) | \
> +	(RV_IMM_SIGN(x_) << I_IMM_SIGN_OFF); })
> +
> +#define EXTRACT_BTYPE_IMM(x) \
> +	({typeof(x) x_ = (x); \
> +	(RV_X(x_, B_IMM_4_1_OPOFF, B_IMM_4_1_MASK) << B_IMM_4_1_OFF) | \
> +	(RV_X(x_, B_IMM_10_5_OPOFF, B_IMM_10_5_MASK) << B_IMM_10_5_OFF) | \
> +	(RV_X(x_, B_IMM_11_OPOFF, B_IMM_11_MASK) << B_IMM_11_OFF) | \
> +	(RV_IMM_SIGN(x_) << B_IMM_SIGN_OFF); })
> +
> +#define EXTRACT_RVC_J_IMM(x) \
> +	({typeof(x) x_ = (x); \
> +	(RVC_X(x_, RVC_J_IMM_3_1_OPOFF, RVC_J_IMM_3_1_MASK) << RVC_J_IMM_3_1_OFF) | \
> +	(RVC_X(x_, RVC_J_IMM_4_OPOFF, RVC_J_IMM_4_MASK) << RVC_J_IMM_4_OFF) | \
> +	(RVC_X(x_, RVC_J_IMM_5_OPOFF, RVC_J_IMM_5_MASK) << RVC_J_IMM_5_OFF) | \
> +	(RVC_X(x_, RVC_J_IMM_6_OPOFF, RVC_J_IMM_6_MASK) << RVC_J_IMM_6_OFF) | \
> +	(RVC_X(x_, RVC_J_IMM_7_OPOFF, RVC_J_IMM_7_MASK) << RVC_J_IMM_7_OFF) | \
> +	(RVC_X(x_, RVC_J_IMM_9_8_OPOFF, RVC_J_IMM_9_8_MASK) << RVC_J_IMM_9_8_OFF) | \
> +	(RVC_X(x_, RVC_J_IMM_10_OPOFF, RVC_J_IMM_10_MASK) << RVC_J_IMM_10_OFF) | \
> +	(RVC_IMM_SIGN(x_) << RVC_J_IMM_SIGN_OFF); })
> +
> +#define EXTRACT_RVC_B_IMM(x) \
> +	({typeof(x) x_ = (x); \
> +	(RVC_X(x_, RVC_B_IMM_2_1_OPOFF, RVC_B_IMM_2_1_MASK) << RVC_B_IMM_2_1_OFF) | \
> +	(RVC_X(x_, RVC_B_IMM_4_3_OPOFF, RVC_B_IMM_4_3_MASK) << RVC_B_IMM_4_3_OFF) | \
> +	(RVC_X(x_, RVC_B_IMM_5_OPOFF, RVC_B_IMM_5_MASK) << RVC_B_IMM_5_OFF) | \
> +	(RVC_X(x_, RVC_B_IMM_7_6_OPOFF, RVC_B_IMM_7_6_MASK) << RVC_B_IMM_7_6_OFF) | \
> +	(RVC_IMM_SIGN(x_) << RVC_B_IMM_SIGN_OFF); })
> diff --git a/arch/riscv/kernel/kgdb.c b/arch/riscv/kernel/kgdb.c
> index 86d891b7ea2c..2debc88f0a8b 100644
> --- a/arch/riscv/kernel/kgdb.c
> +++ b/arch/riscv/kernel/kgdb.c
> @@ -8,13 +8,168 @@
>  #include <linux/string.h>
>  #include <asm/cacheflush.h>
>  #include <asm/gdb_xml.h>
> +#include <asm/parse_asm.h>
>
>  enum {
>  	NOT_KGDB_BREAK = 0,
>  	KGDB_SW_BREAK,
>  	KGDB_COMPILED_BREAK,
> +	KGDB_SW_SINGLE_STEP
>  };
>
> +static unsigned long stepped_address;
> +static unsigned int stepped_opcode;
> +
> +#if __riscv_xlen == 32
> +/* C.JAL is an RV32C-only instruction */
> +DECLARE_INSN(c_jal, MATCH_C_JAL, MASK_C_JAL)
> +#else
> +#define is_c_jal_insn(opcode) 0
> +#endif
> +DECLARE_INSN(jalr, MATCH_JALR, MASK_JALR)
> +DECLARE_INSN(jal, MATCH_JAL, MASK_JAL)
> +DECLARE_INSN(c_jr, MATCH_C_JR, MASK_C_JR)
> +DECLARE_INSN(c_jalr, MATCH_C_JALR, MASK_C_JALR)
> +DECLARE_INSN(c_j, MATCH_C_J, MASK_C_J)
> +DECLARE_INSN(beq, MATCH_BEQ, MASK_BEQ)
> +DECLARE_INSN(bne, MATCH_BNE, MASK_BNE)
> +DECLARE_INSN(blt, MATCH_BLT, MASK_BLT)
> +DECLARE_INSN(bge, MATCH_BGE, MASK_BGE)
> +DECLARE_INSN(bltu, MATCH_BLTU, MASK_BLTU)
> +DECLARE_INSN(bgeu, MATCH_BGEU, MASK_BGEU)
> +DECLARE_INSN(c_beqz, MATCH_C_BEQZ, MASK_C_BEQZ)
> +DECLARE_INSN(c_bnez, MATCH_C_BNEZ, MASK_C_BNEZ)
> +DECLARE_INSN(sret, MATCH_SRET, MASK_SRET)
> +
> +int decode_register_index(unsigned long opcode, int offset)
> +{
> +	return (opcode >> offset) & 0x1F;
> +}
> +
> +int decode_register_index_short(unsigned long opcode, int offset)
> +{
> +	return ((opcode >> offset) & 0x7) + 8;
> +}
> +
> +/* Calculate the new address for after a step */
> +int get_step_address(struct pt_regs *regs, unsigned long *next_addr)
> +{
> +	unsigned long pc = regs->epc;
> +	unsigned long *regs_ptr = (unsigned long *)regs;
> +	unsigned int rs1_num, rs2_num;
> +	int op_code;
> +
> +	if (probe_kernel_address((void *)pc, op_code))
> +		return -EINVAL;
> +	if ((op_code & __INSN_LENGTH_MASK) != __INSN_LENGTH_GE_32) {
> +		if (is_c_jalr_insn(op_code) || is_c_jr_insn(op_code)) {
> +			rs1_num = decode_register_index(op_code, RVC_C2_RS1_OPOFF);
> +			*next_addr = regs_ptr[rs1_num];
> +		} else if (is_c_j_insn(op_code) || is_c_jal_insn(op_code)) {
> +			*next_addr = EXTRACT_RVC_J_IMM(op_code) + pc;
> +		} else if (is_c_beqz_insn(op_code)) {
> +			rs1_num = decode_register_index_short(op_code,
> +							      RVC_C1_RS1_OPOFF);
> +			if (!rs1_num || regs_ptr[rs1_num] == 0)
> +				*next_addr = EXTRACT_RVC_B_IMM(op_code) + pc;
> +			else
> +				*next_addr = pc + 2;
> +		} else if (is_c_bnez_insn(op_code)) {
> +			rs1_num =
> +			    decode_register_index_short(op_code, RVC_C1_RS1_OPOFF);
> +			if (rs1_num && regs_ptr[rs1_num] != 0)
> +				*next_addr = EXTRACT_RVC_B_IMM(op_code) + pc;
> +			else
> +				*next_addr = pc + 2;
> +		} else {
> +			*next_addr = pc + 2;
> +		}
> +	} else {
> +		if ((op_code & __INSN_OPCODE_MASK) == __INSN_BRANCH_OPCODE) {
> +			bool result = false;
> +			long imm = EXTRACT_BTYPE_IMM(op_code);
> +			unsigned long rs1_val = 0, rs2_val = 0;
> +
> +			rs1_num = decode_register_index(op_code, RVG_RS1_OPOFF);
> +			rs2_num = decode_register_index(op_code, RVG_RS2_OPOFF);
> +			if (rs1_num)
> +				rs1_val = regs_ptr[rs1_num];
> +			if (rs2_num)
> +				rs2_val = regs_ptr[rs2_num];
> +
> +			if (is_beq_insn(op_code))
> +				result = (rs1_val == rs2_val) ? true : false;
> +			else if (is_bne_insn(op_code))
> +				result = (rs1_val != rs2_val) ? true : false;
> +			else if (is_blt_insn(op_code))
> +				result =
> +				    ((long)rs1_val <
> +				     (long)rs2_val) ? true : false;
> +			else if (is_bge_insn(op_code))
> +				result =
> +				    ((long)rs1_val >=
> +				     (long)rs2_val) ? true : false;
> +			else if (is_bltu_insn(op_code))
> +				result = (rs1_val < rs2_val) ? true : false;
> +			else if (is_bgeu_insn(op_code))
> +				result = (rs1_val >= rs2_val) ? true : false;
> +			if (result)
> +				*next_addr = imm + pc;
> +			else
> +				*next_addr = pc + 4;
> +		} else if (is_jal_insn(op_code)) {
> +			*next_addr = EXTRACT_JTYPE_IMM(op_code) + pc;
> +		} else if (is_jalr_insn(op_code)) {
> +			rs1_num = decode_register_index(op_code, RVG_RS1_OPOFF);
> +			if (rs1_num)
> +				*next_addr = ((unsigned long *)regs)[rs1_num];
> +			*next_addr += EXTRACT_ITYPE_IMM(op_code);
> +		} else if (is_sret_insn(op_code)) {
> +			*next_addr = pc;
> +		} else {
> +			*next_addr = pc + 4;
> +		}
> +	}
> +	return 0;
> +}
> +
> +int do_single_step(struct pt_regs *regs)
> +{
> +	/* Determine where the target instruction will send us to */
> +	unsigned long addr = 0;
> +	int error = get_step_address(regs, &addr);
> +
> +	if (error)
> +		return error;
> +
> +	stepped_address = addr;
> +
> +	/* Store the op code in the stepped address */
> +	probe_kernel_address((void *)addr, stepped_opcode);
> +	/* Replace the op code with the break instruction */
> +	error = probe_kernel_write((void *)addr,
> +				   arch_kgdb_ops.gdb_bpt_instr,
> +				   BREAK_INSTR_SIZE);
> +	/* Flush and return */
> +	if (!error)
> +		flush_icache_range(addr, addr + BREAK_INSTR_SIZE);
> +
> +	return error;
> +}
> +
> +/* Undo a single step */
> +static void undo_single_step(struct pt_regs *regs)
> +{
> +	if (stepped_opcode != 0) {
> +		probe_kernel_write((void *)stepped_address,
> +				   (void *)&stepped_opcode, BREAK_INSTR_SIZE);
> +		flush_icache_range(stepped_address,
> +				   stepped_address + BREAK_INSTR_SIZE);
> +	}
> +	stepped_address = 0;
> +	stepped_opcode = 0;
> +}
> +
>  struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
>  	{DBG_REG_ZERO, GDB_SIZEOF_REG, -1},
>  	{DBG_REG_RA, GDB_SIZEOF_REG, offsetof(struct pt_regs, ra)},
> @@ -131,6 +286,8 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
>  {
>  	int err = 0;
>
> +	undo_single_step(regs);
> +
>  	switch (remcom_in_buffer[0]) {
>  	case 'c':
>  	case 'D':
> @@ -140,6 +297,15 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
>  		atomic_set(&kgdb_cpu_doing_single_step, -1);
>  		kgdb_single_step = 0;
>  		break;
> +	case 's':
> +		kgdb_arch_update_addr(regs, remcom_in_buffer);
> +		err = do_single_step(regs);
> +		if (!err) {
> +			kgdb_single_step = 1;
> +			atomic_set(&kgdb_cpu_doing_single_step,
> +				   raw_smp_processor_id());
> +		}
> +		break;
>  	default:
>  		err = -1;
>  	}
> @@ -149,6 +315,8 @@ int kgdb_arch_handle_exception(int vector, int signo, int err_code,
>
>  int kgdb_riscv_kgdbbreak(unsigned long addr)
>  {
> +	if (stepped_address == addr)
> +		return KGDB_SW_SINGLE_STEP;
>  	if (atomic_read(&kgdb_setting_breakpoint))
>  		if (addr == (unsigned long)&kgdb_compiled_break)
>  			return KGDB_COMPILED_BREAK;
> @@ -172,7 +340,8 @@ static int kgdb_riscv_notify(struct notifier_block *self, unsigned long cmd,
>  		return NOTIFY_DONE;
>
>  	local_irq_save(flags);
> -	if (kgdb_handle_exception(1, args->signr, cmd, regs))
> +	if (kgdb_handle_exception(type == KGDB_SW_SINGLE_STEP ? 0 : 1,
> +				  args->signr, cmd, regs))
>  		return NOTIFY_DONE;
>
>  	if (type == KGDB_COMPILED_BREAK)

Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>

Re: [PATCH v2 0/5] riscv: Add KGDB and KDB support

[Daniel Thompson]

On Tue, Mar 31, 2020 at 11:23:06PM +0800, Vincent Chen wrote:
> This patch set implements required ports to enable RISC-V kernel to support
> KGDB and KDB features. Because there is no immediate value in the RISC-V
> trap instruction, the kernel cannot identify the purpose of each trap
> exception through the opcode. This makes the existing identification
> schemes in other architecture unsuitable for the RISC-V kernel. In order
> to solve this problem, this patch adds the kgdb_has_hit_break() to kgdb.c
> to help the RISC-V kernel identify the KGDB trap exception. In addition,
> the XML target description was introduced in this patch set to enable KGDB
> to report the contents of the status, cause and steal registers.
>  
> This patchset has passed the kgdbts test suite provided by Linux kernel on
> HiFive unleashed board and QEMU.

Can you share the defconfig and qemu boot lines used for testing.

I'd like to see if they can easily be integrated into kgdbtest. Normally
figuring out the qemu boot line is the hardest bit of adding support for
an architecture one is not familar with.


Daniel.


PS At the moment it helps kgdbtest a lot if qemu is configured with two
   serial ports but I really should get round to relaxing that!

> 
> Changes since v1:
> 1. Replace the magic number with macro when filling the gdb_regs[].
> 2. Only support GDB XML packet instead of all query packets.
> 3. Move the macros used to parse instrcuton to parse_asm.h
> 
> 
> Vincent Chen (5):
>   kgdb: Add kgdb_has_hit_break function
>   riscv: Add KGDB support
>   kgdb: enable arch to support XML packet support.
>   riscv: Use the XML target descriptions to report 3 system registers
>   riscv: Add SW single-step support for KDB
> 
>  arch/riscv/Kconfig                 |   2 +
>  arch/riscv/include/asm/Kbuild      |   1 -
>  arch/riscv/include/asm/gdb_xml.h   | 117 ++++++++++++
>  arch/riscv/include/asm/kdebug.h    |  12 ++
>  arch/riscv/include/asm/kgdb.h      | 113 +++++++++++
>  arch/riscv/include/asm/parse_asm.h | 214 +++++++++++++++++++++
>  arch/riscv/kernel/Makefile         |   1 +
>  arch/riscv/kernel/kgdb.c           | 382 +++++++++++++++++++++++++++++++++++++
>  arch/riscv/kernel/traps.c          |   5 +
>  include/linux/kgdb.h               |   9 +
>  kernel/debug/debug_core.c          |  12 ++
>  kernel/debug/gdbstub.c             |  13 ++
>  lib/Kconfig.kgdb                   |   5 +
>  13 files changed, 885 insertions(+), 1 deletion(-)
>  create mode 100644 arch/riscv/include/asm/gdb_xml.h
>  create mode 100644 arch/riscv/include/asm/kdebug.h
>  create mode 100644 arch/riscv/include/asm/kgdb.h
>  create mode 100644 arch/riscv/include/asm/parse_asm.h
>  create mode 100644 arch/riscv/kernel/kgdb.c
> 
> -- 
> 2.7.4
> 

Re: [PATCH v2 0/5] riscv: Add KGDB and KDB support

[Vincent Chen]

On Fri, Apr 3, 2020 at 6:12 PM Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Tue, Mar 31, 2020 at 11:23:06PM +0800, Vincent Chen wrote:
> > This patch set implements required ports to enable RISC-V kernel to support
> > KGDB and KDB features. Because there is no immediate value in the RISC-V
> > trap instruction, the kernel cannot identify the purpose of each trap
> > exception through the opcode. This makes the existing identification
> > schemes in other architecture unsuitable for the RISC-V kernel. In order
> > to solve this problem, this patch adds the kgdb_has_hit_break() to kgdb.c
> > to help the RISC-V kernel identify the KGDB trap exception. In addition,
> > the XML target description was introduced in this patch set to enable KGDB
> > to report the contents of the status, cause and steal registers.
> >
> > This patchset has passed the kgdbts test suite provided by Linux kernel on
> > HiFive unleashed board and QEMU.
>
> Can you share the defconfig and qemu boot lines used for testing.
>
> I'd like to see if they can easily be integrated into kgdbtest. Normally
> figuring out the qemu boot line is the hardest bit of adding support for
> an architecture one is not familar with.
>
The process of building a RISC-V kernel is a bit different from other
architecture. Maybe you can refer the steps in
https://risc-v-getting-started-guide.readthedocs.io/en/latest/linux-qemu.html
to build the kernel image and run it.

For the Linux configuration used by KGDB, I just enable KGDB related
configuration based on riscv defconfig. The riscv defconfig can
founded in arch/riscv/configs/defconfig

The QEMU boot lines are listed in the following.

qemu-system-riscv64 -M virt -m 256M -nographic \
-kernel <bbl image>\
-append "debug root=/dev/vda rw console=ttyS0" \
-drive file=<root file system>,format=raw,id=hd0 \
-serial tcp:localhost:2345,server \
-gdb tcp::1133 \
-device virtio-blk-device,drive=hd0 \

>
> Daniel.
>
>
> PS At the moment it helps kgdbtest a lot if qemu is configured with two
>    serial ports but I really should get round to relaxing that!
>
> >
> > Changes since v1:
> > 1. Replace the magic number with macro when filling the gdb_regs[].
> > 2. Only support GDB XML packet instead of all query packets.
> > 3. Move the macros used to parse instrcuton to parse_asm.h
> >
> >
> > Vincent Chen (5):
> >   kgdb: Add kgdb_has_hit_break function
> >   riscv: Add KGDB support
> >   kgdb: enable arch to support XML packet support.
> >   riscv: Use the XML target descriptions to report 3 system registers
> >   riscv: Add SW single-step support for KDB
> >
> >  arch/riscv/Kconfig                 |   2 +
> >  arch/riscv/include/asm/Kbuild      |   1 -
> >  arch/riscv/include/asm/gdb_xml.h   | 117 ++++++++++++
> >  arch/riscv/include/asm/kdebug.h    |  12 ++
> >  arch/riscv/include/asm/kgdb.h      | 113 +++++++++++
> >  arch/riscv/include/asm/parse_asm.h | 214 +++++++++++++++++++++
> >  arch/riscv/kernel/Makefile         |   1 +
> >  arch/riscv/kernel/kgdb.c           | 382 +++++++++++++++++++++++++++++++++++++
> >  arch/riscv/kernel/traps.c          |   5 +
> >  include/linux/kgdb.h               |   9 +
> >  kernel/debug/debug_core.c          |  12 ++
> >  kernel/debug/gdbstub.c             |  13 ++
> >  lib/Kconfig.kgdb                   |   5 +
> >  13 files changed, 885 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/riscv/include/asm/gdb_xml.h
> >  create mode 100644 arch/riscv/include/asm/kdebug.h
> >  create mode 100644 arch/riscv/include/asm/kgdb.h
> >  create mode 100644 arch/riscv/include/asm/parse_asm.h
> >  create mode 100644 arch/riscv/kernel/kgdb.c
> >
> > --
> > 2.7.4
> >

Re: [PATCH v2 0/5] riscv: Add KGDB and KDB support

[Anup Patel]

On Mon, Apr 6, 2020 at 8:05 AM Vincent Chen <vincent.chen@sifive.com> wrote:
>
> On Fri, Apr 3, 2020 at 6:12 PM Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
> >
> > On Tue, Mar 31, 2020 at 11:23:06PM +0800, Vincent Chen wrote:
> > > This patch set implements required ports to enable RISC-V kernel to support
> > > KGDB and KDB features. Because there is no immediate value in the RISC-V
> > > trap instruction, the kernel cannot identify the purpose of each trap
> > > exception through the opcode. This makes the existing identification
> > > schemes in other architecture unsuitable for the RISC-V kernel. In order
> > > to solve this problem, this patch adds the kgdb_has_hit_break() to kgdb.c
> > > to help the RISC-V kernel identify the KGDB trap exception. In addition,
> > > the XML target description was introduced in this patch set to enable KGDB
> > > to report the contents of the status, cause and steal registers.
> > >
> > > This patchset has passed the kgdbts test suite provided by Linux kernel on
> > > HiFive unleashed board and QEMU.
> >
> > Can you share the defconfig and qemu boot lines used for testing.
> >
> > I'd like to see if they can easily be integrated into kgdbtest. Normally
> > figuring out the qemu boot line is the hardest bit of adding support for
> > an architecture one is not familar with.
> >
> The process of building a RISC-V kernel is a bit different from other
> architecture. Maybe you can refer the steps in
> https://risc-v-getting-started-guide.readthedocs.io/en/latest/linux-qemu.html
> to build the kernel image and run it.
>
> For the Linux configuration used by KGDB, I just enable KGDB related
> configuration based on riscv defconfig. The riscv defconfig can
> founded in arch/riscv/configs/defconfig
>
> The QEMU boot lines are listed in the following.
>
> qemu-system-riscv64 -M virt -m 256M -nographic \
> -kernel <bbl image>\
> -append "debug root=/dev/vda rw console=ttyS0" \
> -drive file=<root file system>,format=raw,id=hd0 \
> -serial tcp:localhost:2345,server \
> -gdb tcp::1133 \
> -device virtio-blk-device,drive=hd0 \

Majority of folks (including distros) have moved to OpenSBI instead
of BBL. In fact, QEMU releases ship with OpenSBI as default M-mode
firmware.

To boot Linux on QEMU Virt with OpenSBI as M-mode firmware refer:
https://github.com/riscv/opensbi/blob/master/docs/platform/qemu_virt.md

Regards,
Anup

Re: [PATCH v2 0/5] riscv: Add KGDB and KDB support

[Palmer Dabbelt]

On Tue, 31 Mar 2020 08:23:06 PDT (-0700), vincent.chen@sifive.com wrote:
> This patch set implements required ports to enable RISC-V kernel to support
> KGDB and KDB features. Because there is no immediate value in the RISC-V
> trap instruction, the kernel cannot identify the purpose of each trap
> exception through the opcode. This makes the existing identification
> schemes in other architecture unsuitable for the RISC-V kernel. In order
> to solve this problem, this patch adds the kgdb_has_hit_break() to kgdb.c
> to help the RISC-V kernel identify the KGDB trap exception. In addition,
> the XML target description was introduced in this patch set to enable KGDB
> to report the contents of the status, cause and steal registers.
>
> This patchset has passed the kgdbts test suite provided by Linux kernel on
> HiFive unleashed board and QEMU.
>
> Changes since v1:
> 1. Replace the magic number with macro when filling the gdb_regs[].
> 2. Only support GDB XML packet instead of all query packets.
> 3. Move the macros used to parse instrcuton to parse_asm.h
>
>
> Vincent Chen (5):
>   kgdb: Add kgdb_has_hit_break function
>   riscv: Add KGDB support
>   kgdb: enable arch to support XML packet support.
>   riscv: Use the XML target descriptions to report 3 system registers
>   riscv: Add SW single-step support for KDB
>
>  arch/riscv/Kconfig                 |   2 +
>  arch/riscv/include/asm/Kbuild      |   1 -
>  arch/riscv/include/asm/gdb_xml.h   | 117 ++++++++++++
>  arch/riscv/include/asm/kdebug.h    |  12 ++
>  arch/riscv/include/asm/kgdb.h      | 113 +++++++++++
>  arch/riscv/include/asm/parse_asm.h | 214 +++++++++++++++++++++
>  arch/riscv/kernel/Makefile         |   1 +
>  arch/riscv/kernel/kgdb.c           | 382 +++++++++++++++++++++++++++++++++++++
>  arch/riscv/kernel/traps.c          |   5 +
>  include/linux/kgdb.h               |   9 +
>  kernel/debug/debug_core.c          |  12 ++
>  kernel/debug/gdbstub.c             |  13 ++
>  lib/Kconfig.kgdb                   |   5 +
>  13 files changed, 885 insertions(+), 1 deletion(-)
>  create mode 100644 arch/riscv/include/asm/gdb_xml.h
>  create mode 100644 arch/riscv/include/asm/kdebug.h
>  create mode 100644 arch/riscv/include/asm/kgdb.h
>  create mode 100644 arch/riscv/include/asm/parse_asm.h
>  create mode 100644 arch/riscv/kernel/kgdb.c

Looks like there's some comments on #3, so I'm going to drop this patch set and
assum there will be another version coming.

Thanks!