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

[Alistair Francis]

On Tue, Dec 19, 2023 at 10:34 PM Alvin Chang via <qemu-devel@nongnu.org> wrote:
>
> The debug Sdtrig extension defines an CSR "mcontext". This commit
> implements its predicate and read/write operations into CSR table.
> Its value is reset as 0 when the trigger module is reset.
>
> Signed-off-by: Alvin Chang <alvinga@andestech.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
> Changes from v1: Remove dedicated cfg, always implement mcontext.
>
>  target/riscv/cpu.h      |  1 +
>  target/riscv/cpu_bits.h |  7 +++++++
>  target/riscv/csr.c      | 36 +++++++++++++++++++++++++++++++-----
>  target/riscv/debug.c    |  2 ++
>  4 files changed, 41 insertions(+), 5 deletions(-)
>
> 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/csr.c b/target/riscv/csr.c
> index fde7ce1..ff1e128 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -3900,6 +3900,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
> @@ -4794,11 +4819,12 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>      [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   },
> -    [CSR_TINFO]     =  { "tinfo",   debug, read_tinfo,   write_ignore  },
> +    [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    },
> +    [CSR_TINFO]     =  { "tinfo",    debug, read_tinfo,    write_ignore   },
> +    [CSR_MCONTEXT]  =  { "mcontext", debug, 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
>
>