Re: [PATCH v4 2/6] target/riscv: Add new CSR fields for S{sn, mn, m}pm extensions as part of Zjpm v0.8

[Alistair Francis]

On Tue, Jan 9, 2024 at 8:32 PM Alexey Baturo <baturo.alexey@gmail.com> wrote:
>
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>

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

Alistair

> ---
>  target/riscv/cpu.h      |  8 ++++++++
>  target/riscv/cpu_bits.h |  3 +++
>  target/riscv/cpu_cfg.h  |  3 +++
>  target/riscv/csr.c      | 11 +++++++++++
>  target/riscv/machine.c  | 10 +++++++---
>  target/riscv/pmp.c      | 13 ++++++++++---
>  target/riscv/pmp.h      | 11 ++++++-----
>  7 files changed, 48 insertions(+), 11 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index a43c8fba57..c9bed5c9fc 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -101,6 +101,14 @@ typedef enum {
>      EXT_STATUS_DIRTY,
>  } RISCVExtStatus;
>
> +/* Enum holds PMM field values for Zjpm v0.8 extension */
> +typedef enum {
> +    PMM_FIELD_DISABLED = 0,
> +    PMM_FIELD_RESERVED = 1,
> +    PMM_FIELD_PMLEN7   = 2,
> +    PMM_FIELD_PMLEN16  = 3,
> +} RISCVPmPmm;
> +
>  #define MMU_USER_IDX 3
>
>  #define MAX_RISCV_PMPS (16)
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 1c92458a01..7cf1049bf4 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -715,6 +715,7 @@ typedef enum RISCVException {
>  #define MENVCFG_CBIE                       (3UL << 4)
>  #define MENVCFG_CBCFE                      BIT(6)
>  #define MENVCFG_CBZE                       BIT(7)
> +#define MENVCFG_PMM                        (3ULL << 32)
>  #define MENVCFG_ADUE                       (1ULL << 61)
>  #define MENVCFG_PBMTE                      (1ULL << 62)
>  #define MENVCFG_STCE                       (1ULL << 63)
> @@ -728,11 +729,13 @@ typedef enum RISCVException {
>  #define SENVCFG_CBIE                       MENVCFG_CBIE
>  #define SENVCFG_CBCFE                      MENVCFG_CBCFE
>  #define SENVCFG_CBZE                       MENVCFG_CBZE
> +#define SENVCFG_PMM                        MENVCFG_PMM
>
>  #define HENVCFG_FIOM                       MENVCFG_FIOM
>  #define HENVCFG_CBIE                       MENVCFG_CBIE
>  #define HENVCFG_CBCFE                      MENVCFG_CBCFE
>  #define HENVCFG_CBZE                       MENVCFG_CBZE
> +#define HENVCFG_PMM                        MENVCFG_PMM
>  #define HENVCFG_ADUE                       MENVCFG_ADUE
>  #define HENVCFG_PBMTE                      MENVCFG_PBMTE
>  #define HENVCFG_STCE                       MENVCFG_STCE
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index f4605fb190..201f8af6ae 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -113,6 +113,9 @@ struct RISCVCPUConfig {
>      bool ext_ssaia;
>      bool ext_sscofpmf;
>      bool ext_smepmp;
> +    bool ext_ssnpm;
> +    bool ext_smnpm;
> +    bool ext_smmpm;
>      bool rvv_ta_all_1s;
>      bool rvv_ma_all_1s;
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index ea4e1ac6ef..a67ba30494 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -527,6 +527,9 @@ static RISCVException have_mseccfg(CPURISCVState *env, 
> int csrno)
>      if (riscv_cpu_cfg(env)->ext_zkr) {
>          return RISCV_EXCP_NONE;
>      }
> +    if (riscv_cpu_cfg(env)->ext_smmpm) {
> +        return RISCV_EXCP_NONE;
> +    }
>
>      return RISCV_EXCP_ILLEGAL_INST;
>  }
> @@ -2030,6 +2033,10 @@ static RISCVException write_menvcfg(CPURISCVState 
> *env, int csrno,
>                  (cfg->ext_sstc ? MENVCFG_STCE : 0) |
>                  (cfg->ext_svadu ? MENVCFG_ADUE : 0);
>      }
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & MENVCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= MENVCFG_PMM;
> +    }
>      env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
>
>      return RISCV_EXCP_NONE;
> @@ -2074,6 +2081,10 @@ static RISCVException write_senvcfg(CPURISCVState 
> *env, int csrno,
>                                      target_ulong val)
>  {
>      uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE | 
> SENVCFG_CBZE;
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & SENVCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= SENVCFG_PMM;
> +    }
>      RISCVException ret;
>
>      ret = smstateen_acc_ok(env, 0, SMSTATEEN0_HSENVCFG);
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 71ee8bab19..0ad593ed5a 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -152,15 +152,19 @@ static const VMStateDescription vmstate_vector = {
>
>  static bool pointermasking_needed(void *opaque)
>  {
> -    return false;
> +    RISCVCPU *cpu = opaque;
> +    return cpu->cfg.ext_ssnpm || cpu->cfg.ext_smnpm || cpu->cfg.ext_smmpm;
>  }
>
>  static const VMStateDescription vmstate_pointermasking = {
>      .name = "cpu/pointer_masking",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .needed = pointermasking_needed,
>      .fields = (VMStateField[]) {
> +        VMSTATE_UINTTL(env.mseccfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
>          VMSTATE_END_OF_LIST()
>      }
>  };
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 162e88a90a..893ccd58d8 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -576,6 +576,12 @@ target_ulong pmpaddr_csr_read(CPURISCVState *env, 
> uint32_t addr_index)
>  void mseccfg_csr_write(CPURISCVState *env, target_ulong val)
>  {
>      int i;
> +    uint64_t mask = MSECCFG_MMWP | MSECCFG_MML;
> +
> +    /* Update PMM field only if the value is valid according to Zjpm v0.8 */
> +    if (((val & MSECCFG_PMM) >> 32) != PMM_FIELD_RESERVED) {
> +        mask |= MSECCFG_PMM;
> +    }
>
>      trace_mseccfg_csr_write(env->mhartid, val);
>
> @@ -591,12 +597,13 @@ void mseccfg_csr_write(CPURISCVState *env, target_ulong 
> val)
>
>      if (riscv_cpu_cfg(env)->ext_smepmp) {
>          /* Sticky bits */
> -        val |= (env->mseccfg & (MSECCFG_MMWP | MSECCFG_MML));
> -        if ((val ^ env->mseccfg) & (MSECCFG_MMWP | MSECCFG_MML)) {
> +        val |= (env->mseccfg & mask);
> +        if ((val ^ env->mseccfg) & mask) {
>              tlb_flush(env_cpu(env));
>          }
>      } else {
> -        val &= ~(MSECCFG_MMWP | MSECCFG_MML | MSECCFG_RLB);
> +        mask |= MSECCFG_RLB;
> +        val &= ~(mask);
>      }
>
>      env->mseccfg = val;
> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> index 9af8614cd4..b3ca51c26d 100644
> --- a/target/riscv/pmp.h
> +++ b/target/riscv/pmp.h
> @@ -40,11 +40,12 @@ typedef enum {
>  } pmp_am_t;
>
>  typedef enum {
> -    MSECCFG_MML   = 1 << 0,
> -    MSECCFG_MMWP  = 1 << 1,
> -    MSECCFG_RLB   = 1 << 2,
> -    MSECCFG_USEED = 1 << 8,
> -    MSECCFG_SSEED = 1 << 9
> +    MSECCFG_MML       = 1 << 0,
> +    MSECCFG_MMWP      = 1 << 1,
> +    MSECCFG_RLB       = 1 << 2,
> +    MSECCFG_USEED     = 1 << 8,
> +    MSECCFG_SSEED     = 1 << 9,
> +    MSECCFG_PMM       = 3UL << 32,
>  } mseccfg_field_t;
>
>  typedef struct {
> --
> 2.34.1
>
>