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

[Beeman Strong]

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