All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH v3 0/7] target/riscv: Initial support for native debug feature via M-mode CSRs
@ 2022-01-05  3:08 Bin Meng
  2022-01-05  3:08 ` [RESEND PATCH v3 1/7] target/riscv: Add initial support for native debug Bin Meng
                   ` (6 more replies)
  0 siblings, 7 replies; 17+ messages in thread
From: Bin Meng @ 2022-01-05  3:08 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv


This adds initial support for the native debug via the Trigger Module,
as defined in the RISC-V Debug Specification [1].

Only "Address / Data Match" trigger (type 2) is implemented as of now,
which is mainly used for hardware breakpoint and watchpoint. The number
of type 2 triggers implemented is 2, which is the number that we can
find in the SiFive U54/U74 cores.

[1] https://github.com/riscv/riscv-debug-spec/raw/master/riscv-debug-stable.pdf

- RESEND to correct the email address


Changes in v3:
- drop riscv_trigger_init(), which will be moved to patch #5
- add riscv_trigger_init(), moved from patch #1 to this patch
- enable debug feature by default for all CPUs

Changes in v2:
- new patch: add debug state description
- use 0 instead of GETPC()
- change the config option to 'disabled' by default

Bin Meng (7):
  target/riscv: Add initial support for native debug
  target/riscv: machine: Add debug state description
  target/riscv: debug: Implement debug related TCGCPUOps
  target/riscv: cpu: Add a config option for native debug
  target/riscv: csr: Hook debug CSR read/write
  target/riscv: cpu: Enable native debug feature
  hw/core: tcg-cpu-ops.h: Update comments of debug_check_watchpoint()

 include/hw/core/tcg-cpu-ops.h |   1 +
 target/riscv/cpu.h            |   7 +
 target/riscv/debug.h          | 114 +++++++++
 target/riscv/cpu.c            |  14 ++
 target/riscv/csr.c            |  57 +++++
 target/riscv/debug.c          | 441 ++++++++++++++++++++++++++++++++++
 target/riscv/machine.c        |  33 +++
 target/riscv/meson.build      |   1 +
 8 files changed, 668 insertions(+)
 create mode 100644 target/riscv/debug.h
 create mode 100644 target/riscv/debug.c

-- 
2.25.1



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

* [RESEND PATCH v3 1/7] target/riscv: Add initial support for native debug
  2022-01-05  3:08 [RESEND PATCH v3 0/7] target/riscv: Initial support for native debug feature via M-mode CSRs Bin Meng
@ 2022-01-05  3:08 ` Bin Meng
  2022-01-19  3:15     ` Alistair Francis
  2022-01-05  3:08 ` [RESEND PATCH v3 2/7] target/riscv: machine: Add debug state description Bin Meng
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 17+ messages in thread
From: Bin Meng @ 2022-01-05  3:08 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: Bin Meng

From: Bin Meng <bin.meng@windriver.com>

This adds initial support for the native debug via the Trigger Module,
as defined in the RISC-V Debug Specification [1].

Only "Address / Data Match" trigger (type 2) is implemented as of now,
which is mainly used for hardware breakpoint and watchpoint. The number
of type 2 triggers implemented is 2, which is the number that we can
find in the SiFive U54/U74 cores.

[1] https://github.com/riscv/riscv-debug-spec/raw/master/riscv-debug-stable.pdf

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

Changes in v3:
- drop riscv_trigger_init(), which will be moved to patch #5

 target/riscv/cpu.h       |   5 +
 target/riscv/debug.h     | 108 +++++++++++++
 target/riscv/debug.c     | 339 +++++++++++++++++++++++++++++++++++++++
 target/riscv/meson.build |   1 +
 4 files changed, 453 insertions(+)
 create mode 100644 target/riscv/debug.h
 create mode 100644 target/riscv/debug.c

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index dc10f27093..0f3b3a4219 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -98,6 +98,7 @@ typedef struct CPURISCVState CPURISCVState;
 
 #if !defined(CONFIG_USER_ONLY)
 #include "pmp.h"
+#include "debug.h"
 #endif
 
 #define RV_VLEN_MAX 1024
@@ -234,6 +235,10 @@ struct CPURISCVState {
     pmp_table_t pmp_state;
     target_ulong mseccfg;
 
+    /* trigger module */
+    target_ulong trigger_cur;
+    trigger_type2_t trigger_type2[TRIGGER_TYPE2_NUM];
+
     /* machine specific rdtime callback */
     uint64_t (*rdtime_fn)(uint32_t);
     uint32_t rdtime_fn_arg;
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
new file mode 100644
index 0000000000..0a3fda6c72
--- /dev/null
+++ b/target/riscv/debug.h
@@ -0,0 +1,108 @@
+/*
+ * QEMU RISC-V Native Debug Support
+ *
+ * Copyright (c) 2022 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng <bin.meng@windriver.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef RISCV_DEBUG_H
+#define RISCV_DEBUG_H
+
+/* trigger indexes implemented */
+enum {
+    TRIGGER_TYPE2_IDX_0 = 0,
+    TRIGGER_TYPE2_IDX_1,
+    TRIGGER_TYPE2_NUM,
+    TRIGGER_NUM = TRIGGER_TYPE2_NUM
+};
+
+/* register index of tdata CSRs */
+enum {
+    TDATA1 = 0,
+    TDATA2,
+    TDATA3,
+    TDATA_NUM
+};
+
+typedef enum {
+    TRIGGER_TYPE_NO_EXIST = 0,      /* trigger does not exist */
+    TRIGGER_TYPE_AD_MATCH = 2,      /* address/data match trigger */
+    TRIGGER_TYPE_INST_CNT = 3,      /* instruction count trigger */
+    TRIGGER_TYPE_INT = 4,           /* interrupt trigger */
+    TRIGGER_TYPE_EXCP = 5,          /* exception trigger */
+    TRIGGER_TYPE_AD_MATCH6 = 6,     /* new address/data match trigger */
+    TRIGGER_TYPE_EXT_SRC = 7,       /* external source trigger */
+    TRIGGER_TYPE_UNAVAIL = 15       /* trigger exists, but unavailable */
+} trigger_type_t;
+
+typedef struct {
+    target_ulong mcontrol;
+    target_ulong maddress;
+    struct CPUBreakpoint *bp;
+    struct CPUWatchpoint *wp;
+} trigger_type2_t;
+
+/* tdata field masks */
+
+#define RV32_TYPE(t)    ((uint32_t)(t) << 28)
+#define RV32_TYPE_MASK  (0xf << 28)
+#define RV32_DMODE      BIT(27)
+#define RV64_TYPE(t)    ((uint64_t)(t) << 60)
+#define RV64_TYPE_MASK  (0xfULL << 60)
+#define RV64_DMODE      BIT_ULL(59)
+
+/* mcontrol field masks */
+
+#define TYPE2_LOAD      BIT(0)
+#define TYPE2_STORE     BIT(1)
+#define TYPE2_EXEC      BIT(2)
+#define TYPE2_U         BIT(3)
+#define TYPE2_S         BIT(4)
+#define TYPE2_M         BIT(6)
+#define TYPE2_MATCH     (0xf << 7)
+#define TYPE2_CHAIN     BIT(11)
+#define TYPE2_ACTION    (0xf << 12)
+#define TYPE2_SIZELO    (0x3 << 16)
+#define TYPE2_TIMING    BIT(18)
+#define TYPE2_SELECT    BIT(19)
+#define TYPE2_HIT       BIT(20)
+#define TYPE2_SIZEHI    (0x3 << 21) /* RV64 only */
+
+/* access size */
+enum {
+    SIZE_ANY = 0,
+    SIZE_1B,
+    SIZE_2B,
+    SIZE_4B,
+    SIZE_6B,
+    SIZE_8B,
+    SIZE_10B,
+    SIZE_12B,
+    SIZE_14B,
+    SIZE_16B,
+    SIZE_NUM = 16
+};
+
+bool tdata_available(CPURISCVState *env, int tdata_index);
+
+target_ulong tselect_csr_read(CPURISCVState *env);
+void tselect_csr_write(CPURISCVState *env, target_ulong val);
+
+target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index);
+void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val);
+
+#endif /* RISCV_DEBUG_H */
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
new file mode 100644
index 0000000000..530e030007
--- /dev/null
+++ b/target/riscv/debug.c
@@ -0,0 +1,339 @@
+/*
+ * QEMU RISC-V Native Debug Support
+ *
+ * Copyright (c) 2022 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng <bin.meng@windriver.com>
+ *
+ * This provides the native debug support via the Trigger Module, as defined
+ * in the RISC-V Debug Specification:
+ * https://github.com/riscv/riscv-debug-spec/raw/master/riscv-debug-stable.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "cpu.h"
+#include "trace.h"
+#include "exec/exec-all.h"
+
+/*
+ * The following M-mode trigger CSRs are implemented:
+ *
+ * - tselect
+ * - tdata1
+ * - tdata2
+ * - tdata3
+ *
+ * We don't support writable 'type' field in the tdata1 register, so there is
+ * no need to implement the "tinfo" CSR.
+ *
+ * The following triggers are implemented:
+ *
+ * Index | Type |          tdata mapping | Description
+ * ------+------+------------------------+------------
+ *     0 |    2 |         tdata1, tdata2 | Address / Data Match
+ *     1 |    2 |         tdata1, tdata2 | Address / Data Match
+ */
+
+/* tdata availability of a trigger */
+typedef bool tdata_avail[TDATA_NUM];
+
+static tdata_avail tdata_mapping[TRIGGER_NUM] = {
+    [TRIGGER_TYPE2_IDX_0 ... TRIGGER_TYPE2_IDX_1] = { true, true, false },
+};
+
+/* only breakpoint size 1/2/4/8 supported */
+static int access_size[SIZE_NUM] = {
+    [SIZE_ANY] = 0,
+    [SIZE_1B]  = 1,
+    [SIZE_2B]  = 2,
+    [SIZE_4B]  = 4,
+    [SIZE_6B]  = -1,
+    [SIZE_8B]  = 8,
+    [6 ... 15] = -1,
+};
+
+static inline target_ulong trigger_type(CPURISCVState *env,
+                                        trigger_type_t type)
+{
+    target_ulong tdata1;
+
+    switch (riscv_cpu_mxl(env)) {
+    case MXL_RV32:
+        tdata1 = RV32_TYPE(type);
+        break;
+    case MXL_RV64:
+        tdata1 = RV64_TYPE(type);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    return tdata1;
+}
+
+bool tdata_available(CPURISCVState *env, int tdata_index)
+{
+    if (unlikely(tdata_index >= TDATA_NUM)) {
+        return false;
+    }
+
+    if (unlikely(env->trigger_cur >= TRIGGER_NUM)) {
+        return false;
+    }
+
+    return tdata_mapping[env->trigger_cur][tdata_index];
+}
+
+target_ulong tselect_csr_read(CPURISCVState *env)
+{
+    return env->trigger_cur;
+}
+
+void tselect_csr_write(CPURISCVState *env, target_ulong val)
+{
+    /* all target_ulong bits of tselect are implemented */
+    env->trigger_cur = val;
+}
+
+static target_ulong tdata1_validate(CPURISCVState *env, target_ulong val,
+                                    trigger_type_t t)
+{
+    uint32_t type, dmode;
+    target_ulong tdata1;
+
+    switch (riscv_cpu_mxl(env)) {
+    case MXL_RV32:
+        type = extract32(val, 28, 4);
+        dmode = extract32(val, 27, 1);
+        tdata1 = RV32_TYPE(t);
+        break;
+    case MXL_RV64:
+        type = extract64(val, 60, 4);
+        dmode = extract64(val, 59, 1);
+        tdata1 = RV64_TYPE(t);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    if (type != t) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "ignoring type write to tdata1 register\n");
+    }
+    if (dmode != 0) {
+        qemu_log_mask(LOG_UNIMP, "debug mode is not supported\n");
+    }
+
+    return tdata1;
+}
+
+static inline void warn_always_zero_bit(target_ulong val, target_ulong mask,
+                                        const char *msg)
+{
+    if (val & mask) {
+        qemu_log_mask(LOG_UNIMP, "%s bit is always zero\n", msg);
+    }
+}
+
+static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
+{
+    uint32_t size, sizelo, sizehi = 0;
+
+    if (riscv_cpu_mxl(env) == MXL_RV64) {
+        sizehi = extract32(ctrl, 21, 2);
+    }
+    sizelo = extract32(ctrl, 16, 2);
+    size = (sizehi << 2) | sizelo;
+
+    return size;
+}
+
+static inline bool type2_breakpoint_enabled(target_ulong ctrl)
+{
+    bool mode = !!(ctrl & (TYPE2_U | TYPE2_S | TYPE2_M));
+    bool rwx = !!(ctrl & (TYPE2_LOAD | TYPE2_STORE | TYPE2_EXEC));
+
+    return mode && rwx;
+}
+
+static target_ulong type2_mcontrol_validate(CPURISCVState *env,
+                                            target_ulong ctrl)
+{
+    target_ulong val;
+    uint32_t size;
+
+    /* validate the generic part first */
+    val = tdata1_validate(env, ctrl, TRIGGER_TYPE_AD_MATCH);
+
+    /* validate unimplemented (always zero) bits */
+    warn_always_zero_bit(ctrl, TYPE2_MATCH, "match");
+    warn_always_zero_bit(ctrl, TYPE2_CHAIN, "chain");
+    warn_always_zero_bit(ctrl, TYPE2_ACTION, "action");
+    warn_always_zero_bit(ctrl, TYPE2_TIMING, "timing");
+    warn_always_zero_bit(ctrl, TYPE2_SELECT, "select");
+    warn_always_zero_bit(ctrl, TYPE2_HIT, "hit");
+
+    /* validate size encoding */
+    size = type2_breakpoint_size(env, ctrl);
+    if (access_size[size] == -1) {
+        qemu_log_mask(LOG_UNIMP, "access size %d is not supported, using SIZE_ANY\n",
+                      size);
+    } else {
+        val |= (ctrl & TYPE2_SIZELO);
+        if (riscv_cpu_mxl(env) == MXL_RV64) {
+            val |= (ctrl & TYPE2_SIZEHI);
+        }
+    }
+
+    /* keep the mode and attribute bits */
+    val |= (ctrl & (TYPE2_U | TYPE2_S | TYPE2_M |
+                    TYPE2_LOAD | TYPE2_STORE | TYPE2_EXEC));
+
+    return val;
+}
+
+static void type2_breakpoint_insert(CPURISCVState *env, target_ulong index)
+{
+    target_ulong ctrl = env->trigger_type2[index].mcontrol;
+    target_ulong addr = env->trigger_type2[index].maddress;
+    bool enabled = type2_breakpoint_enabled(ctrl);
+    CPUState *cs = env_cpu(env);
+    int flags = BP_CPU | BP_STOP_BEFORE_ACCESS;
+    uint32_t size;
+
+    if (!enabled) {
+        return;
+    }
+
+    if (ctrl & TYPE2_EXEC) {
+        cpu_breakpoint_insert(cs, addr, flags, &env->trigger_type2[index].bp);
+    }
+
+    if (ctrl & TYPE2_LOAD) {
+        flags |= BP_MEM_READ;
+    }
+    if (ctrl & TYPE2_STORE) {
+        flags |= BP_MEM_WRITE;
+    }
+
+    if (flags & BP_MEM_ACCESS) {
+        size = type2_breakpoint_size(env, ctrl);
+        if (size != 0) {
+            cpu_watchpoint_insert(cs, addr, size, flags,
+                                  &env->trigger_type2[index].wp);
+        } else {
+            cpu_watchpoint_insert(cs, addr, 8, flags,
+                                  &env->trigger_type2[index].wp);
+        }
+    }
+}
+
+static void type2_breakpoint_remove(CPURISCVState *env, target_ulong index)
+{
+    CPUState *cs = env_cpu(env);
+
+    if (env->trigger_type2[index].bp) {
+        cpu_breakpoint_remove_by_ref(cs, env->trigger_type2[index].bp);
+        env->trigger_type2[index].bp = NULL;
+    }
+
+    if (env->trigger_type2[index].wp) {
+        cpu_watchpoint_remove_by_ref(cs, env->trigger_type2[index].wp);
+        env->trigger_type2[index].wp = NULL;
+    }
+}
+
+static target_ulong type2_reg_read(CPURISCVState *env,
+                                   target_ulong trigger_index, int tdata_index)
+{
+    uint32_t index = trigger_index - TRIGGER_TYPE2_IDX_0;
+    target_ulong tdata;
+
+    switch (tdata_index) {
+    case TDATA1:
+        tdata = env->trigger_type2[index].mcontrol;
+        break;
+    case TDATA2:
+        tdata = env->trigger_type2[index].maddress;
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    return tdata;
+}
+
+static void type2_reg_write(CPURISCVState *env, target_ulong trigger_index,
+                            int tdata_index, target_ulong val)
+{
+    uint32_t index = trigger_index - TRIGGER_TYPE2_IDX_0;
+    target_ulong new_val;
+
+    switch (tdata_index) {
+    case TDATA1:
+        new_val = type2_mcontrol_validate(env, val);
+        if (new_val != env->trigger_type2[index].mcontrol) {
+            env->trigger_type2[index].mcontrol = new_val;
+            type2_breakpoint_remove(env, index);
+            type2_breakpoint_insert(env, index);
+        }
+        break;
+    case TDATA2:
+        if (val != env->trigger_type2[index].maddress) {
+            env->trigger_type2[index].maddress = val;
+            type2_breakpoint_remove(env, index);
+            type2_breakpoint_insert(env, index);
+        }
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    return;
+}
+
+typedef target_ulong (*tdata_read_func)(CPURISCVState *env,
+                                        target_ulong trigger_index,
+                                        int tdata_index);
+
+static tdata_read_func trigger_read_funcs[TRIGGER_NUM] = {
+    [TRIGGER_TYPE2_IDX_0 ... TRIGGER_TYPE2_IDX_1] = type2_reg_read,
+};
+
+typedef void (*tdata_write_func)(CPURISCVState *env,
+                                 target_ulong trigger_index,
+                                 int tdata_index,
+                                 target_ulong val);
+
+static tdata_write_func trigger_write_funcs[TRIGGER_NUM] = {
+    [TRIGGER_TYPE2_IDX_0 ... TRIGGER_TYPE2_IDX_1] = type2_reg_write,
+};
+
+target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
+{
+    tdata_read_func read_func = trigger_read_funcs[env->trigger_cur];
+
+    return read_func(env, env->trigger_cur, tdata_index);
+}
+
+void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
+{
+    tdata_write_func write_func = trigger_write_funcs[env->trigger_cur];
+
+    return write_func(env, env->trigger_cur, tdata_index, val);
+}
diff --git a/target/riscv/meson.build b/target/riscv/meson.build
index d5e0bc93ea..966d97237a 100644
--- a/target/riscv/meson.build
+++ b/target/riscv/meson.build
@@ -24,6 +24,7 @@ riscv_softmmu_ss = ss.source_set()
 riscv_softmmu_ss.add(files(
   'arch_dump.c',
   'pmp.c',
+  'debug.c',
   'monitor.c',
   'machine.c'
 ))
-- 
2.25.1



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

* [RESEND PATCH v3 2/7] target/riscv: machine: Add debug state description
  2022-01-05  3:08 [RESEND PATCH v3 0/7] target/riscv: Initial support for native debug feature via M-mode CSRs Bin Meng
  2022-01-05  3:08 ` [RESEND PATCH v3 1/7] target/riscv: Add initial support for native debug Bin Meng
