[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH v5 2/4] target/riscv: smstateen check for h/senvcfg
From: |
Alistair Francis |
Subject: |
Re: [RFC PATCH v5 2/4] target/riscv: smstateen check for h/senvcfg |
Date: |
Thu, 16 Jun 2022 16:54:11 +1000 |
On Sat, Jun 4, 2022 at 2:16 AM Mayuresh Chitale
<mchitale@ventanamicro.com> wrote:
>
> Accesses to henvcfg, henvcfgh and senvcfg are allowed
> only if corresponding bit in mstateen0/hstateen0 is
> enabled. Otherwise an illegal instruction trap is
> generated.
>
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> ---
> target/riscv/csr.c | 84 ++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 78 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 324fefce59..ae91ae1f7e 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -39,6 +39,37 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations
> *ops)
> }
>
> /* Predicates */
> +static RISCVException smstateen_acc_ok(CPURISCVState *env, int mode, int bit)
> +{
> + CPUState *cs = env_cpu(env);
> + RISCVCPU *cpu = RISCV_CPU(cs);
> + bool virt = riscv_cpu_virt_enabled(env);
> +
> + if (!cpu->cfg.ext_smstateen) {
> + return RISCV_EXCP_NONE;
> + }
> +
> +#if !defined(CONFIG_USER_ONLY)
> + if (!(env->mstateen[0] & 1UL << bit)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> +
> + if (virt) {
> + if (!(env->hstateen[0] & 1UL << bit)) {
> + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> + }
> + }
> +
> + if (mode == PRV_U) {
> + if (!(env->sstateen[0] & 1UL << bit)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + }
> +#endif
> +
> + return RISCV_EXCP_NONE;
> +}
> +
> static RISCVException fs(CPURISCVState *env, int csrno)
> {
> #if !defined(CONFIG_USER_ONLY)
> @@ -1557,6 +1588,13 @@ static RISCVException write_menvcfgh(CPURISCVState
> *env, int csrno,
> static RISCVException read_senvcfg(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> + RISCVException ret;
> +
> + ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
Couldn't this be part of the original permission check so we don't
need a second check?
Alistair
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
> +
> *val = env->senvcfg;
> return RISCV_EXCP_NONE;
> }
> @@ -1565,15 +1603,27 @@ static RISCVException write_senvcfg(CPURISCVState
> *env, int csrno,
> target_ulong val)
> {
> uint64_t mask = SENVCFG_FIOM | SENVCFG_CBIE | SENVCFG_CBCFE |
> SENVCFG_CBZE;
> + RISCVException ret;
>
> - env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
> + ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
>
> + env->senvcfg = (env->senvcfg & ~mask) | (val & mask);
> return RISCV_EXCP_NONE;
> }
>
> static RISCVException read_henvcfg(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> + RISCVException ret;
> +
> + ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
> +
> *val = env->henvcfg;
> return RISCV_EXCP_NONE;
> }
> @@ -1582,6 +1632,12 @@ static RISCVException write_henvcfg(CPURISCVState
> *env, int csrno,
> target_ulong val)
> {
> uint64_t mask = HENVCFG_FIOM | HENVCFG_CBIE | HENVCFG_CBCFE |
> HENVCFG_CBZE;
> + RISCVException ret;
> +
> + ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
>
> if (riscv_cpu_mxl(env) == MXL_RV64) {
> mask |= HENVCFG_PBMTE | HENVCFG_STCE;
> @@ -1595,6 +1651,13 @@ static RISCVException write_henvcfg(CPURISCVState
> *env, int csrno,
> static RISCVException read_henvcfgh(CPURISCVState *env, int csrno,
> target_ulong *val)
> {
> + RISCVException ret;
> +
> + ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
> +
> *val = env->henvcfg >> 32;
> return RISCV_EXCP_NONE;
> }
> @@ -1604,9 +1667,14 @@ static RISCVException write_henvcfgh(CPURISCVState
> *env, int csrno,
> {
> uint64_t mask = HENVCFG_PBMTE | HENVCFG_STCE;
> uint64_t valh = (uint64_t)val << 32;
> + RISCVException ret;
>
> - env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
> + ret = smstateen_acc_ok(env, PRV_S, SMSTATEEN0_HSENVCFG);
> + if (ret != RISCV_EXCP_NONE) {
> + return ret;
> + }
>
> + env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
> return RISCV_EXCP_NONE;
> }
>
> @@ -1628,7 +1696,8 @@ static RISCVException write_mstateen(CPURISCVState
> *env, int csrno,
> target_ulong new_val)
> {
> uint64_t *reg;
> - uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
> + uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
> + (1UL << SMSTATEEN0_HSENVCFG);
>
> reg = &env->mstateen[csrno - CSR_MSTATEEN0];
> write_smstateen(env, reg, wr_mask, new_val);
> @@ -1649,7 +1718,8 @@ static RISCVException write_mstateenh(CPURISCVState
> *env, int csrno,
> {
> uint64_t *reg;
> uint64_t val;
> - uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
> + uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
> + (1UL << SMSTATEEN0_HSENVCFG);
>
> reg = &env->mstateen[csrno - CSR_MSTATEEN0H];
> val = (uint64_t)new_val << 32;
> @@ -1671,7 +1741,8 @@ static RISCVException write_hstateen(CPURISCVState
> *env, int csrno,
> target_ulong new_val)
> {
> uint64_t *reg;
> - uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
> + uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
> + (1UL << SMSTATEEN0_HSENVCFG);
> int index = csrno - CSR_HSTATEEN0;
>
> reg = &env->hstateen[index];
> @@ -1694,8 +1765,9 @@ static RISCVException write_hstateenh(CPURISCVState
> *env, int csrno,
> {
> uint64_t *reg;
> uint64_t val;
> - uint64_t wr_mask = 1UL << SMSTATEEN_STATEN;
> int index = csrno - CSR_HSTATEEN0H;
> + uint64_t wr_mask = (1UL << SMSTATEEN_STATEN) |
> + (1UL << SMSTATEEN0_HSENVCFG);
>
> reg = &env->hstateen[index];
> val = (uint64_t)new_val << 32;
> --
> 2.25.1
>
>