* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).