Re: [PATCH v6 2/5] target/riscv: smstateen check for h/senvcfg

qemu-devel

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

Re: [PATCH v6 2/5] target/riscv: smstateen check for h/senvcfg

From:	Weiwei Li
Subject:	Re: [PATCH v6 2/5] target/riscv: smstateen check for h/senvcfg
Date:	Thu, 28 Jul 2022 15:59:46 +0800
User-agent:	Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0


在 2022/7/28 下午2:41, Mayuresh Chitale 写道:

On Fri, 2022-07-22 at 08:45 +0800, Weiwei Li wrote:

在 2022/7/21 下午11:31, Mayuresh Chitale 写道:

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 | 100
+++++++++++++++++++++++++++++++++++++++++----
   1 file changed, 93 insertions(+), 7 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 27032a416c..ab06b117f9 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -40,6 +40,55 @@ void riscv_set_csr_ops(int csrno,
riscv_csr_operations *ops)
   }

/* Predicates */

+#if !defined(CONFIG_USER_ONLY)
+static RISCVException smstateen_acc_ok(CPURISCVState *env, int
index,
+                                       uint64_t bit)
+{
+    bool virt = riscv_cpu_virt_enabled(env);
+    CPUState *cs = env_cpu(env);
+    RISCVCPU *cpu = RISCV_CPU(cs);
+    uint64_t hstateen = env->hstateen[index];
+    uint64_t sstateen = env->sstateen[index];
+
+    if (env->priv == PRV_M || !cpu->cfg.ext_smstateen) {
+        return RISCV_EXCP_NONE;
+    }
+
+    if (!(env->mstateen[index] & bit)) {
+        return RISCV_EXCP_ILLEGAL_INST;
+    }
+
+    /*
+     * Treat hstateen and sstateen as read-only zero if
mstateen0.staten
+     * is clear.
+     */
+    if (!(env->mstateen[index] & SMSTATEEN_STATEN)) {
+        hstateen = 0;
+        sstateen = 0;
+    }
+
+    if (virt) {
+        if (!(hstateen & bit)) {
+            return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
+        }
+        /*
+         * Treat sstateen as read-only zero if hstateen0.staten is
clear.
+         */
+        if (!(hstateen & SMSTATEEN_STATEN)) {
+            sstateen = 0;
+        }
+    }
+
+    if (env->priv == PRV_U && riscv_has_ext(env, RVS)) {
+        if (!(sstateen & bit)) {
+            return RISCV_EXCP_ILLEGAL_INST;
+        }
+    }
+
+    return RISCV_EXCP_NONE;
+}
+#endif
+
   static RISCVException fs(CPURISCVState *env, int csrno)
   {
   #if !defined(CONFIG_USER_ONLY)
@@ -1708,6 +1757,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, 0, SMSTATEEN0_HSENVCFG);

I think it's better to add this check into the predicate.

By the way, sharing the same function for all related csrs  is
easily
misunderstood. However, It seems correct.

We use the default global predicates ie hmode/smode etc for the envcfg
registers and the global predicates cant be modified to include
additional checks for envcfg registers. We could implement new
predicates for envcfg but I think the current approach is simpler.


OK. It's acceptable to me.

Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

Regards,

Weiwei Li

Regards,

Weiwei Li

+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
       *val = env->senvcfg;
       return RISCV_EXCP_NONE;
   }
@@ -1716,15 +1772,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, 0, 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, 0, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
       *val = env->henvcfg;
       return RISCV_EXCP_NONE;
   }
@@ -1733,6 +1801,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, 0, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }

if (riscv_cpu_mxl(env) == MXL_RV64) {

           mask |= HENVCFG_PBMTE | HENVCFG_STCE;
@@ -1746,6 +1820,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, 0, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }
+
       *val = env->henvcfg >> 32;
       return RISCV_EXCP_NONE;
   }
@@ -1755,9 +1836,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, 0, SMSTATEEN0_HSENVCFG);
+    if (ret != RISCV_EXCP_NONE) {
+        return ret;
+    }

+ env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);

       return RISCV_EXCP_NONE;
   }

@@ -1789,7 +1875,7 @@ static RISCVException

