| Message ID | 20220610051328.7078-4-frank.chang@sifive.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Improve RISC-V Debug support | expand |
On Fri, Jun 10, 2022 at 1:15 PM <frank.chang@sifive.com> 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> > --- > 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, &env->type2_trig[index].bp); > + cpu_breakpoint_insert(cs, addr, flags, &env->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, > - &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]); > } > } > } > @@ -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; > 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(); > } > @@ -344,28 +330,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: > - 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) > @@ -431,8 +405,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 */ > @@ -466,8 +440,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) { > @@ -513,9 +487,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/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/machine.c b/target/riscv/machine.c > index 54e523c26c..a1db8b9559 100644 > --- a/target/riscv/machine.c > +++ b/target/riscv/machine.c > @@ -228,26 +228,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() > } > }; > -- > Regards, Bin
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, &env->type2_trig[index].bp); + cpu_breakpoint_insert(cs, addr, flags, &env->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, - &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]); } } } @@ -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; 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(); } @@ -344,28 +330,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: - 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) @@ -431,8 +405,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 */ @@ -466,8 +440,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) { @@ -513,9 +487,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/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/machine.c b/target/riscv/machine.c index 54e523c26c..a1db8b9559 100644 --- a/target/riscv/machine.c +++ b/target/riscv/machine.c @@ -228,26 +228,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() } };