qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Atish Kumar Patra
Subject: Re: [RFC 4/5] target/riscv: Add *envcfg* CSRs support
Date: Mon, 31 Jan 2022 12:05:11 -0800



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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]