On Wed, Jan 26, 2022 at 12:37 AM Weiwei Li <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>
> > ---
> > 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
>
>
>
--
Regards,
Atish
