Re: [PATCH v3 4/7] target/riscv: Flush TLB only when pmpcfg/pmpaddr really changes

[LIU Zhiwei]

On 2023/4/19 11:27, Weiwei Li wrote:

TLB needn't be flushed when pmpcfg/pmpaddr don't changes.

Signed-off-by: Weiwei Li <liwei...@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqi...@iscas.ac.cn>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
---
  target/riscv/pmp.c | 24 ++++++++++++++++--------
  1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index bcd190d3a3..7feaddd7eb 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -26,7 +26,7 @@
  #include "trace.h"
  #include "exec/exec-all.h"

-static void pmp_write_cfg(CPURISCVState *env, uint32_t addr_index,

+static bool pmp_write_cfg(CPURISCVState *env, uint32_t addr_index,
                            uint8_t val);
  static uint8_t pmp_read_cfg(CPURISCVState *env, uint32_t addr_index);
  static void pmp_update_rule(CPURISCVState *env, uint32_t pmp_index);
@@ -83,7 +83,7 @@ static inline uint8_t pmp_read_cfg(CPURISCVState *env, 
uint32_t pmp_index)
   * Accessor to set the cfg reg for a specific PMP/HART
   * Bounds checks and relevant lock bit.
   */
-static void pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val)
+static bool pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val)
  {
      if (pmp_index < MAX_RISCV_PMPS) {
          bool locked = true;
@@ -119,14 +119,17 @@ static void pmp_write_cfg(CPURISCVState *env, uint32_t 
pmp_index, uint8_t val)

if (locked) {

              qemu_log_mask(LOG_GUEST_ERROR, "ignoring pmpcfg write - 
locked\n");
-        } else {
+        } else if (env->pmp_state.pmp[pmp_index].cfg_reg != val) {
              env->pmp_state.pmp[pmp_index].cfg_reg = val;
              pmp_update_rule(env, pmp_index);
+            return true;
          }
      } else {
          qemu_log_mask(LOG_GUEST_ERROR,
                        "ignoring pmpcfg write - out of bounds\n");
      }
+
+    return false;
  }

static void pmp_decode_napot(target_ulong a, target_ulong *sa,

@@ -477,16 +480,19 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t 
reg_index,
      int i;
      uint8_t cfg_val;
      int pmpcfg_nums = 2 << riscv_cpu_mxl(env);
+    bool modified = false;

trace_pmpcfg_csr_write(env->mhartid, reg_index, val); for (i = 0; i < pmpcfg_nums; i++) {

          cfg_val = (val >> 8 * i)  & 0xff;
-        pmp_write_cfg(env, (reg_index * 4) + i, cfg_val);
+        modified |= pmp_write_cfg(env, (reg_index * 4) + i, cfg_val);
      }

/* If PMP permission of any addr has been changed, flush TLB pages. */

-    tlb_flush(env_cpu(env));
+    if (modified) {
+        tlb_flush(env_cpu(env));
+    }
  }

@@ -535,9 +541,11 @@ void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,

          }

if (!pmp_is_locked(env, addr_index)) {

-            env->pmp_state.pmp[addr_index].addr_reg = val;
-            pmp_update_rule(env, addr_index);
-            tlb_flush(env_cpu(env));
+            if (env->pmp_state.pmp[addr_index].addr_reg != val) {
+                env->pmp_state.pmp[addr_index].addr_reg = val;
+                pmp_update_rule(env, addr_index);
+                tlb_flush(env_cpu(env));
+            }

Reviewed-by: LIU Zhiwei <zhiwei_...@linux.alibaba.com>

Zhiwei

          } else {
              qemu_log_mask(LOG_GUEST_ERROR,
                            "ignoring pmpaddr write - locked\n");