Re: [PATCH v5 05/13] target/riscv: Make RLB/MML/MMWP bits writable only when Smepmp is enabled

[Alistair Francis]

On Sat, Apr 29, 2023 at 12:37 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
> RLB/MML/MMWP bits in mseccfg CSR are introduced by Smepmp extension.
> So they can only be writable and set to 1s when cfg.epmp is true.
> Then we also need't check on epmp in pmp_hart_has_privs_default().
>
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>

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

Alistair

> ---
>  target/riscv/pmp.c | 50 ++++++++++++++++++++++++----------------------
>  1 file changed, 26 insertions(+), 24 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index b5808538aa..e745842973 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -243,30 +243,28 @@ static bool pmp_hart_has_privs_default(CPURISCVState 
> *env, target_ulong addr,
>  {
>      bool ret;
>
> -    if (riscv_cpu_cfg(env)->epmp) {
> -        if (MSECCFG_MMWP_ISSET(env)) {
> -            /*
> -             * The Machine Mode Whitelist Policy (mseccfg.MMWP) is set
> -             * so we default to deny all, even for M-mode.
> -             */
> +    if (MSECCFG_MMWP_ISSET(env)) {
> +        /*
> +         * The Machine Mode Whitelist Policy (mseccfg.MMWP) is set
> +         * so we default to deny all, even for M-mode.
> +         */
> +        *allowed_privs = 0;
> +        return false;
> +    } else if (MSECCFG_MML_ISSET(env)) {
> +        /*
> +         * The Machine Mode Lockdown (mseccfg.MML) bit is set
> +         * so we can only execute code in M-mode with an applicable
> +         * rule. Other modes are disabled.
> +         */
> +        if (mode == PRV_M && !(privs & PMP_EXEC)) {
> +            ret = true;
> +            *allowed_privs = PMP_READ | PMP_WRITE;
> +        } else {
> +            ret = false;
>              *allowed_privs = 0;
> -            return false;
> -        } else if (MSECCFG_MML_ISSET(env)) {
> -            /*
> -             * The Machine Mode Lockdown (mseccfg.MML) bit is set
> -             * so we can only execute code in M-mode with an applicable
> -             * rule. Other modes are disabled.
> -             */
> -            if (mode == PRV_M && !(privs & PMP_EXEC)) {
> -                ret = true;
> -                *allowed_privs = PMP_READ | PMP_WRITE;
> -            } else {
> -                ret = false;
> -                *allowed_privs = 0;
> -            }
> -
> -            return ret;
>          }
> +
> +        return ret;
>      }
>
>      if (!riscv_cpu_cfg(env)->pmp || (mode == PRV_M)) {
> @@ -580,8 +578,12 @@ void mseccfg_csr_write(CPURISCVState *env, target_ulong 
> val)
>          }
>      }
>
> -    /* Sticky bits */
> -    val |= (env->mseccfg & (MSECCFG_MMWP | MSECCFG_MML));
> +    if (riscv_cpu_cfg(env)->epmp) {
> +        /* Sticky bits */
> +        val |= (env->mseccfg & (MSECCFG_MMWP | MSECCFG_MML));
> +    } else {
> +        val &= ~(MSECCFG_MMWP | MSECCFG_MML | MSECCFG_RLB);
> +    }
>
>      env->mseccfg = val;
>  }
> --
> 2.25.1
>
>