Re: [PATCH] target/riscv: Implement optional CSR mcontext of debug Sdtrig extension

[Alistair Francis]

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
>
>