On Wed, Jan 19, 2022 at 11:06 AM Alistair Francis <alistai...@gmail.com> wrote:
>
> On Wed, Jan 5, 2022 at 1:15 PM Bin Meng <bmeng...@gmail.com> wrote:
> >
> > From: Bin Meng <bin.m...@windriver.com>
> >
> > This adds debug CSR read/write support to the RISC-V CSR RW table.
> >
> > Signed-off-by: Bin Meng <bin.m...@windriver.com>
> > ---
> >
> > Changes in v3:
> > - add riscv_trigger_init(), moved from patch #1 to this patch
> >
> > target/riscv/debug.h | 2 ++
> > target/riscv/cpu.c | 6 +++++
> > target/riscv/csr.c | 57 ++++++++++++++++++++++++++++++++++++++++++++
> > target/riscv/debug.c | 27 +++++++++++++++++++++
> > 4 files changed, 92 insertions(+)
> >
> > diff --git a/target/riscv/debug.h b/target/riscv/debug.h
> > index d0f63e2414..f4da2db35d 100644
> > --- a/target/riscv/debug.h
> > +++ b/target/riscv/debug.h
> > @@ -109,4 +109,6 @@ void riscv_cpu_debug_excp_handler(CPUState *cs);
> > bool riscv_cpu_debug_check_breakpoint(CPUState *cs);
> > bool riscv_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp);
> >
> > +void riscv_trigger_init(CPURISCVState *env);
> > +
> > #endif /* RISCV_DEBUG_H */
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index d36c31ce9a..17dcc3c14f 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -575,6 +575,12 @@ static void riscv_cpu_realize(DeviceState *dev, Error
> > **errp)
> >
> > riscv_cpu_register_gdb_regs_for_features(cs);
> >
> > +#ifndef CONFIG_USER_ONLY
> > + if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> > + riscv_trigger_init(env);
> > + }
> > +#endif
> > +
> > qemu_init_vcpu(cs);
> > cpu_reset(cs);
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index 146447eac5..189b9cc8c6 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -220,6 +220,15 @@ static RISCVException epmp(CPURISCVState *env, int
> > csrno)
> >
> > return RISCV_EXCP_ILLEGAL_INST;
> > }
> > +
> > +static RISCVException debug(CPURISCVState *env, int csrno)
> > +{
> > + if (riscv_feature(env, RISCV_FEATURE_DEBUG)) {
> > + return RISCV_EXCP_NONE;
> > + }
> > +
> > + return RISCV_EXCP_ILLEGAL_INST;
> > +}
> > #endif
> >
> > /* User Floating-Point CSRs */
> > @@ -1464,6 +1473,48 @@ static RISCVException write_pmpaddr(CPURISCVState
> > *env, int csrno,
> > return RISCV_EXCP_NONE;
> > }
> >
> > +static RISCVException read_tselect(CPURISCVState *env, int csrno,
> > + target_ulong *val)
> > +{
> > + *val = tselect_csr_read(env);
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_tselect(CPURISCVState *env, int csrno,
> > + target_ulong val)
> > +{
> > + tselect_csr_write(env, val);
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_tdata(CPURISCVState *env, int csrno,
> > + target_ulong *val)
> > +{
> > + /* return 0 in tdata1 to end the trigger enumeration */
> > + if (env->trigger_cur >= TRIGGER_NUM && csrno == CSR_TDATA1) {
> > + *val = 0;
> > + return RISCV_EXCP_NONE;
> > + }
> > +
> > + if (!tdata_available(env, csrno - CSR_TDATA1)) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > +
> > + *val = tdata_csr_read(env, csrno - CSR_TDATA1);
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_tdata(CPURISCVState *env, int csrno,
> > + target_ulong val)
> > +{
> > + if (!tdata_available(env, csrno - CSR_TDATA1)) {
> > + return RISCV_EXCP_ILLEGAL_INST;
> > + }
> > +
> > + tdata_csr_write(env, csrno - CSR_TDATA1, val);
> > + return RISCV_EXCP_NONE;
> > +}
> > +
> > /*
> > * Functions to access Pointer Masking feature registers
> > * We have to check if current priv lvl could modify
> > @@ -1962,6 +2013,12 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> > [CSR_PMPADDR14] = { "pmpaddr14", pmp, read_pmpaddr, write_pmpaddr },
> > [CSR_PMPADDR15] = { "pmpaddr15", pmp, read_pmpaddr, write_pmpaddr },
> >
> > + /* Debug CSRs */
> > + [CSR_TSELECT] = { "tselect", debug, read_tselect, write_tselect },
> > + [CSR_TDATA1] = { "tdata1", debug, read_tdata, write_tdata },
> > + [CSR_TDATA2] = { "tdata2", debug, read_tdata, write_tdata },
> > + [CSR_TDATA3] = { "tdata3", debug, read_tdata, write_tdata },
> > +
> > /* User Pointer Masking */
> > [CSR_UMTE] = { "umte", pointer_masking, read_umte,
> > write_umte },
> > [CSR_UPMMASK] = { "upmmask", pointer_masking, read_upmmask,
> > write_upmmask },
> > diff --git a/target/riscv/debug.c b/target/riscv/debug.c
> > index 7760c4611f..041a0d3a89 100644
> > --- a/target/riscv/debug.c
> > +++ b/target/riscv/debug.c
> > @@ -412,3 +412,30 @@ bool riscv_cpu_debug_check_watchpoint(CPUState *cs,
> > CPUWatchpoint *wp)
> >
> > return false;
> > }
> > +
> > +void riscv_trigger_init(CPURISCVState *env)
> > +{
> > + target_ulong type2 = trigger_type(env, TRIGGER_TYPE_AD_MATCH);
> > + int i;
> > +
> > + /* type 2 triggers */
> > + for (i = 0; i < TRIGGER_TYPE2_NUM; i++) {
> > + /*
> > + * type = TRIGGER_TYPE_AD_MATCH
> > + * dmode = 0 (both debug and M-mode can write tdata)
> > + * maskmax = 0 (unimplemented, always 0)
> > + * sizehi = 0 (match against any size, RV64 only)
> > + * hit = 0 (unimplemented, always 0)
> > + * select = 0 (always 0, perform match on address)
> > + * timing = 0 (always 0, trigger before instruction)
> > + * sizelo = 0 (match against any size)
> > + * action = 0 (always 0, raise a breakpoint exception)
> > + * chain = 0 (unimplemented, always 0)
> > + * match = 0 (always 0, when any compare value equals tdata2)
> > + */
> > + env->trigger_type2[i].mcontrol = type2;
> > + env->trigger_type2[i].maddress = 0;
> > + env->trigger_type2[i].bp = NULL;
> > + env->trigger_type2[i].wp = NULL;
> > + }
>
> Should this be called at reset instead?
Yes, I think so.
Regards,
Bin
