All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/7] target/riscv: Initial support for the Sdtrig extension via M-mode CSRs
@ 2022-03-15  6:55 Bin Meng
  2022-03-15  6:55 ` [PATCH v4 1/7] target/riscv: Add initial support for the Sdtrig extension Bin Meng
                   ` (7 more replies)
  0 siblings, 8 replies; 35+ messages in thread
From: Bin Meng @ 2022-03-15  6:55 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv


This adds initial support for the Sdtrig extension 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

Changes in v4:
- mention Sdtrig extension in the commit
- rename 'struct trigger_type2_t' to 'type2_trigger_t'
- move riscv_trigger_init() call to riscv_cpu_reset()

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 the Sdtrig extension
  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            |   9 +-
 target/riscv/debug.h          | 114 +++++++++
 target/riscv/cpu.c            |  12 +
 target/riscv/csr.c            |  57 +++++
 target/riscv/debug.c          | 441 ++++++++++++++++++++++++++++++++++
 target/riscv/machine.c        |  32 +++
 target/riscv/meson.build      |   1 +
 8 files changed, 666 insertions(+), 1 deletion(-)
 create mode 100644 target/riscv/debug.h
 create mode 100644 target/riscv/debug.c

-- 
2.25.1



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

* [PATCH v4 1/7] target/riscv: Add initial support for the Sdtrig extension
  2022-03-15  6:55 [PATCH v4 0/7] target/riscv: Initial support for the Sdtrig extension via M-mode CSRs Bin Meng
@ 2022-03-15  6:55 ` Bin Meng
  2022-03-18  2:11     ` Alistair Francis
  2022-03-15  6:55 ` [PATCH v4 2/7] target/riscv: machine: Add debug state description Bin Meng
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 35+ messages in thread
From: Bin Meng @ 2022-03-15  6:55 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 Sdtrig extension 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 v4:
- mention Sdtrig extension in the commit
- rename 'struct trigger_type2_t' to 'type2_trigger_t'

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 c069fe85fa..ad35129239 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -102,6 +102,7 @@ typedef struct CPUArchState CPURISCVState;
 
 #if !defined(CONFIG_USER_ONLY)
 #include "pmp.h"
+#include "debug.h"
 #endif
 
 #define RV_VLEN_MAX 1024
@@ -267,6 +268,10 @@ struct CPUArchState {
     pmp_table_t pmp_state;
     target_ulong mseccfg;
 
+    /* trigger module */
+    target_ulong trigger_cur;
+    type2_trigger_t type2_trig[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..fbc5f946e2
--- /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;
+} type2_trigger_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..c8cec39217
--- /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->type2_trig[index].mcontrol;
+    target_ulong addr = env->type2_trig[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->type2_trig[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->type2_trig[index].wp);
+        } else {
+            cpu_watchpoint_insert(cs, addr, 8, flags,
+                                  &env->type2_trig[index].wp);
+        }
+    }
+}
+
+static void type2_breakpoint_remove(CPURISCVState *env, target_ulong index)
+{
+    CPUState *cs = env_cpu(env);
+
+    if (env->type2_trig[index].bp) {
+        cpu_breakpoint_remove_by_ref(cs, env->type2_trig[index].bp);
+        env->type2_trig[index].bp = NULL;
+    }
+
+    if (env->type2_trig[index].wp) {
+        cpu_watchpoint_remove_by_ref(cs, env->type2_trig[index].wp);
+        env->type2_trig[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->type2_trig[index].mcontrol;
+        break;
+    case TDATA2:
+        tdata = env->type2_trig[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->type2_trig[index].mcontrol) {
+            env->type2_trig[index].mcontrol = new_val;
+            type2_breakpoint_remove(env, index);
+            type2_breakpoint_insert(env, index);
+        }
+        break;
+    case TDATA2:
+        if (val != env->type2_trig[index].maddress) {
+            env->type2_trig[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 91f0ac32ff..2c20f3dd8e 100644
--- a/target/riscv/meson.build
+++ b/target/riscv/meson.build
@@ -27,6 +27,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] 35+ messages in thread

* [PATCH v4 2/7] target/riscv: machine: Add debug state description
  2022-03-15  6:55 [PATCH v4 0/7] target/riscv: Initial support for the Sdtrig extension via M-mode CSRs Bin Meng
  2022-03-15  6:55 ` [PATCH v4 1/7] target/riscv: Add initial support for the Sdtrig extension Bin Meng
@ 2022-03-15  6:55 ` Bin Meng
  2022-04-20  7:30     ` Alistair Francis
  2022-03-15  6:55 ` [PATCH v4 3/7] target/riscv: debug: Implement debug related TCGCPUOps Bin Meng
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 35+ messages in thread
From: Bin Meng @ 2022-03-15  6:55 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 | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 5178b3fec9..4921dad09d 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
         VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
         VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
         VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+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, type2_trigger_t),
+        VMSTATE_UINTTL(maddress, type2_trigger_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.type2_trig, RISCVCPU, TRIGGER_TYPE2_NUM,
+                             0, vmstate_debug_type2, type2_trigger_t),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -292,6 +323,7 @@ const VMStateDescription vmstate_riscv_cpu = {
         &vmstate_pointermasking,
         &vmstate_rv128,
         &vmstate_kvmtimer,
+        &vmstate_debug,
         NULL
     }
 };
-- 
2.25.1



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

* [PATCH v4 3/7] target/riscv: debug: Implement debug related TCGCPUOps
  2022-03-15  6:55 [PATCH v4 0/7] target/riscv: Initial support for the Sdtrig extension via M-mode CSRs Bin Meng
  2022-03-15  6:55 ` [PATCH v4 1/7] target/riscv: Add initial support for the Sdtrig extension Bin Meng
  2022-03-15  6:55 ` [PATCH v4 2/7] target/riscv: machine: Add debug state description Bin Meng
@ 2022-03-15  6:55 ` Bin Meng
  2022-03-15  6:55 ` [PATCH v4 4/7] target/riscv: cpu: Add a config option for native debug Bin Meng
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-03-15  6:55 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 fbc5f946e2..fb21706e1c 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 ddda4906ff..6a4c94da2a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -865,6 +865,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 c8cec39217..1a9392645e 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->type2_trig[i].mcontrol;
+            pc = env->type2_trig[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->type2_trig[i].mcontrol;
+        addr = env->type2_trig[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] 35+ messages in thread

* [PATCH v4 4/7] target/riscv: cpu: Add a config option for native debug
  2022-03-15  6:55 [PATCH v4 0/7] target/riscv: Initial support for the Sdtrig extension via M-mode CSRs Bin Meng
                   ` (2 preceding siblings ...)
  2022-03-15  6:55 ` [PATCH v4 3/7] target/riscv: debug: Implement debug related TCGCPUOps Bin Meng
@ 2022-03-15  6:55 ` Bin Meng
  2022-03-15  6:55 ` [PATCH v4 5/7] target/riscv: csr: Hook debug CSR read/write Bin Meng
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-03-15  6:55 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 | 4 +++-
 target/riscv/cpu.c | 5 +++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index ad35129239..d3e884452b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -79,7 +79,8 @@ enum {
     RISCV_FEATURE_PMP,
     RISCV_FEATURE_EPMP,
     RISCV_FEATURE_MISA,
-    RISCV_FEATURE_AIA
+    RISCV_FEATURE_AIA,
+    RISCV_FEATURE_DEBUG
 };
 
 #define PRIV_VERSION_1_10_0 0x00011000
@@ -388,6 +389,7 @@ struct RISCVCPUConfig {
     bool pmp;
     bool epmp;
     bool aia;
+    bool debug;
     uint64_t resetvec;
 };
 
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 6a4c94da2a..eb2be5fa05 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -541,6 +541,10 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
         riscv_set_feature(env, RISCV_FEATURE_AIA);
     }
 
+    if (cpu->cfg.debug) {
+        riscv_set_feature(env, RISCV_FEATURE_DEBUG);
+    }
+
     set_resetvec(env, cpu->cfg.resetvec);
 
     /* Validate that MISA_MXL is set properly. */
@@ -780,6 +784,7 @@ static Property riscv_cpu_properties[] = {
     DEFINE_PROP_BOOL("Zve64f", RISCVCPU, cfg.ext_zve64f, 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] 35+ messages in thread

* [PATCH v4 5/7] target/riscv: csr: Hook debug CSR read/write
  2022-03-15  6:55 [PATCH v4 0/7] target/riscv: Initial support for the Sdtrig extension via M-mode CSRs Bin Meng
                   ` (3 preceding siblings ...)
  2022-03-15  6:55 ` [PATCH v4 4/7] target/riscv: cpu: Add a config option for native debug Bin Meng
@ 2022-03-15  6:55 ` Bin Meng
  2022-03-18  2:14     ` Alistair Francis
  2022-03-15  6:55 ` [PATCH v4 6/7] target/riscv: cpu: Enable native debug feature Bin Meng
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 35+ messages in thread
From: Bin Meng @ 2022-03-15  6:55 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 v4:
- move riscv_trigger_init() call to riscv_cpu_reset()

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

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

diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index fb21706e1c..27b9cac6b4 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 eb2be5fa05..ba9cc3bcd6 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -461,6 +461,10 @@ static void riscv_cpu_reset(DeviceState *dev)
     set_default_nan_mode(1, &env->fp_status);
 
 #ifndef CONFIG_USER_ONLY
+    if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
+        riscv_trigger_init(env);
+    }
+
     if (kvm_enabled()) {
         kvm_riscv_reset_vcpu(cpu);
     }
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0606cd0ea8..3b9008709d 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -290,6 +290,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 */
@@ -2576,6 +2585,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
@@ -3265,6 +3316,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 1a9392645e..2f2a51c732 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->type2_trig[i].mcontrol = type2;
+        env->type2_trig[i].maddress = 0;
+        env->type2_trig[i].bp = NULL;
+        env->type2_trig[i].wp = NULL;
+    }
+}
-- 
2.25.1



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

* [PATCH v4 6/7] target/riscv: cpu: Enable native debug feature
  2022-03-15  6:55 [PATCH v4 0/7] target/riscv: Initial support for the Sdtrig extension via M-mode CSRs Bin Meng
                   ` (4 preceding siblings ...)
  2022-03-15  6:55 ` [PATCH v4 5/7] target/riscv: csr: Hook debug CSR read/write Bin Meng
@ 2022-03-15  6:55 ` Bin Meng
  2022-03-18  2:17     ` Alistair Francis
  2022-03-15  6:55   ` Bin Meng
  2022-03-18  7:38   ` Alistair Francis
  7 siblings, 1 reply; 35+ messages in thread
From: Bin Meng @ 2022-03-15  6:55 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>
---

(no changes since v3)

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 ba9cc3bcd6..08266b163d 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -788,7 +788,7 @@ static Property riscv_cpu_properties[] = {
     DEFINE_PROP_BOOL("Zve64f", RISCVCPU, cfg.ext_zve64f, 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] 35+ messages in thread

* [PATCH v4 7/7] hw/core: tcg-cpu-ops.h: Update comments of debug_check_watchpoint()
  2022-03-15  6:55 [PATCH v4 0/7] target/riscv: Initial support for the Sdtrig extension via M-mode CSRs Bin Meng
@ 2022-03-15  6:55   ` Bin Meng
  2022-03-15  6:55 ` [PATCH v4 2/7] target/riscv: machine: Add debug state description Bin Meng
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-03-15  6:55 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] 35+ messages in thread

* [PATCH v4 7/7] hw/core: tcg-cpu-ops.h: Update comments of debug_check_watchpoint()
@ 2022-03-15  6:55   ` Bin Meng
  0 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-03-15  6:55 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] 35+ messages in thread

* Re: [PATCH v4 1/7] target/riscv: Add initial support for the Sdtrig extension
  2022-03-15  6:55 ` [PATCH v4 1/7] target/riscv: Add initial support for the Sdtrig extension Bin Meng
@ 2022-03-18  2:11     ` Alistair Francis
  0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-03-18  2:11 UTC (permalink / raw)
  To: Bin Meng
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Tue, Mar 15, 2022 at 5:20 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This adds initial support for the Sdtrig extension 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>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
> Changes in v4:
> - mention Sdtrig extension in the commit
> - rename 'struct trigger_type2_t' to 'type2_trigger_t'
>
> 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 c069fe85fa..ad35129239 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -102,6 +102,7 @@ typedef struct CPUArchState CPURISCVState;
>
>  #if !defined(CONFIG_USER_ONLY)
>  #include "pmp.h"
> +#include "debug.h"
>  #endif
>
>  #define RV_VLEN_MAX 1024
> @@ -267,6 +268,10 @@ struct CPUArchState {
>      pmp_table_t pmp_state;
>      target_ulong mseccfg;
>
> +    /* trigger module */
> +    target_ulong trigger_cur;
> +    type2_trigger_t type2_trig[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..fbc5f946e2
> --- /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;
> +} type2_trigger_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..c8cec39217
> --- /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->type2_trig[index].mcontrol;
> +    target_ulong addr = env->type2_trig[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->type2_trig[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->type2_trig[index].wp);
> +        } else {
> +            cpu_watchpoint_insert(cs, addr, 8, flags,
> +                                  &env->type2_trig[index].wp);
> +        }
> +    }
> +}
> +
> +static void type2_breakpoint_remove(CPURISCVState *env, target_ulong index)
> +{
> +    CPUState *cs = env_cpu(env);
> +
> +    if (env->type2_trig[index].bp) {
> +        cpu_breakpoint_remove_by_ref(cs, env->type2_trig[index].bp);
> +        env->type2_trig[index].bp = NULL;
> +    }
> +
> +    if (env->type2_trig[index].wp) {
> +        cpu_watchpoint_remove_by_ref(cs, env->type2_trig[index].wp);
> +        env->type2_trig[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->type2_trig[index].mcontrol;
> +        break;
> +    case TDATA2:
> +        tdata = env->type2_trig[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->type2_trig[index].mcontrol) {
> +            env->type2_trig[index].mcontrol = new_val;
> +            type2_breakpoint_remove(env, index);
> +            type2_breakpoint_insert(env, index);
> +        }
> +        break;
> +    case TDATA2:
> +        if (val != env->type2_trig[index].maddress) {
> +            env->type2_trig[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 91f0ac32ff..2c20f3dd8e 100644
> --- a/target/riscv/meson.build
> +++ b/target/riscv/meson.build
> @@ -27,6 +27,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] 35+ messages in thread

* Re: [PATCH v4 1/7] target/riscv: Add initial support for the Sdtrig extension
@ 2022-03-18  2:11     ` Alistair Francis
  0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-03-18  2:11 UTC (permalink / raw)
  To: Bin Meng
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Bin Meng

