Re: [PATCH 3/9] target/riscv: debug: Introduce tdata1, tdata2, and tdata3 CSRs
On Fri, Jun 10, 2022 at 1:15 PM wrote:
>
> From: Frank Chang
>
> 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
> ---
> target/riscv/cpu.h | 6 ++-
> target/riscv/debug.c | 101 -
> target/riscv/debug.h | 7 ---
> target/riscv/machine.c | 20 ++--
> 4 files changed, 48 insertions(+), 86 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 535123a989..bac5f00722 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -289,7 +289,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];
I believe the breakpoint and watchpoint here does not make sense to
every type of trigger. It only makes sense to type 2, 6. Type 3 only
has breakpoint.
>
> /* machine specific rdtime callback */
> uint64_t (*rdtime_fn)(void *);
> diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> index 089aae0696..6913682f75 100644
> --- a/target/riscv/debug.c
> +++ b/target/riscv/debug.c
> @@ -90,8 +90,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,
> @@ -187,6 +186,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;
> @@ -246,8 +247,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;
> @@ -258,7 +259,7 @@ static void type2_breakpoint_insert(CPURISCVState *env,
> target_ulong index)
> }
>
> if (ctrl & TYPE2_EXEC) {
> -cpu_breakpoint_insert(cs, addr, flags, >type2_trig[index].bp);
> +cpu_breakpoint_insert(cs, addr, flags, >cpu_breakpoint[index]);
> }
>
> if (ctrl & TYPE2_LOAD) {
> @@ -272,10 +273,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,
> - >type2_trig[index].wp);
> + >cpu_watchpoint[index]);
> } else {
> cpu_watchpoint_insert(cs, addr, 8, flags,
> - >type2_trig[index].wp);
> + >cpu_watchpoint[index]);
> }
> }
> }
> @@ -284,34 +285,15 @@ 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;
> -}
> -}
> -
> -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();
> +if (env->cpu_watchpoint[index]) {
> +cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
> +env->cpu_watchpoint[index] = NULL;
> }
> -
> -return tdata;
> }
>
> static void
[PATCH 3/9] target/riscv: debug: Introduce tdata1, tdata2, and tdata3 CSRs
From: Frank Chang
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
---
target/riscv/cpu.h | 6 ++-
target/riscv/debug.c | 101 -
target/riscv/debug.h | 7 ---
target/riscv/machine.c | 20 ++--
4 files changed, 48 insertions(+), 86 deletions(-)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 535123a989..bac5f00722 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -289,7 +289,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.c b/target/riscv/debug.c
index 089aae0696..6913682f75 100644
--- a/target/riscv/debug.c
+++ b/target/riscv/debug.c
@@ -90,8 +90,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,
@@ -187,6 +186,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;
@@ -246,8 +247,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;
@@ -258,7 +259,7 @@ static void type2_breakpoint_insert(CPURISCVState *env,
target_ulong index)
}
if (ctrl & TYPE2_EXEC) {
-cpu_breakpoint_insert(cs, addr, flags, >type2_trig[index].bp);
+cpu_breakpoint_insert(cs, addr, flags, >cpu_breakpoint[index]);
}
if (ctrl & TYPE2_LOAD) {
@@ -272,10 +273,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,
- >type2_trig[index].wp);
+ >cpu_watchpoint[index]);
} else {
cpu_watchpoint_insert(cs, addr, 8, flags,
- >type2_trig[index].wp);
+ >cpu_watchpoint[index]);
}
}
}
@@ -284,34 +285,15 @@ 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;
-}
-}
-
-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();
+if (env->cpu_watchpoint[index]) {
+cpu_watchpoint_remove_by_ref(cs, env->cpu_watchpoint[index]);
+env->cpu_watchpoint[index] = NULL;
}
-
-return tdata;
}
static void type2_reg_write(CPURISCVState *env, target_ulong index,
@@ -322,19 +304,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;
