* [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support
@ 2022-09-09 13:42 Bin Meng
2022-09-09 13:42 ` [PATCH v2 1/8] target/riscv: debug: Determine the trigger type from tdata1.type Bin Meng
` (8 more replies)
0 siblings, 9 replies; 17+ messages in thread
From: Bin Meng @ 2022-09-09 13:42 UTC (permalink / raw)
To: qemu-devel; +Cc: Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
This patchset refactors RISC-V Debug support to allow more types of
triggers to be extended.
The initial support of type 6 trigger, which is similar to type 2
trigger with additional functionality, is also introduced in this
patchset.
This is a v2 respin of previous patch originally done by Frank Chang
at SiFive. I've incorperated my review comments in v2 and rebased
against QEMU mainline.
Changes in v2:
- fixed MXL_RV128 case
- moved macros to patch#2
- added log guest errors for TRIGGER_TYPE_{NO_EXIST,UNAVAIL}
- moved RV{32,64}_DATA_MASK definition to this patch
- add handling of the DBG_ACTION_NONE case in do_trigger_action()
- drop patch: "target/riscv: debug: Return 0 if previous value written to tselect >= number of triggers"
Frank Chang (8):
target/riscv: debug: Determine the trigger type from tdata1.type
target/riscv: debug: Introduce build_tdata1() to build tdata1 register
content
target/riscv: debug: Introduce tdata1, tdata2, and tdata3 CSRs
target/riscv: debug: Restrict the range of tselect value can be
written
target/riscv: debug: Introduce tinfo CSR
target/riscv: debug: Create common trigger actions function
target/riscv: debug: Check VU/VS modes for type 2 trigger
target/riscv: debug: Add initial support of type 6 trigger
target/riscv/cpu.h | 6 +-
target/riscv/cpu_bits.h | 1 +
target/riscv/debug.h | 55 +++--
target/riscv/csr.c | 10 +-
target/riscv/debug.c | 484 ++++++++++++++++++++++++++++++++--------
target/riscv/machine.c | 20 +-
6 files changed, 445 insertions(+), 131 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 1/8] target/riscv: debug: Determine the trigger type from tdata1.type
2022-09-09 13:42 [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support Bin Meng
@ 2022-09-09 13:42 ` Bin Meng
2022-09-16 1:53 ` LIU Zhiwei
2022-09-16 2:42 ` LIU Zhiwei
2022-09-09 13:42 ` [PATCH v2 2/8] target/riscv: debug: Introduce build_tdata1() to build tdata1 register content Bin Meng
` (7 subsequent siblings)
8 siblings, 2 replies; 17+ messages in thread
From: Bin Meng @ 2022-09-09 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
From: Frank Chang <frank.chang@sifive.com>
Current RISC-V debug assumes that only type 2 trigger is supported.
To allow more types of triggers to be supported in the future
(e.g. type 6 trigger, which is similar to type 2 trigger with additional
functionality), we should determine the trigger type from tdata1.type.
RV_MAX_TRIGGERS is also introduced in replacement of TRIGGER_TYPE2_NUM.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
[bmeng: fixed MXL_RV128 case, and moved macros to the following patch]
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---
Changes in v2:
- fixed MXL_RV128 case
- moved macros to patch#2
- added log guest errors for TRIGGER_TYPE_{NO_EXIST,UNAVAIL}
target/riscv/cpu.h | 2 +-
target/riscv/debug.h | 13 +--
target/riscv/csr.c | 2 +-
target/riscv/debug.c | 188 +++++++++++++++++++++++++++++------------
target/riscv/machine.c | 2 +-
5 files changed, 140 insertions(+), 67 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 06751e1e3e..4d82a3250b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -324,7 +324,7 @@ struct CPUArchState {
/* trigger module */
target_ulong trigger_cur;
- type2_trigger_t type2_trig[TRIGGER_TYPE2_NUM];
+ type2_trigger_t type2_trig[RV_MAX_TRIGGERS];
/* machine specific rdtime callback */
uint64_t (*rdtime_fn)(void *);
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index 27b9cac6b4..72e4edcd8c 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -22,13 +22,7 @@
#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
-};
+#define RV_MAX_TRIGGERS 2
/* register index of tdata CSRs */
enum {
@@ -46,7 +40,8 @@ typedef enum {
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_UNAVAIL = 15, /* trigger exists, but unavailable */
+ TRIGGER_TYPE_NUM
} trigger_type_t;
typedef struct {
@@ -56,7 +51,7 @@ typedef struct {
struct CPUWatchpoint *wp;
} type2_trigger_t;
-/* tdata field masks */
+/* tdata1 field masks */
#define RV32_TYPE(t) ((uint32_t)(t) << 28)
#define RV32_TYPE_MASK (0xf << 28)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index b96db1b62b..3d0d8e0340 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3065,7 +3065,7 @@ 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) {
+ if (env->trigger_cur >= RV_MAX_TRIGGERS && csrno == CSR_TDATA1) {
*val = 0;
return RISCV_EXCP_NONE;
}
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index fc6e13222f..9dd468753a 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -52,8 +52,15 @@
/* 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 },
+static tdata_avail tdata_mapping[TRIGGER_TYPE_NUM] = {
+ [TRIGGER_TYPE_NO_EXIST] = { false, false, false },
+ [TRIGGER_TYPE_AD_MATCH] = { true, true, true },
+ [TRIGGER_TYPE_INST_CNT] = { true, false, true },
+ [TRIGGER_TYPE_INT] = { true, true, true },
+ [TRIGGER_TYPE_EXCP] = { true, true, true },
+ [TRIGGER_TYPE_AD_MATCH6] = { true, true, true },
+ [TRIGGER_TYPE_EXT_SRC] = { true, false, false },
+ [TRIGGER_TYPE_UNAVAIL] = { true, true, true }
};
/* only breakpoint size 1/2/4/8 supported */
@@ -67,6 +74,27 @@ static int access_size[SIZE_NUM] = {
[6 ... 15] = -1,
};
+static inline target_ulong extract_trigger_type(CPURISCVState *env,
+ target_ulong tdata1)
+{
+ switch (riscv_cpu_mxl(env)) {
+ case MXL_RV32:
+ return extract32(tdata1, 28, 4);
+ case MXL_RV64:
+ case MXL_RV128:
+ return extract64(tdata1, 60, 4);
+ default:
+ g_assert_not_reached();
+ }
+}
+
+static inline target_ulong get_trigger_type(CPURISCVState *env,
+ target_ulong trigger_index)
+{
+ target_ulong tdata1 = env->type2_trig[trigger_index].mcontrol;
+ return extract_trigger_type(env, tdata1);
+}
+
static inline target_ulong trigger_type(CPURISCVState *env,
trigger_type_t type)
{
@@ -89,15 +117,17 @@ static inline target_ulong trigger_type(CPURISCVState *env,
bool tdata_available(CPURISCVState *env, int tdata_index)
{
+ int trigger_type = get_trigger_type(env, env->trigger_cur);
+
if (unlikely(tdata_index >= TDATA_NUM)) {
return false;
}
- if (unlikely(env->trigger_cur >= TRIGGER_NUM)) {
+ if (unlikely(env->trigger_cur >= RV_MAX_TRIGGERS)) {
return false;
}
- return tdata_mapping[env->trigger_cur][tdata_index];
+ return tdata_mapping[trigger_type][tdata_index];
}
target_ulong tselect_csr_read(CPURISCVState *env)
@@ -137,6 +167,7 @@ static target_ulong tdata1_validate(CPURISCVState *env, target_ulong val,
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");
}
@@ -261,9 +292,8 @@ static void type2_breakpoint_remove(CPURISCVState *env, target_ulong index)
}
static target_ulong type2_reg_read(CPURISCVState *env,
- target_ulong trigger_index, int tdata_index)
+ target_ulong index, int tdata_index)
{
- uint32_t index = trigger_index - TRIGGER_TYPE2_IDX_0;
target_ulong tdata;
switch (tdata_index) {
@@ -280,10 +310,9 @@ static target_ulong type2_reg_read(CPURISCVState *env,
return tdata;
}
-static void type2_reg_write(CPURISCVState *env, target_ulong trigger_index,
+static void type2_reg_write(CPURISCVState *env, target_ulong index,
int tdata_index, target_ulong val)
{
- uint32_t index = trigger_index - TRIGGER_TYPE2_IDX_0;
target_ulong new_val;
switch (tdata_index) {
@@ -309,35 +338,64 @@ static void type2_reg_write(CPURISCVState *env, target_ulong trigger_index,
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];
+ int trigger_type = get_trigger_type(env, env->trigger_cur);
+
+ switch (trigger_type) {
+ case TRIGGER_TYPE_AD_MATCH:
+ return type2_reg_read(env, env->trigger_cur, tdata_index);
+ break;
+ case TRIGGER_TYPE_INST_CNT:
+ case TRIGGER_TYPE_INT:
+ case TRIGGER_TYPE_EXCP:
+ case TRIGGER_TYPE_AD_MATCH6:
+ case TRIGGER_TYPE_EXT_SRC:
+ qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
+ trigger_type);
+ break;
+ case TRIGGER_TYPE_NO_EXIST:
+ case TRIGGER_TYPE_UNAVAIL:
+ qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
+ trigger_type);
+ break;
+ default:
+ g_assert_not_reached();
+ }
- return read_func(env, env->trigger_cur, tdata_index);
+ return 0;
}
void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
{
- tdata_write_func write_func = trigger_write_funcs[env->trigger_cur];
+ int trigger_type;
- return write_func(env, env->trigger_cur, tdata_index, val);
+ if (tdata_index == TDATA1) {
+ trigger_type = extract_trigger_type(env, val);
+ } else {
+ trigger_type = get_trigger_type(env, env->trigger_cur);
+ }
+
+ switch (trigger_type) {
+ case TRIGGER_TYPE_AD_MATCH:
+ type2_reg_write(env, env->trigger_cur, tdata_index, val);
+ break;
+ case TRIGGER_TYPE_INST_CNT:
+ case TRIGGER_TYPE_INT:
+ case TRIGGER_TYPE_EXCP:
+ case TRIGGER_TYPE_AD_MATCH6:
+ case TRIGGER_TYPE_EXT_SRC:
+ qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
+ trigger_type);
+ break;
+ case TRIGGER_TYPE_NO_EXIST:
+ case TRIGGER_TYPE_UNAVAIL:
+ qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
+ trigger_type);
+ break;
+ default:
+ g_assert_not_reached();
+ }
}
void riscv_cpu_debug_excp_handler(CPUState *cs)
@@ -364,18 +422,28 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
CPUBreakpoint *bp;
target_ulong ctrl;
target_ulong pc;
+ int trigger_type;
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;
+ for (i = 0; i < RV_MAX_TRIGGERS; i++) {
+ trigger_type = get_trigger_type(env, i);
+
+ switch (trigger_type) {
+ case TRIGGER_TYPE_AD_MATCH:
+ 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;
+ }
}
+ break;
+ default:
+ /* other trigger types are not supported or irrelevant */
+ break;
}
}
}
@@ -389,26 +457,36 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
CPURISCVState *env = &cpu->env;
target_ulong ctrl;
target_ulong addr;
+ int trigger_type;
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;
+ for (i = 0; i < RV_MAX_TRIGGERS; i++) {
+ trigger_type = get_trigger_type(env, i);
- if (ctrl & TYPE2_LOAD) {
- flags |= BP_MEM_READ;
- }
- if (ctrl & TYPE2_STORE) {
- flags |= BP_MEM_WRITE;
- }
+ switch (trigger_type) {
+ case TRIGGER_TYPE_AD_MATCH:
+ ctrl = env->type2_trig[i].mcontrol;
+ addr = env->type2_trig[i].maddress;
+ flags = 0;
- if ((wp->flags & flags) && (wp->vaddr == addr)) {
- /* check U/S/M bit against current privilege level */
- if ((ctrl >> 3) & BIT(env->priv)) {
- return true;
+ 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;
+ }
}
+ break;
+ default:
+ /* other trigger types are not supported */
+ break;
}
}
@@ -417,11 +495,11 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
void riscv_trigger_init(CPURISCVState *env)
{
- target_ulong type2 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
+ target_ulong tdata1 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
int i;
- /* type 2 triggers */
- for (i = 0; i < TRIGGER_TYPE2_NUM; i++) {
+ /* init to type 2 triggers */
+ for (i = 0; i < RV_MAX_TRIGGERS; i++) {
/*
* type = TRIGGER_TYPE_AD_MATCH
* dmode = 0 (both debug and M-mode can write tdata)
@@ -435,7 +513,7 @@ void riscv_trigger_init(CPURISCVState *env)
* 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].mcontrol = tdata1;
env->type2_trig[i].maddress = 0;
env->type2_trig[i].bp = NULL;
env->type2_trig[i].wp = NULL;
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 41098f6ad0..b8173394a2 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -247,7 +247,7 @@ static const VMStateDescription vmstate_debug = {
.needed = debug_needed,
.fields = (VMStateField[]) {
VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
- VMSTATE_STRUCT_ARRAY(env.type2_trig, RISCVCPU, TRIGGER_TYPE2_NUM,
+ VMSTATE_STRUCT_ARRAY(env.type2_trig, RISCVCPU, RV_MAX_TRIGGERS,
0, vmstate_debug_type2, type2_trigger_t),
VMSTATE_END_OF_LIST()
}
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 2/8] target/riscv: debug: Introduce build_tdata1() to build tdata1 register content
2022-09-09 13:42 [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support Bin Meng
2022-09-09 13:42 ` [PATCH v2 1/8] target/riscv: debug: Determine the trigger type from tdata1.type Bin Meng
@ 2022-09-09 13:42 ` Bin Meng
2022-09-16 1:55 ` LIU Zhiwei
2022-09-09 13:42 ` [PATCH v2 3/8] target/riscv: debug: Introduce tdata1, tdata2, and tdata3 CSRs Bin Meng
` (6 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Bin Meng @ 2022-09-09 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
From: Frank Chang <frank.chang@sifive.com>
Introduce build_tdata1() to build tdata1 register content, which can be
shared among all types of triggers.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
[bmeng: moved RV{32,64}_DATA_MASK definition to this patch]
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---
Changes in v2:
- moved RV{32,64}_DATA_MASK definition to this patch
target/riscv/debug.h | 2 ++
target/riscv/debug.c | 15 ++++++++++-----
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index 72e4edcd8c..c422553c27 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -56,9 +56,11 @@ typedef struct {
#define RV32_TYPE(t) ((uint32_t)(t) << 28)
#define RV32_TYPE_MASK (0xf << 28)
#define RV32_DMODE BIT(27)
+#define RV32_DATA_MASK 0x7ffffff
#define RV64_TYPE(t) ((uint64_t)(t) << 60)
#define RV64_TYPE_MASK (0xfULL << 60)
#define RV64_DMODE BIT_ULL(59)
+#define RV64_DATA_MASK 0x7ffffffffffffff
/* mcontrol field masks */
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 9dd468753a..45aae87ec3 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -95,18 +95,23 @@ static inline target_ulong get_trigger_type(CPURISCVState *env,
return extract_trigger_type(env, tdata1);
}
-static inline target_ulong trigger_type(CPURISCVState *env,
- trigger_type_t type)
+static inline target_ulong build_tdata1(CPURISCVState *env,
+ trigger_type_t type,
+ bool dmode, target_ulong data)
{
target_ulong tdata1;
switch (riscv_cpu_mxl(env)) {
case MXL_RV32:
- tdata1 = RV32_TYPE(type);
+ tdata1 = RV32_TYPE(type) |
+ (dmode ? RV32_DMODE : 0) |
+ (data & RV32_DATA_MASK);
break;
case MXL_RV64:
case MXL_RV128:
- tdata1 = RV64_TYPE(type);
+ tdata1 = RV64_TYPE(type) |
+ (dmode ? RV64_DMODE : 0) |
+ (data & RV64_DATA_MASK);
break;
default:
g_assert_not_reached();
@@ -495,7 +500,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
void riscv_trigger_init(CPURISCVState *env)
{
- target_ulong tdata1 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
+ target_ulong tdata1 = build_tdata1(env, TRIGGER_TYPE_AD_MATCH, 0, 0);
int i;
/* init to type 2 triggers */
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 3/8] target/riscv: debug: Introduce tdata1, tdata2, and tdata3 CSRs
2022-09-09 13:42 [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support Bin Meng
2022-09-09 13:42 ` [PATCH v2 1/8] target/riscv: debug: Determine the trigger type from tdata1.type Bin Meng
2022-09-09 13:42 ` [PATCH v2 2/8] target/riscv: debug: Introduce build_tdata1() to build tdata1 register content Bin Meng
@ 2022-09-09 13:42 ` Bin Meng
2022-09-16 1:58 ` LIU Zhiwei
2022-09-09 13:42 ` [PATCH v2 4/8] target/riscv: debug: Restrict the range of tselect value can be written Bin Meng
` (5 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Bin Meng @ 2022-09-09 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
From: Frank Chang <frank.chang@sifive.com>
Replace type2_trigger_t with the real tdata1, tdata2, and tdata3 CSRs,
which allows us to support more types of triggers in the future.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---
(no changes since v1)
target/riscv/cpu.h | 6 ++-
target/riscv/debug.h | 7 ---
target/riscv/debug.c | 103 +++++++++++++++--------------------------
target/riscv/machine.c | 20 ++------
4 files changed, 48 insertions(+), 88 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 4d82a3250b..b0b05c19ca 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -324,7 +324,11 @@ struct CPUArchState {
/* trigger module */
target_ulong trigger_cur;
- type2_trigger_t type2_trig[RV_MAX_TRIGGERS];
+ target_ulong tdata1[RV_MAX_TRIGGERS];
+ target_ulong tdata2[RV_MAX_TRIGGERS];
+ target_ulong tdata3[RV_MAX_TRIGGERS];
+ struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS];
+ struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS];
/* machine specific rdtime callback */
uint64_t (*rdtime_fn)(void *);
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index c422553c27..76146f373a 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -44,13 +44,6 @@ typedef enum {
TRIGGER_TYPE_NUM
} trigger_type_t;
-typedef struct {
- target_ulong mcontrol;
- target_ulong maddress;
- struct CPUBreakpoint *bp;
- struct CPUWatchpoint *wp;
-} type2_trigger_t;
-
/* tdata1 field masks */
#define RV32_TYPE(t) ((uint32_t)(t) << 28)
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 45aae87ec3..06feef7d67 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -91,8 +91,7 @@ static inline target_ulong extract_trigger_type(CPURISCVState *env,
static inline target_ulong get_trigger_type(CPURISCVState *env,
target_ulong trigger_index)
{
- target_ulong tdata1 = env->type2_trig[trigger_index].mcontrol;
- return extract_trigger_type(env, tdata1);
+ return extract_trigger_type(env, env->tdata1[trigger_index]);
}
static inline target_ulong build_tdata1(CPURISCVState *env,
@@ -188,6 +187,8 @@ static inline void warn_always_zero_bit(target_ulong val, target_ulong mask,
}
}
+/* type 2 trigger */
+
static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
{
uint32_t size, sizelo, sizehi = 0;
@@ -247,8 +248,8 @@ static target_ulong type2_mcontrol_validate(CPURISCVState *env,
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;
+ target_ulong ctrl = env->tdata1[index];
+ target_ulong addr = env->tdata2[index];
bool enabled = type2_breakpoint_enabled(ctrl);
CPUState *cs = env_cpu(env);
int flags = BP_CPU | BP_STOP_BEFORE_ACCESS;
@@ -259,7 +260,7 @@ static void type2_breakpoint_insert(CPURISCVState *env, target_ulong index)
}
if (ctrl & TYPE2_EXEC) {
- cpu_breakpoint_insert(cs, addr, flags, &env->type2_trig[index].bp);
+ cpu_breakpoint_insert(cs, addr, flags, &env->cpu_breakpoint[index]);
}
if (ctrl & TYPE2_LOAD) {
@@ -273,10 +274,10 @@ static void type2_breakpoint_insert(CPURISCVState *env, target_ulong index)
size = type2_breakpoint_size(env, ctrl);
if (size != 0) {
cpu_watchpoint_insert(cs, addr, size, flags,
- &env->type2_trig[index].wp);
+ &env->cpu_watchpoint[index]);
} else {
cpu_watchpoint_insert(cs, addr, 8, flags,
- &env->type2_trig[index].wp);
+ &env->cpu_watchpoint[index]);
}
}
}
@@ -285,36 +286,17 @@ 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->cpu_breakpoint[index]) {
+ cpu_breakpoint_remove_by_ref(cs, env->cpu_breakpoint[index]);
+ env->cpu_breakpoint[index] = NULL;
}
- if (env->type2_trig[index].wp) {
- cpu_watchpoint_remove_by_ref(cs, env->type2_trig[index].wp);
- env->type2_trig[index].wp = NULL;
+ if (env->cpu_watchpoint[index]) {
+ cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
+ env->cpu_watchpoint[index] = NULL;
}
}
-static target_ulong type2_reg_read(CPURISCVState *env,
- target_ulong index, int tdata_index)
-{
- 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 index,
int tdata_index, target_ulong val)
{
@@ -323,19 +305,23 @@ static void type2_reg_write(CPURISCVState *env, target_ulong index,
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;
+ if (new_val != env->tdata1[index]) {
+ env->tdata1[index] = 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;
+ if (val != env->tdata2[index]) {
+ env->tdata2[index] = val;
type2_breakpoint_remove(env, index);
type2_breakpoint_insert(env, index);
}
break;
+ case TDATA3:
+ qemu_log_mask(LOG_UNIMP,
+ "tdata3 is not supported for type 2 trigger\n");
+ break;
default:
g_assert_not_reached();
}
@@ -345,30 +331,16 @@ static void type2_reg_write(CPURISCVState *env, target_ulong index,
target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
{
- int trigger_type = get_trigger_type(env, env->trigger_cur);
-
- switch (trigger_type) {
- case TRIGGER_TYPE_AD_MATCH:
- return type2_reg_read(env, env->trigger_cur, tdata_index);
- break;
- case TRIGGER_TYPE_INST_CNT:
- case TRIGGER_TYPE_INT:
- case TRIGGER_TYPE_EXCP:
- case TRIGGER_TYPE_AD_MATCH6:
- case TRIGGER_TYPE_EXT_SRC:
- qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
- trigger_type);
- break;
- case TRIGGER_TYPE_NO_EXIST:
- case TRIGGER_TYPE_UNAVAIL:
- qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
- trigger_type);
- break;
+ switch (tdata_index) {
+ case TDATA1:
+ return env->tdata1[env->trigger_cur];
+ case TDATA2:
+ return env->tdata2[env->trigger_cur];
+ case TDATA3:
+ return env->tdata3[env->trigger_cur];
default:
g_assert_not_reached();
}
-
- return 0;
}
void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
@@ -436,8 +408,8 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
switch (trigger_type) {
case TRIGGER_TYPE_AD_MATCH:
- ctrl = env->type2_trig[i].mcontrol;
- pc = env->type2_trig[i].maddress;
+ ctrl = env->tdata1[i];
+ pc = env->tdata2[i];
if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
/* check U/S/M bit against current privilege level */
@@ -471,8 +443,8 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
switch (trigger_type) {
case TRIGGER_TYPE_AD_MATCH:
- ctrl = env->type2_trig[i].mcontrol;
- addr = env->type2_trig[i].maddress;
+ ctrl = env->tdata1[i];
+ addr = env->tdata2[i];
flags = 0;
if (ctrl & TYPE2_LOAD) {
@@ -518,9 +490,10 @@ void riscv_trigger_init(CPURISCVState *env)
* chain = 0 (unimplemented, always 0)
* match = 0 (always 0, when any compare value equals tdata2)
*/
- env->type2_trig[i].mcontrol = tdata1;
- env->type2_trig[i].maddress = 0;
- env->type2_trig[i].bp = NULL;
- env->type2_trig[i].wp = NULL;
+ env->tdata1[i] = tdata1;
+ env->tdata2[i] = 0;
+ env->tdata3[i] = 0;
+ env->cpu_breakpoint[i] = NULL;
+ env->cpu_watchpoint[i] = NULL;
}
}
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index b8173394a2..cb1c4b83b7 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -229,26 +229,16 @@ static bool debug_needed(void *opaque)
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,
+ .version_id = 2,
+ .minimum_version_id = 2,
.needed = debug_needed,
.fields = (VMStateField[]) {
VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
- VMSTATE_STRUCT_ARRAY(env.type2_trig, RISCVCPU, RV_MAX_TRIGGERS,
- 0, vmstate_debug_type2, type2_trigger_t),
+ VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS),
+ VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, RV_MAX_TRIGGERS),
+ VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, RV_MAX_TRIGGERS),
VMSTATE_END_OF_LIST()
}
};
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 4/8] target/riscv: debug: Restrict the range of tselect value can be written
2022-09-09 13:42 [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support Bin Meng
` (2 preceding siblings ...)
2022-09-09 13:42 ` [PATCH v2 3/8] target/riscv: debug: Introduce tdata1, tdata2, and tdata3 CSRs Bin Meng
@ 2022-09-09 13:42 ` Bin Meng
2022-09-16 1:59 ` LIU Zhiwei
2022-09-09 13:42 ` [PATCH v2 5/8] target/riscv: debug: Introduce tinfo CSR Bin Meng
` (4 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Bin Meng @ 2022-09-09 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
From: Frank Chang <frank.chang@sifive.com>
The value of tselect CSR can be written should be limited within the
range of supported triggers number.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---
(no changes since v1)
target/riscv/debug.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 06feef7d67..d6666164cd 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -127,10 +127,6 @@ bool tdata_available(CPURISCVState *env, int tdata_index)
return false;
}
- if (unlikely(env->trigger_cur >= RV_MAX_TRIGGERS)) {
- return false;
- }
-
return tdata_mapping[trigger_type][tdata_index];
}
@@ -141,8 +137,9 @@ target_ulong tselect_csr_read(CPURISCVState *env)
void tselect_csr_write(CPURISCVState *env, target_ulong val)
{
- /* all target_ulong bits of tselect are implemented */
- env->trigger_cur = val;
+ if (val < RV_MAX_TRIGGERS) {
+ env->trigger_cur = val;
+ }
}
static target_ulong tdata1_validate(CPURISCVState *env, target_ulong val,
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 5/8] target/riscv: debug: Introduce tinfo CSR
2022-09-09 13:42 [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support Bin Meng
` (3 preceding siblings ...)
2022-09-09 13:42 ` [PATCH v2 4/8] target/riscv: debug: Restrict the range of tselect value can be written Bin Meng
@ 2022-09-09 13:42 ` Bin Meng
2022-09-16 2:26 ` LIU Zhiwei
2022-09-09 13:42 ` [PATCH v2 6/8] target/riscv: debug: Create common trigger actions function Bin Meng
` (3 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Bin Meng @ 2022-09-09 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
From: Frank Chang <frank.chang@sifive.com>
tinfo.info:
One bit for each possible type enumerated in tdata1.
If the bit is set, then that type is supported by the currently
selected trigger.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---
(no changes since v1)
target/riscv/cpu_bits.h | 1 +
target/riscv/debug.h | 2 ++
target/riscv/csr.c | 8 ++++++++
target/riscv/debug.c | 10 +++++++---
4 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 7be12cac2e..1972aee3bb 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -321,6 +321,7 @@
#define CSR_TDATA1 0x7a1
#define CSR_TDATA2 0x7a2
#define CSR_TDATA3 0x7a3
+#define CSR_TINFO 0x7a4
/* Debug Mode Registers */
#define CSR_DCSR 0x7b0
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index 76146f373a..9f69c64591 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -95,6 +95,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);
+target_ulong tinfo_csr_read(CPURISCVState *env);
+
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);
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 3d0d8e0340..e66019048d 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3089,6 +3089,13 @@ static RISCVException write_tdata(CPURISCVState *env, int csrno,
return RISCV_EXCP_NONE;
}
+static RISCVException read_tinfo(CPURISCVState *env, int csrno,
+ target_ulong *val)
+{
+ *val = tinfo_csr_read(env);
+ return RISCV_EXCP_NONE;
+}
+
/*
* Functions to access Pointer Masking feature registers
* We have to check if current priv lvl could modify
@@ -3893,6 +3900,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
[CSR_TDATA1] = { "tdata1", debug, read_tdata, write_tdata },
[CSR_TDATA2] = { "tdata2", debug, read_tdata, write_tdata },
[CSR_TDATA3] = { "tdata3", debug, read_tdata, write_tdata },
+ [CSR_TINFO] = { "tinfo", debug, read_tinfo, write_ignore },
/* User Pointer Masking */
[CSR_UMTE] = { "umte", pointer_masking, read_umte, write_umte },
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index d6666164cd..7d546ace42 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -37,9 +37,7 @@
* - tdata1
* - tdata2
* - tdata3
- *
- * We don't support writable 'type' field in the tdata1 register, so there is
- * no need to implement the "tinfo" CSR.
+ * - tinfo
*
* The following triggers are implemented:
*
@@ -372,6 +370,12 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
}
}
+target_ulong tinfo_csr_read(CPURISCVState *env)
+{
+ /* assume all triggers support the same types of triggers */
+ return BIT(TRIGGER_TYPE_AD_MATCH);
+}
+
void riscv_cpu_debug_excp_handler(CPUState *cs)
{
RISCVCPU *cpu = RISCV_CPU(cs);
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 6/8] target/riscv: debug: Create common trigger actions function
2022-09-09 13:42 [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support Bin Meng
` (4 preceding siblings ...)
2022-09-09 13:42 ` [PATCH v2 5/8] target/riscv: debug: Introduce tinfo CSR Bin Meng
@ 2022-09-09 13:42 ` Bin Meng
2022-09-16 2:40 ` LIU Zhiwei
2022-09-09 13:42 ` [PATCH v2 7/8] target/riscv: debug: Check VU/VS modes for type 2 trigger Bin Meng
` (2 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Bin Meng @ 2022-09-09 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
From: Frank Chang <frank.chang@sifive.com>
Trigger actions are shared among all triggers. Extract to a common
function.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
[bmeng: handle the DBG_ACTION_NONE case]
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---
Changes in v2:
- add handling of the DBG_ACTION_NONE case in do_trigger_action()
target/riscv/debug.h | 13 ++++++++++
target/riscv/debug.c | 59 ++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 70 insertions(+), 2 deletions(-)
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index 9f69c64591..0e4859cf74 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -44,6 +44,19 @@ typedef enum {
TRIGGER_TYPE_NUM
} trigger_type_t;
+/* actions */
+typedef enum {
+ DBG_ACTION_NONE = -1, /* sentinel value */
+ DBG_ACTION_BP = 0,
+ DBG_ACTION_DBG_MODE,
+ DBG_ACTION_TRACE0,
+ DBG_ACTION_TRACE1,
+ DBG_ACTION_TRACE2,
+ DBG_ACTION_TRACE3,
+ DBG_ACTION_EXT_DBG0 = 8,
+ DBG_ACTION_EXT_DBG1
+} trigger_action_t;
+
/* tdata1 field masks */
#define RV32_TYPE(t) ((uint32_t)(t) << 28)
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 7d546ace42..7a8910f980 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -92,6 +92,37 @@ static inline target_ulong get_trigger_type(CPURISCVState *env,
return extract_trigger_type(env, env->tdata1[trigger_index]);
}
+static trigger_action_t get_trigger_action(CPURISCVState *env,
+ target_ulong trigger_index)
+{
+ target_ulong tdata1 = env->tdata1[trigger_index];
+ int trigger_type = get_trigger_type(env, trigger_index);
+ trigger_action_t action = DBG_ACTION_NONE;
+
+ switch (trigger_type) {
+ case TRIGGER_TYPE_AD_MATCH:
+ action = (tdata1 & TYPE2_ACTION) >> 12;
+ break;
+ case TRIGGER_TYPE_INST_CNT:
+ case TRIGGER_TYPE_INT:
+ case TRIGGER_TYPE_EXCP:
+ case TRIGGER_TYPE_AD_MATCH6:
+ case TRIGGER_TYPE_EXT_SRC:
+ qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
+ trigger_type);
+ break;
+ case TRIGGER_TYPE_NO_EXIST:
+ case TRIGGER_TYPE_UNAVAIL:
+ qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
+ trigger_type);
+ break;
+ default:
+ g_assert_not_reached();
+ }
+
+ return action;
+}
+
static inline target_ulong build_tdata1(CPURISCVState *env,
trigger_type_t type,
bool dmode, target_ulong data)
@@ -182,6 +213,30 @@ static inline void warn_always_zero_bit(target_ulong val, target_ulong mask,
}
}
+static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
+{
+ trigger_action_t action = get_trigger_action(env, trigger_index);
+
+ switch (action) {
+ case DBG_ACTION_NONE:
+ break;
+ case DBG_ACTION_BP:
+ riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
+ break;
+ case DBG_ACTION_DBG_MODE:
+ case DBG_ACTION_TRACE0:
+ case DBG_ACTION_TRACE1:
+ case DBG_ACTION_TRACE2:
+ case DBG_ACTION_TRACE3:
+ case DBG_ACTION_EXT_DBG0:
+ case DBG_ACTION_EXT_DBG1:
+ qemu_log_mask(LOG_UNIMP, "action: %d is not supported\n", action);
+ break;
+ default:
+ g_assert_not_reached();
+ }
+}
+
/* type 2 trigger */
static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
@@ -384,11 +439,11 @@ void riscv_cpu_debug_excp_handler(CPUState *cs)
if (cs->watchpoint_hit) {
if (cs->watchpoint_hit->flags & BP_CPU) {
cs->watchpoint_hit = NULL;
- riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
+ do_trigger_action(env, DBG_ACTION_BP);
}
} else {
if (cpu_breakpoint_test(cs, env->pc, BP_CPU)) {
- riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
+ do_trigger_action(env, DBG_ACTION_BP);
}
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 7/8] target/riscv: debug: Check VU/VS modes for type 2 trigger
2022-09-09 13:42 [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support Bin Meng
` (5 preceding siblings ...)
2022-09-09 13:42 ` [PATCH v2 6/8] target/riscv: debug: Create common trigger actions function Bin Meng
@ 2022-09-09 13:42 ` Bin Meng
2022-09-09 13:42 ` [PATCH v2 8/8] target/riscv: debug: Add initial support of type 6 trigger Bin Meng
2022-09-23 4:46 ` [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support Alistair Francis
8 siblings, 0 replies; 17+ messages in thread
From: Bin Meng @ 2022-09-09 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
From: Frank Chang <frank.chang@sifive.com>
Type 2 trigger cannot be fired in VU/VS modes.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---
(no changes since v1)
target/riscv/debug.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index 7a8910f980..e16d5c070a 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -464,6 +464,11 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
switch (trigger_type) {
case TRIGGER_TYPE_AD_MATCH:
+ /* type 2 trigger cannot be fired in VU/VS mode */
+ if (riscv_cpu_virt_enabled(env)) {
+ return false;
+ }
+
ctrl = env->tdata1[i];
pc = env->tdata2[i];
@@ -499,6 +504,11 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
switch (trigger_type) {
case TRIGGER_TYPE_AD_MATCH:
+ /* type 2 trigger cannot be fired in VU/VS mode */
+ if (riscv_cpu_virt_enabled(env)) {
+ return false;
+ }
+
ctrl = env->tdata1[i];
addr = env->tdata2[i];
flags = 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v2 8/8] target/riscv: debug: Add initial support of type 6 trigger
2022-09-09 13:42 [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support Bin Meng
` (6 preceding siblings ...)
2022-09-09 13:42 ` [PATCH v2 7/8] target/riscv: debug: Check VU/VS modes for type 2 trigger Bin Meng
@ 2022-09-09 13:42 ` Bin Meng
2022-09-23 4:46 ` [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support Alistair Francis
8 siblings, 0 replies; 17+ messages in thread
From: Bin Meng @ 2022-09-09 13:42 UTC (permalink / raw)
To: qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
From: Frank Chang <frank.chang@sifive.com>
Type 6 trigger is similar to a type 2 trigger, but provides additional
functionality and should be used instead of type 2 in newer
implementations.
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
---
(no changes since v1)
target/riscv/debug.h | 18 +++++
target/riscv/debug.c | 174 ++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 188 insertions(+), 4 deletions(-)
diff --git a/target/riscv/debug.h b/target/riscv/debug.h
index 0e4859cf74..a1226b4d29 100644
--- a/target/riscv/debug.h
+++ b/target/riscv/debug.h
@@ -85,6 +85,24 @@ typedef enum {
#define TYPE2_HIT BIT(20)
#define TYPE2_SIZEHI (0x3 << 21) /* RV64 only */
+/* mcontrol6 field masks */
+
+#define TYPE6_LOAD BIT(0)
+#define TYPE6_STORE BIT(1)
+#define TYPE6_EXEC BIT(2)
+#define TYPE6_U BIT(3)
+#define TYPE6_S BIT(4)
+#define TYPE6_M BIT(6)
+#define TYPE6_MATCH (0xf << 7)
+#define TYPE6_CHAIN BIT(11)
+#define TYPE6_ACTION (0xf << 12)
+#define TYPE6_SIZE (0xf << 16)
+#define TYPE6_TIMING BIT(20)
+#define TYPE6_SELECT BIT(21)
+#define TYPE6_HIT BIT(22)
+#define TYPE6_VU BIT(23)
+#define TYPE6_VS BIT(24)
+
/* access size */
enum {
SIZE_ANY = 0,
diff --git a/target/riscv/debug.c b/target/riscv/debug.c
index e16d5c070a..26ea764407 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -39,7 +39,7 @@
* - tdata3
* - tinfo
*
- * The following triggers are implemented:
+ * The following triggers are initialized by default:
*
* Index | Type | tdata mapping | Description
* ------+------+------------------------+------------
@@ -103,10 +103,12 @@ static trigger_action_t get_trigger_action(CPURISCVState *env,
case TRIGGER_TYPE_AD_MATCH:
action = (tdata1 & TYPE2_ACTION) >> 12;
break;
+ case TRIGGER_TYPE_AD_MATCH6:
+ action = (tdata1 & TYPE6_ACTION) >> 12;
+ break;
case TRIGGER_TYPE_INST_CNT:
case TRIGGER_TYPE_INT:
case TRIGGER_TYPE_EXCP:
- case TRIGGER_TYPE_AD_MATCH6:
case TRIGGER_TYPE_EXT_SRC:
qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
trigger_type);
@@ -379,6 +381,123 @@ static void type2_reg_write(CPURISCVState *env, target_ulong index,
return;
}
+/* type 6 trigger */
+
+static inline bool type6_breakpoint_enabled(target_ulong ctrl)
+{
+ bool mode = !!(ctrl & (TYPE6_VU | TYPE6_VS | TYPE6_U | TYPE6_S | TYPE6_M));
+ bool rwx = !!(ctrl & (TYPE6_LOAD | TYPE6_STORE | TYPE6_EXEC));
+
+ return mode && rwx;
+}
+
+static target_ulong type6_mcontrol6_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_MATCH6);
+
+ /* validate unimplemented (always zero) bits */
+ warn_always_zero_bit(ctrl, TYPE6_MATCH, "match");
+ warn_always_zero_bit(ctrl, TYPE6_CHAIN, "chain");
+ warn_always_zero_bit(ctrl, TYPE6_ACTION, "action");
+ warn_always_zero_bit(ctrl, TYPE6_TIMING, "timing");
+ warn_always_zero_bit(ctrl, TYPE6_SELECT, "select");
+ warn_always_zero_bit(ctrl, TYPE6_HIT, "hit");
+
+ /* validate size encoding */
+ size = extract32(ctrl, 16, 4);
+ if (access_size[size] == -1) {
+ qemu_log_mask(LOG_UNIMP, "access size %d is not supported, using SIZE_ANY\n",
+ size);
+ } else {
+ val |= (ctrl & TYPE6_SIZE);
+ }
+
+ /* keep the mode and attribute bits */
+ val |= (ctrl & (TYPE6_VU | TYPE6_VS | TYPE6_U | TYPE6_S | TYPE6_M |
+ TYPE6_LOAD | TYPE6_STORE | TYPE6_EXEC));
+
+ return val;
+}
+
+static void type6_breakpoint_insert(CPURISCVState *env, target_ulong index)
+{
+ target_ulong ctrl = env->tdata1[index];
+ target_ulong addr = env->tdata2[index];
+ bool enabled = type6_breakpoint_enabled(ctrl);
+ CPUState *cs = env_cpu(env);
+ int flags = BP_CPU | BP_STOP_BEFORE_ACCESS;
+ uint32_t size;
+
+ if (!enabled) {
+ return;
+ }
+
+ if (ctrl & TYPE6_EXEC) {
+ cpu_breakpoint_insert(cs, addr, flags, &env->cpu_breakpoint[index]);
+ }
+
+ if (ctrl & TYPE6_LOAD) {
+ flags |= BP_MEM_READ;
+ }
+
+ if (ctrl & TYPE6_STORE) {
+ flags |= BP_MEM_WRITE;
+ }
+
+ if (flags & BP_MEM_ACCESS) {
+ size = extract32(ctrl, 16, 4);
+ if (size != 0) {
+ cpu_watchpoint_insert(cs, addr, size, flags,
+ &env->cpu_watchpoint[index]);
+ } else {
+ cpu_watchpoint_insert(cs, addr, 8, flags,
+ &env->cpu_watchpoint[index]);
+ }
+ }
+}
+
+static void type6_breakpoint_remove(CPURISCVState *env, target_ulong index)
+{
+ type2_breakpoint_remove(env, index);
+}
+
+static void type6_reg_write(CPURISCVState *env, target_ulong index,
+ int tdata_index, target_ulong val)
+{
+ target_ulong new_val;
+
+ switch (tdata_index) {
+ case TDATA1:
+ new_val = type6_mcontrol6_validate(env, val);
+ if (new_val != env->tdata1[index]) {
+ env->tdata1[index] = new_val;
+ type6_breakpoint_remove(env, index);
+ type6_breakpoint_insert(env, index);
+ }
+ break;
+ case TDATA2:
+ if (val != env->tdata2[index]) {
+ env->tdata2[index] = val;
+ type6_breakpoint_remove(env, index);
+ type6_breakpoint_insert(env, index);
+ }
+ break;
+ case TDATA3:
+ qemu_log_mask(LOG_UNIMP,
+ "tdata3 is not supported for type 6 trigger\n");
+ break;
+ default:
+ g_assert_not_reached();
+ }
+
+ return;
+}
+
target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
{
switch (tdata_index) {
@@ -407,10 +526,12 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
case TRIGGER_TYPE_AD_MATCH:
type2_reg_write(env, env->trigger_cur, tdata_index, val);
break;
+ case TRIGGER_TYPE_AD_MATCH6:
+ type6_reg_write(env, env->trigger_cur, tdata_index, val);
+ break;
case TRIGGER_TYPE_INST_CNT:
case TRIGGER_TYPE_INT:
case TRIGGER_TYPE_EXCP:
- case TRIGGER_TYPE_AD_MATCH6:
case TRIGGER_TYPE_EXT_SRC:
qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
trigger_type);
@@ -428,7 +549,8 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
target_ulong tinfo_csr_read(CPURISCVState *env)
{
/* assume all triggers support the same types of triggers */
- return BIT(TRIGGER_TYPE_AD_MATCH);
+ return BIT(TRIGGER_TYPE_AD_MATCH) |
+ BIT(TRIGGER_TYPE_AD_MATCH6);
}
void riscv_cpu_debug_excp_handler(CPUState *cs)
@@ -479,6 +601,24 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
}
}
break;
+ case TRIGGER_TYPE_AD_MATCH6:
+ ctrl = env->tdata1[i];
+ pc = env->tdata2[i];
+
+ if ((ctrl & TYPE6_EXEC) && (bp->pc == pc)) {
+ if (riscv_cpu_virt_enabled(env)) {
+ /* check VU/VS bit against current privilege level */
+ if ((ctrl >> 23) & BIT(env->priv)) {
+ return true;
+ }
+ } else {
+ /* check U/S/M bit against current privilege level */
+ if ((ctrl >> 3) & BIT(env->priv)) {
+ return true;
+ }
+ }
+ }
+ break;
default:
/* other trigger types are not supported or irrelevant */
break;
@@ -527,6 +667,32 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
}
}
break;
+ case TRIGGER_TYPE_AD_MATCH6:
+ ctrl = env->tdata1[i];
+ addr = env->tdata2[i];
+ flags = 0;
+
+ if (ctrl & TYPE6_LOAD) {
+ flags |= BP_MEM_READ;
+ }
+ if (ctrl & TYPE6_STORE) {
+ flags |= BP_MEM_WRITE;
+ }
+
+ if ((wp->flags & flags) && (wp->vaddr == addr)) {
+ if (riscv_cpu_virt_enabled(env)) {
+ /* check VU/VS bit against current privilege level */
+ if ((ctrl >> 23) & BIT(env->priv)) {
+ return true;
+ }
+ } else {
+ /* check U/S/M bit against current privilege level */
+ if ((ctrl >> 3) & BIT(env->priv)) {
+ return true;
+ }
+ }
+ }
+ break;
default:
/* other trigger types are not supported */
break;
--
2.34.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/8] target/riscv: debug: Determine the trigger type from tdata1.type
2022-09-09 13:42 ` [PATCH v2 1/8] target/riscv: debug: Determine the trigger type from tdata1.type Bin Meng
@ 2022-09-16 1:53 ` LIU Zhiwei
2022-09-16 2:42 ` LIU Zhiwei
1 sibling, 0 replies; 17+ messages in thread
From: LIU Zhiwei @ 2022-09-16 1:53 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
On 2022/9/9 21:42, Bin Meng wrote:
> From: Frank Chang <frank.chang@sifive.com>
>
> Current RISC-V debug assumes that only type 2 trigger is supported.
> To allow more types of triggers to be supported in the future
> (e.g. type 6 trigger, which is similar to type 2 trigger with additional
> functionality), we should determine the trigger type from tdata1.type.
>
> RV_MAX_TRIGGERS is also introduced in replacement of TRIGGER_TYPE2_NUM.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> [bmeng: fixed MXL_RV128 case, and moved macros to the following patch]
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
> Changes in v2:
> - fixed MXL_RV128 case
> - moved macros to patch#2
> - added log guest errors for TRIGGER_TYPE_{NO_EXIST,UNAVAIL}
>
> target/riscv/cpu.h | 2 +-
> target/riscv/debug.h | 13 +--
> target/riscv/csr.c | 2 +-
> target/riscv/debug.c | 188 +++++++++++++++++++++++++++++------------
> target/riscv/machine.c | 2 +-
> 5 files changed, 140 insertions(+), 67 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 06751e1e3e..4d82a3250b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -324,7 +324,7 @@ struct CPUArchState {
>
> /* trigger module */
> target_ulong trigger_cur;
> - type2_trigger_t type2_trig[TRIGGER_TYPE2_NUM];
> + type2_trigger_t type2_trig[RV_MAX_TRIGGERS];
>
> /* machine specific rdtime callback */
> uint64_t (*rdtime_fn)(void *);
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index 27b9cac6b4..72e4edcd8c 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -22,13 +22,7 @@
> #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
> -};
> +#define RV_MAX_TRIGGERS 2
>
> /* register index of tdata CSRs */
> enum {
> @@ -46,7 +40,8 @@ typedef enum {
> 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_UNAVAIL = 15, /* trigger exists, but unavailable */
> + TRIGGER_TYPE_NUM
> } trigger_type_t;
>
> typedef struct {
> @@ -56,7 +51,7 @@ typedef struct {
> struct CPUWatchpoint *wp;
> } type2_trigger_t;
>
> -/* tdata field masks */
> +/* tdata1 field masks */
>
> #define RV32_TYPE(t) ((uint32_t)(t) << 28)
> #define RV32_TYPE_MASK (0xf << 28)
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index b96db1b62b..3d0d8e0340 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3065,7 +3065,7 @@ 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) {
> + if (env->trigger_cur >= RV_MAX_TRIGGERS && csrno == CSR_TDATA1) {
> *val = 0;
> return RISCV_EXCP_NONE;
> }
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index fc6e13222f..9dd468753a 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -52,8 +52,15 @@
> /* 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 },
> +static tdata_avail tdata_mapping[TRIGGER_TYPE_NUM] = {
> + [TRIGGER_TYPE_NO_EXIST] = { false, false, false },
> + [TRIGGER_TYPE_AD_MATCH] = { true, true, true },
> + [TRIGGER_TYPE_INST_CNT] = { true, false, true },
> + [TRIGGER_TYPE_INT] = { true, true, true },
> + [TRIGGER_TYPE_EXCP] = { true, true, true },
> + [TRIGGER_TYPE_AD_MATCH6] = { true, true, true },
> + [TRIGGER_TYPE_EXT_SRC] = { true, false, false },
> + [TRIGGER_TYPE_UNAVAIL] = { true, true, true }
> };
>
> /* only breakpoint size 1/2/4/8 supported */
> @@ -67,6 +74,27 @@ static int access_size[SIZE_NUM] = {
> [6 ... 15] = -1,
> };
>
> +static inline target_ulong extract_trigger_type(CPURISCVState *env,
> + target_ulong tdata1)
> +{
> + switch (riscv_cpu_mxl(env)) {
> + case MXL_RV32:
> + return extract32(tdata1, 28, 4);
> + case MXL_RV64:
> + case MXL_RV128:
> + return extract64(tdata1, 60, 4);
> + default:
> + g_assert_not_reached();
> + }
> +}
> +
> +static inline target_ulong get_trigger_type(CPURISCVState *env,
> + target_ulong trigger_index)
> +{
> + target_ulong tdata1 = env->type2_trig[trigger_index].mcontrol;
> + return extract_trigger_type(env, tdata1);
> +}
> +
> static inline target_ulong trigger_type(CPURISCVState *env,
> trigger_type_t type)
> {
> @@ -89,15 +117,17 @@ static inline target_ulong trigger_type(CPURISCVState *env,
>
> bool tdata_available(CPURISCVState *env, int tdata_index)
> {
> + int trigger_type = get_trigger_type(env, env->trigger_cur);
> +
> if (unlikely(tdata_index >= TDATA_NUM)) {
> return false;
> }
>
> - if (unlikely(env->trigger_cur >= TRIGGER_NUM)) {
> + if (unlikely(env->trigger_cur >= RV_MAX_TRIGGERS)) {
> return false;
> }
>
> - return tdata_mapping[env->trigger_cur][tdata_index];
> + return tdata_mapping[trigger_type][tdata_index];
> }
>
> target_ulong tselect_csr_read(CPURISCVState *env)
> @@ -137,6 +167,7 @@ static target_ulong tdata1_validate(CPURISCVState *env, target_ulong val,
> 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");
> }
> @@ -261,9 +292,8 @@ static void type2_breakpoint_remove(CPURISCVState *env, target_ulong index)
> }
>
> static target_ulong type2_reg_read(CPURISCVState *env,
> - target_ulong trigger_index, int tdata_index)
> + target_ulong index, int tdata_index)
> {
> - uint32_t index = trigger_index - TRIGGER_TYPE2_IDX_0;
> target_ulong tdata;
>
> switch (tdata_index) {
> @@ -280,10 +310,9 @@ static target_ulong type2_reg_read(CPURISCVState *env,
> return tdata;
> }
>
> -static void type2_reg_write(CPURISCVState *env, target_ulong trigger_index,
> +static void type2_reg_write(CPURISCVState *env, target_ulong index,
> int tdata_index, target_ulong val)
> {
> - uint32_t index = trigger_index - TRIGGER_TYPE2_IDX_0;
> target_ulong new_val;
>
> switch (tdata_index) {
> @@ -309,35 +338,64 @@ static void type2_reg_write(CPURISCVState *env, target_ulong trigger_index,
> 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];
> + int trigger_type = get_trigger_type(env, env->trigger_cur);
> +
> + switch (trigger_type) {
> + case TRIGGER_TYPE_AD_MATCH:
> + return type2_reg_read(env, env->trigger_cur, tdata_index);
> + break;
> + case TRIGGER_TYPE_INST_CNT:
> + case TRIGGER_TYPE_INT:
> + case TRIGGER_TYPE_EXCP:
> + case TRIGGER_TYPE_AD_MATCH6:
> + case TRIGGER_TYPE_EXT_SRC:
> + qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
> + trigger_type);
> + break;
> + case TRIGGER_TYPE_NO_EXIST:
> + case TRIGGER_TYPE_UNAVAIL:
> + qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
> + trigger_type);
> + break;
> + default:
> + g_assert_not_reached();
> + }
>
> - return read_func(env, env->trigger_cur, tdata_index);
> + return 0;
> }
>
> void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
> {
> - tdata_write_func write_func = trigger_write_funcs[env->trigger_cur];
> + int trigger_type;
>
> - return write_func(env, env->trigger_cur, tdata_index, val);
> + if (tdata_index == TDATA1) {
> + trigger_type = extract_trigger_type(env, val);
> + } else {
> + trigger_type = get_trigger_type(env, env->trigger_cur);
> + }
> +
> + switch (trigger_type) {
> + case TRIGGER_TYPE_AD_MATCH:
> + type2_reg_write(env, env->trigger_cur, tdata_index, val);
> + break;
> + case TRIGGER_TYPE_INST_CNT:
> + case TRIGGER_TYPE_INT:
> + case TRIGGER_TYPE_EXCP:
> + case TRIGGER_TYPE_AD_MATCH6:
> + case TRIGGER_TYPE_EXT_SRC:
> + qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
> + trigger_type);
> + break;
> + case TRIGGER_TYPE_NO_EXIST:
> + case TRIGGER_TYPE_UNAVAIL:
> + qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
> + trigger_type);
> + break;
> + default:
> + g_assert_not_reached();
> + }
> }
>
> void riscv_cpu_debug_excp_handler(CPUState *cs)
> @@ -364,18 +422,28 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
> CPUBreakpoint *bp;
> target_ulong ctrl;
> target_ulong pc;
> + int trigger_type;
> 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;
> + for (i = 0; i < RV_MAX_TRIGGERS; i++) {
> + trigger_type = get_trigger_type(env, i);
> +
> + switch (trigger_type) {
> + case TRIGGER_TYPE_AD_MATCH:
> + 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;
> + }
> }
> + break;
> + default:
> + /* other trigger types are not supported or irrelevant */
> + break;
> }
> }
> }
> @@ -389,26 +457,36 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
> CPURISCVState *env = &cpu->env;
> target_ulong ctrl;
> target_ulong addr;
> + int trigger_type;
> 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;
> + for (i = 0; i < RV_MAX_TRIGGERS; i++) {
> + trigger_type = get_trigger_type(env, i);
>
> - if (ctrl & TYPE2_LOAD) {
> - flags |= BP_MEM_READ;
> - }
> - if (ctrl & TYPE2_STORE) {
> - flags |= BP_MEM_WRITE;
> - }
> + switch (trigger_type) {
> + case TRIGGER_TYPE_AD_MATCH:
> + ctrl = env->type2_trig[i].mcontrol;
> + addr = env->type2_trig[i].maddress;
> + flags = 0;
>
> - if ((wp->flags & flags) && (wp->vaddr == addr)) {
> - /* check U/S/M bit against current privilege level */
> - if ((ctrl >> 3) & BIT(env->priv)) {
> - return true;
> + 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;
> + }
> }
> + break;
> + default:
> + /* other trigger types are not supported */
> + break;
> }
> }
>
> @@ -417,11 +495,11 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>
> void riscv_trigger_init(CPURISCVState *env)
> {
> - target_ulong type2 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
> + target_ulong tdata1 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
> int i;
>
> - /* type 2 triggers */
> - for (i = 0; i < TRIGGER_TYPE2_NUM; i++) {
> + /* init to type 2 triggers */
> + for (i = 0; i < RV_MAX_TRIGGERS; i++) {
> /*
> * type = TRIGGER_TYPE_AD_MATCH
> * dmode = 0 (both debug and M-mode can write tdata)
> @@ -435,7 +513,7 @@ void riscv_trigger_init(CPURISCVState *env)
> * 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].mcontrol = tdata1;
> env->type2_trig[i].maddress = 0;
> env->type2_trig[i].bp = NULL;
> env->type2_trig[i].wp = NULL;
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 41098f6ad0..b8173394a2 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -247,7 +247,7 @@ static const VMStateDescription vmstate_debug = {
> .needed = debug_needed,
> .fields = (VMStateField[]) {
> VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
> - VMSTATE_STRUCT_ARRAY(env.type2_trig, RISCVCPU, TRIGGER_TYPE2_NUM,
> + VMSTATE_STRUCT_ARRAY(env.type2_trig, RISCVCPU, RV_MAX_TRIGGERS,
> 0, vmstate_debug_type2, type2_trigger_t),
> VMSTATE_END_OF_LIST()
> }
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 2/8] target/riscv: debug: Introduce build_tdata1() to build tdata1 register content
2022-09-09 13:42 ` [PATCH v2 2/8] target/riscv: debug: Introduce build_tdata1() to build tdata1 register content Bin Meng
@ 2022-09-16 1:55 ` LIU Zhiwei
0 siblings, 0 replies; 17+ messages in thread
From: LIU Zhiwei @ 2022-09-16 1:55 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
On 2022/9/9 21:42, Bin Meng wrote:
> From: Frank Chang <frank.chang@sifive.com>
>
> Introduce build_tdata1() to build tdata1 register content, which can be
> shared among all types of triggers.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> [bmeng: moved RV{32,64}_DATA_MASK definition to this patch]
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
> Changes in v2:
> - moved RV{32,64}_DATA_MASK definition to this patch
>
> target/riscv/debug.h | 2 ++
> target/riscv/debug.c | 15 ++++++++++-----
> 2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index 72e4edcd8c..c422553c27 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -56,9 +56,11 @@ typedef struct {
> #define RV32_TYPE(t) ((uint32_t)(t) << 28)
> #define RV32_TYPE_MASK (0xf << 28)
> #define RV32_DMODE BIT(27)
> +#define RV32_DATA_MASK 0x7ffffff
> #define RV64_TYPE(t) ((uint64_t)(t) << 60)
> #define RV64_TYPE_MASK (0xfULL << 60)
> #define RV64_DMODE BIT_ULL(59)
> +#define RV64_DATA_MASK 0x7ffffffffffffff
>
> /* mcontrol field masks */
>
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 9dd468753a..45aae87ec3 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -95,18 +95,23 @@ static inline target_ulong get_trigger_type(CPURISCVState *env,
> return extract_trigger_type(env, tdata1);
> }
>
> -static inline target_ulong trigger_type(CPURISCVState *env,
> - trigger_type_t type)
> +static inline target_ulong build_tdata1(CPURISCVState *env,
> + trigger_type_t type,
> + bool dmode, target_ulong data)
> {
> target_ulong tdata1;
>
> switch (riscv_cpu_mxl(env)) {
> case MXL_RV32:
> - tdata1 = RV32_TYPE(type);
> + tdata1 = RV32_TYPE(type) |
> + (dmode ? RV32_DMODE : 0) |
> + (data & RV32_DATA_MASK);
> break;
> case MXL_RV64:
> case MXL_RV128:
> - tdata1 = RV64_TYPE(type);
> + tdata1 = RV64_TYPE(type) |
> + (dmode ? RV64_DMODE : 0) |
> + (data & RV64_DATA_MASK);
> break;
> default:
> g_assert_not_reached();
> @@ -495,7 +500,7 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>
> void riscv_trigger_init(CPURISCVState *env)
> {
> - target_ulong tdata1 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
> + target_ulong tdata1 = build_tdata1(env, TRIGGER_TYPE_AD_MATCH, 0, 0);
> int i;
>
> /* init to type 2 triggers */
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 3/8] target/riscv: debug: Introduce tdata1, tdata2, and tdata3 CSRs
2022-09-09 13:42 ` [PATCH v2 3/8] target/riscv: debug: Introduce tdata1, tdata2, and tdata3 CSRs Bin Meng
@ 2022-09-16 1:58 ` LIU Zhiwei
0 siblings, 0 replies; 17+ messages in thread
From: LIU Zhiwei @ 2022-09-16 1:58 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
On 2022/9/9 21:42, Bin Meng wrote:
> From: Frank Chang <frank.chang@sifive.com>
>
> Replace type2_trigger_t with the real tdata1, tdata2, and tdata3 CSRs,
> which allows us to support more types of triggers in the future.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
> (no changes since v1)
>
> target/riscv/cpu.h | 6 ++-
> target/riscv/debug.h | 7 ---
> target/riscv/debug.c | 103 +++++++++++++++--------------------------
> target/riscv/machine.c | 20 ++------
> 4 files changed, 48 insertions(+), 88 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 4d82a3250b..b0b05c19ca 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -324,7 +324,11 @@ struct CPUArchState {
>
> /* trigger module */
> target_ulong trigger_cur;
> - type2_trigger_t type2_trig[RV_MAX_TRIGGERS];
> + target_ulong tdata1[RV_MAX_TRIGGERS];
> + target_ulong tdata2[RV_MAX_TRIGGERS];
> + target_ulong tdata3[RV_MAX_TRIGGERS];
> + struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS];
> + struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS];
These fields should not packed into env. As it had already existed
before this patch set,
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
>
> /* machine specific rdtime callback */
> uint64_t (*rdtime_fn)(void *);
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index c422553c27..76146f373a 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -44,13 +44,6 @@ typedef enum {
> TRIGGER_TYPE_NUM
> } trigger_type_t;
>
> -typedef struct {
> - target_ulong mcontrol;
> - target_ulong maddress;
> - struct CPUBreakpoint *bp;
> - struct CPUWatchpoint *wp;
> -} type2_trigger_t;
> -
> /* tdata1 field masks */
>
> #define RV32_TYPE(t) ((uint32_t)(t) << 28)
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 45aae87ec3..06feef7d67 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -91,8 +91,7 @@ static inline target_ulong extract_trigger_type(CPURISCVState *env,
> static inline target_ulong get_trigger_type(CPURISCVState *env,
> target_ulong trigger_index)
> {
> - target_ulong tdata1 = env->type2_trig[trigger_index].mcontrol;
> - return extract_trigger_type(env, tdata1);
> + return extract_trigger_type(env, env->tdata1[trigger_index]);
> }
>
> static inline target_ulong build_tdata1(CPURISCVState *env,
> @@ -188,6 +187,8 @@ static inline void warn_always_zero_bit(target_ulong val, target_ulong mask,
> }
> }
>
> +/* type 2 trigger */
> +
> static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
> {
> uint32_t size, sizelo, sizehi = 0;
> @@ -247,8 +248,8 @@ static target_ulong type2_mcontrol_validate(CPURISCVState *env,
>
> 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;
> + target_ulong ctrl = env->tdata1[index];
> + target_ulong addr = env->tdata2[index];
> bool enabled = type2_breakpoint_enabled(ctrl);
> CPUState *cs = env_cpu(env);
> int flags = BP_CPU | BP_STOP_BEFORE_ACCESS;
> @@ -259,7 +260,7 @@ static void type2_breakpoint_insert(CPURISCVState *env, target_ulong index)
> }
>
> if (ctrl & TYPE2_EXEC) {
> - cpu_breakpoint_insert(cs, addr, flags, &env->type2_trig[index].bp);
> + cpu_breakpoint_insert(cs, addr, flags, &env->cpu_breakpoint[index]);
> }
>
> if (ctrl & TYPE2_LOAD) {
> @@ -273,10 +274,10 @@ static void type2_breakpoint_insert(CPURISCVState *env, target_ulong index)
> size = type2_breakpoint_size(env, ctrl);
> if (size != 0) {
> cpu_watchpoint_insert(cs, addr, size, flags,
> - &env->type2_trig[index].wp);
> + &env->cpu_watchpoint[index]);
> } else {
> cpu_watchpoint_insert(cs, addr, 8, flags,
> - &env->type2_trig[index].wp);
> + &env->cpu_watchpoint[index]);
> }
> }
> }
> @@ -285,36 +286,17 @@ 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->cpu_breakpoint[index]) {
> + cpu_breakpoint_remove_by_ref(cs, env->cpu_breakpoint[index]);
> + env->cpu_breakpoint[index] = NULL;
> }
>
> - if (env->type2_trig[index].wp) {
> - cpu_watchpoint_remove_by_ref(cs, env->type2_trig[index].wp);
> - env->type2_trig[index].wp = NULL;
> + if (env->cpu_watchpoint[index]) {
> + cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
> + env->cpu_watchpoint[index] = NULL;
> }
> }
>
> -static target_ulong type2_reg_read(CPURISCVState *env,
> - target_ulong index, int tdata_index)
> -{
> - 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 index,
> int tdata_index, target_ulong val)
> {
> @@ -323,19 +305,23 @@ static void type2_reg_write(CPURISCVState *env, target_ulong index,
> 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;
> + if (new_val != env->tdata1[index]) {
> + env->tdata1[index] = 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;
> + if (val != env->tdata2[index]) {
> + env->tdata2[index] = val;
> type2_breakpoint_remove(env, index);
> type2_breakpoint_insert(env, index);
> }
> break;
> + case TDATA3:
> + qemu_log_mask(LOG_UNIMP,
> + "tdata3 is not supported for type 2 trigger\n");
> + break;
> default:
> g_assert_not_reached();
> }
> @@ -345,30 +331,16 @@ static void type2_reg_write(CPURISCVState *env, target_ulong index,
>
> target_ulong tdata_csr_read(CPURISCVState *env, int tdata_index)
> {
> - int trigger_type = get_trigger_type(env, env->trigger_cur);
> -
> - switch (trigger_type) {
> - case TRIGGER_TYPE_AD_MATCH:
> - return type2_reg_read(env, env->trigger_cur, tdata_index);
> - break;
> - case TRIGGER_TYPE_INST_CNT:
> - case TRIGGER_TYPE_INT:
> - case TRIGGER_TYPE_EXCP:
> - case TRIGGER_TYPE_AD_MATCH6:
> - case TRIGGER_TYPE_EXT_SRC:
> - qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
> - trigger_type);
> - break;
> - case TRIGGER_TYPE_NO_EXIST:
> - case TRIGGER_TYPE_UNAVAIL:
> - qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
> - trigger_type);
> - break;
> + switch (tdata_index) {
> + case TDATA1:
> + return env->tdata1[env->trigger_cur];
> + case TDATA2:
> + return env->tdata2[env->trigger_cur];
> + case TDATA3:
> + return env->tdata3[env->trigger_cur];
> default:
> g_assert_not_reached();
> }
> -
> - return 0;
> }
>
> void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
> @@ -436,8 +408,8 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
>
> switch (trigger_type) {
> case TRIGGER_TYPE_AD_MATCH:
> - ctrl = env->type2_trig[i].mcontrol;
> - pc = env->type2_trig[i].maddress;
> + ctrl = env->tdata1[i];
> + pc = env->tdata2[i];
>
> if ((ctrl & TYPE2_EXEC) && (bp->pc == pc)) {
> /* check U/S/M bit against current privilege level */
> @@ -471,8 +443,8 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>
> switch (trigger_type) {
> case TRIGGER_TYPE_AD_MATCH:
> - ctrl = env->type2_trig[i].mcontrol;
> - addr = env->type2_trig[i].maddress;
> + ctrl = env->tdata1[i];
> + addr = env->tdata2[i];
> flags = 0;
>
> if (ctrl & TYPE2_LOAD) {
> @@ -518,9 +490,10 @@ void riscv_trigger_init(CPURISCVState *env)
> * chain = 0 (unimplemented, always 0)
> * match = 0 (always 0, when any compare value equals tdata2)
> */
> - env->type2_trig[i].mcontrol = tdata1;
> - env->type2_trig[i].maddress = 0;
> - env->type2_trig[i].bp = NULL;
> - env->type2_trig[i].wp = NULL;
> + env->tdata1[i] = tdata1;
> + env->tdata2[i] = 0;
> + env->tdata3[i] = 0;
> + env->cpu_breakpoint[i] = NULL;
> + env->cpu_watchpoint[i] = NULL;
> }
> }
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index b8173394a2..cb1c4b83b7 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -229,26 +229,16 @@ static bool debug_needed(void *opaque)
> 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,
> + .version_id = 2,
> + .minimum_version_id = 2,
> .needed = debug_needed,
> .fields = (VMStateField[]) {
> VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
> - VMSTATE_STRUCT_ARRAY(env.type2_trig, RISCVCPU, RV_MAX_TRIGGERS,
> - 0, vmstate_debug_type2, type2_trigger_t),
> + VMSTATE_UINTTL_ARRAY(env.tdata1, RISCVCPU, RV_MAX_TRIGGERS),
> + VMSTATE_UINTTL_ARRAY(env.tdata2, RISCVCPU, RV_MAX_TRIGGERS),
> + VMSTATE_UINTTL_ARRAY(env.tdata3, RISCVCPU, RV_MAX_TRIGGERS),
> VMSTATE_END_OF_LIST()
> }
> };
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 4/8] target/riscv: debug: Restrict the range of tselect value can be written
2022-09-09 13:42 ` [PATCH v2 4/8] target/riscv: debug: Restrict the range of tselect value can be written Bin Meng
@ 2022-09-16 1:59 ` LIU Zhiwei
0 siblings, 0 replies; 17+ messages in thread
From: LIU Zhiwei @ 2022-09-16 1:59 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
On 2022/9/9 21:42, Bin Meng wrote:
> From: Frank Chang <frank.chang@sifive.com>
>
> The value of tselect CSR can be written should be limited within the
> range of supported triggers number.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
> (no changes since v1)
>
> target/riscv/debug.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 06feef7d67..d6666164cd 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -127,10 +127,6 @@ bool tdata_available(CPURISCVState *env, int tdata_index)
> return false;
> }
>
> - if (unlikely(env->trigger_cur >= RV_MAX_TRIGGERS)) {
> - return false;
> - }
> -
> return tdata_mapping[trigger_type][tdata_index];
> }
>
> @@ -141,8 +137,9 @@ target_ulong tselect_csr_read(CPURISCVState *env)
>
> void tselect_csr_write(CPURISCVState *env, target_ulong val)
> {
> - /* all target_ulong bits of tselect are implemented */
> - env->trigger_cur = val;
> + if (val < RV_MAX_TRIGGERS) {
> + env->trigger_cur = val;
> + }
> }
>
> static target_ulong tdata1_validate(CPURISCVState *env, target_ulong val,
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 5/8] target/riscv: debug: Introduce tinfo CSR
2022-09-09 13:42 ` [PATCH v2 5/8] target/riscv: debug: Introduce tinfo CSR Bin Meng
@ 2022-09-16 2:26 ` LIU Zhiwei
0 siblings, 0 replies; 17+ messages in thread
From: LIU Zhiwei @ 2022-09-16 2:26 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
On 2022/9/9 21:42, Bin Meng wrote:
> From: Frank Chang <frank.chang@sifive.com>
>
> tinfo.info:
> One bit for each possible type enumerated in tdata1.
> If the bit is set, then that type is supported by the currently
> selected trigger.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> ---
>
> (no changes since v1)
>
> target/riscv/cpu_bits.h | 1 +
> target/riscv/debug.h | 2 ++
> target/riscv/csr.c | 8 ++++++++
> target/riscv/debug.c | 10 +++++++---
> 4 files changed, 18 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 7be12cac2e..1972aee3bb 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -321,6 +321,7 @@
> #define CSR_TDATA1 0x7a1
> #define CSR_TDATA2 0x7a2
> #define CSR_TDATA3 0x7a3
> +#define CSR_TINFO 0x7a4
>
> /* Debug Mode Registers */
> #define CSR_DCSR 0x7b0
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index 76146f373a..9f69c64591 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -95,6 +95,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);
>
> +target_ulong tinfo_csr_read(CPURISCVState *env);
> +
> 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);
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 3d0d8e0340..e66019048d 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3089,6 +3089,13 @@ static RISCVException write_tdata(CPURISCVState *env, int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +static RISCVException read_tinfo(CPURISCVState *env, int csrno,
> + target_ulong *val)
> +{
> + *val = tinfo_csr_read(env);
> + return RISCV_EXCP_NONE;
> +}
> +
> /*
> * Functions to access Pointer Masking feature registers
> * We have to check if current priv lvl could modify
> @@ -3893,6 +3900,7 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> [CSR_TDATA1] = { "tdata1", debug, read_tdata, write_tdata },
> [CSR_TDATA2] = { "tdata2", debug, read_tdata, write_tdata },
> [CSR_TDATA3] = { "tdata3", debug, read_tdata, write_tdata },
> + [CSR_TINFO] = { "tinfo", debug, read_tinfo, write_ignore },
>
> /* User Pointer Masking */
> [CSR_UMTE] = { "umte", pointer_masking, read_umte, write_umte },
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index d6666164cd..7d546ace42 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -37,9 +37,7 @@
> * - tdata1
> * - tdata2
> * - tdata3
> - *
> - * We don't support writable 'type' field in the tdata1 register, so there is
> - * no need to implement the "tinfo" CSR.
> + * - tinfo
> *
> * The following triggers are implemented:
> *
> @@ -372,6 +370,12 @@ void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
> }
> }
>
> +target_ulong tinfo_csr_read(CPURISCVState *env)
> +{
> + /* assume all triggers support the same types of triggers */
> + return BIT(TRIGGER_TYPE_AD_MATCH);
> +}
> +
> void riscv_cpu_debug_excp_handler(CPUState *cs)
> {
> RISCVCPU *cpu = RISCV_CPU(cs);
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 6/8] target/riscv: debug: Create common trigger actions function
2022-09-09 13:42 ` [PATCH v2 6/8] target/riscv: debug: Create common trigger actions function Bin Meng
@ 2022-09-16 2:40 ` LIU Zhiwei
0 siblings, 0 replies; 17+ messages in thread
From: LIU Zhiwei @ 2022-09-16 2:40 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Zhiwei
On 2022/9/9 21:42, Bin Meng wrote:
> From: Frank Chang <frank.chang@sifive.com>
>
> Trigger actions are shared among all triggers. Extract to a common
> function.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> [bmeng: handle the DBG_ACTION_NONE case]
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
> Changes in v2:
> - add handling of the DBG_ACTION_NONE case in do_trigger_action()
>
> target/riscv/debug.h | 13 ++++++++++
> target/riscv/debug.c | 59 ++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 70 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index 9f69c64591..0e4859cf74 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -44,6 +44,19 @@ typedef enum {
> TRIGGER_TYPE_NUM
> } trigger_type_t;
>
> +/* actions */
> +typedef enum {
> + DBG_ACTION_NONE = -1, /* sentinel value */
> + DBG_ACTION_BP = 0,
> + DBG_ACTION_DBG_MODE,
> + DBG_ACTION_TRACE0,
> + DBG_ACTION_TRACE1,
> + DBG_ACTION_TRACE2,
> + DBG_ACTION_TRACE3,
> + DBG_ACTION_EXT_DBG0 = 8,
> + DBG_ACTION_EXT_DBG1
> +} trigger_action_t;
> +
> /* tdata1 field masks */
>
> #define RV32_TYPE(t) ((uint32_t)(t) << 28)
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 7d546ace42..7a8910f980 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -92,6 +92,37 @@ static inline target_ulong get_trigger_type(CPURISCVState *env,
> return extract_trigger_type(env, env->tdata1[trigger_index]);
> }
>
> +static trigger_action_t get_trigger_action(CPURISCVState *env,
> + target_ulong trigger_index)
> +{
> + target_ulong tdata1 = env->tdata1[trigger_index];
> + int trigger_type = get_trigger_type(env, trigger_index);
> + trigger_action_t action = DBG_ACTION_NONE;
> +
> + switch (trigger_type) {
> + case TRIGGER_TYPE_AD_MATCH:
> + action = (tdata1 & TYPE2_ACTION) >> 12;
> + break;
> + case TRIGGER_TYPE_INST_CNT:
> + case TRIGGER_TYPE_INT:
> + case TRIGGER_TYPE_EXCP:
> + case TRIGGER_TYPE_AD_MATCH6:
> + case TRIGGER_TYPE_EXT_SRC:
> + qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
> + trigger_type);
> + break;
> + case TRIGGER_TYPE_NO_EXIST:
> + case TRIGGER_TYPE_UNAVAIL:
> + qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
> + trigger_type);
> + break;
> + default:
> + g_assert_not_reached();
> + }
> +
> + return action;
> +}
> +
> static inline target_ulong build_tdata1(CPURISCVState *env,
> trigger_type_t type,
> bool dmode, target_ulong data)
> @@ -182,6 +213,30 @@ static inline void warn_always_zero_bit(target_ulong val, target_ulong mask,
> }
> }
>
> +static void do_trigger_action(CPURISCVState *env, target_ulong trigger_index)
> +{
> + trigger_action_t action = get_trigger_action(env, trigger_index);
> +
> + switch (action) {
> + case DBG_ACTION_NONE:
> + break;
> + case DBG_ACTION_BP:
> + riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
> + break;
> + case DBG_ACTION_DBG_MODE:
> + case DBG_ACTION_TRACE0:
> + case DBG_ACTION_TRACE1:
> + case DBG_ACTION_TRACE2:
> + case DBG_ACTION_TRACE3:
> + case DBG_ACTION_EXT_DBG0:
> + case DBG_ACTION_EXT_DBG1:
> + qemu_log_mask(LOG_UNIMP, "action: %d is not supported\n", action);
> + break;
> + default:
> + g_assert_not_reached();
> + }
> +}
> +
> /* type 2 trigger */
>
> static uint32_t type2_breakpoint_size(CPURISCVState *env, target_ulong ctrl)
> @@ -384,11 +439,11 @@ void riscv_cpu_debug_excp_handler(CPUState *cs)
> if (cs->watchpoint_hit) {
> if (cs->watchpoint_hit->flags & BP_CPU) {
> cs->watchpoint_hit = NULL;
> - riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
> + do_trigger_action(env, DBG_ACTION_BP);
> }
> } else {
> if (cpu_breakpoint_test(cs, env->pc, BP_CPU)) {
> - riscv_raise_exception(env, RISCV_EXCP_BREAKPOINT, 0);
> + do_trigger_action(env, DBG_ACTION_BP);
> }
> }
> }
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 1/8] target/riscv: debug: Determine the trigger type from tdata1.type
2022-09-09 13:42 ` [PATCH v2 1/8] target/riscv: debug: Determine the trigger type from tdata1.type Bin Meng
2022-09-16 1:53 ` LIU Zhiwei
@ 2022-09-16 2:42 ` LIU Zhiwei
1 sibling, 0 replies; 17+ messages in thread
From: LIU Zhiwei @ 2022-09-16 2:42 UTC (permalink / raw)
To: Bin Meng, qemu-devel
Cc: Frank Chang, Alistair Francis, Bin Meng, Palmer Dabbelt, qemu-riscv
On 2022/9/9 21:42, Bin Meng wrote:
> From: Frank Chang <frank.chang@sifive.com>
>
> Current RISC-V debug assumes that only type 2 trigger is supported.
> To allow more types of triggers to be supported in the future
> (e.g. type 6 trigger, which is similar to type 2 trigger with additional
> functionality), we should determine the trigger type from tdata1.type.
>
> RV_MAX_TRIGGERS is also introduced in replacement of TRIGGER_TYPE2_NUM.
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> [bmeng: fixed MXL_RV128 case, and moved macros to the following patch]
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
>
> ---
>
> Changes in v2:
> - fixed MXL_RV128 case
> - moved macros to patch#2
> - added log guest errors for TRIGGER_TYPE_{NO_EXIST,UNAVAIL}
>
> target/riscv/cpu.h | 2 +-
> target/riscv/debug.h | 13 +--
> target/riscv/csr.c | 2 +-
> target/riscv/debug.c | 188 +++++++++++++++++++++++++++++------------
> target/riscv/machine.c | 2 +-
> 5 files changed, 140 insertions(+), 67 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 06751e1e3e..4d82a3250b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -324,7 +324,7 @@ struct CPUArchState {
>
> /* trigger module */
> target_ulong trigger_cur;
> - type2_trigger_t type2_trig[TRIGGER_TYPE2_NUM];
> + type2_trigger_t type2_trig[RV_MAX_TRIGGERS];
>
> /* machine specific rdtime callback */
> uint64_t (*rdtime_fn)(void *);
> diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> index 27b9cac6b4..72e4edcd8c 100644
> --- a/target/riscv/debug.h
> +++ b/target/riscv/debug.h
> @@ -22,13 +22,7 @@
> #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
> -};
> +#define RV_MAX_TRIGGERS 2
>
> /* register index of tdata CSRs */
> enum {
> @@ -46,7 +40,8 @@ typedef enum {
> 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_UNAVAIL = 15, /* trigger exists, but unavailable */
> + TRIGGER_TYPE_NUM
> } trigger_type_t;
>
> typedef struct {
> @@ -56,7 +51,7 @@ typedef struct {
> struct CPUWatchpoint *wp;
> } type2_trigger_t;
>
> -/* tdata field masks */
> +/* tdata1 field masks */
>
> #define RV32_TYPE(t) ((uint32_t)(t) << 28)
> #define RV32_TYPE_MASK (0xf << 28)
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index b96db1b62b..3d0d8e0340 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3065,7 +3065,7 @@ 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) {
> + if (env->trigger_cur >= RV_MAX_TRIGGERS && csrno == CSR_TDATA1) {
> *val = 0;
> return RISCV_EXCP_NONE;
> }
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index fc6e13222f..9dd468753a 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -52,8 +52,15 @@
> /* 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 },
> +static tdata_avail tdata_mapping[TRIGGER_TYPE_NUM] = {
> + [TRIGGER_TYPE_NO_EXIST] = { false, false, false },
> + [TRIGGER_TYPE_AD_MATCH] = { true, true, true },
> + [TRIGGER_TYPE_INST_CNT] = { true, false, true },
> + [TRIGGER_TYPE_INT] = { true, true, true },
> + [TRIGGER_TYPE_EXCP] = { true, true, true },
> + [TRIGGER_TYPE_AD_MATCH6] = { true, true, true },
> + [TRIGGER_TYPE_EXT_SRC] = { true, false, false },
> + [TRIGGER_TYPE_UNAVAIL] = { true, true, true }
> };
>
> /* only breakpoint size 1/2/4/8 supported */
> @@ -67,6 +74,27 @@ static int access_size[SIZE_NUM] = {
> [6 ... 15] = -1,
> };
>
> +static inline target_ulong extract_trigger_type(CPURISCVState *env,
> + target_ulong tdata1)
> +{
> + switch (riscv_cpu_mxl(env)) {
> + case MXL_RV32:
> + return extract32(tdata1, 28, 4);
> + case MXL_RV64:
> + case MXL_RV128:
> + return extract64(tdata1, 60, 4);
> + default:
> + g_assert_not_reached();
> + }
> +}
> +
Maybe
get_type_from_tdata
> +static inline target_ulong get_trigger_type(CPURISCVState *env,
> + target_ulong trigger_index)
> +{
> + target_ulong tdata1 = env->type2_trig[trigger_index].mcontrol;
> + return extract_trigger_type(env, tdata1);
> +}
> +
and
get_type_from_index
> static inline target_ulong trigger_type(CPURISCVState *env,
> trigger_type_t type)
> {
> @@ -89,15 +117,17 @@ static inline target_ulong trigger_type(CPURISCVState *env,
>
> bool tdata_available(CPURISCVState *env, int tdata_index)
> {
> + int trigger_type = get_trigger_type(env, env->trigger_cur);
> +
> if (unlikely(tdata_index >= TDATA_NUM)) {
> return false;
> }
>
> - if (unlikely(env->trigger_cur >= TRIGGER_NUM)) {
> + if (unlikely(env->trigger_cur >= RV_MAX_TRIGGERS)) {
> return false;
> }
>
> - return tdata_mapping[env->trigger_cur][tdata_index];
> + return tdata_mapping[trigger_type][tdata_index];
> }
>
> target_ulong tselect_csr_read(CPURISCVState *env)
> @@ -137,6 +167,7 @@ static target_ulong tdata1_validate(CPURISCVState *env, target_ulong val,
> 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");
> }
> @@ -261,9 +292,8 @@ static void type2_breakpoint_remove(CPURISCVState *env, target_ulong index)
> }
>
> static target_ulong type2_reg_read(CPURISCVState *env,
> - target_ulong trigger_index, int tdata_index)
> + target_ulong index, int tdata_index)
> {
> - uint32_t index = trigger_index - TRIGGER_TYPE2_IDX_0;
> target_ulong tdata;
>
> switch (tdata_index) {
> @@ -280,10 +310,9 @@ static target_ulong type2_reg_read(CPURISCVState *env,
> return tdata;
> }
>
> -static void type2_reg_write(CPURISCVState *env, target_ulong trigger_index,
> +static void type2_reg_write(CPURISCVState *env, target_ulong index,
> int tdata_index, target_ulong val)
> {
> - uint32_t index = trigger_index - TRIGGER_TYPE2_IDX_0;
> target_ulong new_val;
>
> switch (tdata_index) {
> @@ -309,35 +338,64 @@ static void type2_reg_write(CPURISCVState *env, target_ulong trigger_index,
> 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];
> + int trigger_type = get_trigger_type(env, env->trigger_cur);
> +
> + switch (trigger_type) {
> + case TRIGGER_TYPE_AD_MATCH:
> + return type2_reg_read(env, env->trigger_cur, tdata_index);
> + break;
> + case TRIGGER_TYPE_INST_CNT:
> + case TRIGGER_TYPE_INT:
> + case TRIGGER_TYPE_EXCP:
> + case TRIGGER_TYPE_AD_MATCH6:
> + case TRIGGER_TYPE_EXT_SRC:
> + qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
> + trigger_type);
> + break;
> + case TRIGGER_TYPE_NO_EXIST:
> + case TRIGGER_TYPE_UNAVAIL:
> + qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
> + trigger_type);
> + break;
> + default:
> + g_assert_not_reached();
> + }
>
> - return read_func(env, env->trigger_cur, tdata_index);
> + return 0;
> }
>
> void tdata_csr_write(CPURISCVState *env, int tdata_index, target_ulong val)
> {
> - tdata_write_func write_func = trigger_write_funcs[env->trigger_cur];
> + int trigger_type;
>
> - return write_func(env, env->trigger_cur, tdata_index, val);
> + if (tdata_index == TDATA1) {
> + trigger_type = extract_trigger_type(env, val);
> + } else {
> + trigger_type = get_trigger_type(env, env->trigger_cur);
> + }
> +
> + switch (trigger_type) {
> + case TRIGGER_TYPE_AD_MATCH:
> + type2_reg_write(env, env->trigger_cur, tdata_index, val);
> + break;
> + case TRIGGER_TYPE_INST_CNT:
> + case TRIGGER_TYPE_INT:
> + case TRIGGER_TYPE_EXCP:
> + case TRIGGER_TYPE_AD_MATCH6:
> + case TRIGGER_TYPE_EXT_SRC:
> + qemu_log_mask(LOG_UNIMP, "trigger type: %d is not supported\n",
> + trigger_type);
> + break;
> + case TRIGGER_TYPE_NO_EXIST:
> + case TRIGGER_TYPE_UNAVAIL:
> + qemu_log_mask(LOG_GUEST_ERROR, "trigger type: %d does not exit\n",
> + trigger_type);
> + break;
> + default:
> + g_assert_not_reached();
> + }
> }
>
> void riscv_cpu_debug_excp_handler(CPUState *cs)
> @@ -364,18 +422,28 @@ bool riscv_cpu_debug_check_breakpoint(CPUState *cs)
> CPUBreakpoint *bp;
> target_ulong ctrl;
> target_ulong pc;
> + int trigger_type;
> 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;
> + for (i = 0; i < RV_MAX_TRIGGERS; i++) {
> + trigger_type = get_trigger_type(env, i);
> +
> + switch (trigger_type) {
> + case TRIGGER_TYPE_AD_MATCH:
> + 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;
> + }
> }
> + break;
> + default:
> + /* other trigger types are not supported or irrelevant */
> + break;
> }
> }
> }
> @@ -389,26 +457,36 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
> CPURISCVState *env = &cpu->env;
> target_ulong ctrl;
> target_ulong addr;
> + int trigger_type;
> 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;
> + for (i = 0; i < RV_MAX_TRIGGERS; i++) {
> + trigger_type = get_trigger_type(env, i);
>
> - if (ctrl & TYPE2_LOAD) {
> - flags |= BP_MEM_READ;
> - }
> - if (ctrl & TYPE2_STORE) {
> - flags |= BP_MEM_WRITE;
> - }
> + switch (trigger_type) {
> + case TRIGGER_TYPE_AD_MATCH:
> + ctrl = env->type2_trig[i].mcontrol;
> + addr = env->type2_trig[i].maddress;
> + flags = 0;
>
> - if ((wp->flags & flags) && (wp->vaddr == addr)) {
> - /* check U/S/M bit against current privilege level */
> - if ((ctrl >> 3) & BIT(env->priv)) {
> - return true;
> + 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;
> + }
> }
> + break;
> + default:
> + /* other trigger types are not supported */
> + break;
> }
> }
>
> @@ -417,11 +495,11 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
>
> void riscv_trigger_init(CPURISCVState *env)
> {
> - target_ulong type2 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
> + target_ulong tdata1 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
> int i;
>
> - /* type 2 triggers */
> - for (i = 0; i < TRIGGER_TYPE2_NUM; i++) {
> + /* init to type 2 triggers */
> + for (i = 0; i < RV_MAX_TRIGGERS; i++) {
> /*
> * type = TRIGGER_TYPE_AD_MATCH
> * dmode = 0 (both debug and M-mode can write tdata)
> @@ -435,7 +513,7 @@ void riscv_trigger_init(CPURISCVState *env)
> * 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].mcontrol = tdata1;
> env->type2_trig[i].maddress = 0;
> env->type2_trig[i].bp = NULL;
> env->type2_trig[i].wp = NULL;
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 41098f6ad0..b8173394a2 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -247,7 +247,7 @@ static const VMStateDescription vmstate_debug = {
> .needed = debug_needed,
> .fields = (VMStateField[]) {
> VMSTATE_UINTTL(env.trigger_cur, RISCVCPU),
> - VMSTATE_STRUCT_ARRAY(env.type2_trig, RISCVCPU, TRIGGER_TYPE2_NUM,
> + VMSTATE_STRUCT_ARRAY(env.type2_trig, RISCVCPU, RV_MAX_TRIGGERS,
> 0, vmstate_debug_type2, type2_trigger_t),
> VMSTATE_END_OF_LIST()
> }
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support
2022-09-09 13:42 [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support Bin Meng
` (7 preceding siblings ...)
2022-09-09 13:42 ` [PATCH v2 8/8] target/riscv: debug: Add initial support of type 6 trigger Bin Meng
@ 2022-09-23 4:46 ` Alistair Francis
8 siblings, 0 replies; 17+ messages in thread
From: Alistair Francis @ 2022-09-23 4:46 UTC (permalink / raw)
To: Bin Meng
Cc: qemu-devel@nongnu.org Developers, Alistair Francis, Bin Meng,
Palmer Dabbelt, open list:RISC-V
On Fri, Sep 9, 2022 at 11:48 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> This patchset refactors RISC-V Debug support to allow more types of
> triggers to be extended.
>
> The initial support of type 6 trigger, which is similar to type 2
> trigger with additional functionality, is also introduced in this
> patchset.
>
> This is a v2 respin of previous patch originally done by Frank Chang
> at SiFive. I've incorperated my review comments in v2 and rebased
> against QEMU mainline.
>
> Changes in v2:
> - fixed MXL_RV128 case
> - moved macros to patch#2
> - added log guest errors for TRIGGER_TYPE_{NO_EXIST,UNAVAIL}
> - moved RV{32,64}_DATA_MASK definition to this patch
> - add handling of the DBG_ACTION_NONE case in do_trigger_action()
> - drop patch: "target/riscv: debug: Return 0 if previous value written to tselect >= number of triggers"
>
> Frank Chang (8):
> target/riscv: debug: Determine the trigger type from tdata1.type
> target/riscv: debug: Introduce build_tdata1() to build tdata1 register
> content
> target/riscv: debug: Introduce tdata1, tdata2, and tdata3 CSRs
> target/riscv: debug: Restrict the range of tselect value can be
> written
> target/riscv: debug: Introduce tinfo CSR
> target/riscv: debug: Create common trigger actions function
> target/riscv: debug: Check VU/VS modes for type 2 trigger
> target/riscv: debug: Add initial support of type 6 trigger
Thanks!
Applied to riscv-to-apply.next
Alistair
>
> target/riscv/cpu.h | 6 +-
> target/riscv/cpu_bits.h | 1 +
> target/riscv/debug.h | 55 +++--
> target/riscv/csr.c | 10 +-
> target/riscv/debug.c | 484 ++++++++++++++++++++++++++++++++--------
> target/riscv/machine.c | 20 +-
> 6 files changed, 445 insertions(+), 131 deletions(-)
>
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-09-23 4:49 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-09 13:42 [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support Bin Meng
2022-09-09 13:42 ` [PATCH v2 1/8] target/riscv: debug: Determine the trigger type from tdata1.type Bin Meng
2022-09-16 1:53 ` LIU Zhiwei
2022-09-16 2:42 ` LIU Zhiwei
2022-09-09 13:42 ` [PATCH v2 2/8] target/riscv: debug: Introduce build_tdata1() to build tdata1 register content Bin Meng
2022-09-16 1:55 ` LIU Zhiwei
2022-09-09 13:42 ` [PATCH v2 3/8] target/riscv: debug: Introduce tdata1, tdata2, and tdata3 CSRs Bin Meng
2022-09-16 1:58 ` LIU Zhiwei
2022-09-09 13:42 ` [PATCH v2 4/8] target/riscv: debug: Restrict the range of tselect value can be written Bin Meng
2022-09-16 1:59 ` LIU Zhiwei
2022-09-09 13:42 ` [PATCH v2 5/8] target/riscv: debug: Introduce tinfo CSR Bin Meng
2022-09-16 2:26 ` LIU Zhiwei
2022-09-09 13:42 ` [PATCH v2 6/8] target/riscv: debug: Create common trigger actions function Bin Meng
2022-09-16 2:40 ` LIU Zhiwei
2022-09-09 13:42 ` [PATCH v2 7/8] target/riscv: debug: Check VU/VS modes for type 2 trigger Bin Meng
2022-09-09 13:42 ` [PATCH v2 8/8] target/riscv: debug: Add initial support of type 6 trigger Bin Meng
2022-09-23 4:46 ` [PATCH v2 0/8] target/riscv: Improve RISC-V Debug support 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.