write_mstateen(CPURISCVState *env, int csrno,
   static RISCVException write_mstateen0(CPURISCVState *env, int
csrno,
                                         target_ulong new_val)
   {
-    uint64_t wr_mask = SMSTATEEN_STATEN;
+    uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;

return write_mstateen(env, csrno, wr_mask, new_val);

   }
@@ -1836,7 +1922,7 @@ static RISCVException
write_mstateenh(CPURISCVState *env, int csrno,
   static RISCVException write_mstateen0h(CPURISCVState *env, int
csrno,
                                         target_ulong new_val)
   {
-    uint64_t wr_mask = SMSTATEEN_STATEN;
+    uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;

return write_mstateenh(env, csrno, wr_mask, new_val);

   }
@@ -1885,7 +1971,7 @@ static RISCVException
write_hstateen(CPURISCVState *env, int csrno,
   static RISCVException write_hstateen0(CPURISCVState *env, int
csrno,
                                         target_ulong new_val)
   {
-    uint64_t wr_mask = SMSTATEEN_STATEN;
+    uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;

return write_hstateen(env, csrno, wr_mask, new_val);

   }
@@ -1936,7 +2022,7 @@ static RISCVException
write_hstateenh(CPURISCVState *env, int csrno,
   static RISCVException write_hstateen0h(CPURISCVState *env, int
csrno,
                                          target_ulong new_val)
   {
-    uint64_t wr_mask = SMSTATEEN_STATEN;
+    uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;

return write_hstateenh(env, csrno, wr_mask, new_val);

   }
@@ -1995,7 +2081,7 @@ static RISCVException
write_sstateen(CPURISCVState *env, int csrno,
   static RISCVException write_sstateen0(CPURISCVState *env, int
csrno,
                                         target_ulong new_val)
   {
-    uint64_t wr_mask = SMSTATEEN_STATEN;
+    uint64_t wr_mask = SMSTATEEN_STATEN | SMSTATEEN0_HSENVCFG;

return write_sstateen(env, csrno, wr_mask, new_val);

[Prev in Thread]

Current Thread

[Next in Thread]

[PATCH v6 0/5] RISC-V Smstateen support, Mayuresh Chitale, 2022/07/21
- [PATCH v6 1/5] target/riscv: Add smstateen support, Mayuresh Chitale, 2022/07/21
  - Re: [PATCH v6 1/5] target/riscv: Add smstateen support, Weiwei Li, 2022/07/21
    - Re: [PATCH v6 1/5] target/riscv: Add smstateen support, Mayuresh Chitale, 2022/07/24
    - Re: [PATCH v6 1/5] target/riscv: Add smstateen support, Weiwei Li, 2022/07/25
    - Re: [PATCH v6 1/5] target/riscv: Add smstateen support, Mayuresh Chitale, 2022/07/28
- [PATCH v6 2/5] target/riscv: smstateen check for h/senvcfg, Mayuresh Chitale, 2022/07/21
  - Re: [PATCH v6 2/5] target/riscv: smstateen check for h/senvcfg, Weiwei Li, 2022/07/21
    - Re: [PATCH v6 2/5] target/riscv: smstateen check for h/senvcfg, Mayuresh Chitale, 2022/07/28
    - Re: [PATCH v6 2/5] target/riscv: smstateen check for h/senvcfg, Weiwei Li <=
- [PATCH v6 3/5] target/riscv: smstateen check for fcsr, Mayuresh Chitale, 2022/07/21
  - Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr, Weiwei Li, 2022/07/21
    - Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr, Mayuresh Chitale, 2022/07/24
    - Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr, Weiwei Li, 2022/07/25
    - Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr, Mayuresh Chitale, 2022/07/28
    - Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr, Weiwei Li, 2022/07/28
    - Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr, Ben Dooks, 2022/07/28
    - Re: [PATCH v6 3/5] target/riscv: smstateen check for fcsr, Mayuresh Chitale, 2022/07/29
- [PATCH v6 4/5] target/riscv: smstateen check for AIA/IMSIC, Mayuresh Chitale, 2022/07/21
  - Re: [PATCH v6 4/5] target/riscv: smstateen check for AIA/IMSIC, Weiwei Li, 2022/07/21

Prev by Date: Re: virtio: why no full reset on virtio_set_status 0 ?
Next by Date: [PATCH] hw/nvme: Add helper functions for qid-db conversion
Previous by thread: Re: [PATCH v6 2/5] target/riscv: smstateen check for h/senvcfg
Next by thread: [PATCH v6 3/5] target/riscv: smstateen check for fcsr
Index(es):
- Date
- Thread