@ 2022-01-05  3:08 ` Bin Meng
  2022-01-05  3:08 ` [RESEND PATCH v3 3/7] target/riscv: debug: Implement debug related TCGCPUOps Bin Meng
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Bin Meng @ 2022-01-05  3:08 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: Bin Meng, Alistair Francis

From: Bin Meng <bin.meng@windriver.com>

Add a subsection to machine.c to migrate debug CSR state.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---

(no changes since v2)

Changes in v2:
- new patch: add debug state description

 target/riscv/machine.c | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index ad8248ebfd..25aa3b38f7 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -164,6 +164,38 @@ static const VMStateDescription vmstate_pointermasking = {
     }
 };
 
+static bool debug_needed(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+    CPURISCVState *env = &cpu->env;
+
+    return riscv_feature(env, RISCV_FEATURE_DEBUG);
+}
+
+static const VMStateDescription vmstate_debug_type2 = {
+    .name = "cpu/debug/type2",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINTTL(mcontrol, trigger_type2_t),
+        VMSTATE_UINTTL(maddress, trigger_type2_t),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static const VMStateDescription vmstate_debug = {
+    .name = "cpu/debug",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = debug_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
+        VMSTATE_STRUCT_ARRAY(env.trigger_type2, RISCVCPU, TRIGGER_TYPE2_NUM,
+                             0, vmstate_debug_type2, trigger_type2_t),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_riscv_cpu = {
     .name = "cpu",
     .version_id = 3,
@@ -218,6 +250,7 @@ const VMStateDescription vmstate_riscv_cpu = {
         &vmstate_hyper,
         &vmstate_vector,
         &vmstate_pointermasking,
+        &vmstate_debug,
         NULL
     }
 };
-- 
2.25.1



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

* [RESEND PATCH v3 3/7] target/riscv: debug: Implement debug related TCGCPUOps
  2022-01-05  3:08 [RESEND PATCH v3 0/7] target/riscv: Initial support for native debug feature via M-mode CSRs Bin Meng
  2022-01-05  3:08 ` [RESEND PATCH v3 1/7] target/riscv: Add initial support for native debug Bin Meng
  2022-01-05  3:08 ` [RESEND PATCH v3 2/7] target/riscv: machine: Add debug state description Bin Meng
@ 2022-01-05  3:08 ` Bin Meng
  2022-01-05  3:08 ` [RESEND PATCH v3 4/7] target/riscv: cpu: Add a config option for native debug Bin Meng
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Bin Meng @ 2022-01-05  3:08 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: Bin Meng, Alistair Francis

From: Bin Meng <bin.meng@windriver.com>

Implement .debug_excp_handler, .debug_check_{breakpoint, watchpoint}
TCGCPUOps and hook them into riscv_tcg_ops.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---

(no changes since v2)

Changes in v2:
- use 0 instead of GETPC()

 target/riscv/debug.h |  4 +++
 target/riscv/cpu.c   |  3 ++
 target/riscv/debug.c | 75 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+)

diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index 0a3fda6c72..d0f63e2414 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -105,4 +105,8 @@ void tselect_csr_write(CPURISCVState *env, target_ulong val);
 target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index);
 void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val);
 
+void riscv_cpu_debug_excp_handler(CPUState *cs);
+bool riscv_cpu_debug_check_breakpoint(CPUState *cs);
+bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
+
 #endif /* RISCV_DEBUG_H */
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 6ef3314bce..3aa07bc019 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -705,6 +705,9 @@ static const struct TCGCPUOps riscv_tcg_ops = {
     .do_interrupt = riscv_cpu_do_interrupt,
     .do_transaction_failed = riscv_cpu_do_transaction_failed,
     .do_unaligned_access = riscv_cpu_do_unaligned_access,
+    .debug_excp_handler = riscv_cpu_debug_excp_handler,
+    .debug_check_breakpoint = riscv_cpu_debug_check_breakpoint,
+    .debug_check_watchpoint = riscv_cpu_debug_check_watchpoint,
 #endif /* !CONFIG_USER_ONLY */
 };
 
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 530e030007..7760c4611f 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -337,3 +337,78 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
 
     return write_func(env, env->trigger_cur, tdata_index, val);
 }
+
+void riscv_cpu_debug_excp_handler(CPUState *cs)
+{
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+
+    if (cs->watchpoint_hit) {
+        if (cs->watchpoint_hit->flags & BP_CPU) {
+            cs->watchpoint_hit = NULL;
+            riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
+        }
+    } else {
+        if (cpu_breakpoint_test(cs, env->pc, BP_CPU)) {
+            riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
+        }
+    }
+}
+
+bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
+{
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+    CPUBreakpoint *bp;
+    target_ulong ctrl;
+    target_ulong pc;
+    int i;
+
+    QTAILQ_FOREACH(bp, &cs->breakpoints, entry) {
+        for (i = 0; i < TRIGGER_TYPE2_NUM; i++) {
+            ctrl = env->trigger_type2[i].mcontrol;
+            pc = env->trigger_type2[i].maddress;
+
+            if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
+                /* check U/S/M bit against current privilege level */
+                if ((ctrl >> 3) & BIT(env->priv)) {
+                    return true;
+                }
+            }
+        }
+    }
+
+    return false;
+}
+
+bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
+{
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    CPURISCVState *env = &cpu->env;
+    target_ulong ctrl;
+    target_ulong addr;
+    int flags;
+    int i;
+
+    for (i = 0; i < TRIGGER_TYPE2_NUM; i++) {
+        ctrl = env->trigger_type2[i].mcontrol;
+        addr = env->trigger_type2[i].maddress;
+        flags = 0;
+
+        if (ctrl & TYPE2_LOAD) {
+            flags |= BP_MEM_READ;
+        }
+        if (ctrl & TYPE2_STORE) {
+            flags |= BP_MEM_WRITE;
+        }
+
+        if ((wp->flags & flags) && (wp->vaddr == addr)) {
+            /* check U/S/M bit against current privilege level */
+            if ((ctrl >> 3) & BIT(env->priv)) {
+                return true;
+            }
+        }
+    }
+
+    return false;
+}
-- 
2.25.1



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

* [RESEND PATCH v3 4/7] target/riscv: cpu: Add a config option for native debug
  2022-01-05  3:08 [RESEND PATCH v3 0/7] target/riscv: Initial support for native debug feature via M-mode CSRs Bin Meng
                   ` (2 preceding siblings ...)
  2022-01-05  3:08 ` [RESEND PATCH v3 3/7] target/riscv: debug: Implement debug related TCGCPUOps Bin Meng
@ 2022-01-05  3:08 ` Bin Meng
  2022-01-05  3:08 ` [RESEND PATCH v3 5/7] target/riscv: csr: Hook debug CSR read/write Bin Meng
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Bin Meng @ 2022-01-05  3:08 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: Bin Meng, Alistair Francis

From: Bin Meng <bin.meng@windriver.com>

Add a config option to enable support for native M-mode debug.
This is disabled by default and can be enabled with 'debug=true'.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---

(no changes since v2)

Changes in v2:
- change the config option to 'disabled' by default

 target/riscv/cpu.h | 2 ++
 target/riscv/cpu.c | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0f3b3a4219..35445bbc86 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -75,6 +75,7 @@ enum {
     RISCV_FEATURE_MMU,
     RISCV_FEATURE_PMP,
     RISCV_FEATURE_EPMP,
+    RISCV_FEATURE_DEBUG,
     RISCV_FEATURE_MISA
 };
 
@@ -332,6 +333,7 @@ struct RISCVCPU {
         bool mmu;
         bool pmp;
         bool epmp;
+        bool debug;
         uint64_t resetvec;
     } cfg;
 };
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 3aa07bc019..d36c31ce9a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -448,6 +448,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
         }
     }
 
+    if (cpu->cfg.debug) {
+        set_feature(env, RISCV_FEATURE_DEBUG);
+    }
+
     set_resetvec(env, cpu->cfg.resetvec);
 
     /* Validate that MISA_MXL is set properly. */
