Re: [PATCH 5/6] target/riscv: Add CTR sctrclr instruction.

[Beeman Strong <beeman@rivosinc.com>]

[-- Attachment #1: Type: text/plain, Size: 4586 bytes --]

On Tue, Jun 4, 2024 at 10:19 AM Jason Chien <jason.chien@sifive.com> wrote:

>
> Rajnesh Kanwal 於 2024/5/30 上午 12:09 寫道:
> > CTR extension adds a new instruction sctrclr to quickly
> > clear the recorded entries buffer.
> >
> > Signed-off-by: Rajnesh Kanwal <rkanwal@rivosinc.com>
> > ---
> >   target/riscv/cpu.h                             |  1 +
> >   target/riscv/cpu_helper.c                      |  7 +++++++
> >   target/riscv/insn32.decode                     |  1 +
> >   target/riscv/insn_trans/trans_privileged.c.inc | 10 ++++++++++
> >   target/riscv/op_helper.c                       |  5 +++++
> >   5 files changed, 24 insertions(+)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index a294a5372a..fade71aa09 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -572,6 +572,7 @@ void riscv_cpu_set_mode(CPURISCVState *env,
> target_ulong newpriv, bool virt_en);
> >   void riscv_ctr_freeze(CPURISCVState *env, uint64_t freeze_mask);
> >   void riscv_ctr_add_entry(CPURISCVState *env, target_long src,
> target_long dst,
> >                            uint64_t type, target_ulong prev_priv, bool
> prev_virt);
> > +void riscv_ctr_clear(CPURISCVState *env);
> >
> >   void riscv_translate_init(void);
> >   G_NORETURN void riscv_raise_exception(CPURISCVState *env,
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index e064a7306e..45502f50a7 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -704,6 +704,13 @@ void riscv_ctr_freeze(CPURISCVState *env, uint64_t
> freeze_mask)
> >       }
> >   }
> >
> > +void riscv_ctr_clear(CPURISCVState *env)
> > +{
> > +    memset(env->ctr_src, 0x0, sizeof(env->ctr_src));
> > +    memset(env->ctr_dst, 0x0, sizeof(env->ctr_dst));
> > +    memset(env->ctr_data, 0x0, sizeof(env->ctr_data));
> > +}
> > +
> >   static uint64_t riscv_ctr_priv_to_mask(target_ulong priv, bool virt)
> >   {
> >       switch (priv) {
> > diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> > index 9cb1a1b4ec..d3d38c7c68 100644
> > --- a/target/riscv/insn32.decode
> > +++ b/target/riscv/insn32.decode
> > @@ -107,6 +107,7 @@
> >   # *** Privileged Instructions ***
> >   ecall       000000000000     00000 000 00000 1110011
> >   ebreak      000000000001     00000 000 00000 1110011
> > +sctrclr     000100000100     00000 000 00000 1110011
> >   uret        0000000    00010 00000 000 00000 1110011
> >   sret        0001000    00010 00000 000 00000 1110011
> >   mret        0011000    00010 00000 000 00000 1110011
> > diff --git a/target/riscv/insn_trans/trans_privileged.c.inc
> b/target/riscv/insn_trans/trans_privileged.c.inc
> > index 339d659151..dd9da8651f 100644
> > --- a/target/riscv/insn_trans/trans_privileged.c.inc
> > +++ b/target/riscv/insn_trans/trans_privileged.c.inc
> > @@ -69,6 +69,16 @@ static bool trans_ebreak(DisasContext *ctx,
> arg_ebreak *a)
> >       return true;
> >   }
> >
> > +static bool trans_sctrclr(DisasContext *ctx, arg_sctrclr *a)
> > +{
> > +#ifndef CONFIG_USER_ONLY
> If both of smctr and ssctr are not enabled, it is an illegal instruction.
> > +    gen_helper_ctr_clear(tcg_env);
> > +    return true;
> > +#else
> > +    return false;
> > +#endif
> > +}
> > +
> >   static bool trans_uret(DisasContext *ctx, arg_uret *a)
> >   {
> >       return false;
> > diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> > index c8053d9c2f..89423c04b3 100644
> > --- a/target/riscv/op_helper.c
> > +++ b/target/riscv/op_helper.c
> > @@ -461,6 +461,11 @@ void helper_ctr_branch(CPURISCVState *env,
> target_ulong src, target_ulong dest,
> >       }
> >   }
> >
> > +void helper_ctr_clear(CPURISCVState *env)
> > +{
>
> There should be some checks here.
> The spec states:
> SCTRCLR raises an illegal-instruction exception in U-mode, and a
> virtual-instruction exception in VU-mode, unless CTR state enable access
> restrictions apply.
>
> I don't quite understand "unless CTR state enable access restrictions
> apply" though.
>

The next sentence says "See Chapter 5", which states that execution of
SCTRCLR raises an illegal instruction exception if mstateen0.CTR=0 and the
priv mode is not M-mode, and it raises a virtual instruction exception if
mstateen0.CTR=0 && hstateen0.CTR=1 and the priv mode is VS-mode.


>
> > +    riscv_ctr_clear(env);
> > +}
> > +
> >   void helper_wfi(CPURISCVState *env)
> >   {
> >       CPUState *cs = env_cpu(env);
>

[-- Attachment #2: Type: text/html, Size: 5842 bytes --]