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

Rajnesh Kanwal posted 6 patches 5 months, 4 weeks ago
There is a newer version of this series
[PATCH 5/6] target/riscv: Add CTR sctrclr instruction.
Posted by Rajnesh Kanwal 5 months, 4 weeks ago
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
+    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)
+{
+    riscv_ctr_clear(env);
+}
+
 void helper_wfi(CPURISCVState *env)
 {
     CPUState *cs = env_cpu(env);
-- 
2.34.1
Re: [PATCH 5/6] target/riscv: Add CTR sctrclr instruction.
Posted by Jason Chien 5 months, 3 weeks ago
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.

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

Re: [PATCH 5/6] target/riscv: Add CTR sctrclr instruction.
Posted by Beeman Strong 5 months, 3 weeks ago
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);
>
Re: [PATCH 5/6] target/riscv: Add CTR sctrclr instruction.
Posted by Jason Chien 5 months, 3 weeks ago
Chapter 5 describes the CTR behavior when Smstaten is enabled, but it 
does not talk about the CTR behavior when Smstateen is disabled, that 
is, there is no mstateen0 and hstateen0 CSR.

The spec states:

  * Attempts to access sctrdepth from VS-mode or VU-mode raise a
    virtual-instruction exception, unless CTR state enable access
    restrictions apply.

  * sctrdepth is not included in the above list of supervisor CTR state
    controlled by hstateen0.CTR since accesses to sctrdepth from VS-mode
    raise a virtual-instruction exception regardless of the value of
    hstateen0.CTR.