@@ -634,6 +638,7 @@ static Property riscv_cpu_properties[] = {
     DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
     DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
     DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
+    DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, false),
 
     DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
     DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
-- 
2.25.1



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

* [RESEND PATCH v3 5/7] target/riscv: csr: Hook debug CSR read/write
  2022-01-05  3:08 [RESEND PATCH v3 0/7] target/riscv: Initial support for native debug feature via M-mode CSRs Bin Meng
                   ` (3 preceding siblings ...)
  2022-01-05  3:08 ` [RESEND PATCH v3 4/7] target/riscv: cpu: Add a config option for native debug Bin Meng
@ 2022-01-05  3:08 ` Bin Meng
  2022-01-19  3:06     ` Alistair Francis
  2022-01-05  3:08 ` [RESEND PATCH v3 6/7] target/riscv: cpu: Enable native debug feature Bin Meng
  2022-01-05  3:08   ` Bin Meng
  6 siblings, 1 reply; 17+ messages in thread
From: Bin Meng @ 2022-01-05  3:08 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: Bin Meng

From: Bin Meng <bin.meng@windriver.com>

This adds debug CSR read/write support to the RISC-V CSR RW table.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

Changes in v3:
- add riscv_trigger_init(), moved from patch #1 to this patch

 target/riscv/debug.h |  2 ++
 target/riscv/cpu.c   |  6 +++++
 target/riscv/csr.c   | 57 ++++++++++++++++++++++++++++++++++++++++++++
 target/riscv/debug.c | 27 +++++++++++++++++++++
 4 files changed, 92 insertions(+)

diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index d0f63e2414..f4da2db35d 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -109,4 +109,6 @@ void riscv_cpu_debug_excp_handler(CPUState *cs);
 bool riscv_cpu_debug_check_breakpoint(CPUState *cs);
 bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
 
+void riscv_trigger_init(CPURISCVState *env);
+
 #endif /* RISCV_DEBUG_H */
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d36c31ce9a..17dcc3c14f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -575,6 +575,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
 
     riscv_cpu_register_gdb_regs_for_features(cs);
 
+#ifndef CONFIG_USER_ONLY
+    if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
+        riscv_trigger_init(env);
+    }
+#endif
+
     qemu_init_vcpu(cs);
     cpu_reset(cs);
 
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 146447eac5..189b9cc8c6 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -220,6 +220,15 @@ static RISCVException epmp(CPURISCVState *env, int csrno)
 
     return RISCV_EXCP_ILLEGAL_INST;
 }
+
+static RISCVException debug(CPURISCVState *env, int csrno)
+{
+    if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
+        return RISCV_EXCP_NONE;
+    }
+
+    return RISCV_EXCP_ILLEGAL_INST;
+}
 #endif
 
 /* User Floating-Point CSRs */
@@ -1464,6 +1473,48 @@ static RISCVException write_pmpaddr(CPURISCVState *env, int csrno,
     return RISCV_EXCP_NONE;
 }
 