On Tue, Mar 15, 2022 at 5:20 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> From: Bin Meng <bin.meng@windriver.com>
>
> This adds initial support for the Sdtrig extension 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>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
> Changes in v4:
> - mention Sdtrig extension in the commit
> - rename 'struct trigger_type2_t' to 'type2_trigger_t'
>
> 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 c069fe85fa..ad35129239 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -102,6 +102,7 @@ typedef struct CPUArchState CPURISCVState;
>
>  #if !defined(CONFIG_USER_ONLY)
>  #include "pmp.h"
> +#include "debug.h"
>  #endif
>
>  #define RV_VLEN_MAX 1024
> @@ -267,6 +268,10 @@ struct CPUArchState {
>      pmp_table_t pmp_state;
>      target_ulong mseccfg;
>
> +    /* trigger module */
> +    target_ulong trigger_cur;
> +    type2_trigger_t type2_trig[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..fbc5f946e2
> --- /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;
> +} type2_trigger_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..c8cec39217
> --- /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->type2_trig[index].mcontrol;
> +    target_ulong addr = env->type2_trig[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->type2_trig[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->type2_trig[index].wp);
> +        } else {
> +            cpu_watchpoint_insert(cs, addr, 8, flags,
> +                                  &env->type2_trig[index].wp);
> +        }
> +    }
> +}
> +
> +static void type2_breakpoint_remove(CPURISCVState *env, target_ulong index)
> +{
> +    CPUState *cs = env_cpu(env);
> +
> +    if (env->type2_trig[index].bp) {
> +        cpu_breakpoint_remove_by_ref(cs, env->type2_trig[index].bp);
> +        env->type2_trig[index].bp = NULL;
> +    }
> +
> +    if (env->type2_trig[index].wp) {
> +        cpu_watchpoint_remove_by_ref(cs, env->type2_trig[index].wp);
> +        env->type2_trig[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->type2_trig[index].mcontrol;
> +        break;
> +    case TDATA2:
> +        tdata = env->type2_trig[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->type2_trig[index].mcontrol) {
> +            env->type2_trig[index].mcontrol = new_val;
> +            type2_breakpoint_remove(env, index);
> +            type2_breakpoint_insert(env, index);
> +        }
> +        break;
> +    case TDATA2:
> +        if (val != env->type2_trig[index].maddress) {
> +            env->type2_trig[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 91f0ac32ff..2c20f3dd8e 100644
> --- a/target/riscv/meson.build
> +++ b/target/riscv/meson.build
> @@ -27,6 +27,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] 35+ messages in thread

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

On Tue, Mar 15, 2022 at 5:08 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>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
> Changes in v4:
> - move riscv_trigger_init() call to riscv_cpu_reset()
>
> Changes in v3:
> - add riscv_trigger_init(), moved from patch #1 to this patch
>
>  target/riscv/debug.h |  2 ++
>  target/riscv/cpu.c   |  4 ++++
>  target/riscv/csr.c   | 57 ++++++++++++++++++++++++++++++++++++++++++++
>  target/riscv/debug.c | 27 +++++++++++++++++++++
>  4 files changed, 90 insertions(+)
>
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index fb21706e1c..27b9cac6b4 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 eb2be5fa05..ba9cc3bcd6 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -461,6 +461,10 @@ static void riscv_cpu_reset(DeviceState *dev)
>      set_default_nan_mode(1, &env->fp_status);
>
>  #ifndef CONFIG_USER_ONLY
> +    if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> +        riscv_trigger_init(env);
> +    }
> +
>      if (kvm_enabled()) {
>          kvm_riscv_reset_vcpu(cpu);
>      }
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 0606cd0ea8..3b9008709d 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -290,6 +290,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 */
> @@ -2576,6 +2585,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
> @@ -3265,6 +3316,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 1a9392645e..2f2a51c732 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->type2_trig[i].mcontrol = type2;
> +        env->type2_trig[i].maddress = 0;
> +        env->type2_trig[i].bp = NULL;
> +        env->type2_trig[i].wp = NULL;
> +    }
> +}
> --
> 2.25.1
>
>


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

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

On Tue, Mar 15, 2022 at 5:08 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>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
> Changes in v4:
> - move riscv_trigger_init() call to riscv_cpu_reset()
>
> Changes in v3:
> - add riscv_trigger_init(), moved from patch #1 to this patch
>
>  target/riscv/debug.h |  2 ++
>  target/riscv/cpu.c   |  4 ++++
>  target/riscv/csr.c   | 57 ++++++++++++++++++++++++++++++++++++++++++++
>  target/riscv/debug.c | 27 +++++++++++++++++++++
>  4 files changed, 90 insertions(+)
>
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index fb21706e1c..27b9cac6b4 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 eb2be5fa05..ba9cc3bcd6 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -461,6 +461,10 @@ static void riscv_cpu_reset(DeviceState *dev)
>      set_default_nan_mode(1, &env->fp_status);
>
>  #ifndef CONFIG_USER_ONLY
> +    if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> +        riscv_trigger_init(env);
> +    }
> +
>      if (kvm_enabled()) {
>          kvm_riscv_reset_vcpu(cpu);
>      }
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 0606cd0ea8..3b9008709d 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -290,6 +290,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 */
> @@ -2576,6 +2585,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
> @@ -3265,6 +3316,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 1a9392645e..2f2a51c732 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->type2_trig[i].mcontrol = type2;
> +        env->type2_trig[i].maddress = 0;
> +        env->type2_trig[i].bp = NULL;
> +        env->type2_trig[i].wp = NULL;
> +    }
> +}
> --
> 2.25.1
>
>


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

