diff mbox series

[RFC,4/5] target/riscv: Add *envcfg* CSRs support

Message ID 20220120200735.2739543-5-atishp@rivosinc.com (mailing list archive)
State New, archived
Headers show
Series Privilege version update | expand

Commit Message

Atish Kumar Patra Jan. 20, 2022, 8:07 p.m. UTC
The RISC-V privileged specification v1.12 defines few execution
environment configuration CSRs that can be used enable/disable
extensions per privilege levels.

Add the basic support for these CSRs.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 target/riscv/cpu.h      |  8 ++++
 target/riscv/cpu_bits.h | 31 +++++++++++++++
 target/riscv/csr.c      | 84 +++++++++++++++++++++++++++++++++++++++++
 target/riscv/machine.c  | 26 +++++++++++++
 4 files changed, 149 insertions(+)

Comments

Weiwei Li Jan. 26, 2022, 8:37 a.m. UTC | #1
在 2022/1/21 上午4:07, Atish Patra 写道:
> The RISC-V privileged specification v1.12 defines few execution
> environment configuration CSRs that can be used enable/disable
> extensions per privilege levels.
>
> Add the basic support for these CSRs.
>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>   target/riscv/cpu.h      |  8 ++++
>   target/riscv/cpu_bits.h | 31 +++++++++++++++
>   target/riscv/csr.c      | 84 +++++++++++++++++++++++++++++++++++++++++
>   target/riscv/machine.c  | 26 +++++++++++++
>   4 files changed, 149 insertions(+)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 7f87917204c5..b9462300a472 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -264,6 +264,14 @@ struct CPURISCVState {
>       target_ulong spmbase;
>       target_ulong upmmask;
>       target_ulong upmbase;
> +
> +    /* CSRs for execution enviornment configuration */
> +
> +    target_ulong menvcfg;
> +    target_ulong menvcfgh;

I think we needn't maintain seperate menvcfg and menvcfgh, just use 
"uint64_t menvcfg" as the way of mstatus.

Similar to  henvcfg and henvcfg.

> +    target_ulong senvcfg;
> +    target_ulong henvcfg;
> +    target_ulong henvcfgh;
>   #endif
>   
>       float_status fp_status;
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index f6f90b5cbd52..afb237c2313b 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -177,6 +177,9 @@
>   #define CSR_STVEC           0x105
>   #define CSR_SCOUNTEREN      0x106
>   
> +/* Supervisor Configuration CSRs */
> +#define CSR_SENVCFG         0x10A
> +
>   /* Supervisor Trap Handling */
>   #define CSR_SSCRATCH        0x140
>   #define CSR_SEPC            0x141
> @@ -204,6 +207,10 @@
>   #define CSR_HTIMEDELTA      0x605
>   #define CSR_HTIMEDELTAH     0x615
>   
> +/* Hypervisor Configuration CSRs */
> +#define CSR_HENVCFG         0x60A
> +#define CSR_HENVCFGH        0x61A
> +
>   /* Virtual CSRs */
>   #define CSR_VSSTATUS        0x200
>   #define CSR_VSIE            0x204
> @@ -218,6 +225,10 @@
>   #define CSR_MTINST          0x34a
>   #define CSR_MTVAL2          0x34b
>   
> +/* Machine Configuration CSRs */
> +#define CSR_MENVCFG         0x30A
> +#define CSR_MENVCFGH        0x31A
> +
>   /* Enhanced Physical Memory Protection (ePMP) */
>   #define CSR_MSECCFG         0x747
>   #define CSR_MSECCFGH        0x757
> @@ -578,6 +589,26 @@ typedef enum RISCVException {
>   #define PM_EXT_CLEAN    0x00000002ULL
>   #define PM_EXT_DIRTY    0x00000003ULL
>   
> +/* Execution enviornment configuration bits */
> +#define MENVCFG_FIOM                       (1 << 0)
> +#define MENVCFG_CBE                        0x30000ULL
> +#define MENVCFG_CBCFE                      (1 << 6)
> +#define MENVCFG_CBZE                       (1 << 7)
> +#define MENVCFG_PBMTE                      (1 << 62)
> +#define MENVCFG_STCE                       (1 << 63)
> +
> +#define SENVCFG_FIOM                       MENVCFG_FIOM
> +#define SENVCFG_CBE                        MENVCFG_CBE
> +#define SENVCFG_CBCFE                      MENVCFG_CBCFE
> +#define SENVCFG_CBZE                       MENVCFG_CBZE
> +
> +#define HENVCFG_FIOM                       MENVCFG_FIOM
> +#define HENVCFG_CBE                        MENVCFG_CBE
> +#define HENVCFG_CBCFE                      MENVCFG_CBCFE
> +#define HENVCFG_CBZE                       MENVCFG_CBZE
> +#define HENVCFG_PBMTE                      MENVCFG_PBMTE
> +#define HENVCFG_STCE                       MENVCFG_STCE
> +
>   /* Offsets for every pair of control bits per each priv level */
>   #define XS_OFFSET    0ULL
>   #define U_OFFSET     2ULL
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index e66bf2201857..a4bbae7a1bbd 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -853,6 +853,77 @@ static RISCVException write_mtval(CPURISCVState *env, int csrno,
>       return RISCV_EXCP_NONE;
>   }
>   
> +/* Execution environment configuration setup */
> +static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->menvcfg;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->menvcfg = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->menvcfgh;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->menvcfgh = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->senvcfg;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->senvcfg = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->henvcfg;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->henvcfg = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
> +                                 target_ulong *val)
> +{
> +    *val = env->henvcfgh;
> +    return RISCV_EXCP_NONE;
> +}
> +
> +static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
> +                                  target_ulong val)
> +{
> +    env->henvcfgh = val;
> +    return RISCV_EXCP_NONE;
> +}
> +
>   static RISCVException rmw_mip(CPURISCVState *env, int csrno,
>                                 target_ulong *ret_value,
>                                 target_ulong new_value, target_ulong write_mask)
> @@ -2054,6 +2125,19 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>       [CSR_MTVAL]    = { "mtval",    any,  read_mtval,    write_mtval    },
>       [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip        },
>   
> +    /* Execution environment configuration */
> +    [CSR_MENVCFG]  = { "menvcfg",  any,     read_menvcfg,  write_menvcfg, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +    [CSR_MENVCFGH] = { "menvcfgh", any32,   read_menvcfgh, write_menvcfgh, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +    [CSR_SENVCFG]  = { "senvcfg",  smode,   read_senvcfg,  write_senvcfg, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +    [CSR_HENVCFG]  = { "henvcfg",  hmode,   read_henvcfg, write_henvcfg, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh, write_henvcfgh, NULL,
> +                                            NULL, NULL, PRIV_VERSION_1_12_0},
> +
> +
Two new lines here.
>       /* Supervisor Trap Setup */
>       [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,    write_sstatus, NULL,
>                                                 read_sstatus_i128                 },
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 13b9ab375b95..59479a999b87 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -185,6 +185,31 @@ static const VMStateDescription vmstate_rv128 = {
>       }
>   };
>   
> +/* TODO: henvcfg need both hyper_needed & envcfg_needed */
> +static bool envcfg_needed(void *opaque)
> +{
> +    RISCVCPU *cpu = opaque;
> +    CPURISCVState *env = &cpu->env;
> +
> +    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
> +}
> +
> +static const VMStateDescription vmstate_envcfg = {
> +    .name = "cpu/envcfg",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .needed = envcfg_needed,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
> +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
> +        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
> +
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>   const VMStateDescription vmstate_riscv_cpu = {
>       .name = "cpu",
>       .version_id = 3,
> @@ -240,6 +265,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>           &vmstate_vector,
>           &vmstate_pointermasking,
>           &vmstate_rv128,
> +        &vmstate_envcfg,
>           NULL
>       }
>   };

Regards,

Weiwei Li
Atish Patra Jan. 29, 2022, 1:28 a.m. UTC | #2
On Wed, Jan 26, 2022 at 12:37 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:

>
> 在 2022/1/21 上午4:07, Atish Patra 写道:
> > The RISC-V privileged specification v1.12 defines few execution
> > environment configuration CSRs that can be used enable/disable
> > extensions per privilege levels.
> >
> > Add the basic support for these CSRs.
> >
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > ---
> >   target/riscv/cpu.h      |  8 ++++
> >   target/riscv/cpu_bits.h | 31 +++++++++++++++
> >   target/riscv/csr.c      | 84 +++++++++++++++++++++++++++++++++++++++++
> >   target/riscv/machine.c  | 26 +++++++++++++
> >   4 files changed, 149 insertions(+)
> >
> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> > index 7f87917204c5..b9462300a472 100644
> > --- a/target/riscv/cpu.h
> > +++ b/target/riscv/cpu.h
> > @@ -264,6 +264,14 @@ struct CPURISCVState {
> >       target_ulong spmbase;
> >       target_ulong upmmask;
> >       target_ulong upmbase;
> > +
> > +    /* CSRs for execution enviornment configuration */
> > +
> > +    target_ulong menvcfg;
> > +    target_ulong menvcfgh;
>
> I think we needn't maintain seperate menvcfg and menvcfgh, just use
> "uint64_t menvcfg" as the way of mstatus.
>
>
unlike mstatush, menvcfgh/henvcfgh will be accessed directly to do runtime
predicate for stimecmp/vstimecmp.

We have to do the 32 bit shifting during every check which makes the code
hard to read
at the cost of 2 ulongs.

IMO, having separate variables is much simpler.

Similar to  henvcfg and henvcfg.
>
> > +    target_ulong senvcfg;
> > +    target_ulong henvcfg;
> > +    target_ulong henvcfgh;
> >   #endif
> >
> >       float_status fp_status;
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index f6f90b5cbd52..afb237c2313b 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -177,6 +177,9 @@
> >   #define CSR_STVEC           0x105
> >   #define CSR_SCOUNTEREN      0x106
> >
> > +/* Supervisor Configuration CSRs */
> > +#define CSR_SENVCFG         0x10A
> > +
> >   /* Supervisor Trap Handling */
> >   #define CSR_SSCRATCH        0x140
> >   #define CSR_SEPC            0x141
> > @@ -204,6 +207,10 @@
> >   #define CSR_HTIMEDELTA      0x605
> >   #define CSR_HTIMEDELTAH     0x615
> >
> > +/* Hypervisor Configuration CSRs */
> > +#define CSR_HENVCFG         0x60A
> > +#define CSR_HENVCFGH        0x61A
> > +
> >   /* Virtual CSRs */
> >   #define CSR_VSSTATUS        0x200
> >   #define CSR_VSIE            0x204
> > @@ -218,6 +225,10 @@
> >   #define CSR_MTINST          0x34a
> >   #define CSR_MTVAL2          0x34b
> >
> > +/* Machine Configuration CSRs */
> > +#define CSR_MENVCFG         0x30A
> > +#define CSR_MENVCFGH        0x31A
> > +
> >   /* Enhanced Physical Memory Protection (ePMP) */
> >   #define CSR_MSECCFG         0x747
> >   #define CSR_MSECCFGH        0x757
> > @@ -578,6 +589,26 @@ typedef enum RISCVException {
> >   #define PM_EXT_CLEAN    0x00000002ULL
> >   #define PM_EXT_DIRTY    0x00000003ULL
> >
> > +/* Execution enviornment configuration bits */
> > +#define MENVCFG_FIOM                       (1 << 0)
> > +#define MENVCFG_CBE                        0x30000ULL
> > +#define MENVCFG_CBCFE                      (1 << 6)
> > +#define MENVCFG_CBZE                       (1 << 7)
> > +#define MENVCFG_PBMTE                      (1 << 62)
> > +#define MENVCFG_STCE                       (1 << 63)
> > +
> > +#define SENVCFG_FIOM                       MENVCFG_FIOM
> > +#define SENVCFG_CBE                        MENVCFG_CBE
> > +#define SENVCFG_CBCFE                      MENVCFG_CBCFE
> > +#define SENVCFG_CBZE                       MENVCFG_CBZE
> > +
> > +#define HENVCFG_FIOM                       MENVCFG_FIOM
> > +#define HENVCFG_CBE                        MENVCFG_CBE
> > +#define HENVCFG_CBCFE                      MENVCFG_CBCFE
> > +#define HENVCFG_CBZE                       MENVCFG_CBZE
> > +#define HENVCFG_PBMTE                      MENVCFG_PBMTE
> > +#define HENVCFG_STCE                       MENVCFG_STCE
> > +
> >   /* Offsets for every pair of control bits per each priv level */
> >   #define XS_OFFSET    0ULL
> >   #define U_OFFSET     2ULL
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index e66bf2201857..a4bbae7a1bbd 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -853,6 +853,77 @@ static RISCVException write_mtval(CPURISCVState
> *env, int csrno,
> >       return RISCV_EXCP_NONE;
> >   }
> >
> > +/* Execution environment configuration setup */
> > +static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->menvcfg;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->menvcfg = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->menvcfgh;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->menvcfgh = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->senvcfg;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->senvcfg = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->henvcfg;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->henvcfg = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
> > +                                 target_ulong *val)
> > +{
> > +    *val = env->henvcfgh;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> > +static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
> > +                                  target_ulong val)
> > +{
> > +    env->henvcfgh = val;
> > +    return RISCV_EXCP_NONE;
> > +}
> > +
> >   static RISCVException rmw_mip(CPURISCVState *env, int csrno,
> >                                 target_ulong *ret_value,
> >                                 target_ulong new_value, target_ulong
> write_mask)
> > @@ -2054,6 +2125,19 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
> >       [CSR_MTVAL]    = { "mtval",    any,  read_mtval,    write_mtval
> },
> >       [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip
> },
> >
> > +    /* Execution environment configuration */
> > +    [CSR_MENVCFG]  = { "menvcfg",  any,     read_menvcfg,
> write_menvcfg, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +    [CSR_MENVCFGH] = { "menvcfgh", any32,   read_menvcfgh,
> write_menvcfgh, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +    [CSR_SENVCFG]  = { "senvcfg",  smode,   read_senvcfg,
> write_senvcfg, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +    [CSR_HENVCFG]  = { "henvcfg",  hmode,   read_henvcfg,
> write_henvcfg, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh,
> write_henvcfgh, NULL,
> > +                                            NULL, NULL,
> PRIV_VERSION_1_12_0},
> > +
> > +
> Two new lines here.
>

Fixed it. Thanks.


> >       /* Supervisor Trap Setup */
> >       [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,
> write_sstatus, NULL,
> >                                                 read_sstatus_i128
>          },
> > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> > index 13b9ab375b95..59479a999b87 100644
> > --- a/target/riscv/machine.c
> > +++ b/target/riscv/machine.c
> > @@ -185,6 +185,31 @@ static const VMStateDescription vmstate_rv128 = {
> >       }
> >   };
> >
> > +/* TODO: henvcfg need both hyper_needed & envcfg_needed */
> > +static bool envcfg_needed(void *opaque)
> > +{
> > +    RISCVCPU *cpu = opaque;
> > +    CPURISCVState *env = &cpu->env;
> > +
> > +    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
> > +}
> > +
> > +static const VMStateDescription vmstate_envcfg = {
> > +    .name = "cpu/envcfg",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .needed = envcfg_needed,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
> > +        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
> > +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
> > +        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
> > +        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
> > +
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> >   const VMStateDescription vmstate_riscv_cpu = {
> >       .name = "cpu",
> >       .version_id = 3,
> > @@ -240,6 +265,7 @@ const VMStateDescription vmstate_riscv_cpu = {
> >           &vmstate_vector,
> >           &vmstate_pointermasking,
> >           &vmstate_rv128,
> > +        &vmstate_envcfg,
> >           NULL
> >       }
> >   };
>
> Regards,
>
> Weiwei Li
>
>
>
angell1518 Jan. 29, 2022, 1:50 a.m. UTC | #3
在 2022/1/29 上午9:28, Atish Patra 写道:
>
>
> On Wed, Jan 26, 2022 at 12:37 AM Weiwei Li <liweiwei@iscas.ac.cn 
> <mailto:liweiwei@iscas.ac.cn>> wrote:
>
>
>     在 2022/1/21 上午4:07, Atish Patra 写道:
>     > The RISC-V privileged specification v1.12 defines few execution
>     > environment configuration CSRs that can be used enable/disable
>     > extensions per privilege levels.
>     >
>     > Add the basic support for these CSRs.
>     >
>     > Signed-off-by: Atish Patra <atishp@rivosinc.com
>     <mailto:atishp@rivosinc.com>>
>     > ---
>     >   target/riscv/cpu.h      |  8 ++++
>     >   target/riscv/cpu_bits.h | 31 +++++++++++++++
>     >   target/riscv/csr.c      | 84
>     +++++++++++++++++++++++++++++++++++++++++
>     >   target/riscv/machine.c  | 26 +++++++++++++
>     >   4 files changed, 149 insertions(+)
>     >
>     > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>     > index 7f87917204c5..b9462300a472 100644
>     > --- a/target/riscv/cpu.h
>     > +++ b/target/riscv/cpu.h
>     > @@ -264,6 +264,14 @@ struct CPURISCVState {
>     >       target_ulong spmbase;
>     >       target_ulong upmmask;
>     >       target_ulong upmbase;
>     > +
>     > +    /* CSRs for execution enviornment configuration */
>     > +
>     > +    target_ulong menvcfg;
>     > +    target_ulong menvcfgh;
>
>     I think we needn't maintain seperate menvcfg and menvcfgh, just use
>     "uint64_t menvcfg" as the way of mstatus.
>
>
> unlike mstatush, menvcfgh/henvcfgh will be accessed directly to do runtime
> predicate for stimecmp/vstimecmp.
>
> We have to do the 32 bit shifting during every check which makes the 
> code hard to read
> at the cost of 2 ulongs.
>
> IMO, having separate variables is much simpler.

Do you mean check STCE/VSTCE bit in menvcfg/henvcfg?

If so, I think use a simple "uint64_t menvcfg/henvcfg" may be better,  
then we can only check the 63 bit of them.

Or we should decide where to get this bit from(mencvfg/henvcfg, or 
mencfgh/henvcfgh) based on the MXLEN/HSXLEN.

Regards,

Weiwei Li

>
>     Similar to  henvcfg and henvcfg.
>
>     > +    target_ulong senvcfg;
>     > +    target_ulong henvcfg;
>     > +    target_ulong henvcfgh;
>     >   #endif
>     >
>     >       float_status fp_status;
>     > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>     > index f6f90b5cbd52..afb237c2313b 100644
>     > --- a/target/riscv/cpu_bits.h
>     > +++ b/target/riscv/cpu_bits.h
>     > @@ -177,6 +177,9 @@
>     >   #define CSR_STVEC           0x105
>     >   #define CSR_SCOUNTEREN      0x106
>     >
>     > +/* Supervisor Configuration CSRs */
>     > +#define CSR_SENVCFG         0x10A
>     > +
>     >   /* Supervisor Trap Handling */
>     >   #define CSR_SSCRATCH        0x140
>     >   #define CSR_SEPC            0x141
>     > @@ -204,6 +207,10 @@
>     >   #define CSR_HTIMEDELTA      0x605
>     >   #define CSR_HTIMEDELTAH     0x615
>     >
>     > +/* Hypervisor Configuration CSRs */
>     > +#define CSR_HENVCFG         0x60A
>     > +#define CSR_HENVCFGH        0x61A
>     > +
>     >   /* Virtual CSRs */
>     >   #define CSR_VSSTATUS        0x200
>     >   #define CSR_VSIE            0x204
>     > @@ -218,6 +225,10 @@
>     >   #define CSR_MTINST          0x34a
>     >   #define CSR_MTVAL2          0x34b
>     >
>     > +/* Machine Configuration CSRs */
>     > +#define CSR_MENVCFG         0x30A
>     > +#define CSR_MENVCFGH        0x31A
>     > +
>     >   /* Enhanced Physical Memory Protection (ePMP) */
>     >   #define CSR_MSECCFG         0x747
>     >   #define CSR_MSECCFGH        0x757
>     > @@ -578,6 +589,26 @@ typedef enum RISCVException {
>     >   #define PM_EXT_CLEAN    0x00000002ULL
>     >   #define PM_EXT_DIRTY    0x00000003ULL
>     >
>     > +/* Execution enviornment configuration bits */
>     > +#define MENVCFG_FIOM                       (1 << 0)
>     > +#define MENVCFG_CBE                        0x30000ULL
>     > +#define MENVCFG_CBCFE                      (1 << 6)
>     > +#define MENVCFG_CBZE                       (1 << 7)
>     > +#define MENVCFG_PBMTE                      (1 << 62)
>     > +#define MENVCFG_STCE                       (1 << 63)
>     > +
>     > +#define SENVCFG_FIOM  MENVCFG_FIOM
>     > +#define SENVCFG_CBE                        MENVCFG_CBE
>     > +#define SENVCFG_CBCFE MENVCFG_CBCFE
>     > +#define SENVCFG_CBZE  MENVCFG_CBZE
>     > +
>     > +#define HENVCFG_FIOM  MENVCFG_FIOM
>     > +#define HENVCFG_CBE                        MENVCFG_CBE
>     > +#define HENVCFG_CBCFE MENVCFG_CBCFE
>     > +#define HENVCFG_CBZE  MENVCFG_CBZE
>     > +#define HENVCFG_PBMTE MENVCFG_PBMTE
>     > +#define HENVCFG_STCE  MENVCFG_STCE
>     > +
>     >   /* Offsets for every pair of control bits per each priv level */
>     >   #define XS_OFFSET    0ULL
>     >   #define U_OFFSET     2ULL
>     > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>     > index e66bf2201857..a4bbae7a1bbd 100644
>     > --- a/target/riscv/csr.c
>     > +++ b/target/riscv/csr.c
>     > @@ -853,6 +853,77 @@ static RISCVException
>     write_mtval(CPURISCVState *env, int csrno,
>     >       return RISCV_EXCP_NONE;
>     >   }
>     >
>     > +/* Execution environment configuration setup */
>     > +static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
>     > +                                 target_ulong *val)
>     > +{
>     > +    *val = env->menvcfg;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
>     > +                                  target_ulong val)
>     > +{
>     > +    env->menvcfg = val;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
>     > +                                 target_ulong *val)
>     > +{
>     > +    *val = env->menvcfgh;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
>     > +                                  target_ulong val)
>     > +{
>     > +    env->menvcfgh = val;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
>     > +                                 target_ulong *val)
>     > +{
>     > +    *val = env->senvcfg;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
>     > +                                  target_ulong val)
>     > +{
>     > +    env->senvcfg = val;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
>     > +                                 target_ulong *val)
>     > +{
>     > +    *val = env->henvcfg;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
>     > +                                  target_ulong val)
>     > +{
>     > +    env->henvcfg = val;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
>     > +                                 target_ulong *val)
>     > +{
>     > +    *val = env->henvcfgh;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     > +static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
>     > +                                  target_ulong val)
>     > +{
>     > +    env->henvcfgh = val;
>     > +    return RISCV_EXCP_NONE;
>     > +}
>     > +
>     >   static RISCVException rmw_mip(CPURISCVState *env, int csrno,
>     >                                 target_ulong *ret_value,
>     >                                 target_ulong new_value,
>     target_ulong write_mask)
>     > @@ -2054,6 +2125,19 @@ riscv_csr_operations
>     csr_ops[CSR_TABLE_SIZE] = {
>     >       [CSR_MTVAL]    = { "mtval",    any,  read_mtval,  
>     write_mtval    },
>     >       [CSR_MIP]      = { "mip",      any,  NULL, NULL, rmw_mip 
>           },
>     >
>     > +    /* Execution environment configuration */
>     > +    [CSR_MENVCFG]  = { "menvcfg",  any,  read_menvcfg, 
>     write_menvcfg, NULL,
>     > +                                            NULL, NULL,
>     PRIV_VERSION_1_12_0},
>     > +    [CSR_MENVCFGH] = { "menvcfgh", any32,  read_menvcfgh,
>     write_menvcfgh, NULL,
>     > +                                            NULL, NULL,
>     PRIV_VERSION_1_12_0},
>     > +    [CSR_SENVCFG]  = { "senvcfg",  smode,  read_senvcfg, 
>     write_senvcfg, NULL,
>     > +                                            NULL, NULL,
>     PRIV_VERSION_1_12_0},
>     > +    [CSR_HENVCFG]  = { "henvcfg",  hmode,  read_henvcfg,
>     write_henvcfg, NULL,
>     > +                                            NULL, NULL,
>     PRIV_VERSION_1_12_0},
>     > +    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh,
>     write_henvcfgh, NULL,
>     > +                                            NULL, NULL,
>     PRIV_VERSION_1_12_0},
>     > +
>     > +
>     Two new lines here.
>
>
> Fixed it. Thanks.
>
>     >       /* Supervisor Trap Setup */
>     >       [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,   
>     write_sstatus, NULL,
>     >  read_sstatus_i128                 },
>     > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>     > index 13b9ab375b95..59479a999b87 100644
>     > --- a/target/riscv/machine.c
>     > +++ b/target/riscv/machine.c
>     > @@ -185,6 +185,31 @@ static const VMStateDescription
>     vmstate_rv128 = {
>     >       }
>     >   };
>     >
>     > +/* TODO: henvcfg need both hyper_needed & envcfg_needed */
>     > +static bool envcfg_needed(void *opaque)
>     > +{
>     > +    RISCVCPU *cpu = opaque;
>     > +    CPURISCVState *env = &cpu->env;
>     > +
>     > +    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
>     > +}
>     > +
>     > +static const VMStateDescription vmstate_envcfg = {
>     > +    .name = "cpu/envcfg",
>     > +    .version_id = 1,
>     > +    .minimum_version_id = 1,
>     > +    .needed = envcfg_needed,
>     > +    .fields = (VMStateField[]) {
>     > +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
>     > +        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
>     > +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
>     > +        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
>     > +        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
>     > +
>     > +        VMSTATE_END_OF_LIST()
>     > +    }
>     > +};
>     > +
>     >   const VMStateDescription vmstate_riscv_cpu = {
>     >       .name = "cpu",
>     >       .version_id = 3,
>     > @@ -240,6 +265,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>     >           &vmstate_vector,
>     >           &vmstate_pointermasking,
>     >           &vmstate_rv128,
>     > +        &vmstate_envcfg,
>     >           NULL
>     >       }
>     >   };
>
>     Regards,
>
>     Weiwei Li
>
>
>
>
> -- 
> Regards,
> Atish
Atish Kumar Patra Jan. 31, 2022, 8:05 p.m. UTC | #4
On Fri, Jan 28, 2022 at 5:50 PM angell1518 <angell1518@163.com> wrote:

>
> 在 2022/1/29 上午9:28, Atish Patra 写道:
>
>
>
> On Wed, Jan 26, 2022 at 12:37 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
>>
>> 在 2022/1/21 上午4:07, Atish Patra 写道:
>> > The RISC-V privileged specification v1.12 defines few execution
>> > environment configuration CSRs that can be used enable/disable
>> > extensions per privilege levels.
>> >
>> > Add the basic support for these CSRs.
>> >
>> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
>> > ---
>> >   target/riscv/cpu.h      |  8 ++++
>> >   target/riscv/cpu_bits.h | 31 +++++++++++++++
>> >   target/riscv/csr.c      | 84 +++++++++++++++++++++++++++++++++++++++++
>> >   target/riscv/machine.c  | 26 +++++++++++++
>> >   4 files changed, 149 insertions(+)
>> >
>> > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> > index 7f87917204c5..b9462300a472 100644
>> > --- a/target/riscv/cpu.h
>> > +++ b/target/riscv/cpu.h
>> > @@ -264,6 +264,14 @@ struct CPURISCVState {
>> >       target_ulong spmbase;
>> >       target_ulong upmmask;
>> >       target_ulong upmbase;
>> > +
>> > +    /* CSRs for execution enviornment configuration */
>> > +
>> > +    target_ulong menvcfg;
>> > +    target_ulong menvcfgh;
>>
>> I think we needn't maintain seperate menvcfg and menvcfgh, just use
>> "uint64_t menvcfg" as the way of mstatus.
>>
>>
> unlike mstatush, menvcfgh/henvcfgh will be accessed directly to do runtime
> predicate for stimecmp/vstimecmp.
>
> We have to do the 32 bit shifting during every check which makes the code
> hard to read
> at the cost of 2 ulongs.
>
> IMO, having separate variables is much simpler.
>
> Do you mean check STCE/VSTCE bit in menvcfg/henvcfg?
>
> If so, I think use a simple "uint64_t menvcfg/henvcfg" may be better,
> then we can only check the 63 bit of them.
>

Which is a bit confusing as the STCE bit in mencfgh/henvcfgh is 31 not 63.
But that's my personal preference.

I will just leave a comment to clarify the confusion for now. I will send a
patch with unified menvcfg and wait for others's feedback.

> Or we should decide where to get this bit from(mencvfg/henvcfg, or
> mencfgh/henvcfgh) based on the MXLEN/HSXLEN.
>
> Regards,
>
> Weiwei Li
>
>
> Similar to  henvcfg and henvcfg.
>>
>> > +    target_ulong senvcfg;
>> > +    target_ulong henvcfg;
>> > +    target_ulong henvcfgh;
>> >   #endif
>> >
>> >       float_status fp_status;
>> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> > index f6f90b5cbd52..afb237c2313b 100644
>> > --- a/target/riscv/cpu_bits.h
>> > +++ b/target/riscv/cpu_bits.h
>> > @@ -177,6 +177,9 @@
>> >   #define CSR_STVEC           0x105
>> >   #define CSR_SCOUNTEREN      0x106
>> >
>> > +/* Supervisor Configuration CSRs */
>> > +#define CSR_SENVCFG         0x10A
>> > +
>> >   /* Supervisor Trap Handling */
>> >   #define CSR_SSCRATCH        0x140
>> >   #define CSR_SEPC            0x141
>> > @@ -204,6 +207,10 @@
>> >   #define CSR_HTIMEDELTA      0x605
>> >   #define CSR_HTIMEDELTAH     0x615
>> >
>> > +/* Hypervisor Configuration CSRs */
>> > +#define CSR_HENVCFG         0x60A
>> > +#define CSR_HENVCFGH        0x61A
>> > +
>> >   /* Virtual CSRs */
>> >   #define CSR_VSSTATUS        0x200
>> >   #define CSR_VSIE            0x204
>> > @@ -218,6 +225,10 @@
>> >   #define CSR_MTINST          0x34a
>> >   #define CSR_MTVAL2          0x34b
>> >
>> > +/* Machine Configuration CSRs */
>> > +#define CSR_MENVCFG         0x30A
>> > +#define CSR_MENVCFGH        0x31A
>> > +
>> >   /* Enhanced Physical Memory Protection (ePMP) */
>> >   #define CSR_MSECCFG         0x747
>> >   #define CSR_MSECCFGH        0x757
>> > @@ -578,6 +589,26 @@ typedef enum RISCVException {
>> >   #define PM_EXT_CLEAN    0x00000002ULL
>> >   #define PM_EXT_DIRTY    0x00000003ULL
>> >
>> > +/* Execution enviornment configuration bits */
>> > +#define MENVCFG_FIOM                       (1 << 0)
>> > +#define MENVCFG_CBE                        0x30000ULL
>> > +#define MENVCFG_CBCFE                      (1 << 6)
>> > +#define MENVCFG_CBZE                       (1 << 7)
>> > +#define MENVCFG_PBMTE                      (1 << 62)
>> > +#define MENVCFG_STCE                       (1 << 63)
>> > +
>> > +#define SENVCFG_FIOM                       MENVCFG_FIOM
>> > +#define SENVCFG_CBE                        MENVCFG_CBE
>> > +#define SENVCFG_CBCFE                      MENVCFG_CBCFE
>> > +#define SENVCFG_CBZE                       MENVCFG_CBZE
>> > +
>> > +#define HENVCFG_FIOM                       MENVCFG_FIOM
>> > +#define HENVCFG_CBE                        MENVCFG_CBE
>> > +#define HENVCFG_CBCFE                      MENVCFG_CBCFE
>> > +#define HENVCFG_CBZE                       MENVCFG_CBZE
>> > +#define HENVCFG_PBMTE                      MENVCFG_PBMTE
>> > +#define HENVCFG_STCE                       MENVCFG_STCE
>> > +
>> >   /* Offsets for every pair of control bits per each priv level */
>> >   #define XS_OFFSET    0ULL
>> >   #define U_OFFSET     2ULL
>> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> > index e66bf2201857..a4bbae7a1bbd 100644
>> > --- a/target/riscv/csr.c
>> > +++ b/target/riscv/csr.c
>> > @@ -853,6 +853,77 @@ static RISCVException write_mtval(CPURISCVState
>> *env, int csrno,
>> >       return RISCV_EXCP_NONE;
>> >   }
>> >
>> > +/* Execution environment configuration setup */
>> > +static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->menvcfg;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->menvcfg = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->menvcfgh;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->menvcfgh = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->senvcfg;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->senvcfg = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->henvcfg;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->henvcfg = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
>> > +                                 target_ulong *val)
>> > +{
>> > +    *val = env->henvcfgh;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> > +static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
>> > +                                  target_ulong val)
>> > +{
>> > +    env->henvcfgh = val;
>> > +    return RISCV_EXCP_NONE;
>> > +}
>> > +
>> >   static RISCVException rmw_mip(CPURISCVState *env, int csrno,
>> >                                 target_ulong *ret_value,
>> >                                 target_ulong new_value, target_ulong
>> write_mask)
>> > @@ -2054,6 +2125,19 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
>> >       [CSR_MTVAL]    = { "mtval",    any,  read_mtval,    write_mtval
>>   },
>> >       [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip
>>   },
>> >
>> > +    /* Execution environment configuration */
>> > +    [CSR_MENVCFG]  = { "menvcfg",  any,     read_menvcfg,
>> write_menvcfg, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +    [CSR_MENVCFGH] = { "menvcfgh", any32,   read_menvcfgh,
>> write_menvcfgh, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +    [CSR_SENVCFG]  = { "senvcfg",  smode,   read_senvcfg,
>> write_senvcfg, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +    [CSR_HENVCFG]  = { "henvcfg",  hmode,   read_henvcfg,
>> write_henvcfg, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh,
>> write_henvcfgh, NULL,
>> > +                                            NULL, NULL,
>> PRIV_VERSION_1_12_0},
>> > +
>> > +
>> Two new lines here.
>>
>
> Fixed it. Thanks.
>
>
>> >       /* Supervisor Trap Setup */
>> >       [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,
>> write_sstatus, NULL,
>> >                                                 read_sstatus_i128
>>            },
>> > diff --git a/target/riscv/machine.c b/target/riscv/machine.c
>> > index 13b9ab375b95..59479a999b87 100644
>> > --- a/target/riscv/machine.c
>> > +++ b/target/riscv/machine.c
>> > @@ -185,6 +185,31 @@ static const VMStateDescription vmstate_rv128 = {
>> >       }
>> >   };
>> >
>> > +/* TODO: henvcfg need both hyper_needed & envcfg_needed */
>> > +static bool envcfg_needed(void *opaque)
>> > +{
>> > +    RISCVCPU *cpu = opaque;
>> > +    CPURISCVState *env = &cpu->env;
>> > +
>> > +    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
>> > +}
>> > +
>> > +static const VMStateDescription vmstate_envcfg = {
>> > +    .name = "cpu/envcfg",
>> > +    .version_id = 1,
>> > +    .minimum_version_id = 1,
>> > +    .needed = envcfg_needed,
>> > +    .fields = (VMStateField[]) {
>> > +        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
>> > +        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
>> > +        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
>> > +        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
>> > +        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
>> > +
>> > +        VMSTATE_END_OF_LIST()
>> > +    }
>> > +};
>> > +
>> >   const VMStateDescription vmstate_riscv_cpu = {
>> >       .name = "cpu",
>> >       .version_id = 3,
>> > @@ -240,6 +265,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>> >           &vmstate_vector,
>> >           &vmstate_pointermasking,
>> >           &vmstate_rv128,
>> > +        &vmstate_envcfg,
>> >           NULL
>> >       }
>> >   };
>>
>> Regards,
>>
>> Weiwei Li
>>
>>
>>
>
> --
> Regards,
> Atish
>
>
Heiko Stübner Feb. 3, 2022, 12:23 p.m. UTC | #5
Hi Atish,

Am Donnerstag, 20. Januar 2022, 21:07:34 CET schrieb Atish Patra:
> The RISC-V privileged specification v1.12 defines few execution
> environment configuration CSRs that can be used enable/disable
> extensions per privilege levels.
> 
> Add the basic support for these CSRs.
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---
>  target/riscv/cpu.h      |  8 ++++
>  target/riscv/cpu_bits.h | 31 +++++++++++++++
>  target/riscv/csr.c      | 84 +++++++++++++++++++++++++++++++++++++++++
>  target/riscv/machine.c  | 26 +++++++++++++
>  4 files changed, 149 insertions(+)
> 
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index f6f90b5cbd52..afb237c2313b 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h

[...]

> @@ -578,6 +589,26 @@ typedef enum RISCVException {
>  #define PM_EXT_CLEAN    0x00000002ULL
>  #define PM_EXT_DIRTY    0x00000003ULL
>  
> +/* Execution enviornment configuration bits */
> +#define MENVCFG_FIOM                       (1 << 0)

> +#define MENVCFG_CBE                        0x30000ULL

Looking both at the cmo spec as well as the most recent privileged spec
(draft) the field is called CBIE it seems.

Also the shift looks wrong. Both cmo as well as privileged spec show
it at bits [5:4] and _not_ [17:16].

Also wouldn't doing it like (_UL(3) << 4) be better to catch such things?

> +#define MENVCFG_CBCFE                      (1 << 6)
> +#define MENVCFG_CBZE                       (1 << 7)
> +#define MENVCFG_PBMTE                      (1 << 62)
> +#define MENVCFG_STCE                       (1 << 63)
> +
> +#define SENVCFG_FIOM                       MENVCFG_FIOM
> +#define SENVCFG_CBE                        MENVCFG_CBE
> +#define SENVCFG_CBCFE                      MENVCFG_CBCFE
> +#define SENVCFG_CBZE                       MENVCFG_CBZE
> +
> +#define HENVCFG_FIOM                       MENVCFG_FIOM
> +#define HENVCFG_CBE                        MENVCFG_CBE
> +#define HENVCFG_CBCFE                      MENVCFG_CBCFE
> +#define HENVCFG_CBZE                       MENVCFG_CBZE
> +#define HENVCFG_PBMTE                      MENVCFG_PBMTE
> +#define HENVCFG_STCE                       MENVCFG_STCE
> +
>  /* Offsets for every pair of control bits per each priv level */
>  #define XS_OFFSET    0ULL
>  #define U_OFFSET     2ULL


Heiko
Atish Kumar Patra Feb. 3, 2022, 8:39 p.m. UTC | #6
On Thu, Feb 3, 2022 at 4:23 AM Heiko Stübner <heiko@sntech.de> wrote:

> Hi Atish,
>
> Am Donnerstag, 20. Januar 2022, 21:07:34 CET schrieb Atish Patra:
> > The RISC-V privileged specification v1.12 defines few execution
> > environment configuration CSRs that can be used enable/disable
> > extensions per privilege levels.
> >
> > Add the basic support for these CSRs.
> >
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > ---
> >  target/riscv/cpu.h      |  8 ++++
> >  target/riscv/cpu_bits.h | 31 +++++++++++++++
> >  target/riscv/csr.c      | 84 +++++++++++++++++++++++++++++++++++++++++
> >  target/riscv/machine.c  | 26 +++++++++++++
> >  4 files changed, 149 insertions(+)
> >
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index f6f90b5cbd52..afb237c2313b 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
>
> [...]
>
> > @@ -578,6 +589,26 @@ typedef enum RISCVException {
> >  #define PM_EXT_CLEAN    0x00000002ULL
> >  #define PM_EXT_DIRTY    0x00000003ULL
> >
> > +/* Execution enviornment configuration bits */
> > +#define MENVCFG_FIOM                       (1 << 0)
>
> > +#define MENVCFG_CBE                        0x30000ULL
>
> Looking both at the cmo spec as well as the most recent privileged spec
> (draft) the field is called CBIE it seems.
>
> Also the shift looks wrong. Both cmo as well as privileged spec show
> it at bits [5:4] and _not_ [17:16].
>
>
This looks like a typo from my side. These bits are reserved in the spec!
Apologies for such a silly mistake. Fixed it in v2.


> Also wouldn't doing it like (_UL(3) << 4) be better to catch such things?
>
>
Of course.


> > +#define MENVCFG_CBCFE                      (1 << 6)
> > +#define MENVCFG_CBZE                       (1 << 7)
> > +#define MENVCFG_PBMTE                      (1 << 62)
> > +#define MENVCFG_STCE                       (1 << 63)
> > +
> > +#define SENVCFG_FIOM                       MENVCFG_FIOM
> > +#define SENVCFG_CBE                        MENVCFG_CBE
> > +#define SENVCFG_CBCFE                      MENVCFG_CBCFE
> > +#define SENVCFG_CBZE                       MENVCFG_CBZE
> > +
> > +#define HENVCFG_FIOM                       MENVCFG_FIOM
> > +#define HENVCFG_CBE                        MENVCFG_CBE
> > +#define HENVCFG_CBCFE                      MENVCFG_CBCFE
> > +#define HENVCFG_CBZE                       MENVCFG_CBZE
> > +#define HENVCFG_PBMTE                      MENVCFG_PBMTE
> > +#define HENVCFG_STCE                       MENVCFG_STCE
> > +
> >  /* Offsets for every pair of control bits per each priv level */
> >  #define XS_OFFSET    0ULL
> >  #define U_OFFSET     2ULL
>
>
> Heiko
>
>
>
diff mbox series

Patch

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 7f87917204c5..b9462300a472 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -264,6 +264,14 @@  struct CPURISCVState {
     target_ulong spmbase;
     target_ulong upmmask;
     target_ulong upmbase;
+
+    /* CSRs for execution enviornment configuration */
+
+    target_ulong menvcfg;
+    target_ulong menvcfgh;
+    target_ulong senvcfg;
+    target_ulong henvcfg;
+    target_ulong henvcfgh;
 #endif
 
     float_status fp_status;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index f6f90b5cbd52..afb237c2313b 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -177,6 +177,9 @@ 
 #define CSR_STVEC           0x105
 #define CSR_SCOUNTEREN      0x106
 
+/* Supervisor Configuration CSRs */
+#define CSR_SENVCFG         0x10A
+
 /* Supervisor Trap Handling */
 #define CSR_SSCRATCH        0x140
 #define CSR_SEPC            0x141
@@ -204,6 +207,10 @@ 
 #define CSR_HTIMEDELTA      0x605
 #define CSR_HTIMEDELTAH     0x615
 
+/* Hypervisor Configuration CSRs */
+#define CSR_HENVCFG         0x60A
+#define CSR_HENVCFGH        0x61A
+
 /* Virtual CSRs */
 #define CSR_VSSTATUS        0x200
 #define CSR_VSIE            0x204
@@ -218,6 +225,10 @@ 
 #define CSR_MTINST          0x34a
 #define CSR_MTVAL2          0x34b
 
+/* Machine Configuration CSRs */
+#define CSR_MENVCFG         0x30A
+#define CSR_MENVCFGH        0x31A
+
 /* Enhanced Physical Memory Protection (ePMP) */
 #define CSR_MSECCFG         0x747
 #define CSR_MSECCFGH        0x757
@@ -578,6 +589,26 @@  typedef enum RISCVException {
 #define PM_EXT_CLEAN    0x00000002ULL
 #define PM_EXT_DIRTY    0x00000003ULL
 
+/* Execution enviornment configuration bits */
+#define MENVCFG_FIOM                       (1 << 0)
+#define MENVCFG_CBE                        0x30000ULL
+#define MENVCFG_CBCFE                      (1 << 6)
+#define MENVCFG_CBZE                       (1 << 7)
+#define MENVCFG_PBMTE                      (1 << 62)
+#define MENVCFG_STCE                       (1 << 63)
+
+#define SENVCFG_FIOM                       MENVCFG_FIOM
+#define SENVCFG_CBE                        MENVCFG_CBE
+#define SENVCFG_CBCFE                      MENVCFG_CBCFE
+#define SENVCFG_CBZE                       MENVCFG_CBZE
+
+#define HENVCFG_FIOM                       MENVCFG_FIOM
+#define HENVCFG_CBE                        MENVCFG_CBE
+#define HENVCFG_CBCFE                      MENVCFG_CBCFE
+#define HENVCFG_CBZE                       MENVCFG_CBZE
+#define HENVCFG_PBMTE                      MENVCFG_PBMTE
+#define HENVCFG_STCE                       MENVCFG_STCE
+
 /* Offsets for every pair of control bits per each priv level */
 #define XS_OFFSET    0ULL
 #define U_OFFSET     2ULL
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e66bf2201857..a4bbae7a1bbd 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -853,6 +853,77 @@  static RISCVException write_mtval(CPURISCVState *env, int csrno,
     return RISCV_EXCP_NONE;
 }
 
+/* Execution environment configuration setup */
+static RISCVException read_menvcfg(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->menvcfg;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->menvcfg = val;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_menvcfgh(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->menvcfgh;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->menvcfgh = val;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->senvcfg;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_senvcfg(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->senvcfg = val;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->henvcfg;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->henvcfg = val;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
+                                 target_ulong *val)
+{
+    *val = env->henvcfgh;
+    return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
+                                  target_ulong val)
+{
+    env->henvcfgh = val;
+    return RISCV_EXCP_NONE;
+}
+
 static RISCVException rmw_mip(CPURISCVState *env, int csrno,
                               target_ulong *ret_value,
                               target_ulong new_value, target_ulong write_mask)
@@ -2054,6 +2125,19 @@  riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_MTVAL]    = { "mtval",    any,  read_mtval,    write_mtval    },
     [CSR_MIP]      = { "mip",      any,  NULL,    NULL, rmw_mip        },
 
+    /* Execution environment configuration */
+    [CSR_MENVCFG]  = { "menvcfg",  any,     read_menvcfg,  write_menvcfg, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+    [CSR_MENVCFGH] = { "menvcfgh", any32,   read_menvcfgh, write_menvcfgh, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+    [CSR_SENVCFG]  = { "senvcfg",  smode,   read_senvcfg,  write_senvcfg, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+    [CSR_HENVCFG]  = { "henvcfg",  hmode,   read_henvcfg, write_henvcfg, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+    [CSR_HENVCFGH] = { "henvcfgh", hmode32, read_henvcfgh, write_henvcfgh, NULL,
+                                            NULL, NULL, PRIV_VERSION_1_12_0},
+
+
     /* Supervisor Trap Setup */
     [CSR_SSTATUS]    = { "sstatus",    smode, read_sstatus,    write_sstatus, NULL,
                                               read_sstatus_i128                 },
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 13b9ab375b95..59479a999b87 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -185,6 +185,31 @@  static const VMStateDescription vmstate_rv128 = {
     }
 };
 
+/* TODO: henvcfg need both hyper_needed & envcfg_needed */
+static bool envcfg_needed(void *opaque)
+{
+    RISCVCPU *cpu = opaque;
+    CPURISCVState *env = &cpu->env;
+
+    return (env->priv_ver >= PRIV_VERSION_1_12_0 ? 1 : 0);
+}
+
+static const VMStateDescription vmstate_envcfg = {
+    .name = "cpu/envcfg",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = envcfg_needed,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINTTL(env.menvcfg, RISCVCPU),
+        VMSTATE_UINTTL(env.menvcfgh, RISCVCPU),
+        VMSTATE_UINTTL(env.senvcfg, RISCVCPU),
+        VMSTATE_UINTTL(env.henvcfg, RISCVCPU),
+        VMSTATE_UINTTL(env.henvcfgh, RISCVCPU),
+
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 const VMStateDescription vmstate_riscv_cpu = {
     .name = "cpu",
     .version_id = 3,
@@ -240,6 +265,7 @@  const VMStateDescription vmstate_riscv_cpu = {
         &vmstate_vector,
         &vmstate_pointermasking,
         &vmstate_rv128,
+        &vmstate_envcfg,
         NULL
     }
 };