+static RISCVException read_tselect(CPURISCVState *env, int csrno,
+                                   target_ulong *val)
+{
+    *val = tselect_csr_read(env);
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_tselect(CPURISCVState *env, int csrno,
+                                    target_ulong val)
+{
+    tselect_csr_write(env, val);
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_tdata(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    /* return 0 in tdata1 to end the trigger enumeration */
+    if (env->trigger_cur >= TRIGGER_NUM && csrno == CSR_TDATA1) {
+        *val = 0;
+        return RISCV_EXCP_NONE;
+    }
+
+    if (!tdata_available(env, csrno - CSR_TDATA1)) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    *val = tdata_csr_read(env, csrno - CSR_TDATA1);
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_tdata(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    if (!tdata_available(env, csrno - CSR_TDATA1)) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    tdata_csr_write(env, csrno - CSR_TDATA1, val);
+    return RISCV_EXCP_NONE;
+}
+
 /*
  * Functions to access Pointer Masking feature registers
  * We have to check if current priv lvl could modify
@@ -1962,6 +2013,12 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_PMPADDR14] =  { "pmpaddr14", pmp, read_pmpaddr, write_pmpaddr },
     [CSR_PMPADDR15] =  { "pmpaddr15", pmp, read_pmpaddr, write_pmpaddr },
 
+    /* Debug CSRs */
+    [CSR_TSELECT]   =  { "tselect", debug, read_tselect, write_tselect },
+    [CSR_TDATA1]    =  { "tdata1",  debug, read_tdata,   write_tdata   },
+    [CSR_TDATA2]    =  { "tdata2",  debug, read_tdata,   write_tdata   },
+    [CSR_TDATA3]    =  { "tdata3",  debug, read_tdata,   write_tdata   },
+
     /* User Pointer Masking */
     [CSR_UMTE]    =    { "umte",    pointer_masking, read_umte,    write_umte    },
     [CSR_UPMMASK] =    { "upmmask", pointer_masking, read_upmmask, write_upmmask },
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 7760c4611f..041a0d3a89 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -412,3 +412,30 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
 
     return false;
 }
+
+void riscv_trigger_init(CPURISCVState *env)
+{
+    target_ulong type2 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
+    int i;
+
+    /* type 2 triggers */
+    for (i = 0; i < TRIGGER_TYPE2_NUM; i++) {
+        /*
+         * type = TRIGGER_TYPE_AD_MATCH
+         * dmode = 0 (both debug and M-mode can write tdata)
+         * maskmax = 0 (unimplemented, always 0)
+         * sizehi = 0 (match against any size, RV64 only)
+         * hit = 0 (unimplemented, always 0)
+         * select = 0 (always 0, perform match on address)
+         * timing = 0 (always 0, trigger before instruction)
+         * sizelo = 0 (match against any size)
+         * action = 0 (always 0, raise a breakpoint exception)
+         * chain = 0 (unimplemented, always 0)
+         * match = 0 (always 0, when any compare value equals tdata2)
+         */
+        env->trigger_type2[i].mcontrol = type2;
+        env->trigger_type2[i].maddress = 0;
+        env->trigger_type2[i].bp = NULL;
+        env->trigger_type2[i].wp = NULL;
+    }
+}
-- 
2.25.1



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

* [RESEND PATCH v3 6/7] target/riscv: cpu: Enable native debug feature
  2022-01-05  3:08 [RESEND PATCH v3 0/7] target/riscv: Initial support for native debug feature via M-mode CSRs Bin Meng
                   ` (4 preceding siblings ...)
  2022-01-05  3:08 ` [RESEND PATCH v3 5/7] target/riscv: csr: Hook debug CSR read/write Bin Meng
@ 2022-01-05  3:08 ` Bin Meng
  2022-01-05  3:08   ` Bin Meng
  6 siblings, 0 replies; 17+ messages in thread
From: Bin Meng @ 2022-01-05  3:08 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: Bin Meng

From: Bin Meng <bin.meng@windriver.com>

Turn on native debug feature by default for all CPUs.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
---

Changes in v3:
- enable debug feature by default for all CPUs

 target/riscv/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 17dcc3c14f..17444b458f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -644,7 +644,7 @@ static Property riscv_cpu_properties[] = {
     DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
     DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
     DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
-    DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, false),
+    DEFINE_PROP_BOOL("debug", RISCVCPU, cfg.debug, true),
 
     DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
     DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
-- 
2.25.1



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

* [RESEND PATCH v3 7/7] hw/core: tcg-cpu-ops.h: Update comments of debug_check_watchpoint()
  2022-01-05  3:08 [RESEND PATCH v3 0/7] target/riscv: Initial support for native debug feature via M-mode CSRs Bin Meng
@ 2022-01-05  3:08   ` Bin Meng
  2022-01-05  3:08 ` [RESEND PATCH v3 2/7] target/riscv: machine: Add debug state description Bin Meng
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 17+ messages in thread
From: Bin Meng @ 2022-01-05  3:08 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv
  Cc: Alistair Francis, Bin Meng, Richard Henderson

From: Bin Meng <bin.meng@windriver.com>

This is now used by RISC-V as well. Update the comments.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---

(no changes since v1)

 include/hw/core/tcg-cpu-ops.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
index e13898553a..f98671ff32 100644
--- a/include/hw/core/tcg-cpu-ops.h
+++ b/include/hw/core/tcg-cpu-ops.h
@@ -90,6 +90,7 @@ struct TCGCPUOps {
     /**
      * @debug_check_watchpoint: return true if the architectural
      * watchpoint whose address has matched should really fire, used by ARM
+     * and RISC-V
      */
     bool (*debug_check_watchpoint)(CPUState *cpu, CPUWatchpoint *wp);
 
-- 
2.25.1



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

* [RESEND PATCH v3 7/7] hw/core: tcg-cpu-ops.h: Update comments of debug_check_watchpoint()
@ 2022-01-05  3:08   ` Bin Meng
  0 siblings, 0 replies; 17+ messages in thread
From: Bin Meng @ 2022-01-05  3:08 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv
  Cc: Bin Meng, Richard Henderson, Alistair Francis

From: Bin Meng <bin.meng@windriver.com>

This is now used by RISC-V as well. Update the comments.

Signed-off-by: Bin Meng <bin.meng@windriver.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---

(no changes since v1)

 include/hw/core/tcg-cpu-ops.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/hw/core/tcg-cpu-ops.h b/include/hw/core/tcg-cpu-ops.h
index e13898553a..f98671ff32 100644
--- a/include/hw/core/tcg-cpu-ops.h
+++ b/include/hw/core/tcg-cpu-ops.h
@@ -90,6 +90,7 @@ struct TCGCPUOps {
     /**
      * @debug_check_watchpoint: return true if the architectural
      * watchpoint whose address has matched should really fire, used by ARM
+     * and RISC-V
      */
     bool (*debug_check_watchpoint)(CPUState *cpu, CPUWatchpoint *wp);
 
-- 
2.25.1



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

* Re: [RESEND PATCH v3 5/7] target/riscv: csr: Hook debug CSR read/write
  2022-01-05  3:08 ` [RESEND PATCH v3 5/7] target/riscv: csr: Hook debug CSR read/write Bin Meng
@ 2022-01-19  3:06     ` Alistair Francis
  0 siblings, 0 replies; 17+ messages in thread
From: Alistair Francis @ 2022-01-19  3:06 UTC (permalink / raw)
  To: Bin Meng
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Wed, Jan 5, 2022 at 1:15 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This adds debug CSR read/write support to the RISC-V CSR RW table.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
> Changes in v3:
> - add riscv_trigger_init(), moved from patch #1 to this patch
>
>  target/riscv/debug.h |  2 ++
>  target/riscv/cpu.c   |  6 +++++
>  target/riscv/csr.c   | 57 ++++++++++++++++++++++++++++++++++++++++++++
>  target/riscv/debug.c | 27 +++++++++++++++++++++
>  4 files changed, 92 insertions(+)
>
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index d0f63e2414..f4da2db35d 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -109,4 +109,6 @@ void riscv_cpu_debug_excp_handler(CPUState *cs);
>  bool riscv_cpu_debug_check_breakpoint(CPUState *cs);
>  bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
>
> +void riscv_trigger_init(CPURISCVState *env);
> +
>  #endif /* RISCV_DEBUG_H */
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index d36c31ce9a..17dcc3c14f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -575,6 +575,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>
>      riscv_cpu_register_gdb_regs_for_features(cs);
>
> +#ifndef CONFIG_USER_ONLY
> +    if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> +        riscv_trigger_init(env);
> +    }
> +#endif
> +
>      qemu_init_vcpu(cs);
>      cpu_reset(cs);
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 146447eac5..189b9cc8c6 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -220,6 +220,15 @@ static RISCVException epmp(CPURISCVState *env, int csrno)
>
>      return RISCV_EXCP_ILLEGAL_INST;
>  }
> +
> +static RISCVException debug(CPURISCVState *env, int csrno)
> +{
> +    if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> +        return RISCV_EXCP_NONE;
> +    }
> +
> +    return RISCV_EXCP_ILLEGAL_INST;
> +}
>  #endif
>
>  /* User Floating-Point CSRs */
> @@ -1464,6 +1473,48 @@ static RISCVException write_pmpaddr(CPURISCVState *env, int csrno,
>      return RISCV_EXCP_NONE;
>  }
>
> +static RISCVException read_tselect(CPURISCVState *env, int csrno,
> +                                   target_ulong *val)
> +{
> +    *val = tselect_csr_read(env);
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_tselect(CPURISCVState *env, int csrno,
> +                                    target_ulong val)
> +{
> +    tselect_csr_write(env, val);
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_tdata(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    /* return 0 in tdata1 to end the trigger enumeration */
> +    if (env->trigger_cur >= TRIGGER_NUM && csrno == CSR_TDATA1) {
> +        *val = 0;
> +        return RISCV_EXCP_NONE;
> +    }
> +
> +    if (!tdata_available(env, csrno - CSR_TDATA1)) {
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
> +
> +    *val = tdata_csr_read(env, csrno - CSR_TDATA1);
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_tdata(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    if (!tdata_available(env, csrno - CSR_TDATA1)) {
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
> +
> +    tdata_csr_write(env, csrno - CSR_TDATA1, val);
> +    return RISCV_EXCP_NONE;
> +}
> +
>  /*
>   * Functions to access Pointer Masking feature registers
>   * We have to check if current priv lvl could modify
> @@ -1962,6 +2013,12 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      [CSR_PMPADDR14] =  { "pmpaddr14", pmp, read_pmpaddr, write_pmpaddr },
>      [CSR_PMPADDR15] =  { "pmpaddr15", pmp, read_pmpaddr, write_pmpaddr },
>
> +    /* Debug CSRs */
> +    [CSR_TSELECT]   =  { "tselect", debug, read_tselect, write_tselect },
> +    [CSR_TDATA1]    =  { "tdata1",  debug, read_tdata,   write_tdata   },
> +    [CSR_TDATA2]    =  { "tdata2",  debug, read_tdata,   write_tdata   },
> +    [CSR_TDATA3]    =  { "tdata3",  debug, read_tdata,   write_tdata   },
> +
>      /* User Pointer Masking */
>      [CSR_UMTE]    =    { "umte",    pointer_masking, read_umte,    write_umte    },
>      [CSR_UPMMASK] =    { "upmmask", pointer_masking, read_upmmask, write_upmmask },
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 7760c4611f..041a0d3a89 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -412,3 +412,30 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>
>      return false;
>  }
> +
> +void riscv_trigger_init(CPURISCVState *env)
> +{
> +    target_ulong type2 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
> +    int i;
> +
> +    /* type 2 triggers */
> +    for (i = 0; i < TRIGGER_TYPE2_NUM; i++) {
> +        /*
> +         * type = TRIGGER_TYPE_AD_MATCH
> +         * dmode = 0 (both debug and M-mode can write tdata)
> +         * maskmax = 0 (unimplemented, always 0)
> +         * sizehi = 0 (match against any size, RV64 only)
> +         * hit = 0 (unimplemented, always 0)
> +         * select = 0 (always 0, perform match on address)
> +         * timing = 0 (always 0, trigger before instruction)
> +         * sizelo = 0 (match against any size)
> +         * action = 0 (always 0, raise a breakpoint exception)
> +         * chain = 0 (unimplemented, always 0)
> +         * match = 0 (always 0, when any compare value equals tdata2)
> +         */
> +        env->trigger_type2[i].mcontrol = type2;
> +        env->trigger_type2[i].maddress = 0;
> +        env->trigger_type2[i].bp = NULL;
> +        env->trigger_type2[i].wp = NULL;
> +    }

Should this be called at reset instead?

Alistair

> +}
> --
> 2.25.1
>
>


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

* Re: [RESEND PATCH v3 5/7] target/riscv: csr: Hook debug CSR read/write
@ 2022-01-19  3:06     ` Alistair Francis
  0 siblings, 0 replies; 17+ messages in thread
From: Alistair Francis @ 2022-01-19  3:06 UTC (permalink / raw)
  To: Bin Meng
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Bin Meng

On Wed, Jan 5, 2022 at 1:15 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This adds debug CSR read/write support to the RISC-V CSR RW table.
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
> Changes in v3:
> - add riscv_trigger_init(), moved from patch #1 to this patch
>
>  target/riscv/debug.h |  2 ++
>  target/riscv/cpu.c   |  6 +++++
>  target/riscv/csr.c   | 57 ++++++++++++++++++++++++++++++++++++++++++++
>  target/riscv/debug.c | 27 +++++++++++++++++++++
>  4 files changed, 92 insertions(+)
>
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index d0f63e2414..f4da2db35d 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -109,4 +109,6 @@ void riscv_cpu_debug_excp_handler(CPUState *cs);
>  bool riscv_cpu_debug_check_breakpoint(CPUState *cs);
>  bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
>
> +void riscv_trigger_init(CPURISCVState *env);
> +
>  #endif /* RISCV_DEBUG_H */
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index d36c31ce9a..17dcc3c14f 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -575,6 +575,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>
>      riscv_cpu_register_gdb_regs_for_features(cs);
>
> +#ifndef CONFIG_USER_ONLY
> +    if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> +        riscv_trigger_init(env);
> +    }
> +#endif
> +
>      qemu_init_vcpu(cs);
>      cpu_reset(cs);
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 146447eac5..189b9cc8c6 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -220,6 +220,15 @@ static RISCVException epmp(CPURISCVState *env, int csrno)
>
>      return RISCV_EXCP_ILLEGAL_INST;
>  }
> +
> +static RISCVException debug(CPURISCVState *env, int csrno)
> +{
> +    if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> +        return RISCV_EXCP_NONE;
> +    }
> +
> +    return RISCV_EXCP_ILLEGAL_INST;
> +}
>  #endif
>
>  /* User Floating-Point CSRs */
> @@ -1464,6 +1473,48 @@ static RISCVException write_pmpaddr(CPURISCVState *env, int csrno,
>      return RISCV_EXCP_NONE;
>  }
>
> +static RISCVException read_tselect(CPURISCVState *env, int csrno,
> +                                   target_ulong *val)
> +{
> +    *val = tselect_csr_read(env);
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_tselect(CPURISCVState *env, int csrno,
> +                                    target_ulong val)
> +{
> +    tselect_csr_write(env, val);
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_tdata(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    /* return 0 in tdata1 to end the trigger enumeration */
> +    if (env->trigger_cur >= TRIGGER_NUM && csrno == CSR_TDATA1) {
> +        *val = 0;
> +        return RISCV_EXCP_NONE;
> +    }
> +
> +    if (!tdata_available(env, csrno - CSR_TDATA1)) {
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
> +
> +    *val = tdata_csr_read(env, csrno - CSR_TDATA1);
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_tdata(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    if (!tdata_available(env, csrno - CSR_TDATA1)) {
> +        return RISCV_EXCP_ILLEGAL_INST;
> +    }
> +
> +    tdata_csr_write(env, csrno - CSR_TDATA1, val);
> +    return RISCV_EXCP_NONE;
> +}
> +
>  /*
>   * Functions to access Pointer Masking feature registers
>   * We have to check if current priv lvl could modify
> @@ -1962,6 +2013,12 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      [CSR_PMPADDR14] =  { "pmpaddr14", pmp, read_pmpaddr, write_pmpaddr },
>      [CSR_PMPADDR15] =  { "pmpaddr15", pmp, read_pmpaddr, write_pmpaddr },
>
> +    /* Debug CSRs */
> +    [CSR_TSELECT]   =  { "tselect", debug, read_tselect, write_tselect },
> +    [CSR_TDATA1]    =  { "tdata1",  debug, read_tdata,   write_tdata   },
> +    [CSR_TDATA2]    =  { "tdata2",  debug, read_tdata,   write_tdata   },
> +    [CSR_TDATA3]    =  { "tdata3",  debug, read_tdata,   write_tdata   },
> +
>      /* User Pointer Masking */
>      [CSR_UMTE]    =    { "umte",    pointer_masking, read_umte,    write_umte    },
>      [CSR_UPMMASK] =    { "upmmask", pointer_masking, read_upmmask, write_upmmask },
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 7760c4611f..041a0d3a89 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -412,3 +412,30 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>
>      return false;
>  }
> +
> +void riscv_trigger_init(CPURISCVState *env)
> +{
> +    target_ulong type2 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
> +    int i;
> +
> +    /* type 2 triggers */
> +    for (i = 0; i < TRIGGER_TYPE2_NUM; i++) {
> +        /*
> +         * type = TRIGGER_TYPE_AD_MATCH
> +         * dmode = 0 (both debug and M-mode can write tdata)
> +         * maskmax = 0 (unimplemented, always 0)
> +         * sizehi = 0 (match against any size, RV64 only)
> +         * hit = 0 (unimplemented, always 0)
> +         * select = 0 (always 0, perform match on address)
> +         * timing = 0 (always 0, trigger before instruction)
> +         * sizelo = 0 (match against any size)
> +         * action = 0 (always 0, raise a breakpoint exception)
> +         * chain = 0 (unimplemented, always 0)
> +         * match = 0 (always 0, when any compare value equals tdata2)
> +         */
> +        env->trigger_type2[i].mcontrol = type2;
> +        env->trigger_type2[i].maddress = 0;
> +        env->trigger_type2[i].bp = NULL;
> +        env->trigger_type2[i].wp = NULL;
> +    }

Should this be called at reset instead?

Alistair

> +}
> --
> 2.25.1
>
>


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

* Re: [RESEND PATCH v3 1/7] target/riscv: Add initial support for native debug
  2022-01-05  3:08 ` [RESEND PATCH v3 1/7] target/riscv: Add initial support for native debug Bin Meng
@ 2022-01-19  3:15     ` Alistair Francis
  0 siblings, 0 replies; 17+ messages in thread
From: Alistair Francis @ 2022-01-19  3:15 UTC (permalink / raw)
  To: Bin Meng
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Wed, Jan 5, 2022 at 1:09 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This adds initial support for the native debug via the Trigger Module,
> as defined in the RISC-V Debug Specification [1].

Doesn't this mean we are just supporting the Sdtrig extension?

>
> Only "Address / Data Match" trigger (type 2) is implemented as of now,
> which is mainly used for hardware breakpoint and watchpoint. The number
> of type 2 triggers implemented is 2, which is the number that we can
> find in the SiFive U54/U74 cores.
>
> [1] https://github.com/riscv/riscv-debug-spec/raw/master/riscv-debug-stable.pdf
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
> Changes in v3:
> - drop riscv_trigger_init(), which will be moved to patch #5
>
>  target/riscv/cpu.h       |   5 +
>  target/riscv/debug.h     | 108 +++++++++++++
>  target/riscv/debug.c     | 339 +++++++++++++++++++++++++++++++++++++++
>  target/riscv/meson.build |   1 +
>  4 files changed, 453 insertions(+)
>  create mode 100644 target/riscv/debug.h
>  create mode 100644 target/riscv/debug.c
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index dc10f27093..0f3b3a4219 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -98,6 +98,7 @@ typedef struct CPURISCVState CPURISCVState;
>
>  #if !defined(CONFIG_USER_ONLY)
>  #include "pmp.h"
> +#include "debug.h"
>  #endif
>
>  #define RV_VLEN_MAX 1024
> @@ -234,6 +235,10 @@ struct CPURISCVState {
>      pmp_table_t pmp_state;
>      target_ulong mseccfg;
>
> +    /* trigger module */
> +    target_ulong trigger_cur;
> +    trigger_type2_t trigger_type2[TRIGGER_TYPE2_NUM];
> +
>      /* machine specific rdtime callback */
>      uint64_t (*rdtime_fn)(uint32_t);
>      uint32_t rdtime_fn_arg;
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> new file mode 100644
> index 0000000000..0a3fda6c72
> --- /dev/null
> +++ b/target/riscv/debug.h
> @@ -0,0 +1,108 @@
> +/*
> + * QEMU RISC-V Native Debug Support
> + *
> + * Copyright (c) 2022 Wind River Systems, Inc.
> + *
> + * Author:
> + *   Bin Meng <bin.meng@windriver.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef RISCV_DEBUG_H
> +#define RISCV_DEBUG_H
> +
> +/* trigger indexes implemented */
> +enum {
> +    TRIGGER_TYPE2_IDX_0 = 0,
> +    TRIGGER_TYPE2_IDX_1,
> +    TRIGGER_TYPE2_NUM,
> +    TRIGGER_NUM = TRIGGER_TYPE2_NUM
> +};
> +
> +/* register index of tdata CSRs */
> +enum {
> +    TDATA1 = 0,
> +    TDATA2,
> +    TDATA3,
> +    TDATA_NUM
> +};
> +
> +typedef enum {
> +    TRIGGER_TYPE_NO_EXIST = 0,      /* trigger does not exist */
> +    TRIGGER_TYPE_AD_MATCH = 2,      /* address/data match trigger */
> +    TRIGGER_TYPE_INST_CNT = 3,      /* instruction count trigger */
> +    TRIGGER_TYPE_INT = 4,           /* interrupt trigger */
> +    TRIGGER_TYPE_EXCP = 5,          /* exception trigger */
> +    TRIGGER_TYPE_AD_MATCH6 = 6,     /* new address/data match trigger */
> +    TRIGGER_TYPE_EXT_SRC = 7,       /* external source trigger */
> +    TRIGGER_TYPE_UNAVAIL = 15       /* trigger exists, but unavailable */
> +} trigger_type_t;
> +
> +typedef struct {
> +    target_ulong mcontrol;
> +    target_ulong maddress;
> +    struct CPUBreakpoint *bp;
> +    struct CPUWatchpoint *wp;
> +} trigger_type2_t;

This is a confusing name

Alistair

> +
> +/* tdata field masks */
> +
> +#define RV32_TYPE(t)    ((uint32_t)(t) << 28)
> +#define RV32_TYPE_MASK  (0xf << 28)
> +#define RV32_DMODE      BIT(27)
> +#define RV64_TYPE(t)    ((uint64_t)(t) << 60)
> +#define RV64_TYPE_MASK  (0xfULL << 60)
> +#define RV64_DMODE      BIT_ULL(59)
> +
> +/* mcontrol field masks */
> +
> +#define TYPE2_LOAD      BIT(0)
> +#define TYPE2_STORE     BIT(1)
> +#define TYPE2_EXEC      BIT(2)
> +#define TYPE2_U         BIT(3)
> +#define TYPE2_S         BIT(4)
> +#define TYPE2_M         BIT(6)
> +#define TYPE2_MATCH     (0xf << 7)
> +#define TYPE2_CHAIN     BIT(11)
> +#define TYPE2_ACTION    (0xf << 12)
> +#define TYPE2_SIZELO    (0x3 << 16)
> +#define TYPE2_TIMING    BIT(18)
> +#define TYPE2_SELECT    BIT(19)
> +#define TYPE2_HIT       BIT(20)
> +#define TYPE2_SIZEHI    (0x3 << 21) /* RV64 only */
> +
> +/* access size */
> +enum {
> +    SIZE_ANY = 0,
> +    SIZE_1B,
> +    SIZE_2B,
> +    SIZE_4B,
> +    SIZE_6B,
> +    SIZE_8B,
> +    SIZE_10B,
> +    SIZE_12B,
> +    SIZE_14B,
> +    SIZE_16B,
> +    SIZE_NUM = 16
> +};
> +
> +bool tdata_available(CPURISCVState *env, int tdata_index);
> +
> +target_ulong tselect_csr_read(CPURISCVState *env);
> +void tselect_csr_write(CPURISCVState *env, target_ulong val);
> +
> +target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index);
> +void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val);
> +
> +#endif /* RISCV_DEBUG_H */
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> new file mode 100644
> index 0000000000..530e030007
> --- /dev/null
> +++ b/target/riscv/debug.c
> @@ -0,0 +1,339 @@
> +/*
> + * QEMU RISC-V Native Debug Support
> + *
> + * Copyright (c) 2022 Wind River Systems, Inc.
> + *
> + * Author:
> + *   Bin Meng <bin.meng@windriver.com>
> + *
> + * This provides the native debug support via the Trigger Module, as defined
> + * in the RISC-V Debug Specification:
> + * https://github.com/riscv/riscv-debug-spec/raw/master/riscv-debug-stable.pdf
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qapi/error.h"
> +#include "cpu.h"
> +#include "trace.h"
> +#include "exec/exec-all.h"
> +
> +/*
> + * The following M-mode trigger CSRs are implemented:
> + *
> + * - tselect
> + * - tdata1
> + * - tdata2
> + * - tdata3
> + *
> + * We don't support writable 'type' field in the tdata1 register, so there is
> + * no need to implement the "tinfo" CSR.
> + *
> + * The following triggers are implemented:
> + *
> + * Index | Type |          tdata mapping | Description
> + * ------+------+------------------------+------------
> + *     0 |    2 |         tdata1, tdata2 | Address / Data Match
> + *     1 |    2 |         tdata1, tdata2 | Address / Data Match
> + */
> +
> +/* tdata availability of a trigger */
> +typedef bool tdata_avail[TDATA_NUM];
> +
> +static tdata_avail tdata_mapping[TRIGGER_NUM] = {
> +    [TRIGGER_TYPE2_IDX_0 ... TRIGGER_TYPE2_IDX_1] = { true, true, false },
> +};
> +
> +/* only breakpoint size 1/2/4/8 supported */
> +static int access_size[SIZE_NUM] = {
> +    [SIZE_ANY] = 0,
> +    [SIZE_1B]  = 1,
> +    [SIZE_2B]  = 2,
> +    [SIZE_4B]  = 4,
> +    [SIZE_6B]  = -1,
> +    [SIZE_8B]  = 8,
> +    [6 ... 15] = -1,
> +};
> +
> +static inline target_ulong trigger_type(CPURISCVState *env,
> +                                        trigger_type_t type)
> +{
> +    target_ulong tdata1;
> +
> +    switch (riscv_cpu_mxl(env)) {
> +    case MXL_RV32:
> +        tdata1 = RV32_TYPE(type);
> +        break;
> +    case MXL_RV64:
> +        tdata1 = RV64_TYPE(type);
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    return tdata1;
> +}
> +
> +bool tdata_available(CPURISCVState *env, int tdata_index)
> +{
> +    if (unlikely(tdata_index >= TDATA_NUM)) {
> +        return false;
> +    }
> +
> +    if (unlikely(env->trigger_cur >= TRIGGER_NUM)) {
> +        return false;
> +    }
> +
> +    return tdata_mapping[env->trigger_cur][tdata_index];
> +}
> +
> +target_ulong tselect_csr_read(CPURISCVState *env)
> +{
> +    return env->trigger_cur;
> +}
> +
> +void tselect_csr_write(CPURISCVState *env, target_ulong val)
> +{
> +    /* all target_ulong bits of tselect are implemented */
> +    env->trigger_cur = val;
> +}
> +
> +static target_ulong tdata1_validate(CPURISCVState *env, target_ulong val,
> +                                    trigger_type_t t)
> +{
> +    uint32_t type, dmode;
> +    target_ulong tdata1;
> +
> +    switch (riscv_cpu_mxl(env)) {
> +    case MXL_RV32:
> +        type = extract32(val, 28, 4);
> +        dmode = extract32(val, 27, 1);
> +        tdata1 = RV32_TYPE(t);
> +        break;
> +    case MXL_RV64:
> +        type = extract64(val, 60, 4);
> +        dmode = extract64(val, 59, 1);
> +        tdata1 = RV64_TYPE(t);
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    if (type != t) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "ignoring type write to tdata1 register\n");
> +    }
> +    if (dmode != 0) {
> +        qemu_log_mask(LOG_UNIMP, "debug mode is not supported\n");
> +    }
> +
> +    return tdata1;
> +}
> +
> +static inline void warn_always_zero_bit(target_ulong val, target_ulong mask,
> +                                        const char *msg)
> +{
> +    if (val & mask) {
> +        qemu_log_mask(LOG_UNIMP, "%s bit is always zero\n", msg);
> +    }
> +}
> +
> +static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
> +{
> +    uint32_t size, sizelo, sizehi = 0;
> +
> +    if (riscv_cpu_mxl(env) == MXL_RV64) {
> +        sizehi = extract32(ctrl, 21, 2);
> +    }
> +    sizelo = extract32(ctrl, 16, 2);
> +    size = (sizehi << 2) | sizelo;
> +
> +    return size;
> +}
> +
> +static inline bool type2_breakpoint_enabled(target_ulong ctrl)
> +{
> +    bool mode = !!(ctrl & (TYPE2_U | TYPE2_S | TYPE2_M));
> +    bool rwx = !!(ctrl & (TYPE2_LOAD | TYPE2_STORE | TYPE2_EXEC));
> +
> +    return mode && rwx;
> +}
> +
> +static target_ulong type2_mcontrol_validate(CPURISCVState *env,
> +                                            target_ulong ctrl)
> +{
> +    target_ulong val;
> +    uint32_t size;
> +
> +    /* validate the generic part first */
> +    val = tdata1_validate(env, ctrl, TRIGGER_TYPE_AD_MATCH);
> +
> +    /* validate unimplemented (always zero) bits */
> +    warn_always_zero_bit(ctrl, TYPE2_MATCH, "match");
> +    warn_always_zero_bit(ctrl, TYPE2_CHAIN, "chain");
> +    warn_always_zero_bit(ctrl, TYPE2_ACTION, "action");
> +    warn_always_zero_bit(ctrl, TYPE2_TIMING, "timing");
> +    warn_always_zero_bit(ctrl, TYPE2_SELECT, "select");
> +    warn_always_zero_bit(ctrl, TYPE2_HIT, "hit");
> +
> +    /* validate size encoding */
> +    size = type2_breakpoint_size(env, ctrl);
> +    if (access_size[size] == -1) {
> +        qemu_log_mask(LOG_UNIMP, "access size %d is not supported, using SIZE_ANY\n",
> +                      size);
> +    } else {
> +        val |= (ctrl & TYPE2_SIZELO);
> +        if (riscv_cpu_mxl(env) == MXL_RV64) {
> +            val |= (ctrl & TYPE2_SIZEHI);
> +        }
> +    }
> +
> +    /* keep the mode and attribute bits */
> +    val |= (ctrl & (TYPE2_U | TYPE2_S | TYPE2_M |
> +                    TYPE2_LOAD | TYPE2_STORE | TYPE2_EXEC));
> +
> +    return val;
> +}
> +
> +static void type2_breakpoint_insert(CPURISCVState *env, target_ulong index)
> +{
> +    target_ulong ctrl = env->trigger_type2[index].mcontrol;
> +    target_ulong addr = env->trigger_type2[index].maddress;
> +    bool enabled = type2_breakpoint_enabled(ctrl);
> +    CPUState *cs = env_cpu(env);
> +    int flags = BP_CPU | BP_STOP_BEFORE_ACCESS;
> +    uint32_t size;
> +
> +    if (!enabled) {
> +        return;
> +    }
> +
> +    if (ctrl & TYPE2_EXEC) {
> +        cpu_breakpoint_insert(cs, addr, flags, &env->trigger_type2[index].bp);
> +    }
> +
> +    if (ctrl & TYPE2_LOAD) {
> +        flags |= BP_MEM_READ;
> +    }
> +    if (ctrl & TYPE2_STORE) {
> +        flags |= BP_MEM_WRITE;
> +    }
> +
> +    if (flags & BP_MEM_ACCESS) {
> +        size = type2_breakpoint_size(env, ctrl);
> +        if (size != 0) {
> +            cpu_watchpoint_insert(cs, addr, size, flags,
> +                                  &env->trigger_type2[index].wp);
> +        } else {
> +            cpu_watchpoint_insert(cs, addr, 8, flags,
> +                                  &env->trigger_type2[index].wp);
> +        }
> +    }
> +}
> +
> +static void type2_breakpoint_remove(CPURISCVState *env, target_ulong index)
> +{
> +    CPUState *cs = env_cpu(env);
> +
> +    if (env->trigger_type2[index].bp) {
> +        cpu_breakpoint_remove_by_ref(cs, env->trigger_type2[index].bp);
> +        env->trigger_type2[index].bp = NULL;
> +    }
> +
> +    if (env->trigger_type2[index].wp) {
> +        cpu_watchpoint_remove_by_ref(cs, env->trigger_type2[index].wp);
> +        env->trigger_type2[index].wp = NULL;
> +    }
> +}
> +
> +static target_ulong type2_reg_read(CPURISCVState *env,
> +                                   target_ulong trigger_index, int tdata_index)
> +{
> +    uint32_t index = trigger_index - TRIGGER_TYPE2_IDX_0;
> +    target_ulong tdata;
> +
> +    switch (tdata_index) {
> +    case TDATA1:
> +        tdata = env->trigger_type2[index].mcontrol;
> +        break;
> +    case TDATA2:
> +        tdata = env->trigger_type2[index].maddress;
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    return tdata;
> +}
> +
> +static void type2_reg_write(CPURISCVState *env, target_ulong trigger_index,
> +                            int tdata_index, target_ulong val)
> +{
> +    uint32_t index = trigger_index - TRIGGER_TYPE2_IDX_0;
> +    target_ulong new_val;
> +
> +    switch (tdata_index) {
> +    case TDATA1:
> +        new_val = type2_mcontrol_validate(env, val);
> +        if (new_val != env->trigger_type2[index].mcontrol) {
> +            env->trigger_type2[index].mcontrol = new_val;
> +            type2_breakpoint_remove(env, index);
> +            type2_breakpoint_insert(env, index);
> +        }
> +        break;
> +    case TDATA2:
> +        if (val != env->trigger_type2[index].maddress) {
> +            env->trigger_type2[index].maddress = val;
> +            type2_breakpoint_remove(env, index);
> +            type2_breakpoint_insert(env, index);
> +        }
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    return;
> +}
> +
> +typedef target_ulong (*tdata_read_func)(CPURISCVState *env,
> +                                        target_ulong trigger_index,
> +                                        int tdata_index);
> +
> +static tdata_read_func trigger_read_funcs[TRIGGER_NUM] = {
> +    [TRIGGER_TYPE2_IDX_0 ... TRIGGER_TYPE2_IDX_1] = type2_reg_read,
> +};
> +
> +typedef void (*tdata_write_func)(CPURISCVState *env,
> +                                 target_ulong trigger_index,
> +                                 int tdata_index,
> +                                 target_ulong val);
> +
> +static tdata_write_func trigger_write_funcs[TRIGGER_NUM] = {
> +    [TRIGGER_TYPE2_IDX_0 ... TRIGGER_TYPE2_IDX_1] = type2_reg_write,
> +};
> +
> +target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
> +{
> +    tdata_read_func read_func = trigger_read_funcs[env->trigger_cur];
> +
> +    return read_func(env, env->trigger_cur, tdata_index);
> +}
> +
> +void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
> +{
> +    tdata_write_func write_func = trigger_write_funcs[env->trigger_cur];
> +
> +    return write_func(env, env->trigger_cur, tdata_index, val);
> +}
> diff --git a/target/riscv/meson.build b/target/riscv/meson.build
> index d5e0bc93ea..966d97237a 100644
> --- a/target/riscv/meson.build
> +++ b/target/riscv/meson.build
> @@ -24,6 +24,7 @@ riscv_softmmu_ss = ss.source_set()
>  riscv_softmmu_ss.add(files(
>    'arch_dump.c',
>    'pmp.c',
> +  'debug.c',
>    'monitor.c',
>    'machine.c'
>  ))
> --
> 2.25.1
>
>


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

* Re: [RESEND PATCH v3 1/7] target/riscv: Add initial support for native debug
@ 2022-01-19  3:15     ` Alistair Francis
  0 siblings, 0 replies; 17+ messages in thread
From: Alistair Francis @ 2022-01-19  3:15 UTC (permalink / raw)
  To: Bin Meng
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Bin Meng

On Wed, Jan 5, 2022 at 1:09 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This adds initial support for the native debug via the Trigger Module,
> as defined in the RISC-V Debug Specification [1].

Doesn't this mean we are just supporting the Sdtrig extension?

>
> Only "Address / Data Match" trigger (type 2) is implemented as of now,
> which is mainly used for hardware breakpoint and watchpoint. The number
> of type 2 triggers implemented is 2, which is the number that we can
> find in the SiFive U54/U74 cores.
>
> [1] https://github.com/riscv/riscv-debug-spec/raw/master/riscv-debug-stable.pdf
>
> Signed-off-by: Bin Meng <bin.meng@windriver.com>
> ---
>
> Changes in v3:
> - drop riscv_trigger_init(), which will be moved to patch #5
>
>  target/riscv/cpu.h       |   5 +
>  target/riscv/debug.h     | 108 +++++++++++++
>  target/riscv/debug.c     | 339 +++++++++++++++++++++++++++++++++++++++
>  target/riscv/meson.build |   1 +
>  4 files changed, 453 insertions(+)
>  create mode 100644 target/riscv/debug.h
>  create mode 100644 target/riscv/debug.c
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index dc10f27093..0f3b3a4219 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -98,6 +98,7 @@ typedef struct CPURISCVState CPURISCVState;
>
>  #if !defined(CONFIG_USER_ONLY)
>  #include "pmp.h"
> +#include "debug.h"
>  #endif
>
>  #define RV_VLEN_MAX 1024
> @@ -234,6 +235,10 @@ struct CPURISCVState {
>      pmp_table_t pmp_state;
>      target_ulong mseccfg;
>
> +    /* trigger module */
> +    target_ulong trigger_cur;
> +    trigger_type2_t trigger_type2[TRIGGER_TYPE2_NUM];
> +
>      /* machine specific rdtime callback */
>      uint64_t (*rdtime_fn)(uint32_t);
>      uint32_t rdtime_fn_arg;
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> new file mode 100644
> index 0000000000..0a3fda6c72
> --- /dev/null
> +++ b/target/riscv/debug.h
> @@ -0,0 +1,108 @@
> +/*
> + * QEMU RISC-V Native Debug Support
> + *
> + * Copyright (c) 2022 Wind River Systems, Inc.
> + *
> + * Author:
> + *   Bin Meng <bin.meng@windriver.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef RISCV_DEBUG_H
> +#define RISCV_DEBUG_H
> +
> +/* trigger indexes implemented */
> +enum {
> +    TRIGGER_TYPE2_IDX_0 = 0,
> +    TRIGGER_TYPE2_IDX_1,
> +    TRIGGER_TYPE2_NUM,
> +    TRIGGER_NUM = TRIGGER_TYPE2_NUM
> +};
> +
> +/* register index of tdata CSRs */
> +enum {
> +    TDATA1 = 0,
> +    TDATA2,
> +    TDATA3,
> +    TDATA_NUM
> +};
> +
> +typedef enum {
> +    TRIGGER_TYPE_NO_EXIST = 0,      /* trigger does not exist */
> +    TRIGGER_TYPE_AD_MATCH = 2,      /* address/data match trigger */
> +    TRIGGER_TYPE_INST_CNT = 3,      /* instruction count trigger */
> +    TRIGGER_TYPE_INT = 4,           /* interrupt trigger */
> +    TRIGGER_TYPE_EXCP = 5,          /* exception trigger */
> +    TRIGGER_TYPE_AD_MATCH6 = 6,     /* new address/data match trigger */
> +    TRIGGER_TYPE_EXT_SRC = 7,       /* external source trigger */
> +    TRIGGER_TYPE_UNAVAIL = 15       /* trigger exists, but unavailable */
> +} trigger_type_t;
> +
> +typedef struct {
> +    target_ulong mcontrol;
> +    target_ulong maddress;
> +    struct CPUBreakpoint *bp;
> +    struct CPUWatchpoint *wp;
> +} trigger_type2_t;

This is a confusing name

Alistair

> +
> +/* tdata field masks */
> +
> +#define RV32_TYPE(t)    ((uint32_t)(t) << 28)
> +#define RV32_TYPE_MASK  (0xf << 28)
> +#define RV32_DMODE      BIT(27)
> +#define RV64_TYPE(t)    ((uint64_t)(t) << 60)
> +#define RV64_TYPE_MASK  (0xfULL << 60)
> +#define RV64_DMODE      BIT_ULL(59)
> +
> +/* mcontrol field masks */
> +
> +#define TYPE2_LOAD      BIT(0)
> +#define TYPE2_STORE     BIT(1)
> +#define TYPE2_EXEC      BIT(2)
> +#define TYPE2_U         BIT(3)
> +#define TYPE2_S         BIT(4)
> +#define TYPE2_M         BIT(6)
> +#define TYPE2_MATCH     (0xf << 7)
> +#define TYPE2_CHAIN     BIT(11)
> +#define TYPE2_ACTION    (0xf << 12)
> +#define TYPE2_SIZELO    (0x3 << 16)
> +#define TYPE2_TIMING    BIT(18)
> +#define TYPE2_SELECT    BIT(19)
> +#define TYPE2_HIT       BIT(20)
> +#define TYPE2_SIZEHI    (0x3 << 21) /* RV64 only */
> +
> +/* access size */
> +enum {
> +    SIZE_ANY = 0,
> +    SIZE_1B,
> +    SIZE_2B,
> +    SIZE_4B,
> +    SIZE_6B,
> +    SIZE_8B,
> +    SIZE_10B,
> +    SIZE_12B,
> +    SIZE_14B,
> +    SIZE_16B,
> +    SIZE_NUM = 16
> +};
> +
> +bool tdata_available(CPURISCVState *env, int tdata_index);
> +
> +target_ulong tselect_csr_read(CPURISCVState *env);
> +void tselect_csr_write(CPURISCVState *env, target_ulong val);
> +
> +target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index);
> +void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val);
> +
> +#endif /* RISCV_DEBUG_H */
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> new file mode 100644
> index 0000000000..530e030007
> --- /dev/null
> +++ b/target/riscv/debug.c
> @@ -0,0 +1,339 @@
> +/*
> + * QEMU RISC-V Native Debug Support
> + *
> + * Copyright (c) 2022 Wind River Systems, Inc.
> + *
> + * Author:
> + *   Bin Meng <bin.meng@windriver.com>
> + *
> + * This provides the native debug support via the Trigger Module, as defined
> + * in the RISC-V Debug Specification:
> + * https://github.com/riscv/riscv-debug-spec/raw/master/riscv-debug-stable.pdf
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qapi/error.h"
> +#include "cpu.h"
> +#include "trace.h"
> +#include "exec/exec-all.h"
> +
> +/*
> + * The following M-mode trigger CSRs are implemented:
> + *
> + * - tselect
> + * - tdata1
> + * - tdata2
> + * - tdata3
> + *
> + * We don't support writable 'type' field in the tdata1 register, so there is
> + * no need to implement the "tinfo" CSR.
> + *
> + * The following triggers are implemented:
> + *
> + * Index | Type |          tdata mapping | Description
> + * ------+------+------------------------+------------
> + *     0 |    2 |         tdata1, tdata2 | Address / Data Match
> + *     1 |    2 |         tdata1, tdata2 | Address / Data Match
> + */
> +
> +/* tdata availability of a trigger */
> +typedef bool tdata_avail[TDATA_NUM];
> +
> +static tdata_avail tdata_mapping[TRIGGER_NUM] = {
> +    [TRIGGER_TYPE2_IDX_0 ... TRIGGER_TYPE2_IDX_1] = { true, true, false },
> +};
> +
> +/* only breakpoint size 1/2/4/8 supported */
> +static int access_size[SIZE_NUM] = {
> +    [SIZE_ANY] = 0,
> +    [SIZE_1B]  = 1,
> +    [SIZE_2B]  = 2,
> +    [SIZE_4B]  = 4,
> +    [SIZE_6B]  = -1,
> +    [SIZE_8B]  = 8,
> +    [6 ... 15] = -1,
> +};
> +
> +static inline target_ulong trigger_type(CPURISCVState *env,
> +                                        trigger_type_t type)
> +{
> +    target_ulong tdata1;
> +
> +    switch (riscv_cpu_mxl(env)) {
> +    case MXL_RV32:
> +        tdata1 = RV32_TYPE(type);
> +        break;
> +    case MXL_RV64:
> +        tdata1 = RV64_TYPE(type);
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    return tdata1;
> +}
> +
> +bool tdata_available(CPURISCVState *env, int tdata_index)
> +{
> +    if (unlikely(tdata_index >= TDATA_NUM)) {
> +        return false;
> +    }
> +
> +    if (unlikely(env->trigger_cur >= TRIGGER_NUM)) {
> +        return false;
> +    }
> +
> +    return tdata_mapping[env->trigger_cur][tdata_index];
> +}
> +
> +target_ulong tselect_csr_read(CPURISCVState *env)
> +{
> +    return env->trigger_cur;
> +}
> +
> +void tselect_csr_write(CPURISCVState *env, target_ulong val)
> +{
> +    /* all target_ulong bits of tselect are implemented */
> +    env->trigger_cur = val;
> +}
> +
> +static target_ulong tdata1_validate(CPURISCVState *env, target_ulong val,
> +                                    trigger_type_t t)
> +{
> +    uint32_t type, dmode;
> +    target_ulong tdata1;
> +
> +    switch (riscv_cpu_mxl(env)) {
> +    case MXL_RV32:
> +        type = extract32(val, 28, 4);
> +        dmode = extract32(val, 27, 1);
> +        tdata1 = RV32_TYPE(t);
> +        break;
> +    case MXL_RV64:
> +        type = extract64(val, 60, 4);
> +        dmode = extract64(val, 59, 1);
> +        tdata1 = RV64_TYPE(t);
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    if (type != t) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "ignoring type write to tdata1 register\n");
> +    }
> +    if (dmode != 0) {
> +        qemu_log_mask(LOG_UNIMP, "debug mode is not supported\n");
> +    }
> +
> +    return tdata1;
> +}
> +
> +static inline void warn_always_zero_bit(target_ulong val, target_ulong mask,
> +                                        const char *msg)
> +{
> +    if (val & mask) {
> +        qemu_log_mask(LOG_UNIMP, "%s bit is always zero\n", msg);
> +    }
> +}
> +
> +static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
> +{
> +    uint32_t size, sizelo, sizehi = 0;
> +
> +    if (riscv_cpu_mxl(env) == MXL_RV64) {
> +        sizehi = extract32(ctrl, 21, 2);
> +    }
> +    sizelo = extract32(ctrl, 16, 2);
> +    size = (sizehi << 2) | sizelo;
> +
> +    return size;
> +}
> +
> +static inline bool type2_breakpoint_enabled(target_ulong ctrl)
> +{
> +    bool mode = !!(ctrl & (TYPE2_U | TYPE2_S | TYPE2_M));
> +    bool rwx = !!(ctrl & (TYPE2_LOAD | TYPE2_STORE | TYPE2_EXEC));
> +
> +    return mode && rwx;
> +}
> +
> +static target_ulong type2_mcontrol_validate(CPURISCVState *env,
> +                                            target_ulong ctrl)
> +{
> +    target_ulong val;
> +    uint32_t size;
> +
> +    /* validate the generic part first */
> +    val = tdata1_validate(env, ctrl, TRIGGER_TYPE_AD_MATCH);
> +
> +    /* validate unimplemented (always zero) bits */
> +    warn_always_zero_bit(ctrl, TYPE2_MATCH, "match");
> +    warn_always_zero_bit(ctrl, TYPE2_CHAIN, "chain");
> +    warn_always_zero_bit(ctrl, TYPE2_ACTION, "action");
> +    warn_always_zero_bit(ctrl, TYPE2_TIMING, "timing");
> +    warn_always_zero_bit(ctrl, TYPE2_SELECT, "select");
> +    warn_always_zero_bit(ctrl, TYPE2_HIT, "hit");
> +
> +    /* validate size encoding */
> +    size = type2_breakpoint_size(env, ctrl);
> +    if (access_size[size] == -1) {
> +        qemu_log_mask(LOG_UNIMP, "access size %d is not supported, using SIZE_ANY\n",
> +                      size);
> +    } else {
> +        val |= (ctrl & TYPE2_SIZELO);
> +        if (riscv_cpu_mxl(env) == MXL_RV64) {
> +            val |= (ctrl & TYPE2_SIZEHI);
> +        }
> +    }
> +
> +    /* keep the mode and attribute bits */
> +    val |= (ctrl & (TYPE2_U | TYPE2_S | TYPE2_M |
> +                    TYPE2_LOAD | TYPE2_STORE | TYPE2_EXEC));
> +
> +    return val;
> +}
> +
> +static void type2_breakpoint_insert(CPURISCVState *env, target_ulong index)
> +{
> +    target_ulong ctrl = env->trigger_type2[index].mcontrol;
> +    target_ulong addr = env->trigger_type2[index].maddress;
> +    bool enabled = type2_breakpoint_enabled(ctrl);
> +    CPUState *cs = env_cpu(env);
> +    int flags = BP_CPU | BP_STOP_BEFORE_ACCESS;
> +    uint32_t size;
> +
> +    if (!enabled) {
> +        return;
> +    }
> +
> +    if (ctrl & TYPE2_EXEC) {
> +        cpu_breakpoint_insert(cs, addr, flags, &env->trigger_type2[index].bp);
> +    }
> +
> +    if (ctrl & TYPE2_LOAD) {
> +        flags |= BP_MEM_READ;
> +    }
> +    if (ctrl & TYPE2_STORE) {
> +        flags |= BP_MEM_WRITE;
> +    }
> +
> +    if (flags & BP_MEM_ACCESS) {
> +        size = type2_breakpoint_size(env, ctrl);
> +        if (size != 0) {
> +            cpu_watchpoint_insert(cs, addr, size, flags,
> +                                  &env->trigger_type2[index].wp);
> +        } else {
> +            cpu_watchpoint_insert(cs, addr, 8, flags,
> +                                  &env->trigger_type2[index].wp);
> +        }
> +    }
> +}
> +
> +static void type2_breakpoint_remove(CPURISCVState *env, target_ulong index)
> +{
> +    CPUState *cs = env_cpu(env);
> +
> +    if (env->trigger_type2[index].bp) {
> +        cpu_breakpoint_remove_by_ref(cs, env->trigger_type2[index].bp);
> +        env->trigger_type2[index].bp = NULL;
> +    }
> +
> +    if (env->trigger_type2[index].wp) {
> +        cpu_watchpoint_remove_by_ref(cs, env->trigger_type2[index].wp);
> +        env->trigger_type2[index].wp = NULL;
> +    }
> +}
> +
> +static target_ulong type2_reg_read(CPURISCVState *env,
> +                                   target_ulong trigger_index, int tdata_index)
> +{
> +    uint32_t index = trigger_index - TRIGGER_TYPE2_IDX_0;
> +    target_ulong tdata;
> +
> +    switch (tdata_index) {
> +    case TDATA1:
> +        tdata = env->trigger_type2[index].mcontrol;
> +        break;
> +    case TDATA2:
> +        tdata = env->trigger_type2[index].maddress;
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    return tdata;
> +}
> +
> +static void type2_reg_write(CPURISCVState *env, target_ulong trigger_index,
> +                            int tdata_index, target_ulong val)
> +{
> +    uint32_t index = trigger_index - TRIGGER_TYPE2_IDX_0;
> +    target_ulong new_val;
> +
> +    switch (tdata_index) {
> +    case TDATA1:
> +        new_val = type2_mcontrol_validate(env, val);
> +        if (new_val != env->trigger_type2[index].mcontrol) {
> +            env->trigger_type2[index].mcontrol = new_val;
> +            type2_breakpoint_remove(env, index);
> +            type2_breakpoint_insert(env, index);
> +        }
> +        break;
> +    case TDATA2:
> +        if (val != env->trigger_type2[index].maddress) {
> +            env->trigger_type2[index].maddress = val;
> +            type2_breakpoint_remove(env, index);
> +            type2_breakpoint_insert(env, index);
> +        }
> +        break;
> +    default:
> +        g_assert_not_reached();
> +    }
> +
> +    return;
> +}
> +
> +typedef target_ulong (*tdata_read_func)(CPURISCVState *env,
> +                                        target_ulong trigger_index,
> +                                        int tdata_index);
> +
> +static tdata_read_func trigger_read_funcs[TRIGGER_NUM] = {
> +    [TRIGGER_TYPE2_IDX_0 ... TRIGGER_TYPE2_IDX_1] = type2_reg_read,
> +};
> +
> +typedef void (*tdata_write_func)(CPURISCVState *env,
> +                                 target_ulong trigger_index,
> +                                 int tdata_index,
> +                                 target_ulong val);
> +
> +static tdata_write_func trigger_write_funcs[TRIGGER_NUM] = {
> +    [TRIGGER_TYPE2_IDX_0 ... TRIGGER_TYPE2_IDX_1] = type2_reg_write,
> +};
> +
> +target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
> +{
> +    tdata_read_func read_func = trigger_read_funcs[env->trigger_cur];
> +
> +    return read_func(env, env->trigger_cur, tdata_index);
> +}
> +
> +void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
> +{
> +    tdata_write_func write_func = trigger_write_funcs[env->trigger_cur];
> +
> +    return write_func(env, env->trigger_cur, tdata_index, val);
> +}
> diff --git a/target/riscv/meson.build b/target/riscv/meson.build
> index d5e0bc93ea..966d97237a 100644
> --- a/target/riscv/meson.build
> +++ b/target/riscv/meson.build
> @@ -24,6 +24,7 @@ riscv_softmmu_ss = ss.source_set()
>  riscv_softmmu_ss.add(files(
>    'arch_dump.c',
>    'pmp.c',
> +  'debug.c',
>    'monitor.c',
>    'machine.c'
>  ))
> --
> 2.25.1
>
>


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

* Re: [RESEND PATCH v3 1/7] target/riscv: Add initial support for native debug
  2022-01-19  3:15     ` Alistair Francis
@ 2022-03-14  9:25       ` Bin Meng
  -1 siblings, 0 replies; 17+ messages in thread
From: Bin Meng @ 2022-03-14  9:25 UTC (permalink / raw)
  To: Alistair Francis
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Wed, Jan 19, 2022 at 11:16 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Jan 5, 2022 at 1:09 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > This adds initial support for the native debug via the Trigger Module,
> > as defined in the RISC-V Debug Specification [1].
>
> Doesn't this mean we are just supporting the Sdtrig extension?

I was looking at where Sdtrig is defined. It turns out this new name
was assigned in a later debug spec and when this patch series was
worked on the Sdtrig extention was not invented yet ...

So the answer is yes, only Sdtrig is supported. Sdext does not make
sense in the QEMU context as it is for the on-chip-debugging.

>
> >
> > Only "Address / Data Match" trigger (type 2) is implemented as of now,
> > which is mainly used for hardware breakpoint and watchpoint. The number
> > of type 2 triggers implemented is 2, which is the number that we can
> > find in the SiFive U54/U74 cores.
> >
> > [1] https://github.com/riscv/riscv-debug-spec/raw/master/riscv-debug-stable.pdf
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> > Changes in v3:
> > - drop riscv_trigger_init(), which will be moved to patch #5
> >
> >  target/riscv/cpu.h       |   5 +
> >  target/riscv/debug.h     | 108 +++++++++++++
> >  target/riscv/debug.c     | 339 +++++++++++++++++++++++++++++++++++++++
> >  target/riscv/meson.build |   1 +
> >  4 files changed, 453 insertions(+)
> >  create mode 100644 target/riscv/debug.h
> >  create mode 100644 target/riscv/debug.c
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index dc10f27093..0f3b3a4219 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -98,6 +98,7 @@ typedef struct CPURISCVState CPURISCVState;
> >
> >  #if !defined(CONFIG_USER_ONLY)
> >  #include "pmp.h"
> > +#include "debug.h"
> >  #endif
> >
> >  #define RV_VLEN_MAX 1024
> > @@ -234,6 +235,10 @@ struct CPURISCVState {
> >      pmp_table_t pmp_state;
> >      target_ulong mseccfg;
> >
> > +    /* trigger module */
> > +    target_ulong trigger_cur;
> > +    trigger_type2_t trigger_type2[TRIGGER_TYPE2_NUM];
> > +
> >      /* machine specific rdtime callback */
> >      uint64_t (*rdtime_fn)(uint32_t);
> >      uint32_t rdtime_fn_arg;
> > diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> > new file mode 100644
> > index 0000000000..0a3fda6c72
> > --- /dev/null
> > +++ b/target/riscv/debug.h
> > @@ -0,0 +1,108 @@
> > +/*
> > + * QEMU RISC-V Native Debug Support
> > + *
> > + * Copyright (c) 2022 Wind River Systems, Inc.
> > + *
> > + * Author:
> > + *   Bin Meng <bin.meng@windriver.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along with
> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef RISCV_DEBUG_H
> > +#define RISCV_DEBUG_H
> > +
> > +/* trigger indexes implemented */
> > +enum {
> > +    TRIGGER_TYPE2_IDX_0 = 0,
> > +    TRIGGER_TYPE2_IDX_1,
> > +    TRIGGER_TYPE2_NUM,
> > +    TRIGGER_NUM = TRIGGER_TYPE2_NUM
> > +};
> > +
> > +/* register index of tdata CSRs */
> > +enum {
> > +    TDATA1 = 0,
> > +    TDATA2,
> > +    TDATA3,
> > +    TDATA_NUM
> > +};
> > +
> > +typedef enum {
> > +    TRIGGER_TYPE_NO_EXIST = 0,      /* trigger does not exist */
> > +    TRIGGER_TYPE_AD_MATCH = 2,      /* address/data match trigger */
> > +    TRIGGER_TYPE_INST_CNT = 3,      /* instruction count trigger */
> > +    TRIGGER_TYPE_INT = 4,           /* interrupt trigger */
> > +    TRIGGER_TYPE_EXCP = 5,          /* exception trigger */
> > +    TRIGGER_TYPE_AD_MATCH6 = 6,     /* new address/data match trigger */
> > +    TRIGGER_TYPE_EXT_SRC = 7,       /* external source trigger */
> > +    TRIGGER_TYPE_UNAVAIL = 15       /* trigger exists, but unavailable */
> > +} trigger_type_t;
> > +
> > +typedef struct {
> > +    target_ulong mcontrol;
> > +    target_ulong maddress;
> > +    struct CPUBreakpoint *bp;
> > +    struct CPUWatchpoint *wp;
> > +} trigger_type2_t;
>
> This is a confusing name
>

I will change it to type2_tigger,

Regards,
Bin


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

* Re: [RESEND PATCH v3 1/7] target/riscv: Add initial support for native debug
@ 2022-03-14  9:25       ` Bin Meng
  0 siblings, 0 replies; 17+ messages in thread
From: Bin Meng @ 2022-03-14  9:25 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Bin Meng

On Wed, Jan 19, 2022 at 11:16 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Jan 5, 2022 at 1:09 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > This adds initial support for the native debug via the Trigger Module,
> > as defined in the RISC-V Debug Specification [1].
>
> Doesn't this mean we are just supporting the Sdtrig extension?

I was looking at where Sdtrig is defined. It turns out this new name
was assigned in a later debug spec and when this patch series was
worked on the Sdtrig extention was not invented yet ...

So the answer is yes, only Sdtrig is supported. Sdext does not make
sense in the QEMU context as it is for the on-chip-debugging.

>
> >
> > Only "Address / Data Match" trigger (type 2) is implemented as of now,
> > which is mainly used for hardware breakpoint and watchpoint. The number
> > of type 2 triggers implemented is 2, which is the number that we can
> > find in the SiFive U54/U74 cores.
> >
> > [1] https://github.com/riscv/riscv-debug-spec/raw/master/riscv-debug-stable.pdf
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> > Changes in v3:
> > - drop riscv_trigger_init(), which will be moved to patch #5
> >
> >  target/riscv/cpu.h       |   5 +
> >  target/riscv/debug.h     | 108 +++++++++++++
> >  target/riscv/debug.c     | 339 +++++++++++++++++++++++++++++++++++++++
> >  target/riscv/meson.build |   1 +
> >  4 files changed, 453 insertions(+)
> >  create mode 100644 target/riscv/debug.h
> >  create mode 100644 target/riscv/debug.c
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index dc10f27093..0f3b3a4219 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -98,6 +98,7 @@ typedef struct CPURISCVState CPURISCVState;
> >
> >  #if !defined(CONFIG_USER_ONLY)
> >  #include "pmp.h"
> > +#include "debug.h"
> >  #endif
> >
> >  #define RV_VLEN_MAX 1024
> > @@ -234,6 +235,10 @@ struct CPURISCVState {
> >      pmp_table_t pmp_state;
> >      target_ulong mseccfg;
> >
> > +    /* trigger module */
> > +    target_ulong trigger_cur;
> > +    trigger_type2_t trigger_type2[TRIGGER_TYPE2_NUM];
> > +
> >      /* machine specific rdtime callback */
> >      uint64_t (*rdtime_fn)(uint32_t);
> >      uint32_t rdtime_fn_arg;
> > diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> > new file mode 100644
> > index 0000000000..0a3fda6c72
> > --- /dev/null
> > +++ b/target/riscv/debug.h
> > @@ -0,0 +1,108 @@
> > +/*
> > + * QEMU RISC-V Native Debug Support
> > + *
> > + * Copyright (c) 2022 Wind River Systems, Inc.
> > + *
> > + * Author:
> > + *   Bin Meng <bin.meng@windriver.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2 or later, as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope it will be useful, but WITHOUT
> > + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> > + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> > + * more details.
> > + *
> > + * You should have received a copy of the GNU General Public License along with
> > + * this program.  If not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef RISCV_DEBUG_H
> > +#define RISCV_DEBUG_H
> > +
> > +/* trigger indexes implemented */
> > +enum {
> > +    TRIGGER_TYPE2_IDX_0 = 0,
> > +    TRIGGER_TYPE2_IDX_1,
> > +    TRIGGER_TYPE2_NUM,
> > +    TRIGGER_NUM = TRIGGER_TYPE2_NUM
> > +};
> > +
> > +/* register index of tdata CSRs */
> > +enum {
> > +    TDATA1 = 0,
> > +    TDATA2,
> > +    TDATA3,
> > +    TDATA_NUM
> > +};
> > +
> > +typedef enum {
> > +    TRIGGER_TYPE_NO_EXIST = 0,      /* trigger does not exist */
> > +    TRIGGER_TYPE_AD_MATCH = 2,      /* address/data match trigger */
> > +    TRIGGER_TYPE_INST_CNT = 3,      /* instruction count trigger */
> > +    TRIGGER_TYPE_INT = 4,           /* interrupt trigger */
> > +    TRIGGER_TYPE_EXCP = 5,          /* exception trigger */
> > +    TRIGGER_TYPE_AD_MATCH6 = 6,     /* new address/data match trigger */
> > +    TRIGGER_TYPE_EXT_SRC = 7,       /* external source trigger */
> > +    TRIGGER_TYPE_UNAVAIL = 15       /* trigger exists, but unavailable */
> > +} trigger_type_t;
> > +
> > +typedef struct {
> > +    target_ulong mcontrol;
> > +    target_ulong maddress;
> > +    struct CPUBreakpoint *bp;
> > +    struct CPUWatchpoint *wp;
> > +} trigger_type2_t;
>
> This is a confusing name
>

I will change it to type2_tigger,

Regards,
Bin


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

* Re: [RESEND PATCH v3 5/7] target/riscv: csr: Hook debug CSR read/write
  2022-01-19  3:06     ` Alistair Francis
@ 2022-03-14  9:44       ` Bin Meng
  -1 siblings, 0 replies; 17+ messages in thread
From: Bin Meng @ 2022-03-14  9:44 UTC (permalink / raw)
  To: Alistair Francis
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Wed, Jan 19, 2022 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Jan 5, 2022 at 1:15 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > This adds debug CSR read/write support to the RISC-V CSR RW table.
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> > Changes in v3:
> > - add riscv_trigger_init(), moved from patch #1 to this patch
> >
> >  target/riscv/debug.h |  2 ++
> >  target/riscv/cpu.c   |  6 +++++
> >  target/riscv/csr.c   | 57 ++++++++++++++++++++++++++++++++++++++++++++
> >  target/riscv/debug.c | 27 +++++++++++++++++++++
> >  4 files changed, 92 insertions(+)
> >
> > diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> > index d0f63e2414..f4da2db35d 100644
> > --- a/target/riscv/debug.h
> > +++ b/target/riscv/debug.h
> > @@ -109,4 +109,6 @@ void riscv_cpu_debug_excp_handler(CPUState *cs);
> >  bool riscv_cpu_debug_check_breakpoint(CPUState *cs);
> >  bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
> >
> > +void riscv_trigger_init(CPURISCVState *env);
> > +
> >  #endif /* RISCV_DEBUG_H */
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index d36c31ce9a..17dcc3c14f 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -575,6 +575,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> >
> >      riscv_cpu_register_gdb_regs_for_features(cs);
> >
> > +#ifndef CONFIG_USER_ONLY
> > +    if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> > +        riscv_trigger_init(env);
> > +    }
> > +#endif
> > +
> >      qemu_init_vcpu(cs);
> >      cpu_reset(cs);
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 146447eac5..189b9cc8c6 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -220,6 +220,15 @@ static RISCVException epmp(CPURISCVState *env, int csrno)
> >
> >      return RISCV_EXCP_ILLEGAL_INST;
> >  }
> > +
> > +static RISCVException debug(CPURISCVState *env, int csrno)
> > +{
> > +    if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> > +        return RISCV_EXCP_NONE;
> > +    }
> > +
> > +    return RISCV_EXCP_ILLEGAL_INST;
> > +}
> >  #endif
> >
> >  /* User Floating-Point CSRs */
> > @@ -1464,6 +1473,48 @@ static RISCVException write_pmpaddr(CPURISCVState *env, int csrno,
> >      return RISCV_EXCP_NONE;
> >  }
> >
> > +static RISCVException read_tselect(CPURISCVState *env, int csrno,
> > +                                   target_ulong *val)
> > +{
> > +    *val = tselect_csr_read(env);
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_tselect(CPURISCVState *env, int csrno,
> > +                                    target_ulong val)
> > +{
> > +    tselect_csr_write(env, val);
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_tdata(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    /* return 0 in tdata1 to end the trigger enumeration */
> > +    if (env->trigger_cur >= TRIGGER_NUM && csrno == CSR_TDATA1) {
> > +        *val = 0;
> > +        return RISCV_EXCP_NONE;
> > +    }
> > +
> > +    if (!tdata_available(env, csrno - CSR_TDATA1)) {
> > +        return RISCV_EXCP_ILLEGAL_INST;
> > +    }
> > +
> > +    *val = tdata_csr_read(env, csrno - CSR_TDATA1);
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_tdata(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    if (!tdata_available(env, csrno - CSR_TDATA1)) {
> > +        return RISCV_EXCP_ILLEGAL_INST;
> > +    }
> > +
> > +    tdata_csr_write(env, csrno - CSR_TDATA1, val);
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> >  /*
> >   * Functions to access Pointer Masking feature registers
> >   * We have to check if current priv lvl could modify
> > @@ -1962,6 +2013,12 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> >      [CSR_PMPADDR14] =  { "pmpaddr14", pmp, read_pmpaddr, write_pmpaddr },
> >      [CSR_PMPADDR15] =  { "pmpaddr15", pmp, read_pmpaddr, write_pmpaddr },
> >
> > +    /* Debug CSRs */
> > +    [CSR_TSELECT]   =  { "tselect", debug, read_tselect, write_tselect },
> > +    [CSR_TDATA1]    =  { "tdata1",  debug, read_tdata,   write_tdata   },
> > +    [CSR_TDATA2]    =  { "tdata2",  debug, read_tdata,   write_tdata   },
> > +    [CSR_TDATA3]    =  { "tdata3",  debug, read_tdata,   write_tdata   },
> > +
> >      /* User Pointer Masking */
> >      [CSR_UMTE]    =    { "umte",    pointer_masking, read_umte,    write_umte    },
> >      [CSR_UPMMASK] =    { "upmmask", pointer_masking, read_upmmask, write_upmmask },
> > diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> > index 7760c4611f..041a0d3a89 100644
> > --- a/target/riscv/debug.c
> > +++ b/target/riscv/debug.c
> > @@ -412,3 +412,30 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
> >
> >      return false;
> >  }
> > +
> > +void riscv_trigger_init(CPURISCVState *env)
> > +{
> > +    target_ulong type2 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
> > +    int i;
> > +
> > +    /* type 2 triggers */
> > +    for (i = 0; i < TRIGGER_TYPE2_NUM; i++) {
> > +        /*
> > +         * type = TRIGGER_TYPE_AD_MATCH
> > +         * dmode = 0 (both debug and M-mode can write tdata)
> > +         * maskmax = 0 (unimplemented, always 0)
> > +         * sizehi = 0 (match against any size, RV64 only)
> > +         * hit = 0 (unimplemented, always 0)
> > +         * select = 0 (always 0, perform match on address)
> > +         * timing = 0 (always 0, trigger before instruction)
> > +         * sizelo = 0 (match against any size)
> > +         * action = 0 (always 0, raise a breakpoint exception)
> > +         * chain = 0 (unimplemented, always 0)
> > +         * match = 0 (always 0, when any compare value equals tdata2)
> > +         */
> > +        env->trigger_type2[i].mcontrol = type2;
> > +        env->trigger_type2[i].maddress = 0;
> > +        env->trigger_type2[i].bp = NULL;
> > +        env->trigger_type2[i].wp = NULL;
> > +    }
>
> Should this be called at reset instead?

Yes, I think so.

Regards,
Bin


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

* Re: [RESEND PATCH v3 5/7] target/riscv: csr: Hook debug CSR read/write
@ 2022-03-14  9:44       ` Bin Meng
  0 siblings, 0 replies; 17+ messages in thread
From: Bin Meng @ 2022-03-14  9:44 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Bin Meng

On Wed, Jan 19, 2022 at 11:06 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Jan 5, 2022 at 1:15 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > From: Bin Meng <bin.meng@windriver.com>
> >
> > This adds debug CSR read/write support to the RISC-V CSR RW table.
> >
> > Signed-off-by: Bin Meng <bin.meng@windriver.com>
> > ---
> >
> > Changes in v3:
> > - add riscv_trigger_init(), moved from patch #1 to this patch
> >
> >  target/riscv/debug.h |  2 ++
> >  target/riscv/cpu.c   |  6 +++++
> >  target/riscv/csr.c   | 57 ++++++++++++++++++++++++++++++++++++++++++++
> >  target/riscv/debug.c | 27 +++++++++++++++++++++
> >  4 files changed, 92 insertions(+)
> >
> > diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> > index d0f63e2414..f4da2db35d 100644
> > --- a/target/riscv/debug.h
> > +++ b/target/riscv/debug.h
> > @@ -109,4 +109,6 @@ void riscv_cpu_debug_excp_handler(CPUState *cs);
> >  bool riscv_cpu_debug_check_breakpoint(CPUState *cs);
> >  bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
> >
> > +void riscv_trigger_init(CPURISCVState *env);
> > +
> >  #endif /* RISCV_DEBUG_H */
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index d36c31ce9a..17dcc3c14f 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -575,6 +575,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> >
> >      riscv_cpu_register_gdb_regs_for_features(cs);
> >
> > +#ifndef CONFIG_USER_ONLY
> > +    if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> > +        riscv_trigger_init(env);
> > +    }
> > +#endif
> > +
> >      qemu_init_vcpu(cs);
> >      cpu_reset(cs);
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 146447eac5..189b9cc8c6 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -220,6 +220,15 @@ static RISCVException epmp(CPURISCVState *env, int csrno)
> >
> >      return RISCV_EXCP_ILLEGAL_INST;
> >  }
> > +
> > +static RISCVException debug(CPURISCVState *env, int csrno)
> > +{
> > +    if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> > +        return RISCV_EXCP_NONE;
> > +    }
> > +
> > +    return RISCV_EXCP_ILLEGAL_INST;
> > +}
> >  #endif
> >
> >  /* User Floating-Point CSRs */
> > @@ -1464,6 +1473,48 @@ static RISCVException write_pmpaddr(CPURISCVState *env, int csrno,
> >      return RISCV_EXCP_NONE;
> >  }
> >
> > +static RISCVException read_tselect(CPURISCVState *env, int csrno,
> > +                                   target_ulong *val)
> > +{
> > +    *val = tselect_csr_read(env);
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_tselect(CPURISCVState *env, int csrno,
> > +                                    target_ulong val)
> > +{
> > +    tselect_csr_write(env, val);
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_tdata(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    /* return 0 in tdata1 to end the trigger enumeration */
> > +    if (env->trigger_cur >= TRIGGER_NUM && csrno == CSR_TDATA1) {
> > +        *val = 0;
> > +        return RISCV_EXCP_NONE;
> > +    }
> > +
> > +    if (!tdata_available(env, csrno - CSR_TDATA1)) {
> > +        return RISCV_EXCP_ILLEGAL_INST;
> > +    }
> > +
> > +    *val = tdata_csr_read(env, csrno - CSR_TDATA1);
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_tdata(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    if (!tdata_available(env, csrno - CSR_TDATA1)) {
> > +        return RISCV_EXCP_ILLEGAL_INST;
> > +    }
> > +
> > +    tdata_csr_write(env, csrno - CSR_TDATA1, val);
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> >  /*
> >   * Functions to access Pointer Masking feature registers
> >   * We have to check if current priv lvl could modify
> > @@ -1962,6 +2013,12 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> >      [CSR_PMPADDR14] =  { "pmpaddr14", pmp, read_pmpaddr, write_pmpaddr },
> >      [CSR_PMPADDR15] =  { "pmpaddr15", pmp, read_pmpaddr, write_pmpaddr },
> >
> > +    /* Debug CSRs */
> > +    [CSR_TSELECT]   =  { "tselect", debug, read_tselect, write_tselect },
> > +    [CSR_TDATA1]    =  { "tdata1",  debug, read_tdata,   write_tdata   },
> > +    [CSR_TDATA2]    =  { "tdata2",  debug, read_tdata,   write_tdata   },
> > +    [CSR_TDATA3]    =  { "tdata3",  debug, read_tdata,   write_tdata   },
> > +
> >      /* User Pointer Masking */
> >      [CSR_UMTE]    =    { "umte",    pointer_masking, read_umte,    write_umte    },
> >      [CSR_UPMMASK] =    { "upmmask", pointer_masking, read_upmmask, write_upmmask },
> > diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> > index 7760c4611f..041a0d3a89 100644
> > --- a/target/riscv/debug.c
> > +++ b/target/riscv/debug.c
> > @@ -412,3 +412,30 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
> >
> >      return false;
> >  }
> > +
> > +void riscv_trigger_init(CPURISCVState *env)
> > +{
> > +    target_ulong type2 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
> > +    int i;
> > +
> > +    /* type 2 triggers */
> > +    for (i = 0; i < TRIGGER_TYPE2_NUM; i++) {
> > +        /*
> > +         * type = TRIGGER_TYPE_AD_MATCH
> > +         * dmode = 0 (both debug and M-mode can write tdata)
> > +         * maskmax = 0 (unimplemented, always 0)
> > +         * sizehi = 0 (match against any size, RV64 only)
> > +         * hit = 0 (unimplemented, always 0)
> > +         * select = 0 (always 0, perform match on address)
> > +         * timing = 0 (always 0, trigger before instruction)
> > +         * sizelo = 0 (match against any size)
> > +         * action = 0 (always 0, raise a breakpoint exception)
> > +         * chain = 0 (unimplemented, always 0)
> > +         * match = 0 (always 0, when any compare value equals tdata2)
> > +         */
> > +        env->trigger_type2[i].mcontrol = type2;
> > +        env->trigger_type2[i].maddress = 0;
> > +        env->trigger_type2[i].bp = NULL;
> > +        env->trigger_type2[i].wp = NULL;
> > +    }
>
> Should this be called at reset instead?

Yes, I think so.

Regards,
Bin


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

end of thread, other threads:[~2022-03-14 10:03 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-05  3:08 [RESEND PATCH v3 0/7] target/riscv: Initial support for native debug feature via M-mode CSRs Bin Meng
2022-01-05  3:08 ` [RESEND PATCH v3 1/7] target/riscv: Add initial support for native debug Bin Meng
2022-01-19  3:15   ` Alistair Francis
2022-01-19  3:15     ` Alistair Francis
2022-03-14  9:25     ` Bin Meng
2022-03-14  9:25       ` Bin Meng
2022-01-05  3:08 ` [RESEND PATCH v3 2/7] target/riscv: machine: Add debug state description Bin Meng
2022-01-05  3:08 ` [RESEND PATCH v3 3/7] target/riscv: debug: Implement debug related TCGCPUOps Bin Meng
2022-01-05  3:08 ` [RESEND PATCH v3 4/7] target/riscv: cpu: Add a config option for native debug Bin Meng
2022-01-05  3:08 ` [RESEND PATCH v3 5/7] target/riscv: csr: Hook debug CSR read/write Bin Meng
2022-01-19  3:06   ` Alistair Francis
2022-01-19  3:06     ` Alistair Francis
2022-03-14  9:44     ` Bin Meng
2022-03-14  9:44       ` Bin Meng
2022-01-05  3:08 ` [RESEND PATCH v3 6/7] target/riscv: cpu: Enable native debug feature Bin Meng
2022-01-05  3:08 ` [RESEND PATCH v3 7/7] hw/core: tcg-cpu-ops.h: Update comments of debug_check_watchpoint() Bin Meng
2022-01-05  3:08   ` Bin Meng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.