在 2022/1/29 上午9:28, Atish Patra 写道:


On Wed, Jan 26, 2022 at 12:37 AM Weiwei Li <liwei...@iscas.ac.cn <mailto:liwei...@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 <ati...@rivosinc.com
    <mailto:ati...@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

Reply via email to