| Message ID | 20231217071628.151599-1-alvinga@andestech.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/riscv: Implement optional CSR mcontext of debug Sdtrig extension | expand |
On Sun, Dec 17, 2023 at 5:17 PM Alvin Chang via <qemu-devel@nongnu.org> wrote: > > The debug Sdtrig extension defines an optional CSR "mcontext". Since it > is optional, this commit adds new CPU configuration > "ext_sdtrig_mcontext" and uses property "sdtrig_mcontext" to control > whether it is implemented or not. Its predicate and read/write > operations are also implemented into CSR table. Its value is reset as 0 > when the trigger module is reset. We don't support the Sdtrig extension though. I'm guessing it's all packaged up as part of the "debug" extension but should we expose Sdtrig before we expose options for it? Also, why can't we just always implement mcontext if Sdtrig exists? Is there a reason to gate it behind a config? Alistair > > Signed-off-by: Alvin Chang <alvinga@andestech.com> > --- > target/riscv/cpu.c | 4 ++++ > target/riscv/cpu.h | 1 + > target/riscv/cpu_bits.h | 7 +++++++ > target/riscv/cpu_cfg.h | 1 + > target/riscv/csr.c | 36 ++++++++++++++++++++++++++++++++++++ > target/riscv/debug.c | 2 ++ > 6 files changed, 51 insertions(+) > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > index 83c7c0c..dff757f 100644 > --- a/target/riscv/cpu.c > +++ b/target/riscv/cpu.c > @@ -1479,6 +1479,10 @@ Property riscv_cpu_options[] = { > DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64), > DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64), > > + /* Optional CSR of debug Sdtrig extension */ > + DEFINE_PROP_BOOL("sdtrig_mcontext", RISCVCPU, cfg.ext_sdtrig_mcontext, > + false), > + > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index d74b361..e117641 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -345,6 +345,7 @@ struct CPUArchState { > target_ulong tdata1[RV_MAX_TRIGGERS]; > target_ulong tdata2[RV_MAX_TRIGGERS]; > target_ulong tdata3[RV_MAX_TRIGGERS]; > + target_ulong mcontext; > struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS]; > struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS]; > QEMUTimer *itrigger_timer[RV_MAX_TRIGGERS]; > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h > index ebd7917..3296648 100644 > --- a/target/riscv/cpu_bits.h > +++ b/target/riscv/cpu_bits.h > @@ -361,6 +361,7 @@ > #define CSR_TDATA2 0x7a2 > #define CSR_TDATA3 0x7a3 > #define CSR_TINFO 0x7a4 > +#define CSR_MCONTEXT 0x7a8 > > /* Debug Mode Registers */ > #define CSR_DCSR 0x7b0 > @@ -905,4 +906,10 @@ typedef enum RISCVException { > /* JVT CSR bits */ > #define JVT_MODE 0x3F > #define JVT_BASE (~0x3F) > + > +/* Debug Sdtrig CSR masks */ > +#define MCONTEXT32 0x0000003F > +#define MCONTEXT64 0x0000000000001FFFULL > +#define MCONTEXT32_HCONTEXT 0x0000007F > +#define MCONTEXT64_HCONTEXT 0x0000000000003FFFULL > #endif > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h > index f4605fb..4f1cb04 100644 > --- a/target/riscv/cpu_cfg.h > +++ b/target/riscv/cpu_cfg.h > @@ -147,6 +147,7 @@ struct RISCVCPUConfig { > bool pmp; > bool debug; > bool misa_w; > + bool ext_sdtrig_mcontext; > > bool short_isa_string; > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index fde7ce1..0b68787 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -549,6 +549,15 @@ static RISCVException debug(CPURISCVState *env, int csrno) > > return RISCV_EXCP_ILLEGAL_INST; > } > + > +static RISCVException sdtrig_mcontext(CPURISCVState *env, int csrno) > +{ > + if (riscv_cpu_cfg(env)->debug && riscv_cpu_cfg(env)->ext_sdtrig_mcontext) { > + return RISCV_EXCP_NONE; > + } > + > + return RISCV_EXCP_ILLEGAL_INST; > +} > #endif > > static RISCVException seed(CPURISCVState *env, int csrno) > @@ -3900,6 +3909,31 @@ static RISCVException read_tinfo(CPURISCVState *env, int csrno, > return RISCV_EXCP_NONE; > } > > +static RISCVException read_mcontext(CPURISCVState *env, int csrno, > + target_ulong *val) > +{ > + *val = env->mcontext; > + return RISCV_EXCP_NONE; > +} > + > +static RISCVException write_mcontext(CPURISCVState *env, int csrno, > + target_ulong val) > +{ > + bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false; > + int32_t mask; > + > + if (riscv_has_ext(env, RVH)) { > + /* Spec suggest 7-bit for RV32 and 14-bit for RV64 w/ H extension */ > + mask = rv32 ? MCONTEXT32_HCONTEXT : MCONTEXT64_HCONTEXT; > + } else { > + /* Spec suggest 6-bit for RV32 and 13-bit for RV64 w/o H extension */ > + mask = rv32 ? MCONTEXT32 : MCONTEXT64; > + } > + > + env->mcontext = val & mask; > + return RISCV_EXCP_NONE; > +} > + > /* > * Functions to access Pointer Masking feature registers > * We have to check if current priv lvl could modify > @@ -4799,6 +4833,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = { > [CSR_TDATA2] = { "tdata2", debug, read_tdata, write_tdata }, > [CSR_TDATA3] = { "tdata3", debug, read_tdata, write_tdata }, > [CSR_TINFO] = { "tinfo", debug, read_tinfo, write_ignore }, > + [CSR_MCONTEXT] = { "mcontext", sdtrig_mcontext, read_mcontext, > + write_mcontext }, > > /* 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 4945d1a..e30d99c 100644 > --- a/target/riscv/debug.c > +++ b/target/riscv/debug.c > @@ -940,4 +940,6 @@ void riscv_trigger_reset_hold(CPURISCVState *env) > env->cpu_watchpoint[i] = NULL; > timer_del(env->itrigger_timer[i]); > } > + > + env->mcontext = 0; > } > -- > 2.34.1 > >
> -----Original Message----- > From: Alistair Francis <alistair23@gmail.com> > Sent: Monday, December 18, 2023 12:46 PM > To: Alvin Che-Chia Chang(張哲嘉) <alvinga@andestech.com> > Cc: qemu-riscv@nongnu.org; qemu-devel@nongnu.org; > alistair.francis@wdc.com; bin.meng@windriver.com; liwei1518@gmail.com; > dbarboza@ventanamicro.com; zhiwei_liu@linux.alibaba.com > Subject: Re: [PATCH] target/riscv: Implement optional CSR mcontext of debug > Sdtrig extension > > On Sun, Dec 17, 2023 at 5:17 PM Alvin Chang via <qemu-devel@nongnu.org> > wrote: > > > > The debug Sdtrig extension defines an optional CSR "mcontext". Since > > it is optional, this commit adds new CPU configuration > > "ext_sdtrig_mcontext" and uses property "sdtrig_mcontext" to control > > whether it is implemented or not. Its predicate and read/write > > operations are also implemented into CSR table. Its value is reset as > > 0 when the trigger module is reset. > > We don't support the Sdtrig extension though. I'm guessing it's all packaged up > as part of the "debug" extension but should we expose Sdtrig before we expose > options for it? Yes, Sdtrig extension is part of the "debug" extension. There have been several trigger implementations in target/riscv/debug.{c|h}. I can rename it to cfg.debug_mcontext instead. > > Also, why can't we just always implement mcontext if Sdtrig exists? Is there a > reason to gate it behind a config? > > Alistair I gate it behind a config because spec says that mcontext is optional CSR. Some CPUs might implement it (so they can say cfg->ext_sdtrig_mcontext = true; in their CPU init), while others don't. Or do you think we should always implement it? Alvin > > > > > Signed-off-by: Alvin Chang <alvinga@andestech.com> > > --- > > target/riscv/cpu.c | 4 ++++ > > target/riscv/cpu.h | 1 + > > target/riscv/cpu_bits.h | 7 +++++++ > > target/riscv/cpu_cfg.h | 1 + > > target/riscv/csr.c | 36 ++++++++++++++++++++++++++++++++++++ > > target/riscv/debug.c | 2 ++ > > 6 files changed, 51 insertions(+) > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index > > 83c7c0c..dff757f 100644 > > --- a/target/riscv/cpu.c > > +++ b/target/riscv/cpu.c > > @@ -1479,6 +1479,10 @@ Property riscv_cpu_options[] = { > > DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, > cfg.cbom_blocksize, 64), > > DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, > > cfg.cboz_blocksize, 64), > > > > + /* Optional CSR of debug Sdtrig extension */ > > + DEFINE_PROP_BOOL("sdtrig_mcontext", RISCVCPU, > cfg.ext_sdtrig_mcontext, > > + false), > > + > > DEFINE_PROP_END_OF_LIST(), > > }; > > > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index > > d74b361..e117641 100644 > > --- a/target/riscv/cpu.h > > +++ b/target/riscv/cpu.h > > @@ -345,6 +345,7 @@ struct CPUArchState { > > target_ulong tdata1[RV_MAX_TRIGGERS]; > > target_ulong tdata2[RV_MAX_TRIGGERS]; > > target_ulong tdata3[RV_MAX_TRIGGERS]; > > + target_ulong mcontext; > > struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS]; > > struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS]; > > QEMUTimer *itrigger_timer[RV_MAX_TRIGGERS]; diff --git > > a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h index > > ebd7917..3296648 100644 > > --- a/target/riscv/cpu_bits.h > > +++ b/target/riscv/cpu_bits.h > > @@ -361,6 +361,7 @@ > > #define CSR_TDATA2 0x7a2 > > #define CSR_TDATA3 0x7a3 > > #define CSR_TINFO 0x7a4 > > +#define CSR_MCONTEXT 0x7a8 > > > > /* Debug Mode Registers */ > > #define CSR_DCSR 0x7b0 > > @@ -905,4 +906,10 @@ typedef enum RISCVException { > > /* JVT CSR bits */ > > #define JVT_MODE 0x3F > > #define JVT_BASE (~0x3F) > > + > > +/* Debug Sdtrig CSR masks */ > > +#define MCONTEXT32 0x0000003F > > +#define MCONTEXT64 > 0x0000000000001FFFULL > > +#define MCONTEXT32_HCONTEXT 0x0000007F > > +#define MCONTEXT64_HCONTEXT > 0x0000000000003FFFULL > > #endif > > diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h index > > f4605fb..4f1cb04 100644 > > --- a/target/riscv/cpu_cfg.h > > +++ b/target/riscv/cpu_cfg.h > > @@ -147,6 +147,7 @@ struct RISCVCPUConfig { > > bool pmp; > > bool debug; > > bool misa_w; > > + bool ext_sdtrig_mcontext; > > > > bool short_isa_string; > > > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c index > > fde7ce1..0b68787 100644 > > --- a/target/riscv/csr.c > > +++ b/target/riscv/csr.c > > @@ -549,6 +549,15 @@ static RISCVException debug(CPURISCVState *env, > > int csrno) > > > > return RISCV_EXCP_ILLEGAL_INST; > > } > > + > > +static RISCVException sdtrig_mcontext(CPURISCVState *env, int csrno) > > +{ > > + if (riscv_cpu_cfg(env)->debug && > riscv_cpu_cfg(env)->ext_sdtrig_mcontext) { > > + return RISCV_EXCP_NONE; > > + } > > + > > + return RISCV_EXCP_ILLEGAL_INST; > > +} > > #endif > > > > static RISCVException seed(CPURISCVState *env, int csrno) @@ -3900,6 > > +3909,31 @@ static RISCVException read_tinfo(CPURISCVState *env, int > csrno, > > return RISCV_EXCP_NONE; > > } > > > > +static RISCVException read_mcontext(CPURISCVState *env, int csrno, > > + target_ulong *val) { > > + *val = env->mcontext; > > + return RISCV_EXCP_NONE; > > +} > > + > > +static RISCVException write_mcontext(CPURISCVState *env, int csrno, > > + target_ulong val) { > > + bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false; > > + int32_t mask; > > + > > + if (riscv_has_ext(env, RVH)) { > > + /* Spec suggest 7-bit for RV32 and 14-bit for RV64 w/ H extension > */ > > + mask = rv32 ? MCONTEXT32_HCONTEXT : > MCONTEXT64_HCONTEXT; > > + } else { > > + /* Spec suggest 6-bit for RV32 and 13-bit for RV64 w/o H > extension */ > > + mask = rv32 ? MCONTEXT32 : MCONTEXT64; > > + } > > + > > + env->mcontext = val & mask; > > + return RISCV_EXCP_NONE; > > +} > > + > > /* > > * Functions to access Pointer Masking feature registers > > * We have to check if current priv lvl could modify @@ -4799,6 > > +4833,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = { > > [CSR_TDATA2] = { "tdata2", debug, read_tdata, > write_tdata }, > > [CSR_TDATA3] = { "tdata3", debug, read_tdata, > write_tdata }, > > [CSR_TINFO] = { "tinfo", debug, read_tinfo, > write_ignore }, > > + [CSR_MCONTEXT] = { "mcontext", sdtrig_mcontext, > read_mcontext, > > + > write_mcontext }, > > > > /* 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 > > 4945d1a..e30d99c 100644 > > --- a/target/riscv/debug.c > > +++ b/target/riscv/debug.c > > @@ -940,4 +940,6 @@ void riscv_trigger_reset_hold(CPURISCVState *env) > > env->cpu_watchpoint[i] = NULL; > > timer_del(env->itrigger_timer[i]); > > } > > + > > + env->mcontext = 0; > > } > > -- > > 2.34.1 > > > >
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 83c7c0c..dff757f 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -1479,6 +1479,10 @@ Property riscv_cpu_options[] = { DEFINE_PROP_UINT16("cbom_blocksize", RISCVCPU, cfg.cbom_blocksize, 64), DEFINE_PROP_UINT16("cboz_blocksize", RISCVCPU, cfg.cboz_blocksize, 64), + /* Optional CSR of debug Sdtrig extension */ + DEFINE_PROP_BOOL("sdtrig_mcontext", RISCVCPU, cfg.ext_sdtrig_mcontext, + false), + DEFINE_PROP_END_OF_LIST(), }; diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index d74b361..e117641 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -345,6 +345,7 @@ struct CPUArchState { target_ulong tdata1[RV_MAX_TRIGGERS]; target_ulong tdata2[RV_MAX_TRIGGERS]; target_ulong tdata3[RV_MAX_TRIGGERS]; + target_ulong mcontext; struct CPUBreakpoint *cpu_breakpoint[RV_MAX_TRIGGERS]; struct CPUWatchpoint *cpu_watchpoint[RV_MAX_TRIGGERS]; QEMUTimer *itrigger_timer[RV_MAX_TRIGGERS]; diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h index ebd7917..3296648 100644 --- a/target/riscv/cpu_bits.h +++ b/target/riscv/cpu_bits.h @@ -361,6 +361,7 @@ #define CSR_TDATA2 0x7a2 #define CSR_TDATA3 0x7a3 #define CSR_TINFO 0x7a4 +#define CSR_MCONTEXT 0x7a8 /* Debug Mode Registers */ #define CSR_DCSR 0x7b0 @@ -905,4 +906,10 @@ typedef enum RISCVException { /* JVT CSR bits */ #define JVT_MODE 0x3F #define JVT_BASE (~0x3F) + +/* Debug Sdtrig CSR masks */ +#define MCONTEXT32 0x0000003F +#define MCONTEXT64 0x0000000000001FFFULL +#define MCONTEXT32_HCONTEXT 0x0000007F +#define MCONTEXT64_HCONTEXT 0x0000000000003FFFULL #endif diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h index f4605fb..4f1cb04 100644 --- a/target/riscv/cpu_cfg.h +++ b/target/riscv/cpu_cfg.h @@ -147,6 +147,7 @@ struct RISCVCPUConfig { bool pmp; bool debug; bool misa_w; + bool ext_sdtrig_mcontext; bool short_isa_string; diff --git a/target/riscv/csr.c b/target/riscv/csr.c index fde7ce1..0b68787 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -549,6 +549,15 @@ static RISCVException debug(CPURISCVState *env, int csrno) return RISCV_EXCP_ILLEGAL_INST; } + +static RISCVException sdtrig_mcontext(CPURISCVState *env, int csrno) +{ + if (riscv_cpu_cfg(env)->debug && riscv_cpu_cfg(env)->ext_sdtrig_mcontext) { + return RISCV_EXCP_NONE; + } + + return RISCV_EXCP_ILLEGAL_INST; +} #endif static RISCVException seed(CPURISCVState *env, int csrno) @@ -3900,6 +3909,31 @@ static RISCVException read_tinfo(CPURISCVState *env, int csrno, return RISCV_EXCP_NONE; } +static RISCVException read_mcontext(CPURISCVState *env, int csrno, + target_ulong *val) +{ + *val = env->mcontext; + return RISCV_EXCP_NONE; +} + +static RISCVException write_mcontext(CPURISCVState *env, int csrno, + target_ulong val) +{ + bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false; + int32_t mask; + + if (riscv_has_ext(env, RVH)) { + /* Spec suggest 7-bit for RV32 and 14-bit for RV64 w/ H extension */ + mask = rv32 ? MCONTEXT32_HCONTEXT : MCONTEXT64_HCONTEXT; + } else { + /* Spec suggest 6-bit for RV32 and 13-bit for RV64 w/o H extension */ + mask = rv32 ? MCONTEXT32 : MCONTEXT64; + } + + env->mcontext = val & mask; + return RISCV_EXCP_NONE; +} + /* * Functions to access Pointer Masking feature registers * We have to check if current priv lvl could modify @@ -4799,6 +4833,8 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = { [CSR_TDATA2] = { "tdata2", debug, read_tdata, write_tdata }, [CSR_TDATA3] = { "tdata3", debug, read_tdata, write_tdata }, [CSR_TINFO] = { "tinfo", debug, read_tinfo, write_ignore }, + [CSR_MCONTEXT] = { "mcontext", sdtrig_mcontext, read_mcontext, + write_mcontext }, /* 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 4945d1a..e30d99c 100644 --- a/target/riscv/debug.c +++ b/target/riscv/debug.c @@ -940,4 +940,6 @@ void riscv_trigger_reset_hold(CPURISCVState *env) env->cpu_watchpoint[i] = NULL; timer_del(env->itrigger_timer[i]); } + + env->mcontext = 0; }
The debug Sdtrig extension defines an optional CSR "mcontext". Since it is optional, this commit adds new CPU configuration "ext_sdtrig_mcontext" and uses property "sdtrig_mcontext" to control whether it is implemented or not. Its predicate and read/write operations are also implemented into CSR table. Its value is reset as 0 when the trigger module is reset. Signed-off-by: Alvin Chang <alvinga@andestech.com> --- target/riscv/cpu.c | 4 ++++ target/riscv/cpu.h | 1 + target/riscv/cpu_bits.h | 7 +++++++ target/riscv/cpu_cfg.h | 1 + target/riscv/csr.c | 36 ++++++++++++++++++++++++++++++++++++ target/riscv/debug.c | 2 ++ 6 files changed, 51 insertions(+)