qemu-riscv
[Top][All Lists]
Advanced

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

Re: [PATCH v3 1/1] target/riscv: Fix PMP propagation for tlb


From: LIU Zhiwei
Subject: Re: [PATCH v3 1/1] target/riscv: Fix PMP propagation for tlb
Date: Fri, 17 Dec 2021 11:28:54 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.14.0


On 2021/11/23 下午5:09, LIU Zhiwei wrote:
Only the pmp index that be checked by pmp_hart_has_privs can be used
by pmp_get_tlb_size to avoid an error pmp index.

Before modification, we may use an error pmp index. For example,
we check address 0x4fc, and the size 0x4 in pmp_hart_has_privs. If there
is an pmp rule, valid range is [0x4fc, 0x500), then pmp_hart_has_privs
will return true;

However, this checked pmp index is discarded as pmp_hart_has_privs
return bool value. In pmp_is_range_in_tlb, it will traverse all pmp
rules. The tlb_sa will be 0x0, and tlb_ea will be 0x4fff. If there is
a pmp rule [0x10, 0x4]. It will be misused as it is legal in
pmp_get_tlb_size.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
  target/riscv/cpu_helper.c | 16 ++++++-----
  target/riscv/pmp.c        | 56 +++++++++++++--------------------------
  target/riscv/pmp.h        |  6 ++---
  3 files changed, 31 insertions(+), 47 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 9eeed38c7e..4239bd2ca5 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -362,24 +362,26 @@ static int get_physical_address_pmp(CPURISCVState *env, 
int *prot,
                                      int mode)
  {
      pmp_priv_t pmp_priv;
-    target_ulong tlb_size_pmp = 0;
+    int pmp_index = -1;
if (!riscv_feature(env, RISCV_FEATURE_PMP)) {
          *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
          return TRANSLATE_SUCCESS;
      }
- if (!pmp_hart_has_privs(env, addr, size, 1 << access_type, &pmp_priv,
-                            mode)) {
+    pmp_index = pmp_hart_has_privs(env, addr, size, 1 << access_type,
+                                   &pmp_priv, mode);
+    if (pmp_index < 0) {
          *prot = 0;
          return TRANSLATE_PMP_FAIL;
      }
*prot = pmp_priv_to_page_prot(pmp_priv);
-    if (tlb_size != NULL) {
-        if (pmp_is_range_in_tlb(env, addr & ~(*tlb_size - 1), &tlb_size_pmp)) {
-            *tlb_size = tlb_size_pmp;
-        }
+    if ((tlb_size != NULL) && pmp_index != MAX_RISCV_PMPS) {
+        target_ulong tlb_sa = addr & ~(*tlb_size - 1);
+        target_ulong tlb_ea = tlb_sa + *tlb_size - 1;
+
+        *tlb_size = pmp_get_tlb_size(env, pmp_index, tlb_sa, tlb_ea);
      }
return TRANSLATE_SUCCESS;
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 54abf42583..1172142e34 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -297,8 +297,11 @@ static bool pmp_hart_has_privs_default(CPURISCVState *env, 
target_ulong addr,
/*
   * Check if the address has required RWX privs to complete desired operation
+ * Return PMP rule index if a pmp rule match
+ * Return MAX_RISCV_PMPS if default match
+ * Return negtive value if no match
   */
-bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
+int pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
      target_ulong size, pmp_priv_t privs, pmp_priv_t *allowed_privs,
      target_ulong mode)
  {
@@ -310,8 +313,10 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong 
addr,
/* Short cut if no rules */
      if (0 == pmp_get_num_rules(env)) {
-        return pmp_hart_has_privs_default(env, addr, size, privs,
-                                          allowed_privs, mode);
+        if (pmp_hart_has_privs_default(env, addr, size, privs,
+                                       allowed_privs, mode)) {
+            ret = MAX_RISCV_PMPS;
+        }
      }
if (size == 0) {
@@ -338,7 +343,7 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong 
addr,
          if ((s + e) == 1) {
              qemu_log_mask(LOG_GUEST_ERROR,
                            "pmp violation - access is partially inside\n");
-            ret = 0;
+            ret = -1;
              break;
          }
@@ -441,18 +446,22 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
                  }
              }
- ret = ((privs & *allowed_privs) == privs);
+            if ((privs & *allowed_privs) == privs) {
+                ret = i;
+            }
              break;
          }
      }
/* No rule matched */
      if (ret == -1) {
-        return pmp_hart_has_privs_default(env, addr, size, privs,
-                                          allowed_privs, mode);
+        if (pmp_hart_has_privs_default(env, addr, size, privs,
+                                       allowed_privs, mode)) {
+            ret = MAX_RISCV_PMPS;
+        }
      }
- return ret == 1 ? true : false;
+    return ret;
  }
/*
@@ -595,8 +604,8 @@ target_ulong mseccfg_csr_read(CPURISCVState *env)
   * Calculate the TLB size if the start address or the end address of
   * PMP entry is presented in the TLB page.
   */
-static target_ulong pmp_get_tlb_size(CPURISCVState *env, int pmp_index,
-                                     target_ulong tlb_sa, target_ulong tlb_ea)
+target_ulong pmp_get_tlb_size(CPURISCVState *env, int pmp_index,
+                              target_ulong tlb_sa, target_ulong tlb_ea)
  {
      target_ulong pmp_sa = env->pmp_state.addr[pmp_index].sa;
      target_ulong pmp_ea = env->pmp_state.addr[pmp_index].ea;

Error here.

Add "

+    if (pmp_sa <= tlb_sa && pmp_ea >= tlb_ea) {
+        return tlb_ea - tlb_sa + 1;
+    }

"

Thanks,
Zhiwei

@@ -616,33 +625,6 @@ static target_ulong pmp_get_tlb_size(CPURISCVState *env, 
int pmp_index,
      return 0;
  }
-/*
- * Check is there a PMP entry which range covers this page. If so,
- * try to find the minimum granularity for the TLB size.
- */
-bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr tlb_sa,
-                         target_ulong *tlb_size)
-{
-    int i;
-    target_ulong val;
-    target_ulong tlb_ea = (tlb_sa + TARGET_PAGE_SIZE - 1);
-
-    for (i = 0; i < MAX_RISCV_PMPS; i++) {
-        val = pmp_get_tlb_size(env, i, tlb_sa, tlb_ea);
-        if (val) {
-            if (*tlb_size == 0 || *tlb_size > val) {
-                *tlb_size = val;
-            }
-        }
-    }
-
-    if (*tlb_size != 0) {
-        return true;
-    }
-
-    return false;
-}
-
  /*
   * Convert PMP privilege to TLB page privilege.
   */
diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
index a9a0b363a7..94c0b960fb 100644
--- a/target/riscv/pmp.h
+++ b/target/riscv/pmp.h
@@ -68,11 +68,11 @@ target_ulong mseccfg_csr_read(CPURISCVState *env);
  void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
      target_ulong val);
  target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index);
-bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
+int pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
      target_ulong size, pmp_priv_t privs, pmp_priv_t *allowed_privs,
      target_ulong mode);
-bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr tlb_sa,
-                         target_ulong *tlb_size);
+target_ulong pmp_get_tlb_size(CPURISCVState *env, int pmp_index,
+                              target_ulong tlb_sa, target_ulong tlb_ea);
  void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index);
  void pmp_update_rule_nums(CPURISCVState *env);
  uint32_t pmp_get_num_rules(CPURISCVState *env);



reply via email to

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