qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] target/riscv: PMP violation due to wrong size parameter
@ 2019-10-11 23:14 Dayeol Lee
  2019-10-12  2:37 ` Jonathan Behrens
  0 siblings, 1 reply; 15+ messages in thread
From: Dayeol Lee @ 2019-10-11 23:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: open list:RISC-V TCG CPUs, Sagar Karandikar, Dayeol Lee,
	Bastian Koppelmann, Palmer Dabbelt, Richard Henderson,
	Alistair Francis, diodesign

riscv_cpu_tlb_fill() uses the `size` parameter to check PMP violation
using pmp_hart_has_privs().
However, if the size is unknown (=0), the ending address will be
`addr - 1` as it is `addr + size - 1` in `pmp_hart_has_privs()`.
This always causes a false PMP violation on the starting address of the
range, as `addr - 1` is not in the range.

In order to fix, we just assume that all bytes from addr to the end of
the page will be accessed if the size is unknown.

Signed-off-by: Dayeol Lee <dayeol@berkeley.edu>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu_helper.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index e32b6126af..7d9a22b601 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -441,6 +441,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
     CPURISCVState *env = &cpu->env;
     hwaddr pa = 0;
     int prot;
+    int pmp_size = 0;
     bool pmp_violation = false;
     int ret = TRANSLATE_FAIL;
     int mode = mmu_idx;
@@ -460,9 +461,19 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                   "%s address=%" VADDR_PRIx " ret %d physical " TARGET_FMT_plx
                   " prot %d\n", __func__, address, ret, pa, prot);
 
+    /*
+     * if size is unknown (0), assume that all bytes
+     * from addr to the end of the page will be accessed.
+     */
+    if (size == 0) {
+        pmp_size = -(address | TARGET_PAGE_MASK);
+    } else {
+        pmp_size = size;
+    }
+
     if (riscv_feature(env, RISCV_FEATURE_PMP) &&
         (ret == TRANSLATE_SUCCESS) &&
-        !pmp_hart_has_privs(env, pa, size, 1 << access_type, mode)) {
+        !pmp_hart_has_privs(env, pa, pmp_size, 1 << access_type, mode)) {
         ret = TRANSLATE_PMP_FAIL;
     }
     if (ret == TRANSLATE_PMP_FAIL) {
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH] target/riscv: PMP violation due to wrong size parameter
@ 2019-10-22 21:21 Dayeol Lee
  2019-10-23 15:50 ` Palmer Dabbelt
  0 siblings, 1 reply; 15+ messages in thread
From: Dayeol Lee @ 2019-10-22 21:21 UTC (permalink / raw)
  To: qemu-devel
  Cc: open list:RISC-V TCG CPUs, Sagar Karandikar, Dayeol Lee,
	Bastian Koppelmann, Palmer Dabbelt, Richard Henderson,
	Alistair Francis, diodesign

riscv_cpu_tlb_fill() uses the `size` parameter to check PMP violation
using pmp_hart_has_privs().
However, if the size is unknown (=0), the ending address will be
`addr - 1` as it is `addr + size - 1` in `pmp_hart_has_privs()`.
This always causes a false PMP violation on the starting address of the
range, as `addr - 1` is not in the range.

In order to fix, we just assume that all bytes from addr to the end of
the page will be accessed if the size is unknown.

Signed-off-by: Dayeol Lee <dayeol@berkeley.edu>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/pmp.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 958c7502a0..7a9fd415ba 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -232,6 +232,7 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
 {
     int i = 0;
     int ret = -1;
+    int pmp_size = 0;
     target_ulong s = 0;
     target_ulong e = 0;
     pmp_priv_t allowed_privs = 0;
@@ -241,11 +242,21 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
         return true;
     }
 