Below is my understanding:

  * If Smstateen is enabled:
      o If mstateen0.CTR=0:
          + Attempts to access sctrctl, vsctrctl, sctrdepth, or
            sctrstatus raise an illegal-instruction exception for
            privilege modes less privileged than M-mode.
          + Attempts to access sireg* when siselect is in 0x200..0x2FF,
            or vsireg* when vsiselect is in 0x200..0x2FF raise an
            illegal-instruction exception for privilege modes less
            privileged than M-mode.
          + Execution of the SCTRCLR instruction raises an
            illegal-instruction exception for privilege modes less
            privileged than M-mode.
      o If mstateen.CTR=1:
          + Attempts to access sctrctl, vsctrctl, sctrdepth, or
            sctrstatus raise an illegal-instruction exception for U-mode.
          + Attempts to access sireg* when siselect is in 0x200..0x2FF,
            or vsireg* when vsiselect is in 0x200..0x2FF raise an
            illegal-instruction exception for U-mode.
          + Execution of the SCTRCLR instruction raises an
            illegal-instruction exception for U-mode.
          + If H extension is enabled:
              # If hstateen0.CTR = 0:
                  * Attempts to access sctrctl, vsctrctl, sctrdepth, or
                    sctrstatus raise an virtual-instruction exception
                    for VU-mode and VS-mode.
                  * Attempts to access sireg* when siselect is in
                    0x200..0x2FF, or vsireg* when vsiselect is in
                    0x200..0x2FF raise an virtual-instruction exception
                    for VU-mode and VS-mode.
                  * Execution of the SCTRCLR instruction raises an
                    virtual-instruction exception for VU-mode and VS-mode.
              # If hstateen0.CTR = 1:
                  * Attempts to access sctrctl, vsctrctl, sctrdepth, or
                    sctrstatus raise an virtual-instruction exception
                    for VU-mode.
                  * *_/Attempts to access sctrdepth raise an
                    virtual-instruction exception for "VS-mode"/._*
                  * Attempts to access sireg* when siselect is in
                    0x200..0x2FF, or vsireg* when vsiselect is in
                    0x200..0x2FF raise an virtual-instruction exception
                    for VU-mode.
                  * Execution of the SCTRCLR instruction raises an
                    virtual-instruction exception for VU-mode.
  * If Smstateen is disabled:
      o Attempts to access sctrctl, vsctrctl, sctrdepth, or sctrstatus
        raise an illegal-instruction exception for U-mode.
      o Attempts to access sireg* when siselect is in 0x200..0x2FF, or
        vsireg* when vsiselect is in 0x200..0x2FF raise an
        illegal-instruction exception for U-mode.
      o Execution of the SCTRCLR instruction raises an
        illegal-instruction exception for U-mode.
      o If H extension is enabled:
          + Attempts to access sctrctl, vsctrctl, sctrdepth, or
            sctrstatus raise an virtual-instruction exception for VU-mode.
          + */_Attempts to access sctrdepth raise an virtual-instruction
            exception for "VS-mode"._/*
          + Attempts to access sireg* when siselect is in 0x200..0x2FF,
            or vsireg* when vsiselect is in 0x200..0x2FF raise an
            virtual-instruction exception for VU-mode.
          + Execution of the SCTRCLR instruction raises an
            virtual-instruction exception for VU-mode.

If there is any misunderstanding, please let me know. Also Sstateen0 
does not involve in CTR. Am I correct?

Thanks in advance.

Beeman Strong 於 2024/6/5 上午 02:46 寫道:
>
>
> 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.
When mstateen0.CTR=0, hstateen0.CTR is read-only 0. I guess you meant 
"mstateen0.CTR=1 && hstateen0.CTR=0" here.
>
>
>     > +    riscv_ctr_clear(env);
>     > +}
>     > +
>     >   void helper_wfi(CPURISCVState *env)
>     >   {
>     >       CPUState *cs = env_cpu(env);
>
Re: [PATCH 5/6] target/riscv: Add CTR sctrclr instruction.
Posted by Beeman Strong 5 months, 3 weeks ago
On Tue, Jun 4, 2024 at 11:28 PM Jason Chien <jason.chien@sifive.com> wrote:

> Chapter 5 describes the CTR behavior when Smstaten is enabled, but it does
> not talk about the CTR behavior when Smstateen is disabled, that is, there
> is no mstateen0 and hstateen0 CSR.
>
If Smstateen is not implemented then chapter 5 can be ignored, and the
behavior is as described in the other chapters.

> The spec states:
>
>    - Attempts to access sctrdepth from VS-mode or VU-mode raise a
>    virtual-instruction exception, unless CTR state enable access restrictions
>    apply.
>
>
>    - sctrdepth is not included in the above list of supervisor CTR state
>    controlled by hstateen0.CTR since accesses to sctrdepth from VS-mode raise
>    a virtual-instruction exception regardless of the value of hstateen0.CTR.
>
> Below is my understanding:
>
>    - If Smstateen is enabled:
>       - If mstateen0.CTR=0:
>          - Attempts to access sctrctl, vsctrctl, sctrdepth, or sctrstatus
>          raise an illegal-instruction exception for privilege modes less privileged
>          than M-mode.
>          - Attempts to access sireg* when siselect is in 0x200..0x2FF, or
>          vsireg* when vsiselect is in 0x200..0x2FF raise an illegal-instruction
>          exception for privilege modes less privileged than M-mode.
>          - Execution of the SCTRCLR instruction raises an
>          illegal-instruction exception for privilege modes less privileged than
>          M-mode.
>          - If mstateen.CTR=1:
>          - Attempts to access sctrctl, vsctrctl, sctrdepth, or sctrstatus
>          raise an illegal-instruction exception for U-mode.
>          - Attempts to access sireg* when siselect is in 0x200..0x2FF, or
>          vsireg* when vsiselect is in 0x200..0x2FF raise an illegal-instruction
>          exception for U-mode.
>          - Execution of the SCTRCLR instruction raises an
>          illegal-instruction exception for U-mode.
>          - If H extension is enabled:
>             - If hstateen0.CTR = 0:
>             - Attempts to access sctrctl, vsctrctl, sctrdepth, or
>                sctrstatus raise an virtual-instruction exception for VU-mode and VS-mode.
>                - Attempts to access sireg* when siselect is in
>                0x200..0x2FF, or vsireg* when vsiselect is in 0x200..0x2FF raise an
>                virtual-instruction exception for VU-mode and VS-mode.
>                - Execution of the SCTRCLR instruction raises an
>                virtual-instruction exception for VU-mode and VS-mode.
>             - If hstateen0.CTR = 1:
>                - Attempts to access sctrctl, vsctrctl, sctrdepth, or
>                sctrstatus raise an virtual-instruction exception for VU-mode.
>                - *Attempts to access sctrdepth raise an
>                virtual-instruction exception for "VS-mode".*
>                - Attempts to access sireg* when siselect is in
>                0x200..0x2FF, or vsireg* when vsiselect is in 0x200..0x2FF raise an
>                virtual-instruction exception for VU-mode.
>                - Execution of the SCTRCLR instruction raises an
>                virtual-instruction exception for VU-mode.
>             - If Smstateen is disabled:
>       - Attempts to access sctrctl, vsctrctl, sctrdepth, or sctrstatus
>       raise an illegal-instruction exception for U-mode.
>       - Attempts to access sireg* when siselect is in 0x200..0x2FF, or
>       vsireg* when vsiselect is in 0x200..0x2FF raise an illegal-instruction
>       exception for U-mode.
>       - Execution of the SCTRCLR instruction raises an
>       illegal-instruction exception for U-mode.
>       - If H extension is enabled:
>          - Attempts to access sctrctl, vsctrctl, sctrdepth, or sctrstatus
>          raise an virtual-instruction exception for VU-mode.
>          - *Attempts to access sctrdepth raise an virtual-instruction
>          exception for "VS-mode".*
>          - Attempts to access sireg* when siselect is in 0x200..0x2FF, or
>          vsireg* when vsiselect is in 0x200..0x2FF raise an virtual-instruction
>          exception for VU-mode.
>          - Execution of the SCTRCLR instruction raises an
>          virtual-instruction exception for VU-mode.
>
> If there is any misunderstanding, please let me know. Also Sstateen0 does
> not involve in CTR. Am I correct?
>
That all looks correct.  sctrdepth gets that special treatment in VS-mode
(bold and underlined above) because it is expected that a hypervisor will
wish to limit the depth options available to a guest, for the purposes of
VM-migration.

And yes, there is no U/VU-mode access to CTR, so Ssstaten does not apply to
CTR.


> Thanks in advance.
> Beeman Strong 於 2024/6/5 上午 02:46 寫道:
>
>
>
> 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.
>
> When mstateen0.CTR=0, hstateen0.CTR is read-only 0. I guess you meant
> "mstateen0.CTR=1 && hstateen0.CTR=0" here.
>

You're right, thanks.

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