* Re: [PATCH v4 6/7] target/riscv: cpu: Enable native debug feature
  2022-03-15  6:55 ` [PATCH v4 6/7] target/riscv: cpu: Enable native debug feature Bin Meng
@ 2022-03-18  2:17     ` Alistair Francis
  0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-03-18  2:17 UTC (permalink / raw)
  To: Bin Meng
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Tue, Mar 15, 2022 at 5:02 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> 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>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
> (no changes since v3)
>
> 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 ba9cc3bcd6..08266b163d 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -788,7 +788,7 @@ static Property riscv_cpu_properties[] = {
>      DEFINE_PROP_BOOL("Zve64f", RISCVCPU, cfg.ext_zve64f, 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	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 6/7] target/riscv: cpu: Enable native debug feature
@ 2022-03-18  2:17     ` Alistair Francis
  0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-03-18  2:17 UTC (permalink / raw)
  To: Bin Meng
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Bin Meng

On Tue, Mar 15, 2022 at 5:02 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> 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>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>
> (no changes since v3)
>
> 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 ba9cc3bcd6..08266b163d 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -788,7 +788,7 @@ static Property riscv_cpu_properties[] = {
>      DEFINE_PROP_BOOL("Zve64f", RISCVCPU, cfg.ext_zve64f, 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	[flat|nested] 35+ messages in thread

* Re: [PATCH v4 0/7] target/riscv: Initial support for the Sdtrig extension via M-mode CSRs
  2022-03-15  6:55 [PATCH v4 0/7] target/riscv: Initial support for the Sdtrig extension via M-mode CSRs Bin Meng
@ 2022-03-18  7:38   ` Alistair Francis
  2022-03-15  6:55 ` [PATCH v4 2/7] target/riscv: machine: Add debug state description Bin Meng
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-03-18  7:38 UTC (permalink / raw)
  To: Bin Meng
  Cc: open list:RISC-V, Alistair Francis, qemu-devel@nongnu.org Developers

On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
>
> This adds initial support for the Sdtrig extension 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
>
> Changes in v4:
> - mention Sdtrig extension in the commit
> - rename 'struct trigger_type2_t' to 'type2_trigger_t'
> - move riscv_trigger_init() call to riscv_cpu_reset()
>
> 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 the Sdtrig extension
>   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()

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  include/hw/core/tcg-cpu-ops.h |   1 +
>  target/riscv/cpu.h            |   9 +-
>  target/riscv/debug.h          | 114 +++++++++
>  target/riscv/cpu.c            |  12 +
>  target/riscv/csr.c            |  57 +++++
>  target/riscv/debug.c          | 441 ++++++++++++++++++++++++++++++++++
>  target/riscv/machine.c        |  32 +++
>  target/riscv/meson.build      |   1 +
>  8 files changed, 666 insertions(+), 1 deletion(-)
>  create mode 100644 target/riscv/debug.h
>  create mode 100644 target/riscv/debug.c
>
> --
> 2.25.1
>
>


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

* Re: [PATCH v4 0/7] target/riscv: Initial support for the Sdtrig extension via M-mode CSRs
@ 2022-03-18  7:38   ` Alistair Francis
  0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-03-18  7:38 UTC (permalink / raw)
  To: Bin Meng
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers, open list:RISC-V

On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
>
> This adds initial support for the Sdtrig extension 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
>
> Changes in v4:
> - mention Sdtrig extension in the commit
> - rename 'struct trigger_type2_t' to 'type2_trigger_t'
> - move riscv_trigger_init() call to riscv_cpu_reset()
>
> 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 the Sdtrig extension
>   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()

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  include/hw/core/tcg-cpu-ops.h |   1 +
>  target/riscv/cpu.h            |   9 +-
>  target/riscv/debug.h          | 114 +++++++++
>  target/riscv/cpu.c            |  12 +
>  target/riscv/csr.c            |  57 +++++
>  target/riscv/debug.c          | 441 ++++++++++++++++++++++++++++++++++
>  target/riscv/machine.c        |  32 +++
>  target/riscv/meson.build      |   1 +
>  8 files changed, 666 insertions(+), 1 deletion(-)
>  create mode 100644 target/riscv/debug.h
>  create mode 100644 target/riscv/debug.c
>
> --
> 2.25.1
>
>


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
  2022-03-15  6:55 ` [PATCH v4 2/7] target/riscv: machine: Add debug state description Bin Meng
@ 2022-04-20  7:30     ` Alistair Francis
  0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-04-20  7:30 UTC (permalink / raw)
  To: Bin Meng
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> 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 | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 5178b3fec9..4921dad09d 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
>          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
>          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
>          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static bool debug_needed(void *opaque)
> +{
> +    RISCVCPU *cpu = opaque;
> +    CPURISCVState *env = &cpu->env;
> +
> +    return riscv_feature(env, RISCV_FEATURE_DEBUG);

This fails to build:

../target/riscv/machine.c: In function ‘debug_needed’:
../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
undeclared (first use in this function); did you mean
‘RISCV_FEATURE_EPMP’?
 228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
     |                               ^~~~~~~~~~~~~~~~~~~
     |                               RISCV_FEATURE_EPMP
../target/riscv/machine.c:228:31: note: each undeclared identifier is
reported only once for each function it appears in
../target/riscv/machine.c:229:1: warning: control reaches end of
non-void function [-Wreturn-type]
 229 | }
     | ^

Alistair

> +}
>
> +static const VMStateDescription vmstate_debug_type2 = {
> +    .name = "cpu/debug/type2",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINTTL(mcontrol, type2_trigger_t),
> +        VMSTATE_UINTTL(maddress, type2_trigger_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.type2_trig, RISCVCPU, TRIGGER_TYPE2_NUM,
> +                             0, vmstate_debug_type2, type2_trigger_t),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> @@ -292,6 +323,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>          &vmstate_pointermasking,
>          &vmstate_rv128,
>          &vmstate_kvmtimer,
> +        &vmstate_debug,
>          NULL
>      }
>  };
> --
> 2.25.1
>
>


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
@ 2022-04-20  7:30     ` Alistair Francis
  0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-04-20  7:30 UTC (permalink / raw)
  To: Bin Meng
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Bin Meng

On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> 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 | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 5178b3fec9..4921dad09d 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
>          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
>          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
>          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static bool debug_needed(void *opaque)
> +{
> +    RISCVCPU *cpu = opaque;
> +    CPURISCVState *env = &cpu->env;
> +
> +    return riscv_feature(env, RISCV_FEATURE_DEBUG);

This fails to build:

../target/riscv/machine.c: In function ‘debug_needed’:
../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
undeclared (first use in this function); did you mean
‘RISCV_FEATURE_EPMP’?
 228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
     |                               ^~~~~~~~~~~~~~~~~~~
     |                               RISCV_FEATURE_EPMP
../target/riscv/machine.c:228:31: note: each undeclared identifier is
reported only once for each function it appears in
../target/riscv/machine.c:229:1: warning: control reaches end of
non-void function [-Wreturn-type]
 229 | }
     | ^

Alistair

> +}
>
> +static const VMStateDescription vmstate_debug_type2 = {
> +    .name = "cpu/debug/type2",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINTTL(mcontrol, type2_trigger_t),
> +        VMSTATE_UINTTL(maddress, type2_trigger_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.type2_trig, RISCVCPU, TRIGGER_TYPE2_NUM,
> +                             0, vmstate_debug_type2, type2_trigger_t),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> @@ -292,6 +323,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>          &vmstate_pointermasking,
>          &vmstate_rv128,
>          &vmstate_kvmtimer,
> +        &vmstate_debug,
>          NULL
>      }
>  };
> --
> 2.25.1
>
>


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
  2022-04-20  7:30     ` Alistair Francis
@ 2022-04-20  7:33       ` Bin Meng
  -1 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-04-20  7:33 UTC (permalink / raw)
  To: Alistair Francis
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Wed, Apr 20, 2022 at 3:31 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > 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 | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > index 5178b3fec9..4921dad09d 100644
> > --- a/target/riscv/machine.c
> > +++ b/target/riscv/machine.c
> > @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
> >          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> >          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> >          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> > +static bool debug_needed(void *opaque)
> > +{
> > +    RISCVCPU *cpu = opaque;
> > +    CPURISCVState *env = &cpu->env;
> > +
> > +    return riscv_feature(env, RISCV_FEATURE_DEBUG);
>
> This fails to build:
>
> ../target/riscv/machine.c: In function ‘debug_needed’:
> ../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
> undeclared (first use in this function); did you mean
> ‘RISCV_FEATURE_EPMP’?
>  228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
>      |                               ^~~~~~~~~~~~~~~~~~~
>      |                               RISCV_FEATURE_EPMP
> ../target/riscv/machine.c:228:31: note: each undeclared identifier is
> reported only once for each function it appears in
> ../target/riscv/machine.c:229:1: warning: control reaches end of
> non-void function [-Wreturn-type]
>  229 | }
>      | ^

That's weird. Maybe it's out of sync or merge conflict? I will take a look.

Regards,
Bin


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
@ 2022-04-20  7:33       ` Bin Meng
  0 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-04-20  7:33 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Bin Meng

On Wed, Apr 20, 2022 at 3:31 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > 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 | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> >
> > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > index 5178b3fec9..4921dad09d 100644
> > --- a/target/riscv/machine.c
> > +++ b/target/riscv/machine.c
> > @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
> >          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> >          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> >          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> > +static bool debug_needed(void *opaque)
> > +{
> > +    RISCVCPU *cpu = opaque;
> > +    CPURISCVState *env = &cpu->env;
> > +
> > +    return riscv_feature(env, RISCV_FEATURE_DEBUG);
>
> This fails to build:
>
> ../target/riscv/machine.c: In function ‘debug_needed’:
> ../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
> undeclared (first use in this function); did you mean
> ‘RISCV_FEATURE_EPMP’?
>  228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
>      |                               ^~~~~~~~~~~~~~~~~~~
>      |                               RISCV_FEATURE_EPMP
> ../target/riscv/machine.c:228:31: note: each undeclared identifier is
> reported only once for each function it appears in
> ../target/riscv/machine.c:229:1: warning: control reaches end of
> non-void function [-Wreturn-type]
>  229 | }
>      | ^

That's weird. Maybe it's out of sync or merge conflict? I will take a look.

Regards,
Bin


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
  2022-04-20  7:33       ` Bin Meng
@ 2022-04-20  9:52         ` Bin Meng
  -1 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-04-20  9:52 UTC (permalink / raw)
  To: Alistair Francis
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

Hi Alistair,

