Re: [PATCH v2 2/5] target/riscv: pmp: move Smepmp operation conversion into a function

[Alistair Francis <alistair23@gmail.com>]

On Fri, Mar 14, 2025 at 5:33 AM Loïc Lefort <loic@rivosinc.com> wrote:
>
> Signed-off-by: Loïc Lefort <loic@rivosinc.com>
> Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

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

Alistair

> ---
>  target/riscv/pmp.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index e1e5ca589e..7d65dc24a5 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -31,6 +31,15 @@ static bool pmp_write_cfg(CPURISCVState *env, uint32_t addr_index,
>                            uint8_t val);
>  static uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t addr_index);
>
> +/*
> + * Convert the PMP permissions to match the truth table in the Smepmp spec.
> + */
> +static inline uint8_t pmp_get_smepmp_operation(uint8_t cfg)
> +{
> +    return ((cfg & PMP_LOCK) >> 4) | ((cfg & PMP_READ) << 2) |
> +           (cfg & PMP_WRITE) | ((cfg & PMP_EXEC) >> 2);
> +}
> +
>  /*
>   * Accessor method to extract address matching type 'a field' from cfg reg
>   */
> @@ -355,16 +364,6 @@ bool pmp_hart_has_privs(CPURISCVState *env, hwaddr addr,
>          const uint8_t a_field =
>              pmp_get_a_field(env->pmp_state.pmp[i].cfg_reg);
>
> -        /*
> -         * Convert the PMP permissions to match the truth table in the
> -         * Smepmp spec.
> -         */
> -        const uint8_t smepmp_operation =
> -            ((env->pmp_state.pmp[i].cfg_reg & PMP_LOCK) >> 4) |
> -            ((env->pmp_state.pmp[i].cfg_reg & PMP_READ) << 2) |
> -            (env->pmp_state.pmp[i].cfg_reg & PMP_WRITE) |
> -            ((env->pmp_state.pmp[i].cfg_reg & PMP_EXEC) >> 2);
> -
>          if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) {
>              /*
>               * If the PMP entry is not off and the address is in range,
> @@ -383,6 +382,9 @@ bool pmp_hart_has_privs(CPURISCVState *env, hwaddr addr,
>                  /*
>                   * If mseccfg.MML Bit set, do the enhanced pmp priv check
>                   */
> +                const uint8_t smepmp_operation =
> +                    pmp_get_smepmp_operation(env->pmp_state.pmp[i].cfg_reg);
> +
>                  if (mode == PRV_M) {
>                      switch (smepmp_operation) {
>                      case 0:
> --
> 2.47.2
>
>