+    /*
+     * if size is unknown (0), assume that all bytes
+     * from addr to the end of the page will be accessed.
+     */
+    if (size == 0) {
+        pmp_size = -(addr | TARGET_PAGE_MASK);
+    } else {
+        pmp_size = size;
+    }
+
     /* 1.10 draft priv spec states there is an implicit order
          from low to high */
     for (i = 0; i < MAX_RISCV_PMPS; i++) {
         s = pmp_is_in_range(env, i, addr);
-        e = pmp_is_in_range(env, i, addr + size - 1);
+        e = pmp_is_in_range(env, i, addr + pmp_size - 1);
 
         /* partially inside */
         if ((s + e) == 1) {
-- 
2.23.0



^ permalink raw reply related	[flat|nested] 15+ messages in thread
* [PATCH] target/riscv: PMP violation due to wrong size parameter
@ 2019-10-07  5:28 Dayeol Lee
  2019-10-07  6:20 ` no-reply
  2019-10-07 13:00 ` Richard Henderson
  0 siblings, 2 replies; 15+ messages in thread
From: Dayeol Lee @ 2019-10-07  5:28 UTC (permalink / raw)
  To: qemu-devel
  Cc: open list:RISC-V TCG CPUs, Sagar Karandikar, Dayeol Lee,
	Bastian Koppelmann, Palmer Dabbelt, Alistair Francis

riscv_cpu_tlb_fill() uses the `size` parameter to check PMP violation
using pmp_hart_has_privs().
However, the size passed from tlb_fill(), which is called by
get_page_addr_code(), is always a hard-coded value 0.
This causes a false PMP violation if the instruction presents on a
PMP boundary.

In order to fix, simply correct the size to 4 if the access_type is
MMU_INST_FETCH.

Signed-off-by: Dayeol Lee <dayeol@berkeley.edu>
---
 target/riscv/cpu.h        | 1 +
 target/riscv/cpu_helper.c | 9 ++++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0adb307f32..386c80e764 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -88,6 +88,7 @@ enum {
 #define MMU_USER_IDX 3
 
 #define MAX_RISCV_PMPS (16)
+#define RISCV_INSN_LENGTH 4
 
 typedef struct CPURISCVState CPURISCVState;
 
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index e32b6126af..877e89dbf2 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -441,6 +441,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
     CPURISCVState *env = &cpu->env;
     hwaddr pa = 0;
     int prot;
+    int pmp_size = 0;
     bool pmp_violation = false;
     int ret = TRANSLATE_FAIL;
     int mode = mmu_idx;
@@ -460,9 +461,15 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                   "%s address=%" VADDR_PRIx " ret %d physical " TARGET_FMT_plx
                   " prot %d\n", __func__, address, ret, pa, prot);
 
+    if (access_type == MMU_INST_FETCH) {
+      pmp_size = RISCV_INSN_LENGTH;
+    } else {
+      pmp_size = size;
+    }
+
     if (riscv_feature(env, RISCV_FEATURE_PMP) &&
         (ret == TRANSLATE_SUCCESS) &&
-        !pmp_hart_has_privs(env, pa, size, 1 << access_type, mode)) {
+        !pmp_hart_has_privs(env, pa, pmp_size, 1 << access_type, mode)) {
         ret = TRANSLATE_PMP_FAIL;
     }
     if (ret == TRANSLATE_PMP_FAIL) {
-- 
2.20.1



^ permalink raw reply related	[flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-10-23 16:15 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-11 23:14 [PATCH] target/riscv: PMP violation due to wrong size parameter Dayeol Lee
2019-10-12  2:37 ` Jonathan Behrens
2019-10-12 18:30   ` Dayeol Lee
2019-10-15 17:04     ` Dayeol Lee
2019-10-18 19:01       ` Palmer Dabbelt
2019-10-18 19:28         ` Dayeol Lee
  -- strict thread matches above, loose matches on Subject: below --
2019-10-22 21:21 Dayeol Lee
2019-10-23 15:50 ` Palmer Dabbelt
2019-10-07  5:28 Dayeol Lee
2019-10-07  6:20 ` no-reply
2019-10-07 13:00 ` Richard Henderson
2019-10-07 17:19   ` Dayeol Lee
2019-10-07 18:25     ` Richard Henderson
2019-10-07 18:41       ` Dayeol Lee
2019-10-08  3:18         ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).