On Wed, Apr 20, 2022 at 3:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Apr 20, 2022 at 3:31 PM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > 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 | 32 ++++++++++++++++++++++++++++++++
> > >  1 file changed, 32 insertions(+)
> > >
> > > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > > index 5178b3fec9..4921dad09d 100644
> > > --- a/target/riscv/machine.c
> > > +++ b/target/riscv/machine.c
> > > @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
> > >          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> > >          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> > >          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> > > +        VMSTATE_END_OF_LIST()
> > > +    }
> > > +};
> > > +
> > > +static bool debug_needed(void *opaque)
> > > +{
> > > +    RISCVCPU *cpu = opaque;
> > > +    CPURISCVState *env = &cpu->env;
> > > +
> > > +    return riscv_feature(env, RISCV_FEATURE_DEBUG);
> >
> > This fails to build:
> >
> > ../target/riscv/machine.c: In function ‘debug_needed’:
> > ../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
> > undeclared (first use in this function); did you mean
> > ‘RISCV_FEATURE_EPMP’?
> >  228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
> >      |                               ^~~~~~~~~~~~~~~~~~~
> >      |                               RISCV_FEATURE_EPMP
> > ../target/riscv/machine.c:228:31: note: each undeclared identifier is
> > reported only once for each function it appears in
> > ../target/riscv/machine.c:229:1: warning: control reaches end of
> > non-void function [-Wreturn-type]
> >  229 | }
> >      | ^
>
> That's weird. Maybe it's out of sync or merge conflict? I will take a look.
>

I rebased the v4 series on top of your riscv-to-apply.next branch,
indeed there is a merge conflict of target/riscv/machine.c. After I
resolved the conflict, the build succeeded.

I suspect you missed something during your handling of the merge conflict?

Regards,
Bin


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
@ 2022-04-20  9:52         ` Bin Meng
  0 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-04-20  9:52 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Bin Meng

Hi Alistair,

On Wed, Apr 20, 2022 at 3:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Apr 20, 2022 at 3:31 PM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > 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 | 32 ++++++++++++++++++++++++++++++++
> > >  1 file changed, 32 insertions(+)
> > >
> > > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > > index 5178b3fec9..4921dad09d 100644
> > > --- a/target/riscv/machine.c
> > > +++ b/target/riscv/machine.c
> > > @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
> > >          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> > >          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> > >          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> > > +        VMSTATE_END_OF_LIST()
> > > +    }
> > > +};
> > > +
> > > +static bool debug_needed(void *opaque)
> > > +{
> > > +    RISCVCPU *cpu = opaque;
> > > +    CPURISCVState *env = &cpu->env;
> > > +
> > > +    return riscv_feature(env, RISCV_FEATURE_DEBUG);
> >
> > This fails to build:
> >
> > ../target/riscv/machine.c: In function ‘debug_needed’:
> > ../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
> > undeclared (first use in this function); did you mean
> > ‘RISCV_FEATURE_EPMP’?
> >  228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
> >      |                               ^~~~~~~~~~~~~~~~~~~
> >      |                               RISCV_FEATURE_EPMP
> > ../target/riscv/machine.c:228:31: note: each undeclared identifier is
> > reported only once for each function it appears in
> > ../target/riscv/machine.c:229:1: warning: control reaches end of
> > non-void function [-Wreturn-type]
> >  229 | }
> >      | ^
>
> That's weird. Maybe it's out of sync or merge conflict? I will take a look.
>

I rebased the v4 series on top of your riscv-to-apply.next branch,
indeed there is a merge conflict of target/riscv/machine.c. After I
resolved the conflict, the build succeeded.

I suspect you missed something during your handling of the merge conflict?

Regards,
Bin


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
  2022-04-20  9:52         ` Bin Meng
@ 2022-04-20 22:45           ` Alistair Francis
  -1 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-04-20 22:45 UTC (permalink / raw)
  To: Bin Meng
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Wed, Apr 20, 2022 at 7:52 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Alistair,
>
> On Wed, Apr 20, 2022 at 3:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Wed, Apr 20, 2022 at 3:31 PM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > 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 | 32 ++++++++++++++++++++++++++++++++
> > > >  1 file changed, 32 insertions(+)
> > > >
> > > > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > > > index 5178b3fec9..4921dad09d 100644
> > > > --- a/target/riscv/machine.c
> > > > +++ b/target/riscv/machine.c
> > > > @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
> > > >          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> > > >          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> > > >          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> > > > +        VMSTATE_END_OF_LIST()
> > > > +    }
> > > > +};
> > > > +
> > > > +static bool debug_needed(void *opaque)
> > > > +{
> > > > +    RISCVCPU *cpu = opaque;
> > > > +    CPURISCVState *env = &cpu->env;
> > > > +
> > > > +    return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > >
> > > This fails to build:
> > >
> > > ../target/riscv/machine.c: In function ‘debug_needed’:
> > > ../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
> > > undeclared (first use in this function); did you mean
> > > ‘RISCV_FEATURE_EPMP’?
> > >  228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > >      |                               ^~~~~~~~~~~~~~~~~~~
> > >      |                               RISCV_FEATURE_EPMP
> > > ../target/riscv/machine.c:228:31: note: each undeclared identifier is
> > > reported only once for each function it appears in
> > > ../target/riscv/machine.c:229:1: warning: control reaches end of
> > > non-void function [-Wreturn-type]
> > >  229 | }
> > >      | ^
> >
> > That's weird. Maybe it's out of sync or merge conflict? I will take a look.
> >
>
> I rebased the v4 series on top of your riscv-to-apply.next branch,
> indeed there is a merge conflict of target/riscv/machine.c. After I
> resolved the conflict, the build succeeded.

Looking at this patch series RISCV_FEATURE_DEBUG is only defined in
patch 4, it doesn't currently exist in the tree. I'm not sure how this
can build.

Are you sure you looked at just this patch and not the entire series?

>
> I suspect you missed something during your handling of the merge conflict?

That's entirely possible. Can you send a rebased version please

Alistair

>
> Regards,
> Bin


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
@ 2022-04-20 22:45           ` Alistair Francis
  0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-04-20 22:45 UTC (permalink / raw)
  To: Bin Meng
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Bin Meng

On Wed, Apr 20, 2022 at 7:52 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Alistair,
>
> On Wed, Apr 20, 2022 at 3:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Wed, Apr 20, 2022 at 3:31 PM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > 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 | 32 ++++++++++++++++++++++++++++++++
> > > >  1 file changed, 32 insertions(+)
> > > >
> > > > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > > > index 5178b3fec9..4921dad09d 100644
> > > > --- a/target/riscv/machine.c
> > > > +++ b/target/riscv/machine.c
> > > > @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
> > > >          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> > > >          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> > > >          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> > > > +        VMSTATE_END_OF_LIST()
> > > > +    }
> > > > +};
> > > > +
> > > > +static bool debug_needed(void *opaque)
> > > > +{
> > > > +    RISCVCPU *cpu = opaque;
> > > > +    CPURISCVState *env = &cpu->env;
> > > > +
> > > > +    return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > >
> > > This fails to build:
> > >
> > > ../target/riscv/machine.c: In function ‘debug_needed’:
> > > ../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
> > > undeclared (first use in this function); did you mean
> > > ‘RISCV_FEATURE_EPMP’?
> > >  228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > >      |                               ^~~~~~~~~~~~~~~~~~~
> > >      |                               RISCV_FEATURE_EPMP
> > > ../target/riscv/machine.c:228:31: note: each undeclared identifier is
> > > reported only once for each function it appears in
> > > ../target/riscv/machine.c:229:1: warning: control reaches end of
> > > non-void function [-Wreturn-type]
> > >  229 | }
> > >      | ^
> >
> > That's weird. Maybe it's out of sync or merge conflict? I will take a look.
> >
>
> I rebased the v4 series on top of your riscv-to-apply.next branch,
> indeed there is a merge conflict of target/riscv/machine.c. After I
> resolved the conflict, the build succeeded.

Looking at this patch series RISCV_FEATURE_DEBUG is only defined in
patch 4, it doesn't currently exist in the tree. I'm not sure how this
can build.

Are you sure you looked at just this patch and not the entire series?

>
> I suspect you missed something during your handling of the merge conflict?

That's entirely possible. Can you send a rebased version please

Alistair

>
> Regards,
> Bin


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
  2022-04-20 22:45           ` Alistair Francis
@ 2022-04-20 23:46             ` Bin Meng
  -1 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-04-20 23:46 UTC (permalink / raw)
  To: Alistair Francis
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

Hi Alistair,

On Thu, Apr 21, 2022 at 6:45 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Apr 20, 2022 at 7:52 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Alistair,
> >
> > On Wed, Apr 20, 2022 at 3:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > On Wed, Apr 20, 2022 at 3:31 PM Alistair Francis <alistair23@gmail.com> wrote:
> > > >
> > > > On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > 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 | 32 ++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 32 insertions(+)
> > > > >
> > > > > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > > > > index 5178b3fec9..4921dad09d 100644
> > > > > --- a/target/riscv/machine.c
> > > > > +++ b/target/riscv/machine.c
> > > > > @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
> > > > >          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> > > > >          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> > > > >          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> > > > > +        VMSTATE_END_OF_LIST()
> > > > > +    }
> > > > > +};
> > > > > +
> > > > > +static bool debug_needed(void *opaque)
> > > > > +{
> > > > > +    RISCVCPU *cpu = opaque;
> > > > > +    CPURISCVState *env = &cpu->env;
> > > > > +
> > > > > +    return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > >
> > > > This fails to build:
> > > >
> > > > ../target/riscv/machine.c: In function ‘debug_needed’:
> > > > ../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
> > > > undeclared (first use in this function); did you mean
> > > > ‘RISCV_FEATURE_EPMP’?
> > > >  228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > >      |                               ^~~~~~~~~~~~~~~~~~~
> > > >      |                               RISCV_FEATURE_EPMP
> > > > ../target/riscv/machine.c:228:31: note: each undeclared identifier is
> > > > reported only once for each function it appears in
> > > > ../target/riscv/machine.c:229:1: warning: control reaches end of
> > > > non-void function [-Wreturn-type]
> > > >  229 | }
> > > >      | ^
> > >
> > > That's weird. Maybe it's out of sync or merge conflict? I will take a look.
> > >
> >
> > I rebased the v4 series on top of your riscv-to-apply.next branch,
> > indeed there is a merge conflict of target/riscv/machine.c. After I
> > resolved the conflict, the build succeeded.
>
> Looking at this patch series RISCV_FEATURE_DEBUG is only defined in
> patch 4, it doesn't currently exist in the tree. I'm not sure how this
> can build.

Ah, it looks like I should adjust the patch order to have patch 4 come first.

>
> Are you sure you looked at just this patch and not the entire series?

I see. I was looking at the series not this patch.

It seems you were trying to build every commit for bisectabliity? Is
there an easy way to do such automatically?

>
> >
> > I suspect you missed something during your handling of the merge conflict?
>
> That's entirely possible. Can you send a rebased version please

Regards,
Bin


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
@ 2022-04-20 23:46             ` Bin Meng
  0 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-04-20 23:46 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Bin Meng

Hi Alistair,

On Thu, Apr 21, 2022 at 6:45 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Wed, Apr 20, 2022 at 7:52 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Alistair,
> >
> > On Wed, Apr 20, 2022 at 3:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > On Wed, Apr 20, 2022 at 3:31 PM Alistair Francis <alistair23@gmail.com> wrote:
> > > >
> > > > On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > 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 | 32 ++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 32 insertions(+)
> > > > >
> > > > > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > > > > index 5178b3fec9..4921dad09d 100644
> > > > > --- a/target/riscv/machine.c
> > > > > +++ b/target/riscv/machine.c
> > > > > @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
> > > > >          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> > > > >          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> > > > >          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> > > > > +        VMSTATE_END_OF_LIST()
> > > > > +    }
> > > > > +};
> > > > > +
> > > > > +static bool debug_needed(void *opaque)
> > > > > +{
> > > > > +    RISCVCPU *cpu = opaque;
> > > > > +    CPURISCVState *env = &cpu->env;
> > > > > +
> > > > > +    return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > >
> > > > This fails to build:
> > > >
> > > > ../target/riscv/machine.c: In function ‘debug_needed’:
> > > > ../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
> > > > undeclared (first use in this function); did you mean
> > > > ‘RISCV_FEATURE_EPMP’?
> > > >  228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > >      |                               ^~~~~~~~~~~~~~~~~~~
> > > >      |                               RISCV_FEATURE_EPMP
> > > > ../target/riscv/machine.c:228:31: note: each undeclared identifier is
> > > > reported only once for each function it appears in
> > > > ../target/riscv/machine.c:229:1: warning: control reaches end of
> > > > non-void function [-Wreturn-type]
> > > >  229 | }
> > > >      | ^
> > >
> > > That's weird. Maybe it's out of sync or merge conflict? I will take a look.
> > >
> >
> > I rebased the v4 series on top of your riscv-to-apply.next branch,
> > indeed there is a merge conflict of target/riscv/machine.c. After I
> > resolved the conflict, the build succeeded.
>
> Looking at this patch series RISCV_FEATURE_DEBUG is only defined in
> patch 4, it doesn't currently exist in the tree. I'm not sure how this
> can build.

Ah, it looks like I should adjust the patch order to have patch 4 come first.

>
> Are you sure you looked at just this patch and not the entire series?

I see. I was looking at the series not this patch.

It seems you were trying to build every commit for bisectabliity? Is
there an easy way to do such automatically?

>
> >
> > I suspect you missed something during your handling of the merge conflict?
>
> That's entirely possible. Can you send a rebased version please

Regards,
Bin


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
  2022-04-20 23:46             ` Bin Meng
@ 2022-04-21  0:13               ` Alistair Francis
  -1 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-04-21  0:13 UTC (permalink / raw)
  To: Bin Meng
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Thu, Apr 21, 2022 at 9:47 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Alistair,
>
> On Thu, Apr 21, 2022 at 6:45 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Wed, Apr 20, 2022 at 7:52 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Alistair,
> > >
> > > On Wed, Apr 20, 2022 at 3:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > On Wed, Apr 20, 2022 at 3:31 PM Alistair Francis <alistair23@gmail.com> wrote:
> > > > >
> > > > > On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > >
> > > > > > 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 | 32 ++++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 32 insertions(+)
> > > > > >
> > > > > > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > > > > > index 5178b3fec9..4921dad09d 100644
> > > > > > --- a/target/riscv/machine.c
> > > > > > +++ b/target/riscv/machine.c
> > > > > > @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
> > > > > >          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> > > > > >          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> > > > > >          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> > > > > > +        VMSTATE_END_OF_LIST()
> > > > > > +    }
> > > > > > +};
> > > > > > +
> > > > > > +static bool debug_needed(void *opaque)
> > > > > > +{
> > > > > > +    RISCVCPU *cpu = opaque;
> > > > > > +    CPURISCVState *env = &cpu->env;
> > > > > > +
> > > > > > +    return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > > >
> > > > > This fails to build:
> > > > >
> > > > > ../target/riscv/machine.c: In function ‘debug_needed’:
> > > > > ../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
> > > > > undeclared (first use in this function); did you mean
> > > > > ‘RISCV_FEATURE_EPMP’?
> > > > >  228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > > >      |                               ^~~~~~~~~~~~~~~~~~~
> > > > >      |                               RISCV_FEATURE_EPMP
> > > > > ../target/riscv/machine.c:228:31: note: each undeclared identifier is
> > > > > reported only once for each function it appears in
> > > > > ../target/riscv/machine.c:229:1: warning: control reaches end of
> > > > > non-void function [-Wreturn-type]
> > > > >  229 | }
> > > > >      | ^
> > > >
> > > > That's weird. Maybe it's out of sync or merge conflict? I will take a look.
> > > >
> > >
> > > I rebased the v4 series on top of your riscv-to-apply.next branch,
> > > indeed there is a merge conflict of target/riscv/machine.c. After I
> > > resolved the conflict, the build succeeded.
> >
> > Looking at this patch series RISCV_FEATURE_DEBUG is only defined in
> > patch 4, it doesn't currently exist in the tree. I'm not sure how this
> > can build.
>
> Ah, it looks like I should adjust the patch order to have patch 4 come first.
>
> >
> > Are you sure you looked at just this patch and not the entire series?
>
> I see. I was looking at the series not this patch.
>
> It seems you were trying to build every commit for bisectabliity? Is
> there an easy way to do such automatically?

Yep, I build test every patch.

I do this automatically with an internal Jenkins server, unfortunately
I can't really share it publically

Alistair

>
> >
> > >
> > > I suspect you missed something during your handling of the merge conflict?
> >
> > That's entirely possible. Can you send a rebased version please
>
> Regards,
> Bin


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
@ 2022-04-21  0:13               ` Alistair Francis
  0 siblings, 0 replies; 35+ messages in thread
From: Alistair Francis @ 2022-04-21  0:13 UTC (permalink / raw)
  To: Bin Meng
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Bin Meng

On Thu, Apr 21, 2022 at 9:47 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Alistair,
>
> On Thu, Apr 21, 2022 at 6:45 AM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Wed, Apr 20, 2022 at 7:52 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Alistair,
> > >
> > > On Wed, Apr 20, 2022 at 3:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > On Wed, Apr 20, 2022 at 3:31 PM Alistair Francis <alistair23@gmail.com> wrote:
> > > > >
> > > > > On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > >
> > > > > > 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 | 32 ++++++++++++++++++++++++++++++++
> > > > > >  1 file changed, 32 insertions(+)
> > > > > >
> > > > > > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > > > > > index 5178b3fec9..4921dad09d 100644
> > > > > > --- a/target/riscv/machine.c
> > > > > > +++ b/target/riscv/machine.c
> > > > > > @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
> > > > > >          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> > > > > >          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> > > > > >          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> > > > > > +        VMSTATE_END_OF_LIST()
> > > > > > +    }
> > > > > > +};
> > > > > > +
> > > > > > +static bool debug_needed(void *opaque)
> > > > > > +{
> > > > > > +    RISCVCPU *cpu = opaque;
> > > > > > +    CPURISCVState *env = &cpu->env;
> > > > > > +
> > > > > > +    return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > > >
> > > > > This fails to build:
> > > > >
> > > > > ../target/riscv/machine.c: In function ‘debug_needed’:
> > > > > ../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
> > > > > undeclared (first use in this function); did you mean
> > > > > ‘RISCV_FEATURE_EPMP’?
> > > > >  228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > > >      |                               ^~~~~~~~~~~~~~~~~~~
> > > > >      |                               RISCV_FEATURE_EPMP
> > > > > ../target/riscv/machine.c:228:31: note: each undeclared identifier is
> > > > > reported only once for each function it appears in
> > > > > ../target/riscv/machine.c:229:1: warning: control reaches end of
> > > > > non-void function [-Wreturn-type]
> > > > >  229 | }
> > > > >      | ^
> > > >
> > > > That's weird. Maybe it's out of sync or merge conflict? I will take a look.
> > > >
> > >
> > > I rebased the v4 series on top of your riscv-to-apply.next branch,
> > > indeed there is a merge conflict of target/riscv/machine.c. After I
> > > resolved the conflict, the build succeeded.
> >
> > Looking at this patch series RISCV_FEATURE_DEBUG is only defined in
> > patch 4, it doesn't currently exist in the tree. I'm not sure how this
> > can build.
>
> Ah, it looks like I should adjust the patch order to have patch 4 come first.
>
> >
> > Are you sure you looked at just this patch and not the entire series?
>
> I see. I was looking at the series not this patch.
>
> It seems you were trying to build every commit for bisectabliity? Is
> there an easy way to do such automatically?

Yep, I build test every patch.

I do this automatically with an internal Jenkins server, unfortunately
I can't really share it publically

Alistair

>
> >
> > >
> > > I suspect you missed something during your handling of the merge conflict?
> >
> > That's entirely possible. Can you send a rebased version please
>
> Regards,
> Bin


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
  2022-04-21  0:13               ` Alistair Francis
@ 2022-04-21  0:19                 ` Bin Meng
  -1 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-04-21  0:19 UTC (permalink / raw)
  To: Alistair Francis
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Thu, Apr 21, 2022 at 8:14 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Thu, Apr 21, 2022 at 9:47 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Alistair,
> >
> > On Thu, Apr 21, 2022 at 6:45 AM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > On Wed, Apr 20, 2022 at 7:52 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > Hi Alistair,
> > > >
> > > > On Wed, Apr 20, 2022 at 3:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > On Wed, Apr 20, 2022 at 3:31 PM Alistair Francis <alistair23@gmail.com> wrote:
> > > > > >
> > > > > > On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > >
> > > > > > > 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 | 32 ++++++++++++++++++++++++++++++++
> > > > > > >  1 file changed, 32 insertions(+)
> > > > > > >
> > > > > > > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > > > > > > index 5178b3fec9..4921dad09d 100644
> > > > > > > --- a/target/riscv/machine.c
> > > > > > > +++ b/target/riscv/machine.c
> > > > > > > @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
> > > > > > >          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> > > > > > >          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> > > > > > >          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> > > > > > > +        VMSTATE_END_OF_LIST()
> > > > > > > +    }
> > > > > > > +};
> > > > > > > +
> > > > > > > +static bool debug_needed(void *opaque)
> > > > > > > +{
> > > > > > > +    RISCVCPU *cpu = opaque;
> > > > > > > +    CPURISCVState *env = &cpu->env;
> > > > > > > +
> > > > > > > +    return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > > > >
> > > > > > This fails to build:
> > > > > >
> > > > > > ../target/riscv/machine.c: In function ‘debug_needed’:
> > > > > > ../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
> > > > > > undeclared (first use in this function); did you mean
> > > > > > ‘RISCV_FEATURE_EPMP’?
> > > > > >  228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > > > >      |                               ^~~~~~~~~~~~~~~~~~~
> > > > > >      |                               RISCV_FEATURE_EPMP
> > > > > > ../target/riscv/machine.c:228:31: note: each undeclared identifier is
> > > > > > reported only once for each function it appears in
> > > > > > ../target/riscv/machine.c:229:1: warning: control reaches end of
> > > > > > non-void function [-Wreturn-type]
> > > > > >  229 | }
> > > > > >      | ^
> > > > >
> > > > > That's weird. Maybe it's out of sync or merge conflict? I will take a look.
> > > > >
> > > >
> > > > I rebased the v4 series on top of your riscv-to-apply.next branch,
> > > > indeed there is a merge conflict of target/riscv/machine.c. After I
> > > > resolved the conflict, the build succeeded.
> > >
> > > Looking at this patch series RISCV_FEATURE_DEBUG is only defined in
> > > patch 4, it doesn't currently exist in the tree. I'm not sure how this
> > > can build.
> >
> > Ah, it looks like I should adjust the patch order to have patch 4 come first.
> >
> > >
> > > Are you sure you looked at just this patch and not the entire series?
> >
> > I see. I was looking at the series not this patch.
> >
> > It seems you were trying to build every commit for bisectabliity? Is
> > there an easy way to do such automatically?
>
> Yep, I build test every patch.
>
> I do this automatically with an internal Jenkins server, unfortunately
> I can't really share it publically
>

Okay, I will send a rebased version, plus fixing the patch order.

Regards,
Bin


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
@ 2022-04-21  0:19                 ` Bin Meng
  0 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-04-21  0:19 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Bin Meng

On Thu, Apr 21, 2022 at 8:14 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Thu, Apr 21, 2022 at 9:47 AM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Alistair,
> >
> > On Thu, Apr 21, 2022 at 6:45 AM Alistair Francis <alistair23@gmail.com> wrote:
> > >
> > > On Wed, Apr 20, 2022 at 7:52 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > >
> > > > Hi Alistair,
> > > >
> > > > On Wed, Apr 20, 2022 at 3:33 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > >
> > > > > On Wed, Apr 20, 2022 at 3:31 PM Alistair Francis <alistair23@gmail.com> wrote:
> > > > > >
> > > > > > On Tue, Mar 15, 2022 at 5:17 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> > > > > > >
> > > > > > > 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 | 32 ++++++++++++++++++++++++++++++++
> > > > > > >  1 file changed, 32 insertions(+)
> > > > > > >
> > > > > > > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > > > > > > index 5178b3fec9..4921dad09d 100644
> > > > > > > --- a/target/riscv/machine.c
> > > > > > > +++ b/target/riscv/machine.c
> > > > > > > @@ -216,7 +216,38 @@ static const VMStateDescription vmstate_kvmtimer = {
> > > > > > >          VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
> > > > > > >          VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
> > > > > > >          VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
> > > > > > > +        VMSTATE_END_OF_LIST()
> > > > > > > +    }
> > > > > > > +};
> > > > > > > +
> > > > > > > +static bool debug_needed(void *opaque)
> > > > > > > +{
> > > > > > > +    RISCVCPU *cpu = opaque;
> > > > > > > +    CPURISCVState *env = &cpu->env;
> > > > > > > +
> > > > > > > +    return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > > > >
> > > > > > This fails to build:
> > > > > >
> > > > > > ../target/riscv/machine.c: In function ‘debug_needed’:
> > > > > > ../target/riscv/machine.c:228:31: error: ‘RISCV_FEATURE_DEBUG’
> > > > > > undeclared (first use in this function); did you mean
> > > > > > ‘RISCV_FEATURE_EPMP’?
> > > > > >  228 |     return riscv_feature(env, RISCV_FEATURE_DEBUG);
> > > > > >      |                               ^~~~~~~~~~~~~~~~~~~
> > > > > >      |                               RISCV_FEATURE_EPMP
> > > > > > ../target/riscv/machine.c:228:31: note: each undeclared identifier is
> > > > > > reported only once for each function it appears in
> > > > > > ../target/riscv/machine.c:229:1: warning: control reaches end of
> > > > > > non-void function [-Wreturn-type]
> > > > > >  229 | }
> > > > > >      | ^
> > > > >
> > > > > That's weird. Maybe it's out of sync or merge conflict? I will take a look.
> > > > >
> > > >
> > > > I rebased the v4 series on top of your riscv-to-apply.next branch,
> > > > indeed there is a merge conflict of target/riscv/machine.c. After I
> > > > resolved the conflict, the build succeeded.
> > >
> > > Looking at this patch series RISCV_FEATURE_DEBUG is only defined in
> > > patch 4, it doesn't currently exist in the tree. I'm not sure how this
> > > can build.
> >
> > Ah, it looks like I should adjust the patch order to have patch 4 come first.
> >
> > >
> > > Are you sure you looked at just this patch and not the entire series?
> >
> > I see. I was looking at the series not this patch.
> >
> > It seems you were trying to build every commit for bisectabliity? Is
> > there an easy way to do such automatically?
>
> Yep, I build test every patch.
>
> I do this automatically with an internal Jenkins server, unfortunately
> I can't really share it publically
>

Okay, I will send a rebased version, plus fixing the patch order.

Regards,
Bin


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
  2022-04-20 23:46             ` Bin Meng
@ 2022-04-21 15:51               ` Richard Henderson
  -1 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2022-04-21 15:51 UTC (permalink / raw)
  To: Bin Meng, Alistair Francis
  Cc: Alistair Francis, Bin Meng, open list:RISC-V,
	qemu-devel@nongnu.org Developers

On 4/20/22 16:46, Bin Meng wrote:
> It seems you were trying to build every commit for bisectabliity? Is
> there an easy way to do such automatically?

git rebase --exec "cd build && make"


r~


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
@ 2022-04-21 15:51               ` Richard Henderson
  0 siblings, 0 replies; 35+ messages in thread
From: Richard Henderson @ 2022-04-21 15:51 UTC (permalink / raw)
  To: Bin Meng, Alistair Francis
  Cc: open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On 4/20/22 16:46, Bin Meng wrote:
> It seems you were trying to build every commit for bisectabliity? Is
> there an easy way to do such automatically?

git rebase --exec "cd build && make"


r~


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
  2022-04-21 15:51               ` Richard Henderson
@ 2022-04-22  1:22                 ` Bin Meng
  -1 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-04-22  1:22 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Alistair Francis, Alistair Francis, Bin Meng, open list:RISC-V,
	qemu-devel@nongnu.org Developers

On Thu, Apr 21, 2022 at 11:51 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/20/22 16:46, Bin Meng wrote:
> > It seems you were trying to build every commit for bisectabliity? Is
> > there an easy way to do such automatically?
>
> git rebase --exec "cd build && make"
>

This works! Thanks Richard.

Regards,
Bin


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

* Re: [PATCH v4 2/7] target/riscv: machine: Add debug state description
@ 2022-04-22  1:22                 ` Bin Meng
  0 siblings, 0 replies; 35+ messages in thread
From: Bin Meng @ 2022-04-22  1:22 UTC (permalink / raw)
  To: Richard Henderson
  Cc: Alistair Francis, open list:RISC-V, Bin Meng, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Thu, Apr 21, 2022 at 11:51 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/20/22 16:46, Bin Meng wrote:
> > It seems you were trying to build every commit for bisectabliity? Is
> > there an easy way to do such automatically?
>
> git rebase --exec "cd build && make"
>

This works! Thanks Richard.

Regards,
Bin


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

end of thread, other threads:[~2022-04-22  1:24 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-15  6:55 [PATCH v4 0/7] target/riscv: Initial support for the Sdtrig extension via M-mode CSRs Bin Meng
2022-03-15  6:55 ` [PATCH v4 1/7] target/riscv: Add initial support for the Sdtrig extension Bin Meng
2022-03-18  2:11   ` Alistair Francis
2022-03-18  2:11     ` Alistair Francis
2022-03-15  6:55 ` [PATCH v4 2/7] target/riscv: machine: Add debug state description Bin Meng
2022-04-20  7:30   ` Alistair Francis
2022-04-20  7:30     ` Alistair Francis
2022-04-20  7:33     ` Bin Meng
2022-04-20  7:33       ` Bin Meng
2022-04-20  9:52       ` Bin Meng
2022-04-20  9:52         ` Bin Meng
2022-04-20 22:45         ` Alistair Francis
2022-04-20 22:45           ` Alistair Francis
2022-04-20 23:46           ` Bin Meng
2022-04-20 23:46             ` Bin Meng
2022-04-21  0:13             ` Alistair Francis
2022-04-21  0:13               ` Alistair Francis
2022-04-21  0:19               ` Bin Meng
2022-04-21  0:19                 ` Bin Meng
2022-04-21 15:51             ` Richard Henderson
2022-04-21 15:51               ` Richard Henderson
2022-04-22  1:22               ` Bin Meng
2022-04-22  1:22                 ` Bin Meng
2022-03-15  6:55 ` [PATCH v4 3/7] target/riscv: debug: Implement debug related TCGCPUOps Bin Meng
2022-03-15  6:55 ` [PATCH v4 4/7] target/riscv: cpu: Add a config option for native debug Bin Meng
2022-03-15  6:55 ` [PATCH v4 5/7] target/riscv: csr: Hook debug CSR read/write Bin Meng
2022-03-18  2:14   ` Alistair Francis
2022-03-18  2:14     ` Alistair Francis
2022-03-15  6:55 ` [PATCH v4 6/7] target/riscv: cpu: Enable native debug feature Bin Meng
2022-03-18  2:17   ` Alistair Francis
2022-03-18  2:17     ` Alistair Francis
2022-03-15  6:55 ` [PATCH v4 7/7] hw/core: tcg-cpu-ops.h: Update comments of debug_check_watchpoint() Bin Meng
2022-03-15  6:55   ` Bin Meng
2022-03-18  7:38 ` [PATCH v4 0/7] target/riscv: Initial support for the Sdtrig extension via M-mode CSRs Alistair Francis
2022-03-18  7:38   ` Alistair